fix(benchmarker): incorrect results when optimised

4 files changed, 19 insertions, 62 deletions

diff --git a/src/crates/benchmarker/src/bench.rs b/src/crates/benchmarker/src/bench.rs
index 9c4cf74..5246ba7 100644
--- a/src/crates/benchmarker/src/bench.rs
+++ b/src/crates/benchmarker/src/bench.rs
@@ -12,10 +12,9 @@ use crate::BenchmarkResult;
 /// If `undo` is invalid, this will return garbage results.
 ///
 /// Warm-up for the setup is done beforehand.
-pub fn benchmark_op<T>(
+pub fn benchmark_op<T, R>(
     mut setup: impl FnMut() -> T,
-    mut op: impl FnMut(&mut T),
-    mut undo: impl FnMut(&mut T),
+    mut op: impl FnMut(&mut T) -> R,
 ) -> BenchmarkResult {
     // let loop_end = Instant::now() + Duration::from_secs(5);
     let loop_end = Instant::now() + Duration::from_millis(100);
@@ -25,11 +24,9 @@ pub fn benchmark_op<T>(
     let mut max = Duration::from_secs(0);
     let mut sum = Duration::from_secs(0);
 
-    let mut target = setup();
     while Instant::now() + max < loop_end {
-        #[allow(clippy::unit_arg)] // pretty sure this is necessary to prevent optimisations
-        let duration = time_singular(|| black_box(op(&mut target)));
-        undo(&mut target);
+        let mut target = setup();
+        let duration = time_singular(|| op(&mut target));
 
         min = cmp::min(min, duration);
         max = cmp::max(max, duration);
@@ -45,9 +42,9 @@ pub fn benchmark_op<T>(
     }
 }
 
-fn time_singular(f: impl FnOnce()) -> Duration {
+fn time_singular<R>(f: impl FnOnce() -> R) -> Duration {
     let start = Instant::now();
-    f();
+    black_box(f());
     let end = Instant::now();
     end - start
 }
@@ -65,19 +62,12 @@ mod tests {
                 assert!(!(*b));
                 *b = true;
             },
-            |b| {
-                *b = false;
-            },
         );
     }
 
     #[test]
     fn benchmark_op_times_properly() {
-        let results = benchmark_op(
-            || (),
-            |_| std::thread::sleep(Duration::from_millis(5)),
-            |_| {},
-        );
+        let results = benchmark_op(|| (), |_| std::thread::sleep(Duration::from_millis(5)));
 
         let avg_millis = results.avg.as_nanos() as f32 / (10.0_f32).powi(6);
         dbg!(avg_millis);
diff --git a/src/crates/benchmarker/src/container.rs b/src/crates/benchmarker/src/container.rs
index 39e2288..9d5fac5 100644
--- a/src/crates/benchmarker/src/container.rs
+++ b/src/crates/benchmarker/src/container.rs
@@ -91,10 +91,7 @@ where
 
                     (c, chosen)
                 },
-                |(c, search)| {
-                    c.contains(search);
-                },
-                |_| (),
+                |(c, search)| c.contains(search),
             ),
         )
     }
@@ -110,10 +107,7 @@ where
                     }
                     c
                 },
-                |c| {
-                    c.len();
-                },
-                |_| (),
+                |c| c.len(),
             ),
         )
     }
@@ -128,14 +122,10 @@ where
         };
         (
             n,
-            benchmark_op(
-                setup_closure,
-                |c| {
-                    // TODO: rng generation could throw off benchmarks
-                    c.insert(random());
-                },
-                |c| *c = setup_closure(),
-            ),
+            benchmark_op(setup_closure, |c| {
+                // TODO: rng generation could throw off benchmarks
+                c.insert(random())
+            }),
         )
     }
 
@@ -147,16 +137,7 @@ where
             }
             c
         };
-        (
-            n,
-            benchmark_op(
-                setup_closure,
-                |c| {
-                    c.clear();
-                },
-                |c| *c = setup_closure(),
-            ),
-        )
+        (n, benchmark_op(setup_closure, |c| c.clear()))
     }
 
     fn benchmark_container_remove(n: usize) -> Observation {
@@ -182,10 +163,7 @@ where
 
                     (c, chosen)
                 },
-                |(c, chosen)| {
-                    c.remove(chosen.clone());
-                },
-                |(c, chosen)| c.insert(chosen.clone()),
+                |(c, chosen)| c.remove(chosen.clone()),
             ),
         )
     }
diff --git a/src/crates/benchmarker/src/indexable.rs b/src/crates/benchmarker/src/indexable.rs
index a30a1f4..86ea759 100644
--- a/src/crates/benchmarker/src/indexable.rs
+++ b/src/crates/benchmarker/src/indexable.rs
@@ -1,4 +1,4 @@
-use std::{any::type_name, collections::HashMap};
+use std::{any::type_name, collections::HashMap, hint::black_box};
 
 use log::debug;
 use primrose_library::traits::{Container, Indexable};
@@ -57,9 +57,8 @@ where
                     c
                 },
                 |c| {
-                    c.first();
+                    black_box(c.first());
                 },
-                |_| (),
             ),
         )
     }
@@ -76,9 +75,8 @@ where
                     c
                 },
                 |c| {
-                    c.last();
+                    black_box(c.last());
                 },
-                |_| (),
             ),
         )
     }
@@ -97,7 +95,6 @@ where
                 |(c, fetch)| {
                     c.nth(*fetch);
                 },
-                |_| (),
             ),
         )
     }
diff --git a/src/crates/benchmarker/src/stack.rs b/src/crates/benchmarker/src/stack.rs
index 7680548..7a1cc6e 100644
--- a/src/crates/benchmarker/src/stack.rs
+++ b/src/crates/benchmarker/src/stack.rs
@@ -49,9 +49,6 @@ where
                     c
                 },
                 |s| s.push(random()),
-                |s| {
-                    s.pop();
-                },
             ),
         )
     }
@@ -67,12 +64,7 @@ where
                     }
                     c
                 },
-                |s| {
-                    s.pop();
-                },
-                |s| {
-                    s.push(random());
-                },
+                |s| s.pop(),
             ),
         )
     }