-
Notifications
You must be signed in to change notification settings - Fork 13k
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
ThinLTO + O2 build time regression in rust-doom #45188
Comments
We should definitely re-investigate this with different settings for |
Ok here's what I did:
Build entire project from scratch Here I just ran
The conclusion from this is that I get clear wins across the board on my machine for using more CGUs. The sweet spot appears around 16 CGUs, and I don't see any clear/obvious regressions in this area for using multiple CGUs. Build project after changing the Here I changed the
Here we see even more drastic wins (as is expected) and again no clear regressions from one CGU. I haven't done much data collection of inline-in-all-cgus in release mode, but @sfackler posted some worrisome numbers which indicate that |
In the thread about testing ThinLTO some main conclusions I had was:
With that in mind I'm sort of tempted to close this and keep pushing hard on #45320, but I'm curious if others can reproduce my own results! |
I'm doing another run with the newest nightly, more values for the number of
I want that engraved on my tombstone :P |
Fair point! |
So using your script (thanks btw!), this what I've got (with
This results match yours more closely than before, with modest or no improvements in the CGU 8-32 regime for the full rebuild and significant speed-ups in the single crate case. There is no difference in runtime benchmarks. So what happened before? I'll put it down to thermal throttling: all runs took much longer, and maybe with more cores in use things got hotter. Sorry for the noise, it sounds like you can close this issue! |
Ok awesome! Thanks so much for gathering that data! In that case I'll... |
Creating an issue for https://internals.rust-lang.org/t/help-test-out-thinlto/6017/3 for more investigation.
cc @cristicbz
The text was updated successfully, but these errors were encountered: