Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement slowness checking and --reference flag to use a command as a reference when printing or exporting summary #579

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 34 additions & 19 deletions src/benchmark/relative_speed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,46 +8,61 @@ pub struct BenchmarkResultWithRelativeSpeed<'a> {
pub result: &'a BenchmarkResult,
pub relative_speed: Scalar,
pub relative_speed_stddev: Option<Scalar>,
pub is_fastest: bool,
pub is_reference: bool,
pub is_faster: bool,
Comment on lines +11 to +12
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe rename the latter to

Suggested change
pub is_reference: bool,
pub is_faster: bool,
pub is_reference: bool,
pub is_faster_than_reference: bool,

Thinking about this some more, it's actually not clear what the value of the latter bool should be for the case where is_reference is true. So maybe introduce an enum instead pub enum RelativeOrdering { Reference, FasterThanReference, SlowerThanReference } and use this instead of the two bool ("make illegal states unrepresentable"):

Suggested change
pub is_reference: bool,
pub is_faster: bool,
pub relative_ordering: RelativeOrdering,

What do you think?

}

pub fn compare_mean_time(l: &BenchmarkResult, r: &BenchmarkResult) -> Ordering {
l.mean.partial_cmp(&r.mean).unwrap_or(Ordering::Equal)
}

pub fn compute(results: &[BenchmarkResult]) -> Option<Vec<BenchmarkResultWithRelativeSpeed>> {
let fastest: &BenchmarkResult = results
pub fn fastest(results: &[BenchmarkResult]) -> &BenchmarkResult {
results
.iter()
.min_by(|&l, &r| compare_mean_time(l, r))
.expect("at least one benchmark result");
.expect("at least one benchmark result")
}

if fastest.mean == 0.0 {
pub fn compute<'a>(
reference: &'a BenchmarkResult,
results: &'a [BenchmarkResult],
) -> Option<Vec<BenchmarkResultWithRelativeSpeed<'a>>> {
if reference.mean == 0.0 {
return None;
}

Some(
results
.iter()
.map(|result| {
let ratio = result.mean / fastest.mean;
let is_reference = result == reference;
let is_faster = result.mean >= reference.mean;

// https://en.wikipedia.org/wiki/Propagation_of_uncertainty#Example_formulas
// Covariance asssumed to be 0, i.e. variables are assumed to be independent
let ratio_stddev = match (result.stddev, fastest.stddev) {
(Some(result_stddev), Some(fastest_stddev)) => Some(
ratio
* ((result_stddev / result.mean).powi(2)
+ (fastest_stddev / fastest.mean).powi(2))
.sqrt(),
),
_ => None,
let ratio = if is_faster {
result.mean / reference.mean
} else {
reference.mean / result.mean
};

// https://en.wikipedia.org/wiki/Propagation_of_uncertainty#Example_formulae
// Covariance asssumed to be 0, i.e. variables are assumed to be independent
let ratio_stddev =
(result.stddev)
.zip(reference.stddev)
.map(|(result_stddev, fastest_stddev)| {
let (a, b) = (
(result_stddev / result.mean),
(fastest_stddev / reference.mean),
);
ratio * (a.powi(2) + b.powi(2)).sqrt()
});

BenchmarkResultWithRelativeSpeed {
result,
relative_speed: ratio,
relative_speed_stddev: ratio_stddev,
is_fastest: result == fastest,
is_reference,
is_faster,
}
})
.collect(),
Expand Down Expand Up @@ -83,7 +98,7 @@ fn test_compute_relative_speed() {
create_result("cmd3", 5.0),
];

let annotated_results = compute(&results).unwrap();
let annotated_results = compute(fastest(&results), &results).unwrap();

assert_relative_eq!(1.5, annotated_results[0].relative_speed);
assert_relative_eq!(1.0, annotated_results[1].relative_speed);
Expand All @@ -94,7 +109,7 @@ fn test_compute_relative_speed() {
fn test_compute_relative_speed_for_zero_times() {
let results = vec![create_result("cmd1", 1.0), create_result("cmd2", 0.0)];

let annotated_results = compute(&results);
let annotated_results = compute(fastest(&results), &results);

assert!(annotated_results.is_none());
}
31 changes: 24 additions & 7 deletions src/benchmark/scheduler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use super::benchmark_result::BenchmarkResult;
use super::executor::{Executor, MockExecutor, RawExecutor, ShellExecutor};
use super::{relative_speed, Benchmark};

use crate::command::Commands;
use crate::command::{Command, Commands};
use crate::export::ExportManager;
use crate::options::{ExecutorKind, Options, OutputStyleOption};

Expand Down Expand Up @@ -40,7 +40,12 @@ impl<'a> Scheduler<'a> {

executor.calibrate()?;

for (number, cmd) in self.commands.iter().enumerate() {
let reference = self
.options
.reference_command
.as_ref()
.map(|cmd| Command::new(None, &cmd));
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
.map(|cmd| Command::new(None, &cmd));
.map(|cmd| Command::new(None, cmd));

for (number, cmd) in reference.iter().chain(self.commands.iter()).enumerate() {
self.results
.push(Benchmark::new(number, cmd, self.options, &*executor).run()?);

Expand All @@ -62,24 +67,36 @@ impl<'a> Scheduler<'a> {
return;
}

if let Some(mut annotated_results) = relative_speed::compute(&self.results) {
annotated_results.sort_by(|l, r| relative_speed::compare_mean_time(l.result, r.result));
let reference = self
.options
.reference_command
.as_ref()
.map(|_| &self.results[0])
.unwrap_or_else(|| relative_speed::fastest(&self.results));

if let Some(mut annotated_results) = relative_speed::compute(reference, &self.results) {
if self.options.reference_command.is_none() {
annotated_results
.sort_by(|l, r| relative_speed::compare_mean_time(l.result, r.result));
}

let fastest = &annotated_results[0];
let reference = &annotated_results[0];
let others = &annotated_results[1..];

println!("{}", "Summary".bold());
println!(" '{}' ran", fastest.result.command.cyan());
println!(" '{}' ran", reference.result.command.cyan());

for item in others {
let comparator = if item.is_faster { "faster" } else { "slower" };
println!(
"{}{} times faster than '{}'",
"{}{} times {} than '{}'",
format!("{:8.2}", item.relative_speed).bold().green(),
if let Some(stddev) = item.relative_speed_stddev {
format!(" ± {}", format!("{:.2}", stddev).green())
} else {
"".into()
},
comparator,
&item.result.command.magenta()
);
}
Expand Down
9 changes: 9 additions & 0 deletions src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,15 @@ fn build_command() -> Command<'static> {
.help("Perform exactly NUM runs for each command. If this option is not specified, \
hyperfine automatically determines the number of runs."),
)
.arg(
Arg::new("reference")
.long("reference")
.short('R')
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's not introduce a short command line option for this right away. We can always add it later if this is really a commonly used option.

.takes_value(true)
.number_of_values(1)
.value_name("CMD")
.help("The reference command to measure the results against."),
Copy link
Owner

@sharkdp sharkdp Oct 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
.help("The reference command to measure the results against."),
.help("The reference command for the relative comparison of results. If this is unset, results are compared with the fastest command as reference."),

)
.arg(
Arg::new("setup")
.long("setup")
Expand Down
4 changes: 2 additions & 2 deletions src/export/markup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ pub trait MarkupExporter {
let min_str = format_duration_value(measurement.min, Some(unit)).0;
let max_str = format_duration_value(measurement.max, Some(unit)).0;
let rel_str = format!("{:.2}", entry.relative_speed);
let rel_stddev_str = if entry.is_fastest {
let rel_stddev_str = if entry.is_reference {
"".into()
} else if let Some(stddev) = entry.relative_speed_stddev {
format!(" ± {:.2}", stddev)
Expand Down Expand Up @@ -104,7 +104,7 @@ fn determine_unit_from_results(results: &[BenchmarkResult]) -> Unit {
impl<T: MarkupExporter> Exporter for T {
fn serialize(&self, results: &[BenchmarkResult], unit: Option<Unit>) -> Result<Vec<u8>> {
let unit = unit.unwrap_or_else(|| determine_unit_from_results(results));
let entries = relative_speed::compute(results);
let entries = relative_speed::compute(relative_speed::fastest(results), results);
if entries.is_none() {
return Err(anyhow!(
"Relative speed comparison is not available for markup exporter."
Expand Down
6 changes: 6 additions & 0 deletions src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,9 @@ pub struct Options {
/// Whether or not to ignore non-zero exit codes
pub command_failure_action: CmdFailureAction,

/// Command to run before each *batch* of timing runs, i.e. before each individual benchmark
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy & paste error?

pub reference_command: Option<String>,

/// Command(s) to run before each timing run
pub preparation_command: Option<Vec<String>>,

Expand Down Expand Up @@ -207,6 +210,7 @@ impl Default for Options {
warmup_count: 0,
min_benchmarking_time: 3.0,
command_failure_action: CmdFailureAction::RaiseError,
reference_command: None,
preparation_command: None,
setup_command: None,
cleanup_command: None,
Expand Down Expand Up @@ -260,6 +264,8 @@ impl Options {
(None, None) => {}
};

options.reference_command = matches.value_of("reference").map(String::from);

options.setup_command = matches.value_of("setup").map(String::from);

options.preparation_command = matches
Expand Down