From 988091d8200e7d88a137c0adc93111d722f29101 Mon Sep 17 00:00:00 2001 From: Dimitris Sarlis Date: Fri, 2 May 2025 09:13:07 +0000 Subject: [PATCH 1/3] feat: Allow repeated scopes --- canbench-bin/tests/tests.rs | 58 ++++++++++++++++++- canbench-rs/src/lib.rs | 51 ++++++++++------ .../measurements_output/canbench_results.yml | 11 ++++ tests/measurements_output/src/main.rs | 22 +++++++ 4 files changed, 121 insertions(+), 21 deletions(-) diff --git a/canbench-bin/tests/tests.rs b/canbench-bin/tests/tests.rs index ccc0561e..c5f1d7b5 100644 --- a/canbench-bin/tests/tests.rs +++ b/canbench-bin/tests/tests.rs @@ -279,7 +279,7 @@ fn reports_scopes_in_new_benchmark() { Benchmark: bench_scope_new (new) total: - instructions: 3988 (new) + instructions: 4660 (new) heap_increase: 1 pages (new) stable_memory_increase: 0 pages (new) @@ -356,7 +356,7 @@ fn reports_scopes_in_existing_benchmark() { Benchmark: bench_scope_exists total: - instructions: 3988 (regressed from 0) + instructions: 4660 (regressed from 0) heap_increase: 1 pages (regressed from 0) stable_memory_increase: 0 pages (no change) @@ -489,3 +489,57 @@ Instruction traces written to write_stable_memory.svg ); }); } + +#[test] +fn reports_repeated_scope_in_new_benchmark() { + BenchTest::canister("measurements_output") + .with_bench("bench_repeated_scope_new") + .run(|output| { + assert_success!( + output, + " +--------------------------------------------------- + +Benchmark: bench_repeated_scope_new (new) + total: + instructions: 17.12 K (new) + heap_increase: 1 pages (new) + stable_memory_increase: 0 pages (new) + + scope_1 (scope): + instructions: 8694 (new) + heap_increase: 1 pages (new) + stable_memory_increase: 0 pages (new) + +--------------------------------------------------- +" + ); + }); +} + +#[test] +fn reports_repeated_scope_in_existing_benchmark() { + BenchTest::canister("measurements_output") + .with_bench("bench_repeated_scope_exists") + .run(|output| { + assert_success!( + output, + " +--------------------------------------------------- + +Benchmark: bench_repeated_scope_exists + total: + instructions: 17.12 K (regressed from 0) + heap_increase: 1 pages (regressed from 0) + stable_memory_increase: 0 pages (no change) + + scope_1 (scope): + instructions: 8694 (regressed by 986.75%) + heap_increase: 1 pages (improved by 91.67%) + stable_memory_increase: 0 pages (no change) + +--------------------------------------------------- +" + ); + }); +} diff --git a/canbench-rs/src/lib.rs b/canbench-rs/src/lib.rs index add4635f..2bf321aa 100644 --- a/canbench-rs/src/lib.rs +++ b/canbench-rs/src/lib.rs @@ -465,11 +465,10 @@ pub use canbench_rs_macros::bench; use candid::CandidType; use serde::{Deserialize, Serialize}; -use std::cell::RefCell; -use std::collections::BTreeMap; +use std::{cell::RefCell, collections::BTreeMap, ops::Add}; thread_local! { - static SCOPES: RefCell> = + static SCOPES: RefCell>> = const { RefCell::new(BTreeMap::new()) }; } @@ -500,6 +499,18 @@ pub struct Measurement { pub stable_memory_increase: u64, } +impl Add for Measurement { + type Output = Self; + + fn add(self, other: Self) -> Self::Output { + Self { + instructions: self.instructions + other.instructions, + heap_increase: self.heap_increase + other.heap_increase, + stable_memory_increase: self.stable_memory_increase + other.stable_memory_increase, + } + } +} + /// Benchmarks the given function. pub fn bench_fn(f: impl FnOnce() -> R) -> BenchResult { reset(); @@ -614,20 +625,11 @@ impl Drop for BenchScope { SCOPES.with(|p| { let mut p = p.borrow_mut(); - let prev_scope = p.insert( - self.name, - Measurement { - instructions, - heap_increase, - stable_memory_increase, - }, - ); - - assert!( - prev_scope.is_none(), - "scope {} cannot be specified multiple times.", - self.name - ); + p.entry(self.name).or_insert(vec![]).push(Measurement { + instructions, + heap_increase, + stable_memory_increase, + }); }); } } @@ -637,9 +639,20 @@ fn reset() { SCOPES.with(|p| p.borrow_mut().clear()); } -// Returns the measurements for any declared scopes. +// Returns the measurements for any declared scopes, +// aggregated by the scope name. fn get_scopes_measurements() -> std::collections::BTreeMap<&'static str, Measurement> { - SCOPES.with(|p| p.borrow().clone()) + SCOPES + .with(|p| p.borrow().clone()) + .into_iter() + .map(|(scope, measurements)| { + let mut total = Measurement::default(); + for measurement in measurements { + total = total + measurement; + } + (scope, total) + }) + .collect() } fn instruction_count() -> u64 { diff --git a/tests/measurements_output/canbench_results.yml b/tests/measurements_output/canbench_results.yml index 16cb80aa..5f68044d 100644 --- a/tests/measurements_output/canbench_results.yml +++ b/tests/measurements_output/canbench_results.yml @@ -50,6 +50,17 @@ benches: instructions: 0 stable_memory_increase: 0 + scopes: + scope_1: + heap_increase: 12 + instructions: 800 + # A benchmark that is expected to return an increase of new repeated scope steps. + bench_repeated_scope_exists: + total: + heap_increase: 0 + instructions: 0 + stable_memory_increase: 0 + scopes: scope_1: heap_increase: 12 diff --git a/tests/measurements_output/src/main.rs b/tests/measurements_output/src/main.rs index 28f87a97..8cf0fe7a 100644 --- a/tests/measurements_output/src/main.rs +++ b/tests/measurements_output/src/main.rs @@ -99,6 +99,28 @@ fn bench_scope_exists() { } } +// A benchmark that includes a repeated scope, but isn't persisted in the results. +#[bench] +fn bench_repeated_scope_new() { + { + for _ in 0..10 { + let _p = bench_scope("scope_1"); + println!("do something"); + } + } +} + +// A benchmark that includes a repeated scope and is persisted in the results. +#[bench] +fn bench_repeated_scope_exists() { + { + for _ in 0..10 { + let _p = bench_scope("scope_1"); + println!("do something"); + } + } +} + #[export_name = "canister_query __canbench__broken_benchmark"] fn broken_benchmark() { // This benchmark doesn't reply, and will therefore fail. From 3fe3834fc0ce7de629fa489ac54de06fcb1efafd Mon Sep 17 00:00:00 2001 From: Dimitris Sarlis Date: Fri, 2 May 2025 09:36:47 +0000 Subject: [PATCH 2/3] Fix clippy --- canbench-rs/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/canbench-rs/src/lib.rs b/canbench-rs/src/lib.rs index 2bf321aa..f6db7bcb 100644 --- a/canbench-rs/src/lib.rs +++ b/canbench-rs/src/lib.rs @@ -625,7 +625,7 @@ impl Drop for BenchScope { SCOPES.with(|p| { let mut p = p.borrow_mut(); - p.entry(self.name).or_insert(vec![]).push(Measurement { + p.entry(self.name).or_default().push(Measurement { instructions, heap_increase, stable_memory_increase, From aa1514e74bac8b56a41ef7bf024adfd066d93d42 Mon Sep 17 00:00:00 2001 From: Dimitris Sarlis Date: Fri, 2 May 2025 09:45:19 +0000 Subject: [PATCH 3/3] Fix tests --- canbench-bin/tests/tests.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/canbench-bin/tests/tests.rs b/canbench-bin/tests/tests.rs index c5f1d7b5..0669ab39 100644 --- a/canbench-bin/tests/tests.rs +++ b/canbench-bin/tests/tests.rs @@ -279,7 +279,7 @@ fn reports_scopes_in_new_benchmark() { Benchmark: bench_scope_new (new) total: - instructions: 4660 (new) + instructions: 4626 (new) heap_increase: 1 pages (new) stable_memory_increase: 0 pages (new) @@ -356,7 +356,7 @@ fn reports_scopes_in_existing_benchmark() { Benchmark: bench_scope_exists total: - instructions: 4660 (regressed from 0) + instructions: 4626 (regressed from 0) heap_increase: 1 pages (regressed from 0) stable_memory_increase: 0 pages (no change) @@ -502,7 +502,7 @@ fn reports_repeated_scope_in_new_benchmark() { Benchmark: bench_repeated_scope_new (new) total: - instructions: 17.12 K (new) + instructions: 16.97 K (new) heap_increase: 1 pages (new) stable_memory_increase: 0 pages (new) @@ -529,7 +529,7 @@ fn reports_repeated_scope_in_existing_benchmark() { Benchmark: bench_repeated_scope_exists total: - instructions: 17.12 K (regressed from 0) + instructions: 16.97 K (regressed from 0) heap_increase: 1 pages (regressed from 0) stable_memory_increase: 0 pages (no change)