diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a674cfbcc..64e619e2a 100644 --- 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,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", diff --git a/collector/README.md b/collector/README.md index 4cf860de8..f66451f1c 100644 --- a/collector/README.md +++ b/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 diff --git a/collector/collect.sh b/collector/collect.sh index 32c94043a..7986c7b1a 100755 --- a/collector/collect.sh +++ b/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 diff --git a/collector/src/execute.rs b/collector/src/execute.rs index 7114dd9a4..86b94d32a 100644 --- a/collector/src/execute.rs +++ b/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 { - 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"); diff --git a/collector/src/execute/rustc.rs b/collector/src/execute/rustc.rs index 62d2774a2..1f6866bdd 100644 --- a/collector/src/execute/rustc.rs +++ b/collector/src/execute/rustc.rs @@ -15,6 +15,7 @@ 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, @@ -22,6 +23,8 @@ pub fn measure( 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)?; diff --git a/collector/src/main.rs b/collector/src/main.rs index be631c6de..6d2e1c927 100644 --- a/collector/src/main.rs +++ b/collector/src/main.rs @@ -18,7 +18,7 @@ use tokio::runtime::Runtime; mod execute; mod sysroot; -use execute::{Benchmark, Profiler}; +use execute::{BenchProcessor, Benchmark, BenchmarkName, ProfileProcessor, Profiler}; use sysroot::Sysroot; #[derive(Debug, Copy, Clone)] @@ -183,9 +183,9 @@ where Ok(v) } -fn n_benchmarks_remaining(n: usize) -> String { +fn n_normal_benchmarks_remaining(n: usize) -> String { let suffix = if n == 1 { "" } else { "s" }; - format!("{} benchmark{} remaining", n, suffix) + format!("{} normal benchmark{} remaining", n, suffix) } struct BenchmarkErrors(usize); @@ -213,6 +213,7 @@ fn bench( artifact_id: &ArtifactId, profile_kinds: &[ProfileKind], scenario_kinds: &[ScenarioKind], + bench_rustc: bool, compiler: Compiler<'_>, benchmarks: &[Benchmark], iterations: Option, @@ -231,10 +232,13 @@ fn bench( } } - let steps = benchmarks + let mut steps = benchmarks .iter() .map(|b| b.name.to_string()) .collect::>(); + if bench_rustc { + steps.push("rustc".to_string()); + } // Make sure there is no observable time when the artifact ID is available // but the in-progress steps are not. @@ -249,57 +253,87 @@ fn bench( let start = Instant::now(); let mut skipped = false; - for (nth_benchmark, benchmark) in benchmarks.iter().enumerate() { - let is_fresh = - rt.block_on(conn.collector_start_step(artifact_row_id, &benchmark.name.to_string())); - if !is_fresh { - skipped = true; - eprintln!("skipping {} -- already benchmarked", benchmark.name); - continue; - } - let mut tx = rt.block_on(conn.transaction()); - rt.block_on( - tx.conn() - .record_benchmark(benchmark.name.0.as_str(), Some(benchmark.supports_stable())), - ); - eprintln!( - "{}", - n_benchmarks_remaining(benchmarks.len() - nth_benchmark) - ); - let mut processor = execute::MeasureProcessor::new( - rt, - tx.conn(), - &benchmark.name, - &artifact_id, - artifact_row_id, - is_self_profile, - ); - let result = benchmark.measure( - &mut processor, - profile_kinds, - scenario_kinds, - compiler, - iterations, - ); - if let Err(s) = result { - eprintln!( - "collector error: Failed to benchmark '{}', recorded: {:#}", - benchmark.name, s + let mut measure_and_record = + |benchmark_name: &BenchmarkName, + benchmark_supports_stable: bool, + print_intro: &dyn Fn(), + measure: &dyn Fn(&mut BenchProcessor) -> anyhow::Result<()>| { + let is_fresh = + rt.block_on(conn.collector_start_step(artifact_row_id, &benchmark_name.0)); + if !is_fresh { + skipped = true; + eprintln!("skipping {} -- already benchmarked", benchmark_name); + return; + } + let mut tx = rt.block_on(conn.transaction()); + rt.block_on( + tx.conn() + .record_benchmark(&benchmark_name.0, Some(benchmark_supports_stable)), ); - errors.incr(); - rt.block_on(tx.conn().record_error( + print_intro(); + + let mut processor = BenchProcessor::new( + rt, + tx.conn(), + benchmark_name, + &artifact_id, artifact_row_id, - benchmark.name.0.as_str(), - &format!("{:?}", s), - )); + is_self_profile, + ); + let result = measure(&mut processor); + if let Err(s) = result { + eprintln!( + "collector error: Failed to benchmark '{}', recorded: {:#}", + benchmark_name, s + ); + errors.incr(); + rt.block_on(tx.conn().record_error( + artifact_row_id, + &benchmark_name.0, + &format!("{:?}", s), + )); + }; + rt.block_on( + tx.conn() + .collector_end_step(artifact_row_id, &benchmark_name.0), + ); + rt.block_on(tx.commit()).expect("committed"); }; - rt.block_on( - tx.conn() - .collector_end_step(artifact_row_id, &benchmark.name.to_string()), + + // Normal benchmarks. + for (nth_benchmark, benchmark) in benchmarks.iter().enumerate() { + measure_and_record( + &benchmark.name, + benchmark.supports_stable(), + &|| { + eprintln!( + "{}", + n_normal_benchmarks_remaining(benchmarks.len() - nth_benchmark) + ) + }, + &|processor| { + benchmark.measure( + processor, + profile_kinds, + scenario_kinds, + compiler, + iterations, + ) + }, + ) + } + + // The special rustc benchmark, if requested. + if bench_rustc { + measure_and_record( + &BenchmarkName("rustc".to_string()), + /* supports_stable */ false, + &|| eprintln!("Special benchmark commencing (due to `--bench-rustc`)"), + &|processor| processor.measure_rustc(compiler).context("measure rustc"), ); - rt.block_on(tx.commit()).expect("committed"); } + let end = start.elapsed(); eprintln!( @@ -373,7 +407,6 @@ fn get_benchmarks( paths.push((path, name)); } - paths.push((PathBuf::from("rustc"), String::from("rustc"))); let mut includes = include.map(|list| list.split(',').collect::>()); let mut excludes = exclude.map(|list| list.split(',').collect::>()); @@ -683,8 +716,8 @@ fn profile( check_measureme_installed().unwrap(); } for (i, benchmark) in benchmarks.iter().enumerate() { - eprintln!("{}", n_benchmarks_remaining(benchmarks.len() - i)); - let mut processor = execute::ProfileProcessor::new(profiler, out_dir, id); + eprintln!("{}", n_normal_benchmarks_remaining(benchmarks.len() - i)); + let mut processor = ProfileProcessor::new(profiler, out_dir, id); let result = benchmark.measure( &mut processor, &profile_kinds, @@ -742,6 +775,8 @@ fn main_result() -> anyhow::Result { (@arg INCLUDE: --include +takes_value "Include only benchmarks that are listed in\n\ this comma-separated list of patterns") + (@arg BENCH_RUSTC: --("bench-rustc") + "Run the special `rustc` benchmark") (@arg RUNS: --runs +takes_value "One or more (comma-separated) of: 'Full',\n\ 'IncrFull', 'IncrUnchanged', 'IncrPatched', 'All'") @@ -752,13 +787,15 @@ fn main_result() -> anyhow::Result { ) (@subcommand bench_next => - (about: "Benchmarks the next commit for perf.rust-lang.org") + (about: "Benchmarks the next commit for perf.rust-lang.org, including the special `rustc` benchmark") // Mandatory arguments (@arg SITE_URL: +required +takes_value "Site URL") // Options (@arg DB: --db +takes_value "Database output file") + (@arg BENCH_RUSTC: --("bench-rustc") + "Run the special `rustc` benchmark") (@arg SELF_PROFILE: --("self-profile") "Collect self-profile data") ) @@ -868,6 +905,7 @@ fn main_result() -> anyhow::Result { let db = sub_m.value_of("DB").unwrap_or(default_db); let exclude = sub_m.value_of("EXCLUDE"); let include = sub_m.value_of("INCLUDE"); + let bench_rustc = sub_m.is_present("BENCH_RUSTC"); let scenario_kinds = scenario_kinds_from_arg(sub_m.value_of("RUNS"))?; let iterations = iterations_from_arg(sub_m.value_of("ITERATIONS"))?; let rustdoc = sub_m.value_of("RUSTDOC"); @@ -886,6 +924,7 @@ fn main_result() -> anyhow::Result { &ArtifactId::Tag(id.to_string()), &profile_kinds, &scenario_kinds, + bench_rustc, Compiler { rustc: &rustc, rustdoc: rustdoc.as_deref(), @@ -907,6 +946,7 @@ fn main_result() -> anyhow::Result { // Options let db = sub_m.value_of("DB").unwrap_or(default_db); + let bench_rustc = sub_m.is_present("BENCH_RUSTC"); let is_self_profile = sub_m.is_present("SELF_PROFILE"); println!("processing commits"); @@ -941,6 +981,7 @@ fn main_result() -> anyhow::Result { &ArtifactId::Commit(commit), &ProfileKind::all(), &ScenarioKind::all(), + bench_rustc, Compiler::from_sysroot(&sysroot), &benchmarks, next.runs.map(|v| v as usize), @@ -1011,6 +1052,7 @@ fn main_result() -> anyhow::Result { &ArtifactId::Tag(toolchain.to_string()), &proile_kinds, &scenario_kinds, + /* bench_rustc */ false, Compiler { rustc: Path::new(rustc.trim()), rustdoc: Some(Path::new(rustdoc.trim())),