Skip to content

Demote style-servo to only run in the "stable" set. #1206

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Mar 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
24 changes: 21 additions & 3 deletions collector/benchmarks/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -120,3 +119,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.
3 changes: 1 addition & 2 deletions collector/benchmarks/encoding/perf-config.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
{
"supports_stable": true,
"touch_file": "src/lib.rs",
"category": "primary"
"category": "primary-and-stable"
}
3 changes: 1 addition & 2 deletions collector/benchmarks/futures/perf-config.json
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
{
"supports_stable": true,
"category": "primary"
"category": "primary-and-stable"
}
3 changes: 1 addition & 2 deletions collector/benchmarks/html5ever/perf-config.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
{
"supports_stable": true,
"touch_file": "src/lib.rs",
"category": "primary"
"category": "primary-and-stable"
}
3 changes: 1 addition & 2 deletions collector/benchmarks/inflate/perf-config.json
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
{
"supports_stable": true,
"category": "primary"
"category": "primary-and-stable"
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
{
"supports_stable": false,
"touch_file": "src/lib.rs",
"category": "secondary"
}
3 changes: 1 addition & 2 deletions collector/benchmarks/piston-image/perf-config.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
{
"supports_stable": true,
"runs": 1,
"category": "primary"
"category": "primary-and-stable"
}
3 changes: 1 addition & 2 deletions collector/benchmarks/regex/perf-config.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
{
"supports_stable": true,
"touch_file": "src/lib.rs",
"category": "primary"
"category": "primary-and-stable"
}
3 changes: 1 addition & 2 deletions collector/benchmarks/style-servo/perf-config.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
3 changes: 1 addition & 2 deletions collector/benchmarks/syn/perf-config.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
{
"supports_stable": true,
"touch_file": "src/lib.rs",
"category": "primary"
"category": "primary-and-stable"
}
3 changes: 1 addition & 2 deletions collector/benchmarks/tokio-webpush-simple/perf-config.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
{
"supports_stable": true,
"touch_file": "src/main.rs",
"category": "primary"
"category": "primary-and-stable"
}
81 changes: 56 additions & 25 deletions collector/src/execute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
}
Expand All @@ -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
Expand All @@ -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);

Expand Down Expand Up @@ -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 {
Expand All @@ -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)]
Expand Down
34 changes: 18 additions & 16 deletions collector/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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)]
Expand Down Expand Up @@ -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 =
Expand All @@ -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();

Expand Down Expand Up @@ -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!(
"{}",
Expand All @@ -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"),
Expand Down Expand Up @@ -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)]
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 =
Expand Down
Loading