-
Notifications
You must be signed in to change notification settings - Fork 17.7k
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: SSA performance regression on polygon code #15532
Comments
Looks like we're making unnecessary copies as a leftover from inlining. Here's a small repro:
This copy is not needed. The old compiler can get rid of the copy, but SSA doesn't. I don't see any obvious fix. I will ponder. |
This isn't going to get fixed for 1.7, too late. As a workaround, you can replace
with
SSA does better with structs than arrays. We'll look again into fixing this for 1.8. |
Basically the same issue as #15925. |
Punting to 1.13, too late for anything major in 1.12. |
go version
)?go 1.6.2 and devel +15f7a66
go env
)?clone this repo: https://github.com/nkovacs/polygonperf
and run
go test -cpu x -bench .
with go 1.6.2 and devel +15f7a66Results for BenchmarkContains (ns/op) on a Core 2 Q6600:
Results for BenchmarkStructContains (ns/op) on a Core 2 Q6600:
(last line is average)
I've seen a similar 30% increase in ns/op on an AMD Athlon II X2 270, but on that CPU the 1 cpu benchmark had the same result as the 2 cpu benchmark.
On the two more modern Intel CPUs I briefly tested, this simple polygon does not show a difference between master and 1.6.2. I added a second polygon (BenchmarkContains2 and BenchmarkStructContains2) that does show a difference, with 1.6.2 again being faster. On the Q6600, go 1.6.2 performs twice as fast in these benchmarks, on a Xeon server, go 1.6.2 is about 100-200 ns/op faster.
The text was updated successfully, but these errors were encountered: