From 436bff76641a190f190dec6b8486c027ecee1a6e Mon Sep 17 00:00:00 2001 From: David Peter Date: Tue, 22 Feb 2022 08:58:57 +0100 Subject: Proper shell overhead computation --- src/benchmark/executor.rs | 16 ++++++++-------- src/benchmark/mod.rs | 13 +++++++------ src/benchmark/scheduler.rs | 2 +- src/options.rs | 6 +++--- tests/integration_tests.rs | 25 +++++++++++++++++++++---- 5 files changed, 40 insertions(+), 22 deletions(-) diff --git a/src/benchmark/executor.rs b/src/benchmark/executor.rs index 9022cad..11978b4 100644 --- a/src/benchmark/executor.rs +++ b/src/benchmark/executor.rs @@ -165,12 +165,12 @@ impl<'a> Executor for ShellExecutor<'a> { } #[derive(Clone)] -pub struct MockExecutor<'a> { - shell: &'a Shell, +pub struct MockExecutor { + shell: Option, } -impl<'a> MockExecutor<'a> { - pub fn new(shell: &'a Shell) -> Self { +impl MockExecutor { + pub fn new(shell: Option) -> Self { MockExecutor { shell } } @@ -184,7 +184,7 @@ impl<'a> MockExecutor<'a> { } } -impl<'a> Executor for MockExecutor<'a> { +impl Executor for MockExecutor { fn time_command( &self, command: &Command<'_>, @@ -217,9 +217,9 @@ impl<'a> Executor for MockExecutor<'a> { } fn time_overhead(&self) -> Second { - match self.shell { - Shell::Default(_) => 0.0, - Shell::Custom(shell) => Self::extract_time(&shell[0]), + match &self.shell { + None => 0.0, + Some(shell) => Self::extract_time(shell), } } } diff --git a/src/benchmark/mod.rs b/src/benchmark/mod.rs index 59f07fb..d26dc39 100644 --- a/src/benchmark/mod.rs +++ b/src/benchmark/mod.rs @@ -141,11 +141,10 @@ impl<'a> Benchmark<'a> { ) }); let run_preparation_command = || { - if let Some(ref cmd) = preparation_command { - self.run_preparation_command(cmd) - } else { - Ok(TimingResult::default()) - } + preparation_command + .as_ref() + .map(|cmd| self.run_preparation_command(cmd)) + .transpose() }; self.run_setup_command(self.command.get_parameters().iter().cloned())?; @@ -186,6 +185,8 @@ impl<'a> Benchmark<'a> { }; let preparation_result = run_preparation_command()?; + let preparation_overhead = + preparation_result.map_or(0.0, |res| res.time_real + self.executor.time_overhead()); // Initial timing run let (res, status) = self.executor.time_command(self.command, None)?; @@ -193,7 +194,7 @@ impl<'a> Benchmark<'a> { // Determine number of benchmark runs let runs_in_min_time = (self.options.min_benchmarking_time - / (res.time_real + preparation_result.time_real + self.executor.time_overhead())) + / (res.time_real + self.executor.time_overhead() + preparation_overhead)) as u64; let count = { diff --git a/src/benchmark/scheduler.rs b/src/benchmark/scheduler.rs index c4e3603..9b259f8 100644 --- a/src/benchmark/scheduler.rs +++ b/src/benchmark/scheduler.rs @@ -33,7 +33,7 @@ impl<'a> Scheduler<'a> { pub fn run_benchmarks(&mut self) -> Result<()> { let mut executor: Box = match self.options.executor_kind { - ExecutorKind::Mock(ref shell) => Box::new(MockExecutor::new(shell)), + 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/options.rs b/src/options.rs index 7d82e39..08aa38e 100644 --- a/src/options.rs +++ b/src/options.rs @@ -126,7 +126,7 @@ impl Default for CommandOutputPolicy { pub enum ExecutorKind { Shell(Shell), - Mock(Shell), + Mock(Option), } impl Default for ExecutorKind { @@ -276,8 +276,8 @@ impl Options { { (false, Some(shell)) => ExecutorKind::Shell(Shell::from_str(shell)?), (false, None) => ExecutorKind::Shell(Shell::default()), - (true, Some(shell)) => ExecutorKind::Mock(Shell::from_str(shell)?), - (true, None) => ExecutorKind::Mock(Shell::default()), + (true, Some(shell)) => ExecutorKind::Mock(Some(shell.into())), + (true, None) => ExecutorKind::Mock(None), }; if matches.is_present("ignore-failure") { diff --git a/tests/integration_tests.rs b/tests/integration_tests.rs index 42aa787..67ebf25 100644 --- a/tests/integration_tests.rs +++ b/tests/integration_tests.rs @@ -189,7 +189,6 @@ fn returns_mean_time_in_correct_unit() { #[test] fn performs_ten_runs_for_slow_commands() { hyperfine_debug() - .arg("--time-unit=millisecond") .arg("sleep 0.5") .assert() .success() @@ -199,7 +198,6 @@ fn performs_ten_runs_for_slow_commands() { #[test] fn performs_three_seconds_of_benchmarking_for_fast_commands() { hyperfine_debug() - .arg("--time-unit=millisecond") .arg("sleep 0.01") .assert() .success() @@ -207,14 +205,33 @@ fn performs_three_seconds_of_benchmarking_for_fast_commands() { } #[test] -fn takes_time_of_preparation_command_into_account() { +fn takes_shell_spawning_time_into_account_for_computing_number_of_runs() { + hyperfine_debug() + .arg("--shell=sleep 0.02") + .arg("sleep 0.01") + .assert() + .success() + .stdout(predicate::str::contains("100 runs")); +} + +#[test] +fn takes_preparation_command_into_account_for_computing_number_of_runs() { hyperfine_debug() - .arg("--time-unit=millisecond") .arg("--prepare=sleep 0.02") .arg("sleep 0.01") .assert() .success() .stdout(predicate::str::contains("100 runs")); + + // Shell overhead needs to be added to both the prepare command and the actual command, + // leading to a total benchmark time of (prepare + shell + cmd + shell = 0.1 s) + hyperfine_debug() + .arg("--shell=sleep 0.01") + .arg("--prepare=sleep 0.03") + .arg("sleep 0.05") + .assert() + .success() + .stdout(predicate::str::contains("30 runs")); } #[test] -- cgit v1.2.3