From a8d3a2c25a86909b617a5dd48f424c50844ccb4f Mon Sep 17 00:00:00 2001 From: marcio-diaz Date: Thu, 13 Feb 2020 05:29:27 +0100 Subject: [PATCH 1/8] Init macro --- frame/identity/src/benchmarking.rs | 65 +-------------------- frame/timestamp/src/benchmarking.rs | 59 +------------------ primitives/runtime/src/traits.rs | 88 ++++++++++++++++++++++++++--- 3 files changed, 83 insertions(+), 129 deletions(-) diff --git a/frame/identity/src/benchmarking.rs b/frame/identity/src/benchmarking.rs index 86ec6b30c9de3..56ca9587d9bba 100644 --- a/frame/identity/src/benchmarking.rs +++ b/frame/identity/src/benchmarking.rs @@ -514,7 +514,7 @@ impl BenchmarkingSetup, RawOrigin> for } // The list of available benchmarks for this pallet. -selected_benchmark!( +selected_benchmarks!( AddRegistrar, SetIdentity, SetSubs, @@ -527,66 +527,3 @@ selected_benchmark!( ProvideJudgement, KillIdentity ); - -impl Benchmarking for Module { - fn run_benchmark(extrinsic: Vec, steps: u32, repeat: u32) -> Result, &'static str> { - // Map the input to the selected benchmark. - let selected_benchmark = match extrinsic.as_slice() { - b"add_registrar" => SelectedBenchmark::AddRegistrar, - b"set_identity" => SelectedBenchmark::SetIdentity, - b"set_subs" => SelectedBenchmark::SetSubs, - b"clear_identity" => SelectedBenchmark::ClearIdentity, - b"request_judgement" => SelectedBenchmark::RequestJudgement, - b"cancel_request" => SelectedBenchmark::CancelRequest, - b"set_fee" => SelectedBenchmark::SetFee, - b"set_account_id" => SelectedBenchmark::SetAccountId, - b"set_fields" => SelectedBenchmark::SetFields, - b"provide_judgement" => SelectedBenchmark::ProvideJudgement, - b"kill_identity" => SelectedBenchmark::KillIdentity, - _ => return Err("Could not find extrinsic."), - }; - - // Warm up the DB - sp_io::benchmarking::commit_db(); - sp_io::benchmarking::wipe_db(); - - // first one is set_identity. - let components = , RawOrigin>>::components(&selected_benchmark); - // results go here - let mut results: Vec = Vec::new(); - // Select the component we will be benchmarking. Each component will be benchmarked. - for (name, low, high) in components.iter() { - // 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; - 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; - - // Select the mid value for all the other components. - let c: Vec<(BenchmarkParameter, u32)> = components.iter() - .map(|(n, l, h)| - (*n, if n == name { component_value } else { (h - l) / 2 + l }) - ).collect(); - - // Run the benchmark `repeat` times. - for _ in 0..repeat { - // Set up the externalities environment for the setup we want to benchmark. - let (call, caller) = , RawOrigin>>::instance(&selected_benchmark, &c)?; - // Commit the externalities to the database, flushing the DB cache. - // This will enable worst case scenario for reading from the database. - sp_io::benchmarking::commit_db(); - // Run the benchmark. - let start = sp_io::benchmarking::current_time(); - call.dispatch(caller.into())?; - let finish = sp_io::benchmarking::current_time(); - let elapsed = finish - start; - results.push((c.clone(), elapsed)); - // Wipe the DB back to the genesis state. - sp_io::benchmarking::wipe_db(); - } - } - } - return Ok(results); - } -} diff --git a/frame/timestamp/src/benchmarking.rs b/frame/timestamp/src/benchmarking.rs index 55d6d7e046740..33950128a80f4 100644 --- a/frame/timestamp/src/benchmarking.rs +++ b/frame/timestamp/src/benchmarking.rs @@ -45,61 +45,4 @@ impl BenchmarkingSetup, RawOrigin> for Set { } } -selected_benchmark!(Set); - -impl Benchmarking for Module { - fn run_benchmark(extrinsic: Vec, steps: u32, repeat: u32) -> Result, &'static str> { - // Map the input to the selected benchmark. - let selected_benchmark = match extrinsic.as_slice() { - b"set" => SelectedBenchmark::Set, - _ => return Err("Could not find extrinsic."), - }; - - // Warm up the DB - sp_io::benchmarking::commit_db(); - sp_io::benchmarking::wipe_db(); - - let components = , RawOrigin>>::components(&selected_benchmark); - let mut results: Vec = Vec::new(); - - // Select the component we will be benchmarking. Each component will be benchmarked. - for (name, low, high) in components.iter() { - // 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; - 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; - - // Select the mid value for all the other components. - let c: Vec<(BenchmarkParameter, u32)> = components.iter() - .map(|(n, l, h)| - (*n, if n == name { component_value } else { (h - l) / 2 + l }) - ).collect(); - - // Run the benchmark `repeat` times. - for _ in 0..repeat { - // Set up the externalities environment for the setup we want to benchmark. - let (call, caller) = , - RawOrigin, - >>::instance(&selected_benchmark, &c)?; - // Commit the externalities to the database, flushing the DB cache. - // This will enable worst case scenario for reading from the database. - sp_io::benchmarking::commit_db(); - // Run the benchmark. - let start = sp_io::benchmarking::current_time(); - call.dispatch(caller.into())?; - let finish = sp_io::benchmarking::current_time(); - let elapsed = finish - start; - results.push((c.clone(), elapsed)); - // Wipe the DB back to the genesis state. - sp_io::benchmarking::wipe_db(); - } - } - } - - return Ok(results); - } -} +selected_benchmarks!(Set); diff --git a/primitives/runtime/src/traits.rs b/primitives/runtime/src/traits.rs index c28fe555f7b3f..7958539a10700 100644 --- a/primitives/runtime/src/traits.rs +++ b/primitives/runtime/src/traits.rs @@ -1314,14 +1314,68 @@ pub trait BlockIdTo { } /// The pallet benchmarking trait. -pub trait Benchmarking { +pub trait Benchmarking +where ::Origin: From, +{ + /// Benchmarking setup for an extrinsic. + type Setup: BenchmarkingSetup; + /// Run the benchmarks for this pallet. /// /// Parameters /// - `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. + /// - `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: u32, repeat: u32) -> Result, u128)>, &'static str> { + let benchmark = Self::select_benchmark(extrinsic)?; + + // Warm up the DB + sp_io::benchmarking::commit_db(); + sp_io::benchmarking::wipe_db(); + + let components = >::components(&benchmark); + let mut results: Vec = Vec::new(); + + // Select the component we will be benchmarking. Each component will be benchmarked. + for (name, low, high) in components.iter() { + // 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; + + 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; + + // Select the mid value for all the other components. + let c: Vec<(BenchmarkParameter, u32)> = components.iter() + .map(|(n, l, h)| + (*n, if n == name { component_value } else { (h - l) / 2 + l }) + ).collect(); + + // Run the benchmark `repeat` times. + for _ in 0..repeat { + // Set up the externalities environment for the setup we want to benchmark. + let (call, caller) = >::instance(&benchmark, &c)?; + // Commit the externalities to the database, flushing the DB cache. + // This will enable worst case scenario for reading from the database. + sp_io::benchmarking::commit_db(); + // Run the benchmark. + let start = sp_io::benchmarking::current_time(); + call.dispatch(caller.into())?; + let finish = sp_io::benchmarking::current_time(); + let elapsed = finish - start; + results.push((c.clone(), elapsed)); + // Wipe the DB back to the genesis state. + sp_io::benchmarking::wipe_db(); + } + } + } + + return Ok(results); + } + + /// Return the benchmarking setup corresponding to `extrinsic`. + fn select_benchmark(extrinsic: Vec) -> Result; } /// The required setup for creating a benchmark. @@ -1345,18 +1399,23 @@ pub trait BenchmarkingSetup { /// struct SetBalance; /// impl BenchmarkingSetup for SetBalance { ... } /// -/// selected_benchmark!(Transfer, SetBalance); +/// selected_benchmarks!(Transfer, SetBalance); /// ``` #[macro_export] -macro_rules! selected_benchmark { +macro_rules! selected_benchmarks { ($($bench:ident),*) => { // The list of available benchmarks for this pallet. - enum SelectedBenchmark { + #[derive(Debug)] + pub enum SelectedBenchmark { $( $bench, )* } // Allow us to select a benchmark from the list of available benchmarks. - impl $crate::traits::BenchmarkingSetup, RawOrigin> for SelectedBenchmark { + impl $crate::traits::BenchmarkingSetup< + T, + Call, + RawOrigin, + > for SelectedBenchmark { fn components(&self) -> Vec<(BenchmarkParameter, u32, u32)> { match self { $( Self::$bench => <$bench as $crate::traits::BenchmarkingSetup< @@ -1379,6 +1438,21 @@ macro_rules! selected_benchmark { } } } + + impl $crate::traits::Benchmarking< + T, + Call, + RawOrigin, + > for Module { + type Setup = SelectedBenchmark; + + fn select_benchmark(extrinsic: Vec) -> Result { + match extrinsic.as_slice() { + $( e @ _ if e == stringify!($bench).as_bytes() => Ok(Self::Setup::$bench), )* + _ => Err("Could not find extrinsic."), + } + } + } }; } From 981d5c358166d63151198d17be72a55bef626296 Mon Sep 17 00:00:00 2001 From: marcio-diaz Date: Tue, 25 Feb 2020 13:40:07 +0100 Subject: [PATCH 2/8] Refactor function. --- frame/vesting/src/benchmarking.rs | 54 +++++++++++++------------------ 1 file changed, 22 insertions(+), 32 deletions(-) diff --git a/frame/vesting/src/benchmarking.rs b/frame/vesting/src/benchmarking.rs index 9c12302b73139..f02d30da95e5d 100644 --- a/frame/vesting/src/benchmarking.rs +++ b/frame/vesting/src/benchmarking.rs @@ -39,20 +39,7 @@ fn add_locks(l: u32) { } } -benchmarks! { - _ { - // Current block. It allows to hit different paths of `update_lock`. - // It doesn't seems to influence the timings which branch is taken. - let b in 0 .. 1 => (); - // Number of previous locks. - // It doesn't seems to influence the timings for lower values. - let l in 0 .. MAX_LOCKS => add_locks::(l); - } - - vest { - let b in ...; - let l in ...; - +fn setup(b: u32) -> T::AccountId { let locked = 1; let per_block = 1; let starting_block = 0; @@ -67,37 +54,40 @@ benchmarks! { starting_block.into(), ); + // Set lock and block number to take different code paths. let reasons = WithdrawReason::Transfer | WithdrawReason::Reserve; T::Currency::set_lock(VESTING_ID, &caller, locked.into(), reasons); - System::::set_block_number(b.into()); + caller +} + +benchmarks! { + _ { + // Current block. It allows to hit different paths of `update_lock`. + // It doesn't seems to influence the timings which branch is taken. + let b in 0 .. 1 => (); + // Number of previous locks. + // It doesn't seems to influence the timings for lower values. + let l in 0 .. MAX_LOCKS => add_locks::(l); + } + + vest { + let b in ...; + let l in ...; + + let caller = setup::(b); + }: _(RawOrigin::Signed(caller)) vest_other { let b in ...; let l in ...; - let locked = 1; - let per_block = 1; - let starting_block = 0; - - let caller = account("caller", 0, SEED); let other: T::AccountId = account("other", 0, SEED); let other_lookup: ::Source = T::Lookup::unlookup(other.clone()); - // Add schedule to avoid `NotVesting` error. - let _ = Vesting::::add_vesting_schedule( - &other, - locked.into(), - per_block.into(), - starting_block.into(), - ); - - let reasons = WithdrawReason::Transfer | WithdrawReason::Reserve; - T::Currency::set_lock(VESTING_ID, &caller, locked.into(), reasons); - - System::::set_block_number(b.into()); + let caller = setup::(b); }: _(RawOrigin::Signed(caller), other_lookup) } From a19302ab42f1b49a8014445678ce9c7df16dcafd Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Sat, 7 Mar 2020 14:12:11 +0100 Subject: [PATCH 3/8] Add feature --- frame/vesting/Cargo.toml | 3 ++- frame/vesting/src/benchmarking.rs | 8 +++++++- frame/vesting/src/lib.rs | 1 + 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/frame/vesting/Cargo.toml b/frame/vesting/Cargo.toml index 11971787dd05d..99a64a92b8fe8 100644 --- a/frame/vesting/Cargo.toml +++ b/frame/vesting/Cargo.toml @@ -17,7 +17,7 @@ sp-io = { version = "2.0.0-alpha.2", default-features = false, path = "../../pri sp-runtime = { version = "2.0.0-alpha.2", default-features = false, path = "../../primitives/runtime" } frame-support = { version = "2.0.0-alpha.2", default-features = false, path = "../support" } frame-system = { version = "2.0.0-alpha.2", default-features = false, path = "../system" } -frame-benchmarking = { version = "2.0.0-alpha.2", default-features = false, path = "../benchmarking" } +frame-benchmarking = { version = "2.0.0-alpha.2", default-features = false, path = "../benchmarking", optional = true } [dev-dependencies] sp-core = { version = "2.0.0-alpha.2", path = "../../primitives/core" } @@ -36,3 +36,4 @@ std = [ "frame-support/std", "frame-system/std", ] +runtime-benchmarks = ["frame-benchmarking"] diff --git a/frame/vesting/src/benchmarking.rs b/frame/vesting/src/benchmarking.rs index f02d30da95e5d..4474f3be02588 100644 --- a/frame/vesting/src/benchmarking.rs +++ b/frame/vesting/src/benchmarking.rs @@ -21,7 +21,6 @@ use super::*; use frame_system::{RawOrigin, Module as System}; use sp_io::hashing::blake2_256; use frame_benchmarking::{benchmarks, account}; -use sp_runtime::traits::Dispatchable; use crate::Module as Vesting; @@ -90,4 +89,11 @@ benchmarks! { let caller = setup::(b); }: _(RawOrigin::Signed(caller), other_lookup) + + // vested_transfer{ + // let u in 0 .. 1000; + // let caller = account("caller", 0, SEED); + // let _ = T::Currency::make_free_balance_be(&caller, balance); + + // }: } diff --git a/frame/vesting/src/lib.rs b/frame/vesting/src/lib.rs index 172cec26bcb1a..edd93bf05ab97 100644 --- a/frame/vesting/src/lib.rs +++ b/frame/vesting/src/lib.rs @@ -60,6 +60,7 @@ use frame_support::traits::{ use frame_support::weights::SimpleDispatchInfo; use frame_system::{self as system, ensure_signed}; +#[cfg(features = "runtime-benchmarks")] pub mod benchmarking; type BalanceOf = <::Currency as Currency<::AccountId>>::Balance; From 5108a38bb762c18a63d438ed7fa6e29f67a50434 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Sat, 7 Mar 2020 14:29:16 +0100 Subject: [PATCH 4/8] vested transfer benchmark --- frame/vesting/src/benchmarking.rs | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/frame/vesting/src/benchmarking.rs b/frame/vesting/src/benchmarking.rs index 4474f3be02588..347183d5b1727 100644 --- a/frame/vesting/src/benchmarking.rs +++ b/frame/vesting/src/benchmarking.rs @@ -90,10 +90,18 @@ benchmarks! { }: _(RawOrigin::Signed(caller), other_lookup) - // vested_transfer{ - // let u in 0 .. 1000; - // let caller = account("caller", 0, SEED); - // let _ = T::Currency::make_free_balance_be(&caller, balance); - - // }: + vested_transfer{ + let u in 0 .. 1000; + let from = account("from", u, SEED); + let to = account("to", u, SEED); + let to_lookup: ::Source = T::Lookup::unlookup(to); + let transfer_amt = T::MinVestedTransfer::get(); + let vesting_schedule = VestingInfo { + locked: transfer_amt, + per_block: 1.into(), + starting_block: 0.into(), + }; + let _ = T::Currency::make_free_balance_be(&from, transfer_amt * 10.into()); + + }: _(RawOrigin::Signed(from), to_lookup, vesting_schedule) } From 1c6ab60299f694044aa061b40a38ad25301d7553 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Sat, 7 Mar 2020 14:41:01 +0100 Subject: [PATCH 5/8] Fix features --- bin/node/runtime/Cargo.toml | 1 + frame/vesting/Cargo.toml | 2 +- frame/vesting/src/lib.rs | 4 ++-- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/bin/node/runtime/Cargo.toml b/bin/node/runtime/Cargo.toml index 0d65cf5339536..15672715a415d 100644 --- a/bin/node/runtime/Cargo.toml +++ b/bin/node/runtime/Cargo.toml @@ -137,4 +137,5 @@ runtime-benchmarks = [ "pallet-timestamp/runtime-benchmarks", "pallet-identity/runtime-benchmarks", "pallet-balances/runtime-benchmarks", + "pallet-vesting/runtime-benchmarks", ] diff --git a/frame/vesting/Cargo.toml b/frame/vesting/Cargo.toml index 99a64a92b8fe8..882c062c43835 100644 --- a/frame/vesting/Cargo.toml +++ b/frame/vesting/Cargo.toml @@ -36,4 +36,4 @@ std = [ "frame-support/std", "frame-system/std", ] -runtime-benchmarks = ["frame-benchmarking"] +runtime-benchmarks = ["frame-benchmarking", "frame-system/runtime-benchmarks"] diff --git a/frame/vesting/src/lib.rs b/frame/vesting/src/lib.rs index edd93bf05ab97..223b840678617 100644 --- a/frame/vesting/src/lib.rs +++ b/frame/vesting/src/lib.rs @@ -60,8 +60,8 @@ use frame_support::traits::{ use frame_support::weights::SimpleDispatchInfo; use frame_system::{self as system, ensure_signed}; -#[cfg(features = "runtime-benchmarks")] -pub mod benchmarking; +#[cfg(feature = "runtime-benchmarks")] +mod benchmarking; type BalanceOf = <::Currency as Currency<::AccountId>>::Balance; From cbeb16644963b6d51f6c2cb1898589c3ab1b5b41 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Sat, 7 Mar 2020 17:15:01 +0100 Subject: [PATCH 6/8] Forgot to push this fix --- frame/vesting/src/benchmarking.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/frame/vesting/src/benchmarking.rs b/frame/vesting/src/benchmarking.rs index 347183d5b1727..deb636a807d8b 100644 --- a/frame/vesting/src/benchmarking.rs +++ b/frame/vesting/src/benchmarking.rs @@ -83,11 +83,10 @@ benchmarks! { let b in ...; let l in ...; - let other: T::AccountId = account("other", 0, SEED); + let other: T::AccountId = setup::(b); let other_lookup: ::Source = T::Lookup::unlookup(other.clone()); - let caller = setup::(b); - + let caller = account("caller", 0, SEED); }: _(RawOrigin::Signed(caller), other_lookup) vested_transfer{ From 8276b093f0beac1f0d086bf2b1f9da6bf26e49c3 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Sat, 7 Mar 2020 17:20:26 +0100 Subject: [PATCH 7/8] bump impl --- 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 510e25fd8ee33..796782a102bf6 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -83,7 +83,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { // implementation changes and behavior does not, then leave spec_version as // is and increment impl_version. spec_version: 233, - impl_version: 0, + impl_version: 1, apis: RUNTIME_API_VERSIONS, }; From 67222a597b1d767d269c7794e7f173eda4e3b2f7 Mon Sep 17 00:00:00 2001 From: marcio-diaz Date: Sun, 8 Mar 2020 20:12:40 +0100 Subject: [PATCH 8/8] Nits. --- frame/vesting/src/benchmarking.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/frame/vesting/src/benchmarking.rs b/frame/vesting/src/benchmarking.rs index deb636a807d8b..79ab0cb6e329e 100644 --- a/frame/vesting/src/benchmarking.rs +++ b/frame/vesting/src/benchmarking.rs @@ -87,20 +87,25 @@ benchmarks! { let other_lookup: ::Source = T::Lookup::unlookup(other.clone()); let caller = account("caller", 0, SEED); + }: _(RawOrigin::Signed(caller), other_lookup) - vested_transfer{ + vested_transfer { let u in 0 .. 1000; + let from = account("from", u, SEED); let to = account("to", u, SEED); let to_lookup: ::Source = T::Lookup::unlookup(to); - let transfer_amt = T::MinVestedTransfer::get(); + + let transfer_amount = T::MinVestedTransfer::get(); + let vesting_schedule = VestingInfo { - locked: transfer_amt, + locked: transfer_amount, per_block: 1.into(), starting_block: 0.into(), }; - let _ = T::Currency::make_free_balance_be(&from, transfer_amt * 10.into()); + + let _ = T::Currency::make_free_balance_be(&from, transfer_amount * 10.into()); }: _(RawOrigin::Signed(from), to_lookup, vesting_schedule) }