Merge pull request #1476 from nnethercote/runtime-tweaks

Runtime benchmarking tweaks

Author: nnethercote

collector/benchlib/src/benchmark.rs

@@ -5,23 +5,25 @@ use crate::measure::benchmark_function;
 use crate::process::raise_process_priority;
 use std::collections::HashMap;
 
-/// Create a new benchmark group. Use the closure argument to define individual benchmarks.
-pub fn run_benchmark_group<F: FnOnce(&mut BenchmarkGroup)>(define_func: F) {
+/// Create and run a new benchmark group. Use the closure argument to register
+/// the individual benchmarks.
+pub fn run_benchmark_group<F>(register: F)
+where
+    F: FnOnce(&mut BenchmarkGroup),
+{
     env_logger::init();
 
     let mut group = BenchmarkGroup::new();
-    define_func(&mut group);
+    register(&mut group);
     group.run().expect("Benchmark group execution has failed");
 }
 
 /// Type-erased function that executes a single benchmark.
-struct BenchmarkWrapper {
-    func: Box<dyn Fn() -> anyhow::Result<BenchmarkStats>>,
-}
+type BenchmarkFn = Box<dyn Fn() -> anyhow::Result<BenchmarkStats>>;
 
 #[derive(Default)]
 pub struct BenchmarkGroup {
-    benchmarks: HashMap<&'static str, BenchmarkWrapper>,
+    benchmarks: HashMap<&'static str, BenchmarkFn>,
 }
 
 impl BenchmarkGroup {
@@ -30,19 +32,21 @@ impl BenchmarkGroup {
     }
 
     /// Registers a single benchmark.
-    /// `constructor` should return a closure that will be benchmarked.
-    pub fn register<F: Fn() -> Bench + Clone + 'static, R, Bench: FnOnce() -> R + 'static>(
-        &mut self,
-        name: &'static str,
-        constructor: F,
-    ) {
+    ///
+    /// `constructor` returns a closure that will be benchmarked. This means
+    /// `constructor` can do initialization steps outside of the code that is
+    /// measured. `constructor` may be called multiple times (e.g. once for a
+    /// run with performance counters and once for a run without), but the
+    /// closure it produces each time will only be called once.
+    pub fn register_benchmark<Ctor, Bench, R>(&mut self, name: &'static str, constructor: Ctor)
+    where
+        Ctor: Fn() -> Bench + Clone + 'static,
+        Bench: FnOnce() -> R + 'static,
+    {
         // We want to type-erase the target `func` by wrapping it in a Box.
-        let benchmark_func = Box::new(move || benchmark_function(constructor.clone()));
-        let benchmark_def = BenchmarkWrapper {
-            func: benchmark_func,
-        };
-        if self.benchmarks.insert(name, benchmark_def).is_some() {
-            panic!("Benchmark {} was registered twice", name);
+        let benchmark_fn = Box::new(move || benchmark_function(constructor.clone()));
+        if self.benchmarks.insert(name, benchmark_fn).is_some() {
+            panic!("Benchmark '{}' was registered twice", name);
         }
     }
 
@@ -63,7 +67,7 @@ impl BenchmarkGroup {
     }
 
     fn run_benchmarks(self, args: BenchmarkArgs) -> anyhow::Result<()> {
-        let mut items: Vec<(&'static str, BenchmarkWrapper)> = self
+        let mut items: Vec<(&'static str, BenchmarkFn)> = self
             .benchmarks
             .into_iter()
             .filter(|(name, _)| {
@@ -74,10 +78,10 @@ impl BenchmarkGroup {
 
         let mut stdout = std::io::stdout().lock();
 
-        for (name, def) in items {
+        for (name, benchmark_fn) in items {
             let mut stats: Vec<BenchmarkStats> = Vec::with_capacity(args.iterations as usize);
             for i in 0..args.iterations {
-                let benchmark_stats = (def.func)()?;
+                let benchmark_stats = benchmark_fn()?;
                 log::info!("Benchmark (run {i}) `{name}` completed: {benchmark_stats:?}");
                 stats.push(benchmark_stats);
             }
@@ -101,24 +105,6 @@ impl BenchmarkGroup {
     }
 }
 
-/// Adds a single benchmark to the benchmark group.
-/// ```ignore
-/// use benchlib::define_benchmark;
-///
-/// define_benchmark!(group, my_bench, {
-///     || do_something()
-/// });
-/// ```
-#[macro_export]
-macro_rules! define_benchmark {
-    ($group:expr, $name:ident, $fun:expr) => {
-        let func = move || $fun;
-        $group.register(stringify!($name), func);
-    };
-}
-
-pub use define_benchmark;
-
 /// Tests if the name of the benchmark passes through the include and exclude filters.
 /// Both filters can contain multiple comma-separated prefixes.
 pub fn passes_filter(name: &str, exclude: Option<&str>, include: Option<&str>) -> bool {

collector/runtime-benchmarks/bufreader/src/main.rs

@@ -1,9 +1,6 @@
-use std::io::{BufRead, BufReader, Write};
-
-use snap::{read::FrameDecoder, write::FrameEncoder};
-
 use benchlib::benchmark::{black_box, run_benchmark_group};
-use benchlib::define_benchmark;
+use snap::{read::FrameDecoder, write::FrameEncoder};
+use std::io::{BufRead, BufReader, Write};
 
 const BYTES: usize = 64 * 1024 * 1024;
 
@@ -12,7 +9,7 @@ fn main() {
     // The pattern we want is a BufReader which wraps a Read impl where one Read::read call will
     // never fill the whole BufReader buffer.
     run_benchmark_group(|group| {
-        define_benchmark!(group, bufreader_snappy, {
+        group.register_benchmark("bufreader_snappy", || {
             let data = vec![0u8; BYTES];
             move || {
                 let mut compressed = Vec::new();

collector/runtime-benchmarks/hashmap/src/main.rs

@@ -1,11 +1,10 @@
 use benchlib;
 use benchlib::benchmark::run_benchmark_group;
-use benchlib::define_benchmark;
 
 fn main() {
     run_benchmark_group(|group| {
         // Measures how long does it take to insert 10 thousand numbers into a `hashbrown` hashmap.
-        define_benchmark!(group, hashmap_insert_10k, {
+        group.register_benchmark("hashmap_insert_10k", || {
             let mut map = hashbrown::HashMap::with_capacity_and_hasher(
                 10000,
                 fxhash::FxBuildHasher::default(),

collector/runtime-benchmarks/nbody/src/main.rs

@@ -2,13 +2,12 @@
 //! Code taken from https://github.com/prestontw/rust-nbody
 
 use benchlib::benchmark::run_benchmark_group;
-use benchlib::define_benchmark;
 
 mod nbody;
 
 fn main() {
     run_benchmark_group(|group| {
-        define_benchmark!(group, nbody_10k, {
+        group.register_benchmark("nbody_10k", || {
             let mut nbody_10k = nbody::init(10000);
             || {
                 for _ in 0..10 {