diff --git a/substrate/frame/benchmarking/README.md b/substrate/frame/benchmarking/README.md index f0fe05cc140f2aaf73e14a5de0275e8dc22255f9..6316cd5903c8bb75157a365978a1aded61a7010d 100644 --- a/substrate/frame/benchmarking/README.md +++ b/substrate/frame/benchmarking/README.md @@ -125,6 +125,8 @@ cargo test -p pallet-balances --features runtime-benchmarks > ``` > To solve this, navigate to the folder of the node (`cd bin/node/cli`) or pallet (`cd frame/pallet`) and run the command there. +This will instance each linear component with different values. The number of values per component is set to six and can be changed with the `VALUES_PER_COMPONENT` environment variable. + ## Adding Benchmarks The benchmarks included with each pallet are not automatically added to your node. To actually diff --git a/substrate/frame/benchmarking/src/lib.rs b/substrate/frame/benchmarking/src/lib.rs index 0a0f8eecdcc3dd3be6e70f7711d9f6fd6f22f248..18472595f15b9f12914e02c26556578e89f5d5bf 100644 --- a/substrate/frame/benchmarking/src/lib.rs +++ b/substrate/frame/benchmarking/src/lib.rs @@ -1153,6 +1153,8 @@ macro_rules! impl_benchmark { // This creates a unit test for one benchmark of the main benchmark macro. // It runs the benchmark using the `high` and `low` value for each component // and ensure that everything completes successfully. +// Instances each component with six values which can be controlled with the +// env variable `VALUES_PER_COMPONENT`. #[macro_export] #[doc(hidden)] macro_rules! impl_benchmark_test { @@ -1197,16 +1199,42 @@ macro_rules! impl_benchmark_test { if components.is_empty() { execute_benchmark(Default::default())?; } else { - for (name, low, high) in components.iter() { - // Test only the low and high value, assuming values in the middle - // won't break - for component_value in $crate::vec![low, high] { + let num_values: u32 = if let Ok(ev) = std::env::var("VALUES_PER_COMPONENT") { + ev.parse().map_err(|_| { + $crate::BenchmarkError::Stop( + "Could not parse env var `VALUES_PER_COMPONENT` as u32." + ) + })? + } else { + 6 + }; + + if num_values < 2 { + return Err("`VALUES_PER_COMPONENT` must be at least 2".into()); + } + + for (name, low, high) in components.clone().into_iter() { + // Test the lowest, highest (if its different from the lowest) + // and up to num_values-2 more equidistant values in between. + // For 0..10 and num_values=6 this would mean: [0, 2, 4, 6, 8, 10] + + let mut values = $crate::vec![low]; + let diff = (high - low).min(num_values - 1); + let slope = (high - low) as f32 / diff as f32; + + for i in 1..=diff { + let value = ((low as f32 + slope * i as f32) as u32) + .clamp(low, high); + values.push(value); + } + + for component_value in values { // Select the max value for all the other components. let c: $crate::Vec<($crate::BenchmarkParameter, u32)> = components .iter() .map(|(n, _, h)| - if n == name { - (*n, *component_value) + if *n == name { + (*n, component_value) } else { (*n, *h) } diff --git a/substrate/frame/benchmarking/src/tests.rs b/substrate/frame/benchmarking/src/tests.rs index 9a97250aeb965e9a7f044838cea9ed1537cac1f9..be5c23fff17f696804b9f81e709e7edd66898a73 100644 --- a/substrate/frame/benchmarking/src/tests.rs +++ b/substrate/frame/benchmarking/src/tests.rs @@ -27,6 +27,7 @@ use sp_runtime::{ BuildStorage, }; use sp_std::prelude::*; +use std::cell::RefCell; #[frame_support::pallet] mod pallet_test { @@ -125,10 +126,16 @@ fn new_test_ext() -> sp_io::TestExternalities { GenesisConfig::default().build_storage().unwrap().into() } +thread_local! { + /// Tracks the used components per value. Needs to be a thread local since the + /// benchmarking clears the storage after each run. + static VALUES_PER_COMPONENT: RefCell<Vec<u32>> = RefCell::new(vec![]); +} + // NOTE: This attribute is only needed for the `modify_in_` functions. #[allow(unreachable_code)] mod benchmarks { - use super::{new_test_ext, pallet_test::Value, Test}; + use super::{new_test_ext, pallet_test::Value, Test, VALUES_PER_COMPONENT}; use crate::{account, BenchmarkError, BenchmarkParameter, BenchmarkResult, BenchmarkingSetup}; use frame_support::{assert_err, assert_ok, ensure, traits::Get}; use frame_system::RawOrigin; @@ -247,6 +254,13 @@ mod benchmarks { Value::<T>::set(Some(123)); return Err(BenchmarkError::Stop("Should error")); } + + // Stores all component values in the thread-local storage. + values_per_component { + let n in 0 .. 10; + }: { + VALUES_PER_COMPONENT.with(|v| v.borrow_mut().push(n)); + } } #[test] @@ -394,4 +408,41 @@ mod benchmarks { assert_eq!(Value::<Test>::get(), None); }); } + + /// Test that the benchmarking uses the correct values for each component and + /// that the number of components can be controlled with `VALUES_PER_COMPONENT`. + #[test] + fn test_values_per_component() { + let tests = vec![ + (Some("1"), Err("`VALUES_PER_COMPONENT` must be at least 2".into())), + (Some("asdf"), Err("Could not parse env var `VALUES_PER_COMPONENT` as u32.".into())), + (None, Ok(vec![0, 2, 4, 6, 8, 10])), + (Some("2"), Ok(vec![0, 10])), + (Some("4"), Ok(vec![0, 3, 6, 10])), + (Some("6"), Ok(vec![0, 2, 4, 6, 8, 10])), + (Some("10"), Ok(vec![0, 1, 2, 3, 4, 5, 6, 7, 8, 10])), + (Some("11"), Ok(vec![0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10])), + (Some("99"), Ok(vec![0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10])), + ]; + + for (num, expected) in tests { + run_test_values_per_component(num, expected); + } + } + + /// Helper for [`test_values_per_component`]. + fn run_test_values_per_component(num: Option<&str>, output: Result<Vec<u32>, BenchmarkError>) { + VALUES_PER_COMPONENT.with(|v| v.borrow_mut().clear()); + match num { + Some(n) => std::env::set_var("VALUES_PER_COMPONENT", n), + None => std::env::remove_var("VALUES_PER_COMPONENT"), + } + + new_test_ext().execute_with(|| { + let got = Pallet::<Test>::test_benchmark_values_per_component() + .map(|_| VALUES_PER_COMPONENT.with(|v| v.borrow().clone())); + + assert_eq!(got, output); + }); + } }