From 9c534795ca364f5bf00d6647f34da263eec6a1b3 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Fri, 26 Jun 2020 21:49:22 -0400 Subject: [PATCH 1/9] Initial implementation of rustdoc benchmarking --- collector/src/bin/rustc-perf-collector/execute.rs | 15 ++++++++++++--- collector/src/bin/rustc-perf-collector/main.rs | 1 + database/src/bin/ingest-json.rs | 1 + database/src/lib.rs | 3 +++ site/src/server.rs | 3 +++ 5 files changed, 20 insertions(+), 3 deletions(-) diff --git a/collector/src/bin/rustc-perf-collector/execute.rs b/collector/src/bin/rustc-perf-collector/execute.rs index 3e9dc48ae..10d1e0683 100644 --- a/collector/src/bin/rustc-perf-collector/execute.rs +++ b/collector/src/bin/rustc-perf-collector/execute.rs @@ -176,7 +176,10 @@ impl Profiler { | Profiler::DHAT | Profiler::Massif | Profiler::Eprintln => true, - Profiler::LlvmLines => build_kind != BuildKind::Check, + Profiler::LlvmLines => match build_kind { + BuildKind::Debug | BuildKind::Opt => true, + BuildKind::Check | BuildKind::Doc => false, + } } } @@ -242,7 +245,7 @@ impl<'a> CargoProcess<'a> { .arg(subcommand) .arg("--manifest-path") .arg(&self.manifest_path); - cmd + dbg!(cmd) } fn get_pkgid(&self, cwd: &Path) -> String { @@ -276,7 +279,11 @@ impl<'a> CargoProcess<'a> { )); } - profiler.subcommand() + if self.build_kind == BuildKind::Doc { + dbg!("doc") + } else { + profiler.subcommand() + } } else { "rustc" }; @@ -288,6 +295,7 @@ impl<'a> CargoProcess<'a> { cmd.arg("--profile").arg("check"); } BuildKind::Debug => {} + BuildKind::Doc => {} BuildKind::Opt => { cmd.arg("--release"); } @@ -438,6 +446,7 @@ impl<'a> MeasureProcessor<'a> { let profile = match build_kind { BuildKind::Check => database::Profile::Check, BuildKind::Debug => database::Profile::Debug, + BuildKind::Doc => database::Profile::Doc, BuildKind::Opt => database::Profile::Opt, }; let mut buf = FuturesUnordered::new(); diff --git a/collector/src/bin/rustc-perf-collector/main.rs b/collector/src/bin/rustc-perf-collector/main.rs index 9c2ebffd4..a0609b0ce 100644 --- a/collector/src/bin/rustc-perf-collector/main.rs +++ b/collector/src/bin/rustc-perf-collector/main.rs @@ -49,6 +49,7 @@ impl<'a> Compiler<'a> { pub enum BuildKind { Check, Debug, + Doc, Opt, } diff --git a/database/src/bin/ingest-json.rs b/database/src/bin/ingest-json.rs index 827845f20..7425fe9c5 100644 --- a/database/src/bin/ingest-json.rs +++ b/database/src/bin/ingest-json.rs @@ -934,6 +934,7 @@ async fn ingest(conn: &T, caches: &mut IdCache, path: &Path) { let profile_str = match profile { Profile::Check => "check", Profile::Debug => "debug", + Profile::Doc => "doc", Profile::Opt => "opt", }; let state = match &run.state { diff --git a/database/src/lib.rs b/database/src/lib.rs index 744455625..272a1fa87 100644 --- a/database/src/lib.rs +++ b/database/src/lib.rs @@ -284,6 +284,7 @@ impl Ord for Commit { pub enum Profile { Check, Debug, + Doc, Opt, } @@ -293,6 +294,7 @@ impl std::str::FromStr for Profile { Ok(match s.to_ascii_lowercase().as_str() { "check" => Profile::Check, "debug" => Profile::Debug, + "doc" => Profile::Doc, "opt" => Profile::Opt, _ => return Err(format!("{} is not a profile", s)), }) @@ -308,6 +310,7 @@ impl fmt::Display for Profile { Profile::Check => "check", Profile::Opt => "opt", Profile::Debug => "debug", + Profile::Doc => "doc", } ) } diff --git a/site/src/server.rs b/site/src/server.rs index 9de3494d0..721f3e052 100644 --- a/site/src/server.rs +++ b/site/src/server.rs @@ -64,6 +64,7 @@ pub fn handle_info(data: &InputData) -> info::Response { pub struct ByProfile { pub check: T, pub debug: T, + pub doc: T, pub opt: T, } @@ -76,6 +77,7 @@ impl ByProfile { Ok(ByProfile { check: f(Profile::Check).await?, debug: f(Profile::Debug).await?, + doc: f(Profile::Doc).await?, opt: f(Profile::Opt).await?, }) } @@ -87,6 +89,7 @@ impl std::ops::Index for ByProfile { match index { Profile::Check => &self.check, Profile::Debug => &self.debug, + Profile::Doc => &self.doc, Profile::Opt => &self.opt, } } From eaa5441d7122b2ed5a79610be815dc1ca152c6b9 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Fri, 26 Jun 2020 23:24:55 -0400 Subject: [PATCH 2/9] Allow profiling rustdoc - Distinguish between rustdoc and rustc - Use `rustdoc` instead of `doc` subcommand This only profiles the last crate and also avoids special casing RUSTDOCFLAGS. - Add a fake rustdoc - Create symlink automatically if it doesn't already exist --- collector/src/bin/rustc-fake.rs | 34 ++++++++----- .../src/bin/rustc-perf-collector/execute.rs | 17 ++++++- .../src/bin/rustc-perf-collector/main.rs | 49 ++++++++++--------- 3 files changed, 64 insertions(+), 36 deletions(-) diff --git a/collector/src/bin/rustc-fake.rs b/collector/src/bin/rustc-fake.rs index e72b2f9c0..8a2298ff8 100644 --- a/collector/src/bin/rustc-fake.rs +++ b/collector/src/bin/rustc-fake.rs @@ -4,8 +4,18 @@ use std::process::Command; use std::time::{Duration, Instant}; fn main() { - let mut args = env::args_os().skip(1).collect::>(); + let mut args_os = env::args_os(); + let name = args_os.next().unwrap().into_string().unwrap(); + + let mut args = args_os.collect::>(); let rustc = env::var_os("RUSTC_REAL").unwrap(); + let rustdoc = env::var_os("RUSTDOC_REAL").unwrap(); + let actually_rustdoc = name.ends_with("rustdoc-fake"); + let tool = if actually_rustdoc { + rustdoc + } else { + rustc + }; if let Some(count) = env::var("RUSTC_THREAD_COUNT") .ok() @@ -36,7 +46,7 @@ fn main() { .arg("instructions:u,cycles:u,task-clock,cpu-clock,faults") .arg("--log-fd") .arg("1") - .arg(&rustc) + .arg(&tool) .args(&args); let prof_out_dir = std::env::current_dir().unwrap().join("self-profile-output"); @@ -91,14 +101,14 @@ fn main() { } "self-profile" => { - let mut cmd = Command::new(&rustc); + let mut cmd = Command::new(&tool); cmd.arg("-Zself-profile=Zsp").args(&args); assert!(cmd.status().expect("failed to spawn").success()); } "time-passes" => { - let mut cmd = Command::new(&rustc); + let mut cmd = Command::new(&tool); cmd.arg("-Ztime-passes").args(&args); assert!(cmd.status().expect("failed to spawn").success()); @@ -113,7 +123,7 @@ fn main() { .arg("--output=perf") .arg("--freq=299") .arg("--event=cycles:u,instructions:u") - .arg(&rustc) + .arg(&tool) .args(&args); assert!(cmd.status().expect("failed to spawn").success()); @@ -124,7 +134,7 @@ fn main() { let has_oprofile = cmd.output().is_ok(); assert!(has_oprofile); // Other possibly useful args: --callgraph, --separate-thread - cmd.arg("operf").arg(&rustc).args(&args); + cmd.arg("operf").arg(&tool).args(&args); assert!(cmd.status().expect("failed to spawn").success()); } @@ -140,7 +150,7 @@ fn main() { .arg("--cache-sim=no") .arg("--branch-sim=no") .arg("--cachegrind-out-file=cgout") - .arg(&rustc) + .arg(&tool) .args(&args); assert!(cmd.status().expect("failed to spawn").success()); @@ -157,7 +167,7 @@ fn main() { .arg("--cache-sim=no") .arg("--branch-sim=no") .arg("--callgrind-out-file=clgout") - .arg(&rustc) + .arg(&tool) .args(&args); assert!(cmd.status().expect("failed to spawn").success()); @@ -170,7 +180,7 @@ fn main() { cmd.arg("--tool=dhat") .arg("--num-callers=4") .arg("--dhat-out-file=dhout") - .arg(&rustc) + .arg(&tool) .args(&args); assert!(cmd.status().expect("failed to spawn").success()); @@ -186,14 +196,14 @@ fn main() { .arg("--threshold=0.2") .arg("--massif-out-file=msout") .arg("--alloc-fn=__rdl_alloc") - .arg(&rustc) + .arg(&tool) .args(&args); assert!(cmd.status().expect("failed to spawn").success()); } "eprintln" | "llvm-lines" => { - let mut cmd = Command::new(&rustc); + let mut cmd = Command::new(&tool); cmd.args(&args); assert!(cmd.status().expect("failed to spawn").success()); @@ -204,7 +214,7 @@ fn main() { } } } else { - let mut cmd = Command::new(&rustc); + let mut cmd = Command::new(&tool); cmd.args(&args); exec(&mut cmd); } diff --git a/collector/src/bin/rustc-perf-collector/execute.rs b/collector/src/bin/rustc-perf-collector/execute.rs index 10d1e0683..24e675814 100644 --- a/collector/src/bin/rustc-perf-collector/execute.rs +++ b/collector/src/bin/rustc-perf-collector/execute.rs @@ -236,7 +236,9 @@ impl<'a> CargoProcess<'a> { // Not all cargo invocations (e.g. `cargo clean`) need all of these // env vars set, but it doesn't hurt to have them. .env("RUSTC", &*FAKE_RUSTC) + .env("RUSTDOC", &*FAKE_RUSTDOC) .env("RUSTC_REAL", &self.compiler.rustc) + .env("RUSTDOC_REAL", dbg!(&self.compiler.rustdoc)) .env( "CARGO_INCREMENTAL", &format!("{}", self.incremental as usize), @@ -280,7 +282,7 @@ impl<'a> CargoProcess<'a> { } if self.build_kind == BuildKind::Doc { - dbg!("doc") + "rustdoc" } else { profiler.subcommand() } @@ -309,8 +311,9 @@ impl<'a> CargoProcess<'a> { // onto rustc for the final crate, which is exactly the crate for which // we want to wrap rustc. if let Some((ref mut processor, ..)) = self.processor_etc { + let profiler = processor.profiler().name(); cmd.arg("--wrap-rustc-with"); - cmd.arg(processor.profiler().name()); + cmd.arg(profiler); cmd.args(&self.rustc_args); } @@ -347,6 +350,16 @@ lazy_static::lazy_static! { fake_rustc.push("rustc-fake"); fake_rustc }; + static ref FAKE_RUSTDOC: PathBuf = { + let mut fake_rustdoc = env::current_exe().unwrap(); + fake_rustdoc.pop(); + fake_rustdoc.push("rustdoc-fake"); + // link from rustc-fake to rustdoc-fake + if !fake_rustdoc.exists() { + std::fs::hard_link(&*FAKE_RUSTC, &fake_rustdoc).expect("failed to make hard link"); + } + fake_rustdoc + }; } /// Used to indicate if we need to retry a run. diff --git a/collector/src/bin/rustc-perf-collector/main.rs b/collector/src/bin/rustc-perf-collector/main.rs index a0609b0ce..8bc0cb819 100644 --- a/collector/src/bin/rustc-perf-collector/main.rs +++ b/collector/src/bin/rustc-perf-collector/main.rs @@ -29,6 +29,7 @@ use sysroot::Sysroot; #[derive(Debug, Copy, Clone)] pub struct Compiler<'a> { pub rustc: &'a Path, + pub rustdoc: &'a Path, pub cargo: &'a Path, pub triple: &'a str, pub is_nightly: bool, @@ -38,6 +39,7 @@ impl<'a> Compiler<'a> { fn from_sysroot(sysroot: &'a Sysroot) -> Compiler<'a> { Compiler { rustc: &sysroot.rustc, + rustdoc: &sysroot.rustdoc, cargo: &sysroot.cargo, triple: &sysroot.triple, is_nightly: true, @@ -86,6 +88,7 @@ pub enum KindError { const STRINGS_AND_BUILD_KINDS: &[(&str, BuildKind)] = &[ ("Check", BuildKind::Check), ("Debug", BuildKind::Debug), + ("Doc", BuildKind::Doc), ("Opt", BuildKind::Opt), ]; @@ -395,6 +398,7 @@ fn main_result() -> anyhow::Result { (@subcommand bench_local => (about: "benchmark a local rustc") (@arg RUSTC: --rustc +required +takes_value "The path to the local rustc to benchmark") + (@arg RUSTDOC: --rustdoc +required +takes_value "The path to the local rustdoc to benchmark") (@arg CARGO: --cargo +required +takes_value "The path to the local Cargo to use") (@arg BUILDS: --builds +takes_value "One or more (comma-separated) of: 'Check', 'Debug',\n\ @@ -482,12 +486,14 @@ fn main_result() -> anyhow::Result { ("bench_local", Some(sub_m)) => { let rustc = sub_m.value_of("RUSTC").unwrap(); + let rustdoc = sub_m.value_of("RUSTDOC").unwrap(); let cargo = sub_m.value_of("CARGO").unwrap(); let build_kinds = build_kinds_from_arg(&sub_m.value_of("BUILDS"))?; let run_kinds = run_kinds_from_arg(&sub_m.value_of("RUNS"))?; let id = sub_m.value_of("ID").unwrap(); let rustc_path = PathBuf::from(rustc).canonicalize()?; + let rustdoc_path = PathBuf::from(rustdoc).canonicalize()?; let cargo_path = PathBuf::from(cargo).canonicalize()?; let conn = rt.block_on(pool.expect("--db passed").connection()); bench_commit( @@ -498,6 +504,7 @@ fn main_result() -> anyhow::Result { &run_kinds, Compiler { rustc: &rustc_path, + rustdoc: &rustdoc_path, cargo: &cargo_path, triple: "x86_64-unknown-linux-gnu", is_nightly: true, @@ -520,28 +527,22 @@ fn main_result() -> anyhow::Result { anyhow::bail!("failed to install toolchain for {}", id); } - let rustc = String::from_utf8( - Command::new("rustup") - .arg("which") - .arg("--toolchain") - .arg(&id) - .arg("rustc") - .output() - .context("rustup which rustc")? - .stdout, - ) - .context("utf8")?; - let cargo = String::from_utf8( - Command::new("rustup") - .arg("which") - .arg("--toolchain") - .arg(&id) - .arg("cargo") - .output() - .context("rustup which cargo")? - .stdout, - ) - .context("utf8")?; + let which = |tool| { + String::from_utf8( + Command::new("rustup") + .arg("which") + .arg("--toolchain") + .arg(&id) + .arg(tool) + .output() + .context(format!("rustup which {}", tool))? + .stdout, + ) + .context("utf8") + }; + let rustc = which("rustc")?; + let rustdoc = which("rustdoc")?; + let cargo = which("cargo")?; // Remove benchmarks that don't work with a stable compiler. benchmarks.retain(|b| b.supports_stable()); @@ -560,6 +561,7 @@ fn main_result() -> anyhow::Result { &run_kinds, Compiler { rustc: Path::new(rustc.trim()), + rustdoc: Path::new(rustdoc.trim()), cargo: Path::new(cargo.trim()), is_nightly: false, triple: "x86_64-unknown-linux-gnu", @@ -584,6 +586,7 @@ fn main_result() -> anyhow::Result { ("profile", Some(sub_m)) => { let rustc = sub_m.value_of("RUSTC").unwrap(); + let rustdoc = sub_m.value_of("RUSTDOC").unwrap(); let cargo = sub_m.value_of("CARGO").unwrap(); let build_kinds = build_kinds_from_arg(&sub_m.value_of("BUILDS"))?; let run_kinds = run_kinds_from_arg(&sub_m.value_of("RUNS"))?; @@ -594,9 +597,11 @@ fn main_result() -> anyhow::Result { eprintln!("Profiling with {:?}", profiler); let rustc_path = PathBuf::from(rustc).canonicalize()?; + let rustdoc_path = PathBuf::from(rustdoc).canonicalize()?; let cargo_path = PathBuf::from(cargo).canonicalize()?; let compiler = Compiler { rustc: &rustc_path, + rustdoc: &rustdoc_path, cargo: &cargo_path, is_nightly: true, triple: "x86_64-unknown-linux-gnu", // XXX: Technically not necessarily true From ef7734e3f4840c06de537f0cb261ba25816a9dd4 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sat, 27 Jun 2020 08:41:55 -0400 Subject: [PATCH 3/9] Add a few missing `Doc` variants - Add 'Doc' to CLI help - Make Doc a default BuildKind when --builds is not passed --- .../src/bin/rustc-perf-collector/execute.rs | 4 ++-- .../src/bin/rustc-perf-collector/main.rs | 23 ++++++++++++++----- database/src/pool/sqlite.rs | 1 + 3 files changed, 20 insertions(+), 8 deletions(-) diff --git a/collector/src/bin/rustc-perf-collector/execute.rs b/collector/src/bin/rustc-perf-collector/execute.rs index 24e675814..58d45efd9 100644 --- a/collector/src/bin/rustc-perf-collector/execute.rs +++ b/collector/src/bin/rustc-perf-collector/execute.rs @@ -238,7 +238,7 @@ impl<'a> CargoProcess<'a> { .env("RUSTC", &*FAKE_RUSTC) .env("RUSTDOC", &*FAKE_RUSTDOC) .env("RUSTC_REAL", &self.compiler.rustc) - .env("RUSTDOC_REAL", dbg!(&self.compiler.rustdoc)) + .env("RUSTDOC_REAL", &self.compiler.rustdoc) .env( "CARGO_INCREMENTAL", &format!("{}", self.incremental as usize), @@ -247,7 +247,7 @@ impl<'a> CargoProcess<'a> { .arg(subcommand) .arg("--manifest-path") .arg(&self.manifest_path); - dbg!(cmd) + cmd } fn get_pkgid(&self, cwd: &Path) -> String { diff --git a/collector/src/bin/rustc-perf-collector/main.rs b/collector/src/bin/rustc-perf-collector/main.rs index 8bc0cb819..a5b8f9f32 100644 --- a/collector/src/bin/rustc-perf-collector/main.rs +++ b/collector/src/bin/rustc-perf-collector/main.rs @@ -55,6 +55,17 @@ pub enum BuildKind { Opt, } +impl BuildKind { + fn all() -> Vec { + vec![ + BuildKind::Check, + BuildKind::Debug, + BuildKind::Doc, + BuildKind::Opt, + ] + } +} + #[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)] pub enum RunKind { Full, @@ -104,7 +115,7 @@ pub fn build_kinds_from_arg(arg: &Option<&str>) -> Result, KindEr if let Some(arg) = arg { kinds_from_arg(STRINGS_AND_BUILD_KINDS, arg) } else { - Ok(vec![BuildKind::Check, BuildKind::Debug, BuildKind::Opt]) + Ok(BuildKind::all()) } } @@ -176,7 +187,7 @@ fn process_commits( rt, conn, &ArtifactId::Commit(commit), - &[BuildKind::Check, BuildKind::Debug, BuildKind::Opt], + &BuildKind::all(), &RunKind::all(), Compiler::from_sysroot(&sysroot), &benchmarks, @@ -402,7 +413,7 @@ fn main_result() -> anyhow::Result { (@arg CARGO: --cargo +required +takes_value "The path to the local Cargo to use") (@arg BUILDS: --builds +takes_value "One or more (comma-separated) of: 'Check', 'Debug',\n\ - 'Opt', 'All'") + 'Doc', 'Opt', 'All'") (@arg RUNS: --runs +takes_value "One or more (comma-separated) of: 'Full',\n\ 'IncrFull', 'IncrUnchanged', 'IncrPatched', 'All'") @@ -466,14 +477,14 @@ fn main_result() -> anyhow::Result { let commit = sub_m.value_of("COMMIT").unwrap(); let commit = get_commit_or_fake_it(&commit)?; let sysroot = Sysroot::install(commit.sha.to_string(), "x86_64-unknown-linux-gnu")?; - let build_kinds = &[BuildKind::Check, BuildKind::Debug, BuildKind::Opt]; + let build_kinds = BuildKind::all(); let run_kinds = RunKind::all(); let conn = rt.block_on(pool.expect("--db passed").connection()); bench_commit( &mut rt, conn, &ArtifactId::Commit(commit), - build_kinds, + &build_kinds, &run_kinds, Compiler::from_sysroot(&sysroot), &benchmarks, @@ -557,7 +568,7 @@ fn main_result() -> anyhow::Result { &mut rt, conn, &ArtifactId::Artifact(id.to_string()), - &[BuildKind::Check, BuildKind::Debug, BuildKind::Opt], + &BuildKind::all(), &run_kinds, Compiler { rustc: Path::new(rustc.trim()), diff --git a/database/src/pool/sqlite.rs b/database/src/pool/sqlite.rs index 7c9b1ac41..a71b09d71 100644 --- a/database/src/pool/sqlite.rs +++ b/database/src/pool/sqlite.rs @@ -298,6 +298,7 @@ impl Connection for SqliteConnection { "check" => Profile::Check, "opt" => Profile::Opt, "debug" => Profile::Debug, + "doc" => Profile::Doc, o => unreachable!("{}: not a profile", o), }, row.get::<_, String>(3)?.as_str().parse().unwrap(), From 2c5171f2a9266ec2bd957e773eb639b686c2b149 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sun, 28 Jun 2020 23:03:41 -0400 Subject: [PATCH 4/9] Avoid panics in the webserver - Don't panic if looking at two local builds in the dashboard - Don't panic if there are no pstat results available --- database/src/pool/sqlite.rs | 1 - site/src/server.rs | 10 +++++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/database/src/pool/sqlite.rs b/database/src/pool/sqlite.rs index a71b09d71..edffb8686 100644 --- a/database/src/pool/sqlite.rs +++ b/database/src/pool/sqlite.rs @@ -331,7 +331,6 @@ impl Connection for SqliteConnection { cid.and_then(|cid| { query .query_row(params![&sid, &cid.0], |row| row.get(0)) - .optional() .unwrap_or_else(|e| { panic!("{:?}: series={:?}, aid={:?}", e, sid, cid); }) diff --git a/site/src/server.rs b/site/src/server.rs index 721f3e052..b562a2946 100644 --- a/site/src/server.rs +++ b/site/src/server.rs @@ -109,14 +109,18 @@ pub async fn handle_dashboard(data: Arc) -> ServerResult a.cmp(&b), (_, _) => { + use std::cmp::Ordering; + if a.starts_with("beta") && b.starts_with("beta") { a.cmp(b) } else if a.starts_with("beta") { - std::cmp::Ordering::Greater + Ordering::Greater } else if b.starts_with("beta") { - std::cmp::Ordering::Less + Ordering::Less } else { - panic!("unexpected version") + // These are both local ids, not a commit. + // There's no way to tell which version they are, so just pretend they're the same. + Ordering::Equal } } } From 82be51a98bd4ca958aece1f0e474968db341707d Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Mon, 29 Jun 2020 20:44:36 -0400 Subject: [PATCH 5/9] Don't hard-code `rustdoc` subcommand - Change `subcommand()` to take BuildKind and return Option - Remove `is_build_kind_supported` --- .../src/bin/rustc-perf-collector/execute.rs | 45 ++++++------------- 1 file changed, 14 insertions(+), 31 deletions(-) diff --git a/collector/src/bin/rustc-perf-collector/execute.rs b/collector/src/bin/rustc-perf-collector/execute.rs index 58d45efd9..45a249221 100644 --- a/collector/src/bin/rustc-perf-collector/execute.rs +++ b/collector/src/bin/rustc-perf-collector/execute.rs @@ -146,7 +146,7 @@ impl Profiler { // What cargo subcommand do we need to run for this profiler? If not // `rustc`, must be a subcommand that itself invokes `rustc`. - fn subcommand(&self) -> &'static str { + fn subcommand(&self, build_kind: BuildKind) -> Option<&'static str> { match self { Profiler::PerfStat | Profiler::PerfStatSelfProfile @@ -158,27 +158,14 @@ impl Profiler { | Profiler::Callgrind | Profiler::DHAT | Profiler::Massif - | Profiler::Eprintln => "rustc", - Profiler::LlvmLines => "llvm-lines", - } - } - - fn is_build_kind_allowed(&self, build_kind: BuildKind) -> bool { - match self { - Profiler::PerfStat - | Profiler::PerfStatSelfProfile - | Profiler::SelfProfile - | Profiler::TimePasses - | Profiler::PerfRecord - | Profiler::OProfile - | Profiler::Cachegrind - | Profiler::Callgrind - | Profiler::DHAT - | Profiler::Massif - | Profiler::Eprintln => true, + | Profiler::Eprintln => if build_kind == BuildKind::Doc { + Some("rustdoc") + } else { + Some("rustc") + }, Profiler::LlvmLines => match build_kind { - BuildKind::Debug | BuildKind::Opt => true, - BuildKind::Check | BuildKind::Doc => false, + BuildKind::Debug | BuildKind::Opt => Some("rustc"), + BuildKind::Check | BuildKind::Doc => None, } } } @@ -268,12 +255,6 @@ impl<'a> CargoProcess<'a> { // machinery works). let subcommand = if let Some((ref mut processor, run_kind, ..)) = self.processor_etc { let profiler = processor.profiler(); - if !profiler.is_build_kind_allowed(self.build_kind) { - return Err(anyhow::anyhow!( - "this profiler doesn't support {:?} builds", - self.build_kind - )); - } if !profiler.is_run_kind_allowed(run_kind) { return Err(anyhow::anyhow!( "this profiler doesn't support {:?} runs", @@ -281,10 +262,12 @@ impl<'a> CargoProcess<'a> { )); } - if self.build_kind == BuildKind::Doc { - "rustdoc" - } else { - profiler.subcommand() + match profiler.subcommand(self.build_kind) { + None => return Err(anyhow::anyhow!( + "this profiler doesn't support {:?} builds", + self.build_kind + )), + Some(sub) => sub, } } else { "rustc" From a40c5a3197b55885dc3886de0a74b3ed0f660c09 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Thu, 2 Jul 2020 12:48:22 -0400 Subject: [PATCH 6/9] Run Doc builds in CI This ensures that the doc build will succeed when run later, since rustdoc and rustc are known to have different ICEs. --- collector/src/bin/rustc-perf-collector/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/collector/src/bin/rustc-perf-collector/main.rs b/collector/src/bin/rustc-perf-collector/main.rs index a5b8f9f32..7b07e451d 100644 --- a/collector/src/bin/rustc-perf-collector/main.rs +++ b/collector/src/bin/rustc-perf-collector/main.rs @@ -650,7 +650,7 @@ fn main_result() -> anyhow::Result { &mut rt, conn, &ArtifactId::Commit(commit), - &[BuildKind::Check], // no Debug or Opt builds + &[BuildKind::Check, BuildKind::Doc], // no Debug or Opt builds &RunKind::all(), Compiler::from_sysroot(&sysroot), &benchmarks, From 5842d3eb8e6f1c56b26f652828b9748b4e86780e Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Thu, 2 Jul 2020 14:42:18 -0400 Subject: [PATCH 7/9] Use symlinks instead of hard links This avoids rustdoc-fake getting out of date when rustc-fake is updated. --- collector/src/bin/rustc-perf-collector/execute.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/collector/src/bin/rustc-perf-collector/execute.rs b/collector/src/bin/rustc-perf-collector/execute.rs index 45a249221..f7ac680f6 100644 --- a/collector/src/bin/rustc-perf-collector/execute.rs +++ b/collector/src/bin/rustc-perf-collector/execute.rs @@ -339,7 +339,12 @@ lazy_static::lazy_static! { fake_rustdoc.push("rustdoc-fake"); // link from rustc-fake to rustdoc-fake if !fake_rustdoc.exists() { - std::fs::hard_link(&*FAKE_RUSTC, &fake_rustdoc).expect("failed to make hard link"); + #[cfg(unix)] + use std::os::unix::fs::symlink; + #[cfg(windows)] + use std::os::windows::fs::symlink_file as symlink; + + symlink(&*FAKE_RUSTC, &fake_rustdoc).expect("failed to make symbolic link"); } fake_rustdoc }; From f3c3a300f56db38de4b0af3e8a59fa98e243519e Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Thu, 2 Jul 2020 16:23:25 -0400 Subject: [PATCH 8/9] Print backtraces for errors --- collector/src/bin/rustc-perf-collector/main.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/collector/src/bin/rustc-perf-collector/main.rs b/collector/src/bin/rustc-perf-collector/main.rs index 7b07e451d..0cdad4fcc 100644 --- a/collector/src/bin/rustc-perf-collector/main.rs +++ b/collector/src/bin/rustc-perf-collector/main.rs @@ -331,7 +331,8 @@ fn get_benchmarks( exclude: Option<&str>, ) -> anyhow::Result> { let mut benchmarks = Vec::new(); - 'outer: for entry in fs::read_dir(benchmark_dir).context("failed to list benchmarks")? { + 'outer: for entry in fs::read_dir(benchmark_dir) + .with_context(|| format!("failed to list benchmark dir {}", benchmark_dir.display()))? { let entry = entry?; let path = entry.path(); let name = match entry.file_name().into_string() { @@ -383,7 +384,7 @@ fn main() { match main_result() { Ok(code) => process::exit(code), Err(err) => { - eprintln!("{}", err); + eprintln!("{:#}\n{}", err, err.backtrace()); process::exit(1); } } From 3c67e796d0750694b33522208b8bd2a922330c1b Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Tue, 14 Jul 2020 09:10:23 -0400 Subject: [PATCH 9/9] Make rustdoc optional for local profiling --- .../src/bin/rustc-perf-collector/execute.rs | 24 +++++++----- .../src/bin/rustc-perf-collector/main.rs | 37 +++++++++++++------ 2 files changed, 41 insertions(+), 20 deletions(-) diff --git a/collector/src/bin/rustc-perf-collector/execute.rs b/collector/src/bin/rustc-perf-collector/execute.rs index f7ac680f6..91af94367 100644 --- a/collector/src/bin/rustc-perf-collector/execute.rs +++ b/collector/src/bin/rustc-perf-collector/execute.rs @@ -158,15 +158,17 @@ impl Profiler { | Profiler::Callgrind | Profiler::DHAT | Profiler::Massif - | Profiler::Eprintln => if build_kind == BuildKind::Doc { + | Profiler::Eprintln => { + if build_kind == BuildKind::Doc { Some("rustdoc") } else { Some("rustc") - }, + } + } Profiler::LlvmLines => match build_kind { BuildKind::Debug | BuildKind::Opt => Some("rustc"), BuildKind::Check | BuildKind::Doc => None, - } + }, } } @@ -223,9 +225,7 @@ impl<'a> CargoProcess<'a> { // Not all cargo invocations (e.g. `cargo clean`) need all of these // env vars set, but it doesn't hurt to have them. .env("RUSTC", &*FAKE_RUSTC) - .env("RUSTDOC", &*FAKE_RUSTDOC) .env("RUSTC_REAL", &self.compiler.rustc) - .env("RUSTDOC_REAL", &self.compiler.rustdoc) .env( "CARGO_INCREMENTAL", &format!("{}", self.incremental as usize), @@ -234,6 +234,10 @@ impl<'a> CargoProcess<'a> { .arg(subcommand) .arg("--manifest-path") .arg(&self.manifest_path); + + if let Some(r) = &self.compiler.rustdoc { + cmd.env("RUSTDOC", &*FAKE_RUSTDOC).env("RUSTDOC_REAL", r); + } cmd } @@ -263,10 +267,12 @@ impl<'a> CargoProcess<'a> { } match profiler.subcommand(self.build_kind) { - None => return Err(anyhow::anyhow!( - "this profiler doesn't support {:?} builds", - self.build_kind - )), + None => { + return Err(anyhow::anyhow!( + "this profiler doesn't support {:?} builds", + self.build_kind + )) + } Some(sub) => sub, } } else { diff --git a/collector/src/bin/rustc-perf-collector/main.rs b/collector/src/bin/rustc-perf-collector/main.rs index 0cdad4fcc..a0d29f2e6 100644 --- a/collector/src/bin/rustc-perf-collector/main.rs +++ b/collector/src/bin/rustc-perf-collector/main.rs @@ -29,7 +29,7 @@ use sysroot::Sysroot; #[derive(Debug, Copy, Clone)] pub struct Compiler<'a> { pub rustc: &'a Path, - pub rustdoc: &'a Path, + pub rustdoc: Option<&'a Path>, pub cargo: &'a Path, pub triple: &'a str, pub is_nightly: bool, @@ -39,7 +39,7 @@ impl<'a> Compiler<'a> { fn from_sysroot(sysroot: &'a Sysroot) -> Compiler<'a> { Compiler { rustc: &sysroot.rustc, - rustdoc: &sysroot.rustdoc, + rustdoc: Some(&sysroot.rustdoc), cargo: &sysroot.cargo, triple: &sysroot.triple, is_nightly: true, @@ -239,6 +239,11 @@ fn bench_commit( call_home: bool, self_profile: bool, ) -> BenchmarkErrors { + if compiler.rustdoc.is_none() && build_kinds.iter().any(|b| *b == BuildKind::Doc) { + eprintln!("Rustdoc build specified but rustdoc path not provided"); + std::process::exit(1); + } + let mut errors_recorded = 0; eprintln!("Benchmarking {} for triple {}", cid, compiler.triple); @@ -332,7 +337,8 @@ fn get_benchmarks( ) -> anyhow::Result> { let mut benchmarks = Vec::new(); 'outer: for entry in fs::read_dir(benchmark_dir) - .with_context(|| format!("failed to list benchmark dir {}", benchmark_dir.display()))? { + .with_context(|| format!("failed to list benchmark dir {}", benchmark_dir.display()))? + { let entry = entry?; let path = entry.path(); let name = match entry.file_name().into_string() { @@ -410,7 +416,7 @@ fn main_result() -> anyhow::Result { (@subcommand bench_local => (about: "benchmark a local rustc") (@arg RUSTC: --rustc +required +takes_value "The path to the local rustc to benchmark") - (@arg RUSTDOC: --rustdoc +required +takes_value "The path to the local rustdoc to benchmark") + (@arg RUSTDOC: --rustdoc +takes_value "The path to the local rustdoc to benchmark") (@arg CARGO: --cargo +required +takes_value "The path to the local Cargo to use") (@arg BUILDS: --builds +takes_value "One or more (comma-separated) of: 'Check', 'Debug',\n\ @@ -431,6 +437,7 @@ fn main_result() -> anyhow::Result { (about: "profile a local rustc") (@arg output_dir: --("output") +required +takes_value "Output directory") (@arg RUSTC: --rustc +required +takes_value "The path to the local rustc to benchmark") + (@arg RUSTDOC: --rustdoc +takes_value "The path to the local rustdoc to benchmark") (@arg CARGO: --cargo +required +takes_value "The path to the local Cargo to use") (@arg BUILDS: --builds +takes_value "One or more (comma-separated) of: 'Check', 'Debug',\n\ @@ -498,14 +505,18 @@ fn main_result() -> anyhow::Result { ("bench_local", Some(sub_m)) => { let rustc = sub_m.value_of("RUSTC").unwrap(); - let rustdoc = sub_m.value_of("RUSTDOC").unwrap(); + let rustdoc = sub_m.value_of("RUSTDOC"); let cargo = sub_m.value_of("CARGO").unwrap(); let build_kinds = build_kinds_from_arg(&sub_m.value_of("BUILDS"))?; let run_kinds = run_kinds_from_arg(&sub_m.value_of("RUNS"))?; let id = sub_m.value_of("ID").unwrap(); let rustc_path = PathBuf::from(rustc).canonicalize()?; - let rustdoc_path = PathBuf::from(rustdoc).canonicalize()?; + let rustdoc_path = if let Some(r) = rustdoc { + Some(PathBuf::from(r).canonicalize()?) + } else { + None + }; let cargo_path = PathBuf::from(cargo).canonicalize()?; let conn = rt.block_on(pool.expect("--db passed").connection()); bench_commit( @@ -516,7 +527,7 @@ fn main_result() -> anyhow::Result { &run_kinds, Compiler { rustc: &rustc_path, - rustdoc: &rustdoc_path, + rustdoc: rustdoc_path.as_deref(), cargo: &cargo_path, triple: "x86_64-unknown-linux-gnu", is_nightly: true, @@ -573,7 +584,7 @@ fn main_result() -> anyhow::Result { &run_kinds, Compiler { rustc: Path::new(rustc.trim()), - rustdoc: Path::new(rustdoc.trim()), + rustdoc: Some(Path::new(rustdoc.trim())), cargo: Path::new(cargo.trim()), is_nightly: false, triple: "x86_64-unknown-linux-gnu", @@ -598,7 +609,7 @@ fn main_result() -> anyhow::Result { ("profile", Some(sub_m)) => { let rustc = sub_m.value_of("RUSTC").unwrap(); - let rustdoc = sub_m.value_of("RUSTDOC").unwrap(); + let rustdoc = sub_m.value_of("RUSTDOC"); let cargo = sub_m.value_of("CARGO").unwrap(); let build_kinds = build_kinds_from_arg(&sub_m.value_of("BUILDS"))?; let run_kinds = run_kinds_from_arg(&sub_m.value_of("RUNS"))?; @@ -609,11 +620,15 @@ fn main_result() -> anyhow::Result { eprintln!("Profiling with {:?}", profiler); let rustc_path = PathBuf::from(rustc).canonicalize()?; - let rustdoc_path = PathBuf::from(rustdoc).canonicalize()?; + let rustdoc_path = if let Some(r) = rustdoc { + Some(PathBuf::from(r).canonicalize()?) + } else { + None + }; let cargo_path = PathBuf::from(cargo).canonicalize()?; let compiler = Compiler { rustc: &rustc_path, - rustdoc: &rustdoc_path, + rustdoc: rustdoc_path.as_deref(), cargo: &cargo_path, is_nightly: true, triple: "x86_64-unknown-linux-gnu", // XXX: Technically not necessarily true