Merge pull request #1140 from nnethercote/overhaul-rustc-benchmark

Overhaul how the `rustc` benchmark works.

Author: Mark-Simulacrum

.github/workflows/ci.yml

@@ -64,8 +64,8 @@ jobs:
     strategy:
       matrix:
         BENCH_INCLUDE_EXCLUDE_OPTS: [
-          "--exclude webrender-wrench,style-servo,cargo,rustc",
-          "--include webrender-wrench,style-servo,cargo --exclude rustc",
+          "--exclude webrender-wrench,style-servo,cargo",
+          "--include webrender-wrench,style-servo,cargo",
         ]
         PROFILE_KINDS: [
           "Check,Doc,Debug",

collector/README.md

@@ -107,6 +107,12 @@ The following options alter the behaviour of the `bench_local` subcommand.
   comma-separated list of benchmark names. When this option is specified, a
   benchmark is included in the run only if its name matches one or more of the
   given names.
+- `--bench-rustc`: there is a special `rustc` benchmark that involves
+  downloading a recent Rust compiler and measuring the time taken to compile
+  it. This benchmark works very differently to all the other benchmarks. For
+  example, `--runs` and `--builds` don't affect it, and the given `ID` is used
+  as the `rust-lang/rust` ref (falling back to `HEAD` if the `ID` is not a
+  valid ref). It is for advanced and CI use only. This option enables it.
 - `--runs $RUNS`: the run kinds to be benchmarked. The possible choices are one
   or more (comma-separated) of `Full`, `IncrFull`, `IncrUnchanged`,
   `IncrPatched`, and `All`. The default is `All`. Note that `IncrFull` is

collector/collect.sh

@@ -26,6 +26,6 @@ while : ; do
         rm todo-artifacts
         touch todo-artifacts
 
-        target/release/collector bench_next $SITE_URL --self-profile --db $DATABASE;
+        target/release/collector bench_next $SITE_URL --self-profile --bench-rustc --db $DATABASE;
         echo finished run at `date`;
 done

collector/src/execute.rs

@@ -571,13 +571,9 @@ pub trait Processor {
     fn finished_first_collection(&mut self, _: ProfileKind) -> bool {
         false
     }
-
-    fn measure_rustc(&mut self, _: Compiler<'_>) -> anyhow::Result<()> {
-        Ok(())
-    }
 }
 
-pub struct MeasureProcessor<'a> {
+pub struct BenchProcessor<'a> {
     rt: &'a mut Runtime,
     benchmark: &'a BenchmarkName,
     conn: &'a mut dyn database::Connection,
@@ -589,7 +585,7 @@ pub struct MeasureProcessor<'a> {
     tries: u8,
 }
 
-impl<'a> MeasureProcessor<'a> {
+impl<'a> BenchProcessor<'a> {
     pub fn new(
         rt: &'a mut Runtime,
         conn: &'a mut dyn database::Connection,
@@ -615,7 +611,7 @@ impl<'a> MeasureProcessor<'a> {
             assert!(has_tracelog);
         }
 
-        MeasureProcessor {
+        BenchProcessor {
             rt,
             upload: None,
             conn,
@@ -722,6 +718,16 @@ impl<'a> MeasureProcessor<'a> {
         self.rt
             .block_on(async move { while let Some(()) = buf.next().await {} });
     }
+
+    pub fn measure_rustc(&mut self, compiler: Compiler<'_>) -> anyhow::Result<()> {
+        rustc::measure(
+            self.rt,
+            self.conn,
+            compiler,
+            self.artifact,
+            self.artifact_row_id,
+        )
+    }
 }
 
 struct Upload(std::process::Child, tempfile::NamedTempFile);
@@ -812,7 +818,7 @@ impl Upload {
     }
 }
 
-impl<'a> Processor for MeasureProcessor<'a> {
+impl<'a> Processor for BenchProcessor<'a> {
     fn profiler(&self, _profile: ProfileKind) -> Profiler {
         if self.is_first_collection && self.is_self_profile {
             if cfg!(unix) {
@@ -897,16 +903,6 @@ impl<'a> Processor for MeasureProcessor<'a> {
             }
         }
     }
-
-    fn measure_rustc(&mut self, compiler: Compiler<'_>) -> anyhow::Result<()> {
-        rustc::measure(
-            self.rt,
-            self.conn,
-            compiler,
-            self.artifact,
-            self.artifact_row_id,
-        )
-    }
 }
 
 pub struct ProfileProcessor<'a> {
@@ -1231,15 +1227,6 @@ impl<'a> Processor for ProfileProcessor<'a> {
 
 impl Benchmark {
     pub fn new(name: String, path: PathBuf) -> anyhow::Result<Self> {
-        if name == "rustc" {
-            return Ok(Benchmark {
-                name: BenchmarkName(name),
-                path,
-                patches: vec![],
-                config: BenchmarkConfig::default(),
-            });
-        }
-
         let mut patches = vec![];
         for entry in fs::read_dir(&path)? {
             let entry = entry?;
@@ -1359,10 +1346,6 @@ impl Benchmark {
     ) -> anyhow::Result<()> {
         let iterations = iterations.unwrap_or(self.config.runs);
 
-        if self.name.0 == "rustc" {
-            return processor.measure_rustc(compiler).context("measure rustc");
-        }
-
         if self.config.disabled || profile_kinds.is_empty() {
             eprintln!("Skipping {}: disabled", self.name);
             bail!("disabled benchmark");

collector/src/execute/rustc.rs

@@ -15,13 +15,16 @@ use std::{collections::HashMap, process::Command};
 use std::{path::Path, time::Duration};
 use tokio::runtime::Runtime;
 
+/// Measure the special rustc benchmark.
 pub fn measure(
     rt: &mut Runtime,
     conn: &mut dyn database::Connection,
     compiler: Compiler<'_>,
     artifact: &database::ArtifactId,
     aid: database::ArtifactIdNumber,
 ) -> anyhow::Result<()> {
+    eprintln!("Running rustc");
+
     checkout(&artifact).context("checking out rust-lang/rust")?;
 
     record(rt, conn, compiler, artifact, aid)?;