Addressing That Post About `final`

I have never had to do a follow up to any blog post I've ever written; but I feel like I really need to with that last one and clarify a few things.

At the time of publishing I thought I was merely lighting a firecracker, but it seems more like I set off a crate of dynamite.  I knew there was going to be some discussion about the results, but I did not anticipate the nearly 350+ comments.  People are very particular about performance and benchmarking (as it is fair to be).  Everyone is allowed to call BS if they see fit.

It's been three weeks since the article went live. I wanted to take some time for the dust to settle in order to read through what everyone wrote; and respond.  If you haven't been privy to any of the discussion, it's been on /r/cpp and Hacker News.  Along with some talk on Hackaday.

 

"I didn't understand how to use final properly"

I saw this comment pop up a few times; that I missed the point of final.  The proper use of final wasn't the thesis of my article.

There are plenty of resources explaining how to use it and its purpose in the design of a C++ application.  My concern was other articles claiming it can improve performance without a benchmark to back up their statements.  Please read the titles of these articles:

None of these have any metrics posted.  But all of these titles imply "final makes code go faster".  They all talk about how final is used, including the generated assembly and what's happening at the machine level.  That fills the "how?" and "why?" of final.  But that isn't a benchmark.  To say it improves performance but not have any proof to back it up is dangerous.

For the longest time we have been living in an environment where we skim articles (reading only a headline) and glean information to take it as fact; without actually verifying anything.  Part of my previous blog post was trying to highlight what can happen if you do this.  It is what I did initially, noticed nothing was matching those claims and decided to test it a bit further.

There was one thing I was wrong about: someone else did a benchmark of final in the past.  In their case they found a consistent performance increase.  They even re-ran their benchmark and saw the same results from 10 years ago.  In my case it was faster in some instances, slower in others.  I thank them for reaching out and have updated my older post.

I recall there being a comment about how my "improper" use of final could be a reason why clang had its performance slowdown.  My counter to that: GCC had a consistent performance increase with use of the keyword.  It was used the keyword as intended (and described by the linked articles above).  I put it on many classes that have no further subclassing, and there was a performance boost in this case.  Clang on the other hand, had a decrease with the exact same code.

 

"This isn't a good benchmark"

The previous article was written with the context that others may have read the project's README or have read some of the other prior posts.  Let me rewind a little:

PeterShirleyRayTracing, a.k.a. PSRayTracing (a.a.k.a. PSRT) didn't start out as a benchmarking tool.  I wanted to revisit a ray tracing book series I read when I was fresh out of university (2016), but this time with all the knowledge of C/C++ I had accumulated since that time.  I first went through the books but as an exercise to learn Nim.  Between then and 2020 I had seen images from the book pop up online here and there. Mr. Shirley had actually made the mini-books free to read during that time.  Reading the book's old code and the newer editions, I noticed there were a lot of areas for improvement in performance.  PSRT at first was an experiment in writing performant C++ code, but with some constraints:

  1. Has to follow the book's original architecture
  2. Needs to be cleaner and modern
  3. Must showcase safer C++
  4. Must be standard and portable
  5. Full support for GCC and Clang (then later MSVC)
  6. Be "Vanilla C++" as possible.  I don't want to force someone to bring in a hefty library
    • There is an exception for libraries (like PCG32) that allow an increase in performance and are easily integrated like being a single header library
  7. Be able to turn on and off changes from the book's original code to see the effects of rewriting parts
  8. Extending is okay, but they can't violate any of the above rules
    • E.g. multithreading was added and some new scenes.  The Qt UI is a massive exception to rule no. 6 (but it's not required to run the project)

For the initial revision of PSRT, its code was 4-5x faster than the books' original code (single threaded).  I was very proud of this.

Later on a Python script was added so PSRT could be better tested via fuzzing.  Parameters could differentiate scenes, how many cores to use, how many samples per pixel, ray depth, etc.  It was both meant to check for correctness and measure performance.  The measurement of performance only is the time spent rendering.  Startup and teardown of PSRT is not measured (and it's negligible). This way if I came across some new technique a change could be made and verify it does not break anything from before.  The script has evolved since then.

To explain how the testing and analysis operates a little more simpler:

  1. Each scene would be fuzz tested, say three times (real tests do way more), and their runtimes recorded.  Parameters could be wildly different
    • For this example let's say once scene resulted in the times of [3.1, 10.5, 7.4] (real testing used 30 values)
  2. Then the same suite would be run, but with a change in code
    • times=[2.7, 8.8, 6.9]
  3. From this a percentage difference of each test case would be computed
    • [13%, 17%, 7%]
  4. A mean & median per scene could be calculated
    • mean=12.3%, median=13%
    • Each scene is different.  Sometimes radically, other times only slightly.  That's why it's important to look at the results per scene
  5. From there a cumulative "how much faster or slower" for the change could be found

I hope this explains it better.

I neglected to mention what compiler flags were used.  All of the code was built with CMake in Release mode.  This uses -O3 in most cases.  This was something I should have specified first.  I know there are other flags that could have been used to eek out some other tiny gains but I do not think it was relevant.  I wanted to use the basics and what most people would do by default.  I also configured the machines to only run the testing script and PSRT.  Nothing else (other than the OS).  Networking was disabled as well so nothing could interrupt and consume any resources available.

 

Simple vs. Complex

One commenter pointed out how they didn't like this, saying that they preferred simpler tests benchmarking atomic units. For example, measuring a bubble sort algorithm and only that.  There are already a plethora of tests out there that do just this.  That isn't good enough. In the real world we're writing complex systems that interact.

Prefer integration tests; verify the whole product works.  Unit testing is good for small components but I only like to do this only when the tiny bits need testing.  E.g. if a single function had a bug and we want to double check it going forward.

 

Other Benchmarks

In all of the comments that I read, I only recall coming across one other benchmark of final; they reported a speedup.  But our methods of testing are completely different.  They were testing atomic components.  Mine was not.

In episode 381 of CppCast it was discussed that there are many practices in the C++ world that are claimed to be more performant without providing any numbers.  To anyone who doesn't think this was an adequate benchmark: Do you have an alternative?  I'm not finding any. If you don't think this was a good benchmark please explain why and tell me what should be done instead.

 

"The author provided no analysis about clang's slowdown"

This is one that I think is a more fair criticism of the article.  In my defense, this is a topic I do not know that much about.  I'm not a compiler engineer, not an expert on the subject of low level performance optimization, nor the inner workings of clang and LLVM.  For earlier development of PSRT, tools like perf, flame graphs, valgrind/cachegrind, Godbolt's Compiler Explorer, etc. were used.  But I do not feel comfortable providing a deep analysis on the issue with clang.

Time could have been spent researching the subject more and doing proper analysis, but this would have taken months.  I did reach out to a friend of mine who works at Apple who provided me with some tips.  Reading the comments on Hacker News, avenues seem to be looking at LTO, icache, inlining, etc. (Tickets have already been filed for further investigation.)

Someone did ask me to check the sizes of the generated binaires with final turned on and off. Devirtualization causing excessive inlining could be the cause. With final turned on, the binary was 8192 bytes larger; I'm not sure how significant that is to impact performance. For comparison, GCC's compiled with final was only 4096 bytes larger than no final. But GCC's binary was about 0.2 MB larger (overall) than clang's. I do not think binary size is a factor.

 

LLVM Engineer

On Hacker News there was a comment left by someone who works on the LLVM project.  Quoting them:

"As an LLVM developer, I really wish the author filed a bug report and waited for some analysis BEFORE publishing an article (that may never get amended) that recommends not using this keyword with clang for performance reasons. I suspect there's just a bug in clang."

  1. I am not sure if this was a bug.  I have had performance drops with clang compared to GCC, so I didn't view this as bug worthy.  I checked the LLVM issue tracker in the week after publishing and saw that no one else had.  So I went ahead and filed a ticket.
  2. I have amended articles in the past in light of new information.  A previous revision of this project added in the aforementioned Qt GUI.  When I noticed some bugs in Qt, an engineer from the company reached out to me and I updated the original article.  Last week, I thought there were no other benchmarks about final in existence.  I found out I was wrong and my previous article has been adjusted to include that new information.

    If there is a bug in clang/LLVM, it becomes fixed, and the slowdown from using final is reduced (or reversed), I will update the article.

 

Random Number Generator Might Be The Cause of Clang's Slowdown

The RNG was already a vector for performance improvement in the past.  Compared with the original book's code, using PCG's RNG showed improved performance over what was available in standard C++.  In the past I was wondering if there could be further improvements in this area.

One reader decided to dig a bit deeper.  That person is Ivan Zechev. He's done some amazing work already and found that the issue with clang might have been related to the RNG and std::uniform_real_distribution.  Calls to logl were not being properly inlined.  And this looks like a long standing issue in clang/LLVM that has never been fixed.

Mr. Zechev sent me a merge request for review, but I have held off on merging it because it actually changed how scenes were set up.  This can drastically alter how long it takes to render an image, because the scene is now different.  In our case, it was book2::final_scene.  At first the floor was completely changed.  Later he was able to correct for that, but other elements were not matching. The uniform distributor (in clang) was producing different numbers with his changes.  For this, I cannot merge.  I commend him for his investigation and will be looking at it in the future.  Thank you.

But this only uncovers a horrible problem: Things in std:: are not portable; which kind of means that the "standard library" really isn't ... well... standard.  In regards to std::uniform_real_distribution there is some more information here.  The C++ standard allows this but it doesn't seem right.

 

"There's no inspection of the assembly"

The other articles have talked about what assembly is generated.  I do not see why it was needed for mine.  What the other articles neglected to do was measure.  This is the gap I wanted to fill in.

I use C++ at the surface level.  I'm pretty sure most people do as well.  Part of the point of having a higher level language is to abstract away these lower level concepts.  PSRT is meant to be written this way; portable, memory safe, modern C++.  Knowing assembly definitely helps, but it should not be a requirement.  This is a C++ project.


 

Update May 15th, 2024:

After posting this article on /r/cpp, user /u/lgovedic provided a well thought out comment.  I'd like to repost that here for other readers:

Glad you addressed the comments on both platforms! But I agree with others here that some things were left unaddressed.

When it comes to software performance, I live by the words "don't trust performance numbers you can't explain". Your measurements seem robust, but I think you went too far in assuming that the correlation between the final keyword and overall performance implies a causal relationship.

I respect that you and many others don't want to jump into assembly, and I agree you should be able to just write high-level code. But I do think diving into assembly and providing evidence for the causal link is required if you want to make fundamental statements about C++ performance like "using the final keyword does not always yield performance gains".

To be fair, on a high-level, that statement is not false. And I appreciate that you shed light on the issue so that people will be more mindful of it and measure the performance of their code more often (that's always a good thing).

But from your results and without further investigation, I think a more appropriate statement would be "using the final keyword can drastically alter the layout and size of generated code, which might result in an overall slowdown". Because (again, without further investigation) that's a much more likely explanation, in my opinion. And more importantly, it provides much better advice for using the final keyword than just "be careful"


 

There will be follow ups and other investigations; but not immediately.  I am willing to amend anything in light of new data.  This is not my full time job and only a hobby project.  Anyone is allowed to contribute and is welcome to do so.

© 16BPP.net – Made using & love.
Back to Top of Page
This site uses cookies.