diff --git a/.gitlab/pipeline/short-benchmarks.yml b/.gitlab/pipeline/short-benchmarks.yml index bc6dd04264c8..9b70ee419c23 100644 --- a/.gitlab/pipeline/short-benchmarks.yml +++ b/.gitlab/pipeline/short-benchmarks.yml @@ -24,7 +24,7 @@ short-benchmark-westend: &short-bench tags: - benchmark script: - - ./artifacts/polkadot benchmark pallet --chain $RUNTIME-dev --pallet "*" --extrinsic "*" --steps 2 --repeat 1 + - ./artifacts/polkadot benchmark pallet --chain $RUNTIME-dev --pallet "*" --extrinsic "*" --steps 2 --repeat 1 --sanity-weight-check ignore # run short-benchmarks for system parachain runtimes from cumulus @@ -47,7 +47,7 @@ short-benchmark-westend: &short-bench tags: - benchmark script: - - ./artifacts/polkadot-parachain benchmark pallet --chain $RUNTIME_CHAIN --pallet "*" --extrinsic "*" --steps 2 --repeat 1 + - ./artifacts/polkadot-parachain benchmark pallet --chain $RUNTIME_CHAIN --pallet "*" --extrinsic "*" --steps 2 --repeat 1 --sanity-weight-check ignore short-benchmark-asset-hub-rococo: <<: *short-bench-cumulus diff --git a/.gitlab/pipeline/test.yml b/.gitlab/pipeline/test.yml index e75700ffddc4..0f69649546d1 100644 --- a/.gitlab/pipeline/test.yml +++ b/.gitlab/pipeline/test.yml @@ -69,7 +69,6 @@ test-linux-stable-runtime-benchmarks: RUSTFLAGS: "-Cdebug-assertions=y -Dwarnings" script: - time cargo nextest run --workspace --features runtime-benchmarks benchmark --locked --cargo-profile testnet - # can be used to run all tests # test-linux-stable-all: # stage: test @@ -320,7 +319,7 @@ quick-benchmarks: WASM_BUILD_NO_COLOR: 1 WASM_BUILD_RUSTFLAGS: "-C debug-assertions -D warnings" script: - - time cargo run --locked --release -p staging-node-cli --bin substrate-node --features runtime-benchmarks -- benchmark pallet --execution wasm --wasm-execution compiled --chain dev --pallet "*" --extrinsic "*" --steps 2 --repeat 1 + - time cargo run --locked --release -p staging-node-cli --bin substrate-node --features runtime-benchmarks -- benchmark pallet --execution wasm --wasm-execution compiled --chain dev --pallet "*" --extrinsic "*" --steps 2 --repeat 1 --sanity-weight-check ignore test-frame-examples-compile-to-wasm: # into one job diff --git a/Cargo.lock b/Cargo.lock index 8e2551f7cc9b..9ce9fd8a7189 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5458,6 +5458,7 @@ dependencies = [ "array-bytes 6.1.0", "chrono", "clap 4.4.18", + "color-print", "comfy-table", "frame-benchmarking", "frame-support", diff --git a/cumulus/parachain-template/runtime/src/lib.rs b/cumulus/parachain-template/runtime/src/lib.rs index d9bc111fcef7..b546f6801f80 100644 --- a/cumulus/parachain-template/runtime/src/lib.rs +++ b/cumulus/parachain-template/runtime/src/lib.rs @@ -707,6 +707,8 @@ impl_runtime_apis! { fn benchmark_metadata(extra: bool) -> ( Vec, Vec, + Weight, + frame_support::weights::RuntimeDbWeight, ) { use frame_benchmarking::{Benchmarking, BenchmarkList}; use frame_support::traits::StorageInfoTrait; @@ -717,7 +719,9 @@ impl_runtime_apis! { list_benchmarks!(list, extra); let storage_info = AllPalletsWithSystem::storage_info(); - (list, storage_info) + let max_extrinsic_weight = RuntimeBlockWeights::get().per_class.get(DispatchClass::Normal).max_extrinsic.unwrap(); + let db_weight: frame_support::weights::RuntimeDbWeight = ::DbWeight::get(); + (list, storage_info, max_extrinsic_weight, db_weight) } fn dispatch_benchmark( diff --git a/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs b/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs index 2c111c0eface..1fc27f75d896 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs @@ -1294,6 +1294,8 @@ impl_runtime_apis! { fn benchmark_metadata(extra: bool) -> ( Vec, Vec, + Weight, + frame_support::weights::RuntimeDbWeight, ) { use frame_benchmarking::{Benchmarking, BenchmarkList}; use frame_support::traits::StorageInfoTrait; @@ -1322,7 +1324,9 @@ impl_runtime_apis! { list_benchmarks!(list, extra); let storage_info = AllPalletsWithSystem::storage_info(); - (list, storage_info) + let max_extrinsic_weight = RuntimeBlockWeights::get().per_class.get(DispatchClass::Normal).max_extrinsic.unwrap(); + let db_weight: frame_support::weights::RuntimeDbWeight = ::DbWeight::get(); + (list, storage_info, max_extrinsic_weight, db_weight) } fn dispatch_benchmark( diff --git a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs index aa319d9f8920..7c141c0c3238 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs @@ -1372,6 +1372,8 @@ impl_runtime_apis! { fn benchmark_metadata(extra: bool) -> ( Vec, Vec, + Weight, + frame_support::weights::RuntimeDbWeight, ) { use frame_benchmarking::{Benchmarking, BenchmarkList}; use frame_support::traits::StorageInfoTrait; @@ -1400,7 +1402,9 @@ impl_runtime_apis! { list_benchmarks!(list, extra); let storage_info = AllPalletsWithSystem::storage_info(); - (list, storage_info) + let max_extrinsic_weight = RuntimeBlockWeights::get().per_class.get(DispatchClass::Normal).max_extrinsic.unwrap(); + let db_weight: frame_support::weights::RuntimeDbWeight = ::DbWeight::get(); + (list, storage_info, max_extrinsic_weight, db_weight) } fn dispatch_benchmark( diff --git a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/lib.rs b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/lib.rs index 2be5652c4347..b650bd49460e 100644 --- a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/lib.rs +++ b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/lib.rs @@ -1059,6 +1059,8 @@ impl_runtime_apis! { fn benchmark_metadata(extra: bool) -> ( Vec, Vec, + Weight, + frame_support::weights::RuntimeDbWeight, ) { use frame_benchmarking::{Benchmarking, BenchmarkList}; use frame_support::traits::StorageInfoTrait; @@ -1083,7 +1085,9 @@ impl_runtime_apis! { list_benchmarks!(list, extra); let storage_info = AllPalletsWithSystem::storage_info(); - (list, storage_info) + let max_extrinsic_weight = RuntimeBlockWeights::get().per_class.get(DispatchClass::Normal).max_extrinsic.unwrap(); + let db_weight: frame_support::weights::RuntimeDbWeight = ::DbWeight::get(); + (list, storage_info, max_extrinsic_weight, db_weight) } fn dispatch_benchmark( diff --git a/cumulus/parachains/runtimes/collectives/collectives-westend/src/lib.rs b/cumulus/parachains/runtimes/collectives/collectives-westend/src/lib.rs index 6016955221c2..d0a2c6008ade 100644 --- a/cumulus/parachains/runtimes/collectives/collectives-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/collectives/collectives-westend/src/lib.rs @@ -949,6 +949,8 @@ impl_runtime_apis! { fn benchmark_metadata(extra: bool) -> ( Vec, Vec, + Weight, + frame_support::weights::RuntimeDbWeight, ) { use frame_benchmarking::{Benchmarking, BenchmarkList}; use frame_support::traits::StorageInfoTrait; @@ -960,7 +962,9 @@ impl_runtime_apis! { list_benchmarks!(list, extra); let storage_info = AllPalletsWithSystem::storage_info(); - (list, storage_info) + let max_extrinsic_weight = RuntimeBlockWeights::get().per_class.get(DispatchClass::Normal).max_extrinsic.unwrap(); + let db_weight: frame_support::weights::RuntimeDbWeight = ::DbWeight::get(); + (list, storage_info, max_extrinsic_weight, db_weight) } fn dispatch_benchmark( diff --git a/cumulus/parachains/runtimes/contracts/contracts-rococo/src/lib.rs b/cumulus/parachains/runtimes/contracts/contracts-rococo/src/lib.rs index b8203701d1a7..eb44b01e1e3e 100644 --- a/cumulus/parachains/runtimes/contracts/contracts-rococo/src/lib.rs +++ b/cumulus/parachains/runtimes/contracts/contracts-rococo/src/lib.rs @@ -675,6 +675,8 @@ impl_runtime_apis! { fn benchmark_metadata(extra: bool) -> ( Vec, Vec, + Weight, + frame_support::weights::RuntimeDbWeight, ) { use frame_benchmarking::{Benchmarking, BenchmarkList}; use frame_support::traits::StorageInfoTrait; @@ -686,7 +688,9 @@ impl_runtime_apis! { list_benchmarks!(list, extra); let storage_info = AllPalletsWithSystem::storage_info(); - (list, storage_info) + let max_extrinsic_weight = RuntimeBlockWeights::get().per_class.get(DispatchClass::Normal).max_extrinsic.unwrap(); + let db_weight: frame_support::weights::RuntimeDbWeight = ::DbWeight::get(); + (list, storage_info, max_extrinsic_weight, db_weight) } fn dispatch_benchmark( diff --git a/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs b/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs index 55fe3ba9fbf7..b36756494b7a 100644 --- a/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs @@ -652,6 +652,8 @@ impl_runtime_apis! { fn benchmark_metadata(extra: bool) -> ( Vec, Vec, + Weight, + frame_support::weights::RuntimeDbWeight, ) { use frame_benchmarking::{Benchmarking, BenchmarkList}; use frame_support::traits::StorageInfoTrait; @@ -669,7 +671,9 @@ impl_runtime_apis! { list_benchmarks!(list, extra); let storage_info = AllPalletsWithSystem::storage_info(); - (list, storage_info) + let max_extrinsic_weight = RuntimeBlockWeights::get().per_class.get(DispatchClass::Normal).max_extrinsic.unwrap(); + let db_weight: frame_support::weights::RuntimeDbWeight = ::DbWeight::get(); + (list, storage_info, max_extrinsic_weight, db_weight) } fn dispatch_benchmark( diff --git a/cumulus/parachains/runtimes/glutton/glutton-westend/src/lib.rs b/cumulus/parachains/runtimes/glutton/glutton-westend/src/lib.rs index e0ab9c590ba9..a6478cd49436 100644 --- a/cumulus/parachains/runtimes/glutton/glutton-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/glutton/glutton-westend/src/lib.rs @@ -435,17 +435,21 @@ impl_runtime_apis! { fn benchmark_metadata(extra: bool) -> ( Vec, Vec, + Weight, + frame_support::weights::RuntimeDbWeight, ) { use frame_benchmarking::{Benchmarking, BenchmarkList}; use frame_support::traits::StorageInfoTrait; use frame_system_benchmarking::Pallet as SystemBench; + use sp_core::Get; let mut list = Vec::::new(); list_benchmarks!(list, extra); let storage_info = AllPalletsWithSystem::storage_info(); - - (list, storage_info) + let max_extrinsic_weight = RuntimeBlockWeights::get().per_class.get(DispatchClass::Normal).max_extrinsic.unwrap(); + let db_weight: frame_support::weights::RuntimeDbWeight = ::DbWeight::get(); + (list, storage_info, max_extrinsic_weight, db_weight) } fn dispatch_benchmark( diff --git a/cumulus/parachains/runtimes/people/people-westend/src/lib.rs b/cumulus/parachains/runtimes/people/people-westend/src/lib.rs index 24e203c9f013..38dff4fed17b 100644 --- a/cumulus/parachains/runtimes/people/people-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/people/people-westend/src/lib.rs @@ -638,6 +638,8 @@ impl_runtime_apis! { fn benchmark_metadata(extra: bool) -> ( Vec, Vec, + Weight, + frame_support::weights::RuntimeDbWeight, ) { use frame_benchmarking::{Benchmarking, BenchmarkList}; use frame_support::traits::StorageInfoTrait; @@ -655,7 +657,9 @@ impl_runtime_apis! { list_benchmarks!(list, extra); let storage_info = AllPalletsWithSystem::storage_info(); - (list, storage_info) + let max_extrinsic_weight = RuntimeBlockWeights::get().per_class.get(DispatchClass::Normal).max_extrinsic.unwrap(); + let db_weight: frame_support::weights::RuntimeDbWeight = ::DbWeight::get(); + (list, storage_info, max_extrinsic_weight, db_weight) } fn dispatch_benchmark( diff --git a/cumulus/parachains/runtimes/testing/penpal/src/lib.rs b/cumulus/parachains/runtimes/testing/penpal/src/lib.rs index bf8dcbc24c8d..575da3b96d8f 100644 --- a/cumulus/parachains/runtimes/testing/penpal/src/lib.rs +++ b/cumulus/parachains/runtimes/testing/penpal/src/lib.rs @@ -847,6 +847,8 @@ impl_runtime_apis! { fn benchmark_metadata(extra: bool) -> ( Vec, Vec, + Weight, + frame_support::weights::RuntimeDbWeight, ) { use frame_benchmarking::{Benchmarking, BenchmarkList}; use frame_support::traits::StorageInfoTrait; @@ -857,7 +859,9 @@ impl_runtime_apis! { list_benchmarks!(list, extra); let storage_info = AllPalletsWithSystem::storage_info(); - (list, storage_info) + let max_extrinsic_weight = RuntimeBlockWeights::get().per_class.get(DispatchClass::Normal).max_extrinsic.unwrap(); + let db_weight: frame_support::weights::RuntimeDbWeight = ::DbWeight::get(); + (list, storage_info, max_extrinsic_weight, db_weight) } fn dispatch_benchmark( diff --git a/polkadot/runtime/rococo/src/lib.rs b/polkadot/runtime/rococo/src/lib.rs index 79c848221fbf..d31460ee1759 100644 --- a/polkadot/runtime/rococo/src/lib.rs +++ b/polkadot/runtime/rococo/src/lib.rs @@ -2229,6 +2229,8 @@ sp_api::impl_runtime_apis! { fn benchmark_metadata(extra: bool) -> ( Vec, Vec, + Weight, + frame_support::weights::RuntimeDbWeight, ) { use frame_benchmarking::{Benchmarking, BenchmarkList}; use frame_support::traits::StorageInfoTrait; @@ -2242,7 +2244,9 @@ sp_api::impl_runtime_apis! { list_benchmarks!(list, extra); let storage_info = AllPalletsWithSystem::storage_info(); - return (list, storage_info) + let max_extrinsic_weight = BlockWeights::get().per_class.get(DispatchClass::Normal).max_extrinsic.unwrap(); + let db_weight: frame_support::weights::RuntimeDbWeight = ::DbWeight::get(); + (list, storage_info, max_extrinsic_weight, db_weight) } fn dispatch_benchmark( diff --git a/polkadot/runtime/westend/src/lib.rs b/polkadot/runtime/westend/src/lib.rs index cd9e57a9bdf2..a7a692e6700b 100644 --- a/polkadot/runtime/westend/src/lib.rs +++ b/polkadot/runtime/westend/src/lib.rs @@ -2243,6 +2243,8 @@ sp_api::impl_runtime_apis! { fn benchmark_metadata(extra: bool) -> ( Vec, Vec, + Weight, + frame_support::weights::RuntimeDbWeight, ) { use frame_benchmarking::{Benchmarking, BenchmarkList}; use frame_support::traits::StorageInfoTrait; @@ -2261,7 +2263,9 @@ sp_api::impl_runtime_apis! { list_benchmarks!(list, extra); let storage_info = AllPalletsWithSystem::storage_info(); - return (list, storage_info) + let max_extrinsic_weight = BlockWeights::get().per_class.get(DispatchClass::Normal).max_extrinsic.unwrap(); + let db_weight: frame_support::weights::RuntimeDbWeight = ::DbWeight::get(); + (list, storage_info, max_extrinsic_weight, db_weight) } fn dispatch_benchmark( diff --git a/substrate/bin/node-template/runtime/src/lib.rs b/substrate/bin/node-template/runtime/src/lib.rs index 3b6a74be2512..9b7cf906737b 100644 --- a/substrate/bin/node-template/runtime/src/lib.rs +++ b/substrate/bin/node-template/runtime/src/lib.rs @@ -24,7 +24,9 @@ use sp_version::RuntimeVersion; use frame_support::genesis_builder_helper::{build_config, create_default_config}; // A few exports that help ease life for downstream crates. pub use frame_support::{ - construct_runtime, derive_impl, parameter_types, + construct_runtime, derive_impl, + dispatch::DispatchClass, + parameter_types, traits::{ ConstBool, ConstU128, ConstU32, ConstU64, ConstU8, KeyOwnerProofSystem, Randomness, StorageInfo, @@ -257,7 +259,7 @@ impl pallet_template::Config for Runtime { // Create the runtime by composing the FRAME pallets that were previously configured. construct_runtime!( - pub enum Runtime { + pub struct Runtime { System: frame_system, Timestamp: pallet_timestamp, Aura: pallet_aura, @@ -494,6 +496,8 @@ impl_runtime_apis! { fn benchmark_metadata(extra: bool) -> ( Vec, Vec, + Weight, + frame_support::weights::RuntimeDbWeight, ) { use frame_benchmarking::{baseline, Benchmarking, BenchmarkList}; use frame_support::traits::StorageInfoTrait; @@ -504,8 +508,9 @@ impl_runtime_apis! { list_benchmarks!(list, extra); let storage_info = AllPalletsWithSystem::storage_info(); - - (list, storage_info) + let max_extrinsic_weight = BlockWeights::get().per_class.get(DispatchClass::Normal).max_extrinsic.unwrap(); + let db_weight: frame_support::weights::RuntimeDbWeight = ::DbWeight::get(); + (list, storage_info, max_extrinsic_weight, db_weight) } fn dispatch_benchmark( diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index 02d084c6c62b..217109ea43d2 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -2848,6 +2848,8 @@ impl_runtime_apis! { fn benchmark_metadata(extra: bool) -> ( Vec, Vec, + Weight, + frame_support::weights::RuntimeDbWeight, ) { use frame_benchmarking::{baseline, Benchmarking, BenchmarkList}; use frame_support::traits::StorageInfoTrait; @@ -2866,8 +2868,10 @@ impl_runtime_apis! { list_benchmarks!(list, extra); let storage_info = AllPalletsWithSystem::storage_info(); + let max_extrinsic_weight = RuntimeBlockWeights::get().per_class.get(DispatchClass::Normal).max_extrinsic.unwrap(); + let db_weight: frame_support::weights::RuntimeDbWeight = ::DbWeight::get(); - (list, storage_info) + (list, storage_info, max_extrinsic_weight, db_weight) } fn dispatch_benchmark( diff --git a/substrate/client/cli/src/arg_enums.rs b/substrate/client/cli/src/arg_enums.rs index d436673cb9de..e11dfd2d0bff 100644 --- a/substrate/client/cli/src/arg_enums.rs +++ b/substrate/client/cli/src/arg_enums.rs @@ -65,6 +65,22 @@ impl std::fmt::Display for WasmExecutionMethod { } } +/// How to output the result of the sanity weight check and what to do when it fails. +#[allow(missing_docs)] +#[derive(Debug, Clone, Copy, ValueEnum, PartialEq)] +#[value(rename_all = "kebab-case")] +pub enum SanityWeightCheck { + /// Prints the results in the terminal and on failing returns an error. + Error, + /// Prints the results in the terminal. + Warning, + /// Sanity weight check is ignored. + Ignore, +} + +/// The default [`SanityWeightCheck`]. +pub const DEFAULT_SANITY_WEIGHT_CHECK: SanityWeightCheck = SanityWeightCheck::Error; + /// Converts the execution method and instantiation strategy command line arguments /// into an execution method which can be used internally. pub fn execution_method_from_cli( diff --git a/substrate/client/cli/src/params/shared_params.rs b/substrate/client/cli/src/params/shared_params.rs index 465372fba17d..2a517bc1143b 100644 --- a/substrate/client/cli/src/params/shared_params.rs +++ b/substrate/client/cli/src/params/shared_params.rs @@ -16,7 +16,7 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . -use crate::arg_enums::TracingReceiver; +use crate::{arg_enums::TracingReceiver, SanityWeightCheck, DEFAULT_SANITY_WEIGHT_CHECK}; use clap::Args; use sc_service::config::BasePath; use std::path::PathBuf; @@ -87,6 +87,16 @@ pub struct SharedParams { /// Receiver to process tracing messages. #[arg(long, value_name = "RECEIVER", value_enum, ignore_case = true, default_value_t = TracingReceiver::Log)] pub tracing_receiver: TracingReceiver, + + /// Sanity weight check for benchmarks. Checks that no benchmark function has a weight larger + /// than the (`DispatchClass::Normal`) max. normal class extrinsic weight. + #[arg( + long, + value_name = "ERROR-LEVEL", + value_enum, + default_value_t = DEFAULT_SANITY_WEIGHT_CHECK, + )] + pub sanity_weight_check: SanityWeightCheck, } impl SharedParams { diff --git a/substrate/frame/benchmarking/src/utils.rs b/substrate/frame/benchmarking/src/utils.rs index b9b3f91e2dd7..1374ebca2c90 100644 --- a/substrate/frame/benchmarking/src/utils.rs +++ b/substrate/frame/benchmarking/src/utils.rs @@ -17,7 +17,10 @@ //! Interfaces, types and utils for benchmarking a FRAME runtime. use codec::{Decode, Encode}; -use frame_support::{dispatch::DispatchErrorWithPostInfo, pallet_prelude::*, traits::StorageInfo}; +use frame_support::{ + dispatch::DispatchErrorWithPostInfo, pallet_prelude::*, traits::StorageInfo, + weights::RuntimeDbWeight, +}; use scale_info::TypeInfo; #[cfg(feature = "std")] use serde::{Deserialize, Serialize}; @@ -235,7 +238,7 @@ sp_api::decl_runtime_apis! { /// Parameters /// - `extra`: Also list benchmarks marked "extra" which would otherwise not be /// needed for weight calculation. - fn benchmark_metadata(extra: bool) -> (Vec, Vec); + fn benchmark_metadata(extra: bool) -> (Vec, Vec, Weight, RuntimeDbWeight); /// Dispatch the given benchmark. fn dispatch_benchmark(config: BenchmarkConfig) -> Result, sp_runtime::RuntimeString>; diff --git a/substrate/utils/frame/benchmarking-cli/Cargo.toml b/substrate/utils/frame/benchmarking-cli/Cargo.toml index 0314e3035c02..e4daecb00f6a 100644 --- a/substrate/utils/frame/benchmarking-cli/Cargo.toml +++ b/substrate/utils/frame/benchmarking-cli/Cargo.toml @@ -21,6 +21,7 @@ chrono = "0.4" clap = { version = "4.4.18", features = ["derive"] } codec = { package = "parity-scale-codec", version = "3.6.1" } comfy-table = { version = "7.1.0", default-features = false } +color-print = "0.3.4" handlebars = "4.2.2" Inflector = "0.11.4" itertools = "0.10.3" diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs index 5c76ca68e85f..a30a824585f0 100644 --- a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs +++ b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs @@ -18,10 +18,13 @@ use super::{writer, PalletCmd}; use codec::{Decode, Encode}; use frame_benchmarking::{ - Analysis, BenchmarkBatch, BenchmarkBatchSplitResults, BenchmarkList, BenchmarkParameter, - BenchmarkResult, BenchmarkSelector, + Analysis, AnalysisChoice, BenchmarkBatch, BenchmarkBatchSplitResults, BenchmarkList, + BenchmarkParameter, BenchmarkResult, BenchmarkSelector, +}; +use frame_support::{ + traits::StorageInfo, + weights::{RuntimeDbWeight, Weight}, }; -use frame_support::traits::StorageInfo; use linked_hash_map::LinkedHashMap; use sc_cli::{execution_method_from_cli, CliConfiguration, Result, SharedParams}; use sc_client_db::BenchmarkingState; @@ -48,11 +51,11 @@ const LOG_TARGET: &'static str = "frame::benchmark::pallet"; #[derive(Serialize, Debug, Clone, Eq, PartialEq)] pub(crate) struct ComponentRange { /// Name of the component. - name: String, + pub(crate) name: String, /// Minimal valid value of the component. min: u32, /// Maximal valid value of the component. - max: u32, + pub(crate) max: u32, } /// How the PoV size of a storage item should be estimated. @@ -175,20 +178,6 @@ impl PalletCmd { }; } - if let Some(json_input) = &self.json_input { - let raw_data = match std::fs::read(json_input) { - Ok(raw_data) => raw_data, - Err(error) => - return Err(format!("Failed to read {:?}: {}", json_input, error).into()), - }; - let batches: Vec = match serde_json::from_slice(&raw_data) { - Ok(batches) => batches, - Err(error) => - return Err(format!("Failed to deserialize {:?}: {}", json_input, error).into()), - }; - return self.output_from_results(&batches) - } - let spec = config.chain_spec; let pallet = self.pallet.clone().unwrap_or_default(); let pallet = pallet.as_bytes(); @@ -265,9 +254,25 @@ impl PalletCmd { .execute() .map_err(|e| format!("{}: {}", ERROR_METADATA_NOT_FOUND, e))?; - let (list, storage_info) = - <(Vec, Vec) as Decode>::decode(&mut &result[..]) - .map_err(|e| format!("Failed to decode benchmark metadata: {:?}", e))?; + let (list, storage_info, max_extrinsic_weight, db_weight) = + <(Vec, Vec, Weight, RuntimeDbWeight) as Decode>::decode( + &mut &result[..], + ) + .map_err(|e| format!("Failed to decode benchmark metadata: {:?}", e))?; + + if let Some(json_input) = &self.json_input { + let raw_data = match std::fs::read(json_input) { + Ok(raw_data) => raw_data, + Err(error) => + return Err(format!("Failed to read {:?}: {}", json_input, error).into()), + }; + let batches: Vec = match serde_json::from_slice(&raw_data) { + Ok(batches) => batches, + Err(error) => + return Err(format!("Failed to deserialize {:?}: {}", json_input, error).into()), + }; + return self.output_from_results(&batches, max_extrinsic_weight, db_weight) + } // Use the benchmark list and the user input to determine the set of benchmarks to run. let mut benchmarks_to_run = Vec::new(); @@ -509,7 +514,14 @@ impl PalletCmd { // Combine all of the benchmark results, so that benchmarks of the same pallet/function // are together. let batches = combine_batches(batches, batches_db); - self.output(&batches, &storage_info, &component_ranges, pov_modes) + self.output( + &batches, + &storage_info, + &component_ranges, + pov_modes, + max_extrinsic_weight, + db_weight, + ) } fn output( @@ -518,6 +530,8 @@ impl PalletCmd { storage_info: &[StorageInfo], component_ranges: &HashMap<(Vec, Vec), Vec>, pov_modes: PovModesMap, + max_extrinsic_weight: Weight, + db_weight: RuntimeDbWeight, ) -> Result<()> { // Jsonify the result and write it to a file or stdout if desired. if !self.jsonify(&batches)? { @@ -525,24 +539,48 @@ impl PalletCmd { self.print_summary(&batches, &storage_info, pov_modes.clone()) } + // Which analysis function should be used when outputting benchmarks + let analysis_choice: AnalysisChoice = + self.output_analysis.clone().try_into().map_err(writer::io_error)?; + let pov_analysis_choice: AnalysisChoice = + self.output_pov_analysis.clone().try_into().map_err(writer::io_error)?; + + // // Organize results by pallet into a JSON map + let all_results = writer::map_results( + &batches, + &storage_info, + &component_ranges, + pov_modes.clone(), + self.default_pov_mode, + &analysis_choice, + &pov_analysis_choice, + self.worst_case_map_values, + self.additional_trie_layers, + )?; + // Create the weights.rs file. if let Some(output_path) = &self.output { - writer::write_results( - &batches, - &storage_info, - &component_ranges, - pov_modes, - self.default_pov_mode, - output_path, - self, - )?; + writer::write_results(output_path, self, analysis_choice, &all_results)?; } + // Execute sanity weight check. + writer::sanity_weight_check( + all_results, + max_extrinsic_weight, + db_weight, + self.shared_params.sanity_weight_check, + )?; + Ok(()) } /// Re-analyze a batch historic benchmark timing data. Will not take the PoV into account. - fn output_from_results(&self, batches: &[BenchmarkBatchSplitResults]) -> Result<()> { + fn output_from_results( + &self, + batches: &[BenchmarkBatchSplitResults], + max_extrinsic_weight: Weight, + db_weight: RuntimeDbWeight, + ) -> Result<()> { let mut component_ranges = HashMap::<(Vec, Vec), HashMap>::new(); for batch in batches { @@ -574,7 +612,14 @@ impl PalletCmd { }) .collect(); - self.output(batches, &[], &component_ranges, Default::default()) + self.output( + batches, + &[], + &component_ranges, + Default::default(), + max_extrinsic_weight, + db_weight, + ) } /// Jsonifies the passed batches and writes them to stdout or into a file. diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/writer.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/writer.rs index 9493a693bbed..dcec7ffc256d 100644 --- a/substrate/utils/frame/benchmarking-cli/src/pallet/writer.rs +++ b/substrate/utils/frame/benchmarking-cli/src/pallet/writer.rs @@ -25,6 +25,7 @@ use std::{ use inflector::Inflector; use itertools::Itertools; +use sc_cli::SanityWeightCheck; use serde::Serialize; use crate::{ @@ -35,7 +36,10 @@ use crate::{ use frame_benchmarking::{ Analysis, AnalysisChoice, BenchmarkBatchSplitResults, BenchmarkResult, BenchmarkSelector, }; -use frame_support::traits::StorageInfo; +use frame_support::{ + traits::StorageInfo, + weights::{RuntimeDbWeight, Weight}, +}; use sp_core::hexdisplay::HexDisplay; use sp_runtime::traits::Zero; @@ -59,7 +63,7 @@ struct TemplateData { // This was the final data we have about each benchmark. #[derive(Serialize, Default, Debug, Clone, PartialEq)] -struct BenchmarkData { +pub(crate) struct BenchmarkData { name: String, components: Vec, #[serde(serialize_with = "string_serialize")] @@ -116,7 +120,7 @@ struct ComponentSlope { } // Small helper to create an `io::Error` from a string. -fn io_error(s: &str) -> std::io::Error { +pub(crate) fn io_error(s: &str) -> std::io::Error { use std::io::{Error, ErrorKind}; Error::new(ErrorKind::Other, s) } @@ -129,7 +133,7 @@ fn io_error(s: &str) -> std::io::Error { // p1 -> [b1, b2, b3] // p2 -> [b1, b2] // ``` -fn map_results( +pub(crate) fn map_results( batches: &[BenchmarkBatchSplitResults], storage_info: &[StorageInfo], component_ranges: &HashMap<(Vec, Vec), Vec>, @@ -172,6 +176,120 @@ fn map_results( Ok(all_benchmarks) } +// Calculates the total maximum weight of an extrinsic (if present, based on the max. component +// value) and compares it with the max. extrinsic weight allowed in a single block. +// +// `max_extrinsic_weight` & `db_weight` are obtained from the runtime configuration. +pub(crate) fn sanity_weight_check( + all_results: HashMap<(String, String), Vec>, + max_extrinsic_weight: Weight, + db_weight: RuntimeDbWeight, + sanity_weight_check: SanityWeightCheck, +) -> Result<(), std::io::Error> { + if sanity_weight_check == SanityWeightCheck::Ignore { + return Ok(()) + } + // Helper function to return max. component value (i.e. max. complexity parameter). + fn max_component(parameter: &ComponentSlope, component_ranges: &Vec) -> u64 { + for component in component_ranges { + if parameter.name == component.name { + return component.max.into() + } + } + 0 + } + + color_print::cprintln!( + "\nSanity Weight Check 🧐: each extrinsic's weight function is executed \ + in the worst case scenario and compared with the maximum extrinsic weight (the maximum weight \ + that can be put in a single block for an extrinsic with `DispatchClass::Normal`). In other words, \ + each extrinsic is checked whether it will fit in an empty (meaning; empty of \ + `DispatchClass::Normal` extrinsics) block.\n\nResults:\n" + ); + let mut sanity_weight_check_passed = true; + // Loop through all benchmark results. + for ((pallet, instance), results) in all_results.iter() { + println!("Pallet: {}\nInstance: {}\n", pallet, instance); + for result in results { + // Constant `ref_time` & `pov_size`. + let mut total_weight = Weight::from_parts( + result.base_weight.try_into().unwrap(), + result.base_calculated_proof_size.try_into().unwrap(), + ); + // `ref_time` multiplied by complexity parameter. + for component in &result.component_weight { + total_weight = total_weight.saturating_add( + Weight::from_parts(component.slope.try_into().unwrap(), 0) + .saturating_mul(max_component(&component, &result.component_ranges)), + ); + } + // Constant storage reads. + total_weight = + total_weight.saturating_add(db_weight.reads(result.base_reads.try_into().unwrap())); + // Storage reads multiplied by complexity parameter. + for component in &result.component_reads { + total_weight = total_weight.saturating_add( + db_weight + .reads(component.slope.try_into().unwrap()) + .saturating_mul(max_component(&component, &result.component_ranges)), + ); + } + // Constant storage writes. + total_weight = total_weight + .saturating_add(db_weight.writes(result.base_writes.try_into().unwrap())); + // Storage writes multiplied by complexity parameter. + for component in &result.component_writes { + total_weight = total_weight.saturating_add( + db_weight + .writes(component.slope.try_into().unwrap()) + .saturating_mul(max_component(&component, &result.component_ranges)), + ); + } + // `pov_size` multiplied by complexity parameter. + for component in &result.component_calculated_proof_size { + total_weight = total_weight.saturating_add( + Weight::from_parts(0, component.slope.try_into().unwrap()) + .saturating_mul(max_component(&component, &result.component_ranges)), + ); + } + // Comparing (worst case scenario) the extrinsic weight against the maximum extrinsic + // weight. + if total_weight.ref_time() > max_extrinsic_weight.ref_time() || + total_weight.proof_size() > max_extrinsic_weight.proof_size() + { + sanity_weight_check_passed = false; + color_print::cprintln!("WARNING!!!",); + color_print::cprintln!( + "The following extrinsic exceeds the maximum extrinsic weight:", + ); + } + color_print::cprintln!("- '{}': {:?}\nPercentage of max. extrinsic weight: {:.2}% (ref_time), {:.2}% (proof_size)\n", + result.name, + total_weight, + (total_weight.ref_time() as f64 / max_extrinsic_weight.ref_time() as f64) * 100.0, + (total_weight.proof_size() as f64 / max_extrinsic_weight.proof_size() as f64) * 100.0, + ); + } + } + match sanity_weight_check_passed { + false => { + color_print::cprintln!( + "Your extrinsics failed the Sanity Weight Check, please review \ + the extrinsic's logic and/or the associated benchmark function.\n", + ); + if sanity_weight_check == SanityWeightCheck::Error { + return Err(io_error(&String::from( + "One or more extrinsics exceed the maximum extrinsic weight", + ))) + } + }, + true => { + color_print::cprintln!("Your extrinsics passed the Sanity Weight Check 😃!\n"); + }, + } + Ok(()) +} + // Get an iterator of errors. fn extract_errors(errors: &Option>) -> impl Iterator + '_ { errors @@ -376,13 +494,10 @@ fn get_benchmark_data( /// Create weight file from benchmark data and Handlebars template. pub(crate) fn write_results( - batches: &[BenchmarkBatchSplitResults], - storage_info: &[StorageInfo], - component_ranges: &HashMap<(Vec, Vec), Vec>, - pov_modes: PovModesMap, - default_pov_mode: PovEstimationMode, path: &PathBuf, cmd: &PalletCmd, + analysis_choice: AnalysisChoice, + all_results: &HashMap<(String, String), Vec>, ) -> Result<(), sc_cli::Error> { // Use custom template if provided. let template: String = match &cmd.template { @@ -405,12 +520,6 @@ pub(crate) fn write_results( // Full CLI args passed to trigger the benchmark. let args = std::env::args().collect::>(); - // Which analysis function should be used when outputting benchmarks - let analysis_choice: AnalysisChoice = - cmd.output_analysis.clone().try_into().map_err(io_error)?; - let pov_analysis_choice: AnalysisChoice = - cmd.output_pov_analysis.clone().try_into().map_err(io_error)?; - if cmd.additional_trie_layers > 4 { println!( "WARNING: `additional_trie_layers` is unexpectedly large. It assumes {} storage items.", @@ -439,18 +548,6 @@ pub(crate) fn write_results( // Don't HTML escape any characters. handlebars.register_escape_fn(|s| -> String { s.to_string() }); - // Organize results by pallet into a JSON map - let all_results = map_results( - batches, - storage_info, - component_ranges, - pov_modes, - default_pov_mode, - &analysis_choice, - &pov_analysis_choice, - cmd.worst_case_map_values, - cmd.additional_trie_layers, - )?; let mut created_files = Vec::new(); for ((pallet, instance), results) in all_results.iter() { @@ -1254,6 +1351,50 @@ mod test { ); } + #[test] + fn sanity_weight_check_works() { + // Create benchmark results that will fail the sanity weight check. + let mapped_results = map_results( + &[test_data(b"first", b"first", BenchmarkParameter::a, 10, 3)], + &test_storage_info(), + &Default::default(), + Default::default(), + PovEstimationMode::MaxEncodedLen, + &AnalysisChoice::default(), + &AnalysisChoice::MedianSlopes, + 1_000_000, + 0, + ) + .unwrap(); + + // Flag set to `error`. + assert!(sanity_weight_check( + mapped_results.clone(), + Weight::from_parts(20_000, 1_000_000), + RuntimeDbWeight { read: 1000, write: 1000 }, + SanityWeightCheck::Error, + ) + .is_err()); + + // Flag set to `warning`. + assert!(sanity_weight_check( + mapped_results.clone(), + Weight::from_parts(20_000, 1_000_000), + RuntimeDbWeight { read: 1000, write: 1000 }, + SanityWeightCheck::Warning, + ) + .is_ok()); + + // Flag set to `ignore`. + assert!(sanity_weight_check( + mapped_results, + Weight::from_parts(20_000, 1_000_000), + RuntimeDbWeight { read: 1000, write: 1000 }, + SanityWeightCheck::Ignore, + ) + .is_ok()); + } + #[test] fn additional_trie_layers_work() { let mapped_results = map_results(