Merge pull request #738 from Mark-Simulacrum/share-build-cache

Share target directories between benchmarks

Author: Mark-Simulacrum

collector/src/execute.rs

@@ -198,7 +198,8 @@ impl Profiler {
 
 struct CargoProcess<'a> {
     compiler: Compiler<'a>,
-    cwd: &'a Path,
+    src_dir: &'a Path,
+    target_directory: &'a Path,
     build_kind: BuildKind,
     incremental: bool,
     processor_etc: Option<(&'a mut dyn Processor, RunKind, &'a str, Option<&'a Patch>)>,
@@ -227,7 +228,7 @@ impl<'a> CargoProcess<'a> {
         self
     }
 
-    fn base_command(&self, cwd: &Path, subcommand: &str) -> Command {
+    fn base_command(&self, subcommand: &str) -> Command {
         let mut cmd = Command::new(Path::new(self.compiler.cargo));
         cmd
             // Not all cargo invocations (e.g. `cargo clean`) need all of these
@@ -239,7 +240,7 @@ impl<'a> CargoProcess<'a> {
             // and any in-tree dependencies, and we don't want that; it wastes
             // time.
             .env("CARGO_INCREMENTAL", "0")
-            .current_dir(cwd)
+            .current_dir(self.src_dir)
             .arg(subcommand)
             .arg("--manifest-path")
             .arg(&self.manifest_path);
@@ -250,10 +251,10 @@ impl<'a> CargoProcess<'a> {
         cmd
     }
 
-    fn get_pkgid(&self, cwd: &Path) -> anyhow::Result<String> {
-        let mut pkgid_cmd = self.base_command(cwd, "pkgid");
+    fn get_pkgid(&self) -> anyhow::Result<String> {
+        let mut pkgid_cmd = self.base_command("pkgid");
         let out = command_output(&mut pkgid_cmd)
-            .with_context(|| format!("failed to obtain pkgid in '{:?}'", cwd))?
+            .with_context(|| format!("failed to obtain pkgid in '{:?}'", self.src_dir))?
             .stdout;
         let package_id = str::from_utf8(&out).unwrap();
         Ok(package_id.trim().to_string())
@@ -305,8 +306,8 @@ impl<'a> CargoProcess<'a> {
                 }
             };
 
-            let mut cmd = self.base_command(self.cwd, subcommand);
-            cmd.arg("-p").arg(self.get_pkgid(self.cwd)?);
+            let mut cmd = self.base_command(subcommand);
+            cmd.arg("-p").arg(self.get_pkgid()?);
             match self.build_kind {
                 BuildKind::Check => {
                     cmd.arg("--profile").arg("check");
@@ -322,6 +323,8 @@ impl<'a> CargoProcess<'a> {
                 cmd.arg("-Zunstable-options");
                 cmd.arg("-Ztimings");
             }
+            // --target-dir is not universally read, but this hopefully is.
+            cmd.env("CARGO_TARGET_DIR", self.target_directory);
             cmd.arg("--");
             // --wrap-rustc-with is not a valid rustc flag. But rustc-fake
             // recognizes it, strips it (and its argument) out, and uses it as an
@@ -353,10 +356,10 @@ impl<'a> CargoProcess<'a> {
                 // in-tree (e.g., in the case of the servo crates there are a lot of
                 // other components).
                 if let Some(file) = &self.touch_file {
-                    touch(&self.cwd, Path::new(&file))?;
+                    touch(&self.src_dir, Path::new(&file))?;
                 } else {
                     touch_all(
-                        &self.cwd.join(
+                        &self.src_dir.join(
                             Path::new(&self.manifest_path)
                                 .parent()
                                 .expect("manifest has parent"),
@@ -374,7 +377,7 @@ impl<'a> CargoProcess<'a> {
             if self.incremental {
                 cmd.arg("-C");
                 let mut incr_arg = std::ffi::OsString::from("incremental=");
-                incr_arg.push(self.cwd.join("incremental-state"));
+                incr_arg.push(self.target_directory.join("incremental-state"));
                 cmd.arg(incr_arg);
             }
 
@@ -388,7 +391,7 @@ impl<'a> CargoProcess<'a> {
             if let Some((ref mut processor, run_kind, run_kind_str, patch)) = self.processor_etc {
                 let data = ProcessOutputData {
                     name: self.processor_name.clone(),
-                    cwd: self.cwd,
+                    cwd: self.src_dir,
                     build_kind: self.build_kind,
                     run_kind,
                     run_kind_str,
@@ -952,7 +955,8 @@ impl Benchmark {
     fn mk_cargo_process<'a>(
         &'a self,
         compiler: Compiler<'a>,
-        cwd: &'a Path,
+        target_directory: &'a Path,
+        src_dir: &'a Path,
         build_kind: BuildKind,
     ) -> CargoProcess<'a> {
         let mut cargo_args = self
@@ -973,7 +977,8 @@ impl Benchmark {
         CargoProcess {
             compiler,
             processor_name: self.name.clone(),
-            cwd,
+            target_directory,
+            src_dir,
             build_kind,
             incremental: false,
             processor_etc: None,
@@ -1000,22 +1005,24 @@ impl Benchmark {
     pub fn measure(
         &self,
         processor: &mut dyn Processor,
-        build_kinds: &[BuildKind],
+        build_kinds_and_target_dir: &[(BuildKind, &Path)],
         run_kinds: &[RunKind],
         compiler: Compiler<'_>,
         iterations: usize,
     ) -> anyhow::Result<()> {
         let iterations = cmp::min(iterations, self.config.runs);
 
-        if self.config.disabled || build_kinds.is_empty() {
+        if self.config.disabled || build_kinds_and_target_dir.is_empty() {
             eprintln!("Skipping {}: disabled", self.name);
             bail!("disabled benchmark");
         }
 
         eprintln!("Preparing {}", self.name);
-        let build_kind_dirs = build_kinds
+
+        // These directories are *target* directories.
+        let build_kind_dirs = build_kinds_and_target_dir
             .iter()
-            .map(|kind| Ok((*kind, self.make_temp_dir(&self.path)?)))
+            .map(|(kind, target_dir)| Ok((*kind, target_dir, self.make_temp_dir(&self.path)?)))
             .collect::<anyhow::Result<Vec<_>>>()?;
 
         // In parallel (but with a limit to the number of CPUs), prepare all
@@ -1041,10 +1048,10 @@ impl Benchmark {
         // target-directory global lock during compilation.
         crossbeam_utils::thread::scope::<_, anyhow::Result<()>>(|s| {
             let server = jobserver::Client::new(num_cpus::get()).context("jobserver::new")?;
-            for (build_kind, prep_dir) in &build_kind_dirs {
+            for (build_kind, target_dir, src_dir) in &build_kind_dirs {
                 let server = server.clone();
                 s.spawn::<_, anyhow::Result<()>>(move |_| {
-                    self.mk_cargo_process(compiler, prep_dir.path(), *build_kind)
+                    self.mk_cargo_process(compiler, target_dir, src_dir.path(), *build_kind)
                         .jobserver(server)
                         .run_rustc(false)?;
                     Ok(())
@@ -1054,7 +1061,7 @@ impl Benchmark {
         })
         .unwrap()?;
 
-        for (build_kind, prep_dir) in build_kind_dirs {
+        for (build_kind, target_dir, src_dir) in build_kind_dirs {
             eprintln!("Running {}: {:?} + {:?}", self.name, build_kind, run_kinds);
 
             // We want at least two runs for all benchmarks (since we run
@@ -1070,12 +1077,14 @@ impl Benchmark {
                     }
                 }
                 log::debug!("Benchmark iteration {}/{}", i + 1, iterations);
-                let timing_dir = self.make_temp_dir(prep_dir.path())?;
-                let cwd = timing_dir.path();
+                let target_dir = self.make_temp_dir(target_dir)?;
+                let target_dir = target_dir.path();
+                let src_dir = self.make_temp_dir(src_dir.path())?;
+                let src_dir = src_dir.path();
 
                 // A full non-incremental build.
                 if run_kinds.contains(&RunKind::Full) {
-                    self.mk_cargo_process(compiler, cwd, build_kind)
+                    self.mk_cargo_process(compiler, target_dir, src_dir, build_kind)
                         .processor(processor, RunKind::Full, "Full", None)
                         .run_rustc(true)?;
                 }
@@ -1088,15 +1097,15 @@ impl Benchmark {
                         || run_kinds.contains(&RunKind::IncrUnchanged)
                         || run_kinds.contains(&RunKind::IncrPatched)
                     {
-                        self.mk_cargo_process(compiler, cwd, build_kind)
+                        self.mk_cargo_process(compiler, target_dir, src_dir, build_kind)
                             .incremental(true)
                             .processor(processor, RunKind::IncrFull, "IncrFull", None)
                             .run_rustc(true)?;
                     }
 
                     // An incremental build with no changes (fastest incremental case).
                     if run_kinds.contains(&RunKind::IncrUnchanged) {
-                        self.mk_cargo_process(compiler, cwd, build_kind)
+                        self.mk_cargo_process(compiler, target_dir, src_dir, build_kind)
                             .incremental(true)
                             .processor(processor, RunKind::IncrUnchanged, "IncrUnchanged", None)
                             .run_rustc(true)?;
@@ -1105,12 +1114,12 @@ impl Benchmark {
                     if run_kinds.contains(&RunKind::IncrPatched) {
                         for (i, patch) in self.patches.iter().enumerate() {
                             log::debug!("applying patch {}", patch.name);
-                            patch.apply(cwd).map_err(|s| anyhow::anyhow!("{}", s))?;
+                            patch.apply(src_dir).map_err(|s| anyhow::anyhow!("{}", s))?;
 
                             // An incremental build with some changes (realistic
                             // incremental case).
                             let run_kind_str = format!("IncrPatched{}", i);
-                            self.mk_cargo_process(compiler, cwd, build_kind)
+                            self.mk_cargo_process(compiler, target_dir, src_dir, build_kind)
                                 .incremental(true)
                                 .processor(
                                     processor,

collector/src/main.rs

@@ -13,6 +13,7 @@ use std::path::{Path, PathBuf};
 use std::process;
 use std::process::Command;
 use std::{str, time::Instant};
+use tempfile::TempDir;
 use tokio::runtime::Runtime;
 
 mod execute;
@@ -207,6 +208,16 @@ fn bench(
         );
     }
 
+    let bk_and_targets = build_kinds
+        .iter()
+        .map(|kind| Ok((*kind, TempDir::new()?)))
+        .collect::<anyhow::Result<Vec<_>>>()
+        .expect("created temp directories");
+    let bk_and_targets = bk_and_targets
+        .iter()
+        .map(|(k, td)| (*k, td.path()))
+        .collect::<Vec<_>>();
+
     let steps = benchmarks
         .iter()
         .map(|b| b.name.to_string())
@@ -250,8 +261,13 @@ fn bench(
             interned_cid,
             self_profile,
         );
-        let result =
-            benchmark.measure(&mut processor, build_kinds, run_kinds, compiler, iterations);
+        let result = benchmark.measure(
+            &mut processor,
+            &bk_and_targets,
+            run_kinds,
+            compiler,
+            iterations,
+        );
         if let Err(s) = result {
             eprintln!(
                 "collector error: Failed to benchmark '{}', recorded: {}",
@@ -734,12 +750,22 @@ fn main_result() -> anyhow::Result<i32> {
 
             eprintln!("Profiling with {:?}", profiler);
 
+            let bk_and_targets = build_kinds
+                .iter()
+                .map(|kind| Ok((*kind, TempDir::new()?)))
+                .collect::<anyhow::Result<Vec<_>>>()
+                .expect("created temp directories");
+            let bk_and_targets = bk_and_targets
+                .iter()
+                .map(|(k, td)| (*k, td.path()))
+                .collect::<Vec<_>>();
+
             let mut errors = BenchmarkErrors::new();
             for (i, benchmark) in benchmarks.iter().enumerate() {
                 eprintln!("{}", n_benchmarks_remaining(benchmarks.len() - i));
                 let mut processor = execute::ProfileProcessor::new(profiler, &out_dir, &id);
                 let result =
-                    benchmark.measure(&mut processor, &build_kinds, &run_kinds, compiler, 1);
+                    benchmark.measure(&mut processor, &bk_and_targets, &run_kinds, compiler, 1);
                 if let Err(ref s) = result {
                     errors.incr();
                     eprintln!(