Avoid Command::output if it's just going to be written to a file anyway

Author: oli-obk

collector/src/compile/execute/bencher.rs

@@ -62,13 +62,13 @@ impl<'a> BenchProcessor<'a> {
             assert!(has_perf);
         } else {
             let has_xperf = Command::new(env::var("XPERF").unwrap_or("xperf.exe".to_string()))
-                .output()
+                .status()
                 .is_ok();
             assert!(has_xperf);
 
             let has_tracelog =
                 Command::new(env::var("TRACELOG").unwrap_or("tracelog.exe".to_string()))
-                    .output()
+                    .status()
                     .is_ok();
             assert!(has_tracelog);
         }

collector/src/compile/execute/profiler.rs

@@ -3,6 +3,7 @@ use crate::utils;
 use crate::utils::cachegrind::cachegrind_annotate;
 use anyhow::Context;
 use std::collections::HashMap;
+use std::fs::File;
 use std::future::Future;
 use std::io::BufRead;
 use std::io::Write;
@@ -143,10 +144,8 @@ impl<'a> Processor for ProfileProcessor<'a> {
                     // Run `summarize`.
                     let mut summarize_cmd = Command::new("summarize");
                     summarize_cmd.arg("summarize").arg(&zsp_files_prefix);
-                    fs::write(
-                        summarize_file,
-                        summarize_cmd.output().context("summarize")?.stdout,
-                    )?;
+                    summarize_cmd.stdout(File::create(summarize_file)?);
+                    summarize_cmd.status().context("summarize")?;
 
                     // Run `flamegraph`.
                     let mut flamegraph_cmd = Command::new("flamegraph");
@@ -198,17 +197,19 @@ impl<'a> Processor for ProfileProcessor<'a> {
                         .arg("--debug-info")
                         .arg("--threshold")
                         .arg("0.5")
-                        .arg(&session_dir_arg);
-                    fs::write(oprep_file, op_report_cmd.output()?.stdout)?;
+                        .arg(&session_dir_arg)
+                        .stdout(File::create(oprep_file)?);
+                    op_report_cmd.status()?;
 
                     let mut op_annotate_cmd = Command::new("opannotate");
                     // Other possibly useful args: --assembly
                     op_annotate_cmd
                         .arg("--source")
                         .arg("--threshold")
                         .arg("0.5")
-                        .arg(&session_dir_arg);
-                    fs::write(opann_file, op_annotate_cmd.output()?.stdout)?;
+                        .arg(&session_dir_arg)
+                        .stdout(File::create(opann_file)?);
+                    op_annotate_cmd.status()?;
                 }
 
                 // Samply produces (via rustc-fake) a data file called
@@ -248,8 +249,9 @@ impl<'a> Processor for ProfileProcessor<'a> {
                     clg_annotate_cmd
                         .arg("--auto=yes")
                         .arg("--show-percs=yes")
-                        .arg(&clgout_file);
-                    fs::write(clgann_file, clg_annotate_cmd.output()?.stdout)?;
+                        .arg(&clgout_file)
+                        .stdout(File::create(clgann_file)?);
+                    clg_annotate_cmd.status()?;
                 }
 
                 // DHAT produces (via rustc-fake) a data file called `dhout`. We

collector/src/toolchain.rs

@@ -372,13 +372,13 @@ pub fn get_local_toolchain(
     // specified. This follows the similar pattern used by rustup's binaries
     // (e.g., `rustc +stage1`).
     let (rustc, id) = if let Some(toolchain) = rustc.strip_prefix('+') {
-        let output = Command::new("rustup")
+        let status = Command::new("rustup")
             .args(["which", "rustc", "--toolchain", toolchain])
-            .output()
+            .status()
             .context("failed to run `rustup which rustc`")?;
 
         // Looks like a commit hash? Try to install it...
-        if !output.status.success() && toolchain.len() == 40 {
+        if !status.success() && toolchain.len() == 40 {
             // No such toolchain exists, so let's try to install it with
             // rustup-toolchain-install-master.
 
@@ -418,9 +418,10 @@ pub fn get_local_toolchain(
             .output()
             .context("failed to run `rustup which rustc`")?;
 
-        if !output.status.success() {
-            anyhow::bail!("did not manage to obtain toolchain {}", toolchain);
-        }
+        anyhow::ensure!(
+            output.status.success(),
+            "did not manage to obtain toolchain {toolchain}"
+        );
 
         let s = String::from_utf8(output.stdout)
             .context("failed to convert `rustup which rustc` output to utf8")?;

collector/src/utils/cachegrind.rs

@@ -1,6 +1,7 @@
 use crate::utils::is_installed;
 use crate::utils::mangling::demangle_file;
 use anyhow::Context;
+use std::fs::File;
 use std::io::{BufRead, Write};
 use std::path::Path;
 use std::process::{Command, Stdio};
@@ -61,8 +62,9 @@ pub fn cachegrind_annotate(
     cg_annotate_cmd
         .arg("--auto=yes")
         .arg("--show-percs=yes")
-        .arg(cgout_output);
-    fs::write(cgann_output, cg_annotate_cmd.output()?.stdout)?;
+        .arg(cgout_output)
+        .stdout(File::create(cgann_output)?);
+    cg_annotate_cmd.status()?;
     Ok(())
 }
 
@@ -81,38 +83,32 @@ fn run_cg_diff(cgout1: &Path, cgout2: &Path, path: &Path) -> anyhow::Result<()>
     if !is_installed("cg_diff") {
         anyhow::bail!("`cg_diff` not installed.");
     }
-    let output = Command::new("cg_diff")
+    let status = Command::new("cg_diff")
         .arg(r"--mod-filename=s/\/rustc\/[^\/]*\///")
         .arg("--mod-funcname=s/[.]llvm[.].*//")
         .arg(cgout1)
         .arg(cgout2)
         .stderr(Stdio::inherit())
-        .output()
+        .stdout(File::create(path)?)
+        .status()
         .context("failed to run `cg_diff`")?;
 
-    if !output.status.success() {
-        anyhow::bail!("failed to generate cachegrind diff");
-    }
-
-    fs::write(path, output.stdout).context("failed to write `cg_diff` output")?;
+    anyhow::ensure!(status.success(), "failed to generate cachegrind diff");
 
     Ok(())
 }
 
 /// Postprocess Cachegrind output file and writes the result to `path`.
 fn annotate_diff(cgout: &Path, path: &Path) -> anyhow::Result<()> {
-    let output = Command::new("cg_annotate")
+    let status = Command::new("cg_annotate")
         .arg("--show-percs=no")
         .arg(cgout)
         .stderr(Stdio::inherit())
-        .output()
+        .stdout(File::create(path)?)
+        .status()
         .context("failed to run `cg_annotate`")?;
 
-    if !output.status.success() {
-        anyhow::bail!("failed to annotate cachegrind output");
-    }
-
-    fs::write(path, output.stdout).context("failed to write `cg_annotate` output")?;
+    anyhow::ensure!(status.success(), "failed to annotate cachegrind output");
 
     Ok(())
 }

collector/src/utils/mod.rs

@@ -1,5 +1,5 @@
 use std::future::Future;
-use std::process::Command;
+use std::process::{Command, Stdio};
 
 pub mod cachegrind;
 pub mod fs;
@@ -17,5 +17,9 @@ pub fn wait_for_future<F: Future<Output = R>, R>(f: F) -> R {
 
 /// Checks if the given binary can be executed.
 pub fn is_installed(name: &str) -> bool {
-    Command::new(name).output().is_ok()
+    Command::new(name)
+        .stdout(Stdio::null())
+        .stderr(Stdio::null())
+        .status()
+        .is_ok()
 }