-
Notifications
You must be signed in to change notification settings - Fork 13.5k
opt-dist: rebuild rustc when doing static LLVM builds #143898
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
base: master
Are you sure you want to change the base?
Conversation
|
Some changes occurred in src/tools/opt-dist cc @Kobzol |
when building LLVM it's obvious that in case of shared build rustc doesn't need to be recompiled, but with static builds it would be better to compile rustc again to ensure we linked proper library. maybe I didn't understand the pipeline correctly, but it was strange for me to see that in Stage 5 LLVM is built while rustc is not
8d3a40a
to
32edd53
Compare
reverted stage 2 change as code comment is pretty self-explanatory |
@bors2 try
In stage 5 we shouldn't be building neither rustc nor LLVM. Bootstrap already skips building LLVM if it exists on disk, which is why we don't need to pass anything like |
opt-dist: rebuild rustc when doing static LLVM builds when building LLVM it's obvious that in case of shared build rustc doesn't need to be recompiled, but with static builds it would be better to compile rustc again to ensure we linked proper library. maybe I didn't understand the pipeline correctly, but it was strange for me to see that in Stage 5 LLVM is built while rustc is not <!-- homu-ignore:start --> <!-- If this PR is related to an unstable feature or an otherwise tracked effort, please link to the relevant tracking issue here. If you don't know of a related tracking issue or there are none, feel free to ignore this. This PR will get automatically assigned to a reviewer. In case you would like a specific user to review your work, you can assign it to them by using r? <reviewer name> --> <!-- homu-ignore:end --> r? `@Kobzol` try-job: dist-x86_64-msvc try-job: dist-x86_64-linux
this happens only if BOLT stage is run. otherwise right after llvm cleanup at stage 2 stage 5 starts |
you can refer to this GHA job to see how the pipeline works on MSYS2 environment |
Right, and on Windows we don't use BOLT, so there LLVM gets rebuilt in the final dist step. But rustc indeed wasn't rebuilt in the final stage though. Which means that we have been using a non-LTO-optimized LLVM build on Windows this whole time? Lol. Could you please post some comparison benchmarks with/without this change? |
sorry, I had literally no skill at good benchmarking, and I need to ensure that I have "clean" environment on my machine... if no one have time, I can try to do so tomorrow by the evening, I guess |
With/without LLVM PGO should be a bit difference, around 10% or something, that should be relatively easy to measure. Doing e.g. You can install these commits using https://github.com/kennytm/rustup-toolchain-install-master. |
thanks for the hint! will do soon |
I wanted to get more hands-on with Rust on Windows for unrelated reasons, so I rebooted into Windows and tried it there (a horrible experience, tbh :D). I found a 10% performance difference.. but in the wrong direction, lol.
Let's see if you can reproduce.. Btw I confirmed that stage2 rustc was now rebuilt on MSVC during the final stage (it wasn't before). |
I can't download cargo with it:
|
I don't think you need cargo, you can just use whatever you have locally. But if you want it, delete the toolchain first ( |
$ hyperfine "cargo +56835d7ac14da9f966e1ff39fd9ffd2e29b764d1 build --release" "cargo +666996a7659a64c146e4765a1bba6ddc563819d6 build --release" --prepare "cargo clean" --runs 5
Benchmark 1: cargo +56835d7ac14da9f966e1ff39fd9ffd2e29b764d1 build --release
Time (mean ± σ): 10.883 s ± 0.491 s [User: 77.163 s, System: 5.557 s]
Range (min … max): 10.260 s … 11.633 s 5 runs
Benchmark 2: cargo +666996a7659a64c146e4765a1bba6ddc563819d6 build --release
Time (mean ± σ): 11.376 s ± 0.291 s [User: 82.654 s, System: 5.692 s]
Range (min … max): 11.085 s … 11.855 s 5 runs
Summary
cargo +56835d7ac14da9f966e1ff39fd9ffd2e29b764d1 build --release ran
1.05 ± 0.05 times faster than cargo +666996a7659a64c146e4765a1bba6ddc563819d6 build --release |
I'm trying to compile ruff now, but I can't share the results soon as I got to go |
compiler with my change is still slower... $ hyperfine "cargo +56835d7ac14da9f966e1ff39fd9ffd2e29b764d1 build --release" "cargo +666996a7659a64c146e4765a1bba6ddc563819d6 build --release" --prepare "cargo clean" --runs 5
Benchmark 1: cargo +56835d7ac14da9f966e1ff39fd9ffd2e29b764d1 build --release
Time (mean ± σ): 382.541 s ± 6.358 s [User: 3822.163 s, System: 264.707 s]
Range (min … max): 375.242 s … 389.466 s 5 runs
Benchmark 2: cargo +666996a7659a64c146e4765a1bba6ddc563819d6 build --release
Time (mean ± σ): 388.287 s ± 0.990 s [User: 3978.029 s, System: 264.682 s]
Range (min … max): 387.549 s … 390.011 s 5 runs
Summary
cargo +56835d7ac14da9f966e1ff39fd9ffd2e29b764d1 build --release ran
1.02 ± 0.02 times faster than cargo +666996a7659a64c146e4765a1bba6ddc563819d6 build --release |
when building LLVM it's obvious that in case of shared build rustc doesn't need to be recompiled, but with static builds it would be better to compile rustc again to ensure we linked proper library.
maybe I didn't understand the pipeline correctly, but it was strange for me to see that in Stage 5 LLVM is built while rustc is not
r? @Kobzol
try-job: dist-x86_64-msvc
try-job: dist-x86_64-linux