Merge pull request #1206 from nnethercote/demote-style-servo

Mark-Simulacrum · web-flow · commit a7f7b0eb0d52 · 2022-03-16T11:06:13.000-04:00
Demote `style-servo` to only run in the "stable" set.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -64,8 +64,8 @@ jobs:
     strategy:
       matrix:
         BENCH_INCLUDE_EXCLUDE_OPTS: [
-          "--exclude webrender-wrench,style-servo,cargo",
-          "--include webrender-wrench,style-servo,cargo",
+          "--exclude webrender-wrench,cargo",
+          "--include webrender-wrench,cargo",
         ]
         PROFILES: [
           "Check,Doc,Debug",
diff --git a/collector/benchmarks/README.md b/collector/benchmarks/README.md
@@ -7,7 +7,8 @@ The suite changes over time. Sometimes the code for a benchmark is updated, in
 which case a small suffix will be added (starting with "-2", then "-3", and so
 on.)
 
-There are two categories of benchmarks, **Primary** and **Secondary**.
+There are three categories of benchmarks, **Primary**, **Secondary**, and
+**Stable**.
 
 ## Primary
 
@@ -38,8 +39,6 @@ They mostly consist of real-world crates.
 - **serde**: A serialization/deserialization crate. Used by many other
   Rust programs.
 - **stm32f4**: A crate that has many thousands of blanket impl blocks.
-- **style-servo**: Servo's `style` crate. A large crate, and one used by
-  Firefox.
 - **syn**: A library for parsing Rust code. An important part of the Rust
   ecosystem.
 - **tokio-webpush-simple**: A simple web server built with tokio. Uses futures
@@ -125,3 +124,22 @@ compiler in interesting ways.
 - **wg-grammar**: A parser generator.
   [Stresses](https://github.com/rust-lang/rust/issues/58178) the borrow
   checker's implementation of NLL.
+
+**Stable**
+
+These are benchmarks used in the
+[dashboard](https://perf.rust-lang.org/dashboard.html). They provide the
+longest continuous data set for compiler performance. As a result, they are
+quite old (e.g. 2017 or earlier), and not necessarily reflective of typical
+Rust code being written today.
+
+- **encoding**: See above.
+- **futures**: See above.
+- **html5ever**: See above.
+- **inflate**: See above.
+- **regex**: See above.
+- **piston-image**: See above.
+- **style-servo**: An old version of Servo's `style` crate. A large crate, and
+  one used by old versions of Firefox.
+- **syn**: See above.
+- **tokio-webpush-simple**: See above.
diff --git a/collector/benchmarks/encoding/perf-config.json b/collector/benchmarks/encoding/perf-config.json
@@ -1,5 +1,4 @@
 {
-    "supports_stable": true,
     "touch_file": "src/lib.rs",
-    "category": "primary"
+    "category": "primary-and-stable"
 }
diff --git a/collector/benchmarks/futures/perf-config.json b/collector/benchmarks/futures/perf-config.json
@@ -1,4 +1,3 @@
 {
-    "supports_stable": true,
-    "category": "primary"
+    "category": "primary-and-stable"
 }
diff --git a/collector/benchmarks/html5ever/perf-config.json b/collector/benchmarks/html5ever/perf-config.json
@@ -1,5 +1,4 @@
 {
-    "supports_stable": true,
     "touch_file": "src/lib.rs",
-    "category": "primary"
+    "category": "primary-and-stable"
 }
diff --git a/collector/benchmarks/inflate/perf-config.json b/collector/benchmarks/inflate/perf-config.json
@@ -1,4 +1,3 @@
 {
-    "supports_stable": true,
-    "category": "primary"
+    "category": "primary-and-stable"
 }
diff --git a/collector/benchmarks/match-stress-exhaustive_patterns/perf-config.json b/collector/benchmarks/match-stress-exhaustive_patterns/perf-config.json
@@ -1,5 +1,4 @@
 {
-    "supports_stable": false,
     "touch_file": "src/lib.rs",
     "category": "secondary"
 }
diff --git a/collector/benchmarks/piston-image/perf-config.json b/collector/benchmarks/piston-image/perf-config.json
@@ -1,5 +1,4 @@
 {
-    "supports_stable": true,
     "runs": 1,
-    "category": "primary"
+    "category": "primary-and-stable"
 }
diff --git a/collector/benchmarks/regex/perf-config.json b/collector/benchmarks/regex/perf-config.json
@@ -1,5 +1,4 @@
 {
-    "supports_stable": true,
     "touch_file": "src/lib.rs",
-    "category": "primary"
+    "category": "primary-and-stable"
 }
diff --git a/collector/benchmarks/style-servo/perf-config.json b/collector/benchmarks/style-servo/perf-config.json
@@ -3,7 +3,6 @@
     "cargo_rustc_opts": "--cap-lints=warn",
     "cargo_toml": "components/style/Cargo.toml",
     "runs": 1,
-    "supports_stable": true,
     "touch_file": "components/style/lib.rs",
-    "category": "primary"
+    "category": "stable"
 }
diff --git a/collector/benchmarks/syn/perf-config.json b/collector/benchmarks/syn/perf-config.json
@@ -1,5 +1,4 @@
 {
-    "supports_stable": true,
     "touch_file": "src/lib.rs",
-    "category": "primary"
+    "category": "primary-and-stable"
 }
diff --git a/collector/benchmarks/tokio-webpush-simple/perf-config.json b/collector/benchmarks/tokio-webpush-simple/perf-config.json
@@ -1,5 +1,4 @@
 {
-    "supports_stable": true,
     "touch_file": "src/main.rs",
-    "category": "primary"
+    "category": "primary-and-stable"
 }
diff --git a/collector/src/execute.rs b/collector/src/execute.rs
@@ -4,9 +4,10 @@ use crate::{Compiler, Profile, Scenario};
 use anyhow::{bail, Context};
 use collector::command_output;
 use collector::etw_parser;
-use database::{Category, PatchName, QueryLabel};
+use database::{PatchName, QueryLabel};
 use futures::stream::FuturesUnordered;
 use futures::stream::StreamExt;
+use serde::{Deserialize, Serialize};
 use std::collections::HashMap;
 use std::env;
 use std::fmt;
@@ -106,6 +107,57 @@ fn touch_all(path: &Path) -> anyhow::Result<()> {
     Ok(())
 }
 
+#[derive(Debug, Clone, Copy, Serialize, Deserialize, clap::ArgEnum)]
+#[serde(rename_all = "kebab-case")]
+pub enum Category {
+    Primary,
+    Secondary,
+    // FIXME: this should disappear after the 2022 benchmark overhaul is
+    // complete.
+    PrimaryAndStable,
+    Stable,
+}
+
+impl Category {
+    pub fn has_stable(&self) -> bool {
+        match self {
+            Category::Primary | Category::Secondary => false,
+            Category::PrimaryAndStable | Category::Stable => true,
+        }
+    }
+
+    pub fn is_primary_or_secondary(&self) -> bool {
+        match self {
+            Category::Primary | Category::Secondary | Category::PrimaryAndStable => true,
+            Category::Stable => false,
+        }
+    }
+
+    // Within the DB, `Category` is represented in two fields:
+    // - a `supports_stable` bool,
+    // - a `category` which is either "primary" or "secondary".
+    pub fn db_representation(&self) -> (bool, String) {
+        match self {
+            Category::Primary => (false, "primary".to_string()),
+            Category::Secondary => (false, "secondary".to_string()),
+            // These two have the same DB representation, even though they are
+            // treated differently when choosing which benchmarks to run.
+            Category::PrimaryAndStable | Category::Stable => (true, "primary".to_string()),
+        }
+    }
+}
+
+impl fmt::Display for Category {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            Category::Primary => f.write_str("primary"),
+            Category::Secondary => f.write_str("secondary"),
+            Category::PrimaryAndStable => f.write_str("primary-and-stable"),
+            Category::Stable => f.write_str("stable"),
+        }
+    }
+}
+
 fn default_runs() -> usize {
     3
 }
@@ -121,8 +173,6 @@ struct BenchmarkConfig {
     disabled: bool,
     #[serde(default = "default_runs")]
     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
@@ -134,21 +184,6 @@ struct BenchmarkConfig {
     category: Category,
 }
 
-impl Default for BenchmarkConfig {
-    fn default() -> BenchmarkConfig {
-        BenchmarkConfig {
-            cargo_opts: None,
-            cargo_rustc_opts: None,
-            cargo_toml: None,
-            disabled: false,
-            runs: default_runs(),
-            supports_stable: false,
-            touch_file: None,
-            category: Category::Secondary,
-        }
-    }
-}
-
 #[derive(Ord, PartialOrd, Eq, PartialEq, Clone, Hash)]
 pub struct BenchmarkName(pub String);
 
@@ -1240,7 +1275,7 @@ impl Benchmark {
             )
             .with_context(|| format!("failed to parse {:?}", config_path))?
         } else {
-            BenchmarkConfig::default()
+            bail!("missing a perf-config.json file for `{}`", name);
         };
 
         Ok(Benchmark {
@@ -1251,12 +1286,8 @@ impl Benchmark {
         })
     }
 
-    pub fn supports_stable(&self) -> bool {
-        self.config.supports_stable
-    }
-
-    pub fn category(&self) -> &Category {
-        &self.config.category
+    pub fn category(&self) -> Category {
+        self.config.category
     }
 
     #[cfg(windows)]
diff --git a/collector/src/main.rs b/collector/src/main.rs
@@ -2,7 +2,7 @@
 
 use anyhow::{bail, Context};
 use clap::Parser;
-use database::{ArtifactId, Category, Commit};
+use database::{ArtifactId, Commit};
 use log::debug;
 use std::collections::HashSet;
 use std::fs;
@@ -18,7 +18,7 @@ use tokio::runtime::Runtime;
 mod execute;
 mod sysroot;
 
-use execute::{BenchProcessor, Benchmark, BenchmarkName, ProfileProcessor, Profiler};
+use execute::{BenchProcessor, Benchmark, BenchmarkName, Category, ProfileProcessor, Profiler};
 use sysroot::Sysroot;
 
 #[derive(Debug, Copy, Clone)]
@@ -190,8 +190,7 @@ fn bench(
 
     let mut measure_and_record =
         |benchmark_name: &BenchmarkName,
-         benchmark_supports_stable: bool,
-         benchmark_category: Category,
+         category: Category,
          print_intro: &dyn Fn(),
          measure: &dyn Fn(&mut BenchProcessor) -> anyhow::Result<()>| {
             let is_fresh =
@@ -202,10 +201,11 @@ fn bench(
                 return;
             }
             let mut tx = rt.block_on(conn.transaction());
+            let (supports_stable, category) = category.db_representation();
             rt.block_on(tx.conn().record_benchmark(
                 &benchmark_name.0,
-                Some(benchmark_supports_stable),
-                benchmark_category,
+                Some(supports_stable),
+                category,
             ));
             print_intro();
 
@@ -241,8 +241,7 @@ fn bench(
     for (nth_benchmark, benchmark) in benchmarks.iter().enumerate() {
         measure_and_record(
             &benchmark.name,
-            benchmark.supports_stable(),
-            benchmark.category().clone(),
+            benchmark.category(),
             &|| {
                 eprintln!(
                     "{}",
@@ -257,7 +256,6 @@ fn bench(
     if bench_rustc {
         measure_and_record(
             &BenchmarkName("rustc".to_string()),
-            /* supports_stable */ false,
             Category::Primary,
             &|| eprintln!("Special benchmark commencing (due to `--bench-rustc`)"),
             &|processor| processor.measure_rustc(compiler).context("measure rustc"),
@@ -860,12 +858,12 @@ struct DownloadCommand {
     #[clap(long, global = true)]
     name: Option<String>,
 
-    /// Overwrite the benchmark directory if it already exists.
+    /// Overwrite the benchmark directory if it already exists
     #[clap(long, short('f'), global = true)]
     force: bool,
 
-    /// What category does the benchmark belong to (primary or secondary).
-    #[clap(long, short('c'), global = true, default_value = "secondary")]
+    /// What category does the benchmark belong to
+    #[clap(long, short('c'), arg_enum, global = true, default_value = "secondary")]
     category: Category,
 
     #[clap(subcommand)]
@@ -924,11 +922,12 @@ fn main_result() -> anyhow::Result<i32> {
                 "",
             )?;
 
-            let benchmarks = get_benchmarks(
+            let mut benchmarks = get_benchmarks(
                 &benchmark_dir,
                 local.include.as_deref(),
                 local.exclude.as_deref(),
             )?;
+            benchmarks.retain(|b| b.category().is_primary_or_secondary());
 
             let res = bench(
                 &mut rt,
@@ -978,11 +977,12 @@ fn main_result() -> anyhow::Result<i32> {
             let sysroot = Sysroot::install(commit.sha.to_string(), &target_triple)
                 .with_context(|| format!("failed to install sysroot for {:?}", commit))?;
 
-            let benchmarks = get_benchmarks(
+            let mut benchmarks = get_benchmarks(
                 &benchmark_dir,
                 next.include.as_deref(),
                 next.exclude.as_deref(),
             )?;
+            benchmarks.retain(|b| b.category().is_primary_or_secondary());
 
             let res = bench(
                 &mut rt,
@@ -1044,7 +1044,7 @@ fn main_result() -> anyhow::Result<i32> {
 
             // Exclude benchmarks that don't work with a stable compiler.
             let mut benchmarks = get_benchmarks(&benchmark_dir, None, None)?;
-            benchmarks.retain(|b| b.supports_stable());
+            benchmarks.retain(|b| b.category().has_stable());
 
             let res = bench(
                 &mut rt,
@@ -1077,11 +1077,13 @@ fn main_result() -> anyhow::Result<i32> {
             let profiles = Profile::expand_all(&local.profiles);
             let scenarios = Scenario::expand_all(&local.scenarios);
 
-            let benchmarks = get_benchmarks(
+            let mut benchmarks = get_benchmarks(
                 &benchmark_dir,
                 local.include.as_deref(),
                 local.exclude.as_deref(),
             )?;
+            benchmarks.retain(|b| b.category().is_primary_or_secondary());
+
             let mut errors = BenchmarkErrors::new();
 
             let mut get_toolchain_and_profile =
diff --git a/database/src/lib.rs b/database/src/lib.rs
diff --git a/database/src/pool.rs b/database/src/pool.rs
diff --git a/database/src/pool/postgres.rs b/database/src/pool/postgres.rs
diff --git a/database/src/pool/sqlite.rs b/database/src/pool/sqlite.rs
diff --git a/docs/glossary.md b/docs/glossary.md
diff --git a/site/src/request_handlers/dashboard.rs b/site/src/request_handlers/dashboard.rs

Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,4 @@`
`1`	`1`	`{`
`2`		`- "supports_stable": true,`
`3`	`2`	`"touch_file": "src/lib.rs",`
`4`		`- "category": "primary"`
	`3`	`+ "category": "primary-and-stable"`
`5`	`4`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,3 @@`
`1`	`1`	`{`
`2`		`- "supports_stable": true,`
`3`		`- "category": "primary"`
	`2`	`+ "category": "primary-and-stable"`
`4`	`3`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,4 @@`
`1`	`1`	`{`
`2`		`- "supports_stable": false,`
`3`	`2`	`"touch_file": "src/lib.rs",`
`4`	`3`	`"category": "secondary"`
`5`	`4`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,4 @@`
`1`	`1`	`{`
`2`		`- "supports_stable": true,`
`3`	`2`	`"runs": 1,`
`4`		`- "category": "primary"`
	`3`	`+ "category": "primary-and-stable"`
`5`	`4`	`}`
Original file line number	Diff line number	Diff line change
`@@ -3,7 +3,6 @@`
`3`	`3`	`"cargo_rustc_opts": "--cap-lints=warn",`
`4`	`4`	`"cargo_toml": "components/style/Cargo.toml",`
`5`	`5`	`"runs": 1,`
`6`		`- "supports_stable": true,`
`7`	`6`	`"touch_file": "components/style/lib.rs",`
`8`		`- "category": "primary"`
	`7`	`+ "category": "stable"`
`9`	`8`	`}`