Thank y'all for watching! ❤️ Do you want to see more optimization/performance stuff? Also don't forget that the first 1,000 people to use this link will get a 1 month free trial of Skillshare: skl.sh/thecherno12211
Just a couple of comments: - I am puzzled by the slowdown in accumulate! I remember testing it against a raw for loop in simpler usecases and it even turned out a few % faster. Granted, it was on gcc so it could be MSVC related, but somehow that would be equally surprising :\ - I don't like the whole 100% object oriented theme of the project as well, but I decided to follow the original book in that regard because I wanted to do everything in a weekend, and changing the whole architecture would have taken way longer :)
Glad you saw it! Was about to send you an email asking if you had these performance issues as well, I had a feeling it might be compiler/library-related. Thanks for sending in your code!
@Harry Byrne Well, the goal was to follow the original book and I wanted to be done within the weekend! So this was my attempt to get familiar with the topic in a reasonable amount of time :) I'd like to solve the big issues first (design a better scene model, get rid of all the unnecessary hierarchy and so on...) before getting on the GPU, which is a completely unexplored land for me!
@@riidefi1575 A unwanted copy of the shared_ptr would explain a lot (with the control block being copied for every ptr in the container...) But the lambda takes the arguments as const& so I think this is up to the library implementer to make sure things are forwarded correctly to the function, right? Maybe this is why I never had problems with gcc and clang libraries... definitely gonna check this out later on
Fun fact! std::accumulate actually did have a bug in it that was fixed in C++20! (See the #if _HAS_CXX20 clause at 24:59). std::accumulate was originally meant to accumulate small objects, so it passes things by value. As of C++20 it moves the values since they could be large. I'd be curious to see the performance improvement just by compiling with C++20! As a side note, I'd say that this operation, finding the closest object, doesn't really accumulate anything, so although accumulate can do the job, I would argue that a for loop expresses the intent better anyway.
As someone who does FP a lot, I feel like it does accumulate: it accumulates the minimum distance for a ray bounce. But maybe I'm just mind-poisoned by HOFs?
@@scoreunder Hahah I wouldn't say you're _poisoned_ by functional programming, perhaps enlightened? Of course, with higher order functionals, you can build very unexpected/general things out of some basic building blocks. But from a more grounded perspective, it seems unlikely that a highly optimized algorithm could remain optimized for an arbitrary functional as input. Personally, I find that if the lambda you pass in is larger in size than the algorithm itself, then it is highly likely you will find a performance hit waiting for you on the other end. I most often prioritize readability and I also find that when the intent of something becomes unclear (perhaps a _clever_ lambda to an algorithm) the optimizer will also not do very well. Don't get me wrong though, I'd love everyone take the opportunity to learn some category theory.
Why std::accumulate at all if you want to do std::for_each?? Typical „I don’t understand the new stuff, so I stick to the old“. He even thinks „range based loop“ is different from „old style for loop“. If so, he should get a working compiler.
@@fromgermany271 I may be misunderstanding what you mean, but several modern Cpp books (notably Accelerated C++) stress the semantic value of std:: accumulate. In other words, std::for_each doesn't tell us the intent if the code; std:: accumulate does (it's a summation).
@@VastyVastyVoid I meant: he has a problem with std::accumulate and says „to slow“ and „classic loop is faster“. I say, accumulate changes a „sum“, that does not really fit to „parallel“. But I cannot see why a „sum“ should be needed at all. But in general saying „I don’t understand something in stdlib, so I just fall back to C-style“ is something not uncommon, but not applicable for talks.
I for one would love to see more optimisation stuff. I'm currently in the process of learning how to write high performance code, so learning how to self evaluate a program's performance via either code inspection or using tools to develop metrics will be huge for me. That said, I'm just happy to see a Cherno video in my sub box either way, so thanks for taking the time to create and upload this video.
Btw. a really nice way that helps is to use a very slow chip that restricts you a lot, like for example an Arduino where you get hit hard if you code inefficiently.
A big part of optimization is similar to this video, finding slow parts, and cutting out unnecessary overhead. Giving the CPU less work. That and keeping cache sizes and layout in mind, which is easier to consider if using simpler methods with less abstraction. Modern desktop CPUs tend to max out at 32MB L3 cache, but it is better to try to stay within 8MB.
It's mostly what you see here - use a profiler, because often there's something bad but hidden somewhere non-obvious. (It's often not worth even guessing like he did, start with the profiler to avoid biases.) Avoid unneeded copies. Avoid replicating the same expensive work. And in C/Rust, prefer stack over heap. It's much much further down the line that it makes sense to start worrying in more detail about cache implementation details. But don't forget readability is critical. Your future self will appreciate it.
For the slowness of `std::accumulate` I'm sure it's down to the copy of the `std::shared_ptr` inside the HitRecord, which do atomic operations to keep the refcount up to date. In C++20, it's not copying it anymore. The best would be to not use shared pointer at all.
That's what I thought too, and made a little proof myself, by cheating and making it (for a bit just plain pointer), then std::accumulate was still fine (C++17) - with 20 and std::move() on it - even better.
@@Narblo It's not that, it's really lots of "lock xadd" (atomic inc/dec due to shared_ptr copying). Granted that was on my machine, with my RAM, my CPUs, etc. - it may be better on others, but maybe even much worse (with say NUMA configs). For in house game editor we had to disable one of the CPU's in it to avoid that, as we've been also guilty of over-using abit more of shared_ptr - though not like this.
Hello Yan, I would love to see you optimize this code. The fact alone that replacing one single function with a for loop can yield such a performance boost shows how good this code is. It is definitely a great starting point for a video about optimization. Given the fact that the accumulate function was written in order to be used in cases like this, it being so slow isn ´t something that a coder should have to worry about. It ´s a language related problem. You have already started talking about playing to the strengths of the hardware architecture. This performance boost was possible because the coder didn ´t know what the accumulate function actually does. You might even go as far as to explain the different features of 32 bit and 64 bit processors and assembly language and then go back to explaining how c++ as a language actually utilizes them. That would be highly informative, albeit really time consuming, but this very code example shows how easy it can be to go from a working piece of software to a really satisfying experience for both coders and users if you are aware of the inner workings of your tools. In short: Please do it. I love your content, by the way, and have been a subscriber for a long time. All the best to you and your loved ones from Germany.
Only the really bottlenecks need to be optimized. I think Yan tackled that well enough via the profiler. And assembly is diminishing returns these days compared to intrinsics and modern compilers that automatically find intrinsic equivalents.
These aren't just code reviews, but absolute masterclasses as to why one approach wins over another. I've learnt so much in this 38 minutes than I've learn in the last 5 years!
also learning the methodology of diagnosing inefficiencies. the way he does it here for a c program still applies to all languages "break down to verbose and deliberate to see which step is causing the problem".
In my experience, when changing a single part of the code can speed things up so much, the problem is almost always some kind of superfluous copy somewhere. Also, an additional optimization video would be really nice!
Less unnecessary copying and more usages of move semantics or using references to objects definitely helps towards improving performance and efficiency making the code already pre-optimized.
This is exactly the content i wanted to see. Old school coder myself, i struggle a bit on learning modern c++ stuff on my free time, so it's great to see that raw quick comparison. The explanation and clarity from Cherno is hard to match on any other material you might find on the web. Loving it. ❤
The speed you analyzed the whole project is impressive! Raytracer is one of the projects that I want to do in the future. At the same time I really love optimization videos so I cannot wait to see the continuation :^)
2 года назад+4
I would love to see you go through and refactor/optimize the code (get rid of the std usage where it's superfluous etc). Someone also mentioned C# and using that to achieve similar results would be really cool. Span was added not too long ago to help with stack-allocations without having to use unsafe and stackalloc.
I think it would be an amazing series, take this one codebase, one episode going deeper on why virtuals were bad in this case, de-OOPing it, a longer one bringing misc stuff closer to the metal, and the final one doing it on a GPU.
10:02 It is so funny to see your cam video freezing while rendering that slow raytracing image. 😂 It's like being a party host kicked out of his own house.
This was a great video! As a person who just rewrote a pure C++ project in a more C-style C++ I can relate. I was taught that the explicitness of C is inherently bad and it was an eyeopener to realize this is not the case.
Very true! C is blazingly fast, C++ is well organized. There is a performance tradeoff in the organizing, forcing memory swaps etc., so would always do a back of the envelope performance calculation to be sure I wasn't wasting cpu. I started in C in '86 when we didn't have CPU to spare! So performance has always been in mind, and the success of many projects came down to that as they grew in size and complexity.
Hey Cherno. Question: I know this channel is mainly focused on C++, but I was wondering if you are also accepting code that is made in for example C# and uses like some wrapper for OpenGL? Edit: I finished the Video and yes watching you make this RayTracer faster would be cool.
No, the optimization part is very interesting, helpful and useful. I think continuing down this road is a great course of action to help others to improve their own code bases / frameworks. It should help to give them insight towards their own approaches on how to analyze and profile their own code, what to look for, and how to optimize or improve its performance. Great Video!
with the std::unique_ptr wrapping a buffer like that - the point of it, and what you're getting here - is exception safety and a guarantee of no use-after-free or forgotten delete call during object destruction - two things that given the multi-threaded nature of this project, are quite important to get right and impossible to do with raw pointers.
I love unique pointer wrapping array, but only if this is necessary. Normally I would use a vector, but when I'm implementing something much lower level. In my codebase you'll see some `std::unique_ptr` and I use it to implement a vector of a runtime defined structure. Mainly to contains all uniforms for a list of objects to render, or to initialize a structure of buffer objects for opengl.
I'd love to see both of your own fully optimised version of this implementation as well as a GPU based "ray tracing in a weekend". I think they would both be fascinating.
Since you asked for opinions about the unique_ptr array, here's how I handle it. If I'm needing an array where I need to push/pop from the back only, I use std::vector. If I'm needing a buffer, I make a buffer class and still use the std::unique_ptr as the data holder. This allows me to focus on adding functionality without having write all the extra code like deleting the copy ctor and copy assignment operators, nor would I have to write the move ctor or move assignment operator, nor would I have to write a destructor. With std::vector, you get copy and move for free, but then you have to write the two lines to delete the copy ctor and copy assignment operator. As for wrapping a naked pointer, you have to write all those yourself including destructor. Not using std::unique_ptr for a heap allocated array is more work compared to a naked pointer, even if utility wrappers like this isn't the bulk of your program.
The problem is not std::accumulate. It is the fact that iterating an accumulate with std::optional make s a "copy" of it every pass of the accumulate. Accumulate should be used with trivial copy/move elements, or ensuring the elements handle properly the move.
A possible suggestion for video format, in the case you want a more complete exploration while not being a 2 hour video; Record your thoughts during the beginning, middle, and end of the process. Beginning - overall impressions of what the weeds will look like, guesses at what needs to change, etc Middle - what the weeds *actually* look like, things that have and haven't surprised you so far. Examples of transforms done. Also kinda fun to see whether you can guess where the 'middle' would be and if you're accurate on that by the end. End - obviously get to discussion the final results and overall learnings
He mentioned not liking the usage of unique_ptr for a buffer, and preferring vector instead. vector gives you a size, which is nice and a bit less implicit, and resize() is a bit more terse. Though there is an overload of make_unique for arrays that takes an array size (e.g. make_unique(1024) or what not). It didn’t look like that was used in the given code, but it’s fairly terse too. One big difference in some contexts is that unique_ptr is a much more lightweight template than vector. I had some heavily templated code with a fixed but dynamically sized array where switching from vector to unique_ptr cut the compile time in half. This code didn’t look to be heavily templated so it likely doesn’t matter, but it’s useful to know.
In most cases, I would avoid std::unique_ptr instance and instead use a std::vector. The only case where I would prefer unique_ptr is if the `any_type` is '`bool`. For most implementations std::vector is space optimized and creates issues when we need a raw pointer on it to pass on.
One downside to vector is that it allows easy resizing on the fly, which may not be something you want. If you need to keep a long-lived pointer to something inside the buffer, using a vector may not be ideal because if someone pushes to it, it may result in the internal data storage being copied to a new location and then freed, leaving you with a subtle use-after-free bug. That's still possible if you use a unique_ptr, but it will be much more obvious because you have to explicitly replace the entire buffer, as opposed to just calling push_back or emplace_back. I don't think there's anything wrong with using unique_ptr for a situation like this; it's basically a safer version of a C-style pointer-to-buffer. A vector serves a different purpose, as a dynamically-sized collection of items. If you don't need the resizing capabilities, why use the more complex type?
I use unique_ptr quite a lot. It's a nice pattern for when you want a non-resizable vector with size specified at runtime. The API is somewhat worse than a vector, but you can certainly wrap it into a nicer type (like an owning version of the buffer helper you describe in this video).
about the accumulate, I dont know what's going on either but it seems like lambda functions just absolutely destroy performance when called very often. There must be some overhead in starting up, finding or setting up lambda functions. Would be interesting to know what happens under the hood with lambda functions. Maybe they cant be inlined by the compiler?
Yeah, at this point that would be my guess as well, and this might be MSVC related. I never saw anything like this on gcc and clang... Visual studio ships also with clang IIRC, so it should be easy for me to see if that's the case
I think wrapping a uint8_t[] inside a unique_ptr is really good because unique_ptr will guarantee that the memory is freed. However, in this use case there should be no downside to just using std::vector. Unless you resize the vector it will behave the same way and it has the benefit that you don't need to store the size elsewhere. The only use case where you can't use vector for something like that is if your element type is not copy constructable (iirc the template instantiation of vector would fail) or if you want a vector of bools. Because yknow vector is some cursed template specialization that should've never be brought into the standard.
21:3824:54 Hi Cherno! I would always use a loop in this case, rather than overengineering it with std algorithms. But I would say that std::accumulate is fine for C++ compilers, don't be afraid it. When it compiled with debug mode, it will not be optimized. But it works well under the release setting. With release setting / -O2 flag, it does get inlined and unrolled into a loop.
I feel like I could have done a better job of maximizing that metric. Granted, all my wtfs are in a single function call so my wtf density is through the roof.
DEFINITELY A LIKE! Big! THANK YOU! The point of OPTIMIZING vs OOP reminds me of John Carmack's wild "Inverse Square Root" logic in Doom. Do the math not the objects...
Go faster!! See how much improvements you can make! Would love to see that! And also I would have also loved to know what was the time using the improved code with the sampling rate and bounce set to the original, 500 and 50 I think...
Time can be attributed to the wrong line of code when profiling an optimized build. If the profiler can't figure out which specific line of code is spending time because the optimization mixed up all the lines, then it could just point to the overall function.
Watching you read through somebody else's code like it's a kindergarten book is so crazy to me. I forget how to read my own projects after a month of inactivity.
25:20 I don't know how expensive it is to copy a sphere, but I would currently guess that is the culprit. Yes, when accumulate calls the lambda, it copies the sphere. As others said, this changed a bit to C++20.
Taking the step to running on the GPU would be very interesting. I'd definitely watch a video (or series) of you coding up a more optimized implementation.
i only ever read one book about c++, but never used what i learend in a software. i am so impressed, that he is able to grasp what's going on so quickly. reading someone elses code is so hard for me.
Thank's Cherno! As always, great video. Definitely interested to see you further optimize the code and squeeze out a better performance. Also I would love to see you porting that to a compute shader.
In case someone wonders whats going on: std::accumulate is the wrong function in this case because it assigns the returned value to temp_value on every iteration. So you are copying all the data in HitRecord for every iteration (unlike in the explicit loop). I am not sure why, but it looks like default assignment/constructors dont show up in the profiler. std::optional is really just a bool + sizeof(T) and constructs T inplace. Performance of it should be identical (or better) than using an "invalid state" value.
You are the second one immediately spotting the problem on the accumulate. Really my bad for not even thinking about it, but that should really have been a raw loop. Also, you're spot on on optional. I don't think it can introduce much overhead, but Yan is right in noting that this leads to more branches. But maybe that's because of the design rather than optional itself
@@vale1286 He is right that it adds extra branches but I disagree with him in that it's a problem. The optional is already in cache and nearby pixels have identical results making the if essentially a noop because the branch predictor is gonna hit in most cases. There are a number of things I don't like about the code (e.g. a chunksize of 1 pixel is way to small, since you'll get bottleneck by the queue and most of the shared_ptr could be unique_ptr's or variants) but in case of the optional usage I really think the improvement in code clarity really outweighs the (very minor) performance impact.
@@Thalhammer1996 that was the main reason I went for optional in the first place, I made a vow to never use function arguments as outputs :) For the rest I agree 100%, as I wrote elsewhere I didn't rework the architecture from the book because I wanted to keep this within a weekend. Now that I had more time to think I have a few ideas on how to redesign the scene (and variants are one of the things I want to try) so maybe I'll work on it this weekend
@@vale1286 You'll probably wanna get rid of the vast majority of virtual functions. When going after performance you want to give the compiler as much explicit information as possible. Since e.g. the number of materials is gonna be limited (most materials are just a variation of parameters of a base materials) there's no point in using a virtual function there. Instead you want a variant of all material types and std::visit. virtual functions are bad for a number of reasons. The most important being cache misses and preventing inline.
Thank you for delivering such a masterpiece video, I love it so much and learned a lot! I am as motivated as you do, just planning to write my own raytracer using compute shader next month!
Reason std::accumulate is slow here: -Use of std::shared_ptr (gets copied before C++20) -Use of std::optional (requires branching and (often) uses very, very padded memory like types double in size) -Lambdas are not always inlined -Likely no vectorization because of function call to lambda I would also try manual vectorization (with raw arrays or std::vector) out of curiosity. Probably could render like 40+ frames/second.
It would be interesting to see the assembly code from the profiling. I suspect there's something "interesting" happening when calling the lambda function (as someone writing compiler code and looking at code generated a lot, I suspect this is what happens!)
Very interesting video. I would like to see more videos where code is checked for issues related to speed, crashing, freezing, etc. A lot of us are just hacking stuff together with no real understanding of why problems occur such as a window freezing for 10 seconds while performing an operation.
OH my gosh I remember watching you and learning some programming from you when I was still in primary school. You made me pursuit programming and suddenly seeing one of your videos in my feed brought back a ton of memories of me trying to figure out Java as a tiny kid. Now I'm studying cybersecurity and software development, working my way through!
Summing HitRecords doesn't really make sense - they're structs with no real way to "sum" them. The std::accumulate in the original implementation was simply choosing between two HitRecords and either keeping the old one or replacing the result, depending on the ray hit distance. Additionally I ran an image diff after recording this video (just to make sure) - aside from noise and other random scattering built into the renderer, the results are identical.
@@TheCherno Thought it was odd it didn't trigger a compile error. Turns out std::accumulate relies on the lambda to do the accumulating by passing in the accumulated value (temp_value). That value gets passed in, you're meant to add to it and return the accumulated value for each iteration. Obviously, "our" lambda isn't doing that. As you were. :)
@@TheCherno Have done some testing. Using the lambda in a standard for loop gets the same slow result - so it's not std::accumulate. However, changing the lambda to return std::optional() on no hits fixes it. Change it back to return temp_value.. slow. Really strange.
@@TroySchrapel Maybe we're losing time by copying HitRecord all the time? The lambda takes the accumulated value by const ref, then returns it by value, so for every iteration, there is an unnecessary copy of a non-empty std::optional.
@@szaszm_ Yeah. It is. Specifically the shared_ptr of mat_ptr. Replacing that with a regular pointer also fixes the issue. Also, replacing all instances of std::optional with std::shared_ptr fixes the issue.
Would like to see a follow-up video on optimizing the code, including checking the assembly and if the compiler doesn't make use of SIMD already, then also something about that for the hot path.
8:59 - There are cases where I might use a unique_ptr in that way, but I agree, in almost all cases I'd prefer vector. Maybe he didn't think of using resize on the vector to pre-allocate all the space or something. Though, there is a problem there because that will insist on initializing all the memory too, which might not be what you want. Though then it seems like you could just use reserve instead and then add the elements one-by-one. Hmmm...
It's really, really key to use profilers for this kind of thing. More often then not "optimized code" is a completely different beast to what the rest of your project will/should look like. You can end up writing some really hard to read/ugly stuff so its really important to only do it in the hot zones. Also "premature optimization" is the route of all evil :p i've seen code that starts using bit shifting and all sorts "to be faster" and then in the very next code block reallocates an array inside a for loop that could have happily lived outside it :p But its an interesting art, particularly on GPUs, like you would be surprised how often you can replace an if statement with pure maths that will achieve the same result and will be so much faster without the conditional. (this is true of standard programing too but more applicable in GPU stuff)
Thank y'all for watching! ❤️ Do you want to see more optimization/performance stuff? Also don't forget that the first 1,000 people to use this link will get a 1 month free trial of Skillshare: skl.sh/thecherno12211
Nice
I want more optimization stuff so we can implement it in our own work
I definitely would like to see more performance related stuff
this topic is so interesting and you explain it very good :) thx!
Hi :), just stop asking and record additional video RIGHT NOW!!! :). Your skills are incredibly insane. Pleeease mooooooore... 😅
Man, I never thought you would pick my project! Definitely made my day!
Just a couple of comments:
- I am puzzled by the slowdown in accumulate! I remember testing it against a raw for loop in simpler usecases and it even turned out a few % faster. Granted, it was on gcc so it could be MSVC related, but somehow that would be equally surprising :\
- I don't like the whole 100% object oriented theme of the project as well, but I decided to follow the original book in that regard because I wanted to do everything in a weekend, and changing the whole architecture would have taken way longer :)
Glad you saw it! Was about to send you an email asking if you had these performance issues as well, I had a feeling it might be compiler/library-related. Thanks for sending in your code!
@Harry Byrne Well, the goal was to follow the original book and I wanted to be done within the weekend! So this was my attempt to get familiar with the topic in a reasonable amount of time :)
I'd like to solve the big issues first (design a better scene model, get rid of all the unnecessary hierarchy and so on...) before getting on the GPU, which is a completely unexplored land for me!
@@riidefi1575 A unwanted copy of the shared_ptr would explain a lot (with the control block being copied for every ptr in the container...)
But the lambda takes the arguments as const& so I think this is up to the library implementer to make sure things are forwarded correctly to the function, right? Maybe this is why I never had problems with gcc and clang libraries... definitely gonna check this out later on
@Harry Byrne Yes, I am! I'll gladly take any help I can get :)
Fun fact! std::accumulate actually did have a bug in it that was fixed in C++20! (See the #if _HAS_CXX20 clause at 24:59). std::accumulate was originally meant to accumulate small objects, so it passes things by value. As of C++20 it moves the values since they could be large. I'd be curious to see the performance improvement just by compiling with C++20! As a side note, I'd say that this operation, finding the closest object, doesn't really accumulate anything, so although accumulate can do the job, I would argue that a for loop expresses the intent better anyway.
As someone who does FP a lot, I feel like it does accumulate: it accumulates the minimum distance for a ray bounce. But maybe I'm just mind-poisoned by HOFs?
@@scoreunder Hahah I wouldn't say you're _poisoned_ by functional programming, perhaps enlightened? Of course, with higher order functionals, you can build very unexpected/general things out of some basic building blocks. But from a more grounded perspective, it seems unlikely that a highly optimized algorithm could remain optimized for an arbitrary functional as input. Personally, I find that if the lambda you pass in is larger in size than the algorithm itself, then it is highly likely you will find a performance hit waiting for you on the other end. I most often prioritize readability and I also find that when the intent of something becomes unclear (perhaps a _clever_ lambda to an algorithm) the optimizer will also not do very well. Don't get me wrong though, I'd love everyone take the opportunity to learn some category theory.
Why std::accumulate at all if you want to do std::for_each??
Typical „I don’t understand the new stuff, so I stick to the old“.
He even thinks „range based loop“ is different from „old style for loop“. If so, he should get a working compiler.
@@fromgermany271 I may be misunderstanding what you mean, but several modern Cpp books (notably Accelerated C++) stress the semantic value of std:: accumulate. In other words, std::for_each doesn't tell us the intent if the code; std:: accumulate does (it's a summation).
@@VastyVastyVoid
I meant: he has a problem with std::accumulate and says „to slow“ and „classic loop is faster“.
I say, accumulate changes a „sum“, that does not really fit to „parallel“. But I cannot see why a „sum“ should be needed at all.
But in general saying „I don’t understand something in stdlib, so I just fall back to C-style“ is something not uncommon, but not applicable for talks.
You can tell this is a physicist's code because they're used to wait weeks for simulation results... 😂
Malappropriation of physics, then, as programming is also physics.
too much philosophy, not enough practicality@@colonthree
Ahh, physicists bashing, I'm in. 😂
I for one would love to see more optimisation stuff. I'm currently in the process of learning how to write high performance code, so learning how to self evaluate a program's performance via either code inspection or using tools to develop metrics will be huge for me.
That said, I'm just happy to see a Cherno video in my sub box either way, so thanks for taking the time to create and upload this video.
Btw. a really nice way that helps is to use a very slow chip that restricts you a lot, like for example an Arduino where you get hit hard if you code inefficiently.
A big part of optimization is similar to this video, finding slow parts, and cutting out unnecessary overhead. Giving the CPU less work. That and keeping cache sizes and layout in mind, which is easier to consider if using simpler methods with less abstraction. Modern desktop CPUs tend to max out at 32MB L3 cache, but it is better to try to stay within 8MB.
It's mostly what you see here - use a profiler, because often there's something bad but hidden somewhere non-obvious. (It's often not worth even guessing like he did, start with the profiler to avoid biases.) Avoid unneeded copies. Avoid replicating the same expensive work. And in C/Rust, prefer stack over heap.
It's much much further down the line that it makes sense to start worrying in more detail about cache implementation details.
But don't forget readability is critical. Your future self will appreciate it.
@@TimeFadesMemoryLasts Sorry im missing something do you mean running with the whole ide or just the compiled code itself?
For the slowness of `std::accumulate` I'm sure it's down to the copy of the `std::shared_ptr` inside the HitRecord, which do atomic operations to keep the refcount up to date. In C++20, it's not copying it anymore. The best would be to not use shared pointer at all.
That's what I thought too, and made a little proof myself, by cheating and making it (for a bit just plain pointer), then std::accumulate was still fine (C++17) - with 20 and std::move() on it - even better.
Yes and we could also have use std::reduce and use C++17 parallele algorithm to get the code vectorized easily
this should be pinned. std::accumulate is in no way the problem. Shared_ptr is.
Could be also the lambda call where here is just a loop. The jump may mess the cache unless the compiler is good enough to inline the lambda in
@@Narblo It's not that, it's really lots of "lock xadd" (atomic inc/dec due to shared_ptr copying). Granted that was on my machine, with my RAM, my CPUs, etc. - it may be better on others, but maybe even much worse (with say NUMA configs). For in house game editor we had to disable one of the CPU's in it to avoid that, as we've been also guilty of over-using abit more of shared_ptr - though not like this.
With 7.5 minutes of rendering time, the objective "Raytracing in a weekend" had already been achieved by a large margin 😊
Hello Yan,
I would love to see you optimize this code. The fact alone that replacing one single function with a for loop can yield such a performance boost shows how good this code is. It is definitely a great starting point for a video about optimization. Given the fact that the accumulate function was written in order to be used in cases like this, it being so slow isn ´t something that a coder should have to worry about. It ´s a language related problem. You have already started talking about playing to the strengths of the hardware architecture. This performance boost was possible because the coder didn ´t know what the accumulate function actually does. You might even go as far as to explain the different features of 32 bit and 64 bit processors and assembly language and then go back to explaining how c++ as a language actually utilizes them. That would be highly informative, albeit really time consuming, but this very code example shows how easy it can be to go from a working piece of software to a really satisfying experience for both coders and users if you are aware of the inner workings of your tools. In short: Please do it. I love your content, by the way, and have been a subscriber for a long time. All the best to you and your loved ones from Germany.
Its not the function, but the copy of the shared pointer that causes the slow down
@@phantom_stnd Exactly this.
Only the really bottlenecks need to be optimized. I think Yan tackled that well enough via the profiler. And assembly is diminishing returns these days compared to intrinsics and modern compilers that automatically find intrinsic equivalents.
These aren't just code reviews, but absolute masterclasses as to why one approach wins over another. I've learnt so much in this 38 minutes than I've learn in the last 5 years!
What have you been doing the last 5 years? Did you have internet?
@@yohannes2kifle Too busy churning out products and applications! 😉
also learning the methodology of diagnosing inefficiencies. the way he does it here for a c program still applies to all languages "break down to verbose and deliberate to see which step is causing the problem".
Please a optimization on this code! It would be very entertaining and educational for everybody! I personally would love to see that video!!!
In my experience, when changing a single part of the code can speed things up so much, the problem is almost always some kind of superfluous copy somewhere. Also, an additional optimization video would be really nice!
Less unnecessary copying and more usages of move semantics or using references to objects definitely helps towards improving performance and efficiency making the code already pre-optimized.
This is exactly the content i wanted to see. Old school coder myself, i struggle a bit on learning modern c++ stuff on my free time, so it's great to see that raw quick comparison. The explanation and clarity from Cherno is hard to match on any other material you might find on the web. Loving it. ❤
The speed you analyzed the whole project is impressive! Raytracer is one of the projects that I want to do in the future. At the same time I really love optimization videos so I cannot wait to see the continuation :^)
I would love to see you go through and refactor/optimize the code (get rid of the std usage where it's superfluous etc). Someone also mentioned C# and using that to achieve similar results would be really cool. Span was added not too long ago to help with stack-allocations without having to use unsafe and stackalloc.
I think it would be an amazing series, take this one codebase, one episode going deeper on why virtuals were bad in this case, de-OOPing it, a longer one bringing misc stuff closer to the metal, and the final one doing it on a GPU.
10:02 It is so funny to see your cam video freezing while rendering that slow raytracing image. 😂 It's like being a party host kicked out of his own house.
This was a great video! As a person who just rewrote a pure C++ project in a more C-style C++ I can relate. I was taught that the explicitness of C is inherently bad and it was an eyeopener to realize this is not the case.
Very true! C is blazingly fast, C++ is well organized. There is a performance tradeoff in the organizing, forcing memory swaps etc., so would always do a back of the envelope performance calculation to be sure I wasn't wasting cpu. I started in C in '86 when we didn't have CPU to spare! So performance has always been in mind, and the success of many projects came down to that as they grew in size and complexity.
Please do more of this!!
"I could have *written* a ray tracer in that amount of time..." GOLDEN! :)
Hey Cherno. Question:
I know this channel is mainly focused on C++, but I was wondering if you are also accepting code that is made in for example C# and uses like some wrapper for OpenGL?
Edit: I finished the Video and yes watching you make this RayTracer faster would be cool.
Of course - I've got quite a bit of C# experience as well, and anything graphics related is always good!
@@TheCherno hey man, how about rust?
@@YoTengoUnLCD rust is dead, it shall never excel c++. It's just overhyped.
@@iXNomad who asked?
@@YoTengoUnLCD it's a joke
No, the optimization part is very interesting, helpful and useful. I think continuing down this road is a great course of action to help others to improve their own code bases / frameworks. It should help to give them insight towards their own approaches on how to analyze and profile their own code, what to look for, and how to optimize or improve its performance. Great Video!
Can you do a reaction to the new Matrix Awakens PS5 experience? I’m curious what your take is on the engine demo from a programmers point of view.
with the std::unique_ptr wrapping a buffer like that - the point of it, and what you're getting here - is exception safety and a guarantee of no use-after-free or forgotten delete call during object destruction - two things that given the multi-threaded nature of this project, are quite important to get right and impossible to do with raw pointers.
I love unique pointer wrapping array, but only if this is necessary. Normally I would use a vector, but when I'm implementing something much lower level.
In my codebase you'll see some `std::unique_ptr` and I use it to implement a vector of a runtime defined structure. Mainly to contains all uniforms for a list of objects to render, or to initialize a structure of buffer objects for opengl.
I'd love to see both of your own fully optimised version of this implementation as well as a GPU based "ray tracing in a weekend". I think they would both be fascinating.
33:52 Yesssss you king
Since you asked for opinions about the unique_ptr array, here's how I handle it.
If I'm needing an array where I need to push/pop from the back only, I use std::vector.
If I'm needing a buffer, I make a buffer class and still use the std::unique_ptr as the data holder. This allows me to focus on adding functionality without having write all the extra code like deleting the copy ctor and copy assignment operators, nor would I have to write the move ctor or move assignment operator, nor would I have to write a destructor. With std::vector, you get copy and move for free, but then you have to write the two lines to delete the copy ctor and copy assignment operator. As for wrapping a naked pointer, you have to write all those yourself including destructor. Not using std::unique_ptr for a heap allocated array is more work compared to a naked pointer, even if utility wrappers like this isn't the bulk of your program.
8:51 a pointer to an array looks like a double pointer to me...
yeah,it does
The problem is not std::accumulate.
It is the fact that iterating an accumulate with std::optional make s a "copy" of it every pass of the accumulate.
Accumulate should be used with trivial copy/move elements, or ensuring the elements handle properly the move.
I want a Cherno, who does such videos with the Rust language 🦀. But still love it.
A possible suggestion for video format, in the case you want a more complete exploration while not being a 2 hour video;
Record your thoughts during the beginning, middle, and end of the process.
Beginning - overall impressions of what the weeds will look like, guesses at what needs to change, etc
Middle - what the weeds *actually* look like, things that have and haven't surprised you so far. Examples of transforms done. Also kinda fun to see whether you can guess where the 'middle' would be and if you're accurate on that by the end.
End - obviously get to discussion the final results and overall learnings
Thoughts on the new un5 matrix demo
He mentioned not liking the usage of unique_ptr for a buffer, and preferring vector instead.
vector gives you a size, which is nice and a bit less implicit, and resize() is a bit more terse. Though there is an overload of make_unique for arrays that takes an array size (e.g. make_unique(1024) or what not). It didn’t look like that was used in the given code, but it’s fairly terse too.
One big difference in some contexts is that unique_ptr is a much more lightweight template than vector. I had some heavily templated code with a fixed but dynamically sized array where switching from vector to unique_ptr cut the compile time in half. This code didn’t look to be heavily templated so it likely doesn’t matter, but it’s useful to know.
it's almost 2022, and we stil waiting for the build system with git configuration video. Sadge.
Bakko: It's multithreaded 😎
Cherno: 👁👄👁
Keep going. Let's see how far you can take it!
glad to hear other people from Italy!
In most cases, I would avoid std::unique_ptr instance and instead use a std::vector. The only case where I would prefer unique_ptr is if the `any_type` is '`bool`. For most implementations std::vector is space optimized and creates issues when we need a raw pointer on it to pass on.
One downside to vector is that it allows easy resizing on the fly, which may not be something you want. If you need to keep a long-lived pointer to something inside the buffer, using a vector may not be ideal because if someone pushes to it, it may result in the internal data storage being copied to a new location and then freed, leaving you with a subtle use-after-free bug. That's still possible if you use a unique_ptr, but it will be much more obvious because you have to explicitly replace the entire buffer, as opposed to just calling push_back or emplace_back.
I don't think there's anything wrong with using unique_ptr for a situation like this; it's basically a safer version of a C-style pointer-to-buffer. A vector serves a different purpose, as a dynamically-sized collection of items. If you don't need the resizing capabilities, why use the more complex type?
I use unique_ptr quite a lot. It's a nice pattern for when you want a non-resizable vector with size specified at runtime. The API is somewhat worse than a vector, but you can certainly wrap it into a nicer type (like an owning version of the buffer helper you describe in this video).
about the accumulate, I dont know what's going on either but it seems like lambda functions just absolutely destroy performance when called very often. There must be some overhead in starting up, finding or setting up lambda functions. Would be interesting to know what happens under the hood with lambda functions. Maybe they cant be inlined by the compiler?
Yeah, at this point that would be my guess as well, and this might be MSVC related. I never saw anything like this on gcc and clang... Visual studio ships also with clang IIRC, so it should be easy for me to see if that's the case
@@vale1286 let us know if you do : )
They are inlined. The issue is the optional in conjunction with accumulate. A bug fixed when compiling in C++20 mode.
You should do a video on how to navigate VS, like shortcuts and what not. Your ability to navigate VS like it's vim is impressive
I love watching optimization. It helps my mind negotiate code decisions to do optimizations by default
"raytracing in a weekend" is actually how long it takes to render the full image on an average computer
😂😂😂😂😂🎉
I think wrapping a uint8_t[] inside a unique_ptr is really good because unique_ptr will guarantee that the memory is freed. However, in this use case there should be no downside to just using std::vector. Unless you resize the vector it will behave the same way and it has the benefit that you don't need to store the size elsewhere.
The only use case where you can't use vector for something like that is if your element type is not copy constructable (iirc the template instantiation of vector would fail) or if you want a vector of bools. Because yknow vector is some cursed template specialization that should've never be brought into the standard.
Congratulations on 420k subscribers. Celebrate it with a big fat blunt.
The optimization episodes are fun lol more please
21:38 24:54
Hi Cherno!
I would always use a loop in this case, rather than overengineering it with std algorithms. But I would say that std::accumulate is fine for C++ compilers, don't be afraid it. When it compiled with debug mode, it will not be optimized. But it works well under the release setting. With release setting / -O2 flag, it does get inlined and unrolled into a loop.
But developers should always use debug mode for everything. Don’t get into this mentality.
It will good to see you optimize that further.
as a dev of 20 years, its always fun to watch optimizations. its very satisfying to watch
Incredibly interesting! If you do end up making a follow up video, then I personally would like to see your take on CPU cache coherency.
i'd love you making a follow-up video
24:00 best measure for code quality during review: wtfs per second...
I feel like I could have done a better job of maximizing that metric.
Granted, all my wtfs are in a single function call so my wtf density is through the roof.
I THINK the problem with using accumulate here is it returns by value instead of by reference, so thats a LOT of copies being made
I would love to see you further improving this code!
DEFINITELY A LIKE! Big! THANK YOU! The point of OPTIMIZING vs OOP reminds me of John Carmack's wild "Inverse Square Root" logic in Doom. Do the math not the objects...
Inspirational! I'd love to see your optimization process
You know things are slow when the camera stutters XD.
I'd be interested in the optimization part. It would be interesting to dig in to see what exactly was slowing it down before.
Go faster!! See how much improvements you can make! Would love to see that! And also I would have also loved to know what was the time using the improved code with the sampling rate and bounce set to the original, 500 and 50 I think...
The frozen cherno just makes this video so much funnier than it should be
Time can be attributed to the wrong line of code when profiling an optimized build. If the profiler can't figure out which specific line of code is spending time because the optimization mixed up all the lines, then it could just point to the overall function.
Watching you read through somebody else's code like it's a kindergarten book is so crazy to me. I forget how to read my own projects after a month of inactivity.
😃
I don’t know what you’re saying, but you say it so well that I feel like I have to keep watching.
"...certainly he wasn't lying about the multithreading"... you cracked me
25:20 I don't know how expensive it is to copy a sphere, but I would currently guess that is the culprit.
Yes, when accumulate calls the lambda, it copies the sphere.
As others said, this changed a bit to C++20.
I would love for a series revolved around optimization of generic code (not just HAZEL)!
24:40 is me whenever I read someone else' code these days. I'm so glad I'm not the only one!
14:16 Never seen a programmer dropping so bars so hard
That was a very interesting code review to watch! Man your understanding on this topic is impressive, keep on the quality work
Taking the step to running on the GPU would be very interesting.
I'd definitely watch a video (or series) of you coding up a more optimized implementation.
Started following you recently because of a friend recommendation. I'd love to see more optimization videos. Loving your channel!!
i only ever read one book about c++, but never used what i learend in a software.
i am so impressed, that he is able to grasp what's going on so quickly. reading someone elses code is so hard for me.
Thank's Cherno! As always, great video.
Definitely interested to see you further optimize the code and squeeze out a better performance.
Also I would love to see you porting that to a compute shader.
This has been an excellent bloody episode Cherno! Please continue with this guys project! so so so informative and practical!
This Videos are amazing. Thanks to watching them I learn a bit of how to optimise my code and I don't even use C++.
Mixing aggregation with such a deep level of recursion might cause such overhead and for loops are way cheaper it seems, at the very least in C++.
yess bro iron out this project in another video. love hearing you optimize stuff and seeing this sort of thing from you!! awesome vid man
I would love it if you decide to make an additional optimization video. It is really important to optimize the code while doing project oneself.
"I think I could've WRITTEN a raytracer in that time..."
In case someone wonders whats going on: std::accumulate is the wrong function in this case because it assigns the returned value to temp_value on every iteration. So you are copying all the data in HitRecord for every iteration (unlike in the explicit loop). I am not sure why, but it looks like default assignment/constructors dont show up in the profiler. std::optional is really just a bool + sizeof(T) and constructs T inplace. Performance of it should be identical (or better) than using an "invalid state" value.
You are the second one immediately spotting the problem on the accumulate. Really my bad for not even thinking about it, but that should really have been a raw loop.
Also, you're spot on on optional. I don't think it can introduce much overhead, but Yan is right in noting that this leads to more branches. But maybe that's because of the design rather than optional itself
@@vale1286 He is right that it adds extra branches but I disagree with him in that it's a problem. The optional is already in cache and nearby pixels have identical results making the if essentially a noop because the branch predictor is gonna hit in most cases. There are a number of things I don't like about the code (e.g. a chunksize of 1 pixel is way to small, since you'll get bottleneck by the queue and most of the shared_ptr could be unique_ptr's or variants) but in case of the optional usage I really think the improvement in code clarity really outweighs the (very minor) performance impact.
@@Thalhammer1996 that was the main reason I went for optional in the first place, I made a vow to never use function arguments as outputs :)
For the rest I agree 100%, as I wrote elsewhere I didn't rework the architecture from the book because I wanted to keep this within a weekend.
Now that I had more time to think I have a few ideas on how to redesign the scene (and variants are one of the things I want to try) so maybe I'll work on it this weekend
@@vale1286 You'll probably wanna get rid of the vast majority of virtual functions. When going after performance you want to give the compiler as much explicit information as possible. Since e.g. the number of materials is gonna be limited (most materials are just a variation of parameters of a base materials) there's no point in using a virtual function there. Instead you want a variant of all material types and std::visit. virtual functions are bad for a number of reasons. The most important being cache misses and preventing inline.
@@Thalhammer1996 yeah that's exactly why I was thinking about variants. I used visit once in another project and I liked it
Thank you for delivering such a masterpiece video, I love it so much and learned a lot! I am as motivated as you do, just planning to write my own raytracer using compute shader next month!
Reason std::accumulate is slow here:
-Use of std::shared_ptr (gets copied before C++20)
-Use of std::optional (requires branching and (often) uses very, very padded memory like types double in size)
-Lambdas are not always inlined
-Likely no vectorization because of function call to lambda
I would also try manual vectorization (with raw arrays or std::vector) out of curiosity. Probably could render like 40+ frames/second.
also, use AVX vectorization.
I understand none of the code, but I am intrigued in how well you are able to read through the code and fix things.
This is exactly how video game developer feel every time anyone reporting a Bug in the game.
It would be interesting to see the assembly code from the profiling. I suspect there's something "interesting" happening when calling the lambda function (as someone writing compiler code and looking at code generated a lot, I suspect this is what happens!)
Very interesting video. I would like to see more videos where code is checked for issues related to speed, crashing, freezing, etc. A lot of us are just hacking stuff together with no real understanding of why problems occur such as a window freezing for 10 seconds while performing an operation.
I would love to see how you are optimizing it to its fullest potential
OH my gosh I remember watching you and learning some programming from you when I was still in primary school. You made me pursuit programming and suddenly seeing one of your videos in my feed brought back a ton of memories of me trying to figure out Java as a tiny kid. Now I'm studying cybersecurity and software development, working my way through!
I would love that additional video, I love optimization/profiling videos!
22:10
CMake projects generate with a RelWithDebInfo configuration by default, hope that saves you this headache in the future.
Optimizations would be great to see, there is so much you can learn out of that!
I think the result has changed. The old code was summing (accumulating) the HitRecord. You're just returning the last one.
Summing HitRecords doesn't really make sense - they're structs with no real way to "sum" them. The std::accumulate in the original implementation was simply choosing between two HitRecords and either keeping the old one or replacing the result, depending on the ray hit distance.
Additionally I ran an image diff after recording this video (just to make sure) - aside from noise and other random scattering built into the renderer, the results are identical.
@@TheCherno Thought it was odd it didn't trigger a compile error. Turns out std::accumulate relies on the lambda to do the accumulating by passing in the accumulated value (temp_value). That value gets passed in, you're meant to add to it and return the accumulated value for each iteration. Obviously, "our" lambda isn't doing that. As you were. :)
@@TheCherno Have done some testing. Using the lambda in a standard for loop gets the same slow result - so it's not std::accumulate. However, changing the lambda to return std::optional() on no hits fixes it. Change it back to return temp_value.. slow. Really strange.
@@TroySchrapel Maybe we're losing time by copying HitRecord all the time? The lambda takes the accumulated value by const ref, then returns it by value, so for every iteration, there is an unnecessary copy of a non-empty std::optional.
@@szaszm_ Yeah. It is. Specifically the shared_ptr of mat_ptr. Replacing that with a regular pointer also fixes the issue. Also, replacing all instances of std::optional with std::shared_ptr fixes the issue.
More optimizing of ray tracing. I appreciate how you got the language overhead out of the way and made the logic closer to the metal.
Fantastic video. I would love to see you optimize step by step this code.
Great Job Cherno :)
LOVE the optimization videos, please make more!
I love how your recording fps dropped to 1 when you rendered the image lol
Would like to see a follow-up video on optimizing the code, including checking the assembly and if the compiler doesn't make use of SIMD already, then also something about that for the hot path.
Hey Cherno, first video I've seen, but I like your approach to code improvement. Hope you kept a similar style since then :D
8:59 - There are cases where I might use a unique_ptr in that way, but I agree, in almost all cases I'd prefer vector. Maybe he didn't think of using resize on the vector to pre-allocate all the space or something. Though, there is a problem there because that will insist on initializing all the memory too, which might not be what you want. Though then it seems like you could just use reserve instead and then add the elements one-by-one. Hmmm...
It's really, really key to use profilers for this kind of thing. More often then not "optimized code" is a completely different beast to what the rest of your project will/should look like.
You can end up writing some really hard to read/ugly stuff so its really important to only do it in the hot zones. Also "premature optimization" is the route of all evil :p i've seen code that starts using bit shifting and all sorts "to be faster" and then in the very next code block reallocates an array inside a for loop that could have happily lived outside it :p
But its an interesting art, particularly on GPUs, like you would be surprised how often you can replace an if statement with pure maths that will achieve the same result and will be so much faster without the conditional. (this is true of standard programing too but more applicable in GPU stuff)
Yes! You should do an entire series on this! Like the lowest level you can reasonably get. Math and all would be awesome!