Skip to content

Commit

Permalink
Rollup merge of #126318 - Kobzol:bootstrap-perf, r=onur-ozkan
Browse files Browse the repository at this point in the history
Add a `x perf` command for integrating bootstrap with `rustc-perf`

This PR adds a new `x perf` command to bootstrap. The idea is to let rustc developers profile (`profile_local`) and benchmark (`bench_local`) a stage1/stage2 compiler directly from within `rust`.

Before, if you wanted to use `rustc-perf`, you had to clone it, set it up, copy the `rustc` sysroot after every change to `rust` etc. This is an attempt to automate that.

I opened this PR mostly for discussion. My idea is to offer an interface that looks something like this (a random sample of commands):
```bash
x perf --stage 2 profile eprintln
x perf --stage1 profile cachegrind
x perf benchmark --id baseline
x perf benchmark --id after-edit
x perf cmp baseline after-edit
```

In this PR, I'd like to only implement the simplest case (`profile_local (eprintln)`), because that only requires a single sysroot (you don't compare anything), and it's relatively easy to set up. Also, I'd like to avoid forcing developers to deal with the rustc-perf UI, so more complex use-cases (like benchmarking two sysroots and comparing the results) should probably wait for rust-lang/rustc-perf#1734 (which is hopefully coming along soon-ish).

I'm not sure if it's better to do this in bootstrap directly, or if I should create some shim tool that will receive a `rustc` sysroot, and offer a simplified CLI on top of `rustc-perf`.

## Why is a separate CLI needed?
We definitely need to add some support to bootstrap to automate preparing `rustc-perf` and the `rustc` sysroot, but in theory after that we could just let people invoke `rustc-perf` manually. While that is definitely possible, you'd need to manually figure out where is your sysroot located, which seems annoying to me. The `rustc-perf` CLI is also relatively complex, and for this use-case it makes sense to only use a subset of it. So I thought that it would be better to offer a simplified interface on top of it that would make life easier for contributors. But maybe it's not worth it.

CC `@onur-ozkan`
  • Loading branch information
matthiaskrgr authored Jun 22, 2024
2 parents f3ced9d + 0bd58d8 commit 162120b
Show file tree
Hide file tree
Showing 11 changed files with 380 additions and 8 deletions.
1 change: 1 addition & 0 deletions src/bootstrap/src/core/build_steps/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ pub(crate) mod doc;
pub(crate) mod format;
pub(crate) mod install;
pub(crate) mod llvm;
pub(crate) mod perf;
pub(crate) mod run;
pub(crate) mod setup;
pub(crate) mod suggest;
Expand Down
45 changes: 45 additions & 0 deletions src/bootstrap/src/core/build_steps/perf.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
use std::process::Command;

use crate::core::build_steps::compile::{Std, Sysroot};
use crate::core::build_steps::tool::RustcPerf;
use crate::core::builder::Builder;
use crate::core::config::DebuginfoLevel;

/// Performs profiling using `rustc-perf` on a built version of the compiler.
pub fn perf(builder: &Builder<'_>) {
let collector = builder.ensure(RustcPerf {
compiler: builder.compiler(0, builder.config.build),
target: builder.config.build,
});

if builder.build.config.rust_debuginfo_level_rustc == DebuginfoLevel::None {
builder.info(r#"WARNING: You are compiling rustc without debuginfo, this will make profiling less useful.
Consider setting `rust.debuginfo-level = 1` in `config.toml`."#);
}

let compiler = builder.compiler(builder.top_stage, builder.config.build);
builder.ensure(Std::new(compiler, builder.config.build));
let sysroot = builder.ensure(Sysroot::new(compiler));
let rustc = sysroot.join("bin/rustc");

let results_dir = builder.build.tempdir().join("rustc-perf");

let mut cmd = Command::new(collector);
let cmd = cmd
.arg("profile_local")
.arg("eprintln")
.arg("--out-dir")
.arg(&results_dir)
.arg("--include")
.arg("helloworld")
.arg(&rustc);

builder.info(&format!("Running `rustc-perf` using `{}`", rustc.display()));

// We need to set the working directory to `src/tools/perf`, so that it can find the directory
// with compile-time benchmarks.
let cmd = cmd.current_dir(builder.src.join("src/tools/rustc-perf"));
builder.build.run(cmd);

builder.info(&format!("You can find the results at `{}`", results_dir.display()));
}
81 changes: 77 additions & 4 deletions src/bootstrap/src/core/build_steps/tool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ struct ToolBuild {
extra_features: Vec<String>,
/// Nightly-only features that are allowed (comma-separated list).
allow_features: &'static str,
/// Additional arguments to pass to the `cargo` invocation.
cargo_args: Vec<String>,
}

impl Builder<'_> {
Expand Down Expand Up @@ -100,6 +102,7 @@ impl Step for ToolBuild {
if !self.allow_features.is_empty() {
cargo.allow_features(self.allow_features);
}
cargo.args(self.cargo_args);
let _guard = builder.msg_tool(
Kind::Build,
self.mode,
Expand All @@ -126,10 +129,7 @@ impl Step for ToolBuild {
if tool == "tidy" {
tool = "rust-tidy";
}
let cargo_out = builder.cargo_out(compiler, self.mode, target).join(exe(tool, target));
let bin = builder.tools_dir(compiler).join(exe(tool, target));
builder.copy_link(&cargo_out, &bin);
bin
copy_link_tool_bin(builder, self.compiler, self.target, self.mode, tool)
}
}
}
Expand Down Expand Up @@ -214,6 +214,21 @@ pub fn prepare_tool_cargo(
cargo
}

/// Links a built tool binary with the given `name` from the build directory to the
/// tools directory.
fn copy_link_tool_bin(
builder: &Builder<'_>,
compiler: Compiler,
target: TargetSelection,
mode: Mode,
name: &str,
) -> PathBuf {
let cargo_out = builder.cargo_out(compiler, mode, target).join(exe(name, target));
let bin = builder.tools_dir(compiler).join(exe(name, target));
builder.copy_link(&cargo_out, &bin);
bin
}

macro_rules! bootstrap_tool {
($(
$name:ident, $path:expr, $tool_name:expr
Expand Down Expand Up @@ -283,6 +298,7 @@ macro_rules! bootstrap_tool {
},
extra_features: vec![],
allow_features: concat!($($allow_features)*),
cargo_args: vec![]
})
}
}
Expand Down Expand Up @@ -349,10 +365,60 @@ impl Step for OptimizedDist {
source_type: SourceType::InTree,
extra_features: Vec::new(),
allow_features: "",
cargo_args: Vec::new(),
})
}
}

/// The [rustc-perf](https://github.com/rust-lang/rustc-perf) benchmark suite, which is added
/// as a submodule at `src/tools/rustc-perf`.
#[derive(Debug, Clone, Hash, PartialEq, Eq)]
pub struct RustcPerf {
pub compiler: Compiler,
pub target: TargetSelection,
}

impl Step for RustcPerf {
/// Path to the built `collector` binary.
type Output = PathBuf;

fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
run.path("src/tools/rustc-perf")
}

fn make_run(run: RunConfig<'_>) {
run.builder.ensure(RustcPerf {
compiler: run.builder.compiler(0, run.builder.config.build),
target: run.target,
});
}

fn run(self, builder: &Builder<'_>) -> PathBuf {
// We need to ensure the rustc-perf submodule is initialized.
builder.update_submodule(Path::new("src/tools/rustc-perf"));

let tool = ToolBuild {
compiler: self.compiler,
target: self.target,
tool: "collector",
mode: Mode::ToolBootstrap,
path: "src/tools/rustc-perf",
source_type: SourceType::Submodule,
extra_features: Vec::new(),
allow_features: "",
// Only build the collector package, which is used for benchmarking through
// a CLI.
cargo_args: vec!["-p".to_string(), "collector".to_string()],
};
let collector_bin = builder.ensure(tool.clone());
// We also need to symlink the `rustc-fake` binary to the corresponding directory,
// because `collector` expects it in the same directory.
copy_link_tool_bin(builder, tool.compiler, tool.target, tool.mode, "rustc-fake");

collector_bin
}
}

#[derive(Debug, Clone, Hash, PartialEq, Eq, Ord, PartialOrd)]
pub struct ErrorIndex {
pub compiler: Compiler,
Expand Down Expand Up @@ -403,6 +469,7 @@ impl Step for ErrorIndex {
source_type: SourceType::InTree,
extra_features: Vec::new(),
allow_features: "",
cargo_args: Vec::new(),
})
}
}
Expand Down Expand Up @@ -437,6 +504,7 @@ impl Step for RemoteTestServer {
source_type: SourceType::InTree,
extra_features: Vec::new(),
allow_features: "",
cargo_args: Vec::new(),
})
}
}
Expand Down Expand Up @@ -595,6 +663,7 @@ impl Step for Cargo {
source_type: SourceType::Submodule,
extra_features: Vec::new(),
allow_features: "",
cargo_args: Vec::new(),
})
}
}
Expand Down Expand Up @@ -622,6 +691,7 @@ impl Step for LldWrapper {
source_type: SourceType::InTree,
extra_features: Vec::new(),
allow_features: "",
cargo_args: Vec::new(),
})
}
}
Expand Down Expand Up @@ -670,6 +740,7 @@ impl Step for RustAnalyzer {
extra_features: vec!["in-rust-tree".to_owned()],
source_type: SourceType::InTree,
allow_features: RustAnalyzer::ALLOW_FEATURES,
cargo_args: Vec::new(),
})
}
}
Expand Down Expand Up @@ -717,6 +788,7 @@ impl Step for RustAnalyzerProcMacroSrv {
extra_features: vec!["in-rust-tree".to_owned()],
source_type: SourceType::InTree,
allow_features: RustAnalyzer::ALLOW_FEATURES,
cargo_args: Vec::new(),
});

// Copy `rust-analyzer-proc-macro-srv` to `<sysroot>/libexec/`
Expand Down Expand Up @@ -923,6 +995,7 @@ macro_rules! tool_extended {
extra_features: $sel.extra_features,
source_type: SourceType::InTree,
allow_features: concat!($($allow_features)*),
cargo_args: vec![]
});

if (false $(|| !$add_bins_to_sysroot.is_empty())?) && $sel.compiler.stage > 0 {
Expand Down
9 changes: 7 additions & 2 deletions src/bootstrap/src/core/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -666,6 +666,7 @@ pub enum Kind {
Setup,
Suggest,
Vendor,
Perf,
}

impl Kind {
Expand All @@ -687,6 +688,7 @@ impl Kind {
Kind::Setup => "setup",
Kind::Suggest => "suggest",
Kind::Vendor => "vendor",
Kind::Perf => "perf",
}
}

Expand All @@ -698,6 +700,7 @@ impl Kind {
Kind::Run => "Running",
Kind::Suggest => "Suggesting",
Kind::Clippy => "Linting",
Kind::Perf => "Profiling & benchmarking",
_ => {
let title_letter = self.as_str()[0..1].to_ascii_uppercase();
return format!("{title_letter}{}ing", &self.as_str()[1..]);
Expand Down Expand Up @@ -749,7 +752,8 @@ impl<'a> Builder<'a> {
tool::RustdocGUITest,
tool::OptimizedDist,
tool::CoverageDump,
tool::LlvmBitcodeLinker
tool::LlvmBitcodeLinker,
tool::RustcPerf,
),
Kind::Clippy => describe!(
clippy::Std,
Expand Down Expand Up @@ -945,7 +949,7 @@ impl<'a> Builder<'a> {
Kind::Clean => describe!(clean::CleanAll, clean::Rustc, clean::Std),
Kind::Vendor => describe!(vendor::Vendor),
// special-cased in Build::build()
Kind::Format | Kind::Suggest => vec![],
Kind::Format | Kind::Suggest | Kind::Perf => vec![],
}
}

Expand Down Expand Up @@ -1017,6 +1021,7 @@ impl<'a> Builder<'a> {
path.as_ref().map_or([].as_slice(), |path| std::slice::from_ref(path)),
),
Subcommand::Vendor { .. } => (Kind::Vendor, &paths[..]),
Subcommand::Perf { .. } => (Kind::Perf, &paths[..]),
};

Self::new_internal(build, kind, paths.to_owned())
Expand Down
4 changes: 3 additions & 1 deletion src/bootstrap/src/core/config/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2043,6 +2043,7 @@ impl Config {
Subcommand::Bench { .. } => flags.stage.or(bench_stage).unwrap_or(2),
Subcommand::Dist { .. } => flags.stage.or(dist_stage).unwrap_or(2),
Subcommand::Install { .. } => flags.stage.or(install_stage).unwrap_or(2),
Subcommand::Perf { .. } => flags.stage.unwrap_or(1),
// These are all bootstrap tools, which don't depend on the compiler.
// The stage we pass shouldn't matter, but use 0 just in case.
Subcommand::Clean { .. }
Expand Down Expand Up @@ -2080,7 +2081,8 @@ impl Config {
| Subcommand::Setup { .. }
| Subcommand::Format { .. }
| Subcommand::Suggest { .. }
| Subcommand::Vendor { .. } => {}
| Subcommand::Vendor { .. }
| Subcommand::Perf { .. } => {}
}
}

Expand Down
4 changes: 4 additions & 0 deletions src/bootstrap/src/core/config/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,9 @@ Arguments:
#[arg(long)]
versioned_dirs: bool,
},
/// Perform profiling and benchmarking of the compiler using the
/// `rustc-perf` benchmark suite.
Perf {},
}

impl Subcommand {
Expand All @@ -490,6 +493,7 @@ impl Subcommand {
Subcommand::Setup { .. } => Kind::Setup,
Subcommand::Suggest { .. } => Kind::Suggest,
Subcommand::Vendor { .. } => Kind::Vendor,
Subcommand::Perf { .. } => Kind::Perf,
}
}

Expand Down
3 changes: 3 additions & 0 deletions src/bootstrap/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -659,6 +659,9 @@ impl Build {
Subcommand::Suggest { run } => {
return core::build_steps::suggest::suggest(&builder::Builder::new(self), *run);
}
Subcommand::Perf { .. } => {
return core::build_steps::perf::perf(&builder::Builder::new(self));
}
_ => (),
}

Expand Down
35 changes: 35 additions & 0 deletions src/etc/completions/x.py.fish
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ complete -c x.py -n "__fish_use_subcommand" -f -a "run" -d 'Run tools contained
complete -c x.py -n "__fish_use_subcommand" -f -a "setup" -d 'Set up the environment for development'
complete -c x.py -n "__fish_use_subcommand" -f -a "suggest" -d 'Suggest a subset of tests to run, based on modified files'
complete -c x.py -n "__fish_use_subcommand" -f -a "vendor" -d 'Vendor dependencies'
complete -c x.py -n "__fish_use_subcommand" -f -a "perf" -d 'Perform profiling and benchmarking of the compiler using the `rustc-perf` benchmark suite'
complete -c x.py -n "__fish_seen_subcommand_from build" -l config -d 'TOML configuration file for build' -r -F
complete -c x.py -n "__fish_seen_subcommand_from build" -l build-dir -d 'Build directory, overrides `build.build-dir` in `config.toml`' -r -f -a "(__fish_complete_directories)"
complete -c x.py -n "__fish_seen_subcommand_from build" -l build -d 'build target of the stage0 compiler' -r -f
Expand Down Expand Up @@ -628,3 +629,37 @@ complete -c x.py -n "__fish_seen_subcommand_from vendor" -l llvm-profile-generat
complete -c x.py -n "__fish_seen_subcommand_from vendor" -l enable-bolt-settings -d 'Enable BOLT link flags'
complete -c x.py -n "__fish_seen_subcommand_from vendor" -l skip-stage0-validation -d 'Skip stage0 compiler validation'
complete -c x.py -n "__fish_seen_subcommand_from vendor" -s h -l help -d 'Print help (see more with \'--help\')'
complete -c x.py -n "__fish_seen_subcommand_from perf" -l config -d 'TOML configuration file for build' -r -F
complete -c x.py -n "__fish_seen_subcommand_from perf" -l build-dir -d 'Build directory, overrides `build.build-dir` in `config.toml`' -r -f -a "(__fish_complete_directories)"
complete -c x.py -n "__fish_seen_subcommand_from perf" -l build -d 'build target of the stage0 compiler' -r -f
complete -c x.py -n "__fish_seen_subcommand_from perf" -l host -d 'host targets to build' -r -f
complete -c x.py -n "__fish_seen_subcommand_from perf" -l target -d 'target targets to build' -r -f
complete -c x.py -n "__fish_seen_subcommand_from perf" -l exclude -d 'build paths to exclude' -r -F
complete -c x.py -n "__fish_seen_subcommand_from perf" -l skip -d 'build paths to skip' -r -F
complete -c x.py -n "__fish_seen_subcommand_from perf" -l rustc-error-format -r -f
complete -c x.py -n "__fish_seen_subcommand_from perf" -l on-fail -d 'command to run on failure' -r -f -a "(__fish_complete_command)"
complete -c x.py -n "__fish_seen_subcommand_from perf" -l stage -d 'stage to build (indicates compiler to use/test, e.g., stage 0 uses the bootstrap compiler, stage 1 the stage 0 rustc artifacts, etc.)' -r -f
complete -c x.py -n "__fish_seen_subcommand_from perf" -l keep-stage -d 'stage(s) to keep without recompiling (pass multiple times to keep e.g., both stages 0 and 1)' -r -f
complete -c x.py -n "__fish_seen_subcommand_from perf" -l keep-stage-std -d 'stage(s) of the standard library to keep without recompiling (pass multiple times to keep e.g., both stages 0 and 1)' -r -f
complete -c x.py -n "__fish_seen_subcommand_from perf" -l src -d 'path to the root of the rust checkout' -r -f -a "(__fish_complete_directories)"
complete -c x.py -n "__fish_seen_subcommand_from perf" -s j -l jobs -d 'number of jobs to run in parallel' -r -f
complete -c x.py -n "__fish_seen_subcommand_from perf" -l warnings -d 'if value is deny, will deny warnings if value is warn, will emit warnings otherwise, use the default configured behaviour' -r -f -a "{deny '',warn '',default ''}"
complete -c x.py -n "__fish_seen_subcommand_from perf" -l error-format -d 'rustc error format' -r -f
complete -c x.py -n "__fish_seen_subcommand_from perf" -l color -d 'whether to use color in cargo and rustc output' -r -f -a "{always '',never '',auto ''}"
complete -c x.py -n "__fish_seen_subcommand_from perf" -l llvm-skip-rebuild -d 'whether rebuilding llvm should be skipped, overriding `skip-rebuld` in config.toml' -r -f -a "{true '',false ''}"
complete -c x.py -n "__fish_seen_subcommand_from perf" -l rust-profile-generate -d 'generate PGO profile with rustc build' -r -F
complete -c x.py -n "__fish_seen_subcommand_from perf" -l rust-profile-use -d 'use PGO profile for rustc build' -r -F
complete -c x.py -n "__fish_seen_subcommand_from perf" -l llvm-profile-use -d 'use PGO profile for LLVM build' -r -F
complete -c x.py -n "__fish_seen_subcommand_from perf" -l reproducible-artifact -d 'Additional reproducible artifacts that should be added to the reproducible artifacts archive' -r
complete -c x.py -n "__fish_seen_subcommand_from perf" -l set -d 'override options in config.toml' -r -f
complete -c x.py -n "__fish_seen_subcommand_from perf" -s v -l verbose -d 'use verbose output (-vv for very verbose)'
complete -c x.py -n "__fish_seen_subcommand_from perf" -s i -l incremental -d 'use incremental compilation'
complete -c x.py -n "__fish_seen_subcommand_from perf" -l include-default-paths -d 'include default paths in addition to the provided ones'
complete -c x.py -n "__fish_seen_subcommand_from perf" -l dry-run -d 'dry run; don\'t build anything'
complete -c x.py -n "__fish_seen_subcommand_from perf" -l dump-bootstrap-shims -d 'Indicates whether to dump the work done from bootstrap shims'
complete -c x.py -n "__fish_seen_subcommand_from perf" -l json-output -d 'use message-format=json'
complete -c x.py -n "__fish_seen_subcommand_from perf" -l bypass-bootstrap-lock -d 'Bootstrap uses this value to decide whether it should bypass locking the build process. This is rarely needed (e.g., compiling the std library for different targets in parallel)'
complete -c x.py -n "__fish_seen_subcommand_from perf" -l llvm-profile-generate -d 'generate PGO profile with llvm built for rustc'
complete -c x.py -n "__fish_seen_subcommand_from perf" -l enable-bolt-settings -d 'Enable BOLT link flags'
complete -c x.py -n "__fish_seen_subcommand_from perf" -l skip-stage0-validation -d 'Skip stage0 compiler validation'
complete -c x.py -n "__fish_seen_subcommand_from perf" -s h -l help -d 'Print help (see more with \'--help\')'
Loading

0 comments on commit 162120b

Please sign in to comment.