Unify TimingResult/TimerResult/Measurement

src/benchmark/benchmark_result.rs

@@ -38,7 +38,7 @@ pub struct BenchmarkResult {
 impl BenchmarkResult {
     /// The average wall clock time
     pub fn mean_wall_clock_time(&self) -> Second {
-        self.measurements.mean()
+        self.measurements.time_wall_clock_mean()
     }
 
     /// The full command line of the program that is being benchmarked, possibly including a list of

src/benchmark/executor.rs

@@ -2,7 +2,8 @@
 use std::os::windows::process::CommandExt;
 use std::process::ExitStatus;
 
-use crate::benchmark::quantity::mean;
+use crate::benchmark::measurement::Measurement;
+use crate::benchmark::measurement::Measurements;
 use crate::benchmark::quantity::Byte;
 use crate::benchmark::quantity::Second;
 use crate::command::Command;
@@ -10,12 +11,9 @@ use crate::options::{
     CmdFailureAction, CommandInputPolicy, CommandOutputPolicy, Options, OutputStyleOption, Shell,
 };
 use crate::output::progress_bar::get_progress_bar;
-use crate::quantity::{second, Information, Time, Zero};
 use crate::timer::execute_and_measure;
 use crate::util::randomized_environment_offset;
 
-use super::measurement::{Measurement, Measurements};
-
 use anyhow::{bail, Context, Result};
 
 #[derive(Clone, Copy)]
@@ -82,8 +80,20 @@ fn run_command_and_measure_common(
     let result = execute_and_measure(command)
         .with_context(|| format!("Failed to run command '{command_name}'"))?;
 
-    if !result.exit_status.success() {
-        use crate::util::exit_code::extract_exit_code;
+    if command_failure_action == CmdFailureAction::RaiseError && !result.exit_status.success() {
+        let when = match iteration {
+            BenchmarkIteration::NonBenchmarkRun => "a non-benchmark run".to_string(),
+            BenchmarkIteration::Warmup(0) => "the first warmup run".to_string(),
+            BenchmarkIteration::Warmup(i) => format!("warmup iteration {i}"),
+            BenchmarkIteration::Benchmark(0) => "the first benchmark run".to_string(),
+            BenchmarkIteration::Benchmark(i) => format!("benchmark iteration {i}"),
+        };
+        bail!(
+            "{cause} in {when}. Use the '-i'/'--ignore-failure' option if you want to ignore this. \
+            Alternatively, use the '--show-output' option to debug what went wrong.",
+            cause=result.exit_status.code().map_or(
+                "The process has been terminated by a signal".into(),
+                |c| format!("Command terminated with non-zero exit code {c}")
 
         let should_fail = match command_failure_action {
             CmdFailureAction::RaiseError => true,
@@ -140,7 +150,7 @@ impl Executor for RawExecutor<'_> {
         command_failure_action: Option<CmdFailureAction>,
         output_policy: &CommandOutputPolicy,
     ) -> Result<Measurement> {
-        run_command_and_measure_common(
+        let result = run_command_and_measure_common(
             command.get_command()?,
             iteration,
             command_failure_action.unwrap_or_else(|| self.options.command_failure_action.clone()),
@@ -149,15 +159,13 @@ impl Executor for RawExecutor<'_> {
             &command.get_command_line(),
         )?;
 
-        Ok((
-            TimingResult {
-                time_wall_clock: result.time_wall_clock,
-                time_user: result.time_user,
-                time_system: result.time_system,
-                memory_usage_byte: result.memory_usage_byte,
-            },
-            result.status,
-        ))
+        Ok(Measurement {
+            time_wall_clock: result.time_wall_clock,
+            time_user: result.time_user,
+            time_system: result.time_system,
+            peak_memory_usage: result.peak_memory_usage,
+            exit_status: result.exit_status,
+        })
     }
 
     fn calibrate(&mut self) -> Result<()> {
@@ -223,7 +231,7 @@ impl Executor for ShellExecutor<'_> {
             }
         }
 
-        if let Some(spawning_time) = self.shell_spawning_time {
+        if let Some(ref spawning_time) = self.shell_spawning_time {
             result.time_wall_clock =
                 ensure_non_negative(result.time_wall_clock - spawning_time.time_wall_clock);
             result.time_user = ensure_non_negative(result.time_user - spawning_time.time_user);
@@ -231,15 +239,13 @@ impl Executor for ShellExecutor<'_> {
                 ensure_non_negative(result.time_system - spawning_time.time_system);
         }
 
-        Ok((
-            TimingResult {
-                time_wall_clock: result.time_wall_clock,
-                time_user: result.time_user,
-                time_system: result.time_system,
-                memory_usage_byte: result.memory_usage_byte,
-            },
-            result.status,
-        ))
+        Ok(Measurement {
+            time_wall_clock: result.time_wall_clock,
+            time_user: result.time_user,
+            time_system: result.time_system,
+            peak_memory_usage: result.peak_memory_usage,
+            exit_status: result.exit_status,
+        })
     }
 
     /// Measure the average shell spawning time
@@ -255,20 +261,18 @@ impl Executor for ShellExecutor<'_> {
             None
         };
 
-        let mut times_wall_clock: Vec<Second> = vec![]; // TODO
-        let mut times_user: Vec<Second> = vec![];
-        let mut times_system: Vec<Second> = vec![];
+        let mut measurements = Measurements::default();
 
         for _ in 0..COUNT {
             // Just run the shell without any command
-            let res = self.run_command_and_measure(
+            let measurement = self.run_command_and_measure(
                 &Command::new(None, ""),
                 BenchmarkIteration::NonBenchmarkRun,
                 None,
                 &CommandOutputPolicy::Null,
             );
 
-            match res {
+            match measurement {
                 Err(_) => {
                     let shell_cmd = if cfg!(windows) {
                         format!("{} /C \"\"", self.shell)
@@ -281,10 +285,8 @@ impl Executor for ShellExecutor<'_> {
                         shell_cmd
                     );
                 }
-                Ok((r, _)) => {
-                    times_wall_clock.push(r.time_wall_clock);
-                    times_user.push(r.time_user);
-                    times_system.push(r.time_system);
+                Ok(result) => {
+                    measurements.push(result);
                 }
             }
 
@@ -297,18 +299,19 @@ impl Executor for ShellExecutor<'_> {
             bar.finish_and_clear()
         }
 
-        self.shell_spawning_time = Some(TimingResult {
-            time_wall_clock: mean(&times_wall_clock),
-            time_user: mean(&times_user),
-            time_system: mean(&times_system),
-            memory_usage_byte: Byte::new(0),
+        self.shell_spawning_time = Some(Measurement {
+            time_wall_clock: measurements.time_wall_clock_mean(),
+            time_user: measurements.time_user_mean(),
+            time_system: measurements.time_system_mean(),
+            peak_memory_usage: measurements.peak_memory_usage_mean(),
+            exit_status: ExitStatus::default(),
         });
 
         Ok(())
     }
 
     fn time_overhead(&self) -> Second {
-        self.shell_spawning_time.unwrap().time_wall_clock
+        self.shell_spawning_time.as_ref().unwrap().time_wall_clock
     }
 }
 
@@ -354,15 +357,13 @@ impl Executor for MockExecutor {
             ExitStatus::from_raw(0)
         };
 
-        Ok((
-            TimingResult {
-                time_wall_clock: Self::extract_time(command.get_command_line()),
-                time_user: Second::zero(),
-                time_system: Second::zero(),
-                memory_usage_byte: Byte::new(0),
-            },
-            status,
-        ))
+        Ok(Measurement {
+            time_wall_clock: Self::extract_time(command.get_command_line()),
+            time_user: Second::zero(),
+            time_system: Second::zero(),
+            peak_memory_usage: Byte::new(0),
+            exit_status,
+        })
     }
 
     fn calibrate(&mut self) -> Result<()> {

src/benchmark/measurement.rs

@@ -1,25 +1,39 @@
+use std::process::ExitStatus;
+
 use serde::Serialize;
 
 use crate::benchmark::quantity::{max, mean, median, min, standard_deviation, Byte, Second};
 use crate::outlier_detection::modified_zscores;
+use crate::util::exit_code::extract_exit_code;
+
+fn serialize_exit_status<S>(exit_status: &ExitStatus, serializer: S) -> Result<S::Ok, S::Error>
+where
+    S: serde::Serializer,
+{
+    match extract_exit_code(*exit_status) {
+        Some(code) => serializer.serialize_i32(code),
+        None => serializer.serialize_unit(),
+    }
+}
 
 /// Performance metric measurements and exit code for a single run
-#[derive(Debug, Clone, Serialize, PartialEq)]
+#[derive(Debug, Clone, Default, Serialize, PartialEq)]
 pub struct Measurement {
     /// Elapsed wall clock time (real time)
-    pub wall_clock_time: Second,
+    pub time_wall_clock: Second,
 
     /// Time spent in user mode
-    pub user_time: Second,
+    pub time_user: Second,
 
     /// Time spent in kernel mode
-    pub system_time: Second,
+    pub time_system: Second,
 
     /// Maximum memory usage of the process
-    pub memory_usage_byte: Byte,
+    pub peak_memory_usage: Byte,
 
-    /// Exit codes of the process
-    pub exit_code: Option<i32>,
+    // The exit status of the process
+    #[serde(serialize_with = "serialize_exit_status")]
+    pub exit_status: ExitStatus,
 }
 
 #[derive(Debug, Default, Clone, Serialize, PartialEq)]
@@ -47,12 +61,12 @@ impl Measurements {
     pub fn wall_clock_times(&self) -> Vec<Second> {
         self.measurements
             .iter()
-            .map(|m| m.wall_clock_time)
+            .map(|m| m.time_wall_clock)
             .collect()
     }
 
     /// The average wall clock time
-    pub fn mean(&self) -> Second {
+    pub fn time_wall_clock_mean(&self) -> Second {
         mean(&self.wall_clock_times())
     }
 
@@ -83,27 +97,35 @@ impl Measurements {
     }
 
     /// The average user time
-    pub fn user_mean(&self) -> Second {
+    pub fn time_user_mean(&self) -> Second {
         mean(
             &self
                 .measurements
                 .iter()
-                .map(|m| m.user_time)
+                .map(|m| m.time_user)
                 .collect::<Vec<_>>(),
         )
     }
 
     /// The average system time
-    pub fn system_mean(&self) -> Second {
+    pub fn time_system_mean(&self) -> Second {
         mean(
             &self
                 .measurements
                 .iter()
-                .map(|m| m.system_time)
+                .map(|m| m.time_system)
                 .collect::<Vec<_>>(),
         )
     }
 
+    pub fn peak_memory_usage_mean(&self) -> Byte {
+        self.measurements
+            .iter()
+            .map(|m| m.peak_memory_usage)
+            .max_by(|a, b| a.partial_cmp(b).unwrap()) // TODO
+            .unwrap() // TODO
+    }
+
     pub fn modified_zscores(&self) -> Vec<f64> {
         modified_zscores(
             &self

src/benchmark/mod.rs

@@ -25,10 +25,7 @@ use crate::output::progress_bar::{
 };
 use crate::output::warnings::{OutlierWarningOptions, Warnings};
 use crate::parameter::ParameterNameAndValue;
-use crate::quantity::{byte, const_time_from_seconds, ratio, second, FormatQuantity, Time, Zero};
-use crate::util::exit_code::extract_exit_code;
 use benchmark_result::BenchmarkResult;
-use measurement::{Measurement, Measurements};
 
 use anyhow::{anyhow, Result};
 use colored::*;
@@ -273,20 +270,20 @@ impl<'a> Benchmark<'a> {
         if let Some(bar) = progress_bar.as_ref() {
             start_initial_measurement(bar, Instant::now());
         }
-        let res = self.executor.run_command_and_measure(
+        let result = self.executor.run_command_and_measure(
             self.command,
             benchmark_iteration,
             None,
             output_policy,
         )?;
-        let success = res.exit_status.success();
+        let success = result.exit_status.success();
 
         if let Some(bar) = progress_bar.as_ref() {
             let time_unit = self
                 .options
                 .time_unit
-                .unwrap_or(res.time_wall_clock.suitable_unit());
-            let estimate = res.time_wall_clock.format(time_unit);
+                .unwrap_or(result.time_wall_clock.suitable_unit());
+            let estimate = result.time_wall_clock.format(time_unit);
             finish_initial_measurement(
                 bar,
                 format!("Current estimate: {}", estimate.to_string().green()),
@@ -300,7 +297,7 @@ impl<'a> Benchmark<'a> {
 
         // Determine number of benchmark runs
         let runs_in_min_time = (self.options.min_benchmarking_time
-            / (res.time_wall_clock
+            / (result.time_wall_clock
                 + self.executor.time_overhead()
                 + preparation_overhead
                 + conclusion_overhead))
@@ -321,11 +318,11 @@ impl<'a> Benchmark<'a> {
 
         // Save the first result
         measurements.push(Measurement {
-            wall_clock_time: res.time_wall_clock,
-            user_time: res.time_user,
-            system_time: res.time_system,
-            memory_usage_byte: res.memory_usage_byte,
-            exit_code: extract_exit_code(status),
+            time_wall_clock: result.time_wall_clock,
+            time_user: result.time_user,
+            time_system: result.time_system,
+            peak_memory_usage: result.peak_memory_usage,
+            exit_status: result.exit_status,
         });
 
         all_succeeded = all_succeeded && success;
@@ -341,7 +338,8 @@ impl<'a> Benchmark<'a> {
             let benchmark_iteration = BenchmarkIteration::Benchmark(i + 1);
 
             let msg = {
-                let mean = format_duration(measurements.mean(), self.options.time_unit);
+                let mean =
+                    format_duration(measurements.time_wall_clock_mean(), self.options.time_unit);
                 format!("Current estimate: {}", mean.to_string().green())
             };
 
@@ -351,20 +349,20 @@ impl<'a> Benchmark<'a> {
 
             run_preparation_command(benchmark_iteration)?;
 
-            let res = self.executor.run_command_and_measure(
+            let result = self.executor.run_command_and_measure(
                 self.command,
                 benchmark_iteration,
                 None,
                 output_policy,
             )?;
-            let success = res.exit_status.success();
+            let success = result.exit_status.success();
 
             measurements.push(Measurement {
-                wall_clock_time: res.time_wall_clock,
-                user_time: res.time_user,
-                system_time: res.time_system,
-                memory_usage_byte: res.memory_usage_byte,
-                exit_code: extract_exit_code(status),
+                time_wall_clock: result.time_wall_clock,
+                time_user: result.time_user,
+                time_system: result.time_system,
+                peak_memory_usage: result.peak_memory_usage,
+                exit_status: result.exit_status,
             });
 
             all_succeeded = all_succeeded && success;
@@ -382,13 +380,13 @@ impl<'a> Benchmark<'a> {
 
         // Formatting and console output
         let (mean_str, time_unit) =
-            format_duration_unit(measurements.mean(), self.options.time_unit);
+            format_duration_unit(measurements.time_wall_clock_mean(), self.options.time_unit);
         let min_str = format_duration(measurements.min(), Some(time_unit));
         let max_str = format_duration(measurements.max(), Some(time_unit));
         let num_str = format!("{num_runs} runs", num_runs = measurements.len());
 
-        let user_str = format_duration(measurements.user_mean(), Some(time_unit));
-        let system_str = format_duration(measurements.system_mean(), Some(time_unit));
+        let user_str = format_duration(measurements.time_user_mean(), Some(time_unit));
+        let system_str = format_duration(measurements.time_system_mean(), Some(time_unit));
 
         if self.options.output_style != OutputStyleOption::Disabled {
             if measurements.len() == 1 {

src/benchmark/relative_speed.rs

@@ -148,7 +148,7 @@ pub fn compute<'a>(
 
 #[cfg(test)]
 fn create_result(name: &str, mean: f64) -> BenchmarkResult {
-    use std::collections::BTreeMap;
+    use std::{collections::BTreeMap, process::ExitStatus};
 
     use crate::benchmark::{
         measurement::{Measurement, Measurements},
@@ -159,11 +159,11 @@ fn create_result(name: &str, mean: f64) -> BenchmarkResult {
         command: name.into(),
         measurements: Measurements {
             measurements: vec![Measurement {
-                wall_clock_time: Second::new(mean),
-                user_time: Second::new(mean),
-                system_time: Second::zero(),
-                memory_usage_byte: Byte::new(1024),
-                exit_code: Some(0),
+                time_wall_clock: Second::new(mean),
+                time_user: Second::new(mean),
+                time_system: Second::zero(),
+                peak_memory_usage: Byte::new(1024),
+                exit_status: ExitStatus::default(),
             }],
         },
         parameters: BTreeMap::new(),