diff --git a/bin/node/executor/tests/basic.rs b/bin/node/executor/tests/basic.rs index f6dc1c3e7eae3..0d69b9700168c 100644 --- a/bin/node/executor/tests/basic.rs +++ b/bin/node/executor/tests/basic.rs @@ -312,6 +312,9 @@ fn full_native_block_import_works() { let mut alice_last_known_balance: Balance = Default::default(); let mut fees = t.execute_with(|| transfer_fee(&xt())); + let transfer_weight = default_transfer_call().get_dispatch_info().weight; + let timestamp_weight = pallet_timestamp::Call::set::(Default::default()).get_dispatch_info().weight; + executor_call:: _>( &mut t, "Core_execute_block", @@ -327,9 +330,8 @@ fn full_native_block_import_works() { let events = vec![ EventRecord { phase: Phase::ApplyExtrinsic(0), - // timestamp set call with weight 8_000_000 + 2 read + 1 write event: Event::frame_system(frame_system::RawEvent::ExtrinsicSuccess( - DispatchInfo { weight: 8_000_000 + 2 * 25_000_000 + 1 * 100_000_000, class: DispatchClass::Mandatory, ..Default::default() } + DispatchInfo { weight: timestamp_weight, class: DispatchClass::Mandatory, ..Default::default() } )), topics: vec![], }, @@ -349,9 +351,8 @@ fn full_native_block_import_works() { }, EventRecord { phase: Phase::ApplyExtrinsic(1), - // Balance Transfer 70_000_000 + 1 Read + 1 Write event: Event::frame_system(frame_system::RawEvent::ExtrinsicSuccess( - DispatchInfo { weight: 70_000_000 + 25_000_000 + 100_000_000, ..Default::default() } + DispatchInfo { weight: transfer_weight, ..Default::default() } )), topics: vec![], }, @@ -381,9 +382,8 @@ fn full_native_block_import_works() { let events = vec![ EventRecord { phase: Phase::ApplyExtrinsic(0), - // timestamp set call with weight 8_000_000 + 2 read + 1 write event: Event::frame_system(frame_system::RawEvent::ExtrinsicSuccess( - DispatchInfo { weight: 8_000_000 + 2 * 25_000_000 + 1 * 100_000_000, class: DispatchClass::Mandatory, ..Default::default() } + DispatchInfo { weight: timestamp_weight, class: DispatchClass::Mandatory, ..Default::default() } )), topics: vec![], }, @@ -405,9 +405,8 @@ fn full_native_block_import_works() { }, EventRecord { phase: Phase::ApplyExtrinsic(1), - // Balance Transfer 70_000_000 + 1 Read + 1 Write event: Event::frame_system(frame_system::RawEvent::ExtrinsicSuccess( - DispatchInfo { weight: 70_000_000 + 25_000_000 + 100_000_000, ..Default::default() } + DispatchInfo { weight: transfer_weight, ..Default::default() } )), topics: vec![], }, @@ -429,9 +428,8 @@ fn full_native_block_import_works() { }, EventRecord { phase: Phase::ApplyExtrinsic(2), - // Balance Transfer 70_000_000 + 1 Read + 1 Write event: Event::frame_system(frame_system::RawEvent::ExtrinsicSuccess( - DispatchInfo { weight: 70_000_000 + 25_000_000 + 100_000_000, ..Default::default() } + DispatchInfo { weight: transfer_weight, ..Default::default() } )), topics: vec![], }, diff --git a/bin/node/executor/tests/submit_transaction.rs b/bin/node/executor/tests/submit_transaction.rs index dd599a996a4a8..64c2deedac788 100644 --- a/bin/node/executor/tests/submit_transaction.rs +++ b/bin/node/executor/tests/submit_transaction.rs @@ -192,7 +192,7 @@ fn should_submit_signed_twice_from_all_accounts() { fn submitted_transaction_should_be_valid() { use codec::Encode; use frame_support::storage::StorageMap; - use sp_runtime::transaction_validity::{ValidTransaction, TransactionSource}; + use sp_runtime::transaction_validity::{TransactionSource, TransactionTag}; use sp_runtime::traits::StaticLookup; let mut t = new_test_ext(compact_code_unwrap(), false); @@ -228,14 +228,12 @@ fn submitted_transaction_should_be_valid() { >::insert(&address, account); // check validity - let res = Executive::validate_transaction(source, extrinsic); - - assert_eq!(res.unwrap(), ValidTransaction { - priority: 1_410_710_000_000, - requires: vec![], - provides: vec![(address, 0).encode()], - longevity: 2048, - propagate: true, - }); + let res = Executive::validate_transaction(source, extrinsic).unwrap(); + + // We ignore res.priority since this number can change based on updates to weights and such. + assert_eq!(res.requires, Vec::::new()); + assert_eq!(res.provides, vec![(address, 0).encode()]); + assert_eq!(res.longevity, 2048); + assert_eq!(res.propagate, true); }); } diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 9dae66a127582..373a01b8ea279 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -84,6 +84,9 @@ use impls::{CurrencyToVoteHandler, Author}; pub mod constants; use constants::{time::*, currency::*}; +/// Weights for pallets used in the runtime. +mod weights; + // Make the WASM binary available. #[cfg(feature = "std")] include!(concat!(env!("OUT_DIR"), "/wasm_binary.rs")); @@ -322,7 +325,7 @@ impl pallet_balances::Trait for Runtime { type Event = Event; type ExistentialDeposit = ExistentialDeposit; type AccountStore = frame_system::Module; - type WeightInfo = (); + type WeightInfo = weights::pallet_balances::WeightInfo; } parameter_types! { @@ -1126,6 +1129,7 @@ impl_runtime_apis! { highest_range_values: Vec, steps: Vec, repeat: u32, + extra: bool, ) -> Result, sp_runtime::RuntimeString> { use frame_benchmarking::{Benchmarking, BenchmarkBatch, add_benchmark}; // Trying to add benchmarks directly to the Session Pallet caused cyclic dependency issues. @@ -1157,7 +1161,7 @@ impl_runtime_apis! { ]; let mut batches = Vec::::new(); - let params = (&pallet, &benchmark, &lowest_range_values, &highest_range_values, &steps, repeat, &whitelist); + let params = (&pallet, &benchmark, &lowest_range_values, &highest_range_values, &steps, repeat, &whitelist, extra); add_benchmark!(params, batches, pallet_babe, Babe); add_benchmark!(params, batches, pallet_balances, Balances); diff --git a/bin/node/runtime/src/weights/mod.rs b/bin/node/runtime/src/weights/mod.rs new file mode 100644 index 0000000000000..70e10d5342f36 --- /dev/null +++ b/bin/node/runtime/src/weights/mod.rs @@ -0,0 +1,18 @@ +// Copyright (C) 2020 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! A list of the different weight modules for our runtime. + +pub mod pallet_balances; diff --git a/bin/node/runtime/src/weights/pallet_balances.rs b/bin/node/runtime/src/weights/pallet_balances.rs new file mode 100644 index 0000000000000..21a90a97e63a1 --- /dev/null +++ b/bin/node/runtime/src/weights/pallet_balances.rs @@ -0,0 +1,47 @@ +// Copyright (C) 2020 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Weights for the Balances Pallet + +use frame_support::weights::{Weight, constants::RocksDbWeight as DbWeight}; + +pub struct WeightInfo; +impl pallet_balances::WeightInfo for WeightInfo { + fn transfer() -> Weight { + (65949000 as Weight) + .saturating_add(DbWeight::get().reads(1 as Weight)) + .saturating_add(DbWeight::get().writes(1 as Weight)) + } + fn transfer_keep_alive() -> Weight { + (46665000 as Weight) + .saturating_add(DbWeight::get().reads(1 as Weight)) + .saturating_add(DbWeight::get().writes(1 as Weight)) + } + fn set_balance_creating() -> Weight { + (27086000 as Weight) + .saturating_add(DbWeight::get().reads(1 as Weight)) + .saturating_add(DbWeight::get().writes(1 as Weight)) + } + fn set_balance_killing() -> Weight { + (33424000 as Weight) + .saturating_add(DbWeight::get().reads(1 as Weight)) + .saturating_add(DbWeight::get().writes(1 as Weight)) + } + fn force_transfer() -> Weight { + (65343000 as Weight) + .saturating_add(DbWeight::get().reads(2 as Weight)) + .saturating_add(DbWeight::get().writes(2 as Weight)) + } +} diff --git a/frame/balances/src/benchmarking.rs b/frame/balances/src/benchmarking.rs index a5f8e6fe36cd4..73547fe814aa2 100644 --- a/frame/balances/src/benchmarking.rs +++ b/frame/balances/src/benchmarking.rs @@ -28,46 +28,40 @@ use sp_runtime::traits::Bounded; use crate::Module as Balances; const SEED: u32 = 0; -const MAX_EXISTENTIAL_DEPOSIT: u32 = 1000; -const MAX_USER_INDEX: u32 = 1000; +// existential deposit multiplier +const ED_MULTIPLIER: u32 = 10; + benchmarks! { - _ { - let e in 2 .. MAX_EXISTENTIAL_DEPOSIT => (); - let u in 1 .. MAX_USER_INDEX => (); - } + _ { } // Benchmark `transfer` extrinsic with the worst possible conditions: // * Transfer will kill the sender account. // * Transfer will create the recipient account. transfer { - let u in ...; - let e in ...; - let existential_deposit = T::ExistentialDeposit::get(); - let caller = account("caller", u, SEED); + let caller = account("caller", 0, SEED); // Give some multiple of the existential deposit + creation fee + transfer fee - let balance = existential_deposit.saturating_mul(e.into()); + let balance = existential_deposit.saturating_mul(ED_MULTIPLIER.into()); let _ = as Currency<_>>::make_free_balance_be(&caller, balance); // Transfer `e - 1` existential deposits + 1 unit, which guarantees to create one account, and reap this user. - let recipient: T::AccountId = account("recipient", u, SEED); + let recipient: T::AccountId = account("recipient", 0, SEED); let recipient_lookup: ::Source = T::Lookup::unlookup(recipient.clone()); - let transfer_amount = existential_deposit.saturating_mul((e - 1).into()) + 1.into(); - }: _(RawOrigin::Signed(caller), recipient_lookup, transfer_amount) + let transfer_amount = existential_deposit.saturating_mul((ED_MULTIPLIER - 1).into()) + 1.into(); + }: transfer(RawOrigin::Signed(caller.clone()), recipient_lookup, transfer_amount) verify { + assert_eq!(Balances::::free_balance(&caller), Zero::zero()); assert_eq!(Balances::::free_balance(&recipient), transfer_amount); } // Benchmark `transfer` with the best possible condition: // * Both accounts exist and will continue to exist. + #[extra] transfer_best_case { - let u in ...; - let e in ...; - - let caller = account("caller", u, SEED); - let recipient: T::AccountId = account("recipient", u, SEED); + let caller = account("caller", 0, SEED); + let recipient: T::AccountId = account("recipient", 0, SEED); let recipient_lookup: ::Source = T::Lookup::unlookup(recipient.clone()); // Give the sender account max funds for transfer (their account will never reasonably be killed). @@ -76,52 +70,80 @@ benchmarks! { // Give the recipient account existential deposit (thus their account already exists). let existential_deposit = T::ExistentialDeposit::get(); let _ = as Currency<_>>::make_free_balance_be(&recipient, existential_deposit); - let transfer_amount = existential_deposit.saturating_mul(e.into()); - }: transfer(RawOrigin::Signed(caller), recipient_lookup, transfer_amount) + let transfer_amount = existential_deposit.saturating_mul(ED_MULTIPLIER.into()); + }: transfer(RawOrigin::Signed(caller.clone()), recipient_lookup, transfer_amount) + verify { + assert!(!Balances::::free_balance(&caller).is_zero()); + assert!(!Balances::::free_balance(&recipient).is_zero()); + } // Benchmark `transfer_keep_alive` with the worst possible condition: // * The recipient account is created. transfer_keep_alive { - let u in ...; - let e in ...; - - let caller = account("caller", u, SEED); - let recipient = account("recipient", u, SEED); - let recipient_lookup: ::Source = T::Lookup::unlookup(recipient); + let caller = account("caller", 0, SEED); + let recipient: T::AccountId = account("recipient", 0, SEED); + let recipient_lookup: ::Source = T::Lookup::unlookup(recipient.clone()); // Give the sender account max funds, thus a transfer will not kill account. let _ = as Currency<_>>::make_free_balance_be(&caller, T::Balance::max_value()); let existential_deposit = T::ExistentialDeposit::get(); - let transfer_amount = existential_deposit.saturating_mul(e.into()); - }: _(RawOrigin::Signed(caller), recipient_lookup, transfer_amount) + let transfer_amount = existential_deposit.saturating_mul(ED_MULTIPLIER.into()); + }: _(RawOrigin::Signed(caller.clone()), recipient_lookup, transfer_amount) + verify { + assert!(!Balances::::free_balance(&caller).is_zero()); + assert_eq!(Balances::::free_balance(&recipient), transfer_amount); + } // Benchmark `set_balance` coming from ROOT account. This always creates an account. - set_balance { - let u in ...; - let e in ...; - - let user: T::AccountId = account("user", u, SEED); + set_balance_creating { + let user: T::AccountId = account("user", 0, SEED); let user_lookup: ::Source = T::Lookup::unlookup(user.clone()); // Give the user some initial balance. let existential_deposit = T::ExistentialDeposit::get(); - let balance_amount = existential_deposit.saturating_mul(e.into()); + let balance_amount = existential_deposit.saturating_mul(ED_MULTIPLIER.into()); let _ = as Currency<_>>::make_free_balance_be(&user, balance_amount); - }: _(RawOrigin::Root, user_lookup, balance_amount, balance_amount) + }: set_balance(RawOrigin::Root, user_lookup, balance_amount, balance_amount) + verify { + assert_eq!(Balances::::free_balance(&user), balance_amount); + assert_eq!(Balances::::reserved_balance(&user), balance_amount); + } // Benchmark `set_balance` coming from ROOT account. This always kills an account. set_balance_killing { - let u in ...; - let e in ...; - - let user: T::AccountId = account("user", u, SEED); + let user: T::AccountId = account("user", 0, SEED); let user_lookup: ::Source = T::Lookup::unlookup(user.clone()); // Give the user some initial balance. let existential_deposit = T::ExistentialDeposit::get(); - let balance_amount = existential_deposit.saturating_mul(e.into()); + let balance_amount = existential_deposit.saturating_mul(ED_MULTIPLIER.into()); let _ = as Currency<_>>::make_free_balance_be(&user, balance_amount); - }: set_balance(RawOrigin::Root, user_lookup, 0.into(), 0.into()) + }: set_balance(RawOrigin::Root, user_lookup, Zero::zero(), Zero::zero()) + verify { + assert!(Balances::::free_balance(&user).is_zero()); + } + + // Benchmark `force_transfer` extrinsic with the worst possible conditions: + // * Transfer will kill the sender account. + // * Transfer will create the recipient account. + force_transfer { + let existential_deposit = T::ExistentialDeposit::get(); + let source: T::AccountId = account("source", 0, SEED); + let source_lookup: ::Source = T::Lookup::unlookup(source.clone()); + + // Give some multiple of the existential deposit + creation fee + transfer fee + let balance = existential_deposit.saturating_mul(ED_MULTIPLIER.into()); + let _ = as Currency<_>>::make_free_balance_be(&source, balance); + + // Transfer `e - 1` existential deposits + 1 unit, which guarantees to create one account, and reap this user. + let recipient: T::AccountId = account("recipient", 0, SEED); + let recipient_lookup: ::Source = T::Lookup::unlookup(recipient.clone()); + let transfer_amount = existential_deposit.saturating_mul((ED_MULTIPLIER - 1).into()) + 1.into(); + }: force_transfer(RawOrigin::Root, source_lookup, recipient_lookup, transfer_amount) + verify { + assert_eq!(Balances::::free_balance(&source), Zero::zero()); + assert_eq!(Balances::::free_balance(&recipient), transfer_amount); + } } #[cfg(test)] @@ -152,9 +174,9 @@ mod tests { } #[test] - fn transfer_set_balance() { + fn transfer_set_balance_creating() { ExtBuilder::default().build().execute_with(|| { - assert_ok!(test_benchmark_set_balance::()); + assert_ok!(test_benchmark_set_balance_creating::()); }); } @@ -164,4 +186,11 @@ mod tests { assert_ok!(test_benchmark_set_balance_killing::()); }); } + + #[test] + fn force_transfer() { + ExtBuilder::default().build().execute_with(|| { + assert_ok!(test_benchmark_force_transfer::()); + }); + } } diff --git a/frame/balances/src/lib.rs b/frame/balances/src/lib.rs index 0bd57e3828c24..cc9d9d179fa9c 100644 --- a/frame/balances/src/lib.rs +++ b/frame/balances/src/lib.rs @@ -180,19 +180,19 @@ use frame_system::{self as system, ensure_signed, ensure_root}; pub use self::imbalances::{PositiveImbalance, NegativeImbalance}; pub trait WeightInfo { - fn transfer(u: u32, e: u32, ) -> Weight; - fn transfer_best_case(u: u32, e: u32, ) -> Weight; - fn transfer_keep_alive(u: u32, e: u32, ) -> Weight; - fn set_balance(u: u32, e: u32, ) -> Weight; - fn set_balance_killing(u: u32, e: u32, ) -> Weight; + fn transfer() -> Weight; + fn transfer_keep_alive() -> Weight; + fn set_balance_creating() -> Weight; + fn set_balance_killing() -> Weight; + fn force_transfer() -> Weight; } impl WeightInfo for () { - fn transfer(_u: u32, _e: u32, ) -> Weight { 1_000_000_000 } - fn transfer_best_case(_u: u32, _e: u32, ) -> Weight { 1_000_000_000 } - fn transfer_keep_alive(_u: u32, _e: u32, ) -> Weight { 1_000_000_000 } - fn set_balance(_u: u32, _e: u32, ) -> Weight { 1_000_000_000 } - fn set_balance_killing(_u: u32, _e: u32, ) -> Weight { 1_000_000_000 } + fn transfer() -> Weight { 1_000_000_000 } + fn transfer_keep_alive() -> Weight { 1_000_000_000 } + fn set_balance_creating() -> Weight { 1_000_000_000 } + fn set_balance_killing() -> Weight { 1_000_000_000 } + fn force_transfer() -> Weight { 1_000_000_000 } } pub trait Subtrait: frame_system::Trait { @@ -462,7 +462,7 @@ decl_module! { /// - DB Weight: 1 Read and 1 Write to destination account /// - Origin account is already in memory, so no DB operations for them. /// # - #[weight = T::DbWeight::get().reads_writes(1, 1) + 70_000_000] + #[weight = T::WeightInfo::transfer()] pub fn transfer( origin, dest: ::Source, @@ -491,7 +491,9 @@ decl_module! { /// - Killing: 35.11 µs /// - DB Weight: 1 Read, 1 Write to `who` /// # - #[weight = T::DbWeight::get().reads_writes(1, 1) + 35_000_000] + #[weight = T::WeightInfo::set_balance_creating() // Creates a new account. + .max(T::WeightInfo::set_balance_killing()) // Kills an existing account. + ] fn set_balance( origin, who: ::Source, @@ -533,7 +535,7 @@ decl_module! { /// - Same as transfer, but additional read and write because the source account is /// not assumed to be in the overlay. /// # - #[weight = T::DbWeight::get().reads_writes(2, 2) + 70_000_000] + #[weight = T::WeightInfo::force_transfer()] pub fn force_transfer( origin, source: ::Source, @@ -557,7 +559,7 @@ decl_module! { /// - Base Weight: 51.4 µs /// - DB Weight: 1 Read and 1 Write to dest (sender is in overlay already) /// # - #[weight = T::DbWeight::get().reads_writes(1, 1) + 50_000_000] + #[weight = T::WeightInfo::transfer_keep_alive()] pub fn transfer_keep_alive( origin, dest: ::Source, diff --git a/frame/benchmarking/src/lib.rs b/frame/benchmarking/src/lib.rs index bd0aabdaa3ffa..7ef274f25b157 100644 --- a/frame/benchmarking/src/lib.rs +++ b/frame/benchmarking/src/lib.rs @@ -188,6 +188,7 @@ macro_rules! benchmarks { { $( $( $where_ty: $where_bound ),* )? } { $( { $common , $common_from , $common_to , $common_instancer } )* } ( ) + ( ) $( $rest )* ); } @@ -210,6 +211,7 @@ macro_rules! benchmarks_instance { { $( $( $where_ty: $where_bound ),* )? } { $( { $common , $common_from , $common_to , $common_instancer } )* } ( ) + ( ) $( $rest )* ); } @@ -218,12 +220,34 @@ macro_rules! benchmarks_instance { #[macro_export] #[doc(hidden)] macro_rules! benchmarks_iter { + // detect and extract extra tag: + ( + { $( $instance:ident )? } + { $( $where_clause:tt )* } + { $( $common:tt )* } + ( $( $names:tt )* ) + ( $( $names_extra:tt )* ) + #[extra] + $name:ident + $( $rest:tt )* + ) => { + $crate::benchmarks_iter! { + { $( $instance)? } + { $( $where_clause )* } + { $( $common )* } + ( $( $names )* ) + ( $( $names_extra )* $name ) + $name + $( $rest )* + } + }; // mutation arm: ( { $( $instance:ident )? } { $( $where_clause:tt )* } { $( $common:tt )* } ( $( $names:tt )* ) // This contains $( $( { $instance } )? $name:ident )* + ( $( $names_extra:tt )* ) $name:ident { $( $code:tt )* }: _ ( $origin:expr $( , $arg:expr )* ) verify $postcode:block $( $rest:tt )* @@ -233,6 +257,7 @@ macro_rules! benchmarks_iter { { $( $where_clause )* } { $( $common )* } ( $( $names )* ) + ( $( $names_extra )* ) $name { $( $code )* }: $name ( $origin $( , $arg )* ) verify $postcode $( $rest )* @@ -244,6 +269,7 @@ macro_rules! benchmarks_iter { { $( $where_clause:tt )* } { $( $common:tt )* } ( $( $names:tt )* ) + ( $( $names_extra:tt )* ) $name:ident { $( $code:tt )* }: $dispatch:ident ( $origin:expr $( , $arg:expr )* ) verify $postcode:block $( $rest:tt )* @@ -253,6 +279,7 @@ macro_rules! benchmarks_iter { { $( $where_clause )* } { $( $common )* } ( $( $names )* ) + ( $( $names_extra )* ) $name { $( $code )* }: { < Call as $crate::frame_support::traits::UnfilteredDispatchable @@ -270,6 +297,7 @@ macro_rules! benchmarks_iter { { $( $where_clause:tt )* } { $( $common:tt )* } ( $( $names:tt )* ) + ( $( $names_extra:tt )* ) $name:ident { $( $code:tt )* }: $eval:block verify $postcode:block $( $rest:tt )* @@ -297,6 +325,7 @@ macro_rules! benchmarks_iter { { $( $where_clause )* } { $( $common )* } ( $( $names )* { $( $instance )? } $name ) + ( $( $names_extra )* ) $( $rest )* ); }; @@ -306,9 +335,19 @@ macro_rules! benchmarks_iter { { $( $where_clause:tt )* } { $( $common:tt )* } ( $( $names:tt )* ) + ( $( $names_extra:tt )* ) ) => { - $crate::selected_benchmark!( { $( $where_clause)* } { $( $instance)? } $( $names )* ); - $crate::impl_benchmark!( { $( $where_clause )* } { $( $instance)? } $( $names )* ); + $crate::selected_benchmark!( + { $( $where_clause)* } + { $( $instance)? } + $( $names )* + ); + $crate::impl_benchmark!( + { $( $where_clause )* } + { $( $instance)? } + ( $( $names )* ) + ( $( $names_extra ),* ) + ); }; // add verify block to _() format ( @@ -316,6 +355,7 @@ macro_rules! benchmarks_iter { { $( $where_clause:tt )* } { $( $common:tt )* } ( $( $names:tt )* ) + ( $( $names_extra:tt )* ) $name:ident { $( $code:tt )* }: _ ( $origin:expr $( , $arg:expr )* ) $( $rest:tt )* ) => { @@ -324,6 +364,7 @@ macro_rules! benchmarks_iter { { $( $where_clause )* } { $( $common )* } ( $( $names )* ) + ( $( $names_extra )* ) $name { $( $code )* }: _ ( $origin $( , $arg )* ) verify { } $( $rest )* @@ -335,6 +376,7 @@ macro_rules! benchmarks_iter { { $( $where_clause:tt )* } { $( $common:tt )* } ( $( $names:tt )* ) + ( $( $names_extra:tt )* ) $name:ident { $( $code:tt )* }: $dispatch:ident ( $origin:expr $( , $arg:expr )* ) $( $rest:tt )* ) => { @@ -343,6 +385,7 @@ macro_rules! benchmarks_iter { { $( $where_clause )* } { $( $common )* } ( $( $names )* ) + ( $( $names_extra )* ) $name { $( $code )* }: $dispatch ( $origin $( , $arg )* ) verify { } $( $rest )* @@ -354,6 +397,7 @@ macro_rules! benchmarks_iter { { $( $where_clause:tt )* } { $( $common:tt )* } ( $( $names:tt )* ) + ( $( $names_extra:tt )* ) $name:ident { $( $code:tt )* }: $eval:block $( $rest:tt )* ) => { @@ -362,6 +406,7 @@ macro_rules! benchmarks_iter { { $( $where_clause )* } { $( $common )* } ( $( $names )* ) + ( $( $names_extra )* ) $name { $( $code )* }: $eval verify { } $( $rest )* @@ -659,14 +704,20 @@ macro_rules! impl_benchmark { ( { $( $where_clause:tt )* } { $( $instance:ident )? } - $( { $( $name_inst:ident )? } $name:ident )* + ( $( { $( $name_inst:ident )? } $name:ident )* ) + ( $( $name_extra:ident ),* ) ) => { impl, I: Instance)? > $crate::Benchmarking<$crate::BenchmarkResults> for Module where T: frame_system::Trait, $( $where_clause )* { - fn benchmarks() -> Vec<&'static [u8]> { - vec![ $( stringify!($name).as_ref() ),* ] + fn benchmarks(extra: bool) -> Vec<&'static [u8]> { + let mut all = vec![ $( stringify!($name).as_ref() ),* ]; + if !extra { + let extra = [ $( stringify!($name_extra).as_ref() ),* ]; + all.retain(|x| !extra.contains(x)); + } + all } fn run_benchmark( @@ -931,10 +982,10 @@ macro_rules! impl_benchmark_test { macro_rules! add_benchmark { ( $params:ident, $batches:ident, $name:ident, $( $location:tt )* ) => ( let name_string = stringify!($name).as_bytes(); - let (pallet, benchmark, lowest_range_values, highest_range_values, steps, repeat, whitelist) = $params; + let (pallet, benchmark, lowest_range_values, highest_range_values, steps, repeat, whitelist, extra) = $params; if &pallet[..] == &name_string[..] || &pallet[..] == &b"*"[..] { if &pallet[..] == &b"*"[..] || &benchmark[..] == &b"*"[..] { - for benchmark in $( $location )*::benchmarks().into_iter() { + for benchmark in $( $location )*::benchmarks(extra).into_iter() { $batches.push($crate::BenchmarkBatch { results: $( $location )*::run_benchmark( benchmark, diff --git a/frame/benchmarking/src/utils.rs b/frame/benchmarking/src/utils.rs index e5d40c7443267..7ed9a862a0479 100644 --- a/frame/benchmarking/src/utils.rs +++ b/frame/benchmarking/src/utils.rs @@ -66,6 +66,7 @@ sp_api::decl_runtime_apis! { highest_range_values: Vec, steps: Vec, repeat: u32, + extra: bool, ) -> Result, RuntimeString>; } } @@ -112,7 +113,11 @@ pub trait Benchmarking { pub trait Benchmarking { /// Get the benchmarks available for this pallet. Generally there is one benchmark per /// extrinsic, so these are sometimes just called "extrinsics". - fn benchmarks() -> Vec<&'static [u8]>; + /// + /// Parameters + /// - `extra`: Also return benchmarks marked "extra" which would otherwise not be + /// needed for weight calculation. + fn benchmarks(extra: bool) -> Vec<&'static [u8]>; /// Run the benchmarks for this pallet. /// diff --git a/utils/frame/benchmarking-cli/src/command.rs b/utils/frame/benchmarking-cli/src/command.rs index 7df23f8dbfc9a..553b68c453fa7 100644 --- a/utils/frame/benchmarking-cli/src/command.rs +++ b/utils/frame/benchmarking-cli/src/command.rs @@ -75,6 +75,7 @@ impl BenchmarkCmd { self.highest_range_values.clone(), self.steps.clone(), self.repeat, + self.extra, ).encode(), extensions, &sp_state_machine::backend::BackendRuntimeCode::new(&state).runtime_code()?, diff --git a/utils/frame/benchmarking-cli/src/lib.rs b/utils/frame/benchmarking-cli/src/lib.rs index 8a53c9fd8b16b..c2a228fc86aef 100644 --- a/utils/frame/benchmarking-cli/src/lib.rs +++ b/utils/frame/benchmarking-cli/src/lib.rs @@ -72,6 +72,10 @@ pub struct BenchmarkCmd { #[structopt(long)] pub heap_pages: Option, + /// Display and run extra benchmarks that would otherwise not be needed for weight construction. + #[structopt(long)] + pub extra: bool, + #[allow(missing_docs)] #[structopt(flatten)] pub shared_params: sc_cli::SharedParams,