From f832727efee5a5a6b96a0b95e8c9b838ee77ae17 Mon Sep 17 00:00:00 2001 From: Tavian Dev Date: Thu, 2 Apr 2026 19:41:57 +0000 Subject: [PATCH 1/3] Add --filter-failed to exclude failed results from speed comparison When using --ignore-failure with parameter scans, some parameter combinations may fail (non-zero exit code) and produce misleadingly fast times that skew the relative speed comparison. This adds a --filter-failed flag that excludes results with non-zero exit codes from the relative speed comparison in the summary output. Closes #591 --- CHANGELOG.md | 1 + src/benchmark/benchmark_result.rs | 43 +++++++++++++++++++++++++++++++ src/benchmark/relative_speed.rs | 30 ++++++++++++++++++++- src/benchmark/scheduler.rs | 18 ++++++++++--- src/cli.rs | 8 ++++++ src/options.rs | 6 +++++ 6 files changed, 102 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fda110bee..ba23e15f4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Features +- Add `--filter-failed` option to exclude results with non-zero exit codes from the relative speed comparison, see #591 (@tavian-dev) - Add `--reference-name` option to give a meaningful name to the reference command, see #808 (@niklasdewally) - The `--ignore-failure` option now supports a comma-separated list of exit codes to ignore (e.g., `--ignore-failure=1,2`), see #836 (@sharkdp) - Python scripts: Add `--time-unit` option to `advanced_statistics.py` (@sharkdp) diff --git a/src/benchmark/benchmark_result.rs b/src/benchmark/benchmark_result.rs index 287c73bef..9813aa3b5 100644 --- a/src/benchmark/benchmark_result.rs +++ b/src/benchmark/benchmark_result.rs @@ -53,3 +53,46 @@ pub struct BenchmarkResult { #[serde(skip_serializing_if = "BTreeMap::is_empty")] pub parameters: BTreeMap, } + +impl BenchmarkResult { + /// Returns true if any run had a non-zero or missing exit code. + pub fn has_failure(&self) -> bool { + self.exit_codes.iter().any(|code| *code != Some(0)) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn result_with_exit_codes(codes: Vec>) -> BenchmarkResult { + BenchmarkResult { + exit_codes: codes, + ..Default::default() + } + } + + #[test] + fn test_has_failure_all_success() { + let r = result_with_exit_codes(vec![Some(0), Some(0), Some(0)]); + assert!(!r.has_failure()); + } + + #[test] + fn test_has_failure_with_nonzero() { + let r = result_with_exit_codes(vec![Some(0), Some(1), Some(0)]); + assert!(r.has_failure()); + } + + #[test] + fn test_has_failure_with_signal_kill() { + let r = result_with_exit_codes(vec![Some(0), None]); + assert!(r.has_failure()); + } + + #[test] + fn test_has_failure_empty() { + let r = result_with_exit_codes(vec![]); + assert!(!r.has_failure()); + } +} diff --git a/src/benchmark/relative_speed.rs b/src/benchmark/relative_speed.rs index 90918e527..b92743746 100644 --- a/src/benchmark/relative_speed.rs +++ b/src/benchmark/relative_speed.rs @@ -120,6 +120,11 @@ pub fn compute( #[cfg(test)] fn create_result(name: &str, mean: Scalar) -> BenchmarkResult { + create_result_with_exit_codes(name, mean, vec![Some(0)]) +} + +#[cfg(test)] +fn create_result_with_exit_codes(name: &str, mean: Scalar, exit_codes: Vec>) -> BenchmarkResult { use std::collections::BTreeMap; BenchmarkResult { @@ -134,7 +139,7 @@ fn create_result(name: &str, mean: Scalar) -> BenchmarkResult { max: mean, times: None, memory_usage_byte: None, - exit_codes: Vec::new(), + exit_codes, parameters: BTreeMap::new(), } } @@ -178,3 +183,26 @@ fn test_compute_relative_speed_for_zero_times() { assert!(annotated_results.is_none()); } + +#[test] +fn test_has_failure_excludes_from_comparison() { + use approx::assert_relative_eq; + + let results = vec![ + create_result("cmd1", 3.0), + create_result_with_exit_codes("cmd_fail", 0.5, vec![Some(1)]), + create_result("cmd3", 5.0), + ]; + + // Without filtering, the failed command (0.5s) would be "fastest" + let all_results = compute_with_check(&results, SortOrder::Command).unwrap(); + assert_eq!(all_results.len(), 3); + assert!(all_results[1].is_reference); // cmd_fail is fastest + + // After filtering out failed results, comparison should work on successes only + let filtered: Vec<_> = results.into_iter().filter(|r| !r.has_failure()).collect(); + let filtered_results = compute_with_check(&filtered, SortOrder::Command).unwrap(); + assert_eq!(filtered_results.len(), 2); + assert!(filtered_results[0].is_reference); // cmd1 is now fastest + assert_relative_eq!(1.0, filtered_results[0].relative_speed); +} diff --git a/src/benchmark/scheduler.rs b/src/benchmark/scheduler.rs index 242dd0e7d..a0a6445a5 100644 --- a/src/benchmark/scheduler.rs +++ b/src/benchmark/scheduler.rs @@ -67,15 +67,27 @@ impl<'a> Scheduler<'a> { return; } + let results: Vec<_> = if self.options.filter_failed { + self.results.iter().filter(|r| !r.has_failure()).collect() + } else { + self.results.iter().collect() + }; + + if results.len() < 2 { + return; + } + + let results_slice: Vec<_> = results.iter().map(|r| (*r).clone()).collect(); + let reference = self .options .reference_command .as_ref() - .map(|_| &self.results[0]) - .unwrap_or_else(|| relative_speed::fastest_of(&self.results)); + .map(|_| &results_slice[0]) + .unwrap_or_else(|| relative_speed::fastest_of(&results_slice)); if let Some(annotated_results) = relative_speed::compute_with_check_from_reference( - &self.results, + &results_slice, reference, self.options.sort_order_speed_comparison, ) { diff --git a/src/cli.rs b/src/cli.rs index b12f6d34c..8b03476f3 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -236,6 +236,14 @@ fn build_command() -> Command { 'all-non-zero', all non-zero exit codes are ignored. You can also provide \ a comma-separated list of exit codes to ignore (e.g., --ignore-failure=1,2)."), ) + .arg( + Arg::new("filter-failed") + .long("filter-failed") + .action(ArgAction::SetTrue) + .help("Exclude results with non-zero exit codes from the relative speed \ + comparison and from exported results. This is useful when using \ + '--ignore-failure' with parameter scans where some combinations fail."), + ) .arg( Arg::new("style") .long("style") diff --git a/src/options.rs b/src/options.rs index 7c83da1c5..d2ae1c988 100644 --- a/src/options.rs +++ b/src/options.rs @@ -246,6 +246,9 @@ pub struct Options { /// Which time unit to use when displaying results pub time_unit: Option, + + /// Whether to exclude results with non-zero exit codes from comparisons + pub filter_failed: bool, } impl Default for Options { @@ -267,6 +270,7 @@ impl Default for Options { executor_kind: ExecutorKind::default(), command_output_policies: vec![CommandOutputPolicy::Null], time_unit: None, + filter_failed: false, command_input_policy: CommandInputPolicy::Null, } } @@ -435,6 +439,8 @@ impl Options { }; } + options.filter_failed = matches.get_flag("filter-failed"); + options.time_unit = match matches.get_one::("time-unit").map(|s| s.as_str()) { Some("microsecond") => Some(Unit::MicroSecond), Some("millisecond") => Some(Unit::MilliSecond), From a40b1f686e0ce6c8d152a99c424c592ddc0c4188 Mon Sep 17 00:00:00 2001 From: Tavian Dev Date: Thu, 2 Apr 2026 20:03:52 +0000 Subject: [PATCH 2/3] Fix: handle reference command being filtered out by --filter-failed When --filter-failed is used with --reference, the reference command might itself have failures and get filtered. Previously this would use the wrong command as reference. Now falls back to fastest_of the remaining results. --- src/benchmark/scheduler.rs | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/benchmark/scheduler.rs b/src/benchmark/scheduler.rs index a0a6445a5..0e516517d 100644 --- a/src/benchmark/scheduler.rs +++ b/src/benchmark/scheduler.rs @@ -79,12 +79,18 @@ impl<'a> Scheduler<'a> { let results_slice: Vec<_> = results.iter().map(|r| (*r).clone()).collect(); - let reference = self - .options - .reference_command - .as_ref() - .map(|_| &results_slice[0]) - .unwrap_or_else(|| relative_speed::fastest_of(&results_slice)); + let reference = if self.options.reference_command.is_some() { + // When a reference command is set, it's always the first result. + // If it was filtered out, fall back to the fastest remaining result. + let ref_cmd = &self.results[0]; + if self.options.filter_failed && ref_cmd.has_failure() { + relative_speed::fastest_of(&results_slice) + } else { + &results_slice[0] + } + } else { + relative_speed::fastest_of(&results_slice) + }; if let Some(annotated_results) = relative_speed::compute_with_check_from_reference( &results_slice, From 2c38c1f76763dd1715673917da356453ee96da98 Mon Sep 17 00:00:00 2001 From: Tavian Dev Date: Thu, 2 Apr 2026 20:32:08 +0000 Subject: [PATCH 3/3] Fix rustfmt: break long function signature into multiple lines --- src/benchmark/relative_speed.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/benchmark/relative_speed.rs b/src/benchmark/relative_speed.rs index b92743746..ebbdb8ee2 100644 --- a/src/benchmark/relative_speed.rs +++ b/src/benchmark/relative_speed.rs @@ -124,7 +124,11 @@ fn create_result(name: &str, mean: Scalar) -> BenchmarkResult { } #[cfg(test)] -fn create_result_with_exit_codes(name: &str, mean: Scalar, exit_codes: Vec>) -> BenchmarkResult { +fn create_result_with_exit_codes( + name: &str, + mean: Scalar, + exit_codes: Vec>, +) -> BenchmarkResult { use std::collections::BTreeMap; BenchmarkResult {