Optimize `librustc_driver.so` with BOLT #116352

Kobzol · 2023-10-02T19:30:53Z

This PR optimizes librustc_driver.so on 64-bit Linux CI with BOLT.

Code

One thing that's not clear yet to me how to resolve is how to best pass a linker flag that we need for BOLT (the second commit). It is currently passed unconditionally, which is not a good idea. We somehow have to:

Only pass it when we actually plan to use BOLT. How to best do that? config.toml entry? Environment variable? CLI flag for bootstrap? BOLT optimization is done by opt-dist, therefore bootstrap doesn't know about it by default.
Only pass it to librustc_driver.so (see performance below).

Some discussion of this flag already happened on Zulip.

Performance

Latest perf. results can be found here. Note that instruction counts are not very interesting here, there are only regressions on hello world programs. Probably caused by a larger C++ libstd (?).

Summary:

✔️ -1.8% mean improvement in cycle counts across many primary benchmarks.
✔️ -1.8% mean Max-RSS improvement.
✖️ 34 MiB (+48%) artifact size regression of librustc_driver.so.
- This is caused by building librustc_driver.so with relocations (which are required for BOLT). Hopefully, it will be fixed in the future with BOLT improvements, but now trying to reduce this size increase is tricky.
- Note that the size of this file was recently reduced in Build rustc with a single CGU on x64 Linux #115554 by pretty much the same amount (33 MiB). So the size after this PR is basically the same as it was for the last ~year.
✖️ 1.4 MiB (+53%) artifact size regression of rustc.
- This is annoying and pretty much unnecessary. It is caused by the way relocations are currently applied in this PR, because they are applied both to librustc_driver.so (where they are needed) and for rustc (where they aren't needed), since both are built with a single cargo invocation. We might need e.g. some tricks in the bootstrap rustc shim to only apply the relocation flag for the shared library and not for rustc.

CI time

CI (try build) got slower by ~5 minutes, which is fine, IMO. It can be further reduced by running LLVM and librustc_driver BOLT profile gathering at the same time (now they are gathered separately for LLVM and librustc_driver).

r? @Mark-Simulacrum

Also CC @onur-ozkan, primarily for the bootstrap linker flag issue.

Mark-Simulacrum · 2023-10-07T20:05:31Z

src/bootstrap/compile.rs

@@ -904,6 +904,11 @@ impl Step for Rustc {
            cargo.arg("-p").arg(krate);
        }

+        if compiler.stage == 1 {
+            // Relocations are required for BOLT to work.


I would've expected this if to gate on the usage of BOLT somehow -- do we have a config flag for that? I think just a generic config flag is fine ("enable-bolt-settings") is ok, we don't need it to actually do optimizations. Maybe even "opt-dist" is the right name for it.

This is what I have discussed in the PR description and on Zulip. @onur-ozkan has expressed a desire to avoid a BOLT-specific flag, although it seems like the most straightforward solution to me.

I don't have problem with doing this with using flags/config.toml. My point was more about its behaviour.

Add something like --bolt-flags, which would set the flag only for the cases where it's needed (and thus tying its behavior to opt-dist)

Like here, "which would set the flag only for the cases where it's needed" this seems very limited experience to me.

Yeah, the interaction of the flag seems very specific. However, I feel like that's also true in general of several other similar flags. For example, the PGO generate/use flags are also only performed in stage 2, and basically I don't think that they are very useful outside of opt-dist.

The fact that we pass a specific linker flag is tied to BOLT - that won't really change. The fact that we only pass it to librustc_driver is tied to the way the compiler is distributed, and unless that changes, it also don't make sense to do it in any other way. And performing BOLT on earlier stage than 2 also doesn't make sense. So from this point of view, I think that the behavior of the flag is not that limited, because it's pretty much the only way we could do it that currently makes sense.

bootstrap and opt-dist are intrinsically intertwined, for better or worse, so I don't think that this flag will change that or create some unique, "anti-systemic" solution :D

Mark-Simulacrum · 2023-10-07T20:06:06Z

One thing that's not clear yet to me how to resolve is how to best pass a linker flag that we need for BOLT (the second commit). It is currently passed unconditionally, which is not a good idea. We somehow have to: [...]

We might need e.g. some tricks in the bootstrap rustc shim to only apply the relocation flag for the shared library and not for rustc.

Given the desire to pass it only to one specific invocation of rustc, my inclination is to use an environment variable set by opt-dist and read by the custom rustc wrapper (src/bootstrap/bin/rustc.rs).

bors · 2023-10-08T02:42:10Z

☔ The latest upstream changes (presumably #114623) made this pull request unmergeable. Please resolve the merge conflicts.

rustbot · 2023-10-09T20:00:27Z

This PR changes src/bootstrap/config.rs. If appropriate, please also update CONFIG_CHANGE_HISTORY in src/bootstrap/lib.rs and change-id in config.example.toml.

Kobzol · 2023-10-09T20:00:57Z

I added the --enable-bolt-settings flag and an env. var passed to the rustc shim.

@bors try @rust-timer queue

bors · 2023-10-09T20:02:07Z

⌛ Trying commit 9a0e90f with merge cdee08f...

Optimize `librustc_driver.so` with BOLT This PR optimizes `librustc_driver.so` on 64-bit Linux CI with BOLT. ### Code One thing that's not clear yet to me how to resolve is how to best pass a linker flag that we need for BOLT (the second commit). It is currently passed unconditionally, which is not a good idea. We somehow have to: 1) Only pass it when we actually plan to use BOLT. How to best do that? `config.toml` entry? Environment variable? CLI flag for bootstrap? BOLT optimization is done by `opt-dist`, therefore bootstrap doesn't know about it by default. 2) Only pass it to `librustc_driver.so` (see performance below). Some discussion of this flag already happened on [Zulip](https://rust-lang.zulipchat.com/#narrow/stream/326414-t-infra.2Fbootstrap/topic/Adding.20a.20one-off.20linker.20flag). ### Performance Latest perf. results can be found [here](rust-lang#102487 (comment)). Note that instruction counts are not very interesting here, there are only regressions on hello world programs. Probably caused by a larger C++ libstd (?). Summary: - ✔️ `-1.8%` mean improvement in cycle counts across many primary benchmarks. - ✔️ `-1.8%` mean Max-RSS improvement. - ✖️ 34 MiB (+48%) artifact size regression of `librustc_driver.so`. - This is caused by building `librustc_driver.so` with relocations (which are required for BOLT). Hopefully, it will be [fixed](https://discourse.llvm.org/t/bolt-rfc-a-new-mode-to-rewrite-entire-binary/68674) in the future with BOLT improvements, but now trying to reduce this size increase is [tricky](rust-lang#114649). - Note that the size of this file was recently reduced in rust-lang#115554 by pretty much the same amount (33 MiB). So the size after this PR is basically the same as it was for the last ~year. - ✖️ 1.4 MiB (+53%) artifact size regression of `rustc`. - This is annoying and pretty much unnecessary. It is caused by the way relocations are currently applied in this PR, because they are applied both to `librustc_driver.so` (where they are needed) and for `rustc` (where they aren't needed), since both are built with a single cargo invocation. We might need e.g. some tricks in the bootstrap `rustc` shim to only apply the relocation flag for the shared library and not for `rustc`. ### CI time CI (try build) got slower by ~5 minutes, which is fine, IMO. It can be further reduced by running LLVM and `librustc_driver` BOLT profile gathering at the same time (now they are gathered separately for LLVM and `librustc_driver`). r? `@Mark-Simulacrum` Also CC `@onur-ozkan,` primarily for the bootstrap linker flag issue.

bors · 2023-10-09T22:04:19Z

💔 Test failed - checks-actions

Kobzol · 2023-10-11T20:40:23Z

@bors try @rust-timer queue

bors · 2023-10-11T20:41:34Z

⌛ Trying commit 482a820 with merge dbae2a1...

Optimize `librustc_driver.so` with BOLT This PR optimizes `librustc_driver.so` on 64-bit Linux CI with BOLT. ### Code One thing that's not clear yet to me how to resolve is how to best pass a linker flag that we need for BOLT (the second commit). It is currently passed unconditionally, which is not a good idea. We somehow have to: 1) Only pass it when we actually plan to use BOLT. How to best do that? `config.toml` entry? Environment variable? CLI flag for bootstrap? BOLT optimization is done by `opt-dist`, therefore bootstrap doesn't know about it by default. 2) Only pass it to `librustc_driver.so` (see performance below). Some discussion of this flag already happened on [Zulip](https://rust-lang.zulipchat.com/#narrow/stream/326414-t-infra.2Fbootstrap/topic/Adding.20a.20one-off.20linker.20flag). ### Performance Latest perf. results can be found [here](rust-lang#102487 (comment)). Note that instruction counts are not very interesting here, there are only regressions on hello world programs. Probably caused by a larger C++ libstd (?). Summary: - ✔️ `-1.8%` mean improvement in cycle counts across many primary benchmarks. - ✔️ `-1.8%` mean Max-RSS improvement. - ✖️ 34 MiB (+48%) artifact size regression of `librustc_driver.so`. - This is caused by building `librustc_driver.so` with relocations (which are required for BOLT). Hopefully, it will be [fixed](https://discourse.llvm.org/t/bolt-rfc-a-new-mode-to-rewrite-entire-binary/68674) in the future with BOLT improvements, but now trying to reduce this size increase is [tricky](rust-lang#114649). - Note that the size of this file was recently reduced in rust-lang#115554 by pretty much the same amount (33 MiB). So the size after this PR is basically the same as it was for the last ~year. - ✖️ 1.4 MiB (+53%) artifact size regression of `rustc`. - This is annoying and pretty much unnecessary. It is caused by the way relocations are currently applied in this PR, because they are applied both to `librustc_driver.so` (where they are needed) and for `rustc` (where they aren't needed), since both are built with a single cargo invocation. We might need e.g. some tricks in the bootstrap `rustc` shim to only apply the relocation flag for the shared library and not for `rustc`. ### CI time CI (try build) got slower by ~5 minutes, which is fine, IMO. It can be further reduced by running LLVM and `librustc_driver` BOLT profile gathering at the same time (now they are gathered separately for LLVM and `librustc_driver`). r? `@Mark-Simulacrum` Also CC `@onur-ozkan,` primarily for the bootstrap linker flag issue.

bors · 2023-10-11T22:03:04Z

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

rust-timer · 2023-10-15T09:05:20Z

Finished benchmarking commit (c543b6f): comparison URL.

Overall result: ❌ regressions - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	2.3%	[0.2%, 5.7%]	10
Regressions ❌ (secondary)	1.9%	[0.3%, 5.0%]	60
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.3%	[-0.3%, -0.3%]	4
All ❌✅ (primary)	2.3%	[0.2%, 5.7%]	10

Max RSS (memory usage)

Results

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

	mean	range	count
Regressions ❌ (primary)	1.2%	[0.6%, 1.6%]	9
Regressions ❌ (secondary)	3.4%	[1.7%, 5.4%]	15
Improvements ✅ (primary)	-2.6%	[-5.0%, -0.6%]	66
Improvements ✅ (secondary)	-2.9%	[-9.0%, -0.6%]	108
All ❌✅ (primary)	-2.2%	[-5.0%, 1.6%]	75

Cycles

Results

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

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-2.0%	[-3.1%, -0.9%]	70
Improvements ✅ (secondary)	-2.5%	[-4.7%, -1.9%]	18
All ❌✅ (primary)	-2.0%	[-3.1%, -0.9%]	70

Binary size

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

Bootstrap: 628.052s -> 623.517s (-0.72%)
Artifact size: 271.29 MiB -> 305.48 MiB (12.60%)

Kobzol · 2023-10-15T09:10:25Z

The interesting metrics here are cycle counts (-2% mean improvement amongst primary benchmarks) and Max-RSS (-2% mean improvement amongst primary benchmarks). Bootstrap time has also been reduced by ~5s (usually 2-3s tends to be noise for bootstrap timings, so 5s seems like a statistically significant improvement to me).

The instruction count regressions stem happens mostly for "hello-world-like" programs that contain tiny amounts of code. It's basically a fixed startup cost regression that happens because the compiler library is now ~30 MiB larger. The size of librustc_driver.so has been recently reduced by ~30 MiB by switching to a single CGU, so this mostly cancels out. It's unfortunate that we lost the size improvements, but I think that it's worth the 2% cycle wins.

@rustbot label: +perf-regression-triaged

RalfJung · 2024-03-29T08:30:52Z

src/bootstrap/bin/rustc.rs

+        if let Some("rustc_driver") = crate_name {
+            cmd.arg("-Clink-args=-Wl,-q");
+        }
+    }


This was added after the debug printing, so the extra flags won't show up when the command is printed. Was that deliberate?

Definitely not, good catch, thanks! #123189

Log BOLT args in bootstrap `rustc` shim Before, the BOLT argument would not be logged, because it was only added after the logging has happened. Found by `@RalfJung` [here](rust-lang#116352 (comment)).

Rollup merge of rust-lang#123189 - Kobzol:rustc-shim-log, r=onur-ozkan Log BOLT args in bootstrap `rustc` shim Before, the BOLT argument would not be logged, because it was only added after the logging has happened. Found by `@RalfJung` [here](rust-lang#116352 (comment)).

Log BOLT args in bootstrap `rustc` shim Before, the BOLT argument would not be logged, because it was only added after the logging has happened. Found by `@RalfJung` [here](rust-lang/rust#116352 (comment)).

rustbot assigned Mark-Simulacrum Oct 2, 2023

Mark-Simulacrum reviewed Oct 7, 2023

View reviewed changes

Kobzol added 4 commits October 9, 2023 21:46

Build GCC with support for BOLT

240a7dd

Enable relocations for BOLT

589e38a

Optimize librustc_driver.so with BOLT

dd7c5a0

Add --enable-bolt-settings bootstrap flag

9a0e90f

Kobzol force-pushed the rustc-driver-bolt branch from 5eb9a9a to 9a0e90f Compare October 9, 2023 20:00

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Oct 9, 2023

This comment has been minimized.

Sign in to view

bors 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 Oct 9, 2023

Pass BOLT settings at the correct step

482a820

This comment has been minimized.

Sign in to view

bors merged commit c543b6f into rust-lang:master Oct 14, 2023
12 checks passed

rustbot added this to the 1.75.0 milestone Oct 14, 2023

bors mentioned this pull request Oct 14, 2023

gcc: build with --disable-multilib, as it used only to bootstrap llvm #114523

Closed

Kobzol deleted the rustc-driver-bolt branch October 14, 2023 23:42

onur-ozkan mentioned this pull request Oct 15, 2023

x.py zsh completion support #116643

Merged

This was referenced Oct 15, 2023

Use BOLT in CI to optimize librustc_driver #102487

Closed

Tracking issue for speeding up rustc via its build configuration #103595

Open

rustbot added the perf-regression-triaged The performance regression has been triaged. label Oct 15, 2023

Veykril mentioned this pull request Oct 17, 2023

Pull upstream master 2023 10 17 ferrocene/ferrocene#54

Merged

zamazan4ik mentioned this pull request Dec 4, 2023

Evaluate using Profile-Guided Optimization (PGO) and Post-Link Optimization (PLO) for C++ part and LLVM clasp-developers/clasp#1526

Open

zamazan4ik mentioned this pull request Jan 11, 2024

Evaluate using Profile-Guided Optimization (PGO) and Post-Link Optimization (PLO) shady-gang/shady#18

Closed

RalfJung reviewed Mar 29, 2024

View reviewed changes

Kobzol mentioned this pull request Mar 29, 2024

Log BOLT args in bootstrap rustc shim #123189

Merged

Urgau mentioned this pull request Jul 16, 2024

[perf] Change stable hasher to Blake3 #127754

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Optimize `librustc_driver.so` with BOLT #116352

Optimize `librustc_driver.so` with BOLT #116352

Kobzol commented Oct 2, 2023

Mark-Simulacrum Oct 7, 2023

Kobzol Oct 9, 2023

onur-ozkan Oct 9, 2023

Kobzol Oct 9, 2023 •

edited

Loading

Mark-Simulacrum commented Oct 7, 2023

bors commented Oct 8, 2023

rustbot commented Oct 9, 2023

Kobzol commented Oct 9, 2023

This comment has been minimized.

bors commented Oct 9, 2023

This comment has been minimized.

bors commented Oct 9, 2023

Kobzol commented Oct 11, 2023

This comment has been minimized.

bors commented Oct 11, 2023

bors commented Oct 11, 2023

This comment has been minimized.

rust-timer commented Oct 15, 2023

Kobzol commented Oct 15, 2023 •

edited

Loading

RalfJung Mar 29, 2024

Kobzol Mar 29, 2024

Optimize librustc_driver.so with BOLT #116352

Optimize librustc_driver.so with BOLT #116352

Conversation

Kobzol commented Oct 2, 2023

Code

Performance

CI time

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Kobzol Oct 9, 2023 • edited Loading

Choose a reason for hiding this comment

Mark-Simulacrum commented Oct 7, 2023

bors commented Oct 8, 2023

rustbot commented Oct 9, 2023

Kobzol commented Oct 9, 2023

This comment has been minimized.

bors commented Oct 9, 2023

This comment has been minimized.

bors commented Oct 9, 2023

Kobzol commented Oct 11, 2023

This comment has been minimized.

bors commented Oct 11, 2023

bors commented Oct 11, 2023

This comment has been minimized.

rust-timer commented Oct 15, 2023

Overall result: ❌ regressions - ACTION NEEDED

Kobzol commented Oct 15, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Optimize `librustc_driver.so` with BOLT #116352

Optimize `librustc_driver.so` with BOLT #116352

Kobzol Oct 9, 2023 •

edited

Loading

Kobzol commented Oct 15, 2023 •

edited

Loading