-
Notifications
You must be signed in to change notification settings - Fork 17.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
cmd/compile: adding panic to a never taken branch of an if
inside a loop significantly speeds it up by removing knots in the CFG and marking that branch cold
#70030
Comments
Related Issues and Documentation (Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.) |
It is easy to assume that code that is never ran does not affect performance, code exists create side effects which could be optimized at runtime but a purely static "naive" compiler like go's one can't see that. var alwaysFalse bool
var leak *int
func f(x *int) int {
if alwaysFalse {
leak = x
}
return *x
} the go compiler will heap allocate For your function, without the panic it needs to handle cases where calling More importantly the compiler assumes * on my computer with the winds blowing 10km/h South-SouthEast Tl;DrAdding panic to a never taken branch of an I have multiple ideas on how to improve this, like interprocedural register allocation or using PGO to annotate cold branches but neither is particularly easy nor fast to implement, and have downsides so I don't know how to make this issue actionable. cc @golang/compiler |
if
inside a loop significantly speeds it up by removing knots in the CFG and marking that branch cold
But it did run, according to the profiler. |
Maybe I misunderstood. Are you saying the time is attributed to that line, which is still not running? |
I'm not sure I understand the question or where the profiler is, when I run your benchmark the branch does not run otherwise it would call panic which would blow up loudly. The benchmark does not blow up loudly so the branch does not run. |
The compiler knows that control flow never continues past a Inner loop with
inner loop without
Note the code being jumped around (as @Jorropo says, in the |
According to my experiments 17ns vs 25ns, 5ns comes from improving the loop layout by marking the branch cold. And the remaining 3ns are unexplained but are likely the extra merge you pointed out as there isn't much other differences. |
Go version
go version go1.23.2 linux/amd64
Output of
go env
in your module/workspace:What did you do?
Ran this benchmark:
( I think I can't post benchmarks on go.dev/play)
What did you see happen?
With the panic it takes 25ns per op, and without it takes 45ns. (Panic is never executed)
According to the profiler, it's calling
copy
, which it shouldn't do. Hence I added a panic to see how it was called, and then it went faster.What did you expect to see?
It shouldn't call
copy
. It shouldn't go faster if I add a redundantpanic
.The text was updated successfully, but these errors were encountered: