-
Notifications
You must be signed in to change notification settings - Fork 13.1k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # 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: