From 01d0d13fada5ce0dcd9186946f76d54eb20ebb50 Mon Sep 17 00:00:00 2001
From: Shawn Tabrizi <shawntabrizi@gmail.com>
Date: Thu, 30 Jul 2020 17:08:23 +0200
Subject: [PATCH] Update Balances Pallet to use `WeightInfo` (#6610)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
* Update balance benchmarks
* Update weight functions
* Remove user component
* make componentless
* Add support for `#[extra]` tag on benchmarks
* Update balances completely
* Apply suggestions from code review
Co-authored-by: Alexander Theißen <alex.theissen@me.com>
* Fix some tests
* Maybe fix to test. Need approval from @tomusdrw this is okay
* Make test better
* keep weights conservative
* Update macro for merge master
* Add headers
* Apply suggestions from code review
Co-authored-by: Alexander Popiak <alexander.popiak@parity.io>
Co-authored-by: Alexander Theißen <alex.theissen@me.com>
Co-authored-by: Alexander Popiak <alexander.popiak@parity.io>
---
substrate/bin/node/executor/tests/basic.rs | 18 ++-
.../node/executor/tests/submit_transaction.rs | 18 ++-
substrate/bin/node/runtime/src/lib.rs | 8 +-
substrate/bin/node/runtime/src/weights/mod.rs | 18 +++
.../runtime/src/weights/pallet_balances.rs | 47 +++++++
substrate/frame/balances/src/benchmarking.rs | 117 +++++++++++-------
substrate/frame/balances/src/lib.rs | 30 ++---
substrate/frame/benchmarking/src/lib.rs | 65 ++++++++--
substrate/frame/benchmarking/src/utils.rs | 7 +-
.../frame/benchmarking-cli/src/command.rs | 1 +
.../utils/frame/benchmarking-cli/src/lib.rs | 4 +
11 files changed, 245 insertions(+), 88 deletions(-)
create mode 100644 substrate/bin/node/runtime/src/weights/mod.rs
create mode 100644 substrate/bin/node/runtime/src/weights/pallet_balances.rs
diff --git a/substrate/bin/node/executor/tests/basic.rs b/substrate/bin/node/executor/tests/basic.rs
index f6dc1c3e7ea..0d69b970016 100644
--- a/substrate/bin/node/executor/tests/basic.rs
+++ b/substrate/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::<Runtime>(Default::default()).get_dispatch_info().weight;
+
executor_call::<NeverNativeValue, fn() -> _>(
&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/substrate/bin/node/executor/tests/submit_transaction.rs b/substrate/bin/node/executor/tests/submit_transaction.rs
index dd599a996a4..64c2deedac7 100644
--- a/substrate/bin/node/executor/tests/submit_transaction.rs
+++ b/substrate/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() {
<frame_system::Account<Runtime>>::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::<TransactionTag>::new());
+ assert_eq!(res.provides, vec![(address, 0).encode()]);
+ assert_eq!(res.longevity, 2048);
+ assert_eq!(res.propagate, true);
});
}
diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs
index 9dae66a1275..373a01b8ea2 100644
--- a/substrate/bin/node/runtime/src/lib.rs
+++ b/substrate/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<Runtime>;
- type WeightInfo = ();
+ type WeightInfo = weights::pallet_balances::WeightInfo;
}
parameter_types! {
@@ -1126,6 +1129,7 @@ impl_runtime_apis! {
highest_range_values: Vec<u32>,
steps: Vec<u32>,
repeat: u32,
+ extra: bool,
) -> Result<Vec<frame_benchmarking::BenchmarkBatch>, 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::<BenchmarkBatch>::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/substrate/bin/node/runtime/src/weights/mod.rs b/substrate/bin/node/runtime/src/weights/mod.rs
new file mode 100644
index 00000000000..70e10d5342f
--- /dev/null
+++ b/substrate/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/substrate/bin/node/runtime/src/weights/pallet_balances.rs b/substrate/bin/node/runtime/src/weights/pallet_balances.rs
new file mode 100644
index 00000000000..21a90a97e63
--- /dev/null
+++ b/substrate/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/substrate/frame/balances/src/benchmarking.rs b/substrate/frame/balances/src/benchmarking.rs
index a5f8e6fe36c..73547fe814a 100644
--- a/substrate/frame/balances/src/benchmarking.rs
+++ b/substrate/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 _ = <Balances<T> 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: <T::Lookup as StaticLookup>::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::<T>::free_balance(&caller), Zero::zero());
assert_eq!(Balances::<T>::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: <T::Lookup as StaticLookup>::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 _ = <Balances<T> 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::<T>::free_balance(&caller).is_zero());
+ assert!(!Balances::<T>::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: <T::Lookup as StaticLookup>::Source = T::Lookup::unlookup(recipient);
+ let caller = account("caller", 0, SEED);
+ let recipient: T::AccountId = account("recipient", 0, SEED);
+ let recipient_lookup: <T::Lookup as StaticLookup>::Source = T::Lookup::unlookup(recipient.clone());
// Give the sender account max funds, thus a transfer will not kill account.
let _ = <Balances<T> 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::<T>::free_balance(&caller).is_zero());
+ assert_eq!(Balances::<T>::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: <T::Lookup as StaticLookup>::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 _ = <Balances<T> 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::<T>::free_balance(&user), balance_amount);
+ assert_eq!(Balances::<T>::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: <T::Lookup as StaticLookup>::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 _ = <Balances<T> 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::<T>::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: <T::Lookup as StaticLookup>::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 _ = <Balances<T> 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: <T::Lookup as StaticLookup>::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::<T>::free_balance(&source), Zero::zero());
+ assert_eq!(Balances::<T>::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::<Test>());
+ assert_ok!(test_benchmark_set_balance_creating::<Test>());
});
}
@@ -164,4 +186,11 @@ mod tests {
assert_ok!(test_benchmark_set_balance_killing::<Test>());
});
}
+
+ #[test]
+ fn force_transfer() {
+ ExtBuilder::default().build().execute_with(|| {
+ assert_ok!(test_benchmark_force_transfer::<Test>());
+ });
+ }
}
diff --git a/substrate/frame/balances/src/lib.rs b/substrate/frame/balances/src/lib.rs
index 0bd57e3828c..cc9d9d179fa 100644
--- a/substrate/frame/balances/src/lib.rs
+++ b/substrate/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<I: Instance = DefaultInstance>: 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>
- #[weight = T::DbWeight::get().reads_writes(1, 1) + 70_000_000]
+ #[weight = T::WeightInfo::transfer()]
pub fn transfer(
origin,
dest: <T::Lookup as StaticLookup>::Source,
@@ -491,7 +491,9 @@ decl_module! {
/// - Killing: 35.11 µs
/// - DB Weight: 1 Read, 1 Write to `who`
/// # </weight>
- #[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: <T::Lookup as StaticLookup>::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>
- #[weight = T::DbWeight::get().reads_writes(2, 2) + 70_000_000]
+ #[weight = T::WeightInfo::force_transfer()]
pub fn force_transfer(
origin,
source: <T::Lookup as StaticLookup>::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>
- #[weight = T::DbWeight::get().reads_writes(1, 1) + 50_000_000]
+ #[weight = T::WeightInfo::transfer_keep_alive()]
pub fn transfer_keep_alive(
origin,
dest: <T::Lookup as StaticLookup>::Source,
diff --git a/substrate/frame/benchmarking/src/lib.rs b/substrate/frame/benchmarking/src/lib.rs
index bd0aabdaa3f..7ef274f25b1 100644
--- a/substrate/frame/benchmarking/src/lib.rs
+++ b/substrate/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<T $(, $instance)? > 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<T: Trait $(<$instance>, I: Instance)? >
$crate::Benchmarking<$crate::BenchmarkResults> for Module<T $(, $instance)? >
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/substrate/frame/benchmarking/src/utils.rs b/substrate/frame/benchmarking/src/utils.rs
index e5d40c74432..7ed9a862a04 100644
--- a/substrate/frame/benchmarking/src/utils.rs
+++ b/substrate/frame/benchmarking/src/utils.rs
@@ -66,6 +66,7 @@ sp_api::decl_runtime_apis! {
highest_range_values: Vec<u32>,
steps: Vec<u32>,
repeat: u32,
+ extra: bool,
) -> Result<Vec<BenchmarkBatch>, RuntimeString>;
}
}
@@ -112,7 +113,11 @@ pub trait Benchmarking {
pub trait Benchmarking<T> {
/// 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/substrate/utils/frame/benchmarking-cli/src/command.rs b/substrate/utils/frame/benchmarking-cli/src/command.rs
index 7df23f8dbfc..553b68c453f 100644
--- a/substrate/utils/frame/benchmarking-cli/src/command.rs
+++ b/substrate/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/substrate/utils/frame/benchmarking-cli/src/lib.rs b/substrate/utils/frame/benchmarking-cli/src/lib.rs
index 8a53c9fd8b1..c2a228fc86a 100644
--- a/substrate/utils/frame/benchmarking-cli/src/lib.rs
+++ b/substrate/utils/frame/benchmarking-cli/src/lib.rs
@@ -72,6 +72,10 @@ pub struct BenchmarkCmd {
#[structopt(long)]
pub heap_pages: Option<u64>,
+ /// 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,
--
GitLab