some bencher improvements (#758)

* some bencher improvements

* fmt

* clippy

Author: ermalkaleci

bencher/Cargo.toml

@@ -19,6 +19,7 @@ wasm-gc-api = { version = "0.1.11", optional = true }
 rand = {version = "0.8.3", optional = true }
 linregress = { version = "0.4.4", optional = true }
 parking_lot = { version = "0.12.0", optional = true }
+thiserror = { version = "1.0", optional = true }
 serde = { version = "1.0.136", optional = true, features = ['derive'] }
 serde_json = {version = "1.0.68", optional = true }
 hash-db = { version = "0.15.2", default-features = false, optional = true }
@@ -52,6 +53,7 @@ std = [
     "rand",
     "linregress",
     "parking_lot",
+    "thiserror",
     "serde/std",
     "serde_json/std",
     "hash-db/std",

bencher/src/ext.rs bencher/src/bench_ext.rsbencher/src/ext.rs renamed to bencher/src/bench_ext.rs

@@ -37,57 +37,57 @@ where
 	}
 
 	fn storage(&self, key: &[u8]) -> Option<Vec<u8>> {
-		self.tracker.reading_key(key.to_vec());
+		self.tracker.on_read_storage(key.to_vec());
 		self.ext.storage(key)
 	}
 
 	fn storage_hash(&self, key: &[u8]) -> Option<Vec<u8>> {
-		self.tracker.reading_key(key.to_vec());
+		self.tracker.on_read_storage(key.to_vec());
 		self.ext.storage_hash(key)
 	}
 
 	fn child_storage_hash(&self, child_info: &ChildInfo, key: &[u8]) -> Option<Vec<u8>> {
-		self.tracker.reading_child_key(child_info, key.to_vec());
+		self.tracker.on_read_child_storage(child_info, key.to_vec());
 		self.ext.child_storage_hash(child_info, key)
 	}
 
 	fn child_storage(&self, child_info: &ChildInfo, key: &[u8]) -> Option<Vec<u8>> {
-		self.tracker.reading_child_key(child_info, key.to_vec());
+		self.tracker.on_read_child_storage(child_info, key.to_vec());
 		self.ext.child_storage(child_info, key)
 	}
 
 	fn next_storage_key(&self, key: &[u8]) -> Option<Vec<u8>> {
-		self.tracker.reading_key(key.to_vec());
+		self.tracker.on_read_storage(key.to_vec());
 		self.ext.next_storage_key(key)
 	}
 
 	fn next_child_storage_key(&self, child_info: &ChildInfo, key: &[u8]) -> Option<Vec<u8>> {
-		self.tracker.reading_child_key(child_info, key.to_vec());
+		self.tracker.on_read_child_storage(child_info, key.to_vec());
 		self.ext.next_child_storage_key(child_info, key)
 	}
 
 	fn kill_child_storage(&mut self, child_info: &ChildInfo, limit: Option<u32>) -> (bool, u32) {
-		self.tracker.warn_child_prefix_removal();
+		self.tracker.on_kill_child_storage(child_info, limit);
 		self.ext.kill_child_storage(child_info, limit)
 	}
 
 	fn clear_prefix(&mut self, prefix: &[u8], limit: Option<u32>) -> (bool, u32) {
-		self.tracker.warn_child_prefix_removal();
+		self.tracker.on_clear_prefix(prefix, limit);
 		self.ext.clear_prefix(prefix, limit)
 	}
 
 	fn clear_child_prefix(&mut self, child_info: &ChildInfo, prefix: &[u8], limit: Option<u32>) -> (bool, u32) {
-		self.tracker.warn_child_prefix_removal();
+		self.tracker.on_clear_child_prefix(child_info, prefix, limit);
 		self.ext.clear_child_prefix(child_info, prefix, limit)
 	}
 
 	fn place_storage(&mut self, key: Vec<u8>, value: Option<Vec<u8>>) {
-		self.tracker.changing_key(key.clone());
+		self.tracker.on_update_storage(key.clone());
 		self.ext.place_storage(key, value);
 	}
 
 	fn place_child_storage(&mut self, child_info: &ChildInfo, key: Vec<u8>, value: Option<Vec<u8>>) {
-		self.tracker.changing_child_key(child_info, key.clone());
+		self.tracker.on_update_child_storage(child_info, key.clone());
 		self.ext.place_child_storage(child_info, key, value);
 	}
 
@@ -100,7 +100,8 @@ where
 	}
 
 	fn storage_append(&mut self, key: Vec<u8>, value: Vec<u8>) {
-		self.tracker.changing_key(key.clone());
+		self.tracker.on_read_storage(key.clone());
+		self.tracker.on_update_storage(key.clone());
 		self.ext.storage_append(key, value);
 	}
 

bencher/src/bench_runner.rs

@@ -1,5 +1,5 @@
 use super::{
-	ext::BenchExt,
+	bench_ext::BenchExt,
 	tracker::{BenchTracker, BenchTrackerExt},
 };
 use frame_benchmarking::frame_support::sp_runtime::traits::Block;

bencher/src/bencher.rs

@@ -5,8 +5,6 @@ use sp_std::prelude::Vec;
 pub struct BenchResult {
 	pub method: Vec<u8>,
 	pub elapses: Vec<u128>,
-	pub reads: u32,
-	pub writes: u32,
 	pub keys: Vec<u8>,
 }
 
@@ -39,7 +37,7 @@ impl Bencher {
 		crate::bench::whitelist(key, read, write);
 	}
 
-	pub fn prepare(&self) {
+	pub fn before_run(&self) {
 		#[cfg(not(feature = "std"))]
 		{
 			frame_benchmarking::benchmarking::commit_db();
@@ -55,22 +53,17 @@ impl Bencher {
 		{
 			frame_benchmarking::benchmarking::commit_db();
 			frame_benchmarking::benchmarking::reset_read_write_count();
-			crate::bench::prepare();
-			crate::bench::instant();
+			crate::bench::start_timer();
 		}
 
 		let ret = black_box(inner());
 
 		#[cfg(not(feature = "std"))]
 		{
-			let elapsed = crate::bench::elapsed().saturating_sub(crate::bench::redundant_time());
+			let elapsed = crate::bench::end_timer().saturating_sub(crate::bench::redundant_time());
 			self.current.elapses.push(elapsed);
 
 			frame_benchmarking::benchmarking::commit_db();
-			let (reads, _, written, _) = frame_benchmarking::benchmarking::read_write_count();
-
-			self.current.reads = reads;
-			self.current.writes = written;
 
 			// changed keys
 			self.current.keys = crate::bench::read_written_keys();

bencher/src/handler.rs

@@ -6,6 +6,7 @@ use codec::Decode;
 use frame_benchmarking::frame_support::traits::StorageInfo;
 use linregress::{FormulaRegressionBuilder, RegressionDataBuilder};
 use serde::{Deserialize, Serialize};
+use sp_core::hexdisplay::HexDisplay;
 use std::io::Write;
 use std::time::Duration;
 
@@ -46,34 +47,38 @@ pub fn handle(output: Vec<u8>, storage_infos: Vec<StorageInfo>) {
 			let mut total_writes = 0u32;
 			let mut comments = Vec::<String>::new();
 			let keys = <Vec<(Vec<u8>, u32, u32)> as Decode>::decode(&mut &result.keys[..]).unwrap();
-			keys.iter().for_each(|(prefix, reads, writes)| {
+			keys.into_iter().for_each(|(prefix, reads, writes)| {
 				total_reads += reads;
 				total_writes += writes;
-				if let Some(info) = storage_infos.iter().find(|x| x.prefix.eq(prefix)) {
+				if let Some(info) = storage_infos.iter().find(|x| x.prefix.eq(&prefix)) {
 					let pallet = String::from_utf8(info.pallet_name.clone()).unwrap();
 					let name = String::from_utf8(info.storage_name.clone()).unwrap();
-					let comment = format!("{}::{} (r: {}, w: {})", pallet, name, reads, writes);
-					comments.push(comment);
+					comments.push(format!("{}::{} (r: {}, w: {})", pallet, name, reads, writes));
 				} else {
-					let comment = format!("Unknown (r: {}, w: {})", reads, writes);
-					comments.push(comment);
+					comments.push(format!(
+						"Unknown 0x{} (r: {}, w: {})",
+						HexDisplay::from(&prefix),
+						reads,
+						writes
+					));
 				}
 			});
 
+			comments.sort();
+
 			println!(
-				"{} {:<40} {:>20} {:<20} {:<20}",
+				"{} {:<40} {:>20} storage: {:<20}",
 				green_bold("Bench"),
 				cyan(&name),
 				green_bold(&format!(
 					"{:?}",
 					Duration::from_nanos(model.parameters.intercept_value as u64)
 				)),
 				green_bold(&format!(
-					"tracked: [r: {}, w: {}]",
+					"[r: {}, w: {}]",
 					&total_reads.to_string(),
 					&total_writes.to_string()
 				)),
-				green_bold(&format!("total: [r: {}, w: {}]", result.reads, result.writes))
 			);
 
 			BenchData {

bencher/src/lib.rs

@@ -26,9 +26,9 @@ pub mod build_wasm;
 pub mod handler;
 
 #[cfg(feature = "std")]
-mod colorize;
+mod bench_ext;
 #[cfg(feature = "std")]
-mod ext;
+mod colorize;
 #[cfg(feature = "std")]
 mod tracker;
 

bencher/src/macros.rs

@@ -35,12 +35,12 @@
 /// use crate::mock::*;
 ///
 /// fn foo(b: &mut Bencher) {
-///     // Run any before code here
+///     // Run anything before code here
 ///     let ret = b.bench(|| {
 ///         // foo must have macro `[orml_weight_meter::weight(..)]` to measure correct redundant info
 ///         YourModule::foo()
 ///     });
-///     // Run any after code here
+///     // Run anything after code here
 /// }
 ///
 /// fn bar(b: &mut Bencher) {
@@ -71,13 +71,13 @@ macro_rules! benches {
                 $crate::bench::print_info("\nRunning benches ...\n".as_bytes().to_vec());
                 let mut bencher = $crate::Bencher::default();
                 $(
-                    $crate::bench::reset();
+                    $crate::bench::init_bench();
 
                     let name = stringify!($method);
                     bencher.current = $crate::BenchResult::with_name(name);
 
                     for _ in 0..1_000 {
-                        bencher.prepare();
+                        bencher.before_run();
                         $method(&mut bencher);
                     }