about summary refs log tree commit diff
path: root/src/bootstrap
diff options
context:
space:
mode:
authorMatthias Krüger <matthias.krueger@famsik.de>2024-06-22 19:33:56 +0200
committerGitHub <noreply@github.com>2024-06-22 19:33:56 +0200
commit162120b4fa85639960c9fc28cb7c9444c20e5017 (patch)
treefbdf602b41935880363bd9ed03d72034ec8ae698 /src/bootstrap
parentf3ced9d5406d83bb8b300603da494cd67a85dad3 (diff)
parent0bd58d8122aa6b6a1ed0a2db6c6a4c5f55ae75c2 (diff)
downloadrust-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.rs1
-rw-r--r--src/bootstrap/src/core/build_steps/perf.rs45
-rw-r--r--src/bootstrap/src/core/build_steps/tool.rs81
-rw-r--r--src/bootstrap/src/core/builder.rs9
-rw-r--r--src/bootstrap/src/core/config/config.rs4
-rw-r--r--src/bootstrap/src/core/config/flags.rs4
-rw-r--r--src/bootstrap/src/lib.rs3
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));
+            }
             _ => (),
         }