From 1930d8360c1d4a68100d5ea622f8bba71a559c82 Mon Sep 17 00:00:00 2001 From: Pascal Seitz Date: Sat, 28 Sep 2024 08:17:37 +0800 Subject: [PATCH] remove clone on BenchRunner remove clone on BenchRunner. setting options is ambigious when there are clones of BenchRunner and there are ripple effects like cloning boxes for the Reporter trait. --- src/bench_group.rs | 33 ++++++++++----------------------- src/bench_input_group.rs | 7 ++++--- src/bench_runner.rs | 24 ++---------------------- src/output_value.rs | 5 +++++ 4 files changed, 21 insertions(+), 48 deletions(-) diff --git a/src/bench_group.rs b/src/bench_group.rs index d7c209a..5c4f6c1 100644 --- a/src/bench_group.rs +++ b/src/bench_group.rs @@ -1,45 +1,31 @@ use crate::{ bench::{Bench, BenchResult, InputWithBenchmark, NamedBench}, - bench_id::{BenchId, PrintOnce}, + bench_id::BenchId, bench_runner::BenchRunner, output_value::OutputValue, }; /// `BenchGroup` is a group of benchmarks wich are executed together. /// -pub struct BenchGroup<'a> { - bench_runner_name: Option, +pub struct BenchGroup<'a, 'runner> { group_name: Option, pub(crate) benches: Vec + 'a>>, /// The size of the input. /// Enables throughput reporting. input_size_in_bytes: Option, - pub(crate) runner: BenchRunner, - pub(crate) coutput_value_column_title: &'static str, + pub(crate) runner: &'runner mut BenchRunner, + pub(crate) output_value_column_title: &'static str, } -impl<'a> BenchGroup<'a> { +impl<'a, 'runner> BenchGroup<'a, 'runner> { /// Create a new BenchGroup with no benchmarks. - pub fn new(runner: BenchRunner) -> Self { + pub fn new(runner: &'runner mut BenchRunner) -> Self { Self { group_name: None, - bench_runner_name: runner.name.to_owned(), benches: Vec::new(), input_size_in_bytes: None, runner, - coutput_value_column_title: "Output", - } - } - - /// Create a new BenchGroup with no benchmarks. - pub fn with_name>(runner: BenchRunner, name: S) -> Self { - Self { - group_name: Some(name.into()), - bench_runner_name: runner.name.to_owned(), - benches: Vec::new(), - input_size_in_bytes: None, - runner, - coutput_value_column_title: "Output", + output_value_column_title: "Output", } } @@ -96,7 +82,7 @@ impl<'a> BenchGroup<'a> { fn get_bench_id(&self, bench_name: String) -> BenchId { BenchId::from_bench_name(bench_name) - .runner_name(self.bench_runner_name.as_deref()) + .runner_name(self.runner.name.as_deref()) .group_name(self.group_name.clone()) } @@ -106,6 +92,7 @@ impl<'a> BenchGroup<'a> { bench: NamedBench<'a, I, O>, input: &'a I, ) { + self.output_value_column_title = O::column_title(); if let Some(filter) = &self.runner.config.filter { let bench_id = bench.bench_id.get_full_name(); @@ -137,7 +124,7 @@ impl<'a> BenchGroup<'a> { self.runner.run_group( self.group_name.as_deref(), &mut self.benches, - self.coutput_value_column_title, + self.output_value_column_title, ) } } diff --git a/src/bench_input_group.rs b/src/bench_input_group.rs index 33c41e3..4dcc5ab 100644 --- a/src/bench_input_group.rs +++ b/src/bench_input_group.rs @@ -9,7 +9,7 @@ use peakmem_alloc::*; pub(crate) type Alloc = &'static dyn PeakMemAllocTrait; -/// `InputGroup` is a collection of benchmarks that are run with the same inputs. +/// `InputGroup` is a collection of benchmarks that are run with the same inputs. /// /// It is self-contained and can be run independently. /// @@ -112,9 +112,10 @@ impl InputGroup { /// Run the benchmarks and report the results. pub fn run(&mut self) { - for (ord, benches) in self.benches_per_input.iter_mut().enumerate() { + let mut benches_per_input = mem::take(&mut self.benches_per_input); + for (ord, benches) in benches_per_input.iter_mut().enumerate() { let input = &self.inputs[ord]; - let mut group = BenchGroup::new(self.runner.clone()); + let mut group = BenchGroup::new(&mut self.runner); group.set_name(&input.name); // reverse so we can use pop and keep the order benches.reverse(); diff --git a/src/bench_runner.rs b/src/bench_runner.rs index 587a344..9e6b7f6 100644 --- a/src/bench_runner.rs +++ b/src/bench_runner.rs @@ -29,18 +29,6 @@ pub struct BenchRunner { reporter: Box, } -impl Clone for BenchRunner { - fn clone(&self) -> Self { - Self { - alloc: self.alloc, - cache_trasher: self.cache_trasher.clone(), - config: self.config.clone(), - input_size_in_bytes: self.input_size_in_bytes, - name: self.name.clone(), - reporter: self.reporter.clone_box(), - } - } -} pub const EMPTY_INPUT: &() = &(); @@ -81,16 +69,8 @@ impl BenchRunner { /// Creates a new `BenchGroup` /// The group is a collection of benchmarks that are run together. - pub fn new_group<'a>(&self) -> BenchGroup<'a> { - BenchGroup::new(self.clone()) - } - /// Creates a new named `BenchGroup` - /// The group is a collection of benchmarks that are run together. - /// - /// The name of the group could be for example the label of the input on which the group is - /// run. - pub fn new_group_with_name<'a, S: Into>(&self, name: S) -> BenchGroup<'a> { - BenchGroup::with_name(self.clone(), name) + pub fn new_group(&mut self) -> BenchGroup<'_, '_> { + BenchGroup::new(self) } /// Set the name of the current test runner. This is like a header for all tests in in this diff --git a/src/output_value.rs b/src/output_value.rs index 55e2088..8f2ef51 100644 --- a/src/output_value.rs +++ b/src/output_value.rs @@ -33,6 +33,11 @@ impl OutputValue for u64 { Some(format_with_underscores(*self)) } } +impl OutputValue for usize { + fn format(&self) -> Option { + Some(format_with_underscores(*self as u64)) + } +} impl OutputValue for String { fn format(&self) -> Option { Some(self.clone())