Skip to content

Utilize PGO for windows x64 rustc dist builds#96978

Merged
bors merged 4 commits intorust-lang:masterfrom
lqd:win_pgo2
Jul 12, 2022
Merged

Utilize PGO for windows x64 rustc dist builds#96978
bors merged 4 commits intorust-lang:masterfrom
lqd:win_pgo2

Conversation

@lqd
Copy link
Member

@lqd lqd commented May 12, 2022

This PR adds PGO support for the CI x64 windows dist builds.

These are the results from running the rustc-perf benchmarks:
image

Thanks to @Kobzol, @michaelwoerister, @wesleywiser, @Mark-Simulacrum for their precious help.

@lqd
Copy link
Member Author

lqd commented May 12, 2022

@bors try

@bors
Copy link
Collaborator

bors commented May 12, 2022

⌛ Trying commit de0a53ad00db52cc21c031b27202fe312896c25e with merge 36167224f1306544c5a9c050ef151c0e6d81b745...

@bors
Copy link
Collaborator

bors commented May 12, 2022

💔 Test failed - checks-actions

@bors bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label May 12, 2022
@rust-log-analyzer

This comment has been minimized.

@lqd
Copy link
Member Author

lqd commented May 12, 2022

@bors try

@bors
Copy link
Collaborator

bors commented May 12, 2022

⌛ Trying commit e006ba81940512f2f255b32594e34197c022d8ea with merge 57b5bac113c6e1100592c1bf7d5a7fc61d74e332...

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Collaborator

bors commented May 12, 2022

💔 Test failed - checks-actions

@lqd
Copy link
Member Author

lqd commented May 12, 2022

@bors try

@bors
Copy link
Collaborator

bors commented May 12, 2022

⌛ Trying commit 588de49ce861165222eb39ce14942ea158bef945 with merge 031d87645f5dacc484cc4590a1aa9abc7409a80e...

@bors
Copy link
Collaborator

bors commented May 12, 2022

💔 Test failed - checks-actions

@rust-log-analyzer

This comment has been minimized.

@lqd
Copy link
Member Author

lqd commented May 12, 2022

@bors try

@bors
Copy link
Collaborator

bors commented May 12, 2022

⌛ Trying commit 721a71042f995816fad00e609f727c5f710895e1 with merge 083263b570c1aa26f2f148f0e329beb40695f37e...

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Collaborator

bors commented May 12, 2022

💔 Test failed - checks-actions

@lqd
Copy link
Member Author

lqd commented May 13, 2022

@bors try

@bors
Copy link
Collaborator

bors commented May 13, 2022

⌛ Trying commit 532f6e1803caa381874b41893a70142b662ce399 with merge e695e58c53b8f4126b9e0d7cf3f060a9e6fd8fac...

@bors
Copy link
Collaborator

bors commented May 13, 2022

💔 Test failed - checks-actions

@rust-log-analyzer

This comment has been minimized.

@lqd
Copy link
Member Author

lqd commented May 13, 2022

@bors try

@bors
Copy link
Collaborator

bors commented May 13, 2022

⌛ Trying commit 95a6d33169202e3185e9155474c7afa7a371d1f7 with merge 2e3c4fb4d1c201ad85cf7cad3e5ff0ab57635b53...

@bors
Copy link
Collaborator

bors commented Jul 8, 2022

☀️ Try build successful - checks-actions
Build commit: dd20c56717daaca1eabea73bbbea7e374edb9f15 (dd20c56717daaca1eabea73bbbea7e374edb9f15)

@rust-timer
Copy link
Collaborator

Queued dd20c56717daaca1eabea73bbbea7e374edb9f15 with parent 0f573a0, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (dd20c56717daaca1eabea73bbbea7e374edb9f15): comparison url.

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

This benchmark run did not return any relevant results for this metric.

Cycles

Results
  • Primary benchmarks: no relevant changes found
  • Secondary benchmarks: 🎉 relevant improvement found
mean1 max count2
Regressions 😿
(primary)
N/A N/A 0
Regressions 😿
(secondary)
N/A N/A 0
Improvements 🎉
(primary)
N/A N/A 0
Improvements 🎉
(secondary)
-2.1% -2.1% 1
All 😿🎉 (primary) N/A N/A 0

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Benchmarking 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.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf -perf-regression

Footnotes

  1. the arithmetic mean of the percent change

  2. number of relevant changes

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Jul 8, 2022
@lqd
Copy link
Member Author

lqd commented Jul 8, 2022

That's reassuring.

@rustbot ready

@Mark-Simulacrum Mark-Simulacrum removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 11, 2022
@Mark-Simulacrum
Copy link
Member

I think this is basically ready to go -- one nit, and I'd appreciate if we could squash away some of the more "intermediate" commits -- ideally we'd probably have these commits, roughly, but if we need a couple more that's OK (mostly want to avoid needing to chase commits too much when running git blame).

  • bump LLVM versions
  • introduce the bootstrap code necessary for PGO to work on Windows
  • adjust linux PGO script (indents, environment variables)
  • add Windows ifs to PGO script, enable it in CI

r=me with nit fixed and commits squashed.

lqd added 4 commits July 11, 2022 15:20
The version 14.0.2 we currently use is busted on windows at the very
least.
When building LLVM/LLD as part of a build that asks LLVM to generate profiles, e.g. when
doing PGO, cmake or clang-cl don't automatically link clang's profiler runtime in,
causing undefined reference errors at link-time.

We do that manually, by adding clang's resource library folder to the library search path:
- for LLVM itself, by extending the linker args that `rustc_llvm`'s build script
  uses, to avoid the linker errors when linking `rustc_driver`.
- for LLD, by extending cmake's linker flags during the LLD build step.
This extracts the linux-isms into variables, so that the script can be
extended to do PGO on windows. These variables will be overriden in a
few spots, in windows-specific blocks.
This adds windows-specific behavior into the PGO script, and enables it
on CI.
@lqd
Copy link
Member Author

lqd commented Jul 11, 2022

I've added the comment about disk space and reworked the commit history as requested, and will wait for PR CI to be green.

@lqd
Copy link
Member Author

lqd commented Jul 11, 2022

let's see how it goes :)

@bors r=Mark-Simulacrum rollup=never

@bors
Copy link
Collaborator

bors commented Jul 11, 2022

📌 Commit 9027f82 has been approved by Mark-Simulacrum

It is now in the queue for this repository.

@Kobzol
Copy link
Member

Kobzol commented Jul 11, 2022

Great work @lqd! I think that it would be nice to let people know about this. Maybe a Rust/Rust inside blog post or a reddit post? I can draft up the latter.

Edit: created a Reddit post.

@bors
Copy link
Collaborator

bors commented Jul 11, 2022

⌛ Testing commit 9027f82 with merge 8a33254...

@bors
Copy link
Collaborator

bors commented Jul 12, 2022

☀️ Test successful - checks-actions
Approved by: Mark-Simulacrum
Pushing 8a33254 to master...

@wesleywiser
Copy link
Member

Great work @lqd! 🎉🎉

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (8a33254): comparison url.

Instruction count

  • Primary benchmarks: no relevant changes found
  • Secondary benchmarks: 😿 relevant regressions found
mean1 max count2
Regressions 😿
(primary)
N/A N/A 0
Regressions 😿
(secondary)
1.5% 1.6% 6
Improvements 🎉
(primary)
N/A N/A 0
Improvements 🎉
(secondary)
N/A N/A 0
All 😿🎉 (primary) N/A N/A 0

Max RSS (memory usage)

Results
  • Primary benchmarks: no relevant changes found
  • Secondary benchmarks: 😿 relevant regression found
mean1 max count2
Regressions 😿
(primary)
N/A N/A 0
Regressions 😿
(secondary)
3.2% 3.2% 1
Improvements 🎉
(primary)
N/A N/A 0
Improvements 🎉
(secondary)
N/A N/A 0
All 😿🎉 (primary) N/A N/A 0

Cycles

Results
  • Primary benchmarks: mixed results
  • Secondary benchmarks: 🎉 relevant improvements found
mean1 max count2
Regressions 😿
(primary)
2.6% 2.8% 2
Regressions 😿
(secondary)
N/A N/A 0
Improvements 🎉
(primary)
-3.4% -3.4% 1
Improvements 🎉
(secondary)
-5.3% -8.3% 2
All 😿🎉 (primary) 0.6% -3.4% 3

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

Footnotes

  1. the arithmetic mean of the percent change 2 3

  2. number of relevant changes 2 3

@lqd
Copy link
Member Author

lqd commented Jul 12, 2022

I think ctfe-stress-5 has been noisy recently (possibly #96267 but this time this benchmark feels jumpy at +/- 1.6%) and is probably unrelated.

I’ll re-check the CI artifacts with cachegrind to make sure it’s indeed similar to #97841 (comment) and try to find the inverse perf win in another PR, much like happened in this other PR’s try build for example.

@lqd
Copy link
Member Author

lqd commented Jul 12, 2022

@wesleywiser I don't think we've announced linux PGO on the Inside Rust blog when it landed (we did for mw's initial explorations), but I'm happy to draft something if we want one. Relnotes may be the expected, or better, venue ?

@Kobzol if you want to do a reddit post, that would be sweet. But if we want an Inside Rust post, I'll ping you on zulip and we can collaborate on that.

@wesleywiser
Copy link
Member

I think it would be totally appropriate to write an Inside Rust post on this work if you're up for it 🙂

I'd be happy to review and r+ it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merged-by-bors This PR was explicitly merged by bors. relnotes Marks issues that should be documented in the release notes of the next release. relnotes-perf Performance improvements that should be mentioned in the release notes. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants