From 5b5daa0493c539168e8b916089e13c52f0d5e8c1 Mon Sep 17 00:00:00 2001 From: David Peter Date: Sun, 20 Nov 2022 21:18:03 +0100 Subject: [PATCH] Improve hints for outlier warnings Improve hints for outlier warnings if `--warmup` or `--prepare` are in use already. Closes #570 --- CHANGELOG.md | 2 ++ src/benchmark/mod.rs | 21 ++++++++++++++++--- src/output/warnings.rs | 47 ++++++++++++++++++++++++++++++++---------- 3 files changed, 56 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6b89ff943..2f8541dc9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ ## Changes +- Improve hints for outlier warnings if `--warmup` or `--prepare` are in use already, + see #570 (@sharkdp) ## Bugfixes diff --git a/src/benchmark/mod.rs b/src/benchmark/mod.rs index c7978b8bd..00402a379 100644 --- a/src/benchmark/mod.rs +++ b/src/benchmark/mod.rs @@ -11,7 +11,7 @@ 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; -use crate::output::warnings::Warnings; +use crate::output::warnings::{OutlierWarningOptions, Warnings}; use crate::parameter::ParameterNameAndValue; use crate::util::exit_code::extract_exit_code; use crate::util::min_max::{max, min}; @@ -333,10 +333,25 @@ impl<'a> Benchmark<'a> { // Run outlier detection let scores = modified_zscores(×_real); + + let outlier_warning_options = OutlierWarningOptions { + warmup_in_use: self.options.warmup_count > 0, + prepare_in_use: self + .options + .preparation_command + .as_ref() + .map(|v| v.len()) + .unwrap_or(0) + > 0, + }; + if scores[0] > OUTLIER_THRESHOLD { - warnings.push(Warnings::SlowInitialRun(times_real[0])); + warnings.push(Warnings::SlowInitialRun( + times_real[0], + outlier_warning_options, + )); } else if scores.iter().any(|&s| s.abs() > OUTLIER_THRESHOLD) { - warnings.push(Warnings::OutliersDetected); + warnings.push(Warnings::OutliersDetected(outlier_warning_options)); } if !warnings.is_empty() { diff --git a/src/output/warnings.rs b/src/output/warnings.rs index 9fd326463..abf8d1d52 100644 --- a/src/output/warnings.rs +++ b/src/output/warnings.rs @@ -4,12 +4,17 @@ use crate::benchmark::MIN_EXECUTION_TIME; use crate::output::format::format_duration; use crate::util::units::Second; +pub struct OutlierWarningOptions { + pub warmup_in_use: bool, + pub prepare_in_use: bool, +} + /// A list of all possible warnings pub enum Warnings { FastExecutionTime, NonZeroExitCode, - SlowInitialRun(Second), - OutliersDetected, + SlowInitialRun(Second, OutlierWarningOptions), + OutliersDetected(OutlierWarningOptions), } impl fmt::Display for Warnings { @@ -24,20 +29,40 @@ impl fmt::Display for Warnings { MIN_EXECUTION_TIME * 1e3 ), Warnings::NonZeroExitCode => write!(f, "Ignoring non-zero exit code."), - Warnings::SlowInitialRun(t_first) => write!( + Warnings::SlowInitialRun(time_first_run, ref options) => write!( f, "The first benchmarking run for this command was significantly slower than the \ - rest ({}). This could be caused by (filesystem) caches that were not filled until \ - after the first run. You should consider using the '--warmup' option to fill \ - those caches before the actual benchmark. Alternatively, use the '--prepare' \ - option to clear the caches before each timing run.", - format_duration(t_first, None) + rest ({time}). This could be caused by (filesystem) caches that were not filled until \ + after the first run. {hints}", + time=format_duration(time_first_run, None), + hints=match (options.warmup_in_use, options.prepare_in_use) { + (true, true) => "You are already using both the '--warmup' option as well \ + as the '--prepare' option. Consider re-running the benchmark on a quiet system. \ + Maybe it was a random outlier. Alternatively, consider increasing the warmup \ + count.", + (true, false) => "You are already using the '--warmup' option which helps \ + to fill these caches before the actual benchmark. You can either try to \ + increase the warmup count further or re-run this benchmark on a quiet system \ + in case it was a random outlier. Alternatively, consider using the '--prepare' \ + option to clear the caches before each timing run.", + (false, true) => "You are already using the '--prepare' option which can \ + be used to clear caches. If you did not use a cache-clearing command with \ + '--prepare', you can either try that or consider using the '--warmup' option \ + to fill those caches before the actual benchmark.", + (false, false) => "You should consider using the '--warmup' option to fill \ + those caches before the actual benchmark. Alternatively, use the '--prepare' \ + option to clear the caches before each timing run." + } ), - Warnings::OutliersDetected => write!( + Warnings::OutliersDetected(ref options) => write!( f, "Statistical outliers were detected. Consider re-running this benchmark on a quiet \ - PC without any interferences from other programs. It might help to use the \ - '--warmup' or '--prepare' options." + system without any interferences from other programs.{hint}", + hint=if options.warmup_in_use && options.prepare_in_use { + "" + } else { + " It might help to use the '--warmup' or '--prepare' options." + } ), } }