[WIP] Build `rustc` with a single CGU on x64 Linux by Kobzol · Pull Request #107...
source link: https://github.com/rust-lang/rust/pull/107651
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.
[WIP] Build rustc with a single CGU on x64 Linux
#107651
Conversation
Contributor
Follow-up attempt to #87650. I wonder if anything changed with the addition of LTO.
I also enabled a single CGU only for the actual build of the compiler on CI, so that we can better see the perf. effects on the bootstrap benchmark.
Collaborator
|
(rustbot has picked a reviewer for you, use r? to override) |
Contributor
Author
added A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.
labels
This comment has been minimized.
added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label
Contributor
|
⌛ Trying commit 96c912abfc4b44ce5f8fca252500880ffba28d3e with merge 2a203d5ea41b38173f8e09d32d989d0c26770e28... |
Contributor
|
☀️ Try build successful - checks-actions |
This comment has been minimized.
Collaborator
|
Finished benchmarking commit (2a203d5ea41b38173f8e09d32d989d0c26770e28): 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)Results CyclesResults |
added perf-regression Performance regressions
and removed S-waiting-on-perf Status: Waiting on a perf run to be completed.
labels
Contributor
|
This seems to perform similarly to #107560, which make sense since that compiles most crates with 1 CGU. It's an overall improvement, but with some regressions. It does seem like there's some problem with ThinLTO + PGO for |
Contributor
Author
|
The CI time hit is also not that terrible, 8600s (I saw a recent master commit had 8000s), not sure what's the average recently (@Mark-Simulacrum ). We can get that time down, too. LLVM rebuild time can be lowered, and we can also speedup rustc builds - e.g. we don't need to build with 1 CGU when we do LLVM PGO/BOLT, and I think that we can also skip LTO/1 CGU when building stage 1 (we'd need to modify bootstrap though). |
Contributor
|
Building with 1 CGU would be useful to make the |
Contributor
|
I did some local benchmarks to test 16 CGUs (Before) vs. 1 CGU (After) with ThinLTO:
|
Contributor
Author
This comment has been minimized.
added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label
Contributor
|
⌛ Trying commit 96c912abfc4b44ce5f8fca252500880ffba28d3e with merge 0b5019f4e7c7c5e8190c0d4e9686840a7a3cfc00... |
Contributor
|
💔 Test failed - checks-actions |
This comment has been minimized.
added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
labels
Contributor
Author
This comment has been minimized.
Contributor
|
☀️ Try build successful - checks-actions |
This comment has been minimized.
Collaborator
|
Finished benchmarking commit (146169d35b066f541a24c7489cd7ef4397300cf5): 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)Results CyclesResults Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 629.118s -> 625.733s (-0.54%) |
Contributor
Author
|
@bors try Trying with LLVM 16. |
added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label
Contributor
Author
|
@bors try |
This comment has been minimized.
This comment has been minimized.
Contributor
|
💔 Test failed - checks-actions |
This comment has been minimized.
Contributor
|
☔ The latest upstream changes (presumably #115959) made this pull request unmergeable. Please resolve the merge conflicts. |
Member
|
How did this get merged by bors? There's no r+. |
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
No reviews
None yet
Successfully merging this pull request may close these issues.
None yet
Recommend
About Joyk
Aggregate valuable and interesting links.
Joyk means Joy of geeK