Improve render perf for MSVC Debug configuration #550
Conversation
MSVC's STL performs extensive bounds and validity checking on iterators in Debug builds. This is desirable for correctness, but causes significant slowdowns in the current renderer's implementation due to the amount of vertex copies and transformations it performs. Until we can fully convert the renderer into more of a 'static data' model where the game does not need to ship thousands of vertices to the GPU each frame, the MSVC Debug build realistically needs to skip the per-iteration checks for its vertex iterators. This is, I think, a happy compromise between full checks and just using bare data pointers: The iterators are still used and checked once each at the start of any operation, but the memory accesses are done via pointers which should be very fast.
|
I'd like just not count Debug build on MSVC as main target of our development. Users does not supposed to use debug builds, and slow performance on Debug is only trade off over clean code, which runs fine on Release. |
|
Agree ideologically, but this is very nearly identically-safe and should eliminate a very big bottleneck in that particular build, making debugging on Windows easier. |
|
We have 10 builds for various platforms, and there only one has performance regression - Windows Debug, so I suggest either fix it alone without affecting other builds or just threat it as "Debug is not about performance". After all, we can just disable iterator debugs on whole project for Windows Debug, I don't see any usefulness of this feature at all for us. |
|
This "kind of" does only change anything for MSVC: On both libcxx and gnustl, I do think it is significantly valuable to have the iterator debug checks active in general. Early on in this project actually, I was converting some |
|
What about CMake's RelWithDebInfo build configuration when you need that speed while still being able to debug? |
That would be good solution too. |
|
Normally I'm pretty strongly in agreement with the "don't alter code for special cases" line of thought, but I do at the same time think it's pretty important to keep the Debug build reasonably usable - otherwise people will never use it. The alternatives to this change that I see are:
Option 1 is, to me, not really an option - we might as well remove the preset for it. Option 2 could be done in a semi-cleanly way using some intermediate local variables, so that significant logic lines remain the same - but this would be entirely a semantic change, because What's the resistance to this one - is it just that the |
|
On RelWithDebInfo we can enable Prebuilt Debug artifacts on Windows already no has any use for users - they cannot run it without MSVC itself (as pointed in #509), so Debug environment is usable only for developer. Slow performance is observed only in one of our supported environments - Windows Debug, so we should either fix our code in way that not affects other environments or fix environment itself. Using I still suggest either to disable iterator checking "feature" for Windows Debug (we don't has any use of this with well written code and boundary checks where it's really needed) or left Windows Debug slow as is. |
This would eliminate the point of the RelWithDebugInfo build - it is meant to be a Release build, just with debug info. The iterator checks are, imo, incredibly important. They've already caught one bug in the recent past for me and will continue to catch others. I will try to rewrite this so that it uses clearer syntax, but I don't want to target the change specifically to Windows. (In #528 I talk about how I actually want to turn on similar checks for Mac and Linux.) |
|
Very interesting about #509. Hadn't seen that one yet. |
|
There is some inconsistency in the current code, regarding these defines. Btw. I can't say that I find the current Windows Debug build to be near-unusably slow, since that is the build I'm usually working with. |
Agree - I talk about this a bit in #528 (comment)
Maybe we just accept the CPU-perf cost of this stuff then? I believe enough in the importance of the iterator checks that I'm unwilling to remove them. @Lgt2x did mention in the original report that his test machine was pretty low-end - as long as the game runs performantly-enough on "most" systems that are going to be running Debug builds, I'm ok with leaving this as-is. |
Pull Request Type
Description
MSVC's STL performs extensive bounds and validity checking on iterators in Debug builds. This is desirable for correctness, but causes significant slowdowns in the current renderer's implementation due to the amount of vertex copies and transformations it performs. Until we can fully convert the renderer into more of a 'static data' model where the game does not need to ship thousands of vertices to the GPU each frame, the MSVC Debug build realistically needs to skip the per-iteration checks for its vertex iterators. This is, I think, a happy compromise between full checks and just using bare data pointers: The iterators are still used and checked once each at the start of any operation, but the memory accesses are done via pointers which should be very fast.
Related Issues
#543, #545
Checklist