diff --git a/collector/src/bin/collector.rs b/collector/src/bin/collector.rs index 7227ec7d4..20e412987 100644 --- a/collector/src/bin/collector.rs +++ b/collector/src/bin/collector.rs @@ -132,14 +132,8 @@ fn check_measureme_installed() -> Result<(), String> { } } -fn check_installed(name: &str) -> anyhow::Result<()> { - if !is_installed(name) { - anyhow::bail!("`{}` is not installed but must be", name); - } - Ok(()) -} - -fn generate_cachegrind_diffs( +#[allow(clippy::too_many_arguments)] +fn generate_diffs( id1: &str, id2: &str, out_dir: &Path, @@ -147,6 +141,7 @@ fn generate_cachegrind_diffs( profiles: &[Profile], scenarios: &[Scenario], errors: &mut BenchmarkErrors, + profiler: &Profiler, ) -> Vec { let mut annotated_diffs = Vec::new(); for benchmark in benchmarks { @@ -166,22 +161,28 @@ fn generate_cachegrind_diffs( }) { let filename = |prefix, id| { format!( - "{}-{}-{}-{:?}-{}", - prefix, id, benchmark.name, profile, scenario + "{}-{}-{}-{:?}-{}{}", + prefix, + id, + benchmark.name, + profile, + scenario, + profiler.postfix() ) }; let id_diff = format!("{}-{}", id1, id2); - let cgout1 = out_dir.join(filename("cgout", id1)); - let cgout2 = out_dir.join(filename("cgout", id2)); - let cgann_diff = out_dir.join(filename("cgann-diff", &id_diff)); + let prefix = profiler.prefix(); + let left = out_dir.join(filename(prefix, id1)); + let right = out_dir.join(filename(prefix, id2)); + let output = out_dir.join(filename(&format!("{prefix}-diff"), &id_diff)); - if let Err(e) = cachegrind_diff(&cgout1, &cgout2, &cgann_diff) { + if let Err(e) = profiler.diff(&left, &right, &output) { errors.incr(); eprintln!("collector error: {:?}", e); continue; } - annotated_diffs.push(cgann_diff); + annotated_diffs.push(output); } } } @@ -1145,29 +1146,25 @@ fn main_result() -> anyhow::Result { let id1 = get_toolchain_and_profile(local.rustc.as_str(), "1")?; let id2 = get_toolchain_and_profile(rustc2.as_str(), "2")?; - if profiler == Profiler::Cachegrind { - check_installed("valgrind")?; - check_installed("cg_annotate")?; - - let diffs = generate_cachegrind_diffs( - &id1, - &id2, - &out_dir, - &benchmarks, - profiles, - scenarios, - &mut errors, - ); - if let [diff] = &diffs[..] { - let short = out_dir.join("cgann-diff-latest"); - std::fs::copy(diff, &short).expect("copy to short path"); - eprintln!("Original diff at: {}", diff.to_string_lossy()); - eprintln!("Short path: {}", short.to_string_lossy()); - } else { - eprintln!("Diffs:"); - for diff in diffs { - eprintln!("{}", diff.to_string_lossy()); - } + let diffs = generate_diffs( + &id1, + &id2, + &out_dir, + &benchmarks, + profiles, + scenarios, + &mut errors, + &profiler, + ); + if let [diff] = &diffs[..] { + let short = out_dir.join(format!("{}-diff-latest", profiler.prefix())); + std::fs::copy(diff, &short).expect("copy to short path"); + eprintln!("Original diff at: {}", diff.to_string_lossy()); + eprintln!("Short path: {}", short.to_string_lossy()); + } else { + eprintln!("Diffs:"); + for diff in diffs { + eprintln!("{}", diff.to_string_lossy()); } } } else { diff --git a/collector/src/compile/execute/profiler.rs b/collector/src/compile/execute/profiler.rs index 1c27617b4..ce6a4562f 100644 --- a/collector/src/compile/execute/profiler.rs +++ b/collector/src/compile/execute/profiler.rs @@ -1,6 +1,7 @@ use crate::compile::execute::{PerfTool, ProcessOutputData, Processor, Retry}; -use crate::utils; -use crate::utils::cachegrind::cachegrind_annotate; +use crate::utils::cachegrind::{cachegrind_annotate, cachegrind_diff}; +use crate::utils::diff::run_diff; +use crate::utils::{self}; use anyhow::Context; use std::collections::HashMap; use std::fs::File; @@ -51,6 +52,42 @@ impl Profiler { | Profiler::DepGraph ) } + + /// A file prefix added to all files of this profiler. + pub fn prefix(&self) -> &'static str { + use Profiler::*; + match self { + Cachegrind => "cgout", + DepGraph => "dep-graph", + + SelfProfile | PerfRecord | Oprofile | Samply | Callgrind | Dhat | DhatCopy | Massif + | Bytehound | Eprintln | LlvmLines | MonoItems | LlvmIr => "", + } + } + + /// A postfix added to the file that gets diffed. + pub fn postfix(&self) -> &'static str { + use Profiler::*; + match self { + Cachegrind => "", + DepGraph => ".txt", + + SelfProfile | PerfRecord | Oprofile | Samply | Callgrind | Dhat | DhatCopy | Massif + | Bytehound | Eprintln | LlvmLines | MonoItems | LlvmIr => "", + } + } + + /// Actually perform the diff + pub fn diff(&self, left: &Path, right: &Path, output: &Path) -> anyhow::Result<()> { + use Profiler::*; + match self { + Cachegrind => cachegrind_diff(left, right, output), + DepGraph => run_diff(left, right, output), + + SelfProfile | PerfRecord | Oprofile | Samply | Callgrind | Dhat | DhatCopy | Massif + | Bytehound | Eprintln | LlvmLines | MonoItems | LlvmIr => Ok(()), + } + } } pub struct ProfileProcessor<'a> { diff --git a/collector/src/utils/cachegrind.rs b/collector/src/utils/cachegrind.rs index fe7584758..682740af8 100644 --- a/collector/src/utils/cachegrind.rs +++ b/collector/src/utils/cachegrind.rs @@ -1,4 +1,3 @@ -use crate::utils::is_installed; use crate::utils::mangling::demangle_file; use anyhow::Context; use std::fs::File; @@ -7,6 +6,8 @@ use std::path::Path; use std::process::{Command, Stdio}; use std::{fs, io}; +use super::check_installed; + /// Annotate and demangle the output of Cachegrind using the `cg_annotate` tool. pub fn cachegrind_annotate( input: &Path, @@ -70,6 +71,7 @@ pub fn cachegrind_annotate( /// Creates a diff between two `cgout` files, and annotates the diff. pub fn cachegrind_diff(cgout_a: &Path, cgout_b: &Path, output: &Path) -> anyhow::Result<()> { + check_installed("valgrind")?; let cgout_diff = tempfile::NamedTempFile::new()?.into_temp_path(); run_cg_diff(cgout_a, cgout_b, &cgout_diff).context("Cannot run cg_diff")?; @@ -80,9 +82,7 @@ pub fn cachegrind_diff(cgout_a: &Path, cgout_b: &Path, output: &Path) -> anyhow: /// Compares two Cachegrind output files using `cg_diff` and writes the result to `path`. fn run_cg_diff(cgout1: &Path, cgout2: &Path, path: &Path) -> anyhow::Result<()> { - if !is_installed("cg_diff") { - anyhow::bail!("`cg_diff` not installed."); - } + check_installed("cg_diff")?; let status = Command::new("cg_diff") .arg(r"--mod-filename=s/\/rustc\/[^\/]*\///") .arg("--mod-funcname=s/[.]llvm[.].*//") @@ -100,6 +100,7 @@ fn run_cg_diff(cgout1: &Path, cgout2: &Path, path: &Path) -> anyhow::Result<()> /// Postprocess Cachegrind output file and writes the result to `path`. fn annotate_diff(cgout: &Path, path: &Path) -> anyhow::Result<()> { + check_installed("cg_annotate")?; let status = Command::new("cg_annotate") .arg("--show-percs=no") .arg(cgout) diff --git a/collector/src/utils/diff.rs b/collector/src/utils/diff.rs new file mode 100644 index 000000000..7a5b70c11 --- /dev/null +++ b/collector/src/utils/diff.rs @@ -0,0 +1,23 @@ +use std::{ + fs::File, + path::Path, + process::{Command, Stdio}, +}; + +use anyhow::Context as _; + +use super::check_installed; + +/// Compares two text files using `diff` and writes the result to `path`. +pub fn run_diff(left: &Path, right: &Path, out_file: &Path) -> anyhow::Result<()> { + check_installed("diff")?; + Command::new("diff") + .arg(left) + .arg(right) + .stderr(Stdio::inherit()) + .stdout(File::create(out_file)?) + .status() + .context("failed to run `diff`")?; + + Ok(()) +} diff --git a/collector/src/utils/mod.rs b/collector/src/utils/mod.rs index bf98cee07..02684e305 100644 --- a/collector/src/utils/mod.rs +++ b/collector/src/utils/mod.rs @@ -2,6 +2,7 @@ use std::future::Future; use std::process::{Command, Stdio}; pub mod cachegrind; +pub mod diff; pub mod fs; pub mod git; pub mod mangling; @@ -23,3 +24,11 @@ pub fn is_installed(name: &str) -> bool { .status() .is_ok() } + +/// Checks if the given binary can be executed and bails otherwise. +pub fn check_installed(name: &str) -> anyhow::Result<()> { + if !is_installed(name) { + anyhow::bail!("`{}` is not installed but must be", name); + } + Ok(()) +}