Reduce run durations

ci/check-benchmarks.sh

@@ -5,7 +5,7 @@ set -x;
 bash -c "while true; do sleep 30; echo \$(date) - running ...; done" &
 PING_LOOP_PID=$!
 trap - ERR
-RUST_BACKTRACE=1 RUST_LOG=collector=debug,rust_sysroot=debug \
+RUST_BACKTRACE=1 RUST_LOG=collector_raw_cargo=trace,collector=debug,rust_sysroot=debug \
     cargo run -p collector --bin collector -- \
     bench_test $BENCH_TEST_OPTS
 code=$?

collector/benchmarks/cranelift-codegen/perf-config.json

@@ -1,3 +1,4 @@
 {
-    "cargo_toml": "cranelift-codegen/Cargo.toml"
+    "cargo_toml": "cranelift-codegen/Cargo.toml",
+    "touch_file": "cranelift-codegen/src/lib.rs"
 }

collector/benchmarks/script-servo-2/components/selectors/build.rs

@@ -24,6 +24,7 @@ fn main() {
         set.build(),
     )
     .unwrap();
+    println!("cargo:rerun-if-changed=build.rs");
 }
 
 /// <https://html.spec.whatwg.org/multipage/#selectors>

collector/benchmarks/script-servo-2/perf-config.json

@@ -1,5 +1,6 @@
 {
     "cargo_opts": "--no-default-features",
     "cargo_toml": "components/script/Cargo.toml",
-    "runs": 1
+    "runs": 1,
+    "touch_file": "components/script/lib.rs"
 }

collector/benchmarks/serde/perf-config.json

@@ -1,3 +1,4 @@
 {
-    "cargo_toml": "serde/Cargo.toml"
+    "cargo_toml": "serde/Cargo.toml",
+    "touch_file": "serde/src/lib.rs"
 }

collector/benchmarks/style-servo/components/selectors/build.rs

@@ -10,18 +10,18 @@ use std::io::{BufWriter, Write};
 use std::path::Path;
 
 fn main() {
-    let path = Path::new(&env::var("OUT_DIR").unwrap())
-        .join("ascii_case_insensitive_html_attributes.rs");
+    let path =
+        Path::new(&env::var("OUT_DIR").unwrap()).join("ascii_case_insensitive_html_attributes.rs");
     let mut file = BufWriter::new(File::create(&path).unwrap());
 
-    write!(&mut file, "{{ static SET: ::phf::Set<&'static str> = ",
-    ).unwrap();
+    write!(&mut file, "{{ static SET: ::phf::Set<&'static str> = ",).unwrap();
     let mut set = phf_codegen::Set::new();
     for name in ASCII_CASE_INSENSITIVE_HTML_ATTRIBUTES.split_whitespace() {
         set.entry(name);
     }
     set.build(&mut file).unwrap();
     write!(&mut file, "; &SET }}").unwrap();
+    println!("cargo:rerun-if-changed=build.rs");
 }
 
 /// https://html.spec.whatwg.org/multipage/#selectors

collector/benchmarks/style-servo/perf-config.json

@@ -3,5 +3,6 @@
     "cargo_rustc_opts": "--cap-lints=warn",
     "cargo_toml": "components/style/Cargo.toml",
     "runs": 1,
-    "supports_stable": true
+    "supports_stable": true,
+    "touch_file": "components/style/lib.rs"
 }

collector/benchmarks/webrender-wrench/perf-config.json

@@ -1,4 +1,5 @@
 {
     "cargo_toml": "wrench/Cargo.toml",
-    "runs": 1
+    "runs": 1,
+    "touch_file": "wrench/src/main.rs"
 }

collector/benchmarks/webrender/perf-config.json

@@ -1,4 +1,5 @@
 {
     "cargo_toml": "webrender/Cargo.toml",
-    "runs": 1
+    "runs": 1,
+    "touch_file": "webrender/src/lib.rs"
 }

collector/src/execute.rs

@@ -19,6 +19,13 @@ use std::time::Duration;
 use tempfile::TempDir;
 use tokio::runtime::Runtime;
 
+fn touch(root: &Path, path: &Path) -> anyhow::Result<()> {
+    let mut cmd = Command::new("touch");
+    cmd.current_dir(root).arg(path);
+    command_output(&mut cmd).with_context(|| format!("touching {:?} in {:?}", path, root))?;
+    Ok(())
+}
+
 fn touch_all(path: &Path) -> anyhow::Result<()> {
     let mut cmd = Command::new("bash");
     cmd.current_dir(path)
@@ -50,6 +57,13 @@ struct BenchmarkConfig {
     runs: usize,
     #[serde(default)]
     supports_stable: bool,
+
+    /// The file that should be touched to ensure cargo re-checks the leaf crate
+    /// we're interested in. Likely, something similar to `src/lib.rs`. The
+    /// default if this is not present is to touch all .rs files in the
+    /// directory that `Cargo.toml` is in.
+    #[serde(default)]
+    touch_file: Option<String>,
 }
 
 impl Default for BenchmarkConfig {
@@ -61,6 +75,7 @@ impl Default for BenchmarkConfig {
             disabled: false,
             runs: default_runs(),
             supports_stable: false,
+            touch_file: None,
         }
     }
 }
@@ -191,6 +206,7 @@ struct CargoProcess<'a> {
     manifest_path: String,
     cargo_args: Vec<String>,
     rustc_args: Vec<String>,
+    touch_file: Option<String>,
 }
 
 impl<'a> CargoProcess<'a> {
@@ -301,13 +317,17 @@ impl<'a> CargoProcess<'a> {
             // benchmarking, so as to not refresh dependencies, which may be
             // in-tree (e.g., in the case of the servo crates there are a lot of
             // other components).
-            touch_all(
-                &self.cwd.join(
-                    Path::new(&self.manifest_path)
-                        .parent()
-                        .expect("manifest has parent"),
-                ),
-            )?;
+            if let Some(file) = &self.touch_file {
+                touch(&self.cwd, Path::new(&file))?;
+            } else {
+                touch_all(
+                    &self.cwd.join(
+                        Path::new(&self.manifest_path)
+                            .parent()
+                            .expect("manifest has parent"),
+                    ),
+                )?;
+            }
 
             let output = command_output(&mut cmd)?;
             if let Some((ref mut processor, run_kind, run_kind_str, patch)) = self.processor_etc {
@@ -396,10 +416,6 @@ pub trait Processor {
     fn finished_first_collection(&mut self) -> bool {
         false
     }
-
-    /// Called when all the runs of a benchmark for a particular `BuildKind`
-    /// and iteration have been completed. Can be used to process/reset accumulated state.
-    fn finish_build_kind(&mut self, _build_kind: BuildKind) {}
 }
 
 pub struct MeasureProcessor<'a> {
@@ -567,10 +583,6 @@ impl<'a> Processor for MeasureProcessor<'a> {
             }
         }
     }
-
-    fn finish_build_kind(&mut self, _: BuildKind) {
-        // do nothing
-    }
 }
 
 pub struct ProfileProcessor<'a> {
@@ -916,6 +928,7 @@ impl Benchmark {
                 .split_whitespace()
                 .map(String::from)
                 .collect(),
+            touch_file: self.config.touch_file.clone(),
         }
     }
 
@@ -976,42 +989,48 @@ impl Benchmark {
                         .run_rustc()?;
                 }
 
-                // An incremental build from scratch (slowest incremental case).
-                // This is required for any subsequent incremental builds.
-                if run_kinds.contains(&RunKind::IncrFull)
-                    || run_kinds.contains(&RunKind::IncrUnchanged)
-                    || run_kinds.contains(&RunKind::IncrPatched)
-                {
-                    self.mk_cargo_process(compiler, cwd, build_kind)
-                        .incremental(true)
-                        .processor(processor, RunKind::IncrFull, "IncrFull", None)
-                        .run_rustc()?;
-                }
-
-                // An incremental build with no changes (fastest incremental case).
-                if run_kinds.contains(&RunKind::IncrUnchanged) {
-                    self.mk_cargo_process(compiler, cwd, build_kind)
-                        .incremental(true)
-                        .processor(processor, RunKind::IncrUnchanged, "IncrUnchanged", None)
-                        .run_rustc()?;
-                }
-
-                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))?;
+                // Rustdoc does not support incremental compilation
+                if build_kind != BuildKind::Doc {
+                    // An incremental build from scratch (slowest incremental case).
+                    // This is required for any subsequent incremental builds.
+                    if run_kinds.contains(&RunKind::IncrFull)
+                        || run_kinds.contains(&RunKind::IncrUnchanged)
+                        || run_kinds.contains(&RunKind::IncrPatched)
+                    {
+                        self.mk_cargo_process(compiler, cwd, build_kind)
+                            .incremental(true)
+                            .processor(processor, RunKind::IncrFull, "IncrFull", None)
+                            .run_rustc()?;
+                    }
 
-                        // An incremental build with some changes (realistic
-                        // incremental case).
-                        let run_kind_str = format!("IncrPatched{}", i);
+                    // An incremental build with no changes (fastest incremental case).
+                    if run_kinds.contains(&RunKind::IncrUnchanged) {
                         self.mk_cargo_process(compiler, cwd, build_kind)
                             .incremental(true)
-                            .processor(processor, RunKind::IncrPatched, &run_kind_str, Some(&patch))
+                            .processor(processor, RunKind::IncrUnchanged, "IncrUnchanged", None)
                             .run_rustc()?;
                     }
-                }
 
-                processor.finish_build_kind(build_kind);
+                    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))?;
+
+                            // An incremental build with some changes (realistic
+                            // incremental case).
+                            let run_kind_str = format!("IncrPatched{}", i);
+                            self.mk_cargo_process(compiler, cwd, build_kind)
+                                .incremental(true)
+                                .processor(
+                                    processor,
+                                    RunKind::IncrPatched,
+                                    &run_kind_str,
+                                    Some(&patch),
+                                )
+                                .run_rustc()?;
+                        }
+                    }
+                }
             }
         }
 

collector/src/lib.rs

@@ -6,8 +6,10 @@ use std::fmt;
 use std::process::{self, Command};
 
 pub mod api;
+mod read2;
 pub mod self_profile;
 
+use process::Stdio;
 pub use self_profile::{QueryData, SelfProfile};
 
 #[derive(Debug, Copy, Clone, PartialEq, PartialOrd, Deserialize)]
@@ -135,20 +137,47 @@ pub fn run_command(cmd: &mut Command) -> anyhow::Result<()> {
 
 pub fn command_output(cmd: &mut Command) -> anyhow::Result<process::Output> {
     log::trace!("running: {:?}", cmd);
-    let output = cmd.output()?;
-    if !output.status.success() {
+    let mut child = cmd.stdout(Stdio::piped()).stderr(Stdio::piped()).spawn()?;
+
+    let mut stdout = Vec::new();
+    let mut stderr = Vec::new();
+    let mut stdout_writer = std::io::LineWriter::new(std::io::stdout());
+    let mut stderr_writer = std::io::LineWriter::new(std::io::stderr());
+    read2::read2(
+        child.stdout.take().unwrap(),
+        child.stderr.take().unwrap(),
+        &mut |is_stdout, buffer, _is_done| {
+            // Send output if trace logging is enabled
+            if log::log_enabled!(target: "collector_raw_cargo", log::Level::Trace) {
+                use std::io::Write;
+                if is_stdout {
+                    stdout_writer.write_all(&buffer[stdout.len()..]).unwrap();
+                } else {
+                    stderr_writer.write_all(&buffer[stderr.len()..]).unwrap();
+                }
+            }
+            if is_stdout {
+                stdout = buffer.clone();
+            } else {
+                stderr = buffer.clone();
+            }
+        },
+    )?;
+
+    let status = child.wait()?;
+    if !status.success() {
         return Err(anyhow::anyhow!(
             "expected success, got {}\n\nstderr={}\n\n stdout={}",
-            output.status,
-            String::from_utf8_lossy(&output.stderr),
-            String::from_utf8_lossy(&output.stdout)
+            status,
+            String::from_utf8_lossy(&stderr),
+            String::from_utf8_lossy(&stdout)
         ));
-    } else {
-        //        log::trace!(
-        //            "stderr={}\n\nstdout={}",
-        //            String::from_utf8_lossy(&output.stderr),
-        //            String::from_utf8_lossy(&output.stdout),
-        //        );
     }
+
+    let output = process::Output {
+        status,
+        stdout: stdout,
+        stderr: stderr,
+    };
     Ok(output)
 }