Custom Benchmark Errors and Override

frame/benchmarking/src/analysis.rs

@@ -17,7 +17,7 @@
 
 //! Tools for analyzing the benchmark results.
 
-use crate::BenchmarkResults;
+use crate::BenchmarkResult;
 use core::convert::TryFrom;
 use linregress::{FormulaRegressionBuilder, RegressionDataBuilder};
 use std::collections::BTreeMap;
@@ -76,7 +76,7 @@ impl TryFrom<Option<String>> for AnalysisChoice {
 impl Analysis {
 	// Useful for when there are no components, and we just need an median value of the benchmark results.
 	// Note: We choose the median value because it is more robust to outliers.
-	fn median_value(r: &Vec<BenchmarkResults>, selector: BenchmarkSelector) -> Option<Self> {
+	fn median_value(r: &Vec<BenchmarkResult>, selector: BenchmarkSelector) -> Option<Self> {
 		if r.is_empty() {
 			return None
 		}
@@ -104,7 +104,7 @@ impl Analysis {
 		})
 	}
 
-	pub fn median_slopes(r: &Vec<BenchmarkResults>, selector: BenchmarkSelector) -> Option<Self> {
+	pub fn median_slopes(r: &Vec<BenchmarkResult>, selector: BenchmarkSelector) -> Option<Self> {
 		if r[0].components.is_empty() {
 			return Self::median_value(r, selector)
 		}
@@ -199,7 +199,7 @@ impl Analysis {
 		})
 	}
 
-	pub fn min_squares_iqr(r: &Vec<BenchmarkResults>, selector: BenchmarkSelector) -> Option<Self> {
+	pub fn min_squares_iqr(r: &Vec<BenchmarkResult>, selector: BenchmarkSelector) -> Option<Self> {
 		if r[0].components.is_empty() {
 			return Self::median_value(r, selector)
 		}
@@ -279,7 +279,7 @@ impl Analysis {
 		})
 	}
 
-	pub fn max(r: &Vec<BenchmarkResults>, selector: BenchmarkSelector) -> Option<Self> {
+	pub fn max(r: &Vec<BenchmarkResult>, selector: BenchmarkSelector) -> Option<Self> {
 		let median_slopes = Self::median_slopes(r, selector);
 		let min_squares = Self::min_squares_iqr(r, selector);
 
@@ -402,8 +402,8 @@ mod tests {
 		storage_root_time: u128,
 		reads: u32,
 		writes: u32,
-	) -> BenchmarkResults {
-		BenchmarkResults {
+	) -> BenchmarkResult {
+		BenchmarkResult {
 			components,
 			extrinsic_time,
 			storage_root_time,

frame/benchmarking/src/lib.rs

@@ -644,7 +644,7 @@ macro_rules! benchmark_backend {
 				&self,
 				components: &[($crate::BenchmarkParameter, u32)],
 				verify: bool
-			) -> Result<$crate::Box<dyn FnOnce() -> Result<(), &'static str>>, &'static str> {
+			) -> Result<$crate::Box<dyn FnOnce() -> Result<(), $crate::BenchmarkError>>, &'static str> {
 				$(
 					// Prepare instance
 					let $param = components.iter()
@@ -658,7 +658,7 @@ macro_rules! benchmark_backend {
 				$( $param_instancer ; )*
 				$( $post )*
 
-				Ok($crate::Box::new(move || -> Result<(), &'static str> {
+				Ok($crate::Box::new(move || -> Result<(), $crate::BenchmarkError> {
 					$eval;
 					if verify {
 						$postcode;
@@ -717,7 +717,7 @@ macro_rules! selected_benchmark {
 				&self,
 				components: &[($crate::BenchmarkParameter, u32)],
 				verify: bool
-			) -> Result<$crate::Box<dyn FnOnce() -> Result<(), &'static str>>, &'static str> {
+			) -> Result<$crate::Box<dyn FnOnce() -> Result<(), $crate::BenchmarkError>>, &'static str> {
 				match self {
 					$(
 						Self::$bench => <
@@ -741,7 +741,7 @@ macro_rules! impl_benchmark {
 		( $( $name_skip_meta:ident ),* )
 	) => {
 		impl<T: Config $(<$instance>, $instance: $instance_bound )? >
-			$crate::Benchmarking<$crate::BenchmarkResults> for Pallet<T $(, $instance)? >
+			$crate::Benchmarking for Pallet<T $(, $instance)? >
 			where T: frame_system::Config, $( $where_clause )*
 		{
 			fn benchmarks(extra: bool) -> $crate::Vec<$crate::BenchmarkMetadata> {
@@ -772,13 +772,13 @@ macro_rules! impl_benchmark {
 				whitelist: &[$crate::TrackedStorageKey],
 				verify: bool,
 				internal_repeats: u32,
-			) -> Result<$crate::Vec<$crate::BenchmarkResults>, &'static str> {
+			) -> Result<$crate::Vec<$crate::BenchmarkResult>, $crate::BenchmarkError> {
 				// Map the input to the selected benchmark.
 				let extrinsic = $crate::sp_std::str::from_utf8(extrinsic)
 					.map_err(|_| "`extrinsic` is not a valid utf8 string!")?;
 				let selected_benchmark = match extrinsic {
 					$( stringify!($name) => SelectedBenchmark::$name, )*
-					_ => return Err("Could not find extrinsic."),
+					_ => return Err("Could not find extrinsic.".into()),
 				};
 
 				// Add whitelist to DB including whitelisted caller
@@ -790,7 +790,7 @@ macro_rules! impl_benchmark {
 				whitelist.push(whitelisted_caller_key.into());
 				$crate::benchmarking::set_whitelist(whitelist);
 
-				let mut results: $crate::Vec<$crate::BenchmarkResults> = $crate::Vec::new();
+				let mut results: $crate::Vec<$crate::BenchmarkResult> = $crate::Vec::new();
 
 				// Always do at least one internal repeat...
 				for _ in 0 .. internal_repeats.max(1) {
@@ -852,13 +852,13 @@ macro_rules! impl_benchmark {
 					let elapsed_storage_root = finish_storage_root - start_storage_root;
 
 					let skip_meta = [ $( stringify!($name_skip_meta).as_ref() ),* ];
-					let read_and_written_keys = if (&skip_meta).contains(&extrinsic) {
+					let read_and_written_keys = if skip_meta.contains(&extrinsic) {
 						$crate::vec![(b"Skipped Metadata".to_vec(), 0, 0, false)]
 					} else {
 						$crate::benchmarking::get_read_and_written_keys()
 					};
 
-					results.push($crate::BenchmarkResults {
+					results.push($crate::BenchmarkResult {
 						components: c.to_vec(),
 						extrinsic_time: elapsed_extrinsic,
 						storage_root_time: elapsed_storage_root,
@@ -893,14 +893,14 @@ macro_rules! impl_benchmark {
 			/// by the `impl_benchmark_test_suite` macro. However, it is not an error if a pallet
 			/// author chooses not to implement benchmarks.
 			#[allow(unused)]
-			fn test_bench_by_name(name: &[u8]) -> Result<(), &'static str> {
+			fn test_bench_by_name(name: &[u8]) -> Result<(), $crate::BenchmarkError> {
 				let name = $crate::sp_std::str::from_utf8(name)
-					.map_err(|_| "`name` is not a valid utf8 string!")?;
+					.map_err(|_| -> $crate::BenchmarkError { "`name` is not a valid utf8 string!".into() })?;
 				match name {
 					$( stringify!($name) => {
 						$crate::paste::paste! { Self::[< test_benchmark_ $name >]() }
 					} )*
-					_ => Err("Could not find test for requested benchmark."),
+					_ => Err("Could not find test for requested benchmark.".into()),
 				}
 			}
 		}
@@ -925,15 +925,15 @@ macro_rules! impl_benchmark_test {
 			where T: frame_system::Config, $( $where_clause )*
 			{
 				#[allow(unused)]
-				fn [<test_benchmark_ $name>] () -> Result<(), &'static str> {
+				fn [<test_benchmark_ $name>] () -> Result<(), $crate::BenchmarkError> {
 					let selected_benchmark = SelectedBenchmark::$name;
 					let components = <
 						SelectedBenchmark as $crate::BenchmarkingSetup<T, _>
 					>::components(&selected_benchmark);
 
 					let execute_benchmark = |
 						c: $crate::Vec<($crate::BenchmarkParameter, u32)>
-					| -> Result<(), &'static str> {
+					| -> Result<(), $crate::BenchmarkError> {
 						// Set up the benchmark, return execution + verification function.
 						let closure_to_verify = <
 							SelectedBenchmark as $crate::BenchmarkingSetup<T, _>
@@ -1211,8 +1211,15 @@ macro_rules! impl_benchmark_test_suite {
 								anything_failed = true;
 							},
 							Ok(Err(err)) => {
-								println!("{}: {}", String::from_utf8_lossy(benchmark_name), err);
-								anything_failed = true;
+								match err {
+									$crate::BenchmarkError::Stop(err) => {
+										println!("{}: {:?}", String::from_utf8_lossy(benchmark_name), err);
+										anything_failed = true;
+									},
+									$crate::BenchmarkError::Override(_) => {
+										// This is still considered a success condition.
+									}
+								}
 							},
 							Ok(Ok(_)) => (),
 						}
@@ -1326,25 +1333,40 @@ macro_rules! add_benchmark {
 			internal_repeats,
 		} = config;
 		if &pallet[..] == &name_string[..] {
-			$batches.push($crate::BenchmarkBatch {
-				pallet: name_string.to_vec(),
-				instance: instance_string.to_vec(),
-				benchmark: benchmark.clone(),
-				results: $( $location )*::run_benchmark(
-					&benchmark[..],
-					&selected_components[..],
-					whitelist,
-					*verify,
-					*internal_repeats,
-				).map_err(|e| {
+			let benchmark_result = $( $location )*::run_benchmark(
+				&benchmark[..],
+				&selected_components[..],
+				whitelist,
+				*verify,
+				*internal_repeats,
+			);
+
+			let final_results = match benchmark_result {
+				Ok(results) => results,
+				Err($crate::BenchmarkError::Override(mut result)) => {
+					// Insert override warning as the first storage key.
+					result.keys.insert(0,
+						(b"Benchmark Override".to_vec(), 0, 0, false)
+					);
+					$crate::vec![result]
+				},
+				Err($crate::BenchmarkError::Stop(e)) => {
 					$crate::show_benchmark_debug_info(
 						instance_string,
 						benchmark,
 						selected_components,
 						verify,
 						e,
-					)
-				})?
+					);
+					return Err(e.into());
+				},
+			};
+
+			$batches.push($crate::BenchmarkBatch {
+				pallet: name_string.to_vec(),
+				instance: instance_string.to_vec(),
+				benchmark: benchmark.clone(),
+				results: final_results,
 			});
 		}
 	)