Simplify parameterized reference handling

This commit is contained in:
David Peter committed 2026-10-05 15:07:46 +02:00
1 parent f7429e2fb2
commit 463a889638
8 files changed
+210 -693

No files matched your search

+22 -52
View File
@@ -16,6 +16,20 @@ pub struct BenchmarkResultWithRelativeSpeed<'a> {
pub relative_ordering: Ordering,
}
impl BenchmarkResultWithRelativeSpeed<'_> {
pub fn reference_label(&self) -> &'static str {
if self.is_reference {
" (reference)"
} else {
match self.relative_ordering {
Ordering::Less => " (faster)",
Ordering::Equal => " (same speed)",
Ordering::Greater => " (slower)",
}
}
}
}
pub fn compare_mean_time(l: &BenchmarkResult, r: &BenchmarkResult) -> Ordering {
l.mean_wall_clock_time()
.partial_cmp(&r.mean_wall_clock_time())
@@ -29,7 +43,7 @@ pub fn fastest_of(results: &[BenchmarkResult]) -> &BenchmarkResult {
.expect("at least one benchmark result")
}
pub fn compute_relative_speeds<'a>(
fn compute_relative_speeds<'a>(
results: &'a [BenchmarkResult],
reference: &'a BenchmarkResult,
sort_order: SortOrder,
@@ -125,14 +139,14 @@ pub fn compute_with_check(
Some(compute_relative_speeds(results, fastest, sort_order))
}
/// Same as compute_with_check, potentially resulting in relative speeds of infinity
pub fn compute(
results: &[BenchmarkResult],
/// Compute relative speeds against the given reference, or the fastest result.
pub fn compute<'a>(
results: &'a [BenchmarkResult],
sort_order: SortOrder,
) -> Vec<BenchmarkResultWithRelativeSpeed<'_>> {
let fastest = fastest_of(results);
compute_relative_speeds(results, fastest, sort_order)
reference: Option<&'a BenchmarkResult>,
) -> Vec<BenchmarkResultWithRelativeSpeed<'a>> {
let reference = reference.unwrap_or_else(|| fastest_of(results));
compute_relative_speeds(results, reference, sort_order)
}
#[cfg(test)]
@@ -205,25 +219,6 @@ fn reference_identity_distinguishes_equal_results() {
assert!(!entries[1].is_reference);
}
#[test]
fn reference_ratios_indicate_faster_and_slower_runs() {
use approx::assert_relative_eq;
let results = vec![
create_result("faster", 1.0),
create_result("reference", 2.0),
create_result("slower", 3.0),
];
let entries = compute_relative_speeds(&results, &results[1], SortOrder::Command);
assert_relative_eq!(entries[0].relative_speed, 2.0);
assert_relative_eq!(entries[1].relative_speed, 1.0);
assert_relative_eq!(entries[2].relative_speed, 1.5);
assert!(!entries[0].is_reference);
assert!(entries[1].is_reference);
assert!(!entries[2].is_reference);
}
#[test]
fn reference_ratios_handle_zero_times() {
let mut results = vec![create_result("reference", 2.0), create_result("zero", 0.0)];
@@ -239,28 +234,3 @@ fn reference_ratios_handle_zero_times() {
assert_eq!(entries[0].relative_speed_stddev, None);
assert_eq!(entries[1].relative_speed, 1.0);
}
#[test]
fn reference_ratio_propagates_uncertainty() {
use crate::benchmark::measurement::Measurement;
use crate::quantity::second;
use approx::assert_relative_eq;
let mut results = vec![
create_result("faster", 1.0),
create_result("reference", 3.0),
];
for (result, times) in results.iter_mut().zip([[1.0, 2.0, 3.0], [3.0, 4.0, 5.0]]) {
result.measurements.measurements = times
.iter()
.copied()
.map(|seconds| Measurement {
time_wall_clock: Time::new::<second>(seconds),
..Default::default()
})
.collect();
}
let entries = compute_relative_speeds(&results, &results[1], SortOrder::Command);
assert_relative_eq!(entries[0].relative_speed, 2.0);
assert_relative_eq!(entries[0].relative_speed_stddev.unwrap(), 1.118033988749895);
}
+7 -20
View File
@@ -44,7 +44,6 @@ impl<'a> Scheduler<'a> {
.options
.reference_command
.as_ref()
.filter(|_| !self.options.parameterized_mode)
.map(|cmd| Command::new(self.options.reference_name.as_deref(), cmd));
executor.calibrate()?;
@@ -56,7 +55,7 @@ impl<'a> Scheduler<'a> {
// We export results after each individual benchmark, because
// we would risk losing them if a later benchmark fails.
self.export_manager
.write_results(&self.results, true, self.reference_index())?;
.write_results(&self.results, true, self.options.reference_index)?;
}
Ok(())
@@ -72,7 +71,8 @@ impl<'a> Scheduler<'a> {
}
let reference = self
.reference_index()
.options
.reference_index
.map(|index| &self.results[index])
.unwrap_or_else(|| relative_speed::fastest_of(&self.results));
@@ -130,16 +130,10 @@ impl<'a> Scheduler<'a> {
console_writeln!(stdout, "{}", "Relative speed comparison".bold())?;
for item in annotated_results {
let relationship = if self.reference_index().is_none() {
""
} else if item.is_reference {
" (reference)"
let relationship = if self.options.reference_index.is_some() {
item.reference_label()
} else {
match item.relative_ordering {
Ordering::Less => " (faster)",
Ordering::Equal => " (same speed)",
Ordering::Greater => " (slower)",
}
""
};
console_writeln!(
stdout,
@@ -174,16 +168,9 @@ impl<'a> Scheduler<'a> {
Ok(())
}
fn reference_index(&self) -> Option<usize> {
self.options
.reference_command
.as_ref()
.map(|_| self.options.parameterized_reference_index.unwrap_or(0))
}
pub fn final_export(&self) -> Result<()> {
self.export_manager
.write_results(&self.results, false, self.reference_index())
.write_results(&self.results, false, self.options.reference_index)
}
}
+17 -26
View File
@@ -67,28 +67,19 @@ pub trait MarkupExporter {
&& !entry.is_reference
&& entry.relative_ordering == Ordering::Equal
&& measurement.mean_wall_clock_time() == Time::zero());
let rel_str = if relative_unavailable {
let relative = if relative_unavailable {
"N/A".to_string()
} else {
format!("{:.2}", entry.relative_speed)
};
let rel_stddev_str = if relative_unavailable || entry.is_reference {
"".into()
} else if let Some(stddev) = entry.relative_speed_stddev {
format!(" ± {stddev:.2}")
} else {
"".into()
};
let rel_direction = if relative_unavailable || !explicit_reference {
""
} else if entry.is_reference {
" (reference)"
} else {
match entry.relative_ordering {
Ordering::Less => " (faster)",
Ordering::Greater => " (slower)",
Ordering::Equal => " (same speed)",
}
let stddev = match (entry.is_reference, entry.relative_speed_stddev) {
(false, Some(stddev)) => format!(" ± {stddev:.2}"),
_ => String::new(),
};
let label = if explicit_reference {
entry.reference_label()
} else {
""
};
format!("{:.2}{stddev}{label}", entry.relative_speed)
};
// prepare table row entries
@@ -97,7 +88,7 @@ pub trait MarkupExporter {
&format!("{mean_str}{stddev_str}"),
&min_str,
&max_str,
&format!("{rel_str}{rel_stddev_str}{rel_direction}"),
&relative,
]))
}
@@ -144,11 +135,11 @@ impl<T: MarkupExporter> Exporter for T {
// Do not report ratios against another benchmark while the selected
// reference is still pending in an intermediate export.
let reference_pending = reference_index.is_some_and(|i| i >= results.len());
let entries = if let Some(reference) = reference_index.and_then(|i| results.get(i)) {
relative_speed::compute_relative_speeds(results, reference, sort_order)
} else {
relative_speed::compute(results, sort_order)
};
let entries = relative_speed::compute(
results,
sort_order,
reference_index.and_then(|i| results.get(i)),
);
let table =
self.table_results(&entries, unit, reference_pending, reference_index.is_some());
+1 -166
View File
@@ -12,174 +12,9 @@ fn get_output<E: Exporter + Default>(
results: &[BenchmarkResult],
unit: Option<TimeUnit>,
sort_order: SortOrder,
) -> String {
get_output_with_reference::<E>(results, unit, sort_order, None)
}
fn get_output_with_reference<E: Exporter + Default>(
results: &[BenchmarkResult],
unit: Option<TimeUnit>,
sort_order: SortOrder,
reference_index: Option<usize>,
) -> String {
let exporter = E::default();
String::from_utf8(
exporter
.serialize(results, unit, sort_order, reference_index)
.unwrap(),
)
.unwrap()
}
#[test]
fn test_markup_exports_identify_explicit_reference_and_direction() {
let benchmark = |name: &str, seconds| {
let measurement = Measurement {
time_wall_clock: Time::new::<second>(seconds),
..Measurement::default()
};
BenchmarkResult {
command: name.to_owned(),
command_with_unused_parameters: name.to_owned(),
measurements: Measurements::new(vec![measurement, measurement]),
..BenchmarkResult::default()
}
};
let results = [
benchmark("faster", 1.0),
benchmark("reference", 2.0),
benchmark("equal", 2.0),
benchmark("slower", 3.0),
];
for sort_order in [SortOrder::Command, SortOrder::MeanTime] {
let outputs = [
get_output_with_reference::<MarkdownExporter>(
&results,
Some(TimeUnit::Second),
sort_order,
Some(1),
),
get_output_with_reference::<AsciidocExporter>(
&results,
Some(TimeUnit::Second),
sort_order,
Some(1),
),
get_output_with_reference::<OrgmodeExporter>(
&results,
Some(TimeUnit::Second),
sort_order,
Some(1),
),
];
for output in outputs {
for expected in [
"2.00 ± 0.00 (faster)",
"1.00 (reference)",
"1.00 ± 0.00 (same speed)",
"1.50 ± 0.00 (slower)",
] {
assert!(output.contains(expected), "{}", output);
}
}
}
}
#[test]
fn test_markup_exports_leave_pending_reference_ratios_unavailable() {
let results = [BenchmarkResult {
command: "sleep 1".into(),
command_with_unused_parameters: "sleep 1".into(),
measurements: Measurements::new(vec![Measurement {
time_wall_clock: Time::new::<second>(1.0),
..Measurement::default()
}]),
..BenchmarkResult::default()
}];
for sort_order in [SortOrder::Command, SortOrder::MeanTime] {
let outputs = [
get_output_with_reference::<MarkdownExporter>(
&results,
Some(TimeUnit::Second),
sort_order,
Some(1),
),
get_output_with_reference::<AsciidocExporter>(
&results,
Some(TimeUnit::Second),
sort_order,
Some(1),
),
get_output_with_reference::<OrgmodeExporter>(
&results,
Some(TimeUnit::Second),
sort_order,
Some(1),
),
];
for output in outputs {
assert!(output.contains("N/A"), "{}", output);
assert!(!output.contains("(reference)"), "{}", output);
}
}
}
#[test]
fn test_markup_exports_with_zero_reference() {
let benchmark = |name: &str, seconds| BenchmarkResult {
command: name.to_owned(),
command_with_unused_parameters: name.to_owned(),
measurements: Measurements::new(vec![Measurement {
time_wall_clock: Time::new::<second>(seconds),
..Measurement::default()
}]),
..BenchmarkResult::default()
};
let results = [
benchmark("other zero", 0.0),
benchmark("reference", 0.0),
benchmark("slower", 1.0),
];
for sort_order in [SortOrder::Command, SortOrder::MeanTime] {
for reference in [Some(1), Some(3), None] {
let outputs = [
get_output_with_reference::<MarkdownExporter>(
&results,
Some(TimeUnit::Second),
sort_order,
reference,
),
get_output_with_reference::<AsciidocExporter>(
&results,
Some(TimeUnit::Second),
sort_order,
reference,
),
get_output_with_reference::<OrgmodeExporter>(
&results,
Some(TimeUnit::Second),
sort_order,
reference,
),
];
for output in outputs {
match reference {
Some(1) => {
assert_eq!(output.matches("N/A").count(), 1, "{}", output);
assert!(output.contains("1.00 (reference)"), "{}", output);
assert!(output.contains("inf (slower)"), "{}", output);
}
Some(_) => assert_eq!(output.matches("N/A").count(), 3, "{}", output),
None => {
assert!(!output.contains("N/A"), "{}", output);
assert!(output.contains("inf"), "{}", output);
assert!(!output.contains("(reference)"), "{}", output);
}
}
}
}
}
String::from_utf8(exporter.serialize(results, unit, sort_order, None).unwrap()).unwrap()
}
/// Ensure the makrup output includes the table header and the multiple
+15 -14
View File
@@ -208,17 +208,14 @@ pub struct Options {
/// Whether or not to ignore non-zero exit codes
pub command_failure_action: CmdFailureAction,
// Command to use as a reference for relative speed comparison
// Standalone reference command, cleared when an existing benchmark is selected.
pub reference_command: Option<String>,
// Name of the reference command
pub reference_name: Option<String>,
// Whether commands are expanded from --parameter-scan or --parameter-list.
pub parameterized_mode: bool,
// Index of the selected command in a parameterized run, if any.
pub parameterized_reference_index: Option<usize>,
// Index of the reference in the benchmark sequence, if any.
pub reference_index: Option<usize>,
/// Command(s) to run before each timing run
pub preparation_command: Option<Vec<String>>,
@@ -263,8 +260,7 @@ impl Default for Options {
command_failure_action: CmdFailureAction::RaiseError,
reference_command: None,
reference_name: None,
parameterized_mode: false,
parameterized_reference_index: None,
reference_index: None,
preparation_command: None,
conclusion_command: None,
setup_command: None,
@@ -317,8 +313,6 @@ impl Options {
options.setup_command = matches.get_one::<String>("setup").map(String::from);
options.parameterized_mode = matches.get_many::<String>("parameter-scan").is_some()
|| matches.get_many::<String>("parameter-list").is_some();
options.reference_command = matches.get_one::<String>("reference").map(String::from);
options.reference_name = matches
.get_one::<String>("reference-name")
@@ -474,8 +468,12 @@ impl Options {
}
pub fn validate_against_command_list(&mut self, commands: &Commands) -> Result<()> {
if self.parameterized_mode {
if let Some(reference) = &self.reference_command {
if let Some(reference) = &self.reference_command {
if commands
.iter()
.next()
.is_some_and(|command| !command.get_parameters().is_empty())
{
ensure!(
self.reference_name.is_none(),
"--reference-name cannot be used with a parameterized reference; use --command-name to name the benchmark"
@@ -493,10 +491,13 @@ impl Options {
matches.next().is_none(),
"Reference '{reference}' matches multiple parameterized benchmarks. Use --command-name to give them unique names."
);
self.parameterized_reference_index = Some(index);
self.reference_index = Some(index);
self.reference_command = None;
} else {
self.reference_index = Some(0);
}
}
let has_reference_command = self.reference_command.is_some() && !self.parameterized_mode;
let has_reference_command = self.reference_command.is_some();
let num_commands = commands.num_commands(has_reference_command);
if let Some(preparation_command) = &self.preparation_command {