pallet macro broke benchmarks_instance, fix by introducing benchmarks_instance_pallet (#8190)

Co-authored-by: Peter Goodspeed-Niklaus <coriolinus@users.noreply.github.com>
This commit is contained in:
Guillaume Thiolliere
2021-02-25 16:25:08 +01:00
committed by GitHub
parent d5b308f13c
commit f2436b87b5
2 changed files with 79 additions and 59 deletions
+20 -20
View File
@@ -22,7 +22,7 @@
use super::*;
use frame_system::RawOrigin;
use frame_benchmarking::{benchmarks, account, whitelisted_caller, impl_benchmark_test_suite};
use frame_benchmarking::{benchmarks_instance_pallet, account, whitelisted_caller, impl_benchmark_test_suite};
use sp_runtime::traits::Bounded;
use crate::Module as Balances;
@@ -32,7 +32,7 @@ const SEED: u32 = 0;
const ED_MULTIPLIER: u32 = 10;
benchmarks! {
benchmarks_instance_pallet! {
// Benchmark `transfer` extrinsic with the worst possible conditions:
// * Transfer will kill the sender account.
// * Transfer will create the recipient account.
@@ -42,7 +42,7 @@ benchmarks! {
// 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(&caller, balance);
let _ = <Balances<T, I> 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", 0, SEED);
@@ -50,8 +50,8 @@ benchmarks! {
let transfer_amount = existential_deposit.saturating_mul((ED_MULTIPLIER - 1).into()) + 1u32.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);
assert_eq!(Balances::<T, I>::free_balance(&caller), Zero::zero());
assert_eq!(Balances::<T, I>::free_balance(&recipient), transfer_amount);
}
// Benchmark `transfer` with the best possible condition:
@@ -63,16 +63,16 @@ benchmarks! {
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).
let _ = <Balances<T> as Currency<_>>::make_free_balance_be(&caller, T::Balance::max_value());
let _ = <Balances<T, I> as Currency<_>>::make_free_balance_be(&caller, T::Balance::max_value());
// 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 _ = <Balances<T, I> as Currency<_>>::make_free_balance_be(&recipient, existential_deposit);
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());
assert!(!Balances::<T, I>::free_balance(&caller).is_zero());
assert!(!Balances::<T, I>::free_balance(&recipient).is_zero());
}
// Benchmark `transfer_keep_alive` with the worst possible condition:
@@ -83,13 +83,13 @@ benchmarks! {
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 _ = <Balances<T, I> as Currency<_>>::make_free_balance_be(&caller, T::Balance::max_value());
let existential_deposit = T::ExistentialDeposit::get();
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);
assert!(!Balances::<T, I>::free_balance(&caller).is_zero());
assert_eq!(Balances::<T, I>::free_balance(&recipient), transfer_amount);
}
// Benchmark `set_balance` coming from ROOT account. This always creates an account.
@@ -100,11 +100,11 @@ benchmarks! {
// Give the user some initial balance.
let existential_deposit = T::ExistentialDeposit::get();
let balance_amount = existential_deposit.saturating_mul(ED_MULTIPLIER.into());
let _ = <Balances<T> as Currency<_>>::make_free_balance_be(&user, balance_amount);
let _ = <Balances<T, I> as Currency<_>>::make_free_balance_be(&user, 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);
assert_eq!(Balances::<T, I>::free_balance(&user), balance_amount);
assert_eq!(Balances::<T, I>::reserved_balance(&user), balance_amount);
}
// Benchmark `set_balance` coming from ROOT account. This always kills an account.
@@ -115,10 +115,10 @@ benchmarks! {
// Give the user some initial balance.
let existential_deposit = T::ExistentialDeposit::get();
let balance_amount = existential_deposit.saturating_mul(ED_MULTIPLIER.into());
let _ = <Balances<T> as Currency<_>>::make_free_balance_be(&user, balance_amount);
let _ = <Balances<T, I> as Currency<_>>::make_free_balance_be(&user, balance_amount);
}: set_balance(RawOrigin::Root, user_lookup, Zero::zero(), Zero::zero())
verify {
assert!(Balances::<T>::free_balance(&user).is_zero());
assert!(Balances::<T, I>::free_balance(&user).is_zero());
}
// Benchmark `force_transfer` extrinsic with the worst possible conditions:
@@ -131,7 +131,7 @@ benchmarks! {
// 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);
let _ = <Balances<T, I> 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);
@@ -139,8 +139,8 @@ benchmarks! {
let transfer_amount = existential_deposit.saturating_mul((ED_MULTIPLIER - 1).into()) + 1u32.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);
assert_eq!(Balances::<T, I>::free_balance(&source), Zero::zero());
assert_eq!(Balances::<T, I>::free_balance(&recipient), transfer_amount);
}
}
+59 -39
View File
@@ -175,13 +175,33 @@ macro_rules! benchmarks {
}
/// Same as [`benchmarks`] but for instantiable module.
///
/// NOTE: For pallet declared with [`frame_support::pallet`], use [`benchmarks_instance_pallet`].
#[macro_export]
macro_rules! benchmarks_instance {
(
$( $rest:tt )*
) => {
$crate::benchmarks_iter!(
{ I }
{ I: Instance }
{ }
( )
( )
$( $rest )*
);
}
}
/// Same as [`benchmarks`] but for instantiable pallet declared [`frame_support::pallet`].
///
/// NOTE: For pallet declared with `decl_module!`, use [`benchmarks_instance`].
#[macro_export]
macro_rules! benchmarks_instance_pallet {
(
$( $rest:tt )*
) => {
$crate::benchmarks_iter!(
{ I: 'static }
{ }
( )
( )
@@ -195,7 +215,7 @@ macro_rules! benchmarks_instance {
macro_rules! benchmarks_iter {
// detect and extract where clause:
(
{ $( $instance:ident )? }
{ $( $instance:ident: $instance_bound:tt )? }
{ $( $where_clause:tt )* }
( $( $names:tt )* )
( $( $names_extra:tt )* )
@@ -203,7 +223,7 @@ macro_rules! benchmarks_iter {
$( $rest:tt )*
) => {
$crate::benchmarks_iter! {
{ $( $instance)? }
{ $( $instance: $instance_bound)? }
{ $( $where_bound )* }
( $( $names )* )
( $( $names_extra )* )
@@ -212,7 +232,7 @@ macro_rules! benchmarks_iter {
};
// detect and extract extra tag:
(
{ $( $instance:ident )? }
{ $( $instance:ident: $instance_bound:tt )? }
{ $( $where_clause:tt )* }
( $( $names:tt )* )
( $( $names_extra:tt )* )
@@ -221,7 +241,7 @@ macro_rules! benchmarks_iter {
$( $rest:tt )*
) => {
$crate::benchmarks_iter! {
{ $( $instance)? }
{ $( $instance: $instance_bound )? }
{ $( $where_clause )* }
( $( $names )* )
( $( $names_extra )* $name )
@@ -231,7 +251,7 @@ macro_rules! benchmarks_iter {
};
// mutation arm:
(
{ $( $instance:ident )? }
{ $( $instance:ident: $instance_bound:tt )? }
{ $( $where_clause:tt )* }
( $( $names:tt )* ) // This contains $( $( { $instance } )? $name:ident )*
( $( $names_extra:tt )* )
@@ -240,7 +260,7 @@ macro_rules! benchmarks_iter {
$( $rest:tt )*
) => {
$crate::benchmarks_iter! {
{ $( $instance)? }
{ $( $instance: $instance_bound )? }
{ $( $where_clause )* }
( $( $names )* )
( $( $names_extra )* )
@@ -251,7 +271,7 @@ macro_rules! benchmarks_iter {
};
// mutation arm:
(
{ $( $instance:ident )? }
{ $( $instance:ident: $instance_bound:tt )? }
{ $( $where_clause:tt )* }
( $( $names:tt )* )
( $( $names_extra:tt )* )
@@ -260,7 +280,7 @@ macro_rules! benchmarks_iter {
$( $rest:tt )*
) => {
$crate::benchmarks_iter! {
{ $( $instance)? }
{ $( $instance: $instance_bound )? }
{ $( $where_clause )* }
( $( $names )* )
( $( $names_extra )* )
@@ -277,7 +297,7 @@ macro_rules! benchmarks_iter {
};
// iteration arm:
(
{ $( $instance:ident )? }
{ $( $instance:ident: $instance_bound:tt )? }
{ $( $where_clause:tt )* }
( $( $names:tt )* )
( $( $names_extra:tt )* )
@@ -286,7 +306,7 @@ macro_rules! benchmarks_iter {
$( $rest:tt )*
) => {
$crate::benchmark_backend! {
{ $( $instance)? }
{ $( $instance: $instance_bound )? }
$name
{ $( $where_clause )* }
{ }
@@ -298,12 +318,12 @@ macro_rules! benchmarks_iter {
#[cfg(test)]
$crate::impl_benchmark_test!(
{ $( $where_clause )* }
{ $( $instance)? }
{ $( $instance: $instance_bound )? }
$name
);
$crate::benchmarks_iter!(
{ $( $instance)? }
{ $( $instance: $instance_bound )? }
{ $( $where_clause )* }
( $( $names )* { $( $instance )? } $name )
( $( $names_extra )* )
@@ -312,26 +332,26 @@ macro_rules! benchmarks_iter {
};
// iteration-exit arm
(
{ $( $instance:ident )? }
{ $( $instance:ident: $instance_bound:tt )? }
{ $( $where_clause:tt )* }
( $( $names:tt )* )
( $( $names_extra:tt )* )
) => {
$crate::selected_benchmark!(
{ $( $where_clause)* }
{ $( $instance)? }
{ $( $instance: $instance_bound )? }
$( $names )*
);
$crate::impl_benchmark!(
{ $( $where_clause )* }
{ $( $instance)? }
{ $( $instance: $instance_bound )? }
( $( $names )* )
( $( $names_extra ),* )
);
};
// add verify block to _() format
(
{ $( $instance:ident )? }
{ $( $instance:ident: $instance_bound:tt )? }
{ $( $where_clause:tt )* }
( $( $names:tt )* )
( $( $names_extra:tt )* )
@@ -339,7 +359,7 @@ macro_rules! benchmarks_iter {
$( $rest:tt )*
) => {
$crate::benchmarks_iter! {
{ $( $instance)? }
{ $( $instance: $instance_bound )? }
{ $( $where_clause )* }
( $( $names )* )
( $( $names_extra )* )
@@ -350,7 +370,7 @@ macro_rules! benchmarks_iter {
};
// add verify block to name() format
(
{ $( $instance:ident )? }
{ $( $instance:ident: $instance_bound:tt )? }
{ $( $where_clause:tt )* }
( $( $names:tt )* )
( $( $names_extra:tt )* )
@@ -358,7 +378,7 @@ macro_rules! benchmarks_iter {
$( $rest:tt )*
) => {
$crate::benchmarks_iter! {
{ $( $instance)? }
{ $( $instance: $instance_bound )? }
{ $( $where_clause )* }
( $( $names )* )
( $( $names_extra )* )
@@ -369,7 +389,7 @@ macro_rules! benchmarks_iter {
};
// add verify block to {} format
(
{ $( $instance:ident )? }
{ $( $instance:ident: $instance_bound:tt )? }
{ $( $where_clause:tt )* }
( $( $names:tt )* )
( $( $names_extra:tt )* )
@@ -377,7 +397,7 @@ macro_rules! benchmarks_iter {
$( $rest:tt )*
) => {
$crate::benchmarks_iter!(
{ $( $instance)? }
{ $( $instance: $instance_bound )? }
{ $( $where_clause )* }
( $( $names )* )
( $( $names_extra )* )
@@ -393,7 +413,7 @@ macro_rules! benchmarks_iter {
macro_rules! benchmark_backend {
// parsing arms
(
{ $( $instance:ident )? }
{ $( $instance:ident: $instance_bound:tt )? }
$name:ident
{ $( $where_clause:tt )* }
{ $( PRE { $( $pre_parsed:tt )* } )* }
@@ -405,7 +425,7 @@ macro_rules! benchmark_backend {
$postcode:block
) => {
$crate::benchmark_backend! {
{ $( $instance)? }
{ $( $instance: $instance_bound )? }
$name
{ $( $where_clause )* }
{
@@ -418,7 +438,7 @@ macro_rules! benchmark_backend {
}
};
(
{ $( $instance:ident )? }
{ $( $instance:ident: $instance_bound:tt )? }
$name:ident
{ $( $where_clause:tt )* }
{ $( $parsed:tt )* }
@@ -430,7 +450,7 @@ macro_rules! benchmark_backend {
$postcode:block
) => {
$crate::benchmark_backend! {
{ $( $instance)? }
{ $( $instance: $instance_bound )? }
$name
{ $( $where_clause )* }
{
@@ -444,7 +464,7 @@ macro_rules! benchmark_backend {
};
// mutation arm to look after a single tt for param_from.
(
{ $( $instance:ident )? }
{ $( $instance:ident: $instance_bound:tt )? }
$name:ident
{ $( $where_clause:tt )* }
{ $( $parsed:tt )* }
@@ -456,7 +476,7 @@ macro_rules! benchmark_backend {
$postcode:block
) => {
$crate::benchmark_backend! {
{ $( $instance)? }
{ $( $instance: $instance_bound )? }
$name
{ $( $where_clause )* }
{ $( $parsed )* }
@@ -470,7 +490,7 @@ macro_rules! benchmark_backend {
};
// mutation arm to look after the default tail of `=> ()`
(
{ $( $instance:ident )? }
{ $( $instance:ident: $instance_bound:tt )? }
$name:ident
{ $( $where_clause:tt )* }
{ $( $parsed:tt )* }
@@ -482,7 +502,7 @@ macro_rules! benchmark_backend {
$postcode:block
) => {
$crate::benchmark_backend! {
{ $( $instance)? }
{ $( $instance: $instance_bound )? }
$name
{ $( $where_clause )* }
{ $( $parsed )* }
@@ -496,7 +516,7 @@ macro_rules! benchmark_backend {
};
// mutation arm to look after `let _ =`
(
{ $( $instance:ident )? }
{ $( $instance:ident: $instance_bound:tt )? }
$name:ident
{ $( $where_clause:tt )* }
{ $( $parsed:tt )* }
@@ -508,7 +528,7 @@ macro_rules! benchmark_backend {
$postcode:block
) => {
$crate::benchmark_backend! {
{ $( $instance)? }
{ $( $instance: $instance_bound )? }
$name
{ $( $where_clause )* }
{ $( $parsed )* }
@@ -522,7 +542,7 @@ macro_rules! benchmark_backend {
};
// actioning arm
(
{ $( $instance:ident )? }
{ $( $instance:ident: $instance_bound:tt )? }
$name:ident
{ $( $where_clause:tt )* }
{
@@ -536,7 +556,7 @@ macro_rules! benchmark_backend {
#[allow(non_camel_case_types)]
struct $name;
#[allow(unused_variables)]
impl<T: Config $( <$instance>, I: Instance)? >
impl<T: Config $( <$instance>, $instance: $instance_bound )? >
$crate::BenchmarkingSetup<T $(, $instance)? > for $name
where $( $where_clause )*
{
@@ -597,7 +617,7 @@ macro_rules! benchmark_backend {
macro_rules! selected_benchmark {
(
{ $( $where_clause:tt )* }
{ $( $instance:ident )? }
{ $( $instance:ident: $instance_bound:tt )? }
$( { $( $bench_inst:ident )? } $bench:ident )*
) => {
// The list of available benchmarks for this pallet.
@@ -607,7 +627,7 @@ macro_rules! selected_benchmark {
}
// Allow us to select a benchmark from the list of available benchmarks.
impl<T: Config $( <$instance>, I: Instance )? >
impl<T: Config $( <$instance>, $instance: $instance_bound )? >
$crate::BenchmarkingSetup<T $(, $instance )? > for SelectedBenchmark
where $( $where_clause )*
{
@@ -643,11 +663,11 @@ macro_rules! selected_benchmark {
macro_rules! impl_benchmark {
(
{ $( $where_clause:tt )* }
{ $( $instance:ident )? }
{ $( $instance:ident: $instance_bound:tt )? }
( $( { $( $name_inst:ident )? } $name:ident )* )
( $( $name_extra:ident ),* )
) => {
impl<T: Config $(<$instance>, I: Instance)? >
impl<T: Config $(<$instance>, $instance: $instance_bound )? >
$crate::Benchmarking<$crate::BenchmarkResults> for Module<T $(, $instance)? >
where T: frame_system::Config, $( $where_clause )*
{
@@ -866,7 +886,7 @@ macro_rules! impl_benchmark {
macro_rules! impl_benchmark_test {
(
{ $( $where_clause:tt )* }
{ $( $instance:ident )? }
{ $( $instance:ident: $instance_bound:tt )? }
$name:ident
) => {
$crate::paste::item! {