Skip to content
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 2x compile-time regression on pathological file #14800

Closed
rasky opened this issue Mar 13, 2016 · 11 comments
Closed

cmd/compile: SSA 2x compile-time regression on pathological file #14800

rasky opened this issue Mar 13, 2016 · 11 comments
Milestone

Comments

@rasky
Copy link
Member

rasky commented Mar 13, 2016

This file:
https://gist.github.com/rasky/0b36110ba703f73e9d64

is generated by stringer in a real project, and manually modified to make it self-contained (and anonymized).

Compilation time:

1.6:                 3.82 real         3.78 user         0.57 sys
tip w/o checks:      8.20 real         9.73 user         1.41 sys
tip:                14.42 real        16.06 user         1.45 sys
@mwhudson mwhudson changed the title SSA 2x compile-time regression on pathological file cmd/compile: SSA 2x compile-time regression on pathological file Mar 13, 2016
@ianlancetaylor ianlancetaylor added this to the Go1.7 milestone Mar 14, 2016
@minux
Copy link
Member

minux commented Mar 14, 2016 via email

@tzneal
Copy link
Member

tzneal commented Mar 15, 2016

The SSA backend performs common subexpression elimination much better than older versions of go. Can you check the resulting binary size? For example, I get a 62x smaller object file from tip with your example (go tool compile slow.go):

version size
1.5.2 20M
1.6 20M
tip 320k

Wrapping slow.go in a main, I get an overall 11x size decrease:

version size
1.5.2 22M
1.6 22M
tip 1.9M

@rasky
Copy link
Member Author

rasky commented Mar 15, 2016

Compiling with today's tip, I not only confirm tzneal's numbers, but the compile-time regression seems gone (!). It's even faster than 1.6 for me. @tzneal would you mind double checking before I close this bug?

@bradfitz
Copy link
Contributor

Fixed by 11a8086, dup of #14781 perhaps?

@bradfitz
Copy link
Contributor

Or the turning off of SSA consistency checks in 5305a32?

@tzneal
Copy link
Member

tzneal commented Mar 15, 2016

I get the same results (slightly faster than 1.6 now). The original slowdown showed up even with check disabled, but the speedup doesn't appear to be related to @josharian's change. I reverted it, and it was still faster.

@josharian
Copy link
Contributor

Anyone want to bisect?

@cespare
Copy link
Contributor

cespare commented Mar 16, 2016

I'll bisect.

@cespare
Copy link
Contributor

cespare commented Mar 16, 2016

I believe that 7c18f8c is the commit in question.

That said, I can't quite reproduce the timings that others have given here. The bad case isn't as bad, and tip is slower for me than 1.6.

commit build time
7bc40ff (go1.6) 3.5s
368507b (parent of 7c18f8c) 6.7s
5630cb7 (tip) 4.4s

(These are run with -gcflags '-ssa=1 -d=ssa/check/off' where appropriate. Numbers are average of 5 runs.)

So tip is still roughly 25% slower than 1.6 for me. I'm on linux/amd64.

@cespare
Copy link
Contributor

cespare commented Mar 16, 2016

I also looked into which commits were responsible for the size reductions.

a0232ea reduced the size from 20M to 320K.

3648d2d gave another 50% reduction to 164K.

Good work @josharian and @skohanim!

@randall77
Copy link
Contributor

Ok, not quite back to parity but pretty close. The object size is way smaller which will probably speed up the linker enough to reach parity. I'm going to close as "fixed enough".

@golang golang locked and limited conversation to collaborators Apr 29, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

9 participants