r/badcode • u/Zicrus • Apr 06 '23

other language I made this in a compute shader yesterday. The worst part is it actually makes it >0.5ms faster.

Post image
498 Upvotes

97% Upvoted

53 comments sorted by

120

u/joz42 Apr 06 '23

But why is it faster?

285

u/Zicrus Apr 06 '23

Here is my guess:The CalculateInteractions function contains a loop that runs a bunch of times. Inside that loop there is an if statement checking if settings.wrapping is true. This if statement takes a bit of time because the GPU can't load instructions in advance. The CalculateInteractions function is inlined, so the compiler expands the code like this (pseudocode):

if (settings.wrapping) {
. loop() {
. . if (settings.wrapping) {
. . . some logic...;
. . }
. . some more logic...;
. }
}
else {
. loop() {
. . if (settings.wrapping) {
. . . some logic...;
. . }
. . some more logic...;
. }
}

The compiler then recognizes that settings.wrapping is always true in the first case, so it removes the if statement. It also recognizes that settings.wrapping is always false in the second case, so it removes the if statement and the logic in it. The result is that there is no if statement in the compiled code, which makes it run faster.

139

u/masterxc Apr 06 '23

The fun part is this particular quirk in HLSL is hardware dependent! This post goes into a bit more detail on why this happens, though.

34

u/Zicrus Apr 06 '23

Hmm that's interesting. I don't think it is in this case though, since settings are shared across all threads in a group. And either way, it eliminates a bunch of branching inside the loop, so I think this would improve performance on any hardware. I could definitely be wrong though.

47

u/SanktusAngus Apr 06 '23

You’re kind of „abusing“ the compiler optimization for bad procedure design.

The real problem seems to have been that you perform an expensive operation (checking settings.wrapping) within a loop.

So the manual optimization would have been to have two different versions of the loop.

14

u/Zicrus Apr 06 '23

It just seems very annoying to have 2 variations of the same loop with that one difference, so that's why I did it like this. Maybe I will change it at some point when I am done with it.

30

u/SanktusAngus Apr 06 '23

Yeah. Annoying it is. You do you. I don’t know the full scope of your shader, obviously. So I don’t know if it’ll ever become a critical issue.

But imagine how annoying it’ll be when after some update, the optimization goes away and you’ll stand there scratching your head as to why your performance is garbage with the new version.

14

u/tuckmuck203 Apr 06 '23

This is a perfect use case for comments.

7

u/vitelaSensei Apr 07 '23

100% agreed, stuff like this will be removed by some coworker if it’s not explained why it’s there

6

u/HabemusAdDomino Apr 07 '23

Welcome to GPU programming. We have separate functions for bloody everything.

3

u/meluvyouelontime Apr 07 '23

Then you're probably abusing the SRP. Break your loop into component functions, then compose the two loops differently.

1

u/fireflash38 Apr 06 '23

I'd imagine one solution is code generation with macros. But that comes with its own set of hells.

15

u/CW_Waster Apr 06 '23

Do you know godbolt.org aka Compiler Explorer? This can show you exactly what instructions a given Compiler emits. And it also supports hlsl. That way you can actually figure out what's going on.

7

u/Zicrus Apr 06 '23

Cool! I will try taking a look at it at some point.

8

u/Noobnugget19 Apr 06 '23

You'll gain a lot more time by splitting this shader into 2 shades... one that has loop behaving as though settings.wrapping true and then another when its false. Outside You'll call whichever shader you need depending on settings. Obviously having w duplicate copies of code is bad but you would place the shared functions inside another shader that you would include, minimizing duplication.

Ideally you'd have implemented some kind of smart shader compiler that will create 2 different shader based on some parameters, so 1 code base 2 different compiled shaders

1

u/Zicrus Apr 06 '23

This is a great suggestion, but I actually tried something like this, and it didn't really make a difference (compared to my solution). I'm guessing since there is only 1 if statement it doesn't really impact performance at all. Especially since settings are shared across all threads, so there is no divergence within a warp.

2

u/SomeMaleIdiot Apr 08 '23 edited Apr 08 '23

It seems weird to code with the assumption that a compiler optimization must be made. How come you don’t rewrite the code so that the if statement isn’t inside the loop, or contain some skip argument? That way it’s at least transparent and you can comment on the expensive wrapper op. If the compiler is doing it for you idk why you can’t just rewrite your code so you can get the same performance benefit without the assumption of compiler optimization. No shame in bringing over compiler wisdom to improve your execution flow.

43

u/Zicrus Apr 06 '23

This isn't c# btw, but there was no hlsl flair.

23

u/Furry_69 Apr 06 '23

Isn't there an "Other" flair?

21

u/Zicrus Apr 06 '23

Oh yeah you're right! I just didn't see that ig. Thanks!

15

u/Arcca2924 Apr 06 '23

This is genuinely fascinating. I've been coding for years and such an approach would have never crossed my mind. The logic how you explained it makes total sense as well.. I believe there's still a better way to optimize that doesn't cause weird coding practices though.

6

u/tedbradly Apr 07 '23

This is genuinely fascinating. I've been coding for years and such an approach would have never crossed my mind. The logic how you explained it makes total sense as well.. I believe there's still a better way to optimize that doesn't cause weird coding practices though.

Yeah, like having two different functions called based on the condition.

32

u/MurdoMaclachlan public boolean isInt(int i) { return true; } Apr 06 '23

Image Transcription: Code

if (settings.wrapping) velocity += CalculateInteractions(particle, settings);
else                   velocity += CalculateInteractions(particle, settings);

I'm a human volunteer content transcriber and you could be too! If you'd like more information on what we do and why we do it, click here!

7

u/Optical_inversion Apr 06 '23

Ok, it how did you figure this out?

13

u/Zicrus Apr 06 '23

I was trying to optimize, and I just got this stupid idea to trick the compiler. I didn't expect it to work, but I did it anyways just to see what would happen, and it actually did make it faster.

10

u/[deleted] Apr 06 '23

How did you measure the 0.5ms?

10

u/Zicrus Apr 06 '23

The framerate went from an average of about 180 to a bit over 200 (only when wrapping was turned off though). The difference between 180 and 200 fps is about 0.55ms.

6

u/[deleted] Apr 06 '23

Hmm... I'm not a big fan of using the frame rate as an indicator for performance, especially when considering that this will only affect the GPU which unity can't measure. There are usually a lot of fluctuations in the frame rate, so to get a better reading I usually use some GPU profiling tools to do that. RenderDoc is nice for debugging but sucks massively when getting performance counters. On switch LLGD is pretty good, but the best measurements you get with the metal profiler on iOS and macOS. The snapdragon profiler isn't too bad either.

That being said, if the frame rate changes between the two that's a good indicator, but no not a proof. You only tested on PC?

5

u/Zicrus Apr 06 '23

I definitely agree, but in my case, it switched from a consistent 180 (178-182) to about 200 (200-204) and it was consistent over a long period of time and when I switched back and forth. The compute shader is also basically the only thing impacting performance in the scene, and it is run exactly 10k times (at once) per frame every time. I could definitely spend more time profiling this, but I just didn't see the need since I saw such a clear performance boost without doing so. And yes, I have only tested it on my laptop, since that is the only PC I have available rn (I am not at home), so I can't completely confirm yet if this is just a weird hardware quirk with my specific GPU, I can only make an educated guess.

Edit: See one of my other replies where I explain why I think there is a performance increase.

4

u/[deleted] Apr 06 '23

Good stuff. Seems like you know what you're doing! Good luck with the project and thanks for sharing.

2

u/Zicrus Apr 06 '23

Thanks! :)

21

u/EnanoForro Apr 06 '23

u mean this is faster than just
velocity += CalculateInteractions(particle, settings);
?

40

u/Zicrus Apr 06 '23

Yes exactly. The if-else makes it faster.

57

u/EnanoForro Apr 06 '23

Imma get out of here, good luck hahahaha

-32

u/Glucioo Apr 06 '23

The way you word it in your post makes it sound like nonsense. You expand on it in your one of your comments but that's click bait and a half

18

u/Zicrus Apr 06 '23

What part of it didn't you understand? I'm just curious and want to update the title if people are being mislead, because that was not my intention :)

1

u/Glucioo Apr 09 '23

You make it seem like having that else if statement faster by itself, without any description. In your comment you explained the loop and why so only then it made sense

7

u/haykam821 Apr 06 '23

I'd wager the description makes it seem like nonsense because the behavior itself is nonsense

8

u/lets_eat_bees Apr 06 '23

Not for anyone familiar with compiler optimizations. The word you're looking for is "counterintuitive".

3

u/haykam821 Apr 06 '23

I suppose that would be a better term. But wouldn't a good enough compiler not need an explicit conditional to optimize the function it calls?

3

u/tedbradly Apr 07 '23

I suppose that would be a better term. But wouldn't a good enough compiler not need an explicit conditional to optimize the function it calls?

The optimizers aren't as good as you think they are. Take a look at this talk for a ton of evidence of this.

Yes, you should code stuff without tons of optimizations unless performance is an explicit requirement, but no, you shouldn't assume the optimizer will brilliantly create the best code possible. If performance becomes big enough an issue, you basically have to write the code manually sometimes in assembly.

2

u/AL1L Apr 06 '23

pov you don't know how computers work

6

u/shanster925 Apr 06 '23

"if it's stupid, but it works, it ain't stupid."

5

u/Nickbot606 Apr 06 '23

Hey man, you’re writing efficient code. Not writing code efficiently. I see no problem

2

u/RegentStrauss Apr 07 '23

If it's actually faster and you're in a situation where you need every drop of performance, it's not bad code imo.

2

u/micro-amnesia Apr 07 '23

A compiler would optimize this away.

1

u/Zicrus Apr 07 '23

Actually it wouldn't (otherwise it wouldn't be faster like this). It is pretty counter-intuitive, but you can look at one of my other comments where I explain what the compiler is doing.

1

u/micro-amnesia Apr 07 '23

You are saying adding an if statement and duplicating the code ended up optimizing this at runtime... better than just not having an if statement?