diff options
| author | Matthias Krüger <matthias.krueger@famsik.de> | 2024-06-22 19:33:56 +0200 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2024-06-22 19:33:56 +0200 |
| commit | 162120b4fa85639960c9fc28cb7c9444c20e5017 (patch) | |
| tree | fbdf602b41935880363bd9ed03d72034ec8ae698 /src/bootstrap | |
| parent | f3ced9d5406d83bb8b300603da494cd67a85dad3 (diff) | |
| parent | 0bd58d8122aa6b6a1ed0a2db6c6a4c5f55ae75c2 (diff) | |
| download | rust-162120b4fa85639960c9fc28cb7c9444c20e5017.tar.gz rust-162120b4fa85639960c9fc28cb7c9444c20e5017.zip | |
Rollup merge of #126318 - Kobzol:bootstrap-perf, r=onur-ozkan
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 https://github.com/rust-lang/rustc-perf/issues/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`
Diffstat (limited to 'src/bootstrap')
| -rw-r--r-- | src/bootstrap/src/core/build_steps/mod.rs | 1 | ||||
| -rw-r--r-- | src/bootstrap/src/core/build_steps/perf.rs | 45 | ||||
| -rw-r--r-- | src/bootstrap/src/core/build_steps/tool.rs | 81 | ||||
| -rw-r--r-- | src/bootstrap/src/core/builder.rs | 9 | ||||
| -rw-r--r-- | src/bootstrap/src/core/config/config.rs | 4 | ||||
| -rw-r--r-- | src/bootstrap/src/core/config/flags.rs | 4 | ||||
| -rw-r--r-- | src/bootstrap/src/lib.rs | 3 |
7 files changed, 140 insertions, 7 deletions
diff --git a/src/bootstrap/src/core/build_steps/mod.rs b/src/bootstrap/src/core/build_steps/mod.rs index 381ee7ef53b..004174e35e1 100644 --- a/src/bootstrap/src/core/build_steps/mod.rs +++ b/src/bootstrap/src/core/build_steps/mod.rs @@ -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; diff --git a/src/bootstrap/src/core/build_steps/perf.rs b/src/bootstrap/src/core/build_steps/perf.rs new file mode 100644 index 00000000000..9d70ca6bd71 --- /dev/null +++ b/src/bootstrap/src/core/build_steps/perf.rs @@ -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())); +} diff --git a/src/bootstrap/src/core/build_steps/tool.rs b/src/bootstrap/src/core/build_steps/tool.rs index 613484788b6..850c8bfe2f8 100644 --- a/src/bootstrap/src/core/build_steps/tool.rs +++ b/src/bootstrap/src/core/build_steps/tool.rs @@ -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<'_> { @@ -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, @@ -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) } } } @@ -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 @@ -283,6 +298,7 @@ macro_rules! bootstrap_tool { }, extra_features: vec![], allow_features: concat!($($allow_features)*), + cargo_args: vec![] }) } } @@ -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, @@ -403,6 +469,7 @@ impl Step for ErrorIndex { source_type: SourceType::InTree, extra_features: Vec::new(), allow_features: "", + cargo_args: Vec::new(), }) } } @@ -437,6 +504,7 @@ impl Step for RemoteTestServer { source_type: SourceType::InTree, extra_features: Vec::new(), allow_features: "", + cargo_args: Vec::new(), }) } } @@ -595,6 +663,7 @@ impl Step for Cargo { source_type: SourceType::Submodule, extra_features: Vec::new(), allow_features: "", + cargo_args: Vec::new(), }) } } @@ -622,6 +691,7 @@ impl Step for LldWrapper { source_type: SourceType::InTree, extra_features: Vec::new(), allow_features: "", + cargo_args: Vec::new(), }) } } @@ -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(), }) } } @@ -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/` @@ -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 { diff --git a/src/bootstrap/src/core/builder.rs b/src/bootstrap/src/core/builder.rs index 3c4806a1311..a638d6f31db 100644 --- a/src/bootstrap/src/core/builder.rs +++ b/src/bootstrap/src/core/builder.rs @@ -666,6 +666,7 @@ pub enum Kind { Setup, Suggest, Vendor, + Perf, } impl Kind { @@ -687,6 +688,7 @@ impl Kind { Kind::Setup => "setup", Kind::Suggest => "suggest", Kind::Vendor => "vendor", + Kind::Perf => "perf", } } @@ -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..]); @@ -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, @@ -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![], } } @@ -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()) diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index 0438dee7241..948f97e746f 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -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 { .. } @@ -2080,7 +2081,8 @@ impl Config { | Subcommand::Setup { .. } | Subcommand::Format { .. } | Subcommand::Suggest { .. } - | Subcommand::Vendor { .. } => {} + | Subcommand::Vendor { .. } + | Subcommand::Perf { .. } => {} } } diff --git a/src/bootstrap/src/core/config/flags.rs b/src/bootstrap/src/core/config/flags.rs index 83def0c6df0..eb5152a3831 100644 --- a/src/bootstrap/src/core/config/flags.rs +++ b/src/bootstrap/src/core/config/flags.rs @@ -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 { @@ -490,6 +493,7 @@ impl Subcommand { Subcommand::Setup { .. } => Kind::Setup, Subcommand::Suggest { .. } => Kind::Suggest, Subcommand::Vendor { .. } => Kind::Vendor, + Subcommand::Perf { .. } => Kind::Perf, } } diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs index 9b414b24d2f..dfc30298c28 100644 --- a/src/bootstrap/src/lib.rs +++ b/src/bootstrap/src/lib.rs @@ -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)); + } _ => (), } |
