Add --filter-failed to exclude failed results from speed comparison

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)

src/benchmark/benchmark_result.rs

@@ -53,3 +53,46 @@ pub struct BenchmarkResult {
     #[serde(skip_serializing_if = "BTreeMap::is_empty")]
     pub parameters: BTreeMap<String, String>,
 }
+
+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<Option<i32>>) -> 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());
+    }
+}

src/benchmark/relative_speed.rs

@@ -120,6 +120,15 @@ 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<Option<i32>>,
+) -> BenchmarkResult {
     use std::collections::BTreeMap;
 
     BenchmarkResult {
@@ -134,7 +143,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 +187,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);
+}

src/benchmark/scheduler.rs

@@ -67,15 +67,33 @@ impl<'a> Scheduler<'a> {
             return;
         }
 
-        let reference = self
-            .options
-            .reference_command
-            .as_ref()
-            .map(|_| &self.results[0])
-            .unwrap_or_else(|| relative_speed::fastest_of(&self.results));
+        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 = 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(
-            &self.results,
+            &results_slice,
             reference,
             self.options.sort_order_speed_comparison,
         ) {

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")

src/options.rs

@@ -246,6 +246,9 @@ pub struct Options {
 
     /// Which time unit to use when displaying results
     pub time_unit: Option<Unit>,
+
+    /// 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::<String>("time-unit").map(|s| s.as_str()) {
             Some("microsecond") => Some(Unit::MicroSecond),
             Some("millisecond") => Some(Unit::MilliSecond),