Skip to content

Overhaul how the rustc benchmark works. #1140

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
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,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",
Expand Down
6 changes: 6 additions & 0 deletions collector/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion collector/collect.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
45 changes: 14 additions & 31 deletions collector/src/execute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -615,7 +611,7 @@ impl<'a> MeasureProcessor<'a> {
assert!(has_tracelog);
}

MeasureProcessor {
BenchProcessor {
rt,
upload: None,
conn,
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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> {
Expand Down Expand Up @@ -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?;
Expand Down Expand Up @@ -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");
Expand Down
3 changes: 3 additions & 0 deletions collector/src/execute/rustc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?;
Expand Down
Loading