From 5868f081ad17b407ba234555764db1da37a221cb Mon Sep 17 00:00:00 2001 From: Tavian Barnes Date: Sat, 14 May 2022 13:21:41 -0400 Subject: [PATCH] Implement --output={null,pipe,} Fixes #377. --- Cargo.lock | 22 +++++++++++++ Cargo.toml | 3 ++ src/benchmark/executor.rs | 8 ++--- src/cli.rs | 14 ++++++++ src/options.rs | 37 +++++++++++++++++---- src/timer/mod.rs | 68 +++++++++++++++++++++++++++++++-------- 6 files changed, 129 insertions(+), 23 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 070f642ae..f1c33ab03 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -265,6 +265,7 @@ dependencies = [ "csv", "indicatif", "libc", + "nix", "predicates", "rand 0.8.5", "rust_decimal", @@ -347,6 +348,27 @@ version = "2.4.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "308cc39be01b73d0d18f82a0e7b2a3df85245f84af96fdddc5d202d27e47b86a" +[[package]] +name = "memoffset" +version = "0.6.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5aa361d4faea93603064a027415f07bd8e1d5c88c9fbf68bf56a285428fd79ce" +dependencies = [ + "autocfg 1.1.0", +] + +[[package]] +name = "nix" +version = "0.24.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8f17df307904acd05aa8e32e97bb20f2a0df1728bbc2d771ae8f9a90463441e9" +dependencies = [ + "bitflags", + "cfg-if", + "libc", + "memoffset", +] + [[package]] name = "normalize-line-endings" version = "0.3.0" diff --git a/Cargo.toml b/Cargo.toml index 19f6b9d2e..8596faef8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -31,6 +31,9 @@ libc = "0.2" [target.'cfg(windows)'.dependencies] winapi = { version = "0.3", features = ["processthreadsapi", "minwindef", "winnt"] } +[target.'cfg(target_os="linux")'.dependencies] +nix = { version = "0.24.1", features = ["zerocopy"] } + [dependencies.clap] version = "3" default-features = false diff --git a/src/benchmark/executor.rs b/src/benchmark/executor.rs index 52017c69f..a8f84a9c6 100644 --- a/src/benchmark/executor.rs +++ b/src/benchmark/executor.rs @@ -36,10 +36,10 @@ pub trait Executor { fn run_command_and_measure_common( mut command: std::process::Command, command_failure_action: CmdFailureAction, - command_output_policy: CommandOutputPolicy, + command_output_policy: &CommandOutputPolicy, command_name: &str, ) -> Result { - let (stdout, stderr) = command_output_policy.get_stdout_stderr(); + let (stdout, stderr) = command_output_policy.get_stdout_stderr()?; command.stdin(Stdio::null()).stdout(stdout).stderr(stderr); command.env( @@ -83,7 +83,7 @@ impl<'a> Executor for RawExecutor<'a> { let result = run_command_and_measure_common( command.get_command()?, command_failure_action.unwrap_or(self.options.command_failure_action), - self.options.command_output_policy, + &self.options.command_output_policy, &command.get_command_line(), )?; @@ -136,7 +136,7 @@ impl<'a> Executor for ShellExecutor<'a> { let mut result = run_command_and_measure_common( command_builder, command_failure_action.unwrap_or(self.options.command_failure_action), - self.options.command_output_policy, + &self.options.command_output_policy, &command.get_command_line(), )?; diff --git a/src/cli.rs b/src/cli.rs index 14ff88481..ef3ca756a 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -252,6 +252,20 @@ fn build_command() -> Command<'static> { when trying to benchmark output speed.", ), ) + .arg( + Arg::new("output") + .long("output") + .conflicts_with("show-output") + .takes_value(true) + .value_name("WHERE") + .help( + "Control where the output of the benchmark is redirected.\n\n \ + null: Redirect output to /dev/null (the default).\n\n \ + pipe: Feed the output through a pipe before discarding it.\n \ + Useful if the benchmark special-cases /dev/null.\n\n \ + : Write the output to the given file.", + ), + ) .arg( Arg::new("command-name") .long("command-name") diff --git a/src/options.rs b/src/options.rs index bad0f3721..167a86a88 100644 --- a/src/options.rs +++ b/src/options.rs @@ -1,5 +1,7 @@ +use std::fs::File; +use std::path::PathBuf; use std::process::{Command, Stdio}; -use std::{cmp, fmt}; +use std::{cmp, fmt, io}; use anyhow::ensure; use atty::Stream; @@ -109,11 +111,17 @@ impl Default for RunBounds { } /// How to handle the output of benchmarked commands -#[derive(Debug, Clone, Copy, PartialEq)] +#[derive(Debug, Clone, PartialEq)] pub enum CommandOutputPolicy { /// Discard all output Discard, + /// Feed output through a pipe before discarding it + Pipe, + + /// Redirect output to a file + File(PathBuf), + /// Show command output on the terminal Forward, } @@ -125,11 +133,22 @@ impl Default for CommandOutputPolicy { } impl CommandOutputPolicy { - pub fn get_stdout_stderr(&self) -> (Stdio, Stdio) { - match self { + pub fn get_stdout_stderr(&self) -> io::Result<(Stdio, Stdio)> { + let streams = match self { CommandOutputPolicy::Discard => (Stdio::null(), Stdio::null()), + + // Typically only stdout is performance-relevant, so just pipe that + CommandOutputPolicy::Pipe => (Stdio::piped(), Stdio::null()), + + CommandOutputPolicy::File(path) => { + let file = File::create(&path)?; + (file.into(), Stdio::null()) + } + CommandOutputPolicy::Forward => (Stdio::inherit(), Stdio::inherit()), - } + }; + + Ok(streams) } } @@ -251,6 +270,12 @@ impl Options { options.command_output_policy = if matches.is_present("show-output") { CommandOutputPolicy::Forward + } else if let Some(output) = matches.value_of("output") { + match output { + "null" => CommandOutputPolicy::Discard, + "pipe" => CommandOutputPolicy::Pipe, + path => CommandOutputPolicy::File(path.into()), + } } else { CommandOutputPolicy::Discard }; @@ -262,7 +287,7 @@ impl Options { Some("color") => OutputStyleOption::Color, Some("none") => OutputStyleOption::Disabled, _ => { - if options.command_output_policy == CommandOutputPolicy::Discard + if options.command_output_policy != CommandOutputPolicy::Forward && atty::is(Stream::Stdout) { OutputStyleOption::Full diff --git a/src/timer/mod.rs b/src/timer/mod.rs index 41e3587f2..4a87c8cf3 100644 --- a/src/timer/mod.rs +++ b/src/timer/mod.rs @@ -6,10 +6,20 @@ mod windows_timer; #[cfg(not(windows))] mod unix_timer; +#[cfg(target_os = "linux")] +use nix::fcntl::{splice, SpliceFFlags}; +#[cfg(target_os = "linux")] +use std::fs::File; +#[cfg(target_os = "linux")] +use std::os::unix::io::AsRawFd; + +#[cfg(not(target_os = "linux"))] +use std::io::Read; + use crate::util::units::Second; use wall_clock_timer::WallClockTimer; -use std::process::{Command, ExitStatus}; +use std::process::{ChildStdout, Command, ExitStatus}; use anyhow::Result; @@ -33,26 +43,58 @@ pub struct TimerResult { pub status: ExitStatus, } +/// Discard the output of a child process. +fn discard(output: ChildStdout) { + const CHUNK_SIZE: usize = 64 << 10; + + #[cfg(target_os = "linux")] + { + if let Ok(file) = File::create("/dev/null") { + while let Ok(bytes) = splice( + output.as_raw_fd(), + None, + file.as_raw_fd(), + None, + CHUNK_SIZE, + SpliceFFlags::empty(), + ) { + if bytes == 0 { + break; + } + } + } + } + + #[cfg(not(target_os = "linux"))] + { + let mut output = output; + let mut buf = [0; CHUNK_SIZE]; + while let Ok(bytes) = output.read(&mut buf) { + if bytes == 0 { + break; + } + } + } +} + /// Execute the given command and return a timing summary pub fn execute_and_measure(mut command: Command) -> Result { let wallclock_timer = WallClockTimer::start(); - #[cfg(not(windows))] - let ((time_user, time_system), status) = { - let cpu_timer = self::unix_timer::CPUTimer::start(); - let status = command.status()?; - (cpu_timer.stop(), status) - }; + let mut child = command.spawn()?; + let output = child.stdout.take(); + #[cfg(not(windows))] + let cpu_timer = self::unix_timer::CPUTimer::start(); #[cfg(windows)] - let ((time_user, time_system), status) = { - let mut child = command.spawn()?; - let cpu_timer = self::windows_timer::CPUTimer::start_for_process(&child); - let status = child.wait()?; + let cpu_timer = self::windows_timer::CPUTimer::start_for_process(&child); - (cpu_timer.stop(), status) - }; + if let Some(output) = output { + discard(output); + } + let status = child.wait()?; + let (time_user, time_system) = cpu_timer.stop(); let time_real = wallclock_timer.stop(); Ok(TimerResult {