-
Notifications
You must be signed in to change notification settings - Fork 12.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
Avoid one rustc
rebuild in the optimized build pipeline
#112012
Conversation
@bors try |
⌛ Trying commit a5f4a6a33ea97eb31d87537e1cf85799897ef000 with merge 0b1d1a7586536966f299fc7f0e863abc2bd87c26... |
💔 Test failed - checks-actions |
This comment has been minimized.
This comment has been minimized.
@bors try |
⌛ Trying commit f1cef095460ec7c82a30b82a02f32d5957f8bd0b with merge a7030bb5524ea580d7262ce64ae4976e22a69cc1... |
This comment has been minimized.
This comment has been minimized.
💔 Test failed - checks-actions |
@bors try |
⌛ Trying commit 79ac5adcac9671b59218206b1d7ae9b7e7a1ac40 with merge 6dac01618ad5cdeb0b7ddc219ac23cc9cbb06049... |
This comment has been minimized.
This comment has been minimized.
💔 Test failed - checks-actions |
@bors try |
⌛ Trying commit 47455a6f11865bebf4acd4d063bf83838affc56a with merge 5e2bb1b90772525393f2fc4640fc7b196c460783... |
This comment has been minimized.
This comment has been minimized.
💔 Test failed - checks-actions |
@bors try |
⌛ Trying commit d6981af07da053d2f70f00d58bffcf2565099c0c with merge 0e7e1014c50ab9680e23008e30859f5dbc7966e7... |
This comment has been minimized.
This comment has been minimized.
💔 Test failed - checks-actions |
The table was produced incorrectly, I fixed it in the meantime. I'll do another try run, so that we can see hopefully a correct version of the table. I'll have to look a bit closer into the final rebuild. @bors try |
⌛ Trying commit 14ed9fa with merge 1bcffba3b86d52ce5726ff61d98719cf178bc073... |
☀️ Try build successful - checks-actions |
The stats looks sensible now:
|
Good. In the final I want to remove the BOLT step from boostrap and apply it in the PGO script instead, which will allow us to optimize |
@bors try |
⌛ Trying commit 41f9f63 with merge bae6b1ef7a7dfd9842d250a3b64da45e71269346... |
☀️ Try build successful - checks-actions |
@rust-timer build bae6b1ef7a7dfd9842d250a3b64da45e71269346 |
This comment has been minimized.
This comment has been minimized.
Finished benchmarking commit (bae6b1ef7a7dfd9842d250a3b64da45e71269346): comparison URL. Overall result: ❌✅ regressions and improvements - ACTION NEEDEDBenchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf. Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @bors rollup=never Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 647.329s -> 646.031s (-0.20%) |
Ok, looks like it works :) This should be ready for review now. |
@bors r+ rollup=never |
☀️ Test successful - checks-actions |
Finished benchmarking commit (50f2176): comparison URL. Overall result: ✅ improvements - no action needed@rustbot label: -perf-regression Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 649.083s -> 647.317s (-0.27%) |
This PR changes the optimized build pipeline to avoid one
rustc
rebuild, inspired by this comment. This speeds up the pipeline by 5-10 minutes. After this change, we no longer gather LLVM PGO profiles from compiling stage 2 ofrustc
.Now we build
rustc
two times (1x PGO instrumented, 1x PGO optimized) and LLVM three times (1x normal, 1x PGO instrumented, 1x PGO optimized). It should be possible to cache the normal LLVM build, but I'll leave that for another PR.