Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add an option to tune compiler crates' CGUs to bootstrap #107560

Closed
wants to merge 2 commits into from
Closed

Add an option to tune compiler crates' CGUs to bootstrap #107560

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b3fd7ad
Add an option to tune compiler crates' CGUs to bootstrap
Zoxc Feb 1, 2023
0c55b04
WIP performance test
Zoxc Feb 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions config.toml.example
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,12 @@ changelog-seen = 2
# Uses the rustc defaults: https://doc.rust-lang.org/rustc/codegen-options/index.html#codegen-units
#codegen-units = if incremental { 256 } else { 16 }

# Tweaks the numbers of codegen units to improve compile time, typically by
# using a single codegen unit for crates. The `codegen-units` configuration is
# preferred over this field. This is recommended to set for release builds and CI,
# but not for development as it can slow down partial rebuilds.
#codegen-units-fast = false

# Sets the number of codegen units to build the standard library with,
# regardless of what the codegen-unit setting for the rest of the compiler is.
# NOTE: building with anything other than 1 is known to occasionally have bugs.
Expand Down
23 changes: 23 additions & 0 deletions src/bootstrap/bin/rustc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,29 @@ fn main() {
cmd.arg("-Ztime-passes");
}
}

if let Some(profile) = env::var_os("RUSTC_TUNE_COMPILER_CODEGEN_UNITS") {
let cgus = match crate_name {
// This crate is large and at the tail of the compiler crates, give it extra CGUs.
//"rustc_query_impl" => Some(96),
// This compiles after all other crates so give it default CGUs to speed it up.
"rustc_driver" => None,
_ => {
if profile == "fast" {
Some(1)
} else {
if crate_name.starts_with("rustc_") {
None
} else {
// Compile crates.io crates with a single CGU for faster compile times
Some(1)
Comment on lines +81 to +88
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand - doesn't decreasing the CGUs slow down compilation because we aren't running multiple LLVM threads in parallel? Or do you mean this makes the compiler faster at runtime in exchange for slower bootstrap times? In that case "fast" seems like a misleading name ...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the crates are still compiled in parallel even if a single crate is limited to 1 thread.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, but rustc_driver still can't start building until all other crates have finished, and this is still pushing up the overall build time.

I'm not saying it's a bad change, for CI it makes sense the same way that PGO makes sense, I'd just like to find a different name. Maybe "reduce-codegen-units" or something like that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It reduces build time with 7 CPU cores, it may increase it for high core count CPUs, so I agree the name is not ideal. Maybe codegen-units-reduce?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will be most useful in combination with LTO, rarely by itself right ? So maybe it could be one of the LTO options, since that's already an enum that doesn't exactly match -C lto's values (e.g. "thin-local"). Something like rust.lto = "thin-1cgu" ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not quite. This option is intended to reduce the compilation time of rustc. LTO increases compile time to give better runtime performance and is better paired with the existing codegen-units=1 so all crates have a single CGU.

}
}
}
};

cgus.map(|cgus| cmd.arg(&format!("-Ccodegen-units={}", cgus)));
}
}

// Print backtrace in case of ICE
Expand Down
3 changes: 3 additions & 0 deletions src/bootstrap/builder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1884,6 +1884,9 @@ impl<'a> Builder<'a> {
(Mode::Std, Some(n), _) | (_, _, Some(n)) => {
cargo.env(profile_var("CODEGEN_UNITS"), n.to_string());
}
(Mode::Rustc, _, _) => {
cargo.env("RUSTC_TUNE_COMPILER_CODEGEN_UNITS", "fast");
}
_ => {
// Don't set anything
}
Expand Down
3 changes: 3 additions & 0 deletions src/bootstrap/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ pub struct Config {
pub rust_optimize: bool,
pub rust_codegen_units: Option<u32>,
pub rust_codegen_units_std: Option<u32>,
pub rust_codegen_units_fast: bool,
pub rust_debug_assertions: bool,
pub rust_debug_assertions_std: bool,
pub rust_overflow_checks: bool,
Expand Down Expand Up @@ -717,6 +718,7 @@ define_config! {
debug: Option<bool> = "debug",
codegen_units: Option<u32> = "codegen-units",
codegen_units_std: Option<u32> = "codegen-units-std",
codegen_units_fast: Option<bool> = "codegen-units-fast",
debug_assertions: Option<bool> = "debug-assertions",
debug_assertions_std: Option<bool> = "debug-assertions-std",
overflow_checks: Option<bool> = "overflow-checks",
Expand Down Expand Up @@ -1130,6 +1132,7 @@ impl Config {

config.rust_codegen_units = rust.codegen_units.map(threads_from_config);
config.rust_codegen_units_std = rust.codegen_units_std.map(threads_from_config);
set(&mut config.rust_codegen_units_fast, rust.codegen_units_fast);
config.rust_profile_use = flags.rust_profile_use.or(rust.profile_use);
config.rust_profile_generate = flags.rust_profile_generate.or(rust.profile_generate);
config.download_rustc_commit = config.download_ci_rustc_commit(rust.download_rustc);
Expand Down
1 change: 1 addition & 0 deletions src/ci/run.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ RUST_CONFIGURE_ARGS="$RUST_CONFIGURE_ARGS --disable-manage-submodules"
RUST_CONFIGURE_ARGS="$RUST_CONFIGURE_ARGS --enable-locked-deps"
RUST_CONFIGURE_ARGS="$RUST_CONFIGURE_ARGS --enable-cargo-native-static"
RUST_CONFIGURE_ARGS="$RUST_CONFIGURE_ARGS --set rust.codegen-units-std=1"
RUST_CONFIGURE_ARGS="$RUST_CONFIGURE_ARGS --set rust.codegen-units-fast=true"

# Only produce xz tarballs on CI. gz tarballs will be generated by the release
# process by recompressing the existing xz ones. This decreases the storage
Expand Down