Skip to content

Commit

Permalink
remove clone on BenchRunner
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
PSeitz committed Sep 29, 2024
1 parent 42b6ea2 commit 1930d83
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 48 deletions.
33 changes: 10 additions & 23 deletions src/bench_group.rs
Original file line number Diff line number Diff line change
@@ -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<PrintOnce>,
pub struct BenchGroup<'a, 'runner> {
group_name: Option<String>,
pub(crate) benches: Vec<Box<dyn Bench<'a> + 'a>>,
/// The size of the input.
/// Enables throughput reporting.
input_size_in_bytes: Option<usize>,
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<S: Into<String>>(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",
}
}

Expand Down Expand Up @@ -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())
}

Expand All @@ -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();

Expand Down Expand Up @@ -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,
)
}
}
7 changes: 4 additions & 3 deletions src/bench_input_group.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Input, OutputValue>` is a collection of benchmarks that are run with the same inputs.
///
/// It is self-contained and can be run independently.
///
Expand Down Expand Up @@ -112,9 +112,10 @@ impl<I: 'static, O: OutputValue + 'static> InputGroup<I, O> {

/// 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();
Expand Down
24 changes: 2 additions & 22 deletions src/bench_runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,18 +29,6 @@ pub struct BenchRunner {

reporter: Box<dyn Reporter>,
}
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: &() = &();

Expand Down Expand Up @@ -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<String>>(&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
Expand Down
5 changes: 5 additions & 0 deletions src/output_value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ impl OutputValue for u64 {
Some(format_with_underscores(*self))
}
}
impl OutputValue for usize {
fn format(&self) -> Option<String> {
Some(format_with_underscores(*self as u64))
}
}
impl OutputValue for String {
fn format(&self) -> Option<String> {
Some(self.clone())
Expand Down

0 comments on commit 1930d83

Please sign in to comment.