From 0469734c3d6160f15076a61ba0e641ebec1cc597 Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Mon, 23 Jan 2023 02:07:48 -0500 Subject: [PATCH] new proc-macro-based benchmarking syntax (#12924) * add stub for new benchmark macro * benchmark syntax * add #[extrinsic call] separator * parse #[benchmark] item as a function * proper emission of error when #[extrinsic_call] annotation is missing * clean up * enclosing module via benchmarks! { } working * use an attribute macro on the module instead of benchmarks! { } * cargo fmt * working component implementation * WIP * working * add syntax for Linear * parsing of param ranges (still need to build tuple though) * params parsing WIP * clean up (don't need extrinsic call name) * use proper Result syntax for BenchmarkDef parsing * proper parsing of Linear<0, 1> style args * successfully parse and make use of linear component ranges :boom: * rename support variable => home because eventually will be moved * compile-time check that param range types implement ParamRange * switch to using balances as example, failing on instance pallet * successfully set up __origin and __call with balances :boom: * clean up * use a module * don't need a variable for transfer * rename benchmark_transfer -> transfer because no longer conflicts * clean up * working with transfer_increasing_users as well :boom: * re-add BareBlock * add comments for undocumented structs+functions+traits * refactor in preparation for removing module requirements * switch to a block instead of a module * use the outer macro pattern to to enable #[benchmarks] aggregation * successfully generate SelectedBenchmark :boom: * implement components for SelectedBenchmark * implement instance for SelectedBenchmark * properly track #[extra] * working impl for fn benchmarks() * run_benchmarks WIP * finish run_benchmark! impl :boom: * import balances transfer_best_case benchmark * import transfer_keep_alive balances pallet benchmark * import set_balance_creating balances pallet benchmark * import set_balance_killing balances pallet benchmark * import force_transfer balances pallet benchmark * add #[extra] annotation and docs to transfer_increasing_users * import transfer_all balances pallet benchmark * import force_unreserve balances pallet benchmark * prepare to implement impl_benchmark_test_suite! * ensure tests cover #[extra] before and after #[benchmark] tag * refactor * clean up * fix * move to outer * switch to benchmarks/instance_benchmarks * test impl almost done, strange compiler error * benchmark test suites working :boom: * clean up * add stub and basic parsing for where_clause * working except where clause and extrinsic calls containing method chains * assume option (2) for now wrt https://github.com/paritytech/substrate/pull/12924#issuecomment-1372938718 * clean up * switch to attribute-style * properly handle where clauses * fix subtle missing where clause, now just MessageQueue issues * fix block formatting in message-queue pallet * switch to block vs non-block parsing of extrinsic call * working now but some benchmark tests failing * message-queue tests working (run order issue fixed) :tada: * add comments and internal docs for fame_support_procedural::benchmark * fix license years * docs for lib.rs * add docs to new support procedural macros * don't allow #[benchmark] outside of benchmarking module * add docs * use benchmark(extra, skip_meta) style args * update docs accordingly * appease clippy * bump ci * add notes about `extra` and `skip_meta` * fix doc tests * re-run CI * use `ignore` instead of `no_run` on doc examples * bump CI * replace some if-lets with if-elses * more refactoring of if-let statements * fix remaining if-lets in BenchmarkDef::from() * fix if-lets in benchmarks() * fix remaining if-lets, use nested find_map for extrinsic call * switch to use #[extrinsic_call] or #[block] situationally * refactor ExtrinsicCallDef => BenchmarkCallDef * update docs with info about #[block] * add macro stub for #[extrinsic_call] * fix docs and add stub for #[block] as well * remove unused extern crate line * fix clippy nits * Use V2 bench syntax in pallet-example-basic Just testing the dev-ex... Signed-off-by: Oliver Tale-Yazdi * carry over comment * use curly-brace style for impl_benchmark_test_suite! * remove unneeded parenthesis * proper handling of _() extrinsic call style * add docs for _() syntax * fix crate access * simplify keyword access Co-authored-by: Keith Yeung * simplify module content destructuring Co-authored-by: Keith Yeung * fix crate access "frame_benchmarking" => "frame-benchmarking", compiles * use _() extrinsic call syntax where possible in balances * simplify attr.path.segments.last() Co-authored-by: Keith Yeung * fix compile error being suppressed * simplify extrinsic call keyword parsing Co-authored-by: Keith Yeung * use ? operator instead of return None Co-authored-by: Keith Yeung * rename generics => type_use_generics rename full_generics => type_impl_generics * simplify extrinsic call extraction with transpose * bump CI * nit * proper handling of too many + too few block/extrinsic call annotations * change to B >= A Co-authored-by: Oliver Tale-Yazdi * remove unneeded ignore Co-authored-by: Oliver Tale-Yazdi * remove another ignore Co-authored-by: Oliver Tale-Yazdi * add ui tests * use _() style extrinsic call on accumulate_dummy Co-authored-by: Oliver Tale-Yazdi * add range check to ParamRange * ui test for bad param ranges * fix failing example * add ignore back to other failing example * tweak expr_call span Co-authored-by: Keith Yeung * fix typo * eliminate a match Co-authored-by: Keith Yeung * change pub fn benchmarks to return Result * fix origin error span * more informative error for invalid benchmark parameter name * fix spans on a few benchmark errors * remove unneeded clone * refactor inner loop of benchmark function parsing * preserve mod attributes * refactor outer loop of benchmark def parsing code, greatly simplified * simplify to use a ? operator when parsing benchmark attr path * fix another ? operator * further simplify benchmark function attr parsing with more ? ops * refactor extrinsic call handling to use if let rather than match * replace is_ok => is_err Co-authored-by: Keith Yeung * re-use name during expansion of benchmark def * remove unneeded clone * fix span for origin missing error * fix missing semi Signed-off-by: Oliver Tale-Yazdi Co-authored-by: Oliver Tale-Yazdi Co-authored-by: Keith Yeung Co-authored-by: parity-processbot <> --- Cargo.lock | 14 + frame/balances/src/benchmarking.rs | 127 ++- frame/examples/basic/src/benchmarking.rs | 62 +- frame/message-queue/src/benchmarking.rs | 217 +++-- frame/support/Cargo.toml | 1 + frame/support/procedural/Cargo.toml | 1 + frame/support/procedural/src/benchmark.rs | 860 ++++++++++++++++++ frame/support/procedural/src/lib.rs | 64 ++ frame/support/src/lib.rs | 223 +++++ frame/support/test/Cargo.toml | 2 + frame/support/test/tests/benchmark_ui.rs | 36 + .../test/tests/benchmark_ui/bad_param_name.rs | 18 + .../tests/benchmark_ui/bad_param_name.stderr | 5 + .../benchmark_ui/bad_param_name_too_long.rs | 14 + .../bad_param_name_too_long.stderr | 5 + .../benchmark_ui/bad_param_name_upper_case.rs | 14 + .../bad_param_name_upper_case.stderr | 5 + .../tests/benchmark_ui/bad_param_range.rs | 16 + .../tests/benchmark_ui/bad_param_range.stderr | 5 + .../test/tests/benchmark_ui/bad_params.rs | 18 + .../test/tests/benchmark_ui/bad_params.stderr | 5 + .../test/tests/benchmark_ui/dup_block.rs | 20 + .../test/tests/benchmark_ui/dup_block.stderr | 5 + .../tests/benchmark_ui/dup_extrinsic_call.rs | 20 + .../benchmark_ui/dup_extrinsic_call.stderr | 5 + .../test/tests/benchmark_ui/extra_extra.rs | 16 + .../tests/benchmark_ui/extra_extra.stderr | 5 + .../tests/benchmark_ui/extra_skip_meta.rs | 16 + .../tests/benchmark_ui/extra_skip_meta.stderr | 5 + .../benchmark_ui/extrinsic_call_out_of_fn.rs | 6 + .../extrinsic_call_out_of_fn.stderr | 7 + .../test/tests/benchmark_ui/missing_call.rs | 13 + .../tests/benchmark_ui/missing_call.stderr | 5 + .../test/tests/benchmark_ui/missing_origin.rs | 16 + .../tests/benchmark_ui/missing_origin.stderr | 5 + .../tests/benchmark_ui/pass/valid_basic.rs | 17 + .../tests/benchmark_ui/unrecognized_option.rs | 16 + .../benchmark_ui/unrecognized_option.stderr | 5 + 38 files changed, 1760 insertions(+), 134 deletions(-) create mode 100644 frame/support/procedural/src/benchmark.rs create mode 100644 frame/support/test/tests/benchmark_ui.rs create mode 100644 frame/support/test/tests/benchmark_ui/bad_param_name.rs create mode 100644 frame/support/test/tests/benchmark_ui/bad_param_name.stderr create mode 100644 frame/support/test/tests/benchmark_ui/bad_param_name_too_long.rs create mode 100644 frame/support/test/tests/benchmark_ui/bad_param_name_too_long.stderr create mode 100644 frame/support/test/tests/benchmark_ui/bad_param_name_upper_case.rs create mode 100644 frame/support/test/tests/benchmark_ui/bad_param_name_upper_case.stderr create mode 100644 frame/support/test/tests/benchmark_ui/bad_param_range.rs create mode 100644 frame/support/test/tests/benchmark_ui/bad_param_range.stderr create mode 100644 frame/support/test/tests/benchmark_ui/bad_params.rs create mode 100644 frame/support/test/tests/benchmark_ui/bad_params.stderr create mode 100644 frame/support/test/tests/benchmark_ui/dup_block.rs create mode 100644 frame/support/test/tests/benchmark_ui/dup_block.stderr create mode 100644 frame/support/test/tests/benchmark_ui/dup_extrinsic_call.rs create mode 100644 frame/support/test/tests/benchmark_ui/dup_extrinsic_call.stderr create mode 100644 frame/support/test/tests/benchmark_ui/extra_extra.rs create mode 100644 frame/support/test/tests/benchmark_ui/extra_extra.stderr create mode 100644 frame/support/test/tests/benchmark_ui/extra_skip_meta.rs create mode 100644 frame/support/test/tests/benchmark_ui/extra_skip_meta.stderr create mode 100644 frame/support/test/tests/benchmark_ui/extrinsic_call_out_of_fn.rs create mode 100644 frame/support/test/tests/benchmark_ui/extrinsic_call_out_of_fn.stderr create mode 100644 frame/support/test/tests/benchmark_ui/missing_call.rs create mode 100644 frame/support/test/tests/benchmark_ui/missing_call.stderr create mode 100644 frame/support/test/tests/benchmark_ui/missing_origin.rs create mode 100644 frame/support/test/tests/benchmark_ui/missing_origin.stderr create mode 100644 frame/support/test/tests/benchmark_ui/pass/valid_basic.rs create mode 100644 frame/support/test/tests/benchmark_ui/unrecognized_option.rs create mode 100644 frame/support/test/tests/benchmark_ui/unrecognized_option.stderr diff --git a/Cargo.lock b/Cargo.lock index 348f23d736d37..08948ffbd970b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1614,6 +1614,17 @@ dependencies = [ "rusticata-macros", ] +[[package]] +name = "derive-syn-parse" +version = "0.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e79116f119dd1dba1abf1f3405f03b9b0e79a27a3883864bfebded8a3dc768cd" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "derive_builder" version = "0.11.2" @@ -2326,6 +2337,7 @@ dependencies = [ "sp-std", "sp-tracing", "sp-weights", + "static_assertions", "tt-call", ] @@ -2335,6 +2347,7 @@ version = "4.0.0-dev" dependencies = [ "Inflector", "cfg-expr", + "derive-syn-parse", "frame-support-procedural-tools", "itertools", "proc-macro2", @@ -2366,6 +2379,7 @@ dependencies = [ name = "frame-support-test" version = "3.0.0" dependencies = [ + "frame-benchmarking", "frame-support", "frame-support-test-pallet", "frame-system", diff --git a/frame/balances/src/benchmarking.rs b/frame/balances/src/benchmarking.rs index 206adba0f044b..f85ff43b715fe 100644 --- a/frame/balances/src/benchmarking.rs +++ b/frame/balances/src/benchmarking.rs @@ -1,6 +1,6 @@ // This file is part of Substrate. -// Copyright (C) 2020-2022 Parity Technologies (UK) Ltd. +// Copyright (C) 2020-2023 Parity Technologies (UK) Ltd. // SPDX-License-Identifier: Apache-2.0 // Licensed under the Apache License, Version 2.0 (the "License"); @@ -20,22 +20,25 @@ #![cfg(feature = "runtime-benchmarks")] use super::*; +use crate::Pallet as Balances; -use frame_benchmarking::{account, benchmarks_instance_pallet, whitelisted_caller}; +use frame_benchmarking::{account, impl_benchmark_test_suite, whitelisted_caller}; +use frame_support::benchmarking::*; use frame_system::RawOrigin; -use sp_runtime::traits::Bounded; - -use crate::Pallet as Balances; const SEED: u32 = 0; // existential deposit multiplier const ED_MULTIPLIER: u32 = 10; -benchmarks_instance_pallet! { +#[instance_benchmarks] +mod benchmarks { + use super::*; + // Benchmark `transfer` extrinsic with the worst possible conditions: // * Transfer will kill the sender account. // * Transfer will create the recipient account. - transfer { + #[benchmark] + fn transfer() { let existential_deposit = T::ExistentialDeposit::get(); let caller = whitelisted_caller(); @@ -47,53 +50,66 @@ benchmarks_instance_pallet! { // and reap this user. let recipient: T::AccountId = account("recipient", 0, SEED); let recipient_lookup = T::Lookup::unlookup(recipient.clone()); - let transfer_amount = existential_deposit.saturating_mul((ED_MULTIPLIER - 1).into()) + 1u32.into(); - }: transfer(RawOrigin::Signed(caller.clone()), recipient_lookup, transfer_amount) - verify { + let transfer_amount = + existential_deposit.saturating_mul((ED_MULTIPLIER - 1).into()) + 1u32.into(); + + #[extrinsic_call] + _(RawOrigin::Signed(caller.clone()), recipient_lookup, transfer_amount); + 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 { + #[benchmark(extra)] + fn transfer_best_case() { let caller = whitelisted_caller(); let recipient: T::AccountId = account("recipient", 0, SEED); let recipient_lookup = T::Lookup::unlookup(recipient.clone()); - // Give the sender account max funds for transfer (their account will never reasonably be killed). - let _ = as Currency<_>>::make_free_balance_be(&caller, T::Balance::max_value()); + // Give the sender account max funds for transfer (their account will never reasonably be + // killed). + let _ = + 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 _ = as Currency<_>>::make_free_balance_be(&recipient, existential_deposit); + let _ = + 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 { + + #[extrinsic_call] + transfer(RawOrigin::Signed(caller.clone()), recipient_lookup, transfer_amount); + 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 { + #[benchmark] + fn transfer_keep_alive() { let caller = whitelisted_caller(); let recipient: T::AccountId = account("recipient", 0, SEED); let recipient_lookup = 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 _ = + 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 { + + #[extrinsic_call] + _(RawOrigin::Signed(caller.clone()), recipient_lookup, transfer_amount); + 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_creating { + #[benchmark] + fn set_balance_creating() { let user: T::AccountId = account("user", 0, SEED); let user_lookup = T::Lookup::unlookup(user.clone()); @@ -101,14 +117,17 @@ benchmarks_instance_pallet! { let existential_deposit = T::ExistentialDeposit::get(); 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, balance_amount, balance_amount) - verify { + + #[extrinsic_call] + set_balance(RawOrigin::Root, user_lookup, balance_amount, balance_amount); + 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 { + #[benchmark] + fn set_balance_killing() { let user: T::AccountId = account("user", 0, SEED); let user_lookup = T::Lookup::unlookup(user.clone()); @@ -116,15 +135,18 @@ benchmarks_instance_pallet! { let existential_deposit = T::ExistentialDeposit::get(); 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, Zero::zero(), Zero::zero()) - verify { + + #[extrinsic_call] + set_balance(RawOrigin::Root, user_lookup, Zero::zero(), Zero::zero()); + 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 { + #[benchmark] + fn force_transfer() { let existential_deposit = T::ExistentialDeposit::get(); let source: T::AccountId = account("source", 0, SEED); let source_lookup = T::Lookup::unlookup(source.clone()); @@ -133,12 +155,16 @@ benchmarks_instance_pallet! { 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. + // 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::unlookup(recipient.clone()); - let transfer_amount = existential_deposit.saturating_mul((ED_MULTIPLIER - 1).into()) + 1u32.into(); - }: force_transfer(RawOrigin::Root, source_lookup, recipient_lookup, transfer_amount) - verify { + let transfer_amount = + existential_deposit.saturating_mul((ED_MULTIPLIER - 1).into()) + 1u32.into(); + + #[extrinsic_call] + _(RawOrigin::Root, source_lookup, recipient_lookup, transfer_amount); + assert_eq!(Balances::::free_balance(&source), Zero::zero()); assert_eq!(Balances::::free_balance(&recipient), transfer_amount); } @@ -146,10 +172,9 @@ benchmarks_instance_pallet! { // This benchmark performs the same operation as `transfer` in the worst case scenario, // but additionally introduces many new users into the storage, increasing the the merkle // trie and PoV size. - #[extra] - transfer_increasing_users { + #[benchmark(extra)] + fn transfer_increasing_users(u: Linear<0, 1_000>) { // 1_000 is not very much, but this upper bound can be controlled by the CLI. - let u in 0 .. 1_000; let existential_deposit = T::ExistentialDeposit::get(); let caller = whitelisted_caller(); @@ -161,17 +186,20 @@ benchmarks_instance_pallet! { // and reap this user. let recipient: T::AccountId = account("recipient", 0, SEED); let recipient_lookup = T::Lookup::unlookup(recipient.clone()); - let transfer_amount = existential_deposit.saturating_mul((ED_MULTIPLIER - 1).into()) + 1u32.into(); + let transfer_amount = + existential_deposit.saturating_mul((ED_MULTIPLIER - 1).into()) + 1u32.into(); // Create a bunch of users in storage. - for i in 0 .. u { + for i in 0..u { // The `account` function uses `blake2_256` to generate unique accounts, so these // should be quite random and evenly distributed in the trie. let new_user: T::AccountId = account("new_user", i, SEED); let _ = as Currency<_>>::make_free_balance_be(&new_user, balance); } - }: transfer(RawOrigin::Signed(caller.clone()), recipient_lookup, transfer_amount) - verify { + + #[extrinsic_call] + transfer(RawOrigin::Signed(caller.clone()), recipient_lookup, transfer_amount); + assert_eq!(Balances::::free_balance(&caller), Zero::zero()); assert_eq!(Balances::::free_balance(&recipient), transfer_amount); } @@ -179,7 +207,8 @@ benchmarks_instance_pallet! { // Benchmark `transfer_all` with the worst possible condition: // * The recipient account is created // * The sender is killed - transfer_all { + #[benchmark] + fn transfer_all() { let caller = whitelisted_caller(); let recipient: T::AccountId = account("recipient", 0, SEED); let recipient_lookup = T::Lookup::unlookup(recipient.clone()); @@ -188,13 +217,16 @@ benchmarks_instance_pallet! { let existential_deposit = T::ExistentialDeposit::get(); let balance = existential_deposit.saturating_mul(ED_MULTIPLIER.into()); let _ = as Currency<_>>::make_free_balance_be(&caller, balance); - }: _(RawOrigin::Signed(caller.clone()), recipient_lookup, false) - verify { + + #[extrinsic_call] + _(RawOrigin::Signed(caller.clone()), recipient_lookup, false); + assert!(Balances::::free_balance(&caller).is_zero()); assert_eq!(Balances::::free_balance(&recipient), balance); } - force_unreserve { + #[benchmark] + fn force_unreserve() { let user: T::AccountId = account("user", 0, SEED); let user_lookup = T::Lookup::unlookup(user.clone()); @@ -208,15 +240,16 @@ benchmarks_instance_pallet! { assert_eq!(Balances::::reserved_balance(&user), balance); assert!(Balances::::free_balance(&user).is_zero()); - }: _(RawOrigin::Root, user_lookup, balance) - verify { + #[extrinsic_call] + _(RawOrigin::Root, user_lookup, balance); + assert!(Balances::::reserved_balance(&user).is_zero()); assert_eq!(Balances::::free_balance(&user), balance); } - impl_benchmark_test_suite!( + impl_benchmark_test_suite! { Balances, crate::tests_composite::ExtBuilder::default().build(), crate::tests_composite::Test, - ) + } } diff --git a/frame/examples/basic/src/benchmarking.rs b/frame/examples/basic/src/benchmarking.rs index 13f069c23e27b..699e56685fbe9 100644 --- a/frame/examples/basic/src/benchmarking.rs +++ b/frame/examples/basic/src/benchmarking.rs @@ -15,12 +15,14 @@ // See the License for the specific language governing permissions and // limitations under the License. -//! Benchmarking for pallet-example-basic. +//! Benchmarking for `pallet-example-basic`. +// Only enable this module for benchmarking. #![cfg(feature = "runtime-benchmarks")] use crate::*; -use frame_benchmarking::{benchmarks, whitelisted_caller}; +use frame_benchmarking::{impl_benchmark_test_suite, whitelisted_caller}; +use frame_support::benchmarking::{benchmarks, Linear}; use frame_system::RawOrigin; // To actually run this benchmark on pallet-example-basic, we need to put this pallet into the @@ -33,14 +35,19 @@ use frame_system::RawOrigin; // Details on using the benchmarks macro can be seen at: // https://paritytech.github.io/substrate/master/frame_benchmarking/trait.Benchmarking.html#tymethod.benchmarks -benchmarks! { +#[benchmarks] +mod benchmarks { + use super::*; + // This will measure the execution time of `set_dummy`. - set_dummy_benchmark { + #[benchmark] + fn set_dummy_benchmark() { // This is the benchmark setup phase. // `set_dummy` is a constant time function, hence we hard-code some random value here. let value = 1000u32.into(); - }: set_dummy(RawOrigin::Root, value) // The execution phase is just running `set_dummy` extrinsic call - verify { + #[extrinsic_call] + set_dummy(RawOrigin::Root, value); // The execution phase is just running `set_dummy` extrinsic call + // This is the optional benchmark verification phase, asserting certain states. assert_eq!(Pallet::::dummy(), Some(value)) } @@ -49,22 +56,43 @@ benchmarks! { // The benchmark execution phase is shorthanded. When the name of the benchmark case is the same // as the extrinsic call. `_(...)` is used to represent the extrinsic name. // The benchmark verification phase is omitted. - accumulate_dummy { + #[benchmark] + fn accumulate_dummy() { let value = 1000u32.into(); // The caller account is whitelisted for DB reads/write by the benchmarking macro. let caller: T::AccountId = whitelisted_caller(); - }: _(RawOrigin::Signed(caller), value) + + // You can use `_` if the name of the Call matches the benchmark name. + #[extrinsic_call] + _(RawOrigin::Signed(caller), value); + } + + /// You can write helper functions in here since its a normal Rust module. + fn setup_vector(len: u32) -> Vec { + let mut vector = Vec::::new(); + for i in (0..len).rev() { + vector.push(i); + } + vector + } // This will measure the execution time of sorting a vector. - sort_vector { - let x in 0 .. 10000; - let mut m = Vec::::new(); - for i in (0..x).rev() { - m.push(i); + // + // Define `x` as a linear component with range `[0, =10_000]`. This means that the benchmarking + // will assume that the weight grows at a linear rate depending on `x`. + #[benchmark] + fn sort_vector(x: Linear<0, 10_000>) { + let mut vector = setup_vector(x); + + // The benchmark execution phase could also be a closure with custom code: + #[block] + { + vector.sort(); } - }: { - // The benchmark execution phase could also be a closure with custom code - m.sort(); + + // Check that it was sorted correctly. This will not be benchmarked and is just for + // verification. + vector.windows(2).for_each(|w| assert!(w[0] <= w[1])); } // This line generates test cases for benchmarking, and could be run by: @@ -75,5 +103,5 @@ benchmarks! { // // The line generates three steps per benchmark, with repeat=1 and the three steps are // [low, mid, high] of the range. - impl_benchmark_test_suite!(Pallet, crate::tests::new_test_ext(), crate::tests::Test) + impl_benchmark_test_suite!(Pallet, crate::tests::new_test_ext(), crate::tests::Test); } diff --git a/frame/message-queue/src/benchmarking.rs b/frame/message-queue/src/benchmarking.rs index 9cd6b75e4d0ae..9651c81e5e66d 100644 --- a/frame/message-queue/src/benchmarking.rs +++ b/frame/message-queue/src/benchmarking.rs @@ -1,6 +1,6 @@ // This file is part of Substrate. -// Copyright (C) 2022 Parity Technologies (UK) Ltd. +// Copyright (C) 2023 Parity Technologies (UK) Ltd. // SPDX-License-Identifier: Apache-2.0 // Licensed under the Apache License, Version 2.0 (the "License"); @@ -22,68 +22,85 @@ use super::{mock_helpers::*, Pallet as MessageQueue, *}; -use frame_benchmarking::{benchmarks, whitelisted_caller}; -use frame_support::traits::Get; +use frame_benchmarking::{impl_benchmark_test_suite, whitelisted_caller}; +use frame_support::{benchmarking::*, traits::Get}; use frame_system::RawOrigin; use sp_std::prelude::*; -benchmarks! { - where_clause { - where - // NOTE: We need to generate multiple origins, therefore Origin is `From`. The - // `PartialEq` is for asserting the outcome of the ring (un)knitting and *could* be - // removed if really necessary. - <::MessageProcessor as ProcessMessage>::Origin: From + PartialEq, - ::Size: From, - } +#[benchmarks( + where + <::MessageProcessor as ProcessMessage>::Origin: From + PartialEq, + ::Size: From, + // NOTE: We need to generate multiple origins, therefore Origin is `From`. The + // `PartialEq` is for asserting the outcome of the ring (un)knitting and *could* be + // removed if really necessary. +)] +mod benchmarks { + use super::*; // Worst case path of `ready_ring_knit`. - ready_ring_knit { - let mid: MessageOriginOf:: = 1.into(); + #[benchmark] + fn ready_ring_knit() { + let mid: MessageOriginOf = 1.into(); build_ring::(&[0.into(), mid.clone(), 2.into()]); unknit::(&mid); assert_ring::(&[0.into(), 2.into()]); let mut neighbours = None; - }: { - neighbours = MessageQueue::::ready_ring_knit(&mid).ok(); - } verify { + + #[block] + { + neighbours = MessageQueue::::ready_ring_knit(&mid).ok(); + } + // The neighbours needs to be modified manually. - BookStateFor::::mutate(&mid, |b| { b.ready_neighbours = neighbours }); + BookStateFor::::mutate(&mid, |b| b.ready_neighbours = neighbours); assert_ring::(&[0.into(), 2.into(), mid]); } // Worst case path of `ready_ring_unknit`. - ready_ring_unknit { + #[benchmark] + fn ready_ring_unknit() { build_ring::(&[0.into(), 1.into(), 2.into()]); assert_ring::(&[0.into(), 1.into(), 2.into()]); - let o: MessageOriginOf:: = 0.into(); + let o: MessageOriginOf = 0.into(); let neighbours = BookStateFor::::get(&o).ready_neighbours.unwrap(); - }: { - MessageQueue::::ready_ring_unknit(&o, neighbours); - } verify { + + #[block] + { + MessageQueue::::ready_ring_unknit(&o, neighbours); + } + assert_ring::(&[1.into(), 2.into()]); } // `service_queues` without any queue processing. - service_queue_base { - }: { - MessageQueue::::service_queue(0.into(), &mut WeightMeter::max_limit(), Weight::MAX) + #[benchmark] + fn service_queue_base() { + #[block] + { + MessageQueue::::service_queue(0.into(), &mut WeightMeter::max_limit(), Weight::MAX); + } } // `service_page` without any message processing but with page completion. - service_page_base_completion { + #[benchmark] + fn service_page_base_completion() { let origin: MessageOriginOf = 0.into(); let page = PageOf::::default(); Pages::::insert(&origin, 0, &page); let mut book_state = single_page_book::(); let mut meter = WeightMeter::max_limit(); let limit = Weight::MAX; - }: { - MessageQueue::::service_page(&origin, &mut book_state, &mut meter, limit) + + #[block] + { + MessageQueue::::service_page(&origin, &mut book_state, &mut meter, limit); + } } // `service_page` without any message processing and without page completion. - service_page_base_no_completion { + #[benchmark] + fn service_page_base_no_completion() { let origin: MessageOriginOf = 0.into(); let mut page = PageOf::::default(); // Mock the storage such that `is_complete` returns `false` but `peek_first` returns `None`. @@ -93,49 +110,73 @@ benchmarks! { let mut book_state = single_page_book::(); let mut meter = WeightMeter::max_limit(); let limit = Weight::MAX; - }: { - MessageQueue::::service_page(&origin, &mut book_state, &mut meter, limit) + + #[block] + { + MessageQueue::::service_page(&origin, &mut book_state, &mut meter, limit); + } } // Processing a single message from a page. - service_page_item { + #[benchmark] + fn service_page_item() { let msg = vec![1u8; MaxMessageLenOf::::get() as usize]; let mut page = page::(&msg.clone()); let mut book = book_for::(&page); assert!(page.peek_first().is_some(), "There is one message"); let mut weight = WeightMeter::max_limit(); - }: { - let status = MessageQueue::::service_page_item(&0u32.into(), 0, &mut book, &mut page, &mut weight, Weight::MAX); - assert_eq!(status, ItemExecutionStatus::Executed(true)); - } verify { + + #[block] + { + let status = MessageQueue::::service_page_item( + &0u32.into(), + 0, + &mut book, + &mut page, + &mut weight, + Weight::MAX, + ); + assert_eq!(status, ItemExecutionStatus::Executed(true)); + } + // Check that it was processed. - assert_last_event::(Event::Processed { - hash: T::Hashing::hash(&msg), origin: 0.into(), - weight_used: 1.into_weight(), success: true - }.into()); + assert_last_event::( + Event::Processed { + hash: T::Hashing::hash(&msg), + origin: 0.into(), + weight_used: 1.into_weight(), + success: true, + } + .into(), + ); let (_, processed, _) = page.peek_index(0).unwrap(); assert!(processed); assert_eq!(book.message_count, 0); } // Worst case for calling `bump_service_head`. - bump_service_head { + #[benchmark] + fn bump_service_head() { setup_bump_service_head::(0.into(), 10.into()); let mut weight = WeightMeter::max_limit(); - }: { - MessageQueue::::bump_service_head(&mut weight); - } verify { + + #[block] + { + MessageQueue::::bump_service_head(&mut weight); + } + assert_eq!(ServiceHead::::get().unwrap(), 10u32.into()); assert_eq!(weight.consumed, T::WeightInfo::bump_service_head()); } - reap_page { + #[benchmark] + fn reap_page() { // Mock the storage to get a *cullable* but not *reapable* page. let origin: MessageOriginOf = 0.into(); let mut book = single_page_book::(); let (page, msgs) = full_page::(); - for p in 0 .. T::MaxStale::get() * T::MaxStale::get() { + for p in 0..T::MaxStale::get() * T::MaxStale::get() { if p == 0 { Pages::::insert(&origin, p, &page); } @@ -148,16 +189,19 @@ benchmarks! { BookStateFor::::insert(&origin, &book); assert!(Pages::::contains_key(&origin, 0)); - }: _(RawOrigin::Signed(whitelisted_caller()), 0u32.into(), 0) - verify { - assert_last_event::(Event::PageReaped{ origin: 0.into(), index: 0 }.into()); + #[extrinsic_call] + _(RawOrigin::Signed(whitelisted_caller()), 0u32.into(), 0); + + assert_last_event::(Event::PageReaped { origin: 0.into(), index: 0 }.into()); assert!(!Pages::::contains_key(&origin, 0)); } // Worst case for `execute_overweight` where the page is removed as completed. // - // The worst case occurs when executing the last message in a page of which all are skipped since it is using `peek_index` which has linear complexities. - execute_overweight_page_removed { + // The worst case occurs when executing the last message in a page of which all are skipped + // since it is using `peek_index` which has linear complexities. + #[benchmark] + fn execute_overweight_page_removed() { let origin: MessageOriginOf = 0.into(); let (mut page, msgs) = full_page::(); // Skip all messages. @@ -168,19 +212,34 @@ benchmarks! { let book = book_for::(&page); Pages::::insert(&origin, 0, &page); BookStateFor::::insert(&origin, &book); - }: { - MessageQueue::::execute_overweight(RawOrigin::Signed(whitelisted_caller()).into(), 0u32.into(), 0u32, ((msgs - 1) as u32).into(), Weight::MAX).unwrap() - } - verify { - assert_last_event::(Event::Processed { - hash: T::Hashing::hash(&((msgs - 1) as u32).encode()), origin: 0.into(), - weight_used: Weight::from_parts(1, 1), success: true - }.into()); + + #[block] + { + MessageQueue::::execute_overweight( + RawOrigin::Signed(whitelisted_caller()).into(), + 0u32.into(), + 0u32, + ((msgs - 1) as u32).into(), + Weight::MAX, + ) + .unwrap(); + } + + assert_last_event::( + Event::Processed { + hash: T::Hashing::hash(&((msgs - 1) as u32).encode()), + origin: 0.into(), + weight_used: Weight::from_parts(1, 1), + success: true, + } + .into(), + ); assert!(!Pages::::contains_key(&origin, 0), "Page must be removed"); } // Worst case for `execute_overweight` where the page is updated. - execute_overweight_page_updated { + #[benchmark] + fn execute_overweight_page_updated() { let origin: MessageOriginOf = 0.into(); let (mut page, msgs) = full_page::(); // Skip all messages. @@ -190,16 +249,34 @@ benchmarks! { let book = book_for::(&page); Pages::::insert(&origin, 0, &page); BookStateFor::::insert(&origin, &book); - }: { - MessageQueue::::execute_overweight(RawOrigin::Signed(whitelisted_caller()).into(), 0u32.into(), 0u32, ((msgs - 1) as u32).into(), Weight::MAX).unwrap() - } - verify { - assert_last_event::(Event::Processed { - hash: T::Hashing::hash(&((msgs - 1) as u32).encode()), origin: 0.into(), - weight_used: Weight::from_parts(1, 1), success: true - }.into()); + + #[block] + { + MessageQueue::::execute_overweight( + RawOrigin::Signed(whitelisted_caller()).into(), + 0u32.into(), + 0u32, + ((msgs - 1) as u32).into(), + Weight::MAX, + ) + .unwrap(); + } + + assert_last_event::( + Event::Processed { + hash: T::Hashing::hash(&((msgs - 1) as u32).encode()), + origin: 0.into(), + weight_used: Weight::from_parts(1, 1), + success: true, + } + .into(), + ); assert!(Pages::::contains_key(&origin, 0), "Page must be updated"); } - impl_benchmark_test_suite!(MessageQueue, crate::mock::new_test_ext::(), crate::integration_test::Test); + impl_benchmark_test_suite! { + MessageQueue, + crate::mock::new_test_ext::(), + crate::integration_test::Test + } } diff --git a/frame/support/Cargo.toml b/frame/support/Cargo.toml index 4f62ae42ef78f..ae2fc358974b6 100644 --- a/frame/support/Cargo.toml +++ b/frame/support/Cargo.toml @@ -27,6 +27,7 @@ sp-arithmetic = { version = "6.0.0", default-features = false, path = "../../pri sp-inherents = { version = "4.0.0-dev", default-features = false, path = "../../primitives/inherents" } sp-staking = { version = "4.0.0-dev", default-features = false, path = "../../primitives/staking" } sp-weights = { version = "4.0.0", default-features = false, path = "../../primitives/weights" } +static_assertions = "1.1.0" tt-call = "1.0.8" frame-support-procedural = { version = "4.0.0-dev", default-features = false, path = "./procedural" } paste = "1.0" diff --git a/frame/support/procedural/Cargo.toml b/frame/support/procedural/Cargo.toml index 06b8056aff982..ee1ca4dff8873 100644 --- a/frame/support/procedural/Cargo.toml +++ b/frame/support/procedural/Cargo.toml @@ -15,6 +15,7 @@ targets = ["x86_64-unknown-linux-gnu"] proc-macro = true [dependencies] +derive-syn-parse = "0.1.5" Inflector = "0.11.4" cfg-expr = "0.10.3" itertools = "0.10.3" diff --git a/frame/support/procedural/src/benchmark.rs b/frame/support/procedural/src/benchmark.rs new file mode 100644 index 0000000000000..43e3e47de5259 --- /dev/null +++ b/frame/support/procedural/src/benchmark.rs @@ -0,0 +1,860 @@ +// This file is part of Substrate. + +// Copyright (C) 2023 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. + +//! Home of the parsing and expansion code for the new pallet benchmarking syntax + +use derive_syn_parse::Parse; +use frame_support_procedural_tools::generate_crate_access_2018; +use proc_macro::TokenStream; +use proc_macro2::{Ident, Span, TokenStream as TokenStream2}; +use quote::{quote, quote_spanned, ToTokens}; +use syn::{ + parenthesized, + parse::{Nothing, ParseStream}, + punctuated::Punctuated, + spanned::Spanned, + token::{Colon2, Comma, Gt, Lt, Paren}, + Attribute, Error, Expr, ExprBlock, ExprCall, ExprPath, FnArg, Item, ItemFn, ItemMod, LitInt, + Pat, Path, PathArguments, PathSegment, Result, Stmt, Token, Type, WhereClause, +}; + +mod keywords { + use syn::custom_keyword; + + custom_keyword!(benchmark); + custom_keyword!(benchmarks); + custom_keyword!(block); + custom_keyword!(extra); + custom_keyword!(extrinsic_call); + custom_keyword!(skip_meta); +} + +/// This represents the raw parsed data for a param definition such as `x: Linear<10, 20>`. +#[derive(Clone)] +struct ParamDef { + name: String, + typ: Type, + start: u32, + end: u32, +} + +/// Allows easy parsing of the `<10, 20>` component of `x: Linear<10, 20>`. +#[derive(Parse)] +struct RangeArgs { + _lt_token: Lt, + start: LitInt, + _comma: Comma, + end: LitInt, + _gt_token: Gt, +} + +#[derive(Clone, Debug)] +struct BenchmarkAttrs { + skip_meta: bool, + extra: bool, +} + +/// Represents a single benchmark option +enum BenchmarkAttrKeyword { + Extra, + SkipMeta, +} + +impl syn::parse::Parse for BenchmarkAttrKeyword { + fn parse(input: ParseStream) -> Result { + let lookahead = input.lookahead1(); + if lookahead.peek(keywords::extra) { + let _extra: keywords::extra = input.parse()?; + return Ok(BenchmarkAttrKeyword::Extra) + } else if lookahead.peek(keywords::skip_meta) { + let _skip_meta: keywords::skip_meta = input.parse()?; + return Ok(BenchmarkAttrKeyword::SkipMeta) + } else { + return Err(lookahead.error()) + } + } +} + +impl syn::parse::Parse for BenchmarkAttrs { + fn parse(input: ParseStream) -> syn::Result { + let lookahead = input.lookahead1(); + if !lookahead.peek(Paren) { + let _nothing: Nothing = input.parse()?; + return Ok(BenchmarkAttrs { skip_meta: false, extra: false }) + } + let content; + let _paren: Paren = parenthesized!(content in input); + let mut extra = false; + let mut skip_meta = false; + let args = Punctuated::::parse_terminated(&content)?; + for arg in args.into_iter() { + match arg { + BenchmarkAttrKeyword::Extra => { + if extra { + return Err(content.error("`extra` can only be specified once")) + } + extra = true; + }, + BenchmarkAttrKeyword::SkipMeta => { + if skip_meta { + return Err(content.error("`skip_meta` can only be specified once")) + } + skip_meta = true; + }, + } + } + Ok(BenchmarkAttrs { extra, skip_meta }) + } +} + +/// Represents the parsed extrinsic call for a benchmark +#[derive(Clone)] +enum BenchmarkCallDef { + ExtrinsicCall { origin: Expr, expr_call: ExprCall, attr_span: Span }, // #[extrinsic_call] + Block { block: ExprBlock, attr_span: Span }, // #[block] +} + +impl BenchmarkCallDef { + /// Returns the `span()` for attribute + fn attr_span(&self) -> Span { + match self { + BenchmarkCallDef::ExtrinsicCall { origin: _, expr_call: _, attr_span } => *attr_span, + BenchmarkCallDef::Block { block: _, attr_span } => *attr_span, + } + } +} + +/// Represents a parsed `#[benchmark]` or `#[instance_banchmark]` item. +#[derive(Clone)] +struct BenchmarkDef { + params: Vec, + setup_stmts: Vec, + call_def: BenchmarkCallDef, + verify_stmts: Vec, + extra: bool, + skip_meta: bool, +} + +impl BenchmarkDef { + /// Constructs a [`BenchmarkDef`] by traversing an existing [`ItemFn`] node. + pub fn from(item_fn: &ItemFn, extra: bool, skip_meta: bool) -> Result { + let mut params: Vec = Vec::new(); + + // parse params such as "x: Linear<0, 1>" + for arg in &item_fn.sig.inputs { + let invalid_param = |span| { + return Err(Error::new(span, "Invalid benchmark function param. A valid example would be `x: Linear<5, 10>`.", )) + }; + + let FnArg::Typed(arg) = arg else { return invalid_param(arg.span()) }; + let Pat::Ident(ident) = &*arg.pat else { return invalid_param(arg.span()) }; + + // check param name + let var_span = ident.span(); + let invalid_param_name = || { + return Err(Error::new( + var_span, + "Benchmark parameter names must consist of a single lowercase letter (a-z) and no other characters.", + )) + }; + let name = ident.ident.to_token_stream().to_string(); + if name.len() > 1 { + return invalid_param_name() + }; + let Some(name_char) = name.chars().next() else { return invalid_param_name() }; + if !name_char.is_alphabetic() || !name_char.is_lowercase() { + return invalid_param_name() + } + + // parse type + let typ = &*arg.ty; + let Type::Path(tpath) = typ else { return invalid_param(typ.span()) }; + let Some(segment) = tpath.path.segments.last() else { return invalid_param(typ.span()) }; + let args = segment.arguments.to_token_stream().into(); + let Ok(args) = syn::parse::(args) else { return invalid_param(typ.span()) }; + let Ok(start) = args.start.base10_parse::() else { return invalid_param(args.start.span()) }; + let Ok(end) = args.end.base10_parse::() else { return invalid_param(args.end.span()) }; + + if end < start { + return Err(Error::new( + args.start.span(), + "The start of a `ParamRange` must be less than or equal to the end", + )) + } + + params.push(ParamDef { name, typ: typ.clone(), start, end }); + } + + // #[extrinsic_call] / #[block] handling + let call_defs = item_fn.block.stmts.iter().enumerate().filter_map(|(i, child)| { + if let Stmt::Semi(Expr::Call(expr_call), _semi) = child { + // #[extrinsic_call] case + expr_call.attrs.iter().enumerate().find_map(|(k, attr)| { + let segment = attr.path.segments.last()?; + let _: keywords::extrinsic_call = syn::parse(segment.ident.to_token_stream().into()).ok()?; + let mut expr_call = expr_call.clone(); + + // consume #[extrinsic_call] tokens + expr_call.attrs.remove(k); + + // extract origin from expr_call + let Some(origin) = expr_call.args.first().cloned() else { + return Some(Err(Error::new(expr_call.span(), "Single-item extrinsic calls must specify their origin as the first argument."))) + }; + + Some(Ok((i, BenchmarkCallDef::ExtrinsicCall { origin, expr_call, attr_span: attr.span() }))) + }) + } else if let Stmt::Expr(Expr::Block(block)) = child { + // #[block] case + block.attrs.iter().enumerate().find_map(|(k, attr)| { + let segment = attr.path.segments.last()?; + let _: keywords::block = syn::parse(segment.ident.to_token_stream().into()).ok()?; + let mut block = block.clone(); + + // consume #[block] tokens + block.attrs.remove(k); + + Some(Ok((i, BenchmarkCallDef::Block { block, attr_span: attr.span() }))) + }) + } else { + None + } + }).collect::>>()?; + let (i, call_def) = match &call_defs[..] { + [(i, call_def)] => (*i, call_def.clone()), // = 1 + [] => return Err(Error::new( // = 0 + item_fn.block.brace_token.span, + "No valid #[extrinsic_call] or #[block] annotation could be found in benchmark function body." + )), + _ => return Err(Error::new( // > 1 + call_defs[1].1.attr_span(), + "Only one #[extrinsic_call] or #[block] attribute is allowed per benchmark." + )), + }; + + Ok(BenchmarkDef { + params, + setup_stmts: Vec::from(&item_fn.block.stmts[0..i]), + call_def, + verify_stmts: Vec::from(&item_fn.block.stmts[(i + 1)..item_fn.block.stmts.len()]), + extra, + skip_meta, + }) + } +} + +/// Parses and expands a `#[benchmarks]` or `#[instance_benchmarks]` invocation +pub fn benchmarks( + attrs: TokenStream, + tokens: TokenStream, + instance: bool, +) -> syn::Result { + // gather module info + let module: ItemMod = syn::parse(tokens)?; + let mod_span = module.span(); + let where_clause = match syn::parse::(attrs.clone()) { + Ok(_) => quote!(), + Err(_) => syn::parse::(attrs)?.predicates.to_token_stream(), + }; + let mod_vis = module.vis; + let mod_name = module.ident; + + // consume #[benchmarks] attribute by exclusing it from mod_attrs + let mod_attrs: Vec<&Attribute> = module + .attrs + .iter() + .filter(|attr| syn::parse2::(attr.to_token_stream()).is_err()) + .collect(); + + let mut benchmark_names: Vec = Vec::new(); + let mut extra_benchmark_names: Vec = Vec::new(); + let mut skip_meta_benchmark_names: Vec = Vec::new(); + + let (_brace, mut content) = + module.content.ok_or(syn::Error::new(mod_span, "Module cannot be empty!"))?; + + // find all function defs marked with #[benchmark] + let benchmark_fn_metas = content.iter_mut().filter_map(|stmt| { + // parse as a function def first + let Item::Fn(func) = stmt else { return None }; + + // find #[benchmark] attribute on function def + let benchmark_attr = func.attrs.iter().find_map(|attr| { + let seg = attr.path.segments.last()?; + syn::parse::(seg.ident.to_token_stream().into()).ok()?; + Some(attr) + })?; + + Some((benchmark_attr.clone(), func.clone(), stmt)) + }); + + // parse individual benchmark defs and args + for (benchmark_attr, func, stmt) in benchmark_fn_metas { + // parse any args provided to #[benchmark] + let attr_tokens = benchmark_attr.tokens.to_token_stream().into(); + let benchmark_args: BenchmarkAttrs = syn::parse(attr_tokens)?; + + // parse benchmark def + let benchmark_def = + BenchmarkDef::from(&func, benchmark_args.extra, benchmark_args.skip_meta)?; + + // record benchmark name + let name = &func.sig.ident; + benchmark_names.push(name.clone()); + + // record name sets + if benchmark_def.extra { + extra_benchmark_names.push(name.clone()); + } + if benchmark_def.skip_meta { + skip_meta_benchmark_names.push(name.clone()) + } + + // expand benchmark + let expanded = expand_benchmark(benchmark_def, name, instance, where_clause.clone()); + + // replace original function def with expanded code + *stmt = Item::Verbatim(expanded); + } + + // generics + let type_use_generics = match instance { + false => quote!(T), + true => quote!(T, I), + }; + let type_impl_generics = match instance { + false => quote!(T: Config), + true => quote!(T: Config, I: 'static), + }; + + let krate = generate_crate_access_2018("frame-benchmarking")?; + let support = quote!(#krate::frame_support); + + // benchmark name variables + let benchmark_names_str: Vec = benchmark_names.iter().map(|n| n.to_string()).collect(); + let extra_benchmark_names_str: Vec = + extra_benchmark_names.iter().map(|n| n.to_string()).collect(); + let skip_meta_benchmark_names_str: Vec = + skip_meta_benchmark_names.iter().map(|n| n.to_string()).collect(); + let mut selected_benchmark_mappings: Vec = Vec::new(); + let mut benchmarks_by_name_mappings: Vec = Vec::new(); + let test_idents: Vec = benchmark_names_str + .iter() + .map(|n| Ident::new(format!("test_{}", n).as_str(), Span::call_site())) + .collect(); + for i in 0..benchmark_names.len() { + let name_ident = &benchmark_names[i]; + let name_str = &benchmark_names_str[i]; + let test_ident = &test_idents[i]; + selected_benchmark_mappings.push(quote!(#name_str => SelectedBenchmark::#name_ident)); + benchmarks_by_name_mappings.push(quote!(#name_str => Self::#test_ident())) + } + + // emit final quoted tokens + let res = quote! { + #(#mod_attrs) + * + #mod_vis mod #mod_name { + #(#content) + * + + #[allow(non_camel_case_types)] + enum SelectedBenchmark { + #(#benchmark_names), + * + } + + impl<#type_impl_generics> #krate::BenchmarkingSetup<#type_use_generics> for SelectedBenchmark where #where_clause { + fn components(&self) -> #krate::Vec<(#krate::BenchmarkParameter, u32, u32)> { + match self { + #( + Self::#benchmark_names => { + <#benchmark_names as #krate::BenchmarkingSetup<#type_use_generics>>::components(&#benchmark_names) + } + ) + * + } + } + + fn instance( + &self, + components: &[(#krate::BenchmarkParameter, u32)], + verify: bool, + ) -> Result< + #krate::Box Result<(), #krate::BenchmarkError>>, + #krate::BenchmarkError, + > { + match self { + #( + Self::#benchmark_names => { + <#benchmark_names as #krate::BenchmarkingSetup< + #type_use_generics + >>::instance(&#benchmark_names, components, verify) + } + ) + * + } + } + } + #[cfg(any(feature = "runtime-benchmarks", test))] + impl<#type_impl_generics> #krate::Benchmarking for Pallet<#type_use_generics> + where T: frame_system::Config, #where_clause + { + fn benchmarks( + extra: bool, + ) -> #krate::Vec<#krate::BenchmarkMetadata> { + let mut all_names = #krate::vec![ + #(#benchmark_names_str), + * + ]; + if !extra { + let extra = [ + #(#extra_benchmark_names_str), + * + ]; + all_names.retain(|x| !extra.contains(x)); + } + all_names.into_iter().map(|benchmark| { + let selected_benchmark = match benchmark { + #(#selected_benchmark_mappings), + *, + _ => panic!("all benchmarks should be selectable") + }; + let components = >::components(&selected_benchmark); + #krate::BenchmarkMetadata { + name: benchmark.as_bytes().to_vec(), + components, + } + }).collect::<#krate::Vec<_>>() + } + + fn run_benchmark( + extrinsic: &[u8], + c: &[(#krate::BenchmarkParameter, u32)], + whitelist: &[#krate::TrackedStorageKey], + verify: bool, + internal_repeats: u32, + ) -> Result<#krate::Vec<#krate::BenchmarkResult>, #krate::BenchmarkError> { + let extrinsic = #krate::str::from_utf8(extrinsic).map_err(|_| "`extrinsic` is not a valid utf-8 string!")?; + let selected_benchmark = match extrinsic { + #(#selected_benchmark_mappings), + *, + _ => return Err("Could not find extrinsic.".into()), + }; + let mut whitelist = whitelist.to_vec(); + let whitelisted_caller_key = as #support::storage::StorageMap<_, _,>>::hashed_key_for( + #krate::whitelisted_caller::() + ); + whitelist.push(whitelisted_caller_key.into()); + let transactional_layer_key = #krate::TrackedStorageKey::new( + #support::storage::transactional::TRANSACTION_LEVEL_KEY.into(), + ); + whitelist.push(transactional_layer_key); + #krate::benchmarking::set_whitelist(whitelist); + let mut results: #krate::Vec<#krate::BenchmarkResult> = #krate::Vec::new(); + + // Always do at least one internal repeat... + for _ in 0 .. internal_repeats.max(1) { + // Always reset the state after the benchmark. + #krate::defer!(#krate::benchmarking::wipe_db()); + + // Set up the externalities environment for the setup we want to + // benchmark. + let closure_to_benchmark = < + SelectedBenchmark as #krate::BenchmarkingSetup<#type_use_generics> + >::instance(&selected_benchmark, c, verify)?; + + // Set the block number to at least 1 so events are deposited. + if #krate::Zero::is_zero(&frame_system::Pallet::::block_number()) { + frame_system::Pallet::::set_block_number(1u32.into()); + } + + // Commit the externalities to the database, flushing the DB cache. + // This will enable worst case scenario for reading from the database. + #krate::benchmarking::commit_db(); + + // Reset the read/write counter so we don't count operations in the setup process. + #krate::benchmarking::reset_read_write_count(); + + // Time the extrinsic logic. + #krate::log::trace!( + target: "benchmark", + "Start Benchmark: {} ({:?})", + extrinsic, + c + ); + + let start_pov = #krate::benchmarking::proof_size(); + let start_extrinsic = #krate::benchmarking::current_time(); + + closure_to_benchmark()?; + + let finish_extrinsic = #krate::benchmarking::current_time(); + let end_pov = #krate::benchmarking::proof_size(); + + // Calculate the diff caused by the benchmark. + let elapsed_extrinsic = finish_extrinsic.saturating_sub(start_extrinsic); + let diff_pov = match (start_pov, end_pov) { + (Some(start), Some(end)) => end.saturating_sub(start), + _ => Default::default(), + }; + + // Commit the changes to get proper write count + #krate::benchmarking::commit_db(); + #krate::log::trace!( + target: "benchmark", + "End Benchmark: {} ns", elapsed_extrinsic + ); + let read_write_count = #krate::benchmarking::read_write_count(); + #krate::log::trace!( + target: "benchmark", + "Read/Write Count {:?}", read_write_count + ); + + // Time the storage root recalculation. + let start_storage_root = #krate::benchmarking::current_time(); + #krate::storage_root(#krate::StateVersion::V1); + let finish_storage_root = #krate::benchmarking::current_time(); + let elapsed_storage_root = finish_storage_root - start_storage_root; + + let skip_meta = [ #(#skip_meta_benchmark_names_str),* ]; + let read_and_written_keys = if skip_meta.contains(&extrinsic) { + #krate::vec![(b"Skipped Metadata".to_vec(), 0, 0, false)] + } else { + #krate::benchmarking::get_read_and_written_keys() + }; + + results.push(#krate::BenchmarkResult { + components: c.to_vec(), + extrinsic_time: elapsed_extrinsic, + storage_root_time: elapsed_storage_root, + reads: read_write_count.0, + repeat_reads: read_write_count.1, + writes: read_write_count.2, + repeat_writes: read_write_count.3, + proof_size: diff_pov, + keys: read_and_written_keys, + }); + } + + return Ok(results); + } + } + + #[cfg(test)] + impl<#type_impl_generics> Pallet<#type_use_generics> where T: ::frame_system::Config, #where_clause { + /// Test a particular benchmark by name. + /// + /// This isn't called `test_benchmark_by_name` just in case some end-user eventually + /// writes a benchmark, itself called `by_name`; the function would be shadowed in + /// that case. + /// + /// This is generally intended to be used by child test modules such as those created + /// by the `impl_benchmark_test_suite` macro. However, it is not an error if a pallet + /// author chooses not to implement benchmarks. + #[allow(unused)] + fn test_bench_by_name(name: &[u8]) -> Result<(), #krate::BenchmarkError> { + let name = #krate::str::from_utf8(name) + .map_err(|_| -> #krate::BenchmarkError { "`name` is not a valid utf8 string!".into() })?; + match name { + #(#benchmarks_by_name_mappings), + *, + _ => Err("Could not find test for requested benchmark.".into()), + } + } + } + } + #mod_vis use #mod_name::*; + }; + Ok(res.into()) +} + +/// Prepares a [`Vec`] to be interpolated by [`quote!`] by creating easily-iterable +/// arrays formatted in such a way that they can be interpolated directly. +struct UnrolledParams { + param_ranges: Vec, + param_names: Vec, + param_types: Vec, +} + +impl UnrolledParams { + /// Constructs an [`UnrolledParams`] from a [`Vec`] + fn from(params: &Vec) -> UnrolledParams { + let param_ranges: Vec = params + .iter() + .map(|p| { + let name = Ident::new(&p.name, Span::call_site()); + let start = p.start; + let end = p.end; + quote!(#name, #start, #end) + }) + .collect(); + let param_names: Vec = params + .iter() + .map(|p| { + let name = Ident::new(&p.name, Span::call_site()); + quote!(#name) + }) + .collect(); + let param_types: Vec = params + .iter() + .map(|p| { + let typ = &p.typ; + quote!(#typ) + }) + .collect(); + UnrolledParams { param_ranges, param_names, param_types } + } +} + +/// Performs expansion of an already-parsed [`BenchmarkDef`]. +fn expand_benchmark( + benchmark_def: BenchmarkDef, + name: &Ident, + is_instance: bool, + where_clause: TokenStream2, +) -> TokenStream2 { + // set up variables needed during quoting + let krate = match generate_crate_access_2018("frame-benchmarking") { + Ok(ident) => ident, + Err(err) => return err.to_compile_error().into(), + }; + let home = quote!(#krate::frame_support::benchmarking); + let codec = quote!(#krate::frame_support::codec); + let traits = quote!(#krate::frame_support::traits); + let setup_stmts = benchmark_def.setup_stmts; + let verify_stmts = benchmark_def.verify_stmts; + let test_ident = Ident::new(format!("test_{}", name.to_string()).as_str(), Span::call_site()); + + // unroll params (prepare for quoting) + let unrolled = UnrolledParams::from(&benchmark_def.params); + let param_names = unrolled.param_names; + let param_ranges = unrolled.param_ranges; + let param_types = unrolled.param_types; + + let type_use_generics = match is_instance { + false => quote!(T), + true => quote!(T, I), + }; + + let type_impl_generics = match is_instance { + false => quote!(T: Config), + true => quote!(T: Config, I: 'static), + }; + + let (pre_call, post_call) = match benchmark_def.call_def { + BenchmarkCallDef::ExtrinsicCall { origin, expr_call, attr_span: _ } => { + let mut expr_call = expr_call.clone(); + + // remove first arg from expr_call + let mut final_args = Punctuated::::new(); + let args: Vec<&Expr> = expr_call.args.iter().collect(); + for arg in &args[1..] { + final_args.push((*(*arg)).clone()); + } + expr_call.args = final_args; + + // determine call name (handles `_` and normal call syntax) + let expr_span = expr_call.span(); + let call_err = || { + quote_spanned!(expr_span=> "Extrinsic call must be a function call or `_`".to_compile_error()).into() + }; + let call_name = match *expr_call.func { + Expr::Path(expr_path) => { + // normal function call + let Some(segment) = expr_path.path.segments.last() else { return call_err(); }; + segment.ident.to_string() + }, + Expr::Verbatim(tokens) => { + // `_` style + // replace `_` with fn name + let Ok(_) = syn::parse::(tokens.to_token_stream().into()) else { return call_err(); }; + name.to_string() + }, + _ => return call_err(), + }; + + // modify extrinsic call to be prefixed with "new_call_variant" + let call_name = format!("new_call_variant_{}", call_name); + let mut punct: Punctuated = Punctuated::new(); + punct.push(PathSegment { + arguments: PathArguments::None, + ident: Ident::new(call_name.as_str(), Span::call_site()), + }); + *expr_call.func = Expr::Path(ExprPath { + attrs: vec![], + qself: None, + path: Path { leading_colon: None, segments: punct }, + }); + + ( + // (pre_call, post_call): + quote! { + let __call = Call::<#type_use_generics>::#expr_call; + let __benchmarked_call_encoded = #codec::Encode::encode(&__call); + }, + quote! { + let __call_decoded = as #codec::Decode> + ::decode(&mut &__benchmarked_call_encoded[..]) + .expect("call is encoded above, encoding must be correct"); + let __origin = #origin.into(); + as #traits::UnfilteredDispatchable>::dispatch_bypass_filter( + __call_decoded, + __origin, + )?; + }, + ) + }, + BenchmarkCallDef::Block { block, attr_span: _ } => (quote!(), quote!(#block)), + }; + + // generate final quoted tokens + let res = quote! { + // compile-time assertions that each referenced param type implements ParamRange + #( + #home::assert_impl_all!(#param_types: #home::ParamRange); + )* + + #[allow(non_camel_case_types)] + struct #name; + + #[allow(unused_variables)] + impl<#type_impl_generics> #krate::BenchmarkingSetup<#type_use_generics> + for #name where #where_clause { + fn components(&self) -> #krate::Vec<(#krate::BenchmarkParameter, u32, u32)> { + #krate::vec! [ + #( + (#krate::BenchmarkParameter::#param_ranges) + ),* + ] + } + + fn instance( + &self, + components: &[(#krate::BenchmarkParameter, u32)], + verify: bool + ) -> Result<#krate::Box Result<(), #krate::BenchmarkError>>, #krate::BenchmarkError> { + #( + // prepare instance #param_names + let #param_names = components.iter() + .find(|&c| c.0 == #krate::BenchmarkParameter::#param_names) + .ok_or("Could not find component during benchmark preparation.")? + .1; + )* + + // benchmark setup code + #( + #setup_stmts + )* + #pre_call + Ok(#krate::Box::new(move || -> Result<(), #krate::BenchmarkError> { + #post_call + if verify { + #( + #verify_stmts + )* + } + Ok(()) + })) + } + } + + #[cfg(test)] + impl<#type_impl_generics> Pallet<#type_use_generics> where T: ::frame_system::Config, #where_clause { + #[allow(unused)] + fn #test_ident() -> Result<(), #krate::BenchmarkError> { + let selected_benchmark = SelectedBenchmark::#name; + let components = < + SelectedBenchmark as #krate::BenchmarkingSetup + >::components(&selected_benchmark); + let execute_benchmark = | + c: #krate::Vec<(#krate::BenchmarkParameter, u32)> + | -> Result<(), #krate::BenchmarkError> { + // Always reset the state after the benchmark. + #krate::defer!(#krate::benchmarking::wipe_db()); + + // Set up the benchmark, return execution + verification function. + let closure_to_verify = < + SelectedBenchmark as #krate::BenchmarkingSetup + >::instance(&selected_benchmark, &c, true)?; + + // Set the block number to at least 1 so events are deposited. + if #krate::Zero::is_zero(&frame_system::Pallet::::block_number()) { + frame_system::Pallet::::set_block_number(1u32.into()); + } + + // Run execution + verification + closure_to_verify() + }; + + if components.is_empty() { + execute_benchmark(Default::default())?; + } else { + let num_values: u32 = if let Ok(ev) = std::env::var("VALUES_PER_COMPONENT") { + ev.parse().map_err(|_| { + #krate::BenchmarkError::Stop( + "Could not parse env var `VALUES_PER_COMPONENT` as u32." + ) + })? + } else { + 6 + }; + + if num_values < 2 { + return Err("`VALUES_PER_COMPONENT` must be at least 2".into()); + } + + for (name, low, high) in components.clone().into_iter() { + // Test the lowest, highest (if its different from the lowest) + // and up to num_values-2 more equidistant values in between. + // For 0..10 and num_values=6 this would mean: [0, 2, 4, 6, 8, 10] + + let mut values = #krate::vec![low]; + let diff = (high - low).min(num_values - 1); + let slope = (high - low) as f32 / diff as f32; + + for i in 1..=diff { + let value = ((low as f32 + slope * i as f32) as u32) + .clamp(low, high); + values.push(value); + } + + for component_value in values { + // Select the max value for all the other components. + let c: #krate::Vec<(#krate::BenchmarkParameter, u32)> = components + .iter() + .map(|(n, _, h)| + if *n == name { + (*n, component_value) + } else { + (*n, *h) + } + ) + .collect(); + + execute_benchmark(c)?; + } + } + } + return Ok(()); + } + } + }; + res +} diff --git a/frame/support/procedural/src/lib.rs b/frame/support/procedural/src/lib.rs index 41dbc4ee9592c..9bdbbd1f27bb2 100644 --- a/frame/support/procedural/src/lib.rs +++ b/frame/support/procedural/src/lib.rs @@ -19,6 +19,7 @@ #![recursion_limit = "512"] +mod benchmark; mod clone_no_bound; mod construct_runtime; mod crate_version; @@ -479,6 +480,69 @@ pub fn pallet(attr: TokenStream, item: TokenStream) -> TokenStream { pallet::pallet(attr, item) } +/// An attribute macro that can be attached to a (non-empty) module declaration. Doing so will +/// designate that module as a benchmarking module. +/// +/// See `frame_support::benchmarking` for more info. +#[proc_macro_attribute] +pub fn benchmarks(attr: TokenStream, tokens: TokenStream) -> TokenStream { + match benchmark::benchmarks(attr, tokens, false) { + Ok(tokens) => tokens, + Err(err) => err.to_compile_error().into(), + } +} + +/// An attribute macro that can be attached to a (non-empty) module declaration. Doing so will +/// designate that module as an instance benchmarking module. +/// +/// See `frame_support::benchmarking` for more info. +#[proc_macro_attribute] +pub fn instance_benchmarks(attr: TokenStream, tokens: TokenStream) -> TokenStream { + match benchmark::benchmarks(attr, tokens, true) { + Ok(tokens) => tokens, + Err(err) => err.to_compile_error().into(), + } +} + +/// An attribute macro used to declare a benchmark within a benchmarking module. Must be +/// attached to a function definition containing an `#[extrinsic_call]` or `#[block]` +/// attribute. +/// +/// See `frame_support::benchmarking` for more info. +#[proc_macro_attribute] +pub fn benchmark(_attrs: TokenStream, _tokens: TokenStream) -> TokenStream { + quote!(compile_error!( + "`#[benchmark]` must be in a module labeled with #[benchmarks] or #[instance_benchmarks]." + )) + .into() +} + +/// An attribute macro used to specify the extrinsic call inside a benchmark function, and also +/// used as a boundary designating where the benchmark setup code ends, and the benchmark +/// verification code begins. +/// +/// See `frame_support::benchmarking` for more info. +#[proc_macro_attribute] +pub fn extrinsic_call(_attrs: TokenStream, _tokens: TokenStream) -> TokenStream { + quote!(compile_error!( + "`#[extrinsic_call]` must be in a benchmark function definition labeled with `#[benchmark]`." + );) + .into() +} + +/// An attribute macro used to specify that a block should be the measured portion of the +/// enclosing benchmark function, This attribute is also used as a boundary designating where +/// the benchmark setup code ends, and the benchmark verification code begins. +/// +/// See `frame_support::benchmarking` for more info. +#[proc_macro_attribute] +pub fn block(_attrs: TokenStream, _tokens: TokenStream) -> TokenStream { + quote!(compile_error!( + "`#[block]` must be in a benchmark function definition labeled with `#[benchmark]`." + )) + .into() +} + /// Execute the annotated function in a new storage transaction. /// /// The return type of the annotated function must be `Result`. All changes to storage performed diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index 902893972f0b1..40bc878cff365 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -2745,5 +2745,228 @@ pub mod pallet_macros { }; } +/// Contains macros, structs, and traits associated with v2 of the pallet benchmarking syntax. +/// This module contains macros, structs, and traits associated with v2 of the pallet +/// benchmarking syntax. +/// +/// The [`benchmarking::benchmarks`] and [`benchmarking::instance_benchmarks`] macros can be +/// used to designate a module as a benchmarking module that can contain benchmarks and +/// benchmark tests. The `#[benchmarks]` variant will set up a regular, non-instance +/// benchmarking module, and the `#[instance_benchmarks]` variant will set up the module in +/// instance benchmarking mode. +/// +/// Benchmarking modules should be gated behind a `#[cfg(feature = "runtime-benchmarks")]` +/// feature gate to ensure benchmarking code that is only compiled when the +/// `runtime-benchmarks` feature is enabled is not referenced. +/// +/// The following is the general syntax for a benchmarks (or instance benchmarks) module: +/// +/// ## General Syntax +/// +/// ```ignore +/// #![cfg(feature = "runtime-benchmarks")] +/// +/// use super::{mock_helpers::*, Pallet as MyPallet}; +/// use frame_support::benchmarking::*; +/// use frame_benchmarking::whitelisted_caller; +/// +/// #[benchmarks] +/// mod benchmarks { +/// use super::*; +/// +/// #[benchmark] +/// fn bench_name_1(x: Linear<7, 1_000>, y: Linear<1_000, 100_0000>) { +/// // setup code +/// let z = x + y; +/// let caller = whitelisted_caller(); +/// +/// #[extrinsic_call] +/// extrinsic_name(SystemOrigin::Signed(caller), other, arguments); +/// +/// // verification code +/// assert_eq!(MyPallet::::my_var(), z); +/// } +/// +/// #[benchmark] +/// fn bench_name_2() { +/// // setup code +/// let caller = whitelisted_caller(); +/// +/// #[block] +/// { +/// something(some, thing); +/// my_extrinsic(RawOrigin::Signed(caller), some, argument); +/// something_else(foo, bar); +/// } +/// +/// // verification code +/// assert_eq!(MyPallet::::something(), 37); +/// } +/// } +/// ``` +/// +/// ## Benchmark Definitions +/// +/// Within a `#[benchmarks]` or `#[instance_benchmarks]` module, you can define individual +/// benchmarks using the `#[benchmark]` attribute, as shown in the example above. +/// +/// The `#[benchmark]` attribute expects a function definition with a blank return type and +/// zero or more arguments whose names are valid `frame_benchmarking::BenchmarkParamater` +/// parameters, such as `x`, `y`, `a`, `b`, etc., and whose param types must implement +/// [`benchmarking::ParamRange`]. At the moment the only valid type that implements +/// [`benchmarking::ParamRange`] is [`benchmarking::Linear`]. +/// +/// The valid syntax for defining a `Linear` is `Linear` where `A`, and `B` are +/// valid integer literals (that fit in a `u32`), such that `B` >= `A`. +/// +/// Note that the benchmark function definition does not actually expand as a function +/// definition, but rather is used to automatically create a number of impls and structs +/// required by the benchmarking engine. For this reason, the visibility of the function +/// definition as well as the return type are not used for any purpose and are discarded by the +/// expansion code. +/// +/// Also note that the `// setup code` and `// verification code` comments shown above are not +/// required and are included simply for demonstration purposes. +/// +/// ### `#[extrinsic_call]` and `#[block]` +/// +/// Within the benchmark function body, either an `#[extrinsic_call]` or a `#[block]` +/// annotation is required. These attributes should be attached to a block (shown in +/// `bench_name_2` above) or a one-line function call (shown in `bench_name_1` above, in `syn` +/// parlance this should be an `ExprCall`), respectively. +/// +/// The `#[block]` syntax is broad and will benchmark any code contained within the block the +/// attribute is attached to. If `#[block]` is attached to something other than a block, a +/// compiler error will be emitted. +/// +/// The one-line `#[extrinsic_call]` syntax must consist of a function call to an extrinsic, +/// where the first argument is the origin. If `#[extrinsic_call]` is attached to an item that +/// doesn't meet these requirements, a compiler error will be emitted. +/// +/// As a short-hand, you may substitute the name of the extrinsic call with `_`, such as the +/// following: +/// +/// ```ignore +/// #[extrinsic_call] +/// _(RawOrigin::Signed(whitelisted_caller()), 0u32.into(), 0); +/// ``` +/// +/// The underscore will be substituted with the name of the benchmark (i.e. the name of the +/// function in the benchmark function definition). +/// +/// Regardless of whether `#[extrinsic_call]` or `#[block]` is used, this attribute also serves +/// the purpose of designating the boundary between the setup code portion of the benchmark +/// (everything before the `#[extrinsic_call]` or `#[block]` attribute) and the verification +/// stage (everything after the item that the `#[extrinsic_call]` or `#[block]` attribute is +/// attached to). The setup code section should contain any code that needs to execute before +/// the measured portion of the benchmark executes. The verification section is where you can +/// perform assertions to verify that the extrinsic call (or whatever is happening in your +/// block, if you used the `#[block]` syntax) executed successfully. +/// +/// Note that neither `#[extrinsic_call]` nor `#[block]` are real attribute macros and are +/// instead consumed by the outer macro pattern as part of the enclosing benchmark function +/// definition. This is why we are able to use `#[extrinsic_call]` and `#[block]` within a +/// function definition even though this behavior has not been stabilized +/// yet—`#[extrinsic_call]` and `#[block]` are parsed and consumed as part of the benchmark +/// definition parsing code, so they never expand as their own attribute macros. +/// +/// ### Optional Attributes +/// +/// The keywords `extra` and `skip_meta` can be provided as optional arguments to the +/// `#[benchmark]` attribute, i.e. `#[benchmark(extra, skip_meta)]`. Including either of these +/// will enable the `extra` or `skip_meta` option, respectively. These options enable the same +/// behavior they did in the old benchmarking syntax in `frame_benchmarking`, namely: +/// +/// #### `extra` +/// +/// Specifies that this benchmark should not normally run. To run benchmarks marked with +/// `extra`, you will need to invoke the `frame-benchmarking-cli` with `--extra`. +/// +/// #### `skip_meta` +/// +/// Specifies that the benchmarking framework should not analyze the storage keys that +/// benchmarked code read or wrote. This useful to suppress the prints in the form of unknown +/// 0x… in case a storage key that does not have metadata. Note that this skips the analysis +/// of all accesses, not just ones without metadata. +/// +/// ## Where Clause +/// +/// Some pallets require a where clause specifying constraints on their generics to make +/// writing benchmarks feasible. To accomodate this situation, you can provide such a where +/// clause as the (only) argument to the `#[benchmarks]` or `#[instance_benchmarks]` attribute +/// macros. Below is an example of this taken from the `message-queue` pallet. +/// +/// ```ignore +/// #[benchmarks( +/// where +/// <::MessageProcessor as ProcessMessage>::Origin: From + PartialEq, +/// ::Size: From, +/// )] +/// mod benchmarks { +/// use super::*; +/// // ... +/// } +/// ``` +/// +/// ## Benchmark Tests +/// +/// Benchmark tests can be generated using the old syntax in `frame_benchmarking`, +/// including the `frame_benchmarking::impl_benchmark_test_suite` macro. +/// +/// An example is shown below (taken from the `message-queue` pallet's `benchmarking` module): +/// ```ignore +/// #[benchmarks] +/// mod benchmarks { +/// use super::*; +/// // ... +/// impl_benchmark_test_suite!( +/// MessageQueue, +/// crate::mock::new_test_ext::(), +/// crate::integration_test::Test +/// ); +/// } +/// ``` +pub mod benchmarking { + pub use frame_support_procedural::{ + benchmark, benchmarks, block, extrinsic_call, instance_benchmarks, + }; + + // Used in #[benchmark] implementation to ensure that benchmark function arguments + // implement [`ParamRange`]. + #[doc(hidden)] + pub use static_assertions::assert_impl_all; + + /// Used by the new benchmarking code to specify that a benchmarking variable is linear + /// over some specified range, i.e. `Linear<0, 1_000>` means that the corresponding variable + /// is allowed to range from `0` to `1000`, inclusive. + /// + /// See [`frame_support::benchmarking`] for more info. + pub struct Linear; + + /// Trait that must be implemented by all structs that can be used as parameter range types + /// in the new benchmarking code (i.e. `Linear<0, 1_000>`). Right now there is just + /// [`Linear`] but this could later be extended to support additional non-linear parameter + /// ranges. + /// + /// See [`frame_support::benchmarking`] for more info. + pub trait ParamRange { + /// Represents the (inclusive) starting number of this [`ParamRange`]. + fn start(&self) -> u32; + + /// Represents the (inclusive) ending number of this [`ParamRange`] + fn end(&self) -> u32; + } + + impl ParamRange for Linear { + fn start(&self) -> u32 { + return A + } + + fn end(&self) -> u32 { + return B + } + } +} + // Generate a macro that will enable/disable code based on `std` feature being active. sp_core::generate_feature_enabled_macro!(std_enabled, feature = "std", $); diff --git a/frame/support/test/Cargo.toml b/frame/support/test/Cargo.toml index fc211d77132f5..90ef243eed6c6 100644 --- a/frame/support/test/Cargo.toml +++ b/frame/support/test/Cargo.toml @@ -19,6 +19,7 @@ sp-arithmetic = { version = "6.0.0", default-features = false, path = "../../../ sp-io = { version = "7.0.0", path = "../../../primitives/io", default-features = false } sp-state-machine = { version = "0.13.0", optional = true, path = "../../../primitives/state-machine" } frame-support = { version = "4.0.0-dev", default-features = false, path = "../" } +frame-benchmarking = { version = "4.0.0-dev", default-features = false, path = "../../benchmarking" } sp-runtime = { version = "7.0.0", default-features = false, path = "../../../primitives/runtime" } sp-core = { version = "7.0.0", default-features = false, path = "../../../primitives/core" } sp-std = { version = "5.0.0", default-features = false, path = "../../../primitives/std" } @@ -36,6 +37,7 @@ std = [ "serde/std", "codec/std", "scale-info/std", + "frame-benchmarking/std", "frame-support/std", "frame-system/std", "sp-core/std", diff --git a/frame/support/test/tests/benchmark_ui.rs b/frame/support/test/tests/benchmark_ui.rs new file mode 100644 index 0000000000000..243030bd07fcd --- /dev/null +++ b/frame/support/test/tests/benchmark_ui.rs @@ -0,0 +1,36 @@ +// This file is part of Substrate. + +// Copyright (C) 2020-2023 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. + +#[rustversion::attr(not(stable), ignore)] +#[cfg(not(feature = "disable-ui-tests"))] +#[test] +fn benchmark_ui() { + // Only run the ui tests when `RUN_UI_TESTS` is set. + if std::env::var("RUN_UI_TESTS").is_err() { + return + } + + // As trybuild is using `cargo check`, we don't need the real WASM binaries. + std::env::set_var("SKIP_WASM_BUILD", "1"); + + // Deny all warnings since we emit warnings as part of a Pallet's UI. + std::env::set_var("RUSTFLAGS", "--deny warnings"); + + let t = trybuild::TestCases::new(); + t.compile_fail("tests/benchmark_ui/*.rs"); + t.pass("tests/benchmark_ui/pass/*.rs"); +} diff --git a/frame/support/test/tests/benchmark_ui/bad_param_name.rs b/frame/support/test/tests/benchmark_ui/bad_param_name.rs new file mode 100644 index 0000000000000..ad4db4f0a4ba4 --- /dev/null +++ b/frame/support/test/tests/benchmark_ui/bad_param_name.rs @@ -0,0 +1,18 @@ +use frame_support::benchmarking::*; +#[allow(unused_imports)] +use frame_support_test::Config; + +#[benchmarks] +mod benches { + use super::*; + + #[benchmark] + fn bench(winton: Linear<1, 2>) { + let a = 2 + 2; + #[block] + {} + assert_eq!(a, 4); + } +} + +fn main() {} diff --git a/frame/support/test/tests/benchmark_ui/bad_param_name.stderr b/frame/support/test/tests/benchmark_ui/bad_param_name.stderr new file mode 100644 index 0000000000000..4e2d63a6b5030 --- /dev/null +++ b/frame/support/test/tests/benchmark_ui/bad_param_name.stderr @@ -0,0 +1,5 @@ +error: Benchmark parameter names must consist of a single lowercase letter (a-z) and no other characters. + --> tests/benchmark_ui/bad_param_name.rs:10:11 + | +10 | fn bench(winton: Linear<1, 2>) { + | ^^^^^^ diff --git a/frame/support/test/tests/benchmark_ui/bad_param_name_too_long.rs b/frame/support/test/tests/benchmark_ui/bad_param_name_too_long.rs new file mode 100644 index 0000000000000..50e4dc6fd4609 --- /dev/null +++ b/frame/support/test/tests/benchmark_ui/bad_param_name_too_long.rs @@ -0,0 +1,14 @@ +use frame_support::benchmarking::*; +#[allow(unused_imports)] +use frame_support_test::Config; + +#[benchmarks] +mod benches { + #[benchmark] + fn bench(xx: Linear<1, 2>) { + #[block] + {} + } +} + +fn main() {} diff --git a/frame/support/test/tests/benchmark_ui/bad_param_name_too_long.stderr b/frame/support/test/tests/benchmark_ui/bad_param_name_too_long.stderr new file mode 100644 index 0000000000000..32f6bf8e47d09 --- /dev/null +++ b/frame/support/test/tests/benchmark_ui/bad_param_name_too_long.stderr @@ -0,0 +1,5 @@ +error: Benchmark parameter names must consist of a single lowercase letter (a-z) and no other characters. + --> tests/benchmark_ui/bad_param_name_too_long.rs:8:11 + | +8 | fn bench(xx: Linear<1, 2>) { + | ^^ diff --git a/frame/support/test/tests/benchmark_ui/bad_param_name_upper_case.rs b/frame/support/test/tests/benchmark_ui/bad_param_name_upper_case.rs new file mode 100644 index 0000000000000..0270582b3b85b --- /dev/null +++ b/frame/support/test/tests/benchmark_ui/bad_param_name_upper_case.rs @@ -0,0 +1,14 @@ +use frame_support::benchmarking::*; +#[allow(unused_imports)] +use frame_support_test::Config; + +#[benchmarks] +mod benches { + #[benchmark] + fn bench(D: Linear<1, 2>) { + #[block] + {} + } +} + +fn main() {} diff --git a/frame/support/test/tests/benchmark_ui/bad_param_name_upper_case.stderr b/frame/support/test/tests/benchmark_ui/bad_param_name_upper_case.stderr new file mode 100644 index 0000000000000..48dd41d3262d7 --- /dev/null +++ b/frame/support/test/tests/benchmark_ui/bad_param_name_upper_case.stderr @@ -0,0 +1,5 @@ +error: Benchmark parameter names must consist of a single lowercase letter (a-z) and no other characters. + --> tests/benchmark_ui/bad_param_name_upper_case.rs:8:11 + | +8 | fn bench(D: Linear<1, 2>) { + | ^ diff --git a/frame/support/test/tests/benchmark_ui/bad_param_range.rs b/frame/support/test/tests/benchmark_ui/bad_param_range.rs new file mode 100644 index 0000000000000..aabb9fa7403a1 --- /dev/null +++ b/frame/support/test/tests/benchmark_ui/bad_param_range.rs @@ -0,0 +1,16 @@ +use frame_support::benchmarking::*; +#[allow(unused_imports)] +use frame_support_test::Config; + +#[benchmarks] +mod benches { + use super::*; + + #[benchmark] + fn bench(x: Linear<3, 1>) { + #[block] + {} + } +} + +fn main() {} diff --git a/frame/support/test/tests/benchmark_ui/bad_param_range.stderr b/frame/support/test/tests/benchmark_ui/bad_param_range.stderr new file mode 100644 index 0000000000000..1347af0a07b8e --- /dev/null +++ b/frame/support/test/tests/benchmark_ui/bad_param_range.stderr @@ -0,0 +1,5 @@ +error: The start of a `ParamRange` must be less than or equal to the end + --> tests/benchmark_ui/bad_param_range.rs:10:21 + | +10 | fn bench(x: Linear<3, 1>) { + | ^ diff --git a/frame/support/test/tests/benchmark_ui/bad_params.rs b/frame/support/test/tests/benchmark_ui/bad_params.rs new file mode 100644 index 0000000000000..a0c9236982c62 --- /dev/null +++ b/frame/support/test/tests/benchmark_ui/bad_params.rs @@ -0,0 +1,18 @@ +use frame_support::benchmarking::*; +#[allow(unused_imports)] +use frame_support_test::Config; + +#[benchmarks] +mod benches { + use super::*; + + #[benchmark] + fn bench(y: Linear<1, 2>, x: u32) { + let a = 2 + 2; + #[block] + {} + assert_eq!(a, 4); + } +} + +fn main() {} diff --git a/frame/support/test/tests/benchmark_ui/bad_params.stderr b/frame/support/test/tests/benchmark_ui/bad_params.stderr new file mode 100644 index 0000000000000..068eaedd531b9 --- /dev/null +++ b/frame/support/test/tests/benchmark_ui/bad_params.stderr @@ -0,0 +1,5 @@ +error: Invalid benchmark function param. A valid example would be `x: Linear<5, 10>`. + --> tests/benchmark_ui/bad_params.rs:10:31 + | +10 | fn bench(y: Linear<1, 2>, x: u32) { + | ^^^ diff --git a/frame/support/test/tests/benchmark_ui/dup_block.rs b/frame/support/test/tests/benchmark_ui/dup_block.rs new file mode 100644 index 0000000000000..4c4a8fc11d30a --- /dev/null +++ b/frame/support/test/tests/benchmark_ui/dup_block.rs @@ -0,0 +1,20 @@ +use frame_support::benchmarking::*; +#[allow(unused_imports)] +use frame_support_test::Config; + +#[benchmarks] +mod benches { + use super::*; + + #[benchmark] + fn bench() { + let a = 2 + 2; + #[block] + {} + #[block] + {} + assert_eq!(a, 4); + } +} + +fn main() {} diff --git a/frame/support/test/tests/benchmark_ui/dup_block.stderr b/frame/support/test/tests/benchmark_ui/dup_block.stderr new file mode 100644 index 0000000000000..3d73c3d6609b1 --- /dev/null +++ b/frame/support/test/tests/benchmark_ui/dup_block.stderr @@ -0,0 +1,5 @@ +error: Only one #[extrinsic_call] or #[block] attribute is allowed per benchmark. + --> tests/benchmark_ui/dup_block.rs:14:3 + | +14 | #[block] + | ^ diff --git a/frame/support/test/tests/benchmark_ui/dup_extrinsic_call.rs b/frame/support/test/tests/benchmark_ui/dup_extrinsic_call.rs new file mode 100644 index 0000000000000..1a91b7c16d6a5 --- /dev/null +++ b/frame/support/test/tests/benchmark_ui/dup_extrinsic_call.rs @@ -0,0 +1,20 @@ +use frame_support::benchmarking::*; +#[allow(unused_imports)] +use frame_support_test::Config; + +#[benchmarks] +mod benches { + use super::*; + + #[benchmark] + fn bench() { + let a = 2 + 2; + #[extrinsic_call] + _(stuff); + #[extrinsic_call] + _(other_stuff); + assert_eq!(a, 4); + } +} + +fn main() {} diff --git a/frame/support/test/tests/benchmark_ui/dup_extrinsic_call.stderr b/frame/support/test/tests/benchmark_ui/dup_extrinsic_call.stderr new file mode 100644 index 0000000000000..593f7072bfa51 --- /dev/null +++ b/frame/support/test/tests/benchmark_ui/dup_extrinsic_call.stderr @@ -0,0 +1,5 @@ +error: Only one #[extrinsic_call] or #[block] attribute is allowed per benchmark. + --> tests/benchmark_ui/dup_extrinsic_call.rs:14:3 + | +14 | #[extrinsic_call] + | ^ diff --git a/frame/support/test/tests/benchmark_ui/extra_extra.rs b/frame/support/test/tests/benchmark_ui/extra_extra.rs new file mode 100644 index 0000000000000..021106c7afc85 --- /dev/null +++ b/frame/support/test/tests/benchmark_ui/extra_extra.rs @@ -0,0 +1,16 @@ +use frame_support::benchmarking::*; +#[allow(unused_imports)] +use frame_support_test::Config; + +#[benchmarks] +mod benches { + use super::*; + + #[benchmark(extra, extra)] + fn bench() { + #[block] + {} + } +} + +fn main() {} diff --git a/frame/support/test/tests/benchmark_ui/extra_extra.stderr b/frame/support/test/tests/benchmark_ui/extra_extra.stderr new file mode 100644 index 0000000000000..bf36b4f08054a --- /dev/null +++ b/frame/support/test/tests/benchmark_ui/extra_extra.stderr @@ -0,0 +1,5 @@ +error: unexpected end of input, `extra` can only be specified once + --> tests/benchmark_ui/extra_extra.rs:9:26 + | +9 | #[benchmark(extra, extra)] + | ^ diff --git a/frame/support/test/tests/benchmark_ui/extra_skip_meta.rs b/frame/support/test/tests/benchmark_ui/extra_skip_meta.rs new file mode 100644 index 0000000000000..1940f4cf1f040 --- /dev/null +++ b/frame/support/test/tests/benchmark_ui/extra_skip_meta.rs @@ -0,0 +1,16 @@ +use frame_support::benchmarking::*; +#[allow(unused_imports)] +use frame_support_test::Config; + +#[benchmarks] +mod benches { + use super::*; + + #[benchmark(skip_meta, skip_meta)] + fn bench() { + #[block] + {} + } +} + +fn main() {} diff --git a/frame/support/test/tests/benchmark_ui/extra_skip_meta.stderr b/frame/support/test/tests/benchmark_ui/extra_skip_meta.stderr new file mode 100644 index 0000000000000..4d48a8ad77a45 --- /dev/null +++ b/frame/support/test/tests/benchmark_ui/extra_skip_meta.stderr @@ -0,0 +1,5 @@ +error: unexpected end of input, `skip_meta` can only be specified once + --> tests/benchmark_ui/extra_skip_meta.rs:9:34 + | +9 | #[benchmark(skip_meta, skip_meta)] + | ^ diff --git a/frame/support/test/tests/benchmark_ui/extrinsic_call_out_of_fn.rs b/frame/support/test/tests/benchmark_ui/extrinsic_call_out_of_fn.rs new file mode 100644 index 0000000000000..4cb6bfc34c58e --- /dev/null +++ b/frame/support/test/tests/benchmark_ui/extrinsic_call_out_of_fn.rs @@ -0,0 +1,6 @@ +use frame_support::benchmarking::*; + +#[extrinsic_call] +mod stuff {} + +fn main() {} diff --git a/frame/support/test/tests/benchmark_ui/extrinsic_call_out_of_fn.stderr b/frame/support/test/tests/benchmark_ui/extrinsic_call_out_of_fn.stderr new file mode 100644 index 0000000000000..c5194d7a66502 --- /dev/null +++ b/frame/support/test/tests/benchmark_ui/extrinsic_call_out_of_fn.stderr @@ -0,0 +1,7 @@ +error: `#[extrinsic_call]` must be in a benchmark function definition labeled with `#[benchmark]`. + --> tests/benchmark_ui/extrinsic_call_out_of_fn.rs:3:1 + | +3 | #[extrinsic_call] + | ^^^^^^^^^^^^^^^^^ + | + = note: this error originates in the attribute macro `extrinsic_call` (in Nightly builds, run with -Z macro-backtrace for more info) diff --git a/frame/support/test/tests/benchmark_ui/missing_call.rs b/frame/support/test/tests/benchmark_ui/missing_call.rs new file mode 100644 index 0000000000000..74370493542c4 --- /dev/null +++ b/frame/support/test/tests/benchmark_ui/missing_call.rs @@ -0,0 +1,13 @@ +use frame_support::benchmarking::*; +#[allow(unused_imports)] +use frame_support_test::Config; + +#[benchmarks] +mod benches { + use super::*; + + #[benchmark] + fn bench() {} +} + +fn main() {} diff --git a/frame/support/test/tests/benchmark_ui/missing_call.stderr b/frame/support/test/tests/benchmark_ui/missing_call.stderr new file mode 100644 index 0000000000000..3b55f77d42562 --- /dev/null +++ b/frame/support/test/tests/benchmark_ui/missing_call.stderr @@ -0,0 +1,5 @@ +error: No valid #[extrinsic_call] or #[block] annotation could be found in benchmark function body. + --> tests/benchmark_ui/missing_call.rs:10:13 + | +10 | fn bench() {} + | ^^ diff --git a/frame/support/test/tests/benchmark_ui/missing_origin.rs b/frame/support/test/tests/benchmark_ui/missing_origin.rs new file mode 100644 index 0000000000000..aad91bc79f825 --- /dev/null +++ b/frame/support/test/tests/benchmark_ui/missing_origin.rs @@ -0,0 +1,16 @@ +use frame_support::benchmarking::*; +#[allow(unused_imports)] +use frame_support_test::Config; + +#[benchmarks] +mod benches { + use super::*; + + #[benchmark] + fn bench() { + #[extrinsic_call] + thing(); + } +} + +fn main() {} diff --git a/frame/support/test/tests/benchmark_ui/missing_origin.stderr b/frame/support/test/tests/benchmark_ui/missing_origin.stderr new file mode 100644 index 0000000000000..0e72bff4747a3 --- /dev/null +++ b/frame/support/test/tests/benchmark_ui/missing_origin.stderr @@ -0,0 +1,5 @@ +error: Single-item extrinsic calls must specify their origin as the first argument. + --> tests/benchmark_ui/missing_origin.rs:12:3 + | +12 | thing(); + | ^^^^^ diff --git a/frame/support/test/tests/benchmark_ui/pass/valid_basic.rs b/frame/support/test/tests/benchmark_ui/pass/valid_basic.rs new file mode 100644 index 0000000000000..5c84d7f76d874 --- /dev/null +++ b/frame/support/test/tests/benchmark_ui/pass/valid_basic.rs @@ -0,0 +1,17 @@ +use frame_support::benchmarking::*; +use frame_support_test::Config; + +#[benchmarks] +mod benches { + use super::*; + + #[benchmark(skip_meta, extra)] + fn bench() { + let a = 2 + 2; + #[block] + {} + assert_eq!(a, 4); + } +} + +fn main() {} diff --git a/frame/support/test/tests/benchmark_ui/unrecognized_option.rs b/frame/support/test/tests/benchmark_ui/unrecognized_option.rs new file mode 100644 index 0000000000000..4c2cea139f80c --- /dev/null +++ b/frame/support/test/tests/benchmark_ui/unrecognized_option.rs @@ -0,0 +1,16 @@ +use frame_support::benchmarking::*; +#[allow(unused_imports)] +use frame_support_test::Config; + +#[benchmarks] +mod benches { + use super::*; + + #[benchmark(skip_meta, extra, bad)] + fn bench() { + #[block] + {} + } +} + +fn main() {} diff --git a/frame/support/test/tests/benchmark_ui/unrecognized_option.stderr b/frame/support/test/tests/benchmark_ui/unrecognized_option.stderr new file mode 100644 index 0000000000000..5cebe9eab05e9 --- /dev/null +++ b/frame/support/test/tests/benchmark_ui/unrecognized_option.stderr @@ -0,0 +1,5 @@ +error: expected `extra` or `skip_meta` + --> tests/benchmark_ui/unrecognized_option.rs:9:32 + | +9 | #[benchmark(skip_meta, extra, bad)] + | ^^^