Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement --output={null,pipe,<FILE>} #509

Merged
merged 5 commits into from
May 17, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions src/benchmark/executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<TimerResult> {
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(
Expand Down Expand Up @@ -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(),
)?;

Expand Down Expand Up @@ -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(),
)?;

Expand Down
17 changes: 17 additions & 0 deletions src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,23 @@ 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. <WHERE> can be:\n\n\
null: Redirect output to /dev/null (the default). \
Note that some programs like 'grep' detect when standard output is /dev/null \
and apply certain optimizations. To avoid that, consider using \
'--output=pipe'.\n\n\
pipe: Feed the output through a pipe before discarding it.\n\n\
inherit: Don't redirect the output at all (same as '--show-output').\n\n\
<FILE>: Write the output to the given file.",
),
)
.arg(
Arg::new("command-name")
.long("command-name")
Expand Down
2 changes: 2 additions & 0 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,6 @@ pub enum OptionsError<'a> {
EmptyShell,
#[error("Failed to parse '--shell <command>' expression as command line: {0}")]
ShellParseError(shell_words::ParseError),
#[error("Unknown output policy '{0}'. Use ./{0} to output to a file named {0}.")]
sharkdp marked this conversation as resolved.
Show resolved Hide resolved
UnknownOutputPolicy(String),
}
44 changes: 38 additions & 6 deletions src/options.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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,
}
Expand All @@ -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)
}
}

Expand Down Expand Up @@ -251,6 +270,19 @@ 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,
"inherit" => CommandOutputPolicy::Forward,
tavianator marked this conversation as resolved.
Show resolved Hide resolved
arg => {
let path = PathBuf::from(arg);
if path.components().count() <= 1 {
return Err(OptionsError::UnknownOutputPolicy(arg.to_string()));
}
CommandOutputPolicy::File(path)
}
}
} else {
CommandOutputPolicy::Discard
};
Expand All @@ -262,7 +294,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
tavianator marked this conversation as resolved.
Show resolved Hide resolved
&& atty::is(Stream::Stdout)
{
OutputStyleOption::Full
Expand Down
68 changes: 55 additions & 13 deletions src/timer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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;
tavianator marked this conversation as resolved.
Show resolved Hide resolved

#[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;
}
}
}
tavianator marked this conversation as resolved.
Show resolved Hide resolved
}

#[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<TimerResult> {
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()?;
tavianator marked this conversation as resolved.
Show resolved Hide resolved
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 {
sharkdp marked this conversation as resolved.
Show resolved Hide resolved
discard(output);
}
let status = child.wait()?;

let (time_user, time_system) = cpu_timer.stop();
let time_real = wallclock_timer.stop();

Ok(TimerResult {
Expand Down