Make it easy to add diffing of output of arbitrary profilers (demonstrate for dep-graph)

Author: oli-obk

collector/src/bin/collector.rs

@@ -132,21 +132,15 @@ 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(
+fn generate_diffs(
     id1: &str,
     id2: &str,
     out_dir: &Path,
     benchmarks: &[Benchmark],
     profiles: &[Profile],
     scenarios: &[Scenario],
     errors: &mut BenchmarkErrors,
+    profiler: &Profiler,
 ) -> Vec<PathBuf> {
     let mut annotated_diffs = Vec::new();
     for benchmark in benchmarks {
@@ -166,22 +160,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(profiler.output_prefix(), &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 +1145,26 @@ fn main_result() -> anyhow::Result<i32> {
                 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: {}", diffs[0].to_string_lossy());
-                        eprintln!("Short path: {}", short.to_string_lossy());
-                    } else {
-                        eprintln!("Diffs:");
-                        for diff in diffs {
-                            eprintln!("{}", diff.to_string_lossy());
-                        }
+                profiler.diff_prereqs()?;
+                let diffs = generate_diffs(
+                    &id1,
+                    &id2,
+                    &out_dir,
+                    &benchmarks,
+                    profiles,
+                    scenarios,
+                    &mut errors,
+                    &profiler,
+                );
+                if let [diff] = &diffs[..] {
+                    let short = out_dir.join(format!("{}-latest", profiler.output_prefix()));
+                    std::fs::copy(diff, &short).expect("copy to short path");
+                    eprintln!("Original diff at: {}", diffs[0].to_string_lossy());
+                    eprintln!("Short path: {}", short.to_string_lossy());
+                } else {
+                    eprintln!("Diffs:");
+                    for diff in diffs {
+                        eprintln!("{}", diff.to_string_lossy());
                     }
                 }
             } else {

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, check_installed};
 use anyhow::Context;
 use std::collections::HashMap;
 use std::fs::File;
@@ -51,6 +52,68 @@ impl Profiler {
                 | Profiler::DepGraph
         )
     }
+
+    pub fn diff_prereqs(&self) -> anyhow::Result<()> {
+        use Profiler::*;
+        match self {
+            Cachegrind => {
+                check_installed("valgrind")?;
+                check_installed("cg_annotate")?;
+            }
+
+            SelfProfile | PerfRecord | Oprofile | Samply | Callgrind | Dhat | DhatCopy | Massif
+            | Bytehound | Eprintln | LlvmLines | MonoItems | DepGraph | LlvmIr => {}
+        }
+        Ok(())
+    }
+
+    /// 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 => "",
+        }
+    }
+
+    /// A file prefix to use for the diff output.
+    pub fn output_prefix(&self) -> &'static str {
+        use Profiler::*;
+        match self {
+            Cachegrind => "cgann-diff",
+            DepGraph => "dep-graph-diff",
+
+            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> {

collector/src/utils/diff.rs

@@ -0,0 +1,25 @@
+use std::{
+    fs::File,
+    path::Path,
+    process::{Command, Stdio},
+};
+
+use anyhow::Context as _;
+
+use super::is_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<()> {
+    if !is_installed("diff") {
+        anyhow::bail!("`diff` not installed.");
+    }
+    Command::new("diff")
+        .arg(left)
+        .arg(right)
+        .stderr(Stdio::inherit())
+        .stdout(File::create(out_file)?)
+        .status()
+        .context("failed to run `diff`")?;
+
+    Ok(())
+}

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(())
+}