From d9418e0731d549e927ecdd0c034d6b808d5b76a4 Mon Sep 17 00:00:00 2001 From: David Peter Date: Tue, 22 Feb 2022 13:31:03 +0100 Subject: [PATCH] Use raw executor by default, skip intermediate shell This adds a new "raw executor" (next to the "shell executor") that allows hyperfine to execute commands directly without any intermediate shell. The command line is split into tokens (using the `shell-words` crate), and according to POSIX rules. The first token is taken as the executable, and the rest as arguments. The new executor is enabled by default. In order to select the shell executor, users will have to pass `--shell=default`. This allows us to reduce measurement noise and to benchmark very quick commands. It also decreases the time to run benchmarks, as we don't need the calibration phase. Also, it allows one to make sure that the executed command is not implemented as a shell builtin. For example `hyperfine true` and `hyperfine --shell=default true` return different times due to the fact that `bash` executes `true` as a NOP. Co-authored: Ciprian Dorin Craciun --- src/benchmark/executor.rs | 42 ++++++++++++++++++++++++++++++++++ src/benchmark/mod.rs | 6 +++-- src/benchmark/scheduler.rs | 3 ++- src/command.rs | 17 +++++++++++++- src/options.rs | 4 +++- tests/common.rs | 6 +++++ tests/execution_order_tests.rs | 4 ++-- tests/integration_tests.rs | 14 +++++++++++- 8 files changed, 88 insertions(+), 8 deletions(-) diff --git a/src/benchmark/executor.rs b/src/benchmark/executor.rs index 39b8cd3..52017c6 100644 --- a/src/benchmark/executor.rs +++ b/src/benchmark/executor.rs @@ -64,6 +64,48 @@ fn run_command_and_measure_common( Ok(result) } +pub struct RawExecutor<'a> { + options: &'a Options, +} + +impl<'a> RawExecutor<'a> { + pub fn new(options: &'a Options) -> Self { + RawExecutor { options } + } +} + +impl<'a> Executor for RawExecutor<'a> { + fn run_command_and_measure( + &self, + command: &Command<'_>, + command_failure_action: Option, + ) -> Result<(TimingResult, ExitStatus)> { + 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, + &command.get_command_line(), + )?; + + Ok(( + TimingResult { + time_real: result.time_real, + time_user: result.time_user, + time_system: result.time_system, + }, + result.status, + )) + } + + fn calibrate(&mut self) -> Result<()> { + Ok(()) + } + + fn time_overhead(&self) -> Second { + 0.0 + } +} + pub struct ShellExecutor<'a> { options: &'a Options, shell: &'a Shell, diff --git a/src/benchmark/mod.rs b/src/benchmark/mod.rs index 410e1c3..6c32546 100644 --- a/src/benchmark/mod.rs +++ b/src/benchmark/mod.rs @@ -7,7 +7,7 @@ pub mod timing_result; use std::cmp; use crate::command::Command; -use crate::options::{CmdFailureAction, Options, OutputStyleOption}; +use crate::options::{CmdFailureAction, ExecutorKind, Options, OutputStyleOption}; use crate::outlier_detection::{modified_zscores, OUTLIER_THRESHOLD}; use crate::output::format::{format_duration, format_duration_unit}; use crate::output::progress_bar::get_progress_bar; @@ -320,7 +320,9 @@ impl<'a> Benchmark<'a> { let mut warnings = vec![]; // Check execution time - if times_real.iter().any(|&t| t < MIN_EXECUTION_TIME) { + if matches!(self.options.executor_kind, ExecutorKind::Shell(_)) + && times_real.iter().any(|&t| t < MIN_EXECUTION_TIME) + { warnings.push(Warnings::FastExecutionTime); } diff --git a/src/benchmark/scheduler.rs b/src/benchmark/scheduler.rs index 89025eb..734efbb 100644 --- a/src/benchmark/scheduler.rs +++ b/src/benchmark/scheduler.rs @@ -1,7 +1,7 @@ use colored::*; use super::benchmark_result::BenchmarkResult; -use super::executor::{Executor, MockExecutor, ShellExecutor}; +use super::executor::{Executor, MockExecutor, RawExecutor, ShellExecutor}; use super::{relative_speed, Benchmark}; use crate::command::Commands; @@ -33,6 +33,7 @@ impl<'a> Scheduler<'a> { pub fn run_benchmarks(&mut self) -> Result<()> { let mut executor: Box = match self.options.executor_kind { + ExecutorKind::Raw => Box::new(RawExecutor::new(self.options)), ExecutorKind::Mock(ref shell) => Box::new(MockExecutor::new(shell.clone())), ExecutorKind::Shell(ref shell) => Box::new(ShellExecutor::new(shell, self.options)), }; diff --git a/src/command.rs b/src/command.rs index f38547a..a3305c2 100644 --- a/src/command.rs +++ b/src/command.rs @@ -15,7 +15,7 @@ use clap::ArgMatches; use crate::parameter::tokenize::tokenize; use crate::parameter::ParameterValue; -use anyhow::{bail, Result}; +use anyhow::{bail, Context, Result}; use clap::Values; use rust_decimal::Decimal; @@ -64,6 +64,21 @@ impl<'a> Command<'a> { self.replace_parameters_in(self.expression) } + pub fn get_command(&self) -> Result { + let command_line = self.get_command_line(); + let mut tokens = shell_words::split(&command_line) + .with_context(|| format!("Failed to parse command '{}'", command_line))? + .into_iter(); + + if let Some(program_name) = tokens.next() { + let mut command_builder = std::process::Command::new(program_name); + command_builder.args(tokens); + Ok(command_builder) + } else { + bail!("Can not execute empty command") + } + } + pub fn get_parameters(&self) -> &[(&'a str, ParameterValue)] { &self.parameters } diff --git a/src/options.rs b/src/options.rs index 1d4c3f7..6a39900 100644 --- a/src/options.rs +++ b/src/options.rs @@ -134,6 +134,7 @@ impl CommandOutputPolicy { } pub enum ExecutorKind { + Raw, Shell(Shell), Mock(Option), } @@ -283,8 +284,9 @@ impl Options { options.executor_kind = match (matches.is_present("debug-mode"), matches.value_of("shell")) { + (false, Some(shell)) if shell == "default" => ExecutorKind::Shell(Shell::default()), (false, Some(shell)) => ExecutorKind::Shell(Shell::parse_from_str(shell)?), - (false, None) => ExecutorKind::Shell(Shell::default()), + (false, None) => ExecutorKind::Raw, (true, Some(shell)) => ExecutorKind::Mock(Some(shell.into())), (true, None) => ExecutorKind::Mock(None), }; diff --git a/tests/common.rs b/tests/common.rs index b740888..6a3d247 100644 --- a/tests/common.rs +++ b/tests/common.rs @@ -11,3 +11,9 @@ pub fn hyperfine_raw_command() -> Command { pub fn hyperfine() -> assert_cmd::Command { assert_cmd::Command::from_std(hyperfine_raw_command()) } + +pub fn hyperfine_shell() -> assert_cmd::Command { + let mut cmd = hyperfine(); + cmd.arg("--shell=default"); + cmd +} diff --git a/tests/execution_order_tests.rs b/tests/execution_order_tests.rs index fbbfe4e..f8c94f4 100644 --- a/tests/execution_order_tests.rs +++ b/tests/execution_order_tests.rs @@ -3,7 +3,7 @@ use std::{fs::File, io::Read, path::PathBuf}; use tempfile::{tempdir, TempDir}; mod common; -use common::hyperfine; +use common::hyperfine_shell; struct ExecutionOrderTest { cmd: assert_cmd::Command, @@ -19,7 +19,7 @@ impl ExecutionOrderTest { let logfile_path = tempdir.path().join("output.log"); ExecutionOrderTest { - cmd: hyperfine(), + cmd: hyperfine_shell(), expected_content: String::new(), logfile_path, tempdir, diff --git a/tests/integration_tests.rs b/tests/integration_tests.rs index e45e7c4..6ea3584 100644 --- a/tests/integration_tests.rs +++ b/tests/integration_tests.rs @@ -1,5 +1,5 @@ mod common; -use common::hyperfine; +use common::{hyperfine, hyperfine_shell}; use predicates::prelude::*; @@ -81,6 +81,18 @@ fn fails_with_duplicate_parameter_names() { #[test] fn fails_for_unknown_command() { hyperfine() + .arg("--runs=1") + .arg("some-nonexisting-program-b5d9574198b7e4b12a71fa4747c0a577") + .assert() + .failure() + .stderr(predicate::str::contains( + "Failed to run command 'some-nonexisting-program-b5d9574198b7e4b12a71fa4747c0a577'", + )); +} + +#[test] +fn fails_for_unknown_shell_command() { + hyperfine_shell() .arg("--runs=1") .arg("some-nonexisting-program-b5d9574198b7e4b12a71fa4747c0a577") .assert()