Convert if/else chain to switch statement

[nukethebees]

RendererSceneCull::_scene_cull contains a large if/else chain which can be converted to a switch statement. In MSVC's optimised assembly, all if/else checks are done sequentially whereas the switch jumps directly. The if/else probably wasn't optimised as the final conditional isn't versus a constant (but we can just handle that in the default branch).

Part of the original if/else asm with MSVC:

  2840: 				uint32_t base_type = idata.flags & InstanceData::FLAG_BASE_TYPE_MASK;
mov         r13d,1  
mov         edx,dword ptr [r15]  
movzx       ecx,dl  
  2841: 				if (base_type == RS::INSTANCE_LIGHT) {
cmp         ecx,5  
jne         RendererSceneCull::_scene_cull+1122h (07FF7A6806AD2h)  


  2848: 				} else if (base_type == RS::INSTANCE_REFLECTION_PROBE) {
cmp         ecx,6  
jne         RendererSceneCull::_scene_cull+1298h (07FF7A6806C48h)



  2868: 				} else if (base_type == RS::INSTANCE_DECAL) {
cmp         ecx,7  
jne         RendererSceneCull::_scene_cull+1370h (07FF7A6806D20h)  

And so on. New switch asm:

  2842: 				switch (base_type) {
lea         eax,[rdx-5]  
cmp         eax,7  
ja          $LN48+0BFh (07FF6BFFC703Ch)  
lea         rbx,[__ImageBase (07FF6BAE40000h)]  
mov         ecx,dword ptr [rbx+rax*4+5188C14h]  
add         rcx,rbx  
jmp         rcx  

[lawnjelly]

It does appear (at least pre-2022) that MSVC compiler might not be so good at optimizing if else compared to switch.

First question though - have you got some profiles indicating this to be a bottleneck in real world (or benchmark) cases?
We usually prefer to limit micro-optimization to cases where there's evidence of a bottleneck.

(Culling can be, so it could well be useful, but it would be nice to confirm this either way. 👍 )

[nukethebees]

I profiled it with the official culling benchmarks and it doesn't make any measurable difference as most of the time is spent in a much larger conditional earlier in the function.

You may as well close this then I guess.

Is there a threshold for what's considered a sufficient improvement?

[lawnjelly]

Is there a threshold for what's considered a sufficient improvement?

There's no hard rules, but generally for optimization only PRs we like to see things make a significant improvement in benchmarks (i.e. measureable), and more so for ones that may make code more difficult to read - there is a trade off here, as optimizations often make less readable code, and often less easily maintainable code.

See https://docs.godotengine.org/en/stable/tutorials/performance/general_optimization.html#principles

Generally it is advisable for optimization PRs (CPU) to:

1. Find a slow project / benchmark
2. Profile
3. Optimize bottleneck

This avoids wasting development / review time on areas which will not improve things.

[jss2a98aj]

I do find this to be an improvement to readability even if the performance impact is negligible.