From a1adbca9b747b4864aca02a66113dda9cfa564af Mon Sep 17 00:00:00 2001 From: marcio-diaz Date: Mon, 24 Feb 2020 15:42:01 +0100 Subject: [PATCH 1/3] Add steps setting to CLI, use max value to hit worst case. --- bin/node/runtime/src/lib.rs | 2 +- frame/benchmarking/src/lib.rs | 18 ++++++++++++------ frame/benchmarking/src/utils.rs | 4 ++-- utils/frame/benchmarking-cli/src/lib.rs | 12 +++++++++--- 4 files changed, 24 insertions(+), 12 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index b6be9335ca94c..f46626f4628bd 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -819,7 +819,7 @@ impl_runtime_apis! { fn dispatch_benchmark( module: Vec, extrinsic: Vec, - steps: u32, + steps: Vec, repeat: u32, ) -> Option> { use frame_benchmarking::Benchmarking; diff --git a/frame/benchmarking/src/lib.rs b/frame/benchmarking/src/lib.rs index a78d59ef33959..0fd4025c60299 100644 --- a/frame/benchmarking/src/lib.rs +++ b/frame/benchmarking/src/lib.rs @@ -138,7 +138,7 @@ macro_rules! impl_benchmark { $( $name:ident ),* ) => { impl $crate::Benchmarking<$crate::BenchmarkResults> for Module { - fn run_benchmark(extrinsic: Vec, steps: u32, repeat: u32) -> Result, &'static str> { + fn run_benchmark(extrinsic: Vec, steps: Vec, repeat: u32) -> Result, &'static str> { // Map the input to the selected benchmark. let extrinsic = sp_std::str::from_utf8(extrinsic.as_slice()) .map_err(|_| "Could not find extrinsic")?; @@ -151,15 +151,21 @@ macro_rules! impl_benchmark { $crate::benchmarking::commit_db(); $crate::benchmarking::wipe_db(); - // first one is set_identity. let components = , RawOrigin>>::components(&selected_benchmark); - // results go here let mut results: Vec<$crate::BenchmarkResults> = Vec::new(); + + // Default number of steps for a component. + let mut prev_steps = &10; + // Select the component we will be benchmarking. Each component will be benchmarked. - for (name, low, high) in components.iter() { + for (idx, (name, low, high)) in components.iter().enumerate() { + // Get the number of steps for this component. + let steps = steps.get(idx).unwrap_or(&prev_steps); + prev_steps = steps; + // Create up to `STEPS` steps for that component between high and low. let step_size = ((high - low) / steps).max(1); - let num_of_steps = (high - low) / step_size; + let num_of_steps = (high - low) / step_size + 1; for s in 0..num_of_steps { // This is the value we will be testing for component `name` let component_value = low + step_size * s; @@ -167,7 +173,7 @@ macro_rules! impl_benchmark { // Select the mid value for all the other components. let c: Vec<($crate::BenchmarkParameter, u32)> = components.iter() .map(|(n, l, h)| - (*n, if n == name { component_value } else { (h - l) / 2 + l }) + (*n, if n == name { component_value } else { *h }) ).collect(); // Run the benchmark `repeat` times. diff --git a/frame/benchmarking/src/utils.rs b/frame/benchmarking/src/utils.rs index 81e87a45f890d..6fcbb2817a7ad 100644 --- a/frame/benchmarking/src/utils.rs +++ b/frame/benchmarking/src/utils.rs @@ -40,7 +40,7 @@ sp_api::decl_runtime_apis! { fn dispatch_benchmark( module: Vec, extrinsic: Vec, - steps: u32, + steps: Vec, repeat: u32, ) -> Option>; } @@ -78,7 +78,7 @@ pub trait Benchmarking { /// - `extrinsic`: The name of extrinsic function you want to benchmark encoded as bytes. /// - `steps`: The number of sample points you want to take across the range of parameters. /// - `repeat`: The number of times you want to repeat a benchmark. - fn run_benchmark(extrinsic: Vec, steps: u32, repeat: u32) -> Result, &'static str>; + fn run_benchmark(extrinsic: Vec, steps: Vec, repeat: u32) -> Result, &'static str>; } /// The required setup for creating a benchmark. diff --git a/utils/frame/benchmarking-cli/src/lib.rs b/utils/frame/benchmarking-cli/src/lib.rs index b515e490131f6..0cc9eaa998c6f 100644 --- a/utils/frame/benchmarking-cli/src/lib.rs +++ b/utils/frame/benchmarking-cli/src/lib.rs @@ -36,8 +36,8 @@ pub struct BenchmarkCmd { pub extrinsic: String, /// Select how many samples we should take across the variable components. - #[structopt(short, long, default_value = "1")] - pub steps: u32, + #[structopt(short, long)] + pub steps: String, /// Select how many repetitions of this benchmark should run. #[structopt(short, long, default_value = "1")] @@ -97,13 +97,19 @@ impl BenchmarkCmd { wasm_method, None, // heap pages ); + + let steps: Vec = self.steps + .split(',') + .map(|step| step.parse::().expect("failed to parse step")) + .collect(); + let result = StateMachine::<_, _, NumberFor, _>::new( &state, None, &mut changes, &executor, "Benchmark_dispatch_benchmark", - &(&self.pallet, &self.extrinsic, self.steps, self.repeat).encode(), + &(&self.pallet, &self.extrinsic, steps, self.repeat).encode(), Default::default(), ) .execute(strategy.into()) From 142156beccdd4e3a9a36ae254b77faf09655f4da Mon Sep 17 00:00:00 2001 From: marcio-diaz Date: Mon, 24 Feb 2020 17:31:54 +0100 Subject: [PATCH 2/3] Bump impl_version. --- bin/node/runtime/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index f46626f4628bd..90d3677bee428 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -82,7 +82,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { // implementation changes and behavior does not, then leave spec_version as // is and increment impl_version. spec_version: 224, - impl_version: 2, + impl_version: 3, apis: RUNTIME_API_VERSIONS, }; From 172de0c8ac3291505b5fd55c257ece2a077a0f6a Mon Sep 17 00:00:00 2001 From: marcio-diaz Date: Mon, 24 Feb 2020 20:03:05 +0100 Subject: [PATCH 3/3] Apply review suggestion. --- utils/frame/benchmarking-cli/src/lib.rs | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/utils/frame/benchmarking-cli/src/lib.rs b/utils/frame/benchmarking-cli/src/lib.rs index 0cc9eaa998c6f..3fc20277cec14 100644 --- a/utils/frame/benchmarking-cli/src/lib.rs +++ b/utils/frame/benchmarking-cli/src/lib.rs @@ -36,8 +36,8 @@ pub struct BenchmarkCmd { pub extrinsic: String, /// Select how many samples we should take across the variable components. - #[structopt(short, long)] - pub steps: String, + #[structopt(short, long, use_delimiter = true)] + pub steps: Vec, /// Select how many repetitions of this benchmark should run. #[structopt(short, long, default_value = "1")] @@ -98,18 +98,13 @@ impl BenchmarkCmd { None, // heap pages ); - let steps: Vec = self.steps - .split(',') - .map(|step| step.parse::().expect("failed to parse step")) - .collect(); - let result = StateMachine::<_, _, NumberFor, _>::new( &state, None, &mut changes, &executor, "Benchmark_dispatch_benchmark", - &(&self.pallet, &self.extrinsic, steps, self.repeat).encode(), + &(&self.pallet, &self.extrinsic, self.steps.clone(), self.repeat).encode(), Default::default(), ) .execute(strategy.into())