From eedbe77a761497e158bf44ca2035f3ca96340db6 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Wed, 28 Feb 2024 17:58:54 +0100 Subject: [PATCH 01/44] Do the stuff Signed-off-by: Oliver Tale-Yazdi --- Cargo.lock | 11 ++ Cargo.toml | 1 + substrate/bin/node/runtime/src/lib.rs | 2 +- .../frame/contracts/src/benchmarking/mod.rs | 4 +- .../freestanding-cli/Cargo.toml | 17 ++ .../freestanding-cli/src/main.rs | 32 +++ .../benchmarking-cli/src/pallet/command.rs | 185 +++++++++++++++--- .../frame/benchmarking-cli/src/pallet/mod.rs | 14 +- .../benchmarking-cli/src/pallet/types.rs | 64 ++++++ 9 files changed, 300 insertions(+), 30 deletions(-) create mode 100644 substrate/utils/frame/benchmarking-cli/freestanding-cli/Cargo.toml create mode 100644 substrate/utils/frame/benchmarking-cli/freestanding-cli/src/main.rs create mode 100644 substrate/utils/frame/benchmarking-cli/src/pallet/types.rs diff --git a/Cargo.lock b/Cargo.lock index c26c4081bbd9..bc8d918c7066 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5526,6 +5526,17 @@ dependencies = [ "thousands", ] +[[package]] +name = "frame-benchmarking-freestanding-cli" +version = "0.1.0" +dependencies = [ + "clap 4.5.1", + "frame-benchmarking-cli", + "sc-cli", + "sp-runtime", + "sp-statement-store", +] + [[package]] name = "frame-benchmarking-pallet-pov" version = "18.0.0" diff --git a/Cargo.toml b/Cargo.toml index d09f19f280cb..ee1756bc39ae 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,6 +11,7 @@ members = [ "bridges/bin/runtime-common", "bridges/modules/grandpa", "bridges/modules/messages", + "substrate/utils/frame/benchmarking-cli/freestanding-cli", "bridges/modules/parachains", "bridges/modules/relayers", "bridges/modules/xcm-bridge-hub", diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index 24c8f6f48e96..2ea420e1c9f3 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -1730,7 +1730,7 @@ impl pallet_nis::Config for Runtime { type Currency = Balances; type CurrencyBalance = Balance; type FundOrigin = frame_system::EnsureSigned; - type Counterpart = ItemOf, AccountId>; + type Counterpart = ItemOf, AccountId>; type CounterpartAmount = WithMaximumOf>; type Deficit = (); type IgnoredIssuance = (); diff --git a/substrate/frame/contracts/src/benchmarking/mod.rs b/substrate/frame/contracts/src/benchmarking/mod.rs index d6c24daa7c4d..a1bf40c06c84 100644 --- a/substrate/frame/contracts/src/benchmarking/mod.rs +++ b/substrate/frame/contracts/src/benchmarking/mod.rs @@ -297,7 +297,7 @@ benchmarks! { #[pov_mode = Measured] migration_noop { let version = LATEST_MIGRATION_VERSION; - assert_eq!(StorageVersion::get::>(), version); + StorageVersion::new(version).put::>(); }: { Migration::::migrate(Weight::MAX) } verify { @@ -321,7 +321,7 @@ benchmarks! { #[pov_mode = Measured] on_runtime_upgrade_noop { let latest_version = LATEST_MIGRATION_VERSION; - assert_eq!(StorageVersion::get::>(), latest_version); + StorageVersion::new(latest_version).put::>(); }: { as frame_support::traits::OnRuntimeUpgrade>::on_runtime_upgrade() } verify { diff --git a/substrate/utils/frame/benchmarking-cli/freestanding-cli/Cargo.toml b/substrate/utils/frame/benchmarking-cli/freestanding-cli/Cargo.toml new file mode 100644 index 000000000000..6d9c81828ad0 --- /dev/null +++ b/substrate/utils/frame/benchmarking-cli/freestanding-cli/Cargo.toml @@ -0,0 +1,17 @@ +[package] +name = "frame-benchmarking-freestanding-cli" +version = "0.1.0" +edition = "2021" + +[dependencies] +sp-runtime = { path = "../../../../primitives/runtime" } +clap = { version = "4.5.1", features = ["derive"] } +frame-benchmarking-cli = { path = "../" } +sc-cli = { path = "../../../../client/cli" } + +# optional host functions +sp-statement-store = { path = "../../../../primitives/statement-store", optional = true } + +[features] +default = [ "extended-host-functions" ] +extended-host-functions = [ "dep:sp-statement-store" ] diff --git a/substrate/utils/frame/benchmarking-cli/freestanding-cli/src/main.rs b/substrate/utils/frame/benchmarking-cli/freestanding-cli/src/main.rs new file mode 100644 index 000000000000..b4b97d9fbeea --- /dev/null +++ b/substrate/utils/frame/benchmarking-cli/freestanding-cli/src/main.rs @@ -0,0 +1,32 @@ +// This file is part of Substrate. + +// Copyright (C) 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. + +//! Entry point for the free standing `benchmark pallet` command runner. + +use clap::Parser; +use frame_benchmarking_cli::PalletCmd; +use sc_cli::Result; +use sp_runtime::traits::BlakeTwo256; + +#[cfg(feature = "extended-host-functions")] +type HostFunctions = sp_statement_store::runtime_api::HostFunctions; +#[cfg(not(feature = "extended-host-functions"))] +type HostFunctions = (); + +fn main() -> Result<()> { + PalletCmd::parse().run_with_spec::(None) +} diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs index dce49db15f7d..1bae499f5aa9 100644 --- a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs +++ b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs @@ -16,6 +16,7 @@ // limitations under the License. use super::{writer, PalletCmd}; +use crate::pallet::{types::FetchedCode, GenesisBuilder}; use codec::{Decode, Encode}; use frame_benchmarking::{ Analysis, BenchmarkBatch, BenchmarkBatchSplitResults, BenchmarkList, BenchmarkParameter, @@ -23,7 +24,7 @@ use frame_benchmarking::{ }; use frame_support::traits::StorageInfo; use linked_hash_map::LinkedHashMap; -use sc_cli::{execution_method_from_cli, CliConfiguration, Result, SharedParams}; +use sc_cli::{execution_method_from_cli, ChainSpec, CliConfiguration, Result, SharedParams}; use sc_client_db::BenchmarkingState; use sc_executor::{HeapAllocStrategy, WasmExecutor, DEFAULT_HEAP_ALLOC_STRATEGY}; use sc_service::Configuration; @@ -33,13 +34,13 @@ use sp_core::{ testing::{TestOffchainExt, TestTransactionPoolExt}, OffchainDbExt, OffchainWorkerExt, TransactionPoolExt, }, - traits::{CallContext, ReadRuntimeVersionExt}, + traits::{CallContext, ReadRuntimeVersionExt, WrappedRuntimeCode}, }; use sp_externalities::Extensions; use sp_keystore::{testing::MemoryKeystore, KeystoreExt}; use sp_runtime::traits::Hash; -use sp_state_machine::StateMachine; -use std::{collections::HashMap, fmt::Debug, fs, str::FromStr, time}; +use sp_state_machine::{OverlayedChanges, StateMachine}; +use std::{borrow::Cow, collections::HashMap, fmt::Debug, fs, str::FromStr, time}; /// Logging target const LOG_TARGET: &'static str = "frame::benchmark::pallet"; @@ -139,17 +140,33 @@ This could mean that you either did not build the node correctly with the \ `--features runtime-benchmarks` flag, or the chain spec that you are using was \ not created by a node that was compiled with the flag"; +/// Warn when using the chain spec to generate the genesis state. +const WARN_SPEC_GENESIS_CTOR: &'static str = "Using the chain spec instead of the runtime to generate the genesis state is deprecated. +Please remove the `--chain`/`--dev`/`--local` argument, point `--runtime` to your runtime blob and set `--genesis-builder=runtime`. +This warning may become a hard error after December 2024."; + impl PalletCmd { /// Runs the command and benchmarks a pallet. pub fn run(&self, config: Configuration) -> Result<()> + where + Hasher: Hash, + ExtraHostFunctions: sp_wasm_interface::HostFunctions, + { + self.run_with_spec::(Some(config.chain_spec)) + } + + pub fn run_with_spec( + &self, + chain_spec: Option>, + ) -> Result<()> where Hasher: Hash, ExtraHostFunctions: sp_wasm_interface::HostFunctions, { let _d = self.execution.as_ref().map(|exec| { - // We print the warning at the end, since there is often A LOT of output. + // We print the error at the end, since there is often A LOT of output. sp_core::defer::DeferGuard::new(move || { - log::warn!( + log::error!( target: LOG_TARGET, "⚠️ Argument `--execution` is deprecated. Its value of `{exec}` has on effect.", ) @@ -188,15 +205,15 @@ impl PalletCmd { return self.output_from_results(&batches) } - let spec = config.chain_spec; let pallet = self.pallet.clone().unwrap_or_default(); let pallet = pallet.as_bytes(); let extrinsic = self.extrinsic.clone().unwrap_or_default(); let extrinsic_split: Vec<&str> = extrinsic.split(',').collect(); let extrinsics: Vec<_> = extrinsic_split.iter().map(|x| x.trim().as_bytes()).collect(); - let genesis_storage = spec.build_storage()?; - let mut changes = Default::default(); + let (genesis_storage, genesis_changes) = self.genesis_storage::(&chain_spec)?; + let mut changes = genesis_changes.clone(); + let cache_size = Some(self.database_cache_size as usize); let state_with_tracking = BenchmarkingState::::new( genesis_storage.clone(), @@ -218,11 +235,11 @@ impl PalletCmd { let method = execution_method_from_cli(self.wasm_method, self.wasmtime_instantiation_strategy); - let heap_pages = - self.heap_pages - .map_or(DEFAULT_HEAP_ALLOC_STRATEGY, |p| HeapAllocStrategy::Static { - extra_pages: p as _, - }); + // Get Benchmark List + let state = &state_without_tracking; + let runtime = self.runtime_blob(&state_without_tracking)?; + let runtime_code = runtime.code()?; + let alloc_strategy = Self::alloc_strategy(runtime_code.heap_pages); let executor = WasmExecutor::<( sp_io::SubstrateHostFunctions, @@ -230,8 +247,8 @@ impl PalletCmd { ExtraHostFunctions, )>::builder() .with_execution_method(method) - .with_onchain_heap_alloc_strategy(heap_pages) - .with_offchain_heap_alloc_strategy(heap_pages) + .with_onchain_heap_alloc_strategy(alloc_strategy) + .with_offchain_heap_alloc_strategy(alloc_strategy) .with_max_runtime_instances(2) .with_runtime_cache_size(2) .build(); @@ -249,8 +266,6 @@ impl PalletCmd { extensions }; - // Get Benchmark List - let state = &state_without_tracking; let result = StateMachine::new( state, &mut changes, @@ -258,7 +273,7 @@ impl PalletCmd { "Benchmark_benchmark_metadata", &(self.extra).encode(), &mut extensions(), - &sp_state_machine::backend::BackendRuntimeCode::new(state).runtime_code()?, + &runtime_code, CallContext::Offchain, ) .execute() @@ -323,6 +338,7 @@ impl PalletCmd { let mut component_ranges = HashMap::<(Vec, Vec), Vec>::new(); let pov_modes = Self::parse_pov_modes(&benchmarks_to_run)?; + //use rayon::prelude::IntoParallelIterator; for (pallet, extrinsic, components, _) in benchmarks_to_run.clone() { log::info!( target: LOG_TARGET, @@ -394,8 +410,7 @@ impl PalletCmd { ) .encode(), &mut extensions(), - &sp_state_machine::backend::BackendRuntimeCode::new(state) - .runtime_code()?, + &runtime_code, CallContext::Offchain, ) .execute() @@ -434,8 +449,7 @@ impl PalletCmd { ) .encode(), &mut extensions(), - &sp_state_machine::backend::BackendRuntimeCode::new(state) - .runtime_code()?, + &runtime_code, CallContext::Offchain, ) .execute() @@ -466,8 +480,7 @@ impl PalletCmd { ) .encode(), &mut extensions(), - &sp_state_machine::backend::BackendRuntimeCode::new(state) - .runtime_code()?, + &runtime_code, CallContext::Offchain, ) .execute() @@ -488,7 +501,7 @@ impl PalletCmd { log::info!( target: LOG_TARGET, - "Running benchmark: {}.{}({} args) {}/{} {}/{}", + "Running benchmark: {}::{}({} args) {}/{} {}/{}", String::from_utf8(pallet.clone()) .expect("Encoded from String; qed"), String::from_utf8(extrinsic.clone()) @@ -511,6 +524,126 @@ impl PalletCmd { self.output(&batches, &storage_info, &component_ranges, pov_modes) } + /// Produce a genesis storage and genesis changes. + /// + /// It would be easier to only return only type, but there is no easy way to convert them. + // TODO: Re-write `BenchmarkingState` to not be such a clusterfuck and only accept + // `OverlayedChanges` instead of a mix between `OverlayedChanges` and `State`. But this can only + // be done once we deprecated and removed the legacy interface. + fn genesis_storage( + &self, + chain_spec: &Option>, + ) -> Result<(sp_storage::Storage, OverlayedChanges)> { + Ok(match (self.genesis_builder, self.runtime.is_some()) { + (Some(GenesisBuilder::None), _) => Default::default(), + (Some(GenesisBuilder::Spec), _) | (None, false) => { + log::warn!("{WARN_SPEC_GENESIS_CTOR}"); + let Some(chain_spec) = chain_spec else { + return Err( + "No chain spec specified although to generate the genesis state".into() + ); + }; + + (chain_spec.build_storage()?, Default::default()) + }, + (Some(GenesisBuilder::Runtime), _) | (None, true) => + (Default::default(), self.genesis_from_runtime::()?), + }) + } + + /// Generate the genesis changeset with the runtime API. + fn genesis_from_runtime(&self) -> Result> { + let state = BenchmarkingState::::new( + Default::default(), + Some(self.database_cache_size as usize), + false, + false, + )?; + + // Create a dummy WasmExecutor just to build the genesis storage. + let executor = WasmExecutor::<( + sp_io::SubstrateHostFunctions, + frame_benchmarking::benchmarking::HostFunctions, + (), + )>::builder() + // TODO add extra host functions + .with_allow_missing_host_functions(true) + .build(); + + let runtime = self.runtime_blob(&state)?; + let runtime_code = runtime.code()?; + + // TODO register extensions + let genesis_json: Vec = StateMachine::new( + &state, + &mut Default::default(), + &executor, + "GenesisBuilder_create_default_config", + &[], // no args for this call + &mut Extensions::default(), + &runtime_code, + CallContext::Offchain, + ) + .execute() + .map_err(|e| format!("Could not call GenesisBuilder Runtime API: {}", e))?; + let genesis_json: Vec = codec::Decode::decode(&mut &genesis_json[..]) + .map_err(|e| format!("Failed to decode genesis config: {:?}", e))?; + // Sanity check that it is JSON before we plug it into the next runtime call. + assert!(serde_json::from_slice::(&genesis_json).is_ok()); + + let mut changes = Default::default(); + let genesis_state: Vec = StateMachine::new( + &state, + &mut changes, + &executor, + "GenesisBuilder_build_config", + &genesis_json.encode(), + &mut Extensions::default(), + &runtime_code, + CallContext::Offchain, + ) + .execute() + .map_err(|e| format!("Could not call GenesisBuilder Runtime API: {}", e))?; + + let _: () = codec::Decode::decode(&mut &genesis_state[..]) + .map_err(|e| format!("Failed to decode (): {:?}", e))?; + + Ok(changes) + } + + /// The runtime blob for this benchmark. + /// + /// The runtime will either be loaded from the `:code` key or can be overwritten with the + /// `--runtime` arg. + fn runtime_blob<'a, H: Hash>( + &self, + state: &'a BenchmarkingState, + ) -> Result, H>> { + match (&self.runtime, &self.shared_params.chain) { + (Some(runtime), None) => { + log::info!("Loading WASM from file: {}", runtime.display()); + let code = fs::read(runtime)?; + let hash = sp_core::blake2_256(&code).to_vec(); + let wrapped_code = WrappedRuntimeCode(Cow::Owned(code)); + + Ok(FetchedCode::FromFile { wrapped_code, heap_pages: self.heap_pages, hash }) + }, + (None, Some(_)) => { + log::info!("Loading WASM from genesis state"); + let state = sp_state_machine::backend::BackendRuntimeCode::new(state); + + Ok(FetchedCode::FromGenesis { state }) + }, + _ => Err("Exactly one of `--runtime` or `--chain` must be provided".into()), + } + } + + fn alloc_strategy(heap_pages: Option) -> HeapAllocStrategy { + heap_pages.map_or(DEFAULT_HEAP_ALLOC_STRATEGY, |p| HeapAllocStrategy::Static { + extra_pages: p as _, + }) + } + fn output( &self, batches: &[BenchmarkBatchSplitResults], diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/mod.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/mod.rs index c69ce1765fc9..291be3492bbf 100644 --- a/substrate/utils/frame/benchmarking-cli/src/pallet/mod.rs +++ b/substrate/utils/frame/benchmarking-cli/src/pallet/mod.rs @@ -16,9 +16,10 @@ // limitations under the License. mod command; +mod types; mod writer; -use crate::shared::HostInfoParams; +use crate::{pallet::types::GenesisBuilder, shared::HostInfoParams}; use sc_cli::{ WasmExecutionMethod, WasmtimeInstantiationStrategy, DEFAULT_WASMTIME_INSTANTIATION_STRATEGY, DEFAULT_WASM_EXECUTION_METHOD, @@ -150,6 +151,17 @@ pub struct PalletCmd { )] pub wasmtime_instantiation_strategy: WasmtimeInstantiationStrategy, + /// Optional runtime blob to use instead of the one from the genesis config. + #[arg(long, conflicts_with = "chain")] + pub runtime: Option, + + /// How to construct the genesis state. + /// + /// Uses `GenesisBuilder::Spec` by default and `GenesisConstructor::Runtime` if `runtime` is + /// set. + #[arg(long, value_enum)] + pub genesis_builder: Option, + /// DEPRECATED: This argument has no effect. #[arg(long = "execution")] pub execution: Option, diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/types.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/types.rs new file mode 100644 index 000000000000..80d2e527e839 --- /dev/null +++ b/substrate/utils/frame/benchmarking-cli/src/pallet/types.rs @@ -0,0 +1,64 @@ +// This file is part of Substrate. + +// Copyright (C) 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. + +//! Various types used by this crate. + +use sc_cli::Result; +use sp_core::traits::{RuntimeCode, WrappedRuntimeCode}; +use sp_runtime::traits::Hash; + +/// How the genesis state for benchmarking should be build. +#[derive(clap::ValueEnum, Debug, Eq, PartialEq, Clone, Copy)] +#[clap(rename_all = "kebab-case")] +pub enum GenesisBuilder { + /// Do not provide any genesis state. + /// + /// Benchmarks are advised to function with this, since they should setup their own required + /// state. However, to keep backwards compatibility, this is not the default. + None, + /// Let the runtime build the genesis state through its `BuildGenesisConfig` runtime API. + Runtime, + /// Use the spec file to build the genesis state. This fails when there is no spec. + Spec, + // NOTE: We could add from JSON file here, but for now its useless. +} + +/// A runtime blob that was either fetched from genesis storage or loaded from a file. +// NOTE: This enum is only needed for the annoying lifetime bounds on `RuntimeCode`. Otherwise we +// could just directly return the blob. +pub enum FetchedCode<'a, B, H> { + FromGenesis { state: sp_state_machine::backend::BackendRuntimeCode<'a, B, H> }, + FromFile { wrapped_code: WrappedRuntimeCode<'a>, heap_pages: Option, hash: Vec }, +} + +impl<'a, B, H> FetchedCode<'a, B, H> +where + H: Hash, + B: sc_client_api::StateBackend, +{ + /// The runtime blob. + pub fn code(&'a self) -> Result> { + match self { + Self::FromGenesis { state } => state.runtime_code().map_err(Into::into), + Self::FromFile { wrapped_code, heap_pages, hash } => Ok(RuntimeCode { + code_fetcher: wrapped_code, + heap_pages: *heap_pages, + hash: hash.clone(), + }), + } + } +} From 1bab526524631d107a78822752b0b645aaf25019 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Wed, 28 Feb 2024 20:25:43 +0100 Subject: [PATCH 02/44] Make stuff work Signed-off-by: Oliver Tale-Yazdi --- Cargo.lock | 29 ++++- polkadot/runtime/common/src/auctions.rs | 7 +- .../runtime/common/src/paras_registrar/mod.rs | 20 +++- .../runtime/parachains/src/configuration.rs | 4 +- substrate/bin/node/cli/src/command.rs | 1 + .../freestanding-cli/Cargo.toml | 15 ++- .../freestanding-cli/src/main.rs | 9 +- .../benchmarking-cli/src/pallet/command.rs | 100 +++++++++++++----- .../frame/benchmarking-cli/src/pallet/mod.rs | 1 - 9 files changed, 148 insertions(+), 38 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5970c00b5fee..d0fa6ec760a1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -275,9 +275,9 @@ dependencies = [ [[package]] name = "anstyle" -version = "1.0.2" +version = "1.0.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "15c4c2c83f81532e5845a733998b6971faca23490340a418e9b72a3ec9de12ea" +checksum = "8901269c6307e8d93993578286ac0edf7f195079ffff5ebdeea6a59ffb7e36bc" [[package]] name = "anstyle-parse" @@ -4996,6 +4996,16 @@ dependencies = [ "syn 2.0.50", ] +[[package]] +name = "env_filter" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a009aa4810eb158359dda09d0c87378e4bbb89b5a801f016885a4707ba24f7ea" +dependencies = [ + "log", + "regex", +] + [[package]] name = "env_logger" version = "0.8.4" @@ -5032,6 +5042,19 @@ dependencies = [ "termcolor", ] +[[package]] +name = "env_logger" +version = "0.11.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6c012a26a7f605efc424dd53697843a72be7dc86ad2d01f7814337794a12231d" +dependencies = [ + "anstream", + "anstyle", + "env_filter", + "humantime", + "log", +] + [[package]] name = "environmental" version = "1.1.4" @@ -5555,7 +5578,9 @@ name = "frame-benchmarking-freestanding-cli" version = "0.1.0" dependencies = [ "clap 4.5.1", + "env_logger 0.11.2", "frame-benchmarking-cli", + "log", "sc-cli", "sp-runtime", "sp-statement-store", diff --git a/polkadot/runtime/common/src/auctions.rs b/polkadot/runtime/common/src/auctions.rs index 46ab673a7a0c..8a2c3c54b5ef 100644 --- a/polkadot/runtime/common/src/auctions.rs +++ b/polkadot/runtime/common/src/auctions.rs @@ -1737,7 +1737,7 @@ mod benchmarking { traits::{EnsureOrigin, OnInitialize}, }; use frame_system::RawOrigin; - use runtime_parachains::paras; + use runtime_parachains::{configuration, paras}; use sp_runtime::{traits::Bounded, SaturatedConversion}; use frame_benchmarking::{account, benchmarks, whitelisted_caller, BenchmarkError}; @@ -1874,6 +1874,11 @@ mod benchmarking { let (lease_length, offset) = T::Leaser::lease_period_length(); frame_system::Pallet::::set_block_number(offset + One::one()); + let mut cfg = configuration::Pallet::::config(); + cfg.max_head_data_size = cfg.max_head_data_size.min(1 * 1024 * 1024); + cfg.max_code_size = cfg.max_code_size.min(1 * 1024 * 1024); + configuration::ActiveConfig::::put(cfg); + // Create a new auction let duration: BlockNumberFor = lease_length / 2u32.into(); let lease_period_index = LeasePeriodOf::::zero(); diff --git a/polkadot/runtime/common/src/paras_registrar/mod.rs b/polkadot/runtime/common/src/paras_registrar/mod.rs index 4541b88ec6fe..63e4a36d748b 100644 --- a/polkadot/runtime/common/src/paras_registrar/mod.rs +++ b/polkadot/runtime/common/src/paras_registrar/mod.rs @@ -520,14 +520,30 @@ impl Registrar for Pallet { #[cfg(any(feature = "runtime-benchmarks", test))] fn worst_head_data() -> HeadData { let max_head_size = configuration::Pallet::::config().max_head_data_size; - assert!(max_head_size > 0, "max_head_data can't be zero for generating worst head data."); + if max_head_size == 0 { + log::warn!( + "max_head_data can't be zero for generating worst head data - using default." + ); + let mut cfg = configuration::ActiveConfig::::get(); + cfg.max_head_data_size = 1 * 1024 * 1024; + configuration::ActiveConfig::::put(&cfg); + } vec![0u8; max_head_size as usize].into() } #[cfg(any(feature = "runtime-benchmarks", test))] fn worst_validation_code() -> ValidationCode { let max_code_size = configuration::Pallet::::config().max_code_size; - assert!(max_code_size > 0, "max_code_size can't be zero for generating worst code data."); + + if max_code_size == 0 { + log::warn!( + "max_code_size can't be zero for generating worst validation code - using default." + ); + let mut cfg = configuration::ActiveConfig::::get(); + cfg.max_code_size = 1 * 1024 * 1024; + configuration::ActiveConfig::::put(&cfg); + } + let validation_code = vec![0u8; max_code_size as usize]; validation_code.into() } diff --git a/polkadot/runtime/parachains/src/configuration.rs b/polkadot/runtime/parachains/src/configuration.rs index b0e9d03df886..37031796d705 100644 --- a/polkadot/runtime/parachains/src/configuration.rs +++ b/polkadot/runtime/parachains/src/configuration.rs @@ -283,7 +283,7 @@ impl> Default for HostConfiguration = + pub type ActiveConfig = StorageValue<_, HostConfiguration>, ValueQuery>; /// Pending configuration changes. diff --git a/substrate/bin/node/cli/src/command.rs b/substrate/bin/node/cli/src/command.rs index 964517320286..31fbdc7b63ad 100644 --- a/substrate/bin/node/cli/src/command.rs +++ b/substrate/bin/node/cli/src/command.rs @@ -107,6 +107,7 @@ pub fn run() -> Result<()> { ) } + HashingFor::::asdf; cmd.run::, sp_statement_store::runtime_api::HostFunctions>(config) }, BenchmarkCmd::Block(cmd) => { diff --git a/substrate/utils/frame/benchmarking-cli/freestanding-cli/Cargo.toml b/substrate/utils/frame/benchmarking-cli/freestanding-cli/Cargo.toml index 6d9c81828ad0..de7aa26bf2a1 100644 --- a/substrate/utils/frame/benchmarking-cli/freestanding-cli/Cargo.toml +++ b/substrate/utils/frame/benchmarking-cli/freestanding-cli/Cargo.toml @@ -1,7 +1,18 @@ [package] name = "frame-benchmarking-freestanding-cli" version = "0.1.0" -edition = "2021" +authors.workspace = true +edition.workspace = true +license = "Apache-2.0" +homepage = "https://substrate.io" +repository.workspace = true +description = "Freestanding CLI for benchmarking FRAME pallets." + +[lints] +workspace = true + +[package.metadata.docs.rs] +targets = ["x86_64-unknown-linux-gnu"] [dependencies] sp-runtime = { path = "../../../../primitives/runtime" } @@ -11,6 +22,8 @@ sc-cli = { path = "../../../../client/cli" } # optional host functions sp-statement-store = { path = "../../../../primitives/statement-store", optional = true } +env_logger = "0.11.2" +log.workspace = true [features] default = [ "extended-host-functions" ] diff --git a/substrate/utils/frame/benchmarking-cli/freestanding-cli/src/main.rs b/substrate/utils/frame/benchmarking-cli/freestanding-cli/src/main.rs index b4b97d9fbeea..209da5dc7a68 100644 --- a/substrate/utils/frame/benchmarking-cli/freestanding-cli/src/main.rs +++ b/substrate/utils/frame/benchmarking-cli/freestanding-cli/src/main.rs @@ -16,6 +16,11 @@ // limitations under the License. //! Entry point for the free standing `benchmark pallet` command runner. +//! +//! This runner has the advantage that the node does not need to be build with the `try-runtime` +//! feature or similar. It can be shipped independently and used by any chain - as long as that +//! chain's runtime is not making use of 3rd party host functions. In that case, it would need to be +//! forked (or extended with some plugin system). use clap::Parser; use frame_benchmarking_cli::PalletCmd; @@ -28,5 +33,7 @@ type HostFunctions = sp_statement_store::runtime_api::HostFunctions; type HostFunctions = (); fn main() -> Result<()> { - PalletCmd::parse().run_with_spec::(None) + env_logger::init(); + log::info!("Benchmarking pallet CLI"); + PalletCmd::parse().run_with_maybe_spec::(None) } diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs index 858a5bf27999..af842d0ca52d 100644 --- a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs +++ b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs @@ -15,9 +15,8 @@ // See the License for the specific language governing permissions and // limitations under the License. -use super::{writer, PalletCmd}; -use crate::pallet::{types::FetchedCode, GenesisBuilder}; use super::{writer, ListOutput, PalletCmd}; +use crate::pallet::{types::FetchedCode, GenesisBuilder}; use codec::{Decode, Encode}; use frame_benchmarking::{ Analysis, BenchmarkBatch, BenchmarkBatchSplitResults, BenchmarkList, BenchmarkParameter, @@ -41,9 +40,8 @@ use sp_externalities::Extensions; use sp_keystore::{testing::MemoryKeystore, KeystoreExt}; use sp_runtime::traits::Hash; use sp_state_machine::{OverlayedChanges, StateMachine}; -use std::{borrow::Cow, collections::HashMap, fmt::Debug, fs, str::FromStr, time}; -use sp_state_machine::StateMachine; use std::{ + borrow::Cow, collections::{BTreeMap, BTreeSet, HashMap}, fmt::Debug, fs, @@ -161,10 +159,10 @@ impl PalletCmd { Hasher: Hash, ExtraHostFunctions: sp_wasm_interface::HostFunctions, { - self.run_with_spec::(Some(config.chain_spec)) + self.run_with_maybe_spec::(Some(config.chain_spec)) } - pub fn run_with_spec( + pub fn run_with_maybe_spec( &self, chain_spec: Option>, ) -> Result<()> @@ -354,12 +352,13 @@ impl PalletCmd { let mut component_ranges = HashMap::<(Vec, Vec), Vec>::new(); let pov_modes = Self::parse_pov_modes(&benchmarks_to_run)?; - for (pallet, extrinsic, components, _, pallet_name, extrinsic_name) in - benchmarks_to_run.clone() + 'outer: for (i, (pallet, extrinsic, components, _, pallet_name, extrinsic_name)) in + benchmarks_to_run.clone().into_iter().enumerate() { log::info!( target: LOG_TARGET, - "Starting benchmark: {pallet_name}::{extrinsic_name}" + "[{: >3} %] Starting benchmark: {pallet_name}::{extrinsic_name}", + (i * 100) / benchmarks_to_run.len(), ); let all_components = if components.is_empty() { vec![Default::default()] @@ -411,7 +410,7 @@ impl PalletCmd { // First we run a verification if !self.no_verify { let state = &state_without_tracking; - let result = StateMachine::new( + let result = match StateMachine::new( state, &mut changes, &executor, @@ -429,23 +428,35 @@ impl PalletCmd { CallContext::Offchain, ) .execute() - .map_err(|e| { - format!("Error executing and verifying runtime benchmark: {}", e) - })?; + { + Err(e) => { + log::error!("Error executing and verifying runtime benchmark: {}", e); + continue 'outer + }, + Ok(r) => r, + }; // Dont use these results since verification code will add overhead. let _batch = - , String> as Decode>::decode( + match , String> as Decode>::decode( &mut &result[..], - ) - .map_err(|e| format!("Failed to decode benchmark results: {:?}", e))? - .map_err(|e| { - format!("Benchmark {pallet_name}::{extrinsic_name} failed: {e}",) - })?; + ) { + Err(e) => { + log::error!("Failed to decode benchmark results: {:?}", e); + continue 'outer + }, + Ok(Err(e)) => { + log::error!( + "Benchmark {pallet_name}::{extrinsic_name} failed: {e}", + ); + continue 'outer + }, + Ok(Ok(b)) => b, + }; } // Do one loop of DB tracking. { let state = &state_with_tracking; - let result = StateMachine::new( + let result = match StateMachine::new( state, // todo remove tracking &mut changes, &executor, @@ -463,15 +474,32 @@ impl PalletCmd { CallContext::Offchain, ) .execute() - .map_err(|e| format!("Error executing runtime benchmark: {}", e))?; + { + Err(e) => { + log::error!("Error executing runtime benchmark: {}", e); + continue 'outer + }, + Ok(r) => r, + }; let batch = - , String> as Decode>::decode( + match , String> as Decode>::decode( &mut &result[..], - ) - .map_err(|e| format!("Failed to decode benchmark results: {:?}", e))??; - - batches_db.extend(batch); + ) { + Err(e) => { + log::error!("Failed to decode benchmark results: {:?}", e); + continue 'outer + }, + Ok(Err(e)) => { + log::error!( + "Benchmark {pallet_name}::{extrinsic_name} failed: {e}", + ); + continue 'outer + }, + Ok(Ok(b)) => b, + }; + + batches_db.extend(batch); // FAIL-CI check if missing and error } // Finally run a bunch of loops to get extrinsic timing information. for r in 0..self.external_repeat { @@ -494,6 +522,7 @@ impl PalletCmd { CallContext::Offchain, ) .execute() + // No continue here, any error should have been caught by the verification step. .map_err(|e| format!("Error executing runtime benchmark: {}", e))?; let batch = @@ -511,7 +540,8 @@ impl PalletCmd { log::info!( target: LOG_TARGET, - "Running benchmark: {pallet_name}::{extrinsic_name}({} args) {}/{} {}/{}", + "[{: >3} %] Running benchmark: {pallet_name}::{extrinsic_name}({} args) {}/{} {}/{}", + (i * 100) / benchmarks_to_run.len(), components.len(), s + 1, // s starts at 0. all_components.len(), @@ -524,6 +554,18 @@ impl PalletCmd { } } + assert!(batches_db.len() == batches.len() / self.external_repeat as usize); + if let Some(failed) = benchmarks_to_run.len().checked_sub(batches.len()) { + log::error!( + target: LOG_TARGET, + "Some benchmarks failed, but the results where still written to disk." + ); + return Err(format!( + "There were {failed} benchmarks that either returned an error or panicked." + ) + .into()); + } + // Combine all of the benchmark results, so that benchmarks of the same pallet/function // are together. let batches = combine_batches(batches, batches_db); @@ -550,7 +592,9 @@ impl PalletCmd { ); }; - (chain_spec.build_storage()?, Default::default()) + let storage = chain_spec.build_storage().map_err(|e| format!("The runtime returned an error when trying to build the genesis storage. Please ensure that all pallets define a genesis config that can be built. For more info see: https://github.com/paritytech/polkadot-sdk/pull/3412\nError: {e}"))?; + + (storage, Default::default()) }, (Some(GenesisBuilder::Runtime), _) | (None, true) => (Default::default(), self.genesis_from_runtime::()?), diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/mod.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/mod.rs index 4dfe05b30f4a..81fb45fe8d12 100644 --- a/substrate/utils/frame/benchmarking-cli/src/pallet/mod.rs +++ b/substrate/utils/frame/benchmarking-cli/src/pallet/mod.rs @@ -20,7 +20,6 @@ mod types; mod writer; use crate::{pallet::types::GenesisBuilder, shared::HostInfoParams}; -use crate::shared::HostInfoParams; use clap::ValueEnum; use sc_cli::{ WasmExecutionMethod, WasmtimeInstantiationStrategy, DEFAULT_WASMTIME_INSTANTIATION_STRATEGY, From a0d27b8bcb4585fc24a2231fc8740a490eebc2a3 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Wed, 28 Feb 2024 20:42:15 +0100 Subject: [PATCH 03/44] Remove test code Signed-off-by: Oliver Tale-Yazdi --- polkadot/runtime/common/src/auctions.rs | 7 +------ .../runtime/common/src/paras_registrar/mod.rs | 20 ++----------------- .../runtime/parachains/src/configuration.rs | 2 +- substrate/bin/node/cli/src/command.rs | 1 - .../freestanding-cli/src/main.rs | 2 +- .../benchmarking-cli/src/pallet/command.rs | 4 ++-- 6 files changed, 7 insertions(+), 29 deletions(-) diff --git a/polkadot/runtime/common/src/auctions.rs b/polkadot/runtime/common/src/auctions.rs index 8a2c3c54b5ef..46ab673a7a0c 100644 --- a/polkadot/runtime/common/src/auctions.rs +++ b/polkadot/runtime/common/src/auctions.rs @@ -1737,7 +1737,7 @@ mod benchmarking { traits::{EnsureOrigin, OnInitialize}, }; use frame_system::RawOrigin; - use runtime_parachains::{configuration, paras}; + use runtime_parachains::paras; use sp_runtime::{traits::Bounded, SaturatedConversion}; use frame_benchmarking::{account, benchmarks, whitelisted_caller, BenchmarkError}; @@ -1874,11 +1874,6 @@ mod benchmarking { let (lease_length, offset) = T::Leaser::lease_period_length(); frame_system::Pallet::::set_block_number(offset + One::one()); - let mut cfg = configuration::Pallet::::config(); - cfg.max_head_data_size = cfg.max_head_data_size.min(1 * 1024 * 1024); - cfg.max_code_size = cfg.max_code_size.min(1 * 1024 * 1024); - configuration::ActiveConfig::::put(cfg); - // Create a new auction let duration: BlockNumberFor = lease_length / 2u32.into(); let lease_period_index = LeasePeriodOf::::zero(); diff --git a/polkadot/runtime/common/src/paras_registrar/mod.rs b/polkadot/runtime/common/src/paras_registrar/mod.rs index 63e4a36d748b..4541b88ec6fe 100644 --- a/polkadot/runtime/common/src/paras_registrar/mod.rs +++ b/polkadot/runtime/common/src/paras_registrar/mod.rs @@ -520,30 +520,14 @@ impl Registrar for Pallet { #[cfg(any(feature = "runtime-benchmarks", test))] fn worst_head_data() -> HeadData { let max_head_size = configuration::Pallet::::config().max_head_data_size; - if max_head_size == 0 { - log::warn!( - "max_head_data can't be zero for generating worst head data - using default." - ); - let mut cfg = configuration::ActiveConfig::::get(); - cfg.max_head_data_size = 1 * 1024 * 1024; - configuration::ActiveConfig::::put(&cfg); - } + assert!(max_head_size > 0, "max_head_data can't be zero for generating worst head data."); vec![0u8; max_head_size as usize].into() } #[cfg(any(feature = "runtime-benchmarks", test))] fn worst_validation_code() -> ValidationCode { let max_code_size = configuration::Pallet::::config().max_code_size; - - if max_code_size == 0 { - log::warn!( - "max_code_size can't be zero for generating worst validation code - using default." - ); - let mut cfg = configuration::ActiveConfig::::get(); - cfg.max_code_size = 1 * 1024 * 1024; - configuration::ActiveConfig::::put(&cfg); - } - + assert!(max_code_size > 0, "max_code_size can't be zero for generating worst code data."); let validation_code = vec![0u8; max_code_size as usize]; validation_code.into() } diff --git a/polkadot/runtime/parachains/src/configuration.rs b/polkadot/runtime/parachains/src/configuration.rs index 37031796d705..72927ca7d308 100644 --- a/polkadot/runtime/parachains/src/configuration.rs +++ b/polkadot/runtime/parachains/src/configuration.rs @@ -543,7 +543,7 @@ pub mod pallet { #[pallet::storage] #[pallet::whitelist_storage] #[pallet::getter(fn config)] - pub type ActiveConfig = + pub(crate) type ActiveConfig = StorageValue<_, HostConfiguration>, ValueQuery>; /// Pending configuration changes. diff --git a/substrate/bin/node/cli/src/command.rs b/substrate/bin/node/cli/src/command.rs index 31fbdc7b63ad..964517320286 100644 --- a/substrate/bin/node/cli/src/command.rs +++ b/substrate/bin/node/cli/src/command.rs @@ -107,7 +107,6 @@ pub fn run() -> Result<()> { ) } - HashingFor::::asdf; cmd.run::, sp_statement_store::runtime_api::HostFunctions>(config) }, BenchmarkCmd::Block(cmd) => { diff --git a/substrate/utils/frame/benchmarking-cli/freestanding-cli/src/main.rs b/substrate/utils/frame/benchmarking-cli/freestanding-cli/src/main.rs index 209da5dc7a68..8664b30bd59b 100644 --- a/substrate/utils/frame/benchmarking-cli/freestanding-cli/src/main.rs +++ b/substrate/utils/frame/benchmarking-cli/freestanding-cli/src/main.rs @@ -34,6 +34,6 @@ type HostFunctions = (); fn main() -> Result<()> { env_logger::init(); - log::info!("Benchmarking pallet CLI"); + PalletCmd::parse().run_with_maybe_spec::(None) } diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs index af842d0ca52d..8e33dd96cd03 100644 --- a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs +++ b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs @@ -574,10 +574,10 @@ impl PalletCmd { /// Produce a genesis storage and genesis changes. /// - /// It would be easier to only return only type, but there is no easy way to convert them. + /// It would be easier to only return one type, but there is no easy way to convert them. // TODO: Re-write `BenchmarkingState` to not be such a clusterfuck and only accept // `OverlayedChanges` instead of a mix between `OverlayedChanges` and `State`. But this can only - // be done once we deprecated and removed the legacy interface. + // be done once we deprecated and removed the legacy interface :( fn genesis_storage( &self, chain_spec: &Option>, From b165650f017ddc9457df1d5748c782d6080aab6a Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Wed, 28 Feb 2024 20:55:13 +0100 Subject: [PATCH 04/44] Cleanup Signed-off-by: Oliver Tale-Yazdi --- .../freestanding-cli/src/main.rs | 8 +++--- .../benchmarking-cli/src/pallet/command.rs | 28 +++++++++++-------- 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/substrate/utils/frame/benchmarking-cli/freestanding-cli/src/main.rs b/substrate/utils/frame/benchmarking-cli/freestanding-cli/src/main.rs index 8664b30bd59b..bbd161ee3eb6 100644 --- a/substrate/utils/frame/benchmarking-cli/freestanding-cli/src/main.rs +++ b/substrate/utils/frame/benchmarking-cli/freestanding-cli/src/main.rs @@ -17,10 +17,10 @@ //! Entry point for the free standing `benchmark pallet` command runner. //! -//! This runner has the advantage that the node does not need to be build with the `try-runtime` -//! feature or similar. It can be shipped independently and used by any chain - as long as that -//! chain's runtime is not making use of 3rd party host functions. In that case, it would need to be -//! forked (or extended with some plugin system). +//! This runner has the advantage that the node does not need to be build with the +//! `runtime-benchmarks` feature or similar. It can be shipped independently and used by any chain - +//! as long as that chain's runtime is not making use of 3rd party host functions. In that case, it +//! would need to be forked (or extended with some plugin system). use clap::Parser; use frame_benchmarking_cli::PalletCmd; diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs index 8e33dd96cd03..594e5ce51577 100644 --- a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs +++ b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs @@ -587,12 +587,10 @@ impl PalletCmd { (Some(GenesisBuilder::Spec), _) | (None, false) => { log::warn!("{WARN_SPEC_GENESIS_CTOR}"); let Some(chain_spec) = chain_spec else { - return Err( - "No chain spec specified although to generate the genesis state".into() - ); + return Err("No chain spec specified to generate the genesis state".into()); }; - let storage = chain_spec.build_storage().map_err(|e| format!("The runtime returned an error when trying to build the genesis storage. Please ensure that all pallets define a genesis config that can be built. For more info see: https://github.com/paritytech/polkadot-sdk/pull/3412\nError: {e}"))?; + let storage = chain_spec.build_storage().map_err(|e| format!("The runtime returned an error when trying to build the genesis storage. Please ensure that all pallets define a genesis config that can be built. For more info, see: https://github.com/paritytech/polkadot-sdk/pull/3412\nError: {e}"))?; (storage, Default::default()) }, @@ -601,7 +599,7 @@ impl PalletCmd { }) } - /// Generate the genesis changeset with the runtime API. + /// Generate the genesis changeset by the runtime API. fn genesis_from_runtime(&self) -> Result> { let state = BenchmarkingState::::new( Default::default(), @@ -636,13 +634,18 @@ impl PalletCmd { ) .execute() .map_err(|e| format!("Could not call GenesisBuilder Runtime API: {}", e))?; + let genesis_json: Vec = codec::Decode::decode(&mut &genesis_json[..]) .map_err(|e| format!("Failed to decode genesis config: {:?}", e))?; + // Sanity check that it is JSON before we plug it into the next runtime call. - assert!(serde_json::from_slice::(&genesis_json).is_ok()); + assert!( + serde_json::from_slice::(&genesis_json).is_ok(), + "The runtime returned invalid an invalid genesis JSON" + ); let mut changes = Default::default(); - let genesis_state: Vec = StateMachine::new( + let build_config_ret: Vec = StateMachine::new( &state, &mut changes, &executor, @@ -655,16 +658,17 @@ impl PalletCmd { .execute() .map_err(|e| format!("Could not call GenesisBuilder Runtime API: {}", e))?; - let _: () = codec::Decode::decode(&mut &genesis_state[..]) - .map_err(|e| format!("Failed to decode (): {:?}", e))?; + if build_config_ret != vec![0u8] { + log::warn!("GenesisBuilder::build_config should not return any data - ignoring"); + } Ok(changes) } - /// The runtime blob for this benchmark. + /// Get the runtime blob for this benchmark. /// - /// The runtime will either be loaded from the `:code` key or can be overwritten with the - /// `--runtime` arg. + /// The runtime will either be loaded from the `:code` key out of the chain spec, or from a file + /// when specified with `--runtime`. fn runtime_blob<'a, H: Hash>( &self, state: &'a BenchmarkingState, From e952e873dc52e299b0851fb46bec4eae7c57dca4 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Fri, 1 Mar 2024 16:14:16 +0100 Subject: [PATCH 05/44] Rococo and Westend work Signed-off-by: Oliver Tale-Yazdi --- .../runtime/parachains/src/configuration.rs | 10 +- .../parachains/src/hrmp/benchmarking.rs | 2 + .../freestanding-cli/Cargo.toml | 4 +- .../freestanding-cli/src/main.rs | 6 +- .../benchmarking-cli/src/pallet/command.rs | 331 +++++++++--------- .../frame/benchmarking-cli/src/pallet/mod.rs | 4 + 6 files changed, 187 insertions(+), 170 deletions(-) diff --git a/polkadot/runtime/parachains/src/configuration.rs b/polkadot/runtime/parachains/src/configuration.rs index 72927ca7d308..1134b316eeb5 100644 --- a/polkadot/runtime/parachains/src/configuration.rs +++ b/polkadot/runtime/parachains/src/configuration.rs @@ -297,16 +297,16 @@ impl> Default for HostConfiguration = config.hrmp_sender_deposit.unique_saturated_into(); let capacity = config.hrmp_channel_max_capacity; let message_size = config.hrmp_channel_max_message_size; + assert!(message_size > 0, "Invalid genesis for benchmarking"); + assert!(capacity > 0, "Invalid genesis for benchmarking"); let sender: ParaId = from.into(); let sender_origin: crate::Origin = from.into(); diff --git a/substrate/utils/frame/benchmarking-cli/freestanding-cli/Cargo.toml b/substrate/utils/frame/benchmarking-cli/freestanding-cli/Cargo.toml index de7aa26bf2a1..0674dc264e4b 100644 --- a/substrate/utils/frame/benchmarking-cli/freestanding-cli/Cargo.toml +++ b/substrate/utils/frame/benchmarking-cli/freestanding-cli/Cargo.toml @@ -19,11 +19,11 @@ sp-runtime = { path = "../../../../primitives/runtime" } clap = { version = "4.5.1", features = ["derive"] } frame-benchmarking-cli = { path = "../" } sc-cli = { path = "../../../../client/cli" } +env_logger = "0.11.2" +log.workspace = true # optional host functions sp-statement-store = { path = "../../../../primitives/statement-store", optional = true } -env_logger = "0.11.2" -log.workspace = true [features] default = [ "extended-host-functions" ] diff --git a/substrate/utils/frame/benchmarking-cli/freestanding-cli/src/main.rs b/substrate/utils/frame/benchmarking-cli/freestanding-cli/src/main.rs index bbd161ee3eb6..4d537b92896d 100644 --- a/substrate/utils/frame/benchmarking-cli/freestanding-cli/src/main.rs +++ b/substrate/utils/frame/benchmarking-cli/freestanding-cli/src/main.rs @@ -28,12 +28,12 @@ use sc_cli::Result; use sp_runtime::traits::BlakeTwo256; #[cfg(feature = "extended-host-functions")] -type HostFunctions = sp_statement_store::runtime_api::HostFunctions; +type ExtendedHostFunctions = sp_statement_store::runtime_api::HostFunctions; #[cfg(not(feature = "extended-host-functions"))] -type HostFunctions = (); +type ExtendedHostFunctions = (); fn main() -> Result<()> { env_logger::init(); - PalletCmd::parse().run_with_maybe_spec::(None) + PalletCmd::parse().run_with_maybe_spec::(None) } diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs index 594e5ce51577..25a61fbc4db2 100644 --- a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs +++ b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs @@ -34,7 +34,7 @@ use sp_core::{ testing::{TestOffchainExt, TestTransactionPoolExt}, OffchainDbExt, OffchainWorkerExt, TransactionPoolExt, }, - traits::{CallContext, ReadRuntimeVersionExt, WrappedRuntimeCode}, + traits::{CallContext, CodeExecutor, ReadRuntimeVersionExt, WrappedRuntimeCode}, }; use sp_externalities::Extensions; use sp_keystore::{testing::MemoryKeystore, KeystoreExt}; @@ -274,22 +274,20 @@ impl PalletCmd { extensions }; - let result = StateMachine::new( - state, - &mut changes, - &executor, - "Benchmark_benchmark_metadata", - &(self.extra).encode(), - &mut extensions(), - &runtime_code, - CallContext::Offchain, - ) - .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): (Vec, Vec) = + Self::exec_state_machine( + StateMachine::new( + state, + &mut changes, + &executor, + "Benchmark_benchmark_metadata", + &(self.extra).encode(), + &mut extensions(), + &runtime_code, + CallContext::Offchain, + ), + ERROR_METADATA_NOT_FOUND, + )?; // Use the benchmark list and the user input to determine the set of benchmarks to run. let mut benchmarks_to_run = Vec::new(); @@ -351,13 +349,14 @@ impl PalletCmd { // Maps (pallet, extrinsic) to its component ranges. let mut component_ranges = HashMap::<(Vec, Vec), Vec>::new(); let pov_modes = Self::parse_pov_modes(&benchmarks_to_run)?; + let mut failed = Vec::<(String, String)>::new(); 'outer: for (i, (pallet, extrinsic, components, _, pallet_name, extrinsic_name)) in benchmarks_to_run.clone().into_iter().enumerate() { log::info!( target: LOG_TARGET, - "[{: >3} %] Starting benchmark: {pallet_name}::{extrinsic_name}", + "[{: >3} % ] Starting benchmark: {pallet_name}::{extrinsic_name}", (i * 100) / benchmarks_to_run.len(), ); let all_components = if components.is_empty() { @@ -410,126 +409,126 @@ impl PalletCmd { // First we run a verification if !self.no_verify { let state = &state_without_tracking; - let result = match StateMachine::new( - state, - &mut changes, - &executor, - "Benchmark_dispatch_benchmark", - &( - &pallet, - &extrinsic, - &selected_components.clone(), - true, // run verification code - 1, // no need to do internal repeats - ) - .encode(), - &mut extensions(), - &runtime_code, - CallContext::Offchain, - ) - .execute() - { + // Dont use these results since verification code will add overhead. + let _batch: Vec = match Self::exec_state_machine::< + std::result::Result, String>, + _, + _, + >( + StateMachine::new( + state, + &mut changes, + &executor, + "Benchmark_dispatch_benchmark", + &( + &pallet, + &extrinsic, + &selected_components.clone(), + true, // run verification code + 1, // no need to do internal repeats + ) + .encode(), + &mut extensions(), + &runtime_code, + CallContext::Offchain, + ), + "dispatch a benchmark", + ) { Err(e) => { log::error!("Error executing and verifying runtime benchmark: {}", e); + failed.push((pallet_name.clone(), extrinsic_name.clone())); + continue 'outer + }, + Ok(Err(e)) => { + log::error!("Error executing and verifying runtime benchmark: {}", e); + failed.push((pallet_name.clone(), extrinsic_name.clone())); continue 'outer }, - Ok(r) => r, + Ok(Ok(b)) => b, }; - // Dont use these results since verification code will add overhead. - let _batch = - match , String> as Decode>::decode( - &mut &result[..], - ) { - Err(e) => { - log::error!("Failed to decode benchmark results: {:?}", e); - continue 'outer - }, - Ok(Err(e)) => { - log::error!( - "Benchmark {pallet_name}::{extrinsic_name} failed: {e}", - ); - continue 'outer - }, - Ok(Ok(b)) => b, - }; } // Do one loop of DB tracking. { + let mut changes = genesis_changes.clone(); let state = &state_with_tracking; - let result = match StateMachine::new( - state, // todo remove tracking - &mut changes, - &executor, - "Benchmark_dispatch_benchmark", - &( - &pallet.clone(), - &extrinsic.clone(), - &selected_components.clone(), - false, // dont run verification code for final values - self.repeat, - ) - .encode(), - &mut extensions(), - &runtime_code, - CallContext::Offchain, - ) - .execute() - { + let batch: Vec = match Self::exec_state_machine::< + std::result::Result, String>, + _, + _, + >( + StateMachine::new( + state, // todo remove tracking + &mut changes, + &executor, + "Benchmark_dispatch_benchmark", + &( + &pallet.clone(), + &extrinsic.clone(), + &selected_components.clone(), + false, // dont run verification code for final values + self.repeat, + ) + .encode(), + &mut extensions(), + &runtime_code, + CallContext::Offchain, + ), + "dispatch a benchmark", + ) { Err(e) => { log::error!("Error executing runtime benchmark: {}", e); + failed.push((pallet_name.clone(), extrinsic_name.clone())); continue 'outer }, - Ok(r) => r, + Ok(Err(e)) => { + log::error!("Benchmark {pallet_name}::{extrinsic_name} failed: {e}",); + failed.push((pallet_name.clone(), extrinsic_name.clone())); + continue 'outer + }, + Ok(Ok(b)) => b, }; - let batch = - match , String> as Decode>::decode( - &mut &result[..], - ) { - Err(e) => { - log::error!("Failed to decode benchmark results: {:?}", e); - continue 'outer - }, - Ok(Err(e)) => { - log::error!( - "Benchmark {pallet_name}::{extrinsic_name} failed: {e}", - ); - continue 'outer - }, - Ok(Ok(b)) => b, - }; - - batches_db.extend(batch); // FAIL-CI check if missing and error + batches_db.extend(batch); } // Finally run a bunch of loops to get extrinsic timing information. for r in 0..self.external_repeat { + let mut changes = genesis_changes.clone(); let state = &state_without_tracking; - let result = StateMachine::new( - state, // todo remove tracking - &mut changes, - &executor, - "Benchmark_dispatch_benchmark", - &( - &pallet.clone(), - &extrinsic.clone(), - &selected_components.clone(), - false, // dont run verification code for final values - self.repeat, - ) - .encode(), - &mut extensions(), - &runtime_code, - CallContext::Offchain, - ) - .execute() - // No continue here, any error should have been caught by the verification step. - .map_err(|e| format!("Error executing runtime benchmark: {}", e))?; - - let batch = - , String> as Decode>::decode( - &mut &result[..], - ) - .map_err(|e| format!("Failed to decode benchmark results: {:?}", e))??; + let batch = match Self::exec_state_machine::< + std::result::Result, String>, + _, + _, + >( + StateMachine::new( + state, // todo remove tracking + &mut changes, + &executor, + "Benchmark_dispatch_benchmark", + &( + &pallet.clone(), + &extrinsic.clone(), + &selected_components.clone(), + false, // dont run verification code for final values + self.repeat, + ) + .encode(), + &mut extensions(), + &runtime_code, + CallContext::Offchain, + ), + "dispatch a benchmark", + ) { + Err(e) => { + return Err(format!("Error executing runtime benchmark: {e}",).into()); + }, + Ok(Err(e)) => { + return Err(format!( + "Benchmark {pallet_name}::{extrinsic_name} failed: {e}", + ) + .into()); + }, + Ok(Ok(b)) => b, + }; batches.extend(batch); @@ -540,7 +539,7 @@ impl PalletCmd { log::info!( target: LOG_TARGET, - "[{: >3} %] Running benchmark: {pallet_name}::{extrinsic_name}({} args) {}/{} {}/{}", + "[{: >3} % ] Running benchmark: {pallet_name}::{extrinsic_name}({} args) {}/{} {}/{}", (i * 100) / benchmarks_to_run.len(), components.len(), s + 1, // s starts at 0. @@ -555,15 +554,15 @@ impl PalletCmd { } assert!(batches_db.len() == batches.len() / self.external_repeat as usize); - if let Some(failed) = benchmarks_to_run.len().checked_sub(batches.len()) { - log::error!( - target: LOG_TARGET, - "Some benchmarks failed, but the results where still written to disk." + + if !failed.is_empty() { + failed.sort(); + eprintln!( + "The following {} benchmarks failed:\n{}", + failed.len(), + failed.iter().map(|(p, e)| format!("- {p}::{e}")).collect::>().join("\n") ); - return Err(format!( - "There were {failed} benchmarks that either returned an error or panicked." - ) - .into()); + return Err("Benchmarks failed - see log".into()) } // Combine all of the benchmark results, so that benchmarks of the same pallet/function @@ -571,7 +570,7 @@ impl PalletCmd { let batches = combine_batches(batches, batches_db); self.output(&batches, &storage_info, &component_ranges, pov_modes) } - + // 5:36 /// Produce a genesis storage and genesis changes. /// /// It would be easier to only return one type, but there is no easy way to convert them. @@ -622,21 +621,19 @@ impl PalletCmd { let runtime_code = runtime.code()?; // TODO register extensions - let genesis_json: Vec = StateMachine::new( - &state, - &mut Default::default(), - &executor, - "GenesisBuilder_create_default_config", - &[], // no args for this call - &mut Extensions::default(), - &runtime_code, - CallContext::Offchain, - ) - .execute() - .map_err(|e| format!("Could not call GenesisBuilder Runtime API: {}", e))?; - - let genesis_json: Vec = codec::Decode::decode(&mut &genesis_json[..]) - .map_err(|e| format!("Failed to decode genesis config: {:?}", e))?; + let genesis_json: Vec = Self::exec_state_machine( + StateMachine::new( + &state, + &mut Default::default(), + &executor, + "GenesisBuilder_create_default_config", + &[], // no args for this call + &mut Extensions::default(), + &runtime_code, + CallContext::Offchain, + ), + "build the genesis spec", + )?; // Sanity check that it is JSON before we plug it into the next runtime call. assert!( @@ -645,26 +642,40 @@ impl PalletCmd { ); let mut changes = Default::default(); - let build_config_ret: Vec = StateMachine::new( - &state, - &mut changes, - &executor, - "GenesisBuilder_build_config", - &genesis_json.encode(), - &mut Extensions::default(), - &runtime_code, - CallContext::Offchain, - ) - .execute() - .map_err(|e| format!("Could not call GenesisBuilder Runtime API: {}", e))?; - - if build_config_ret != vec![0u8] { + let build_config_ret: Vec = Self::exec_state_machine( + StateMachine::new( + &state, + &mut changes, + &executor, + "GenesisBuilder_build_config", + &genesis_json.encode(), + &mut Extensions::default(), + &runtime_code, + CallContext::Offchain, + ), + "populate the genesis state", + )?; + + if !build_config_ret.is_empty() { log::warn!("GenesisBuilder::build_config should not return any data - ignoring"); } Ok(changes) } + /// Execute a state machine and decode its return value as `R`. + fn exec_state_machine( + mut machine: StateMachine, H, Exec>, + hint: &str, + ) -> Result { + let res = machine + .execute() + .map_err(|e| format!("Could not call runtime API to {hint}: {}", e))?; + let res = R::decode(&mut &res[..]) + .map_err(|e| format!("Failed to decode runtime API result: {:?}", e))?; + Ok(res) + } + /// Get the runtime blob for this benchmark. /// /// The runtime will either be loaded from the `:code` key out of the chain spec, or from a file @@ -675,7 +686,7 @@ impl PalletCmd { ) -> Result, H>> { match (&self.runtime, &self.shared_params.chain) { (Some(runtime), None) => { - log::info!("Loading WASM from file: {}", runtime.display()); + log::info!("Loading WASM from {}", runtime.display()); let code = fs::read(runtime)?; let hash = sp_core::blake2_256(&code).to_vec(); let wrapped_code = WrappedRuntimeCode(Cow::Owned(code)); @@ -706,7 +717,7 @@ impl PalletCmd { pov_modes: PovModesMap, ) -> Result<()> { // Jsonify the result and write it to a file or stdout if desired. - if !self.jsonify(&batches)? { + if !self.jsonify(&batches)? && !self.quiet { // Print the summary only if `jsonify` did not write to stdout. self.print_summary(&batches, &storage_info, pov_modes.clone()) } diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/mod.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/mod.rs index 81fb45fe8d12..7e903c369ef0 100644 --- a/substrate/utils/frame/benchmarking-cli/src/pallet/mod.rs +++ b/substrate/utils/frame/benchmarking-cli/src/pallet/mod.rs @@ -233,4 +233,8 @@ pub struct PalletCmd { /// This exists only to restore legacy behaviour. It should never actually be needed. #[arg(long)] pub unsafe_overwrite_results: bool, + + /// Do everything as usual but suppresses most prints. + #[arg(long)] + quiet: bool, } From be0d71f207c78f6bae47ac7f38f151771ad9a61d Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Fri, 1 Mar 2024 17:32:19 +0100 Subject: [PATCH 06/44] Fix overlay rollback Signed-off-by: Oliver Tale-Yazdi --- .../freestanding-cli/src/main.rs | 18 ++++++++++++++++-- .../benchmarking-cli/src/pallet/command.rs | 1 + 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/substrate/utils/frame/benchmarking-cli/freestanding-cli/src/main.rs b/substrate/utils/frame/benchmarking-cli/freestanding-cli/src/main.rs index 4d537b92896d..952fb0ed61e1 100644 --- a/substrate/utils/frame/benchmarking-cli/freestanding-cli/src/main.rs +++ b/substrate/utils/frame/benchmarking-cli/freestanding-cli/src/main.rs @@ -23,10 +23,16 @@ //! would need to be forked (or extended with some plugin system). use clap::Parser; -use frame_benchmarking_cli::PalletCmd; +use frame_benchmarking_cli::BenchmarkCmd; use sc_cli::Result; use sp_runtime::traits::BlakeTwo256; +#[derive(Parser, Debug)] +pub struct Command { + #[command(subcommand)] + sub: BenchmarkCmd, +} + #[cfg(feature = "extended-host-functions")] type ExtendedHostFunctions = sp_statement_store::runtime_api::HostFunctions; #[cfg(not(feature = "extended-host-functions"))] @@ -34,6 +40,14 @@ type ExtendedHostFunctions = (); fn main() -> Result<()> { env_logger::init(); + log::warn!( + "Experimental benchmark runner v{} - usage will change in the future.", + env!("CARGO_PKG_VERSION") + ); - PalletCmd::parse().run_with_maybe_spec::(None) + match Command::parse().sub { + BenchmarkCmd::Pallet(pallet) => + pallet.run_with_maybe_spec::(None), + _ => Err("Invalid subcommand. Only `pallet` is supported.".into()), + } } diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs index 25a61fbc4db2..949a97007430 100644 --- a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs +++ b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs @@ -408,6 +408,7 @@ impl PalletCmd { for (s, selected_components) in all_components.iter().enumerate() { // First we run a verification if !self.no_verify { + let mut changes = genesis_changes.clone(); let state = &state_without_tracking; // Dont use these results since verification code will add overhead. let _batch: Vec = match Self::exec_state_machine::< From ca3564609de5d509343c81ce11d1867ce2e6938b Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Mon, 4 Mar 2024 19:56:21 +0100 Subject: [PATCH 07/44] Refactor Signed-off-by: Oliver Tale-Yazdi --- Cargo.lock | 17 +---- Cargo.toml | 2 +- polkadot/runtime/common/src/auctions.rs | 7 +- polkadot/runtime/rococo/Cargo.toml | 1 + substrate/frame/babe/src/lib.rs | 2 +- substrate/frame/benchmarking/src/v1.rs | 14 ++-- .../election-provider-multi-phase/src/lib.rs | 2 +- substrate/frame/fast-unstake/src/lib.rs | 2 +- .../frame/support/procedural/src/benchmark.rs | 6 +- .../utils/frame/benchmarking-cli/Cargo.toml | 7 ++ .../utils/frame/benchmarking-cli/README.md | 64 ++++++++++++++-- .../freestanding-cli/Cargo.toml | 30 -------- .../main.rs => src/bin/freestanding-cli.rs} | 41 ++++++---- .../benchmarking-cli/src/pallet/command.rs | 75 ++++++++++--------- .../frame/benchmarking-cli/src/pallet/mod.rs | 4 + 15 files changed, 157 insertions(+), 117 deletions(-) delete mode 100644 substrate/utils/frame/benchmarking-cli/freestanding-cli/Cargo.toml rename substrate/utils/frame/benchmarking-cli/{freestanding-cli/src/main.rs => src/bin/freestanding-cli.rs} (50%) diff --git a/Cargo.lock b/Cargo.lock index d0fa6ec760a1..645e9bb619bf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5535,6 +5535,8 @@ dependencies = [ "chrono", "clap 4.5.1", "comfy-table", + "cumulus-primitives-proof-size-hostfunction", + "env_logger 0.11.2", "frame-benchmarking", "frame-support", "frame-system", @@ -5566,6 +5568,7 @@ dependencies = [ "sp-keystore", "sp-runtime", "sp-state-machine", + "sp-statement-store", "sp-storage 19.0.0", "sp-trie", "sp-wasm-interface 20.0.0", @@ -5573,19 +5576,6 @@ dependencies = [ "thousands", ] -[[package]] -name = "frame-benchmarking-freestanding-cli" -version = "0.1.0" -dependencies = [ - "clap 4.5.1", - "env_logger 0.11.2", - "frame-benchmarking-cli", - "log", - "sc-cli", - "sp-runtime", - "sp-statement-store", -] - [[package]] name = "frame-benchmarking-pallet-pov" version = "18.0.0" @@ -15173,6 +15163,7 @@ dependencies = [ "sp-inherents", "sp-io", "sp-keyring", + "sp-keystore", "sp-mmr-primitives", "sp-offchain", "sp-runtime", diff --git a/Cargo.toml b/Cargo.toml index 4f3e05c7c577..f903e5d82b16 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,7 +11,7 @@ members = [ "bridges/bin/runtime-common", "bridges/modules/grandpa", "bridges/modules/messages", - "substrate/utils/frame/benchmarking-cli/freestanding-cli", + #"substrate/utils/frame/benchmarking-cli/freestanding-cli", "bridges/modules/parachains", "bridges/modules/relayers", "bridges/modules/xcm-bridge-hub", diff --git a/polkadot/runtime/common/src/auctions.rs b/polkadot/runtime/common/src/auctions.rs index 46ab673a7a0c..3a9175d33186 100644 --- a/polkadot/runtime/common/src/auctions.rs +++ b/polkadot/runtime/common/src/auctions.rs @@ -1899,10 +1899,13 @@ mod benchmarking { // Trigger epoch change for new random number value: { + pallet_babe::EpochStart::::set((Zero::zero(), u32::MAX.into())); pallet_babe::Pallet::::on_initialize(duration + now + T::EndingPeriod::get()); let authorities = pallet_babe::Pallet::::authorities(); - let next_authorities = authorities.clone(); - pallet_babe::Pallet::::enact_epoch_change(authorities, next_authorities, None); + // Check for non empty authority set since it otherwise emits a No-OP warning. + if !authorities.is_empty() { + pallet_babe::Pallet::::enact_epoch_change(authorities.clone(), authorities, None); + } } }: { diff --git a/polkadot/runtime/rococo/Cargo.toml b/polkadot/runtime/rococo/Cargo.toml index 3dc59cc17281..1ce1ae94fbf3 100644 --- a/polkadot/runtime/rococo/Cargo.toml +++ b/polkadot/runtime/rococo/Cargo.toml @@ -110,6 +110,7 @@ tiny-keccak = { version = "2.0.2", features = ["keccak"] } keyring = { package = "sp-keyring", path = "../../../substrate/primitives/keyring" } remote-externalities = { package = "frame-remote-externalities", path = "../../../substrate/utils/frame/remote-externalities" } sp-trie = { path = "../../../substrate/primitives/trie" } +sp-keystore = { path = "../../../substrate/primitives/keystore" } separator = "0.4.1" serde_json = { workspace = true, default-features = true } sp-tracing = { path = "../../../substrate/primitives/tracing", default-features = false } diff --git a/substrate/frame/babe/src/lib.rs b/substrate/frame/babe/src/lib.rs index 5fb107dde3ba..686ba6ec2d63 100644 --- a/substrate/frame/babe/src/lib.rs +++ b/substrate/frame/babe/src/lib.rs @@ -283,7 +283,7 @@ pub mod pallet { /// entropy was fixed (i.e. it was known to chain observers). Since epochs are defined in /// slots, which may be skipped, the block numbers may not line up with the slot numbers. #[pallet::storage] - pub(super) type EpochStart = + pub type EpochStart = StorageValue<_, (BlockNumberFor, BlockNumberFor), ValueQuery>; /// How late the current block is compared to its parent. diff --git a/substrate/frame/benchmarking/src/v1.rs b/substrate/frame/benchmarking/src/v1.rs index 4ad8cc0edd46..99db3dbe52b1 100644 --- a/substrate/frame/benchmarking/src/v1.rs +++ b/substrate/frame/benchmarking/src/v1.rs @@ -487,7 +487,7 @@ macro_rules! benchmarks_iter { $postcode } - #[cfg(test)] + #[cfg(any(feature = "std", test))] $crate::impl_benchmark_test!( { $( $where_clause )* } { $( $instance: $instance_bound )? } @@ -1169,7 +1169,7 @@ macro_rules! impl_benchmark { } } - #[cfg(test)] + #[cfg(any(feature = "std", test))] impl, $instance: $instance_bound )? > Pallet where T: frame_system::Config, $( $where_clause )* @@ -1184,7 +1184,7 @@ macro_rules! impl_benchmark { /// 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<(), $crate::BenchmarkError> { + pub fn test_bench_by_name(name: &[u8]) -> Result<(), $crate::BenchmarkError> { let name = $crate::__private::str::from_utf8(name) .map_err(|_| -> $crate::BenchmarkError { "`name` is not a valid utf8 string!".into() })?; match name { @@ -1212,7 +1212,7 @@ macro_rules! impl_benchmark_test { $name:ident ) => { $crate::__private::paste::item! { - #[cfg(test)] + #[cfg(any(feature = "std", test))] impl, $instance: $instance_bound )? > Pallet where T: frame_system::Config, $( $where_clause )* @@ -1311,7 +1311,7 @@ macro_rules! impl_benchmark_test { /// It expands to the equivalent of: /// /// ```rust,ignore -/// #[cfg(test)] +/// #[cfg(any(feature = "std", test))] /// mod tests { /// use super::*; /// use crate::tests::{new_test_ext, Test}; @@ -1341,7 +1341,7 @@ macro_rules! impl_benchmark_test { /// It expands to the equivalent of: /// /// ```rust,ignore -/// #[cfg(test)] +/// #[cfg(any(feature = "std", test))] /// mod benchmarking { /// use super::*; /// use crate::tests::{new_test_ext, Test}; @@ -1665,7 +1665,7 @@ macro_rules! impl_test_function { @user: $(,)? ) => { - #[cfg(test)] + #[cfg(any(feature = "std", test))] mod benchmark_tests { use super::$bench_module; diff --git a/substrate/frame/election-provider-multi-phase/src/lib.rs b/substrate/frame/election-provider-multi-phase/src/lib.rs index 6bf4dfe4f1eb..e1216eedae64 100644 --- a/substrate/frame/election-provider-multi-phase/src/lib.rs +++ b/substrate/frame/election-provider-multi-phase/src/lib.rs @@ -933,7 +933,7 @@ pub mod pallet { .expect(error_message); // Store the newly received solution. - log!(info, "queued unsigned solution with score {:?}", ready.score); + log!(debug, "queued unsigned solution with score {:?}", ready.score); let ejected_a_solution = >::exists(); >::put(ready); Self::deposit_event(Event::SolutionStored { diff --git a/substrate/frame/fast-unstake/src/lib.rs b/substrate/frame/fast-unstake/src/lib.rs index 04a50543bcc9..8ba306201310 100644 --- a/substrate/frame/fast-unstake/src/lib.rs +++ b/substrate/frame/fast-unstake/src/lib.rs @@ -560,7 +560,7 @@ pub mod pallet { if !remaining.is_zero() { Self::halt("not enough balance to unreserve"); } else { - log!(info, "unstaked {:?}, outcome: {:?}", stash, result); + log!(debug, "unstaked {:?}, outcome: {:?}", stash, result); Self::deposit_event(Event::::Unstaked { stash, result }); } }; diff --git a/substrate/frame/support/procedural/src/benchmark.rs b/substrate/frame/support/procedural/src/benchmark.rs index 6ded82d91aa5..5f295d47bcea 100644 --- a/substrate/frame/support/procedural/src/benchmark.rs +++ b/substrate/frame/support/procedural/src/benchmark.rs @@ -654,7 +654,7 @@ pub fn benchmarks( } } - #[cfg(test)] + #[cfg(any(feature = "std", test))] impl<#type_impl_generics> Pallet<#type_use_generics> where T: #frame_system::Config, #where_clause { /// Test a particular benchmark by name. /// @@ -666,7 +666,7 @@ pub fn benchmarks( /// 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> { + pub fn test_bench_by_name(name: &[u8]) -> Result<(), #krate::BenchmarkError> { let name = #krate::__private::str::from_utf8(name) .map_err(|_| -> #krate::BenchmarkError { "`name` is not a valid utf8 string!".into() })?; match name { @@ -936,7 +936,7 @@ fn expand_benchmark( } } - #[cfg(test)] + #[cfg(any(feature = "std", test))] impl<#type_impl_generics> Pallet<#type_use_generics> where T: #frame_system::Config, #where_clause { #[allow(unused)] fn #test_ident() -> Result<(), #krate::BenchmarkError> { diff --git a/substrate/utils/frame/benchmarking-cli/Cargo.toml b/substrate/utils/frame/benchmarking-cli/Cargo.toml index a2db83052c54..fc74d23fb829 100644 --- a/substrate/utils/frame/benchmarking-cli/Cargo.toml +++ b/substrate/utils/frame/benchmarking-cli/Cargo.toml @@ -12,6 +12,10 @@ readme = "README.md" [lints] workspace = true +[[bin]] +name = "frame-benchmarking-cli" +path = "src/bin/freestanding-cli.rs" + [package.metadata.docs.rs] targets = ["x86_64-unknown-linux-gnu"] @@ -55,8 +59,11 @@ sp-state-machine = { path = "../../../primitives/state-machine" } sp-storage = { path = "../../../primitives/storage" } sp-trie = { path = "../../../primitives/trie" } sp-io = { path = "../../../primitives/io" } +sp-statement-store = { path = "../../../primitives/statement-store" } sp-wasm-interface = { path = "../../../primitives/wasm-interface" } gethostname = "0.2.3" +env_logger = "0.11.2" +cumulus-primitives-proof-size-hostfunction = { path = "../../../../cumulus/primitives/proof-size-hostfunction" } [features] default = ["rocksdb"] diff --git a/substrate/utils/frame/benchmarking-cli/README.md b/substrate/utils/frame/benchmarking-cli/README.md index 27673ea9580d..7627ce554269 100644 --- a/substrate/utils/frame/benchmarking-cli/README.md +++ b/substrate/utils/frame/benchmarking-cli/README.md @@ -1,13 +1,23 @@ -# The Benchmarking CLI +# The FRAME Benchmarking CLI -This crate contains commands to benchmark various aspects of Substrate and the hardware. -All commands are exposed by the Substrate node but can be exposed by any Substrate client. +This crate contains commands to benchmark various aspects of Substrate and the hardware. The goal is to have a comprehensive suite of benchmarks that cover all aspects of Substrate and the hardware that its -running on. +running on. +There exist fundamentally two ways to use this crate. A node-integrated CLI version, and a freestanding CLI. If you are +only interested in pallet benchmarking, then skip ahead to the [Freestanding CLI](#freestanding-cli). + +# Node Integrated CLI + +Mostly all Substrate nodes will expose some commands for benchmarking. You can refer to the `staging-node-cli` crate as +an example on how to integrate those. Note that for solely benchmarking pallets, the freestanding CLI is more suitable. + +## Usage + +Here we invoke the root command on the `staging-node-cli`. Most Substrate nodes should have a similar output, depending +on their integration of these commands. -Invoking the root benchmark command prints a help menu: ```sh -$ cargo run --profile=production -- benchmark +$ cargo run -p staging-node-cli --profile=production --features=runtime-benchmarks -- benchmark Sub-commands concerned with benchmarking. @@ -31,7 +41,47 @@ use `--release`. For the final results the `production` profile and reference hardware should be used, otherwise the results are not comparable. -The sub-commands are explained in depth here: +# Freestanding CLI + +The freestanding is a standalone CLI that does not rely on any node integration. It can be used to benchmark pallets of +any FRAME runtime that does not utilize 3rd party host functions. +It currently only supports pallet benchmarking, since the other commands still rely on a node. + +## Installation + +Can be installed via cargo from GitHub: +```sh +cargo install --locked --git https://github.com/paritytech/polkadot-sdk.git --bin frame-benchmarking-cli --profile=production +``` +or when you have the code locally checked out: + +```sh +cargo install --locked --path substrate/utils/frame/benchmarking-cli --profile=production +``` + +## Usage + +The exposed pallet sub-command is identical as the node-integrated CLI. The only difference is that it needs to be prefixed +with a `v1` to ensure drop-in compatibility. + +First we need to ensure that there is a runtime available. As example we will build the Westend runtime: + +```sh +cargo build -p westend-runtime --profile production --features runtime-benchmarks +``` + +Now the benchmarking can be started with: + +```sh +frame-benchmarking-cli v1 pallet --runtime target/release/wbuild/westend-runtime/westend-runtime.compact.compressed.wasm --pallet "pallet_balances" --extrinsic "" +``` + +For the exact arguments of the `pallet` command, please refer to the [pallet] sub-module. + +# Commands + +The sub-commands of both CLIs have the same semantics and are documented in their respective sub-modules: + - [block] Compare the weight of a historic block to its actual resource usage - [machine] Gauges the speed of the hardware - [overhead] Creates weight files for the *Block*- and *Extrinsic*-base weights diff --git a/substrate/utils/frame/benchmarking-cli/freestanding-cli/Cargo.toml b/substrate/utils/frame/benchmarking-cli/freestanding-cli/Cargo.toml deleted file mode 100644 index 0674dc264e4b..000000000000 --- a/substrate/utils/frame/benchmarking-cli/freestanding-cli/Cargo.toml +++ /dev/null @@ -1,30 +0,0 @@ -[package] -name = "frame-benchmarking-freestanding-cli" -version = "0.1.0" -authors.workspace = true -edition.workspace = true -license = "Apache-2.0" -homepage = "https://substrate.io" -repository.workspace = true -description = "Freestanding CLI for benchmarking FRAME pallets." - -[lints] -workspace = true - -[package.metadata.docs.rs] -targets = ["x86_64-unknown-linux-gnu"] - -[dependencies] -sp-runtime = { path = "../../../../primitives/runtime" } -clap = { version = "4.5.1", features = ["derive"] } -frame-benchmarking-cli = { path = "../" } -sc-cli = { path = "../../../../client/cli" } -env_logger = "0.11.2" -log.workspace = true - -# optional host functions -sp-statement-store = { path = "../../../../primitives/statement-store", optional = true } - -[features] -default = [ "extended-host-functions" ] -extended-host-functions = [ "dep:sp-statement-store" ] diff --git a/substrate/utils/frame/benchmarking-cli/freestanding-cli/src/main.rs b/substrate/utils/frame/benchmarking-cli/src/bin/freestanding-cli.rs similarity index 50% rename from substrate/utils/frame/benchmarking-cli/freestanding-cli/src/main.rs rename to substrate/utils/frame/benchmarking-cli/src/bin/freestanding-cli.rs index 952fb0ed61e1..df0971454b4d 100644 --- a/substrate/utils/frame/benchmarking-cli/freestanding-cli/src/main.rs +++ b/substrate/utils/frame/benchmarking-cli/src/bin/freestanding-cli.rs @@ -15,39 +15,50 @@ // See the License for the specific language governing permissions and // limitations under the License. -//! Entry point for the free standing `benchmark pallet` command runner. -//! -//! This runner has the advantage that the node does not need to be build with the -//! `runtime-benchmarks` feature or similar. It can be shipped independently and used by any chain - -//! as long as that chain's runtime is not making use of 3rd party host functions. In that case, it -//! would need to be forked (or extended with some plugin system). - use clap::Parser; use frame_benchmarking_cli::BenchmarkCmd; use sc_cli::Result; use sp_runtime::traits::BlakeTwo256; +/// Benchmark FRAME runtimes. #[derive(Parser, Debug)] +#[clap(author, version, about)] pub struct Command { + #[command(subcommand)] + sub: SubCommand, +} + +#[derive(Debug, clap::Subcommand)] +pub enum SubCommand { + /// Sub-commands concerned with benchmarking. + V1(V1Command), + // NOTE: Here we can add new commands in a forward-compatible way. For example when + // transforming the CLI from a monolithic design to a data driven pipeline, there could be + // commands like `measure`, `analyze` and `render`. +} + +/// A command that conforms to the legacy `benchmark` argument syntax. +#[derive(Parser, Debug)] +pub struct V1Command { #[command(subcommand)] sub: BenchmarkCmd, } -#[cfg(feature = "extended-host-functions")] -type ExtendedHostFunctions = sp_statement_store::runtime_api::HostFunctions; -#[cfg(not(feature = "extended-host-functions"))] -type ExtendedHostFunctions = (); +type HostFunctions = ( + sp_statement_store::runtime_api::HostFunctions, + cumulus_primitives_proof_size_hostfunction::storage_proof_size::HostFunctions, +); fn main() -> Result<()> { env_logger::init(); log::warn!( - "Experimental benchmark runner v{} - usage will change in the future.", + "FRAME benchmark runner v{} is not yet battle tested - use with care.", env!("CARGO_PKG_VERSION") ); match Command::parse().sub { - BenchmarkCmd::Pallet(pallet) => - pallet.run_with_maybe_spec::(None), - _ => Err("Invalid subcommand. Only `pallet` is supported.".into()), + SubCommand::V1(V1Command { sub: BenchmarkCmd::Pallet(pallet) }) => + pallet.run::(None), + _ => Err("Invalid subcommand. Only `v1 pallet` is supported.".into()), } } diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs index 949a97007430..73d0e18b59ee 100644 --- a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs +++ b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs @@ -27,7 +27,6 @@ use linked_hash_map::LinkedHashMap; use sc_cli::{execution_method_from_cli, ChainSpec, CliConfiguration, Result, SharedParams}; use sc_client_db::BenchmarkingState; use sc_executor::{HeapAllocStrategy, WasmExecutor, DEFAULT_HEAP_ALLOC_STRATEGY}; -use sc_service::Configuration; use serde::Serialize; use sp_core::{ offchain::{ @@ -40,6 +39,7 @@ use sp_externalities::Extensions; use sp_keystore::{testing::MemoryKeystore, KeystoreExt}; use sp_runtime::traits::Hash; use sp_state_machine::{OverlayedChanges, StateMachine}; +use sp_wasm_interface::HostFunctions; use std::{ borrow::Cow, collections::{BTreeMap, BTreeSet, HashMap}, @@ -154,22 +154,15 @@ This warning may become a hard error after December 2024."; impl PalletCmd { /// Runs the command and benchmarks a pallet. - pub fn run(&self, config: Configuration) -> Result<()> - where - Hasher: Hash, - ExtraHostFunctions: sp_wasm_interface::HostFunctions, - { - self.run_with_maybe_spec::(Some(config.chain_spec)) - } - - pub fn run_with_maybe_spec( + pub fn run( &self, chain_spec: Option>, ) -> Result<()> where Hasher: Hash, - ExtraHostFunctions: sp_wasm_interface::HostFunctions, + ExtraHostFunctions: HostFunctions, { + self.check_args()?; let _d = self.execution.as_ref().map(|exec| { // We print the error at the end, since there is often A LOT of output. sp_core::defer::DeferGuard::new(move || { @@ -180,24 +173,6 @@ impl PalletCmd { }) }); - if let Some(output_path) = &self.output { - if !output_path.is_dir() && output_path.file_name().is_none() { - return Err("Output file or path is invalid!".into()) - } - } - - if let Some(header_file) = &self.header { - if !header_file.is_file() { - return Err("Header file is invalid!".into()) - }; - } - - if let Some(handlebars_template_file) = &self.template { - if !handlebars_template_file.is_file() { - return Err("Handlebars template file is invalid!".into()) - }; - } - if let Some(json_input) = &self.json_input { let raw_data = match std::fs::read(json_input) { Ok(raw_data) => raw_data, @@ -219,7 +194,8 @@ impl PalletCmd { let extrinsic_split: Vec<&str> = extrinsic.split(',').collect(); let extrinsics: Vec<_> = extrinsic_split.iter().map(|x| x.trim().as_bytes()).collect(); - let (genesis_storage, genesis_changes) = self.genesis_storage::(&chain_spec)?; + let (genesis_storage, genesis_changes) = + self.genesis_storage::(&chain_spec)?; let mut changes = genesis_changes.clone(); let cache_size = Some(self.database_cache_size as usize); @@ -255,6 +231,7 @@ impl PalletCmd { ExtraHostFunctions, )>::builder() .with_execution_method(method) + .with_allow_missing_host_functions(self.allow_missing_host_functions) .with_onchain_heap_alloc_strategy(alloc_strategy) .with_offchain_heap_alloc_strategy(alloc_strategy) .with_max_runtime_instances(2) @@ -578,7 +555,7 @@ impl PalletCmd { // TODO: Re-write `BenchmarkingState` to not be such a clusterfuck and only accept // `OverlayedChanges` instead of a mix between `OverlayedChanges` and `State`. But this can only // be done once we deprecated and removed the legacy interface :( - fn genesis_storage( + fn genesis_storage( &self, chain_spec: &Option>, ) -> Result<(sp_storage::Storage, OverlayedChanges)> { @@ -595,12 +572,12 @@ impl PalletCmd { (storage, Default::default()) }, (Some(GenesisBuilder::Runtime), _) | (None, true) => - (Default::default(), self.genesis_from_runtime::()?), + (Default::default(), self.genesis_from_runtime::()?), }) } /// Generate the genesis changeset by the runtime API. - fn genesis_from_runtime(&self) -> Result> { + fn genesis_from_runtime(&self) -> Result> { let state = BenchmarkingState::::new( Default::default(), Some(self.database_cache_size as usize), @@ -612,10 +589,9 @@ impl PalletCmd { let executor = WasmExecutor::<( sp_io::SubstrateHostFunctions, frame_benchmarking::benchmarking::HostFunctions, - (), + F, )>::builder() - // TODO add extra host functions - .with_allow_missing_host_functions(true) + .with_allow_missing_host_functions(self.allow_missing_host_functions) .build(); let runtime = self.runtime_blob(&state)?; @@ -939,6 +915,33 @@ impl PalletCmd { } Ok(parsed) } + + /// Sanity check the CLI arguments. + fn check_args(&self) -> Result<()> { + if self.runtime.is_some() && self.shared_params.chain.is_some() { + unreachable!("Clap should not allow both `--runtime` and `--chain` to be specified") + } + + if let Some(output_path) = &self.output { + if !output_path.is_dir() && output_path.file_name().is_none() { + return Err("Output file or path is invalid!".into()) + } + } + + if let Some(header_file) = &self.header { + if !header_file.is_file() { + return Err("Header file is invalid!".into()) + }; + } + + if let Some(handlebars_template_file) = &self.template { + if !handlebars_template_file.is_file() { + return Err("Handlebars template file is invalid!".into()) + }; + } + + Ok(()) + } } impl CliConfiguration for PalletCmd { diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/mod.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/mod.rs index 7e903c369ef0..167753595859 100644 --- a/substrate/utils/frame/benchmarking-cli/src/pallet/mod.rs +++ b/substrate/utils/frame/benchmarking-cli/src/pallet/mod.rs @@ -171,6 +171,10 @@ pub struct PalletCmd { #[arg(long, conflicts_with = "chain")] pub runtime: Option, + /// Do not fail if there are unknown but also unused host functions in the runtime. + #[arg(long)] + pub allow_missing_host_functions: bool, + /// How to construct the genesis state. /// /// Uses `GenesisBuilder::Spec` by default and `GenesisConstructor::Runtime` if `runtime` is From 9e52ca164674896181afee916e0d5b5c65586145 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Mon, 4 Mar 2024 20:28:00 +0100 Subject: [PATCH 08/44] Revert old code Signed-off-by: Oliver Tale-Yazdi --- Cargo.lock | 1 - Cargo.toml | 1 - polkadot/runtime/rococo/Cargo.toml | 1 - substrate/frame/benchmarking/src/v1.rs | 14 +++++++------- .../frame/support/procedural/src/benchmark.rs | 6 +++--- substrate/utils/frame/benchmarking-cli/README.md | 10 ++++++---- 6 files changed, 16 insertions(+), 17 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 645e9bb619bf..0ef6a1de64ca 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -15163,7 +15163,6 @@ dependencies = [ "sp-inherents", "sp-io", "sp-keyring", - "sp-keystore", "sp-mmr-primitives", "sp-offchain", "sp-runtime", diff --git a/Cargo.toml b/Cargo.toml index f903e5d82b16..654367cc1e21 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,7 +11,6 @@ members = [ "bridges/bin/runtime-common", "bridges/modules/grandpa", "bridges/modules/messages", - #"substrate/utils/frame/benchmarking-cli/freestanding-cli", "bridges/modules/parachains", "bridges/modules/relayers", "bridges/modules/xcm-bridge-hub", diff --git a/polkadot/runtime/rococo/Cargo.toml b/polkadot/runtime/rococo/Cargo.toml index 1ce1ae94fbf3..3dc59cc17281 100644 --- a/polkadot/runtime/rococo/Cargo.toml +++ b/polkadot/runtime/rococo/Cargo.toml @@ -110,7 +110,6 @@ tiny-keccak = { version = "2.0.2", features = ["keccak"] } keyring = { package = "sp-keyring", path = "../../../substrate/primitives/keyring" } remote-externalities = { package = "frame-remote-externalities", path = "../../../substrate/utils/frame/remote-externalities" } sp-trie = { path = "../../../substrate/primitives/trie" } -sp-keystore = { path = "../../../substrate/primitives/keystore" } separator = "0.4.1" serde_json = { workspace = true, default-features = true } sp-tracing = { path = "../../../substrate/primitives/tracing", default-features = false } diff --git a/substrate/frame/benchmarking/src/v1.rs b/substrate/frame/benchmarking/src/v1.rs index 99db3dbe52b1..4ad8cc0edd46 100644 --- a/substrate/frame/benchmarking/src/v1.rs +++ b/substrate/frame/benchmarking/src/v1.rs @@ -487,7 +487,7 @@ macro_rules! benchmarks_iter { $postcode } - #[cfg(any(feature = "std", test))] + #[cfg(test)] $crate::impl_benchmark_test!( { $( $where_clause )* } { $( $instance: $instance_bound )? } @@ -1169,7 +1169,7 @@ macro_rules! impl_benchmark { } } - #[cfg(any(feature = "std", test))] + #[cfg(test)] impl, $instance: $instance_bound )? > Pallet where T: frame_system::Config, $( $where_clause )* @@ -1184,7 +1184,7 @@ macro_rules! impl_benchmark { /// by the `impl_benchmark_test_suite` macro. However, it is not an error if a pallet /// author chooses not to implement benchmarks. #[allow(unused)] - pub fn test_bench_by_name(name: &[u8]) -> Result<(), $crate::BenchmarkError> { + fn test_bench_by_name(name: &[u8]) -> Result<(), $crate::BenchmarkError> { let name = $crate::__private::str::from_utf8(name) .map_err(|_| -> $crate::BenchmarkError { "`name` is not a valid utf8 string!".into() })?; match name { @@ -1212,7 +1212,7 @@ macro_rules! impl_benchmark_test { $name:ident ) => { $crate::__private::paste::item! { - #[cfg(any(feature = "std", test))] + #[cfg(test)] impl, $instance: $instance_bound )? > Pallet where T: frame_system::Config, $( $where_clause )* @@ -1311,7 +1311,7 @@ macro_rules! impl_benchmark_test { /// It expands to the equivalent of: /// /// ```rust,ignore -/// #[cfg(any(feature = "std", test))] +/// #[cfg(test)] /// mod tests { /// use super::*; /// use crate::tests::{new_test_ext, Test}; @@ -1341,7 +1341,7 @@ macro_rules! impl_benchmark_test { /// It expands to the equivalent of: /// /// ```rust,ignore -/// #[cfg(any(feature = "std", test))] +/// #[cfg(test)] /// mod benchmarking { /// use super::*; /// use crate::tests::{new_test_ext, Test}; @@ -1665,7 +1665,7 @@ macro_rules! impl_test_function { @user: $(,)? ) => { - #[cfg(any(feature = "std", test))] + #[cfg(test)] mod benchmark_tests { use super::$bench_module; diff --git a/substrate/frame/support/procedural/src/benchmark.rs b/substrate/frame/support/procedural/src/benchmark.rs index 5f295d47bcea..6ded82d91aa5 100644 --- a/substrate/frame/support/procedural/src/benchmark.rs +++ b/substrate/frame/support/procedural/src/benchmark.rs @@ -654,7 +654,7 @@ pub fn benchmarks( } } - #[cfg(any(feature = "std", test))] + #[cfg(test)] impl<#type_impl_generics> Pallet<#type_use_generics> where T: #frame_system::Config, #where_clause { /// Test a particular benchmark by name. /// @@ -666,7 +666,7 @@ pub fn benchmarks( /// by the `impl_benchmark_test_suite` macro. However, it is not an error if a pallet /// author chooses not to implement benchmarks. #[allow(unused)] - pub fn test_bench_by_name(name: &[u8]) -> Result<(), #krate::BenchmarkError> { + fn test_bench_by_name(name: &[u8]) -> Result<(), #krate::BenchmarkError> { let name = #krate::__private::str::from_utf8(name) .map_err(|_| -> #krate::BenchmarkError { "`name` is not a valid utf8 string!".into() })?; match name { @@ -936,7 +936,7 @@ fn expand_benchmark( } } - #[cfg(any(feature = "std", test))] + #[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> { diff --git a/substrate/utils/frame/benchmarking-cli/README.md b/substrate/utils/frame/benchmarking-cli/README.md index 7627ce554269..43be68e0268c 100644 --- a/substrate/utils/frame/benchmarking-cli/README.md +++ b/substrate/utils/frame/benchmarking-cli/README.md @@ -49,14 +49,16 @@ It currently only supports pallet benchmarking, since the other commands still r ## Installation -Can be installed via cargo from GitHub: +Installing from local source repository is faster than waiting for a re-clone: + ```sh -cargo install --locked --git https://github.com/paritytech/polkadot-sdk.git --bin frame-benchmarking-cli --profile=production +cargo install --locked --path substrate/utils/frame/benchmarking-cli --profile=production ``` -or when you have the code locally checked out: + +Otherwise from the git repository: ```sh -cargo install --locked --path substrate/utils/frame/benchmarking-cli --profile=production +cargo install --locked --git https://github.com/paritytech/polkadot-sdk.git --bin frame-benchmarking-cli --profile=production ``` ## Usage From b797ea29ce40682ab70562269929ae53319f387e Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Tue, 5 Mar 2024 14:30:09 +0100 Subject: [PATCH 09/44] Clippy Signed-off-by: Oliver Tale-Yazdi --- .../parachain-template/node/src/command.rs | 2 +- cumulus/polkadot-parachain/src/command.rs | 2 +- polkadot/cli/src/command.rs | 6 ++-- .../bin/node-template/node/src/command.rs | 4 ++- substrate/bin/node/cli/src/command.rs | 2 +- .../utils/frame/benchmarking-cli/README.md | 4 ++- .../src/bin/freestanding-cli.rs | 2 +- .../benchmarking-cli/src/pallet/command.rs | 31 ++++++++++++++++--- 8 files changed, 40 insertions(+), 13 deletions(-) diff --git a/cumulus/parachain-template/node/src/command.rs b/cumulus/parachain-template/node/src/command.rs index 82624ae0be59..4c36fae30a26 100644 --- a/cumulus/parachain-template/node/src/command.rs +++ b/cumulus/parachain-template/node/src/command.rs @@ -184,7 +184,7 @@ pub fn run() -> Result<()> { match cmd { BenchmarkCmd::Pallet(cmd) => if cfg!(feature = "runtime-benchmarks") { - runner.sync_run(|config| cmd.run::, ReclaimHostFunctions>(config)) + runner.sync_run(|config| cmd.run_with_spec::, ReclaimHostFunctions>(Some(config.chain_spec))) } else { Err("Benchmarking wasn't enabled when building the node. \ You can enable it with `--features runtime-benchmarks`." diff --git a/cumulus/polkadot-parachain/src/command.rs b/cumulus/polkadot-parachain/src/command.rs index 4d44879af515..6f634dc518c1 100644 --- a/cumulus/polkadot-parachain/src/command.rs +++ b/cumulus/polkadot-parachain/src/command.rs @@ -585,7 +585,7 @@ pub fn run() -> Result<()> { match cmd { BenchmarkCmd::Pallet(cmd) => if cfg!(feature = "runtime-benchmarks") { - runner.sync_run(|config| cmd.run::, ReclaimHostFunctions>(config)) + runner.sync_run(|config| cmd.run_with_spec::, ReclaimHostFunctions>(Some(config.chain_spec))) } else { Err("Benchmarking wasn't enabled when building the node. \ You can enable it with `--features runtime-benchmarks`." diff --git a/polkadot/cli/src/command.rs b/polkadot/cli/src/command.rs index f71891ecde34..b163a2c1367d 100644 --- a/polkadot/cli/src/command.rs +++ b/polkadot/cli/src/command.rs @@ -451,8 +451,10 @@ pub fn run() -> Result<()> { if cfg!(feature = "runtime-benchmarks") { runner.sync_run(|config| { - cmd.run::, ()>(config) - .map_err(|e| Error::SubstrateCli(e)) + cmd.run_with_spec::, ()>( + Some(config.chain_spec), + ) + .map_err(|e| Error::SubstrateCli(e)) }) } else { Err(sc_cli::Error::Input( diff --git a/substrate/bin/node-template/node/src/command.rs b/substrate/bin/node-template/node/src/command.rs index 3778df664229..1cf497f2721c 100644 --- a/substrate/bin/node-template/node/src/command.rs +++ b/substrate/bin/node-template/node/src/command.rs @@ -117,7 +117,9 @@ pub fn run() -> sc_cli::Result<()> { ) } - cmd.run::, ()>(config) + cmd.run_with_spec::, ()>(Some( + config.chain_spec, + )) }, BenchmarkCmd::Block(cmd) => { let PartialComponents { client, .. } = service::new_partial(&config)?; diff --git a/substrate/bin/node/cli/src/command.rs b/substrate/bin/node/cli/src/command.rs index 964517320286..d37325c7187e 100644 --- a/substrate/bin/node/cli/src/command.rs +++ b/substrate/bin/node/cli/src/command.rs @@ -107,7 +107,7 @@ pub fn run() -> Result<()> { ) } - cmd.run::, sp_statement_store::runtime_api::HostFunctions>(config) + cmd.run_with_spec::, sp_statement_store::runtime_api::HostFunctions>(Some(config.chain_spec)) }, BenchmarkCmd::Block(cmd) => { // ensure that we keep the task manager alive diff --git a/substrate/utils/frame/benchmarking-cli/README.md b/substrate/utils/frame/benchmarking-cli/README.md index 43be68e0268c..89c93eee5382 100644 --- a/substrate/utils/frame/benchmarking-cli/README.md +++ b/substrate/utils/frame/benchmarking-cli/README.md @@ -75,7 +75,9 @@ cargo build -p westend-runtime --profile production --features runtime-benchmark Now the benchmarking can be started with: ```sh -frame-benchmarking-cli v1 pallet --runtime target/release/wbuild/westend-runtime/westend-runtime.compact.compressed.wasm --pallet "pallet_balances" --extrinsic "" +frame-benchmarking-cli v1 pallet \ + --runtime target/release/wbuild/westend-runtime/westend-runtime.compact.compressed.wasm \ + --pallet "pallet_balances" --extrinsic "" ``` For the exact arguments of the `pallet` command, please refer to the [pallet] sub-module. diff --git a/substrate/utils/frame/benchmarking-cli/src/bin/freestanding-cli.rs b/substrate/utils/frame/benchmarking-cli/src/bin/freestanding-cli.rs index df0971454b4d..0ec8f3b032f4 100644 --- a/substrate/utils/frame/benchmarking-cli/src/bin/freestanding-cli.rs +++ b/substrate/utils/frame/benchmarking-cli/src/bin/freestanding-cli.rs @@ -58,7 +58,7 @@ fn main() -> Result<()> { match Command::parse().sub { SubCommand::V1(V1Command { sub: BenchmarkCmd::Pallet(pallet) }) => - pallet.run::(None), + pallet.run_with_spec::(None), _ => Err("Invalid subcommand. Only `v1 pallet` is supported.".into()), } } diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs index 73d0e18b59ee..a902fa7814c9 100644 --- a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs +++ b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs @@ -148,13 +148,27 @@ This could mean that you either did not build the node correctly with the \ not created by a node that was compiled with the flag"; /// Warn when using the chain spec to generate the genesis state. -const WARN_SPEC_GENESIS_CTOR: &'static str = "Using the chain spec instead of the runtime to generate the genesis state is deprecated. -Please remove the `--chain`/`--dev`/`--local` argument, point `--runtime` to your runtime blob and set `--genesis-builder=runtime`. -This warning may become a hard error after December 2024."; +const WARN_SPEC_GENESIS_CTOR: &'static str = "Using the chain spec instead of the runtime to \ +generate the genesis state is deprecated. Please remove the `--chain`/`--dev`/`--local` argument, \ +point `--runtime` to your runtime blob and set `--genesis-builder=runtime`. This warning may \ +become a hard error any time after December 2024."; impl PalletCmd { /// Runs the command and benchmarks a pallet. - pub fn run( + #[deprecated( + note = "`run` will be removed after December 2024. Use `run_with_spec` instead or \ + completely remove the code and use the `frame-benchmarking-cli` instead." + )] + pub fn run(&self, config: sc_service::Configuration) -> Result<()> + where + Hasher: Hash, + ExtraHostFunctions: HostFunctions, + { + self.run_with_spec::(Some(config.chain_spec)) + } + + /// Runs the pallet benchmarking command. + pub fn run_with_spec( &self, chain_spec: Option>, ) -> Result<()> @@ -567,7 +581,14 @@ impl PalletCmd { return Err("No chain spec specified to generate the genesis state".into()); }; - let storage = chain_spec.build_storage().map_err(|e| format!("The runtime returned an error when trying to build the genesis storage. Please ensure that all pallets define a genesis config that can be built. For more info, see: https://github.com/paritytech/polkadot-sdk/pull/3412\nError: {e}"))?; + let storage = chain_spec.build_storage().map_err(|e| { + format!( + "The runtime returned \ + an error when trying to build the genesis storage. Please ensure that all pallets \ + define a genesis config that can be built. For more info, see: \ + https://github.com/paritytech/polkadot-sdk/pull/3412\nError: {e}" + ) + })?; (storage, Default::default()) }, From 9b00f0b050c12e8262d8b7d452f3a65c73521de1 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Fri, 15 Mar 2024 00:40:39 +0200 Subject: [PATCH 10/44] fixup Signed-off-by: Oliver Tale-Yazdi --- Cargo.lock | 83 ++++++------ Cargo.toml | 1 + .../utils/frame/benchmarking-cli/Cargo.toml | 3 - .../src/bin/freestanding-cli.rs | 64 --------- .../benchmarking-cli/src/pallet/command.rs | 74 +++++------ substrate/utils/frame/omni-bencher/Cargo.toml | 21 +++ .../utils/frame/omni-bencher/src/command.rs | 121 ++++++++++++++++++ .../utils/frame/omni-bencher/src/main.rs | 29 +++++ 8 files changed, 256 insertions(+), 140 deletions(-) delete mode 100644 substrate/utils/frame/benchmarking-cli/src/bin/freestanding-cli.rs create mode 100644 substrate/utils/frame/omni-bencher/Cargo.toml create mode 100644 substrate/utils/frame/omni-bencher/src/command.rs create mode 100644 substrate/utils/frame/omni-bencher/src/main.rs diff --git a/Cargo.lock b/Cargo.lock index 989bb4ce5ccf..25dd1481b8e0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2624,9 +2624,9 @@ dependencies = [ [[package]] name = "clap" -version = "4.5.1" +version = "4.5.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c918d541ef2913577a0f9566e9ce27cb35b6df072075769e0b26cb5a554520da" +checksum = "b230ab84b0ffdf890d5a10abdbc8b83ae1c4918275daea1ab8801f71536b2651" dependencies = [ "clap_builder", "clap_derive 4.5.0", @@ -2643,9 +2643,9 @@ dependencies = [ [[package]] name = "clap_builder" -version = "4.5.1" +version = "4.5.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9f3e7391dad68afb0c2ede1bf619f579a3dc9c2ec67f089baa397123a2f3d1eb" +checksum = "ae129e2e766ae0ec03484e609954119f123cc1fe650337e155d03b022f24f7b4" dependencies = [ "anstream", "anstyle", @@ -2660,7 +2660,7 @@ version = "4.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "586a385f7ef2f8b4d86bddaa0c094794e7ccbfe5ffef1f434fe928143fc783a5" dependencies = [ - "clap 4.5.1", + "clap 4.5.2", ] [[package]] @@ -3395,7 +3395,7 @@ dependencies = [ "anes", "cast", "ciborium", - "clap 4.5.1", + "clap 4.5.2", "criterion-plot", "futures", "is-terminal", @@ -3558,7 +3558,7 @@ dependencies = [ name = "cumulus-client-cli" version = "0.7.0" dependencies = [ - "clap 4.5.1", + "clap 4.5.2", "parity-scale-codec", "sc-chain-spec", "sc-cli", @@ -4323,7 +4323,7 @@ name = "cumulus-test-service" version = "0.1.0" dependencies = [ "async-trait", - "clap 4.5.1", + "clap 4.5.2", "criterion 0.5.1", "cumulus-client-cli", "cumulus-client-consensus-common", @@ -5575,10 +5575,8 @@ dependencies = [ "Inflector", "array-bytes 6.1.0", "chrono", - "clap 4.5.1", + "clap 4.5.2", "comfy-table", - "cumulus-primitives-proof-size-hostfunction", - "env_logger 0.11.2", "frame-benchmarking", "frame-support", "frame-system", @@ -5610,7 +5608,6 @@ dependencies = [ "sp-keystore", "sp-runtime", "sp-state-machine", - "sp-statement-store", "sp-storage 19.0.0", "sp-trie", "sp-wasm-interface 20.0.0", @@ -5670,7 +5667,7 @@ dependencies = [ name = "frame-election-solution-type-fuzzer" version = "2.0.0-alpha.5" dependencies = [ - "clap 4.5.1", + "clap 4.5.2", "frame-election-provider-solution-type", "frame-election-provider-support", "frame-support", @@ -8201,7 +8198,7 @@ checksum = "68354c5c6bd36d73ff3feceb05efa59b6acb7626617f4962be322a825e61f79a" name = "minimal-node" version = "4.0.0-dev" dependencies = [ - "clap 4.5.1", + "clap 4.5.2", "frame", "futures", "futures-timer", @@ -8670,7 +8667,7 @@ name = "node-bench" version = "0.9.0-dev" dependencies = [ "array-bytes 6.1.0", - "clap 4.5.1", + "clap 4.5.2", "derive_more", "fs_extra", "futures", @@ -8747,7 +8744,7 @@ dependencies = [ name = "node-runtime-generate-bags" version = "3.0.0" dependencies = [ - "clap 4.5.1", + "clap 4.5.2", "generate-bags", "kitchensink-runtime", ] @@ -8756,7 +8753,7 @@ dependencies = [ name = "node-template" version = "4.0.0-dev" dependencies = [ - "clap 4.5.1", + "clap 4.5.2", "frame-benchmarking", "frame-benchmarking-cli", "frame-system", @@ -8800,7 +8797,7 @@ dependencies = [ name = "node-template-release" version = "3.0.0" dependencies = [ - "clap 4.5.1", + "clap 4.5.2", "flate2", "fs_extra", "glob", @@ -11417,7 +11414,7 @@ dependencies = [ name = "parachain-template-node" version = "0.1.0" dependencies = [ - "clap 4.5.1", + "clap 4.5.2", "color-print", "cumulus-client-cli", "cumulus-client-collator", @@ -12349,7 +12346,7 @@ name = "polkadot-cli" version = "7.0.0" dependencies = [ "cfg-if", - "clap 4.5.1", + "clap 4.5.2", "frame-benchmarking-cli", "futures", "log", @@ -13163,6 +13160,20 @@ dependencies = [ "tracing-gum", ] +[[package]] +name = "polkadot-omni-bencher" +version = "0.1.0" +dependencies = [ + "clap 4.5.2", + "cumulus-primitives-proof-size-hostfunction", + "env_logger 0.11.2", + "frame-benchmarking-cli", + "log", + "sc-cli", + "sp-runtime", + "sp-statement-store", +] + [[package]] name = "polkadot-overseer" version = "7.0.0" @@ -13199,7 +13210,7 @@ dependencies = [ "async-trait", "bridge-hub-rococo-runtime", "bridge-hub-westend-runtime", - "clap 4.5.1", + "clap 4.5.2", "collectives-westend-runtime", "color-print", "contracts-rococo-runtime", @@ -13734,7 +13745,7 @@ dependencies = [ "async-trait", "bincode", "bitvec", - "clap 4.5.1", + "clap 4.5.2", "clap-num", "color-eyre", "colored", @@ -13828,7 +13839,7 @@ version = "1.0.0" dependencies = [ "assert_matches", "async-trait", - "clap 4.5.1", + "clap 4.5.2", "color-eyre", "futures", "futures-timer", @@ -13975,7 +13986,7 @@ dependencies = [ name = "polkadot-voter-bags" version = "7.0.0" dependencies = [ - "clap 4.5.1", + "clap 4.5.2", "generate-bags", "sp-io", "westend-runtime", @@ -14942,7 +14953,7 @@ checksum = "e898588f33fdd5b9420719948f9f2a32c922a246964576f71ba7f24f80610fbc" name = "remote-ext-tests-bags-list" version = "1.0.0" dependencies = [ - "clap 4.5.1", + "clap 4.5.2", "frame-system", "log", "pallet-bags-list-remote-tests", @@ -15809,7 +15820,7 @@ dependencies = [ "array-bytes 6.1.0", "bip39", "chrono", - "clap 4.5.1", + "clap 4.5.2", "fdlimit", "futures", "futures-timer", @@ -16967,7 +16978,7 @@ dependencies = [ name = "sc-storage-monitor" version = "0.16.0" dependencies = [ - "clap 4.5.1", + "clap 4.5.2", "fs4", "log", "sp-core", @@ -18994,7 +19005,7 @@ dependencies = [ name = "sp-npos-elections-fuzzer" version = "2.0.0-alpha.5" dependencies = [ - "clap 4.5.1", + "clap 4.5.2", "honggfuzz", "rand", "sp-npos-elections", @@ -19518,7 +19529,7 @@ checksum = "a8f112729512f8e442d81f95a8a7ddf2b7c6b8a1a6f509a95864142b30cab2d3" name = "staging-chain-spec-builder" version = "2.0.0" dependencies = [ - "clap 4.5.1", + "clap 4.5.2", "log", "sc-chain-spec", "serde_json", @@ -19531,7 +19542,7 @@ version = "3.0.0-dev" dependencies = [ "array-bytes 6.1.0", "assert_cmd", - "clap 4.5.1", + "clap 4.5.2", "clap_complete", "criterion 0.4.0", "frame-benchmarking", @@ -19641,7 +19652,7 @@ dependencies = [ name = "staging-node-inspect" version = "0.12.0" dependencies = [ - "clap 4.5.1", + "clap 4.5.2", "parity-scale-codec", "sc-cli", "sc-client-api", @@ -19846,7 +19857,7 @@ dependencies = [ name = "subkey" version = "9.0.0" dependencies = [ - "clap 4.5.1", + "clap 4.5.2", "sc-cli", ] @@ -19888,7 +19899,7 @@ dependencies = [ name = "substrate-frame-cli" version = "32.0.0" dependencies = [ - "clap 4.5.1", + "clap 4.5.2", "frame-support", "frame-system", "sc-cli", @@ -20369,7 +20380,7 @@ dependencies = [ name = "test-parachain-adder-collator" version = "1.0.0" dependencies = [ - "clap 4.5.1", + "clap 4.5.2", "futures", "futures-timer", "log", @@ -20417,7 +20428,7 @@ dependencies = [ name = "test-parachain-undying-collator" version = "1.0.0" dependencies = [ - "clap 4.5.1", + "clap 4.5.2", "futures", "futures-timer", "log", @@ -21112,7 +21123,7 @@ version = "0.38.0" dependencies = [ "assert_cmd", "async-trait", - "clap 4.5.1", + "clap 4.5.2", "frame-remote-externalities", "frame-try-runtime", "hex", diff --git a/Cargo.toml b/Cargo.toml index 48aa25f5c5a9..fb0e83373a10 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -496,6 +496,7 @@ members = [ "substrate/utils/frame/frame-utilities-cli", "substrate/utils/frame/generate-bags", "substrate/utils/frame/generate-bags/node-runtime", + "substrate/utils/frame/omni-bencher", "substrate/utils/frame/remote-externalities", "substrate/utils/frame/rpc/client", "substrate/utils/frame/rpc/state-trie-migration-rpc", diff --git a/substrate/utils/frame/benchmarking-cli/Cargo.toml b/substrate/utils/frame/benchmarking-cli/Cargo.toml index fc74d23fb829..c4316af43386 100644 --- a/substrate/utils/frame/benchmarking-cli/Cargo.toml +++ b/substrate/utils/frame/benchmarking-cli/Cargo.toml @@ -59,11 +59,8 @@ sp-state-machine = { path = "../../../primitives/state-machine" } sp-storage = { path = "../../../primitives/storage" } sp-trie = { path = "../../../primitives/trie" } sp-io = { path = "../../../primitives/io" } -sp-statement-store = { path = "../../../primitives/statement-store" } sp-wasm-interface = { path = "../../../primitives/wasm-interface" } gethostname = "0.2.3" -env_logger = "0.11.2" -cumulus-primitives-proof-size-hostfunction = { path = "../../../../cumulus/primitives/proof-size-hostfunction" } [features] default = ["rocksdb"] diff --git a/substrate/utils/frame/benchmarking-cli/src/bin/freestanding-cli.rs b/substrate/utils/frame/benchmarking-cli/src/bin/freestanding-cli.rs deleted file mode 100644 index 0ec8f3b032f4..000000000000 --- a/substrate/utils/frame/benchmarking-cli/src/bin/freestanding-cli.rs +++ /dev/null @@ -1,64 +0,0 @@ -// This file is part of Substrate. - -// Copyright (C) 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. - -use clap::Parser; -use frame_benchmarking_cli::BenchmarkCmd; -use sc_cli::Result; -use sp_runtime::traits::BlakeTwo256; - -/// Benchmark FRAME runtimes. -#[derive(Parser, Debug)] -#[clap(author, version, about)] -pub struct Command { - #[command(subcommand)] - sub: SubCommand, -} - -#[derive(Debug, clap::Subcommand)] -pub enum SubCommand { - /// Sub-commands concerned with benchmarking. - V1(V1Command), - // NOTE: Here we can add new commands in a forward-compatible way. For example when - // transforming the CLI from a monolithic design to a data driven pipeline, there could be - // commands like `measure`, `analyze` and `render`. -} - -/// A command that conforms to the legacy `benchmark` argument syntax. -#[derive(Parser, Debug)] -pub struct V1Command { - #[command(subcommand)] - sub: BenchmarkCmd, -} - -type HostFunctions = ( - sp_statement_store::runtime_api::HostFunctions, - cumulus_primitives_proof_size_hostfunction::storage_proof_size::HostFunctions, -); - -fn main() -> Result<()> { - env_logger::init(); - log::warn!( - "FRAME benchmark runner v{} is not yet battle tested - use with care.", - env!("CARGO_PKG_VERSION") - ); - - match Command::parse().sub { - SubCommand::V1(V1Command { sub: BenchmarkCmd::Pallet(pallet) }) => - pallet.run_with_spec::(None), - _ => Err("Invalid subcommand. Only `v1 pallet` is supported.".into()), - } -} diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs index a902fa7814c9..fa6dddfa3c82 100644 --- a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs +++ b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs @@ -147,6 +147,12 @@ This could mean that you either did not build the node correctly with the \ `--features runtime-benchmarks` flag, or the chain spec that you are using was \ not created by a node that was compiled with the flag"; +/// When the runtime could not build the genesis storage. +const ERROR_CANNOT_BUILD_GENESIS: &str = "The runtime returned \ +an error when trying to build the genesis storage. Please ensure that all pallets \ +define a genesis config that can be built. This can be tested with: \ +https://github.com/paritytech/polkadot-sdk/pull/3412"; + /// Warn when using the chain spec to generate the genesis state. const WARN_SPEC_GENESIS_CTOR: &'static str = "Using the chain spec instead of the runtime to \ generate the genesis state is deprecated. Please remove the `--chain`/`--dev`/`--local` argument, \ @@ -233,7 +239,6 @@ impl PalletCmd { let method = execution_method_from_cli(self.wasm_method, self.wasmtime_instantiation_strategy); - // Get Benchmark List let state = &state_without_tracking; let runtime = self.runtime_blob(&state_without_tracking)?; let runtime_code = runtime.code()?; @@ -252,19 +257,6 @@ impl PalletCmd { .with_runtime_cache_size(2) .build(); - let extensions = || -> Extensions { - let mut extensions = Extensions::default(); - let (offchain, _) = TestOffchainExt::new(); - let (pool, _) = TestTransactionPoolExt::new(); - let keystore = MemoryKeystore::new(); - extensions.register(KeystoreExt::new(keystore)); - extensions.register(OffchainWorkerExt::new(offchain.clone())); - extensions.register(OffchainDbExt::new(offchain)); - extensions.register(TransactionPoolExt::new(pool)); - extensions.register(ReadRuntimeVersionExt::new(executor.clone())); - extensions - }; - let (list, storage_info): (Vec, Vec) = Self::exec_state_machine( StateMachine::new( @@ -273,7 +265,7 @@ impl PalletCmd { &executor, "Benchmark_benchmark_metadata", &(self.extra).encode(), - &mut extensions(), + &mut Self::build_extensions(executor.clone()), &runtime_code, CallContext::Offchain, ), @@ -420,7 +412,7 @@ impl PalletCmd { 1, // no need to do internal repeats ) .encode(), - &mut extensions(), + &mut Self::build_extensions(executor.clone()), &runtime_code, CallContext::Offchain, ), @@ -461,7 +453,7 @@ impl PalletCmd { self.repeat, ) .encode(), - &mut extensions(), + &mut Self::build_extensions(executor.clone()), &runtime_code, CallContext::Offchain, ), @@ -504,7 +496,7 @@ impl PalletCmd { self.repeat, ) .encode(), - &mut extensions(), + &mut Self::build_extensions(executor.clone()), &runtime_code, CallContext::Offchain, ), @@ -581,14 +573,9 @@ impl PalletCmd { return Err("No chain spec specified to generate the genesis state".into()); }; - let storage = chain_spec.build_storage().map_err(|e| { - format!( - "The runtime returned \ - an error when trying to build the genesis storage. Please ensure that all pallets \ - define a genesis config that can be built. For more info, see: \ - https://github.com/paritytech/polkadot-sdk/pull/3412\nError: {e}" - ) - })?; + let storage = chain_spec + .build_storage() + .map_err(|e| format!("{ERROR_CANNOT_BUILD_GENESIS}\nError: {e}"))?; (storage, Default::default()) }, @@ -618,7 +605,6 @@ impl PalletCmd { let runtime = self.runtime_blob(&state)?; let runtime_code = runtime.code()?; - // TODO register extensions let genesis_json: Vec = Self::exec_state_machine( StateMachine::new( &state, @@ -633,11 +619,10 @@ impl PalletCmd { "build the genesis spec", )?; - // Sanity check that it is JSON before we plug it into the next runtime call. - assert!( - serde_json::from_slice::(&genesis_json).is_ok(), - "The runtime returned invalid an invalid genesis JSON" - ); + // Sanity check that it is JSON before we plug it into the next call. + if !serde_json::from_slice::(&genesis_json).is_ok() { + log::warn!("GenesisBuilder::create_default_config returned invalid JSON"); + } let mut changes = Default::default(); let build_config_ret: Vec = Self::exec_state_machine( @@ -655,7 +640,7 @@ impl PalletCmd { )?; if !build_config_ret.is_empty() { - log::warn!("GenesisBuilder::build_config should not return any data - ignoring"); + log::warn!("GenesisBuilder::build_config returned unexpected data"); } Ok(changes) @@ -674,9 +659,23 @@ impl PalletCmd { Ok(res) } - /// Get the runtime blob for this benchmark. + /// Build the extension that are available for pallet benchmarks. + fn build_extensions(exe: E) -> Extensions { + let mut extensions = Extensions::default(); + let (offchain, _) = TestOffchainExt::new(); + let (pool, _) = TestTransactionPoolExt::new(); + let keystore = MemoryKeystore::new(); + extensions.register(KeystoreExt::new(keystore)); + extensions.register(OffchainWorkerExt::new(offchain.clone())); + extensions.register(OffchainDbExt::new(offchain)); + extensions.register(TransactionPoolExt::new(pool)); + extensions.register(ReadRuntimeVersionExt::new(exe)); + extensions + } + + /// Load the runtime blob for this benchmark. /// - /// The runtime will either be loaded from the `:code` key out of the chain spec, or from a file + /// The blob will either be loaded from the `:code` key out of the chain spec, or from a file /// when specified with `--runtime`. fn runtime_blob<'a, H: Hash>( &self, @@ -701,6 +700,7 @@ impl PalletCmd { } } + /// Allocation strategy for pallet benchmarking. fn alloc_strategy(heap_pages: Option) -> HeapAllocStrategy { heap_pages.map_or(DEFAULT_HEAP_ALLOC_STRATEGY, |p| HeapAllocStrategy::Static { extra_pages: p as _, @@ -940,7 +940,7 @@ impl PalletCmd { /// Sanity check the CLI arguments. fn check_args(&self) -> Result<()> { if self.runtime.is_some() && self.shared_params.chain.is_some() { - unreachable!("Clap should not allow both `--runtime` and `--chain` to be specified") + unreachable!("Clap should not allow both `--runtime` and `--chain` to be provided.") } if let Some(output_path) = &self.output { @@ -973,7 +973,7 @@ impl CliConfiguration for PalletCmd { fn chain_id(&self, _is_dev: bool) -> Result { Ok(match self.shared_params.chain { Some(ref chain) => chain.clone(), - None => "dev".into(), + None => "dev".into(), // FAIL-CI }) } } diff --git a/substrate/utils/frame/omni-bencher/Cargo.toml b/substrate/utils/frame/omni-bencher/Cargo.toml new file mode 100644 index 000000000000..37dfb5f1be89 --- /dev/null +++ b/substrate/utils/frame/omni-bencher/Cargo.toml @@ -0,0 +1,21 @@ +[package] +name = "polkadot-omni-bencher" +version = "0.1.0" +description = "Freestanding benchmark runner for any Polkadot runtime." +authors.workspace = true +edition.workspace = true +repository.workspace = true +license.workspace = true + +[lints] +workspace = true + +[dependencies] +clap = { version = "4.5.2", features = ["derive"] } +cumulus-primitives-proof-size-hostfunction = { path = "../../../../cumulus/primitives/proof-size-hostfunction" } +frame-benchmarking-cli = { path = "../benchmarking-cli" } +sc-cli = { path = "../../../client/cli" } +sp-runtime = { path = "../../../primitives/runtime" } +sp-statement-store = { path = "../../../primitives/statement-store" } +env_logger = "0.11.2" +log = { workspace = true } diff --git a/substrate/utils/frame/omni-bencher/src/command.rs b/substrate/utils/frame/omni-bencher/src/command.rs new file mode 100644 index 000000000000..e2ad6501b0be --- /dev/null +++ b/substrate/utils/frame/omni-bencher/src/command.rs @@ -0,0 +1,121 @@ +// This file is part of Substrate. + +// Copyright (C) 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. + +use clap::Parser; +use frame_benchmarking_cli::BenchmarkCmd; +use sc_cli::Result; +use sp_runtime::traits::BlakeTwo256; + +/// # Polkadot Omni Benchmarking CLI +/// +/// The Polkadot Omni benchmarker allows to benchmark the extrinsics of any Polkadot runtime. It is +/// meant to replace the current manual integration of the `benchmark pallet` into every parachain +/// node. This reduces duplicate code and makes maintenance for builders easier. The CLI is +/// currently only able to benchmark extrinsics. In the future it is planned to extend this to some +/// other areas. +/// +/// General FRAME runtimes could also be used with this benchmarker, as long as they don't utilize +/// any host functions that are not part of the Polkadot host specification. +/// +/// ## Installation +/// +/// Directly via crates.io: +/// +/// ```sh +/// cargo install --locked polkadot-omni-bencher +/// ``` +/// +/// or when the sources are locally checked out: +/// +/// ```sh +/// cargo install --locked --path substrate/utils/frame/omni-bencher +/// ``` +/// +/// Check the installed version and print the docs: +/// +/// ```sh +/// polkadot-omni-bencher --help +/// ``` +/// +/// ## Usage +/// +/// First we need to ensure that there is a runtime available. As example we will build the Westend +/// runtime: +/// +/// ```sh +/// cargo build -p westend-runtime --profile production --features runtime-benchmarks +/// ``` +/// +/// Now as example we benchmark `pallet_balances`: +/// +/// ```sh +/// polkadot-omni-bencher v1 pallet \ +/// --runtime target/release/wbuild/westend-runtime/westend-runtime.compact.compressed.wasm \ +/// --pallet "pallet_balances" --extrinsic "" +/// ``` +/// +/// For the exact arguments of the `pallet` command, please refer to the [pallet] sub-module. +/// +/// ## Backwards Compatibility +/// +/// The exposed pallet sub-command is identical as the node-integrated CLI. The only difference is +/// that it needs to be prefixed with a `v1` to ensure drop-in compatibility. +#[derive(Parser, Debug)] +#[clap(author, version, about, verbatim_doc_comment)] +pub struct Command { + #[command(subcommand)] + sub: SubCommand, +} + +#[derive(Debug, clap::Subcommand)] +pub enum SubCommand { + /// Sub-commands concerned with benchmarking. + V1(V1Command), + // NOTE: Here we can add new commands in a forward-compatible way. For example when + // transforming the CLI from a monolithic design to a data driven pipeline, there could be + // commands like `measure`, `analyze` and `render`. +} + +/// A command that conforms to the legacy `benchmark` argument syntax. +#[derive(Parser, Debug)] +pub struct V1Command { + #[command(subcommand)] + sub: BenchmarkCmd, +} + +type HostFunctions = ( + sp_statement_store::runtime_api::HostFunctions, + cumulus_primitives_proof_size_hostfunction::storage_proof_size::HostFunctions, +); + +impl Command { + pub fn run(self) -> Result<()> { + match self.sub { + SubCommand::V1(V1Command { sub: BenchmarkCmd::Pallet(pallet) }) => { + if let Some(spec) = pallet.shared_params.chain { + return Err(format!( + "Chain specs are not supported. Please remove \ + `--chain={spec}` and use `--runtime=` instead" + ) + .into()) + } + pallet.run_with_spec::(None) + }, + _ => Err("Invalid subcommand. Only `v1 pallet` is supported.".into()), + } + } +} diff --git a/substrate/utils/frame/omni-bencher/src/main.rs b/substrate/utils/frame/omni-bencher/src/main.rs new file mode 100644 index 000000000000..138c66918dfd --- /dev/null +++ b/substrate/utils/frame/omni-bencher/src/main.rs @@ -0,0 +1,29 @@ +// This file is part of Substrate. + +// Copyright (C) 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. + +mod command; + +use clap::Parser; +use env_logger::Env; +use sc_cli::Result; + +fn main() -> Result<()> { + env_logger::Builder::from_env(Env::default().default_filter_or("info")).init(); + log::warn!("The Polkadot omni-bencher is not yet battle tested - double check the results.",); + + command::Command::parse().run() +} From 59cf796915238c953fc603af8dfeeb67439ab224 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Thu, 21 Mar 2024 14:52:44 +0200 Subject: [PATCH 11/44] Reset script to master version Signed-off-by: Oliver Tale-Yazdi --- substrate/scripts/run_all_benchmarks.sh | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/substrate/scripts/run_all_benchmarks.sh b/substrate/scripts/run_all_benchmarks.sh index fe5f89a5b56e..6dd7cede319f 100755 --- a/substrate/scripts/run_all_benchmarks.sh +++ b/substrate/scripts/run_all_benchmarks.sh @@ -107,19 +107,6 @@ for PALLET in "${PALLETS[@]}"; do FOLDER="$(echo "${PALLET#*_}" | tr '_' '-')"; WEIGHT_FILE="./frame/${FOLDER}/src/weights.rs" - - # Special handling of custom weight paths. - if [ "$PALLET" == "frame_system_extensions" ] || [ "$PALLET" == "frame-system-extensions" ] - then - WEIGHT_FILE="./frame/system/src/extensions/weights.rs" - elif [ "$PALLET" == "pallet_asset_conversion_tx_payment" ] || [ "$PALLET" == "pallet-asset-conversion-tx-payment" ] - then - WEIGHT_FILE="./frame/transaction-payment/asset-conversion-tx-payment/src/weights.rs" - elif [ "$PALLET" == "pallet_asset_tx_payment" ] || [ "$PALLET" == "pallet-asset-tx-payment" ] - then - WEIGHT_FILE="./frame/transaction-payment/asset-tx-payment/src/weights.rs" - fi - echo "[+] Benchmarking $PALLET with weight file $WEIGHT_FILE"; OUTPUT=$( From 44c037f39ee2d08a81a9169922c87587e51de0e6 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Thu, 21 Mar 2024 15:35:31 +0200 Subject: [PATCH 12/44] Cleanup Signed-off-by: Oliver Tale-Yazdi --- substrate/utils/frame/benchmarking-cli/Cargo.toml | 4 ---- 1 file changed, 4 deletions(-) diff --git a/substrate/utils/frame/benchmarking-cli/Cargo.toml b/substrate/utils/frame/benchmarking-cli/Cargo.toml index e9685040ee48..b7fbb24b1a09 100644 --- a/substrate/utils/frame/benchmarking-cli/Cargo.toml +++ b/substrate/utils/frame/benchmarking-cli/Cargo.toml @@ -12,10 +12,6 @@ readme = "README.md" [lints] workspace = true -[[bin]] -name = "frame-benchmarking-cli" -path = "src/bin/freestanding-cli.rs" - [package.metadata.docs.rs] targets = ["x86_64-unknown-linux-gnu"] From 8f66e7283284ec785bf05e44704bac7bfb12c2fe Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Thu, 21 Mar 2024 15:46:00 +0200 Subject: [PATCH 13/44] lockfile Signed-off-by: Oliver Tale-Yazdi --- Cargo.lock | 41 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 39 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bdbf6ddac268..ced05db79fd0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -275,9 +275,9 @@ dependencies = [ [[package]] name = "anstyle" -version = "1.0.2" +version = "1.0.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "15c4c2c83f81532e5845a733998b6971faca23490340a418e9b72a3ec9de12ea" +checksum = "8901269c6307e8d93993578286ac0edf7f195079ffff5ebdeea6a59ffb7e36bc" [[package]] name = "anstyle-parse" @@ -5004,6 +5004,16 @@ dependencies = [ "syn 2.0.53", ] +[[package]] +name = "env_filter" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a009aa4810eb158359dda09d0c87378e4bbb89b5a801f016885a4707ba24f7ea" +dependencies = [ + "log", + "regex", +] + [[package]] name = "env_logger" version = "0.8.4" @@ -5040,6 +5050,19 @@ dependencies = [ "termcolor", ] +[[package]] +name = "env_logger" +version = "0.11.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "38b35839ba51819680ba087cd351788c9a3c476841207e0b8cee0b04722343b9" +dependencies = [ + "anstream", + "anstyle", + "env_filter", + "humantime", + "log", +] + [[package]] name = "environmental" version = "1.1.4" @@ -13024,6 +13047,20 @@ dependencies = [ "tracing-gum", ] +[[package]] +name = "polkadot-omni-bencher" +version = "0.1.0" +dependencies = [ + "clap 4.5.3", + "cumulus-primitives-proof-size-hostfunction", + "env_logger 0.11.3", + "frame-benchmarking-cli", + "log", + "sc-cli", + "sp-runtime", + "sp-statement-store", +] + [[package]] name = "polkadot-overseer" version = "7.0.0" From 96cdb1d9b53660f132ec5a6bccd2cbf689fa54db Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Thu, 21 Mar 2024 18:35:15 +0200 Subject: [PATCH 14/44] WIP Signed-off-by: Oliver Tale-Yazdi --- substrate/frame/balances/src/tests/currency_tests.rs | 12 ++++++++++++ substrate/primitives/arithmetic/Cargo.toml | 2 +- substrate/utils/frame/omni-bencher/src/command.rs | 2 +- 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/substrate/frame/balances/src/tests/currency_tests.rs b/substrate/frame/balances/src/tests/currency_tests.rs index 46a4c4caefc3..0085647a5f63 100644 --- a/substrate/frame/balances/src/tests/currency_tests.rs +++ b/substrate/frame/balances/src/tests/currency_tests.rs @@ -17,6 +17,18 @@ //! Tests regarding the functionality of the `Currency` trait set implementations. +#[test] +fn asdf() { + ExtBuilder::default().existential_deposit(10).build().execute_with(|| { + let _ = Balances::deposit_creating(&1, 111); + + Balances::set_lock(*b"asdfasdf", &1, 9, WithdrawReasons::all()); + assert_ok!(Balances::reserve(&1, 8)); + + panic!("Acc: {:?}", System::account(1)); + }); +} + use super::*; use crate::{Event, NegativeImbalance}; use frame_support::{ diff --git a/substrate/primitives/arithmetic/Cargo.toml b/substrate/primitives/arithmetic/Cargo.toml index 64c1025b5858..120edd06a660 100644 --- a/substrate/primitives/arithmetic/Cargo.toml +++ b/substrate/primitives/arithmetic/Cargo.toml @@ -43,7 +43,7 @@ std = [ "scale-info/std", "serde/std", "sp-crypto-hashing/std", - "sp-std/std", + "sp-std/std", ] # Serde support without relying on std features. serde = ["dep:serde", "scale-info/serde"] diff --git a/substrate/utils/frame/omni-bencher/src/command.rs b/substrate/utils/frame/omni-bencher/src/command.rs index e2ad6501b0be..60dd1338eb3b 100644 --- a/substrate/utils/frame/omni-bencher/src/command.rs +++ b/substrate/utils/frame/omni-bencher/src/command.rs @@ -68,7 +68,7 @@ use sp_runtime::traits::BlakeTwo256; /// --pallet "pallet_balances" --extrinsic "" /// ``` /// -/// For the exact arguments of the `pallet` command, please refer to the [pallet] sub-module. +/// For the exact arguments of the `pallet` command, please refer to the `pallet` sub-module. /// /// ## Backwards Compatibility /// From 982ab5a0b0c5256ce1fa09f1f0f8f1a707d60184 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Thu, 21 Mar 2024 21:24:36 +0200 Subject: [PATCH 15/44] Add BenchmarkSetup to NIS pallet Signed-off-by: Oliver Tale-Yazdi --- .gitlab/pipeline/test.yml | 2 +- substrate/bin/node/runtime/src/lib.rs | 17 ++++++++++++++++- .../frame/balances/src/tests/currency_tests.rs | 12 ------------ substrate/frame/nis/src/benchmarking.rs | 10 ++++++++++ substrate/frame/nis/src/lib.rs | 6 ++++++ substrate/frame/nis/src/mock.rs | 4 ++++ 6 files changed, 37 insertions(+), 14 deletions(-) diff --git a/.gitlab/pipeline/test.yml b/.gitlab/pipeline/test.yml index d244316000aa..de7cf176d349 100644 --- a/.gitlab/pipeline/test.yml +++ b/.gitlab/pipeline/test.yml @@ -323,7 +323,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 --quiet -- benchmark pallet --execution wasm --wasm-execution compiled --chain dev --pallet "*" --extrinsic "*" --steps 2 --repeat 1 test-frame-examples-compile-to-wasm: # into one job diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index 0906ae4319b9..3181bbeadce4 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -1734,7 +1734,7 @@ impl pallet_nis::Config for Runtime { type Currency = Balances; type CurrencyBalance = Balance; type FundOrigin = frame_system::EnsureSigned; - type Counterpart = ItemOf, AccountId>; + type Counterpart = ItemOf, AccountId>; type CounterpartAmount = WithMaximumOf>; type Deficit = (); type IgnoredIssuance = (); @@ -1752,6 +1752,21 @@ impl pallet_nis::Config for Runtime { type RuntimeHoldReason = RuntimeHoldReason; } +#[cfg(feature = "runtime-benchmarks")] +impl pallet_nis::BenchmarkSetup for Runtime { + fn create_counterpart_asset() { + let owner = AccountId::from([0u8; 32]); + // this may or may not fail depending on if the chain spec or runtime genesis is used. + let _ = Assets::force_create( + RuntimeOrigin::root(), + 9u32.into(), + sp_runtime::MultiAddress::Id(owner), + true, + 1, + ); + } +} + parameter_types! { pub const CollectionDeposit: Balance = 100 * DOLLARS; pub const ItemDeposit: Balance = 1 * DOLLARS; diff --git a/substrate/frame/balances/src/tests/currency_tests.rs b/substrate/frame/balances/src/tests/currency_tests.rs index 0085647a5f63..46a4c4caefc3 100644 --- a/substrate/frame/balances/src/tests/currency_tests.rs +++ b/substrate/frame/balances/src/tests/currency_tests.rs @@ -17,18 +17,6 @@ //! Tests regarding the functionality of the `Currency` trait set implementations. -#[test] -fn asdf() { - ExtBuilder::default().existential_deposit(10).build().execute_with(|| { - let _ = Balances::deposit_creating(&1, 111); - - Balances::set_lock(*b"asdfasdf", &1, 9, WithdrawReasons::all()); - assert_ok!(Balances::reserve(&1, 8)); - - panic!("Acc: {:?}", System::account(1)); - }); -} - use super::*; use crate::{Event, NegativeImbalance}; use frame_support::{ diff --git a/substrate/frame/nis/src/benchmarking.rs b/substrate/frame/nis/src/benchmarking.rs index 0cc9e7421d0e..b9374ca66a7f 100644 --- a/substrate/frame/nis/src/benchmarking.rs +++ b/substrate/frame/nis/src/benchmarking.rs @@ -59,6 +59,11 @@ fn fill_queues() -> Result<(), DispatchError> { } benchmarks! { + where_clause { + where + T: crate::BenchmarkSetup, + } + place_bid { let l in 0..(T::MaxQueueLen::get() - 1); let caller: T::AccountId = whitelisted_caller(); @@ -106,6 +111,7 @@ benchmarks! { } fund_deficit { + T::create_counterpart_asset(); let origin = T::FundOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; let caller: T::AccountId = whitelisted_caller(); @@ -126,6 +132,7 @@ benchmarks! { } communify { + T::create_counterpart_asset(); let caller: T::AccountId = whitelisted_caller(); let bid = T::MinBid::get().max(One::one()) * 100u32.into(); let ed = T::Currency::minimum_balance(); @@ -139,6 +146,7 @@ benchmarks! { } privatize { + T::create_counterpart_asset(); let caller: T::AccountId = whitelisted_caller(); let bid = T::MinBid::get().max(One::one()); let ed = T::Currency::minimum_balance(); @@ -153,6 +161,7 @@ benchmarks! { } thaw_private { + T::create_counterpart_asset(); let whale: T::AccountId = account("whale", 0, SEED); let caller: T::AccountId = whitelisted_caller(); let bid = T::MinBid::get().max(One::one()); @@ -170,6 +179,7 @@ benchmarks! { } thaw_communal { + T::create_counterpart_asset(); let whale: T::AccountId = account("whale", 0, SEED); let caller: T::AccountId = whitelisted_caller(); let bid = T::MinBid::get().max(One::one()); diff --git a/substrate/frame/nis/src/lib.rs b/substrate/frame/nis/src/lib.rs index 5e547b63e547..85abd2e3abb9 100644 --- a/substrate/frame/nis/src/lib.rs +++ b/substrate/frame/nis/src/lib.rs @@ -155,6 +155,12 @@ impl Convert for NoCounterpart { } } +/// Setup the empty genesis state for benchmarking. +pub trait BenchmarkSetup { + /// Create the counterpart asset. Should panic on error. + fn create_counterpart_asset(); +} + #[frame_support::pallet] pub mod pallet { use super::{FunInspect, FunMutate}; diff --git a/substrate/frame/nis/src/mock.rs b/substrate/frame/nis/src/mock.rs index 33464db34c30..d435ef6c67c7 100644 --- a/substrate/frame/nis/src/mock.rs +++ b/substrate/frame/nis/src/mock.rs @@ -123,6 +123,10 @@ impl pallet_nis::Config for Test { type RuntimeHoldReason = RuntimeHoldReason; } +impl crate::BenchmarkSetup for Test { + fn create_counterpart_asset() {} +} + // This function basically just builds a genesis storage key/value store according to // our desired mockup. pub fn new_test_ext() -> sp_io::TestExternalities { From 10cdffd4dbd464e8cd6a5ede1d7e6a1eb347fc3c Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Thu, 21 Mar 2024 21:37:33 +0200 Subject: [PATCH 16/44] Remove FAIL-CI Signed-off-by: Oliver Tale-Yazdi --- substrate/utils/frame/benchmarking-cli/src/pallet/command.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs index fa6dddfa3c82..577f310b11bd 100644 --- a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs +++ b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs @@ -973,7 +973,7 @@ impl CliConfiguration for PalletCmd { fn chain_id(&self, _is_dev: bool) -> Result { Ok(match self.shared_params.chain { Some(ref chain) => chain.clone(), - None => "dev".into(), // FAIL-CI + None => "dev".into(), }) } } From 8cbd538cc5a268a95284c7269e582369d0646367 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Thu, 21 Mar 2024 21:46:31 +0200 Subject: [PATCH 17/44] CI: Add dedicated omni runner job Signed-off-by: Oliver Tale-Yazdi --- .gitlab/pipeline/test.yml | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/.gitlab/pipeline/test.yml b/.gitlab/pipeline/test.yml index de7cf176d349..948e9c823f98 100644 --- a/.gitlab/pipeline/test.yml +++ b/.gitlab/pipeline/test.yml @@ -323,7 +323,23 @@ 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 --quiet -- 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 --quiet -- benchmark pallet --chain dev --pallet "*" --extrinsic "*" --steps 2 --repeat 1 --quiet + +quick-benchmarks-omni: + stage: test + extends: + - .docker-env + - .common-refs + - .run-immediately + variables: + # Enable debug assertions since we are running optimized builds for testing + # but still want to have debug assertions. + RUSTFLAGS: "-C debug-assertions -D warnings" + RUST_BACKTRACE: "full" + WASM_BUILD_NO_COLOR: 1 + WASM_BUILD_RUSTFLAGS: "-C debug-assertions -D warnings" + script: + - time cargo run --locked --release -p polkadot-omni-bencher --quiet -- v1 pallet --runtime target/release/wbuild/kitchensink-runtime/kitchensink_runtime.compact.compressed.wasm --all --steps 2 --repeat 1 --quiet test-frame-examples-compile-to-wasm: # into one job From 71e98bf8697aef2cadcd27bf8105c40309a09595 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Thu, 21 Mar 2024 22:13:19 +0200 Subject: [PATCH 18/44] Fix Signed-off-by: Oliver Tale-Yazdi --- .gitlab/pipeline/check.yml | 4 ++-- polkadot/runtime/rococo/src/lib.rs | 2 ++ substrate/bin/node/runtime/src/lib.rs | 6 +++++- substrate/frame/nis/src/benchmarking.rs | 15 +++++---------- substrate/frame/nis/src/lib.rs | 10 ++++++++++ substrate/frame/nis/src/mock.rs | 6 ++---- 6 files changed, 26 insertions(+), 17 deletions(-) diff --git a/.gitlab/pipeline/check.yml b/.gitlab/pipeline/check.yml index 52da33550508..1b8542e6de29 100644 --- a/.gitlab/pipeline/check.yml +++ b/.gitlab/pipeline/check.yml @@ -7,8 +7,8 @@ cargo-clippy: variables: RUSTFLAGS: "-D warnings" script: - - SKIP_WASM_BUILD=1 cargo clippy --all-targets --locked --workspace - - SKIP_WASM_BUILD=1 cargo clippy --all-targets --all-features --locked --workspace + - SKIP_WASM_BUILD=1 cargo clippy --all-targets --locked --workspace --quiet + - SKIP_WASM_BUILD=1 cargo clippy --all-targets --all-features --locked --workspace --quiet check-try-runtime: stage: check diff --git a/polkadot/runtime/rococo/src/lib.rs b/polkadot/runtime/rococo/src/lib.rs index 90824a2f6f08..8ed5736624fd 100644 --- a/polkadot/runtime/rococo/src/lib.rs +++ b/polkadot/runtime/rococo/src/lib.rs @@ -1199,6 +1199,8 @@ impl pallet_nis::Config for Runtime { type MaxIntakeWeight = MaxIntakeWeight; type ThawThrottle = ThawThrottle; type RuntimeHoldReason = RuntimeHoldReason; + #[cfg(feature = "runtime-benchmarks")] + type BenchmarkSetup = (); } parameter_types! { diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index 3181bbeadce4..2f58b912374c 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -1750,10 +1750,14 @@ impl pallet_nis::Config for Runtime { type MaxIntakeWeight = MaxIntakeWeight; type ThawThrottle = ThawThrottle; type RuntimeHoldReason = RuntimeHoldReason; + #[cfg(feature = "runtime-benchmarks")] + type BenchmarkSetup = SetupAsset; } #[cfg(feature = "runtime-benchmarks")] -impl pallet_nis::BenchmarkSetup for Runtime { +pub struct SetupAsset; +#[cfg(feature = "runtime-benchmarks")] +impl pallet_nis::BenchmarkSetup for SetupAsset { fn create_counterpart_asset() { let owner = AccountId::from([0u8; 32]); // this may or may not fail depending on if the chain spec or runtime genesis is used. diff --git a/substrate/frame/nis/src/benchmarking.rs b/substrate/frame/nis/src/benchmarking.rs index b9374ca66a7f..f6a83b78d518 100644 --- a/substrate/frame/nis/src/benchmarking.rs +++ b/substrate/frame/nis/src/benchmarking.rs @@ -59,11 +59,6 @@ fn fill_queues() -> Result<(), DispatchError> { } benchmarks! { - where_clause { - where - T: crate::BenchmarkSetup, - } - place_bid { let l in 0..(T::MaxQueueLen::get() - 1); let caller: T::AccountId = whitelisted_caller(); @@ -111,7 +106,7 @@ benchmarks! { } fund_deficit { - T::create_counterpart_asset(); + T::BenchmarkSetup::create_counterpart_asset(); let origin = T::FundOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; let caller: T::AccountId = whitelisted_caller(); @@ -132,7 +127,7 @@ benchmarks! { } communify { - T::create_counterpart_asset(); + T::BenchmarkSetup::create_counterpart_asset(); let caller: T::AccountId = whitelisted_caller(); let bid = T::MinBid::get().max(One::one()) * 100u32.into(); let ed = T::Currency::minimum_balance(); @@ -146,7 +141,7 @@ benchmarks! { } privatize { - T::create_counterpart_asset(); + T::BenchmarkSetup::create_counterpart_asset(); let caller: T::AccountId = whitelisted_caller(); let bid = T::MinBid::get().max(One::one()); let ed = T::Currency::minimum_balance(); @@ -161,7 +156,7 @@ benchmarks! { } thaw_private { - T::create_counterpart_asset(); + T::BenchmarkSetup::create_counterpart_asset(); let whale: T::AccountId = account("whale", 0, SEED); let caller: T::AccountId = whitelisted_caller(); let bid = T::MinBid::get().max(One::one()); @@ -179,7 +174,7 @@ benchmarks! { } thaw_communal { - T::create_counterpart_asset(); + T::BenchmarkSetup::create_counterpart_asset(); let whale: T::AccountId = account("whale", 0, SEED); let caller: T::AccountId = whitelisted_caller(); let bid = T::MinBid::get().max(One::one()); diff --git a/substrate/frame/nis/src/lib.rs b/substrate/frame/nis/src/lib.rs index 85abd2e3abb9..50a3a023af63 100644 --- a/substrate/frame/nis/src/lib.rs +++ b/substrate/frame/nis/src/lib.rs @@ -158,9 +158,15 @@ impl Convert for NoCounterpart { /// Setup the empty genesis state for benchmarking. pub trait BenchmarkSetup { /// Create the counterpart asset. Should panic on error. + /// + /// This is called prior to assuming that a counterpart balance exists. fn create_counterpart_asset(); } +impl BenchmarkSetup for () { + fn create_counterpart_asset() {} +} + #[frame_support::pallet] pub mod pallet { use super::{FunInspect, FunMutate}; @@ -303,6 +309,10 @@ pub mod pallet { /// The maximum proportion which may be thawed and the period over which it is reset. #[pallet::constant] type ThawThrottle: Get<(Perquintill, BlockNumberFor)>; + + /// Setup the state for benchmarking. + #[cfg(feature = "runtime-benchmarks")] + type BenchmarkSetup: crate::BenchmarkSetup; } #[pallet::pallet] diff --git a/substrate/frame/nis/src/mock.rs b/substrate/frame/nis/src/mock.rs index d435ef6c67c7..f3320a306df7 100644 --- a/substrate/frame/nis/src/mock.rs +++ b/substrate/frame/nis/src/mock.rs @@ -121,10 +121,8 @@ impl pallet_nis::Config for Test { type MinReceipt = MinReceipt; type ThawThrottle = ThawThrottle; type RuntimeHoldReason = RuntimeHoldReason; -} - -impl crate::BenchmarkSetup for Test { - fn create_counterpart_asset() {} + #[cfg(feature = "runtime-benchmarks")] + type BenchmarkSetup = (); } // This function basically just builds a genesis storage key/value store according to From 4d1dfbc67fbf88fc8fc863155b89f9c5efd7626d Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Thu, 21 Mar 2024 22:21:18 +0200 Subject: [PATCH 19/44] CI: Build runtime Signed-off-by: Oliver Tale-Yazdi --- .gitlab/pipeline/test.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitlab/pipeline/test.yml b/.gitlab/pipeline/test.yml index 948e9c823f98..61289bb9cd7d 100644 --- a/.gitlab/pipeline/test.yml +++ b/.gitlab/pipeline/test.yml @@ -339,6 +339,7 @@ quick-benchmarks-omni: WASM_BUILD_NO_COLOR: 1 WASM_BUILD_RUSTFLAGS: "-C debug-assertions -D warnings" script: + - time cargo build --locked --quiet --release -p kitchensink-runtime --features runtime-benchmarks - time cargo run --locked --release -p polkadot-omni-bencher --quiet -- v1 pallet --runtime target/release/wbuild/kitchensink-runtime/kitchensink_runtime.compact.compressed.wasm --all --steps 2 --repeat 1 --quiet test-frame-examples-compile-to-wasm: From 5ee94256a207f0e545697625a2eca02388831d1c Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Thu, 21 Mar 2024 22:50:47 +0200 Subject: [PATCH 20/44] Keep --dev working Signed-off-by: Oliver Tale-Yazdi --- .../benchmarking-cli/src/pallet/command.rs | 28 ++++++++----------- .../frame/benchmarking-cli/src/pallet/mod.rs | 2 +- .../benchmarking-cli/src/pallet/types.rs | 1 - 3 files changed, 13 insertions(+), 18 deletions(-) diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs index 577f310b11bd..6ca233ff4c6f 100644 --- a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs +++ b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs @@ -681,22 +681,18 @@ impl PalletCmd { &self, state: &'a BenchmarkingState, ) -> Result, H>> { - match (&self.runtime, &self.shared_params.chain) { - (Some(runtime), None) => { - log::info!("Loading WASM from {}", runtime.display()); - let code = fs::read(runtime)?; - let hash = sp_core::blake2_256(&code).to_vec(); - let wrapped_code = WrappedRuntimeCode(Cow::Owned(code)); - - Ok(FetchedCode::FromFile { wrapped_code, heap_pages: self.heap_pages, hash }) - }, - (None, Some(_)) => { - log::info!("Loading WASM from genesis state"); - let state = sp_state_machine::backend::BackendRuntimeCode::new(state); - - Ok(FetchedCode::FromGenesis { state }) - }, - _ => Err("Exactly one of `--runtime` or `--chain` must be provided".into()), + if let Some(runtime) = &self.runtime { + log::info!("Loading WASM from {}", runtime.display()); + let code = fs::read(runtime)?; + let hash = sp_core::blake2_256(&code).to_vec(); + let wrapped_code = WrappedRuntimeCode(Cow::Owned(code)); + + Ok(FetchedCode::FromFile { wrapped_code, heap_pages: self.heap_pages, hash }) + } else { + log::info!("Loading WASM from genesis state"); + let state = sp_state_machine::backend::BackendRuntimeCode::new(state); + + Ok(FetchedCode::FromGenesis { state }) } } diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/mod.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/mod.rs index 167753595859..9a199582b969 100644 --- a/substrate/utils/frame/benchmarking-cli/src/pallet/mod.rs +++ b/substrate/utils/frame/benchmarking-cli/src/pallet/mod.rs @@ -238,7 +238,7 @@ pub struct PalletCmd { #[arg(long)] pub unsafe_overwrite_results: bool, - /// Do everything as usual but suppresses most prints. + /// Suppresses most prints. #[arg(long)] quiet: bool, } diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/types.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/types.rs index 80d2e527e839..66e70eae6538 100644 --- a/substrate/utils/frame/benchmarking-cli/src/pallet/types.rs +++ b/substrate/utils/frame/benchmarking-cli/src/pallet/types.rs @@ -34,7 +34,6 @@ pub enum GenesisBuilder { Runtime, /// Use the spec file to build the genesis state. This fails when there is no spec. Spec, - // NOTE: We could add from JSON file here, but for now its useless. } /// A runtime blob that was either fetched from genesis storage or loaded from a file. From d3762e864a872db5379260530f76d716983bc1c0 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Fri, 22 Mar 2024 18:53:28 +0200 Subject: [PATCH 21/44] Benchmarking default Signed-off-by: Oliver Tale-Yazdi --- .gitlab/pipeline/test.yml | 2 +- Cargo.lock | 1 - .../runtime/parachains/src/configuration.rs | 31 ++++++++++++++----- substrate/primitives/runtime/Cargo.toml | 2 +- .../utils/frame/benchmarking-cli/README.md | 2 +- substrate/utils/frame/omni-bencher/Cargo.toml | 2 +- .../utils/frame/omni-bencher/src/command.rs | 22 ++++++++++--- 7 files changed, 46 insertions(+), 16 deletions(-) diff --git a/.gitlab/pipeline/test.yml b/.gitlab/pipeline/test.yml index 61289bb9cd7d..3ee402f0ae0b 100644 --- a/.gitlab/pipeline/test.yml +++ b/.gitlab/pipeline/test.yml @@ -340,7 +340,7 @@ quick-benchmarks-omni: WASM_BUILD_RUSTFLAGS: "-C debug-assertions -D warnings" script: - time cargo build --locked --quiet --release -p kitchensink-runtime --features runtime-benchmarks - - time cargo run --locked --release -p polkadot-omni-bencher --quiet -- v1 pallet --runtime target/release/wbuild/kitchensink-runtime/kitchensink_runtime.compact.compressed.wasm --all --steps 2 --repeat 1 --quiet + - time cargo run --locked --release -p polkadot-omni-bencher --quiet -- v1 benchmark pallet --runtime target/release/wbuild/kitchensink-runtime/kitchensink_runtime.compact.compressed.wasm --all --steps 2 --repeat 1 --quiet test-frame-examples-compile-to-wasm: # into one job diff --git a/Cargo.lock b/Cargo.lock index ced05db79fd0..8824bbce27e0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -19008,7 +19008,6 @@ dependencies = [ "sp-std 14.0.0", "sp-tracing 16.0.0", "sp-weights", - "substrate-test-runtime-client", "zstd 0.12.4", ] diff --git a/polkadot/runtime/parachains/src/configuration.rs b/polkadot/runtime/parachains/src/configuration.rs index 47aacc7cc710..da434ac8a79a 100644 --- a/polkadot/runtime/parachains/src/configuration.rs +++ b/polkadot/runtime/parachains/src/configuration.rs @@ -243,7 +243,7 @@ impl> Default for HostConfiguration> Default for HostConfiguration> Default for HostConfiguration> HostConfiguration { + fn with_benchmarking_default(mut self) -> Self { + self.max_head_data_size = self.max_head_data_size.max(1 << 20); + self.max_downward_message_size = self.max_downward_message_size.max(1 << 16); + self.hrmp_channel_max_capacity = self.hrmp_channel_max_capacity.max(1000); + self.hrmp_channel_max_message_size = self.hrmp_channel_max_message_size.max(1 << 16); + self.hrmp_max_parachain_outbound_channels = + self.hrmp_max_parachain_outbound_channels.max(100); + self + } +} + /// Enumerates the possible inconsistencies of `HostConfiguration`. #[derive(Debug)] pub enum InconsistentError { @@ -541,8 +554,12 @@ pub mod pallet { #[pallet::genesis_build] impl BuildGenesisConfig for GenesisConfig { fn build(&self) { - self.config.panic_if_not_consistent(); - ActiveConfig::::put(&self.config); + let config = &self.config; + #[cfg(feature = "runtime-benchmarks")] + let config = config.clone().with_benchmarking_default(); + + config.panic_if_not_consistent(); + ActiveConfig::::put(&config); } } diff --git a/substrate/primitives/runtime/Cargo.toml b/substrate/primitives/runtime/Cargo.toml index cacfc0597229..3b087045ed7d 100644 --- a/substrate/primitives/runtime/Cargo.toml +++ b/substrate/primitives/runtime/Cargo.toml @@ -43,7 +43,7 @@ zstd = { version = "0.12.4", default-features = false } sp-api = { path = "../api" } sp-state-machine = { path = "../state-machine" } sp-tracing = { path = "../tracing" } -substrate-test-runtime-client = { path = "../../test-utils/runtime/client" } +#substrate-test-runtime-client = { path = "../../test-utils/runtime/client" } [features] runtime-benchmarks = [] diff --git a/substrate/utils/frame/benchmarking-cli/README.md b/substrate/utils/frame/benchmarking-cli/README.md index 89c93eee5382..9e8f204f0df2 100644 --- a/substrate/utils/frame/benchmarking-cli/README.md +++ b/substrate/utils/frame/benchmarking-cli/README.md @@ -75,7 +75,7 @@ cargo build -p westend-runtime --profile production --features runtime-benchmark Now the benchmarking can be started with: ```sh -frame-benchmarking-cli v1 pallet \ +frame-benchmarking-cli v1 benchmark pallet \ --runtime target/release/wbuild/westend-runtime/westend-runtime.compact.compressed.wasm \ --pallet "pallet_balances" --extrinsic "" ``` diff --git a/substrate/utils/frame/omni-bencher/Cargo.toml b/substrate/utils/frame/omni-bencher/Cargo.toml index 37dfb5f1be89..bbcf2f2dd768 100644 --- a/substrate/utils/frame/omni-bencher/Cargo.toml +++ b/substrate/utils/frame/omni-bencher/Cargo.toml @@ -13,7 +13,7 @@ workspace = true [dependencies] clap = { version = "4.5.2", features = ["derive"] } cumulus-primitives-proof-size-hostfunction = { path = "../../../../cumulus/primitives/proof-size-hostfunction" } -frame-benchmarking-cli = { path = "../benchmarking-cli" } +frame-benchmarking-cli = { path = "../benchmarking-cli", default-features = false } # Disable rockdb feature sc-cli = { path = "../../../client/cli" } sp-runtime = { path = "../../../primitives/runtime" } sp-statement-store = { path = "../../../primitives/statement-store" } diff --git a/substrate/utils/frame/omni-bencher/src/command.rs b/substrate/utils/frame/omni-bencher/src/command.rs index 60dd1338eb3b..d0ba5c6ff7b6 100644 --- a/substrate/utils/frame/omni-bencher/src/command.rs +++ b/substrate/utils/frame/omni-bencher/src/command.rs @@ -63,7 +63,7 @@ use sp_runtime::traits::BlakeTwo256; /// Now as example we benchmark `pallet_balances`: /// /// ```sh -/// polkadot-omni-bencher v1 pallet \ +/// polkadot-omni-bencher v1 benchmark pallet \ /// --runtime target/release/wbuild/westend-runtime/westend-runtime.compact.compressed.wasm \ /// --pallet "pallet_balances" --extrinsic "" /// ``` @@ -83,7 +83,7 @@ pub struct Command { #[derive(Debug, clap::Subcommand)] pub enum SubCommand { - /// Sub-commands concerned with benchmarking. + /// Compatibility syntax with the old benchmark runner. V1(V1Command), // NOTE: Here we can add new commands in a forward-compatible way. For example when // transforming the CLI from a monolithic design to a data driven pipeline, there could be @@ -93,6 +93,17 @@ pub enum SubCommand { /// A command that conforms to the legacy `benchmark` argument syntax. #[derive(Parser, Debug)] pub struct V1Command { + #[command(subcommand)] + sub: V1SubCommand, +} + +#[derive(Debug, clap::Subcommand)] +pub enum V1SubCommand { + Benchmark(V1BenchmarkCommand), +} + +#[derive(Parser, Debug)] +pub struct V1BenchmarkCommand { #[command(subcommand)] sub: BenchmarkCmd, } @@ -105,7 +116,10 @@ type HostFunctions = ( impl Command { pub fn run(self) -> Result<()> { match self.sub { - SubCommand::V1(V1Command { sub: BenchmarkCmd::Pallet(pallet) }) => { + SubCommand::V1(V1Command { + sub: + V1SubCommand::Benchmark(V1BenchmarkCommand { sub: BenchmarkCmd::Pallet(pallet) }), + }) => { if let Some(spec) = pallet.shared_params.chain { return Err(format!( "Chain specs are not supported. Please remove \ @@ -115,7 +129,7 @@ impl Command { } pallet.run_with_spec::(None) }, - _ => Err("Invalid subcommand. Only `v1 pallet` is supported.".into()), + _ => Err("Invalid subcommand. Only `v1 benchmark pallet` is supported.".into()), } } } From 746c9b8979892f6b4506690384a9d088271aed91 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Fri, 22 Mar 2024 19:25:17 +0200 Subject: [PATCH 22/44] what Signed-off-by: Oliver Tale-Yazdi --- Cargo.lock | 1 + substrate/primitives/runtime/Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 8824bbce27e0..ced05db79fd0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -19008,6 +19008,7 @@ dependencies = [ "sp-std 14.0.0", "sp-tracing 16.0.0", "sp-weights", + "substrate-test-runtime-client", "zstd 0.12.4", ] diff --git a/substrate/primitives/runtime/Cargo.toml b/substrate/primitives/runtime/Cargo.toml index 3b087045ed7d..cacfc0597229 100644 --- a/substrate/primitives/runtime/Cargo.toml +++ b/substrate/primitives/runtime/Cargo.toml @@ -43,7 +43,7 @@ zstd = { version = "0.12.4", default-features = false } sp-api = { path = "../api" } sp-state-machine = { path = "../state-machine" } sp-tracing = { path = "../tracing" } -#substrate-test-runtime-client = { path = "../../test-utils/runtime/client" } +substrate-test-runtime-client = { path = "../../test-utils/runtime/client" } [features] runtime-benchmarks = [] From 04133e6ba6424ef967161fa6af3e42d84a78aeb4 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Fri, 22 Mar 2024 20:40:45 +0200 Subject: [PATCH 23/44] Fix Signed-off-by: Oliver Tale-Yazdi --- .../runtime/parachains/src/configuration.rs | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/polkadot/runtime/parachains/src/configuration.rs b/polkadot/runtime/parachains/src/configuration.rs index da434ac8a79a..8f0beb551c05 100644 --- a/polkadot/runtime/parachains/src/configuration.rs +++ b/polkadot/runtime/parachains/src/configuration.rs @@ -232,7 +232,7 @@ pub struct HostConfiguration { impl> Default for HostConfiguration { fn default() -> Self { - Self { + let ret = Self { async_backing_params: AsyncBackingParams { max_candidate_depth: 0, allowed_ancestry_len: 0, @@ -260,7 +260,7 @@ impl> Default for HostConfiguration> Default for HostConfiguration> HostConfiguration { self.max_downward_message_size = self.max_downward_message_size.max(1 << 16); self.hrmp_channel_max_capacity = self.hrmp_channel_max_capacity.max(1000); self.hrmp_channel_max_message_size = self.hrmp_channel_max_message_size.max(1 << 16); + self.hrmp_max_parachain_inbound_channels = + self.hrmp_max_parachain_inbound_channels.max(100); self.hrmp_max_parachain_outbound_channels = self.hrmp_max_parachain_outbound_channels.max(100); self @@ -554,12 +560,8 @@ pub mod pallet { #[pallet::genesis_build] impl BuildGenesisConfig for GenesisConfig { fn build(&self) { - let config = &self.config; - #[cfg(feature = "runtime-benchmarks")] - let config = config.clone().with_benchmarking_default(); - - config.panic_if_not_consistent(); - ActiveConfig::::put(&config); + self.config.panic_if_not_consistent(); + ActiveConfig::::put(&self.config); } } From ccdb8ff2455e5dfb63dde77751830e4d8de10eeb Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Sat, 23 Mar 2024 14:05:26 +0200 Subject: [PATCH 24/44] Docs Signed-off-by: Oliver Tale-Yazdi --- substrate/utils/frame/benchmarking-cli/README.md | 13 ++++--------- substrate/utils/frame/omni-bencher/src/command.rs | 2 +- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/substrate/utils/frame/benchmarking-cli/README.md b/substrate/utils/frame/benchmarking-cli/README.md index 9e8f204f0df2..772da4f31fd7 100644 --- a/substrate/utils/frame/benchmarking-cli/README.md +++ b/substrate/utils/frame/benchmarking-cli/README.md @@ -49,16 +49,10 @@ It currently only supports pallet benchmarking, since the other commands still r ## Installation -Installing from local source repository is faster than waiting for a re-clone: +Installing from local source repository: ```sh -cargo install --locked --path substrate/utils/frame/benchmarking-cli --profile=production -``` - -Otherwise from the git repository: - -```sh -cargo install --locked --git https://github.com/paritytech/polkadot-sdk.git --bin frame-benchmarking-cli --profile=production +cargo install --locked --path substrate/utils/frame/omni-bencher --profile=production ``` ## Usage @@ -75,7 +69,8 @@ cargo build -p westend-runtime --profile production --features runtime-benchmark Now the benchmarking can be started with: ```sh -frame-benchmarking-cli v1 benchmark pallet \ +polkadot-omni-bencher v1 \ + benchmark pallet \ --runtime target/release/wbuild/westend-runtime/westend-runtime.compact.compressed.wasm \ --pallet "pallet_balances" --extrinsic "" ``` diff --git a/substrate/utils/frame/omni-bencher/src/command.rs b/substrate/utils/frame/omni-bencher/src/command.rs index d0ba5c6ff7b6..47204e33f724 100644 --- a/substrate/utils/frame/omni-bencher/src/command.rs +++ b/substrate/utils/frame/omni-bencher/src/command.rs @@ -42,7 +42,7 @@ use sp_runtime::traits::BlakeTwo256; /// or when the sources are locally checked out: /// /// ```sh -/// cargo install --locked --path substrate/utils/frame/omni-bencher +/// cargo install --locked --path substrate/utils/frame/omni-bencher --profile=production /// ``` /// /// Check the installed version and print the docs: From 59bddd141d1bc7c7fe62e9d7fe572f8df858beaf Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Sat, 23 Mar 2024 14:28:23 +0200 Subject: [PATCH 25/44] Cleanup shit code Signed-off-by: Oliver Tale-Yazdi --- .../benchmarking-cli/src/pallet/command.rs | 127 ++++++++++-------- 1 file changed, 73 insertions(+), 54 deletions(-) diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs index 6ca233ff4c6f..13e71fd68a20 100644 --- a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs +++ b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs @@ -207,13 +207,6 @@ impl PalletCmd { return self.output_from_results(&batches) } - let pallet = self.pallet.clone().unwrap_or_default(); - let pallet = pallet.as_bytes(); - - let extrinsic = self.extrinsic.clone().unwrap_or_default(); - let extrinsic_split: Vec<&str> = extrinsic.split(',').collect(); - let extrinsics: Vec<_> = extrinsic_split.iter().map(|x| x.trim().as_bytes()).collect(); - let (genesis_storage, genesis_changes) = self.genesis_storage::(&chain_spec)?; let mut changes = genesis_changes.clone(); @@ -273,52 +266,7 @@ impl PalletCmd { )?; // Use the benchmark list and the user input to determine the set of benchmarks to run. - let mut benchmarks_to_run = Vec::new(); - list.iter() - .filter(|item| pallet.is_empty() || pallet == &b"*"[..] || pallet == &item.pallet[..]) - .for_each(|item| { - for benchmark in &item.benchmarks { - let benchmark_name = &benchmark.name; - if extrinsic.is_empty() || - extrinsic.as_bytes() == &b"*"[..] || - extrinsics.contains(&&benchmark_name[..]) - { - benchmarks_to_run.push(( - item.pallet.clone(), - benchmark.name.clone(), - benchmark.components.clone(), - benchmark.pov_modes.clone(), - )) - } - } - }); - // Convert `Vec` to `String` for better readability. - let benchmarks_to_run: Vec<_> = benchmarks_to_run - .into_iter() - .map(|(pallet, extrinsic, components, pov_modes)| { - let pallet_name = - String::from_utf8(pallet.clone()).expect("Encoded from String; qed"); - let extrinsic_name = - String::from_utf8(extrinsic.clone()).expect("Encoded from String; qed"); - ( - pallet, - extrinsic, - components, - pov_modes - .into_iter() - .map(|(p, s)| { - (String::from_utf8(p).unwrap(), String::from_utf8(s).unwrap()) - }) - .collect(), - pallet_name, - extrinsic_name, - ) - }) - .collect(); - - if benchmarks_to_run.is_empty() { - return Err("No benchmarks found which match your input.".into()) - } + let benchmarks_to_run = self.select_benchmarks_to_run(list)?; if let Some(list_output) = self.list { list_benchmark(benchmarks_to_run, list_output, self.no_csv_header); @@ -554,7 +502,78 @@ impl PalletCmd { let batches = combine_batches(batches, batches_db); self.output(&batches, &storage_info, &component_ranges, pov_modes) } - // 5:36 + + fn select_benchmarks_to_run( + &self, + list: Vec, + ) -> Result< + Vec<( + Vec, + Vec, + Vec<(BenchmarkParameter, u32, u32)>, + Vec<(String, String)>, + String, + String, + )>, + > { + let pallet = self.pallet.clone().unwrap_or_default(); + let pallet = pallet.as_bytes(); + + let extrinsic = self.extrinsic.clone().unwrap_or_default(); + let extrinsic_split: Vec<&str> = extrinsic.split(',').collect(); + let extrinsics: Vec<_> = extrinsic_split.iter().map(|x| x.trim().as_bytes()).collect(); + + // Use the benchmark list and the user input to determine the set of benchmarks to run. + let mut benchmarks_to_run = Vec::new(); + list.iter() + .filter(|item| pallet.is_empty() || pallet == &b"*"[..] || pallet == &item.pallet[..]) + .for_each(|item| { + for benchmark in &item.benchmarks { + let benchmark_name = &benchmark.name; + if extrinsic.is_empty() || + extrinsic.as_bytes() == &b"*"[..] || + extrinsics.contains(&&benchmark_name[..]) + { + benchmarks_to_run.push(( + item.pallet.clone(), + benchmark.name.clone(), + benchmark.components.clone(), + benchmark.pov_modes.clone(), + )) + } + } + }); + // Convert `Vec` to `String` for better readability. + let benchmarks_to_run: Vec<_> = benchmarks_to_run + .into_iter() + .map(|(pallet, extrinsic, components, pov_modes)| { + let pallet_name = + String::from_utf8(pallet.clone()).expect("Encoded from String; qed"); + let extrinsic_name = + String::from_utf8(extrinsic.clone()).expect("Encoded from String; qed"); + ( + pallet, + extrinsic, + components, + pov_modes + .into_iter() + .map(|(p, s)| { + (String::from_utf8(p).unwrap(), String::from_utf8(s).unwrap()) + }) + .collect(), + pallet_name, + extrinsic_name, + ) + }) + .collect(); + + if benchmarks_to_run.is_empty() { + return Err("No benchmarks found which match your input.".into()) + } + + Ok(benchmarks_to_run) + } + /// Produce a genesis storage and genesis changes. /// /// It would be easier to only return one type, but there is no easy way to convert them. From 29f25838b722e392921bdea85354050f29b4bc0f Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Mon, 25 Mar 2024 19:23:19 +0200 Subject: [PATCH 26/44] Fixup Signed-off-by: Oliver Tale-Yazdi --- .../parachains/src/paras/benchmarking.rs | 12 +- substrate/frame/benchmarking/src/v1.rs | 14 +- .../benchmarking-cli/src/pallet/command.rs | 150 +++++++----------- .../benchmarking-cli/src/pallet/types.rs | 15 ++ .../benchmarking-cli/src/pallet/writer.rs | 26 +-- substrate/utils/frame/omni-bencher/Cargo.toml | 2 +- 6 files changed, 103 insertions(+), 116 deletions(-) diff --git a/polkadot/runtime/parachains/src/paras/benchmarking.rs b/polkadot/runtime/parachains/src/paras/benchmarking.rs index 554f0c15af24..7b03de6f79c7 100644 --- a/polkadot/runtime/parachains/src/paras/benchmarking.rs +++ b/polkadot/runtime/parachains/src/paras/benchmarking.rs @@ -82,7 +82,7 @@ fn generate_disordered_actions_queue() { benchmarks! { force_set_current_code { - let c in 1 .. MAX_CODE_SIZE; + let c in MIN_CODE_SIZE .. MAX_CODE_SIZE; let new_code = ValidationCode(vec![0; c as usize]); let para_id = ParaId::from(c as u32); CurrentCodeHash::::insert(¶_id, new_code.hash()); @@ -92,7 +92,7 @@ benchmarks! { assert_last_event::(Event::CurrentCodeUpdated(para_id).into()); } force_set_current_head { - let s in 1 .. MAX_HEAD_DATA_SIZE; + let s in MIN_CODE_SIZE .. MAX_HEAD_DATA_SIZE; let new_head = HeadData(vec![0; s as usize]); let para_id = ParaId::from(1000); }: _(RawOrigin::Root, para_id, new_head) @@ -104,7 +104,7 @@ benchmarks! { let context = BlockNumberFor::::from(1000u32); }: _(RawOrigin::Root, para_id, context) force_schedule_code_upgrade { - let c in 1 .. MAX_CODE_SIZE; + let c in MIN_CODE_SIZE .. MAX_CODE_SIZE; let new_code = ValidationCode(vec![0; c as usize]); let para_id = ParaId::from(c as u32); let block = BlockNumberFor::::from(c); @@ -114,7 +114,7 @@ benchmarks! { assert_last_event::(Event::CodeUpgradeScheduled(para_id).into()); } force_note_new_head { - let s in 1 .. MAX_HEAD_DATA_SIZE; + let s in MIN_CODE_SIZE .. MAX_HEAD_DATA_SIZE; let para_id = ParaId::from(1000); let new_head = HeadData(vec![0; s as usize]); let old_code_hash = ValidationCode(vec![0]).hash(); @@ -126,7 +126,7 @@ benchmarks! { generate_disordered_pruning::(); Pallet::::schedule_code_upgrade( para_id, - ValidationCode(vec![0]), + ValidationCode(vec![0u8; MIN_CODE_SIZE as usize]), expired, &config, SetGoAhead::Yes, @@ -145,7 +145,7 @@ benchmarks! { } add_trusted_validation_code { - let c in 1 .. MAX_CODE_SIZE; + let c in MIN_CODE_SIZE .. MAX_CODE_SIZE; let new_code = ValidationCode(vec![0; c as usize]); pvf_check::prepare_bypassing_bench::(new_code.clone()); diff --git a/substrate/frame/benchmarking/src/v1.rs b/substrate/frame/benchmarking/src/v1.rs index 4ad8cc0edd46..e96768633559 100644 --- a/substrate/frame/benchmarking/src/v1.rs +++ b/substrate/frame/benchmarking/src/v1.rs @@ -850,8 +850,8 @@ macro_rules! impl_bench_name_tests { if !($extra) { let disabled = $crate::__private::vec![ $( stringify!($names_extra).as_ref() ),* ]; if disabled.contains(&stringify!($name)) { - $crate::__private::log::error!( - "INFO: extra benchmark skipped - {}", + $crate::__private::log::debug!( + "extra benchmark skipped - {}", stringify!($name), ); return (); @@ -874,21 +874,21 @@ macro_rules! impl_bench_name_tests { $crate::BenchmarkError::Override(_) => { // This is still considered a success condition. $crate::__private::log::error!( - "WARNING: benchmark error overrided - {}", + "benchmark error overridden - {}", stringify!($name), ); }, $crate::BenchmarkError::Skip => { // This is considered a success condition. - $crate::__private::log::error!( - "WARNING: benchmark error skipped - {}", + $crate::__private::log::debug!( + "benchmark skipped - {}", stringify!($name), ); }, $crate::BenchmarkError::Weightless => { // This is considered a success condition. - $crate::__private::log::error!( - "WARNING: benchmark weightless skipped - {}", + $crate::__private::log::debug!( + "benchmark weightless skipped - {}", stringify!($name), ); } diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs index 13e71fd68a20..bbdcada534ab 100644 --- a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs +++ b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs @@ -15,7 +15,10 @@ // See the License for the specific language governing permissions and // limitations under the License. -use super::{writer, ListOutput, PalletCmd}; +use super::{ + types::{ComponentRange, ComponentRangeMap}, + writer, ListOutput, PalletCmd, +}; use crate::pallet::{types::FetchedCode, GenesisBuilder}; use codec::{Decode, Encode}; use frame_benchmarking::{ @@ -27,7 +30,6 @@ use linked_hash_map::LinkedHashMap; use sc_cli::{execution_method_from_cli, ChainSpec, CliConfiguration, Result, SharedParams}; use sc_client_db::BenchmarkingState; use sc_executor::{HeapAllocStrategy, WasmExecutor, DEFAULT_HEAP_ALLOC_STRATEGY}; -use serde::Serialize; use sp_core::{ offchain::{ testing::{TestOffchainExt, TestTransactionPoolExt}, @@ -52,17 +54,6 @@ use std::{ /// Logging target const LOG_TARGET: &'static str = "frame::benchmark::pallet"; -/// The inclusive range of a component. -#[derive(Serialize, Debug, Clone, Eq, PartialEq)] -pub(crate) struct ComponentRange { - /// Name of the component. - name: String, - /// Minimal valid value of the component. - min: u32, - /// Maximal valid value of the component. - max: u32, -} - /// How the PoV size of a storage item should be estimated. #[derive(clap::ValueEnum, Debug, Eq, PartialEq, Clone, Copy)] pub enum PovEstimationMode { @@ -89,7 +80,15 @@ impl FromStr for PovEstimationMode { /// Maps (pallet, benchmark) -> ((pallet, storage) -> PovEstimationMode) pub(crate) type PovModesMap = - HashMap<(Vec, Vec), HashMap<(String, String), PovEstimationMode>>; + HashMap<(String, String), HashMap<(String, String), PovEstimationMode>>; + +#[derive(Debug, Clone)] +struct SelectedBenchmark { + pallet: String, + extrinsic: String, + components: Vec<(BenchmarkParameter, u32, u32)>, + pov_modes: Vec<(String, String)>, +} // This takes multiple benchmark batches and combines all the results where the pallet, instance, // and benchmark are the same. @@ -278,16 +277,16 @@ impl PalletCmd { let mut batches_db = Vec::new(); let mut timer = time::SystemTime::now(); // Maps (pallet, extrinsic) to its component ranges. - let mut component_ranges = HashMap::<(Vec, Vec), Vec>::new(); + let mut component_ranges = HashMap::<(String, String), Vec>::new(); let pov_modes = Self::parse_pov_modes(&benchmarks_to_run)?; let mut failed = Vec::<(String, String)>::new(); - 'outer: for (i, (pallet, extrinsic, components, _, pallet_name, extrinsic_name)) in + 'outer: for (i, SelectedBenchmark { pallet, extrinsic, components, .. }) in benchmarks_to_run.clone().into_iter().enumerate() { log::info!( target: LOG_TARGET, - "[{: >3} % ] Starting benchmark: {pallet_name}::{extrinsic_name}", + "[{: >3} % ] Starting benchmark: {pallet}::{extrinsic}", (i * 100) / benchmarks_to_run.len(), ); let all_components = if components.is_empty() { @@ -353,8 +352,8 @@ impl PalletCmd { &executor, "Benchmark_dispatch_benchmark", &( - &pallet, - &extrinsic, + pallet.as_bytes(), + extrinsic.as_bytes(), &selected_components.clone(), true, // run verification code 1, // no need to do internal repeats @@ -368,12 +367,12 @@ impl PalletCmd { ) { Err(e) => { log::error!("Error executing and verifying runtime benchmark: {}", e); - failed.push((pallet_name.clone(), extrinsic_name.clone())); + failed.push((pallet.clone(), extrinsic.clone())); continue 'outer }, Ok(Err(e)) => { log::error!("Error executing and verifying runtime benchmark: {}", e); - failed.push((pallet_name.clone(), extrinsic_name.clone())); + failed.push((pallet.clone(), extrinsic.clone())); continue 'outer }, Ok(Ok(b)) => b, @@ -394,8 +393,8 @@ impl PalletCmd { &executor, "Benchmark_dispatch_benchmark", &( - &pallet.clone(), - &extrinsic.clone(), + pallet.as_bytes(), + extrinsic.as_bytes(), &selected_components.clone(), false, // dont run verification code for final values self.repeat, @@ -409,12 +408,12 @@ impl PalletCmd { ) { Err(e) => { log::error!("Error executing runtime benchmark: {}", e); - failed.push((pallet_name.clone(), extrinsic_name.clone())); + failed.push((pallet.clone(), extrinsic.clone())); continue 'outer }, Ok(Err(e)) => { - log::error!("Benchmark {pallet_name}::{extrinsic_name} failed: {e}",); - failed.push((pallet_name.clone(), extrinsic_name.clone())); + log::error!("Benchmark {pallet}::{extrinsic} failed: {e}",); + failed.push((pallet.clone(), extrinsic.clone())); continue 'outer }, Ok(Ok(b)) => b, @@ -437,8 +436,8 @@ impl PalletCmd { &executor, "Benchmark_dispatch_benchmark", &( - &pallet.clone(), - &extrinsic.clone(), + pallet.as_bytes(), + extrinsic.as_bytes(), &selected_components.clone(), false, // dont run verification code for final values self.repeat, @@ -454,10 +453,9 @@ impl PalletCmd { return Err(format!("Error executing runtime benchmark: {e}",).into()); }, Ok(Err(e)) => { - return Err(format!( - "Benchmark {pallet_name}::{extrinsic_name} failed: {e}", - ) - .into()); + return Err( + format!("Benchmark {pallet}::{extrinsic} failed: {e}",).into() + ); }, Ok(Ok(b)) => b, }; @@ -471,7 +469,7 @@ impl PalletCmd { log::info!( target: LOG_TARGET, - "[{: >3} % ] Running benchmark: {pallet_name}::{extrinsic_name}({} args) {}/{} {}/{}", + "[{: >3} % ] Running benchmark: {pallet}::{extrinsic}({} args) {}/{} {}/{}", (i * 100) / benchmarks_to_run.len(), components.len(), s + 1, // s starts at 0. @@ -494,7 +492,7 @@ impl PalletCmd { failed.len(), failed.iter().map(|(p, e)| format!("- {p}::{e}")).collect::>().join("\n") ); - return Err("Benchmarks failed - see log".into()) + return Err(format!("{} benchmarks failed", failed.len()).into()) } // Combine all of the benchmark results, so that benchmarks of the same pallet/function @@ -503,19 +501,7 @@ impl PalletCmd { self.output(&batches, &storage_info, &component_ranges, pov_modes) } - fn select_benchmarks_to_run( - &self, - list: Vec, - ) -> Result< - Vec<( - Vec, - Vec, - Vec<(BenchmarkParameter, u32, u32)>, - Vec<(String, String)>, - String, - String, - )>, - > { + fn select_benchmarks_to_run(&self, list: Vec) -> Result> { let pallet = self.pallet.clone().unwrap_or_default(); let pallet = pallet.as_bytes(); @@ -547,23 +533,21 @@ impl PalletCmd { let benchmarks_to_run: Vec<_> = benchmarks_to_run .into_iter() .map(|(pallet, extrinsic, components, pov_modes)| { - let pallet_name = - String::from_utf8(pallet.clone()).expect("Encoded from String; qed"); - let extrinsic_name = + let pallet = String::from_utf8(pallet.clone()).expect("Encoded from String; qed"); + let extrinsic = String::from_utf8(extrinsic.clone()).expect("Encoded from String; qed"); - ( + + SelectedBenchmark { pallet, extrinsic, components, - pov_modes + pov_modes: pov_modes .into_iter() .map(|(p, s)| { (String::from_utf8(p).unwrap(), String::from_utf8(s).unwrap()) }) .collect(), - pallet_name, - extrinsic_name, - ) + } }) .collect(); @@ -726,7 +710,7 @@ impl PalletCmd { &self, batches: &[BenchmarkBatchSplitResults], storage_info: &[StorageInfo], - component_ranges: &HashMap<(Vec, Vec), Vec>, + component_ranges: &ComponentRangeMap, pov_modes: PovModesMap, ) -> Result<()> { // Jsonify the result and write it to a file or stdout if desired. @@ -753,11 +737,13 @@ impl PalletCmd { /// Re-analyze a batch historic benchmark timing data. Will not take the PoV into account. fn output_from_results(&self, batches: &[BenchmarkBatchSplitResults]) -> Result<()> { - let mut component_ranges = - HashMap::<(Vec, Vec), HashMap>::new(); + let mut component_ranges = HashMap::<(String, String), HashMap>::new(); for batch in batches { let range = component_ranges - .entry((batch.pallet.clone(), batch.benchmark.clone())) + .entry(( + String::from_utf8(batch.pallet.clone()).unwrap(), + String::from_utf8(batch.benchmark.clone()).unwrap(), + )) .or_default(); for result in &batch.time_results { for (param, value) in &result.components { @@ -815,10 +801,13 @@ impl PalletCmd { ) { for batch in batches.iter() { // Print benchmark metadata + let pallet = String::from_utf8(batch.pallet.clone()).expect("Encoded from String; qed"); + let benchmark = + String::from_utf8(batch.benchmark.clone()).expect("Encoded from String; qed"); println!( "Pallet: {:?}, Extrinsic: {:?}, Lowest values: {:?}, Highest values: {:?}, Steps: {:?}, Repeat: {:?}", - String::from_utf8(batch.pallet.clone()).expect("Encoded from String; qed"), - String::from_utf8(batch.benchmark.clone()).expect("Encoded from String; qed"), + pallet, + benchmark, self.lowest_range_values, self.highest_range_values, self.steps, @@ -832,10 +821,7 @@ impl PalletCmd { if !self.no_storage_info { let mut storage_per_prefix = HashMap::, Vec>::new(); - let pov_mode = pov_modes - .get(&(batch.pallet.clone(), batch.benchmark.clone())) - .cloned() - .unwrap_or_default(); + let pov_mode = pov_modes.get(&(pallet, benchmark)).cloned().unwrap_or_default(); let comments = writer::process_storage_results( &mut storage_per_prefix, @@ -906,20 +892,11 @@ impl PalletCmd { } /// Parses the PoV modes per benchmark that were specified by the `#[pov_mode]` attribute. - fn parse_pov_modes( - benchmarks: &Vec<( - Vec, - Vec, - Vec<(BenchmarkParameter, u32, u32)>, - Vec<(String, String)>, - String, - String, - )>, - ) -> Result { + fn parse_pov_modes(benchmarks: &Vec) -> Result { use std::collections::hash_map::Entry; let mut parsed = PovModesMap::new(); - for (pallet, call, _components, pov_modes, _, _) in benchmarks { + for SelectedBenchmark { pallet, extrinsic, pov_modes, .. } in benchmarks { for (pallet_storage, mode) in pov_modes { let mode = PovEstimationMode::from_str(&mode)?; let splits = pallet_storage.split("::").collect::>(); @@ -933,7 +910,7 @@ impl PalletCmd { let (pov_pallet, pov_storage) = (splits[0], splits.get(1).unwrap_or(&"ALL")); match parsed - .entry((pallet.clone(), call.clone())) + .entry((pallet.clone(), extrinsic.clone())) .or_default() .entry((pov_pallet.to_string(), pov_storage.to_string())) { @@ -995,35 +972,28 @@ impl CliConfiguration for PalletCmd { /// List the benchmarks available in the runtime, in a CSV friendly format. fn list_benchmark( - benchmarks_to_run: Vec<( - Vec, - Vec, - Vec<(BenchmarkParameter, u32, u32)>, - Vec<(String, String)>, - String, - String, - )>, + benchmarks_to_run: Vec, list_output: ListOutput, no_csv_header: bool, ) { let mut benchmarks = BTreeMap::new(); // Sort and de-dub by pallet and function name. - benchmarks_to_run.iter().for_each(|(_, _, _, _, pallet_name, extrinsic_name)| { + benchmarks_to_run.iter().for_each(|bench| { benchmarks - .entry(pallet_name) + .entry(&bench.pallet) .or_insert_with(BTreeSet::new) - .insert(extrinsic_name); + .insert(&bench.extrinsic); }); match list_output { ListOutput::All => { if !no_csv_header { - println!("pallet,extrinsic"); + println!("pallet, extrinsic"); } for (pallet, extrinsics) in benchmarks { for extrinsic in extrinsics { - println!("{pallet},{extrinsic}"); + println!("{pallet}, {extrinsic}"); } } }, diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/types.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/types.rs index 66e70eae6538..2bb00d66560f 100644 --- a/substrate/utils/frame/benchmarking-cli/src/pallet/types.rs +++ b/substrate/utils/frame/benchmarking-cli/src/pallet/types.rs @@ -61,3 +61,18 @@ where } } } + +/// Maps a (pallet, benchmark) to its component ranges. +pub(crate) type ComponentRangeMap = + std::collections::HashMap<(String, String), Vec>; + +/// The inclusive range of a component. +#[derive(serde::Serialize, Debug, Clone, Eq, PartialEq)] +pub(crate) struct ComponentRange { + /// Name of the component. + pub(crate) name: String, + /// Minimal valid value of the component. + pub(crate) min: u32, + /// Maximal valid value of the component. + pub(crate) max: u32, +} diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/writer.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/writer.rs index bd4b65d8a2e3..df7d81b2822e 100644 --- a/substrate/utils/frame/benchmarking-cli/src/pallet/writer.rs +++ b/substrate/utils/frame/benchmarking-cli/src/pallet/writer.rs @@ -28,7 +28,10 @@ use itertools::Itertools; use serde::Serialize; use crate::{ - pallet::command::{ComponentRange, PovEstimationMode, PovModesMap}, + pallet::{ + command::{PovEstimationMode, PovModesMap}, + types::{ComponentRange, ComponentRangeMap}, + }, shared::UnderscoreHelper, PalletCmd, }; @@ -132,7 +135,7 @@ fn io_error(s: &str) -> std::io::Error { fn map_results( batches: &[BenchmarkBatchSplitResults], storage_info: &[StorageInfo], - component_ranges: &HashMap<(Vec, Vec), Vec>, + component_ranges: &ComponentRangeMap, pov_modes: PovModesMap, default_pov_mode: PovEstimationMode, analysis_choice: &AnalysisChoice, @@ -188,7 +191,7 @@ fn get_benchmark_data( batch: &BenchmarkBatchSplitResults, storage_info: &[StorageInfo], // Per extrinsic component ranges. - component_ranges: &HashMap<(Vec, Vec), Vec>, + component_ranges: &ComponentRangeMap, pov_modes: PovModesMap, default_pov_mode: PovEstimationMode, analysis_choice: &AnalysisChoice, @@ -207,6 +210,8 @@ fn get_benchmark_data( AnalysisChoice::MedianSlopes => Analysis::median_slopes, AnalysisChoice::Max => Analysis::max, }; + let pallet = String::from_utf8(batch.pallet.clone()).unwrap(); + let benchmark = String::from_utf8(batch.benchmark.clone()).unwrap(); let extrinsic_time = analysis_function(&batch.time_results, BenchmarkSelector::ExtrinsicTime) .expect("analysis function should return an extrinsic time for valid inputs"); @@ -282,10 +287,7 @@ fn get_benchmark_data( // We add additional comments showing which storage items were touched. // We find the worst case proof size, and use that as the final proof size result. let mut storage_per_prefix = HashMap::, Vec>::new(); - let pov_mode = pov_modes - .get(&(batch.pallet.clone(), batch.benchmark.clone())) - .cloned() - .unwrap_or_default(); + let pov_mode = pov_modes.get(&(pallet.clone(), benchmark.clone())).cloned().unwrap_or_default(); let comments = process_storage_results( &mut storage_per_prefix, &batch.db_results, @@ -351,12 +353,12 @@ fn get_benchmark_data( .collect::>(); let component_ranges = component_ranges - .get(&(batch.pallet.clone(), batch.benchmark.clone())) + .get(&(pallet.clone(), benchmark.clone())) .map(|c| c.clone()) .unwrap_or_default(); BenchmarkData { - name: String::from_utf8(batch.benchmark.clone()).unwrap(), + name: benchmark, components, base_weight: extrinsic_time.base, base_reads: reads.base, @@ -378,7 +380,7 @@ fn get_benchmark_data( pub(crate) fn write_results( batches: &[BenchmarkBatchSplitResults], storage_info: &[StorageInfo], - component_ranges: &HashMap<(Vec, Vec), Vec>, + component_ranges: &HashMap<(String, String), Vec>, pov_modes: PovModesMap, default_pov_mode: PovEstimationMode, path: &PathBuf, @@ -871,10 +873,10 @@ mod test { fn test_pov_mode() -> PovModesMap { let mut map = PovModesMap::new(); - map.entry((b"scheduler".to_vec(), b"first_benchmark".to_vec())) + map.entry(("scheduler".into(), "first_benchmark".into())) .or_default() .insert(("scheduler".into(), "mel".into()), PovEstimationMode::MaxEncodedLen); - map.entry((b"scheduler".to_vec(), b"first_benchmark".to_vec())) + map.entry(("scheduler".into(), "first_benchmark".into())) .or_default() .insert(("scheduler".into(), "measured".into()), PovEstimationMode::Measured); map diff --git a/substrate/utils/frame/omni-bencher/Cargo.toml b/substrate/utils/frame/omni-bencher/Cargo.toml index bbcf2f2dd768..7207e9840f13 100644 --- a/substrate/utils/frame/omni-bencher/Cargo.toml +++ b/substrate/utils/frame/omni-bencher/Cargo.toml @@ -13,7 +13,7 @@ workspace = true [dependencies] clap = { version = "4.5.2", features = ["derive"] } cumulus-primitives-proof-size-hostfunction = { path = "../../../../cumulus/primitives/proof-size-hostfunction" } -frame-benchmarking-cli = { path = "../benchmarking-cli", default-features = false } # Disable rockdb feature +frame-benchmarking-cli = { path = "../benchmarking-cli", default-features = false } sc-cli = { path = "../../../client/cli" } sp-runtime = { path = "../../../primitives/runtime" } sp-statement-store = { path = "../../../primitives/statement-store" } From e7f4c21dae984a09cab07a9cba74751305fcd250 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Mon, 25 Mar 2024 21:03:55 +0200 Subject: [PATCH 27/44] Cleanup Signed-off-by: Oliver Tale-Yazdi --- .../benchmarking-cli/src/pallet/command.rs | 5 ++- .../utils/frame/omni-bencher/src/command.rs | 40 +++++++++++++------ 2 files changed, 31 insertions(+), 14 deletions(-) diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs index bbdcada534ab..7ca8ea4d8008 100644 --- a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs +++ b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs @@ -597,11 +597,14 @@ impl PalletCmd { )?; // Create a dummy WasmExecutor just to build the genesis storage. + let method = + execution_method_from_cli(self.wasm_method, self.wasmtime_instantiation_strategy); let executor = WasmExecutor::<( sp_io::SubstrateHostFunctions, frame_benchmarking::benchmarking::HostFunctions, F, )>::builder() + .with_execution_method(method) .with_allow_missing_host_functions(self.allow_missing_host_functions) .build(); @@ -615,7 +618,7 @@ impl PalletCmd { &executor, "GenesisBuilder_create_default_config", &[], // no args for this call - &mut Extensions::default(), + &mut Self::build_extensions(executor.clone()), &runtime_code, CallContext::Offchain, ), diff --git a/substrate/utils/frame/omni-bencher/src/command.rs b/substrate/utils/frame/omni-bencher/src/command.rs index 47204e33f724..f30915a581d2 100644 --- a/substrate/utils/frame/omni-bencher/src/command.rs +++ b/substrate/utils/frame/omni-bencher/src/command.rs @@ -81,6 +81,7 @@ pub struct Command { sub: SubCommand, } +/// Root-level subcommands. #[derive(Debug, clap::Subcommand)] pub enum SubCommand { /// Compatibility syntax with the old benchmark runner. @@ -97,11 +98,13 @@ pub struct V1Command { sub: V1SubCommand, } +/// The `v1 benchmark` subcommand. #[derive(Debug, clap::Subcommand)] pub enum V1SubCommand { Benchmark(V1BenchmarkCommand), } +/// Subcommands for `v1 benchmark`. #[derive(Parser, Debug)] pub struct V1BenchmarkCommand { #[command(subcommand)] @@ -116,20 +119,31 @@ type HostFunctions = ( impl Command { pub fn run(self) -> Result<()> { match self.sub { - SubCommand::V1(V1Command { - sub: - V1SubCommand::Benchmark(V1BenchmarkCommand { sub: BenchmarkCmd::Pallet(pallet) }), - }) => { - if let Some(spec) = pallet.shared_params.chain { - return Err(format!( - "Chain specs are not supported. Please remove \ - `--chain={spec}` and use `--runtime=` instead" - ) - .into()) - } - pallet.run_with_spec::(None) + SubCommand::V1(V1Command { sub }) => sub.run(), + } + } +} + +impl V1SubCommand { + pub fn run(self) -> Result<()> { + let pallet = match self { + V1SubCommand::Benchmark(V1BenchmarkCommand { sub }) => match sub { + BenchmarkCmd::Pallet(pallet) => pallet, + _ => + return Err( + "Only the `v1 benchmark pallet` command is currently supported".into() + ), }, - _ => Err("Invalid subcommand. Only `v1 benchmark pallet` is supported.".into()), + }; + + if let Some(spec) = pallet.shared_params.chain { + return Err(format!( + "Chain specs are not supported. Please remove `--chain={spec}` and use \ + `--runtime=` instead" + ) + .into()) } + + pallet.run_with_spec::(None) } } From 7eeeee04faf451b5ee4c94be359ac90a12d0fb18 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Mon, 25 Mar 2024 21:05:16 +0200 Subject: [PATCH 28/44] Add PRdoc Signed-off-by: Oliver Tale-Yazdi --- prdoc/pr_3512.prdoc | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 prdoc/pr_3512.prdoc diff --git a/prdoc/pr_3512.prdoc b/prdoc/pr_3512.prdoc new file mode 100644 index 000000000000..56e1813cfe0c --- /dev/null +++ b/prdoc/pr_3512.prdoc @@ -0,0 +1,23 @@ +title: "Introduce Runtime Omni Bencher" + +doc: + - audience: Node Dev + description: | + Introduces a new freestanding binary; the `polkadot-omni-bencher`. This can be used as alternative to the node-integrated `benchmark pallet` command. It currently only supports pallet benchmarking. + + The optional change to integrate this MR is in the node. The `run` function is now deprecated in favour or `run_with_spec`. This should be rather easy to integrate: + + ```patch + runner.sync_run(|config| cmd + - .run::, ReclaimHostFunctions>(config) + + .run_with_spec::, ReclaimHostFunctions>(Some(config.chain_spec)) + ) + ``` + + Additionally, a new `--runtime` CLI arg was introduced to the `benchmark pallet` command. It allows to generate the genesis state directly by the runtime, instead of using a spec file. + +crates: + - name: pallet-nis + bump: major + - name: frame-benchmarking-cli + bump: major From 2d8203bed321d2f23a3457c5cabdd087df89d61a Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Fri, 5 Apr 2024 14:38:25 +0200 Subject: [PATCH 29/44] Rename to frame-omni-bencher Signed-off-by: Oliver Tale-Yazdi --- .gitlab/pipeline/test.yml | 2 +- Cargo.lock | 28 +++++++++---------- prdoc/pr_3512.prdoc | 2 +- .../utils/frame/benchmarking-cli/README.md | 2 +- substrate/utils/frame/omni-bencher/Cargo.toml | 2 +- .../utils/frame/omni-bencher/src/command.rs | 6 ++-- .../utils/frame/omni-bencher/src/main.rs | 2 +- 7 files changed, 22 insertions(+), 22 deletions(-) diff --git a/.gitlab/pipeline/test.yml b/.gitlab/pipeline/test.yml index 3ee402f0ae0b..8a3907683c54 100644 --- a/.gitlab/pipeline/test.yml +++ b/.gitlab/pipeline/test.yml @@ -340,7 +340,7 @@ quick-benchmarks-omni: WASM_BUILD_RUSTFLAGS: "-C debug-assertions -D warnings" script: - time cargo build --locked --quiet --release -p kitchensink-runtime --features runtime-benchmarks - - time cargo run --locked --release -p polkadot-omni-bencher --quiet -- v1 benchmark pallet --runtime target/release/wbuild/kitchensink-runtime/kitchensink_runtime.compact.compressed.wasm --all --steps 2 --repeat 1 --quiet + - time cargo run --locked --release -p frame-omni-bencher --quiet -- v1 benchmark pallet --runtime target/release/wbuild/kitchensink-runtime/kitchensink_runtime.compact.compressed.wasm --all --steps 2 --repeat 1 --quiet test-frame-examples-compile-to-wasm: # into one job diff --git a/Cargo.lock b/Cargo.lock index ced05db79fd0..25eb9374324a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5675,6 +5675,20 @@ dependencies = [ "serde", ] +[[package]] +name = "frame-omni-bencher" +version = "0.1.0" +dependencies = [ + "clap 4.5.3", + "cumulus-primitives-proof-size-hostfunction", + "env_logger 0.11.3", + "frame-benchmarking-cli", + "log", + "sc-cli", + "sp-runtime", + "sp-statement-store", +] + [[package]] name = "frame-remote-externalities" version = "0.35.0" @@ -13047,20 +13061,6 @@ dependencies = [ "tracing-gum", ] -[[package]] -name = "polkadot-omni-bencher" -version = "0.1.0" -dependencies = [ - "clap 4.5.3", - "cumulus-primitives-proof-size-hostfunction", - "env_logger 0.11.3", - "frame-benchmarking-cli", - "log", - "sc-cli", - "sp-runtime", - "sp-statement-store", -] - [[package]] name = "polkadot-overseer" version = "7.0.0" diff --git a/prdoc/pr_3512.prdoc b/prdoc/pr_3512.prdoc index 56e1813cfe0c..4211baa484ca 100644 --- a/prdoc/pr_3512.prdoc +++ b/prdoc/pr_3512.prdoc @@ -3,7 +3,7 @@ title: "Introduce Runtime Omni Bencher" doc: - audience: Node Dev description: | - Introduces a new freestanding binary; the `polkadot-omni-bencher`. This can be used as alternative to the node-integrated `benchmark pallet` command. It currently only supports pallet benchmarking. + Introduces a new freestanding binary; the `frame-omni-bencher`. This can be used as alternative to the node-integrated `benchmark pallet` command. It currently only supports pallet benchmarking. The optional change to integrate this MR is in the node. The `run` function is now deprecated in favour or `run_with_spec`. This should be rather easy to integrate: diff --git a/substrate/utils/frame/benchmarking-cli/README.md b/substrate/utils/frame/benchmarking-cli/README.md index 772da4f31fd7..5deb5098b5bf 100644 --- a/substrate/utils/frame/benchmarking-cli/README.md +++ b/substrate/utils/frame/benchmarking-cli/README.md @@ -69,7 +69,7 @@ cargo build -p westend-runtime --profile production --features runtime-benchmark Now the benchmarking can be started with: ```sh -polkadot-omni-bencher v1 \ +frame-omni-bencher v1 \ benchmark pallet \ --runtime target/release/wbuild/westend-runtime/westend-runtime.compact.compressed.wasm \ --pallet "pallet_balances" --extrinsic "" diff --git a/substrate/utils/frame/omni-bencher/Cargo.toml b/substrate/utils/frame/omni-bencher/Cargo.toml index 7207e9840f13..0c2d1a1b32b1 100644 --- a/substrate/utils/frame/omni-bencher/Cargo.toml +++ b/substrate/utils/frame/omni-bencher/Cargo.toml @@ -1,5 +1,5 @@ [package] -name = "polkadot-omni-bencher" +name = "frame-omni-bencher" version = "0.1.0" description = "Freestanding benchmark runner for any Polkadot runtime." authors.workspace = true diff --git a/substrate/utils/frame/omni-bencher/src/command.rs b/substrate/utils/frame/omni-bencher/src/command.rs index f30915a581d2..f0159f4307d6 100644 --- a/substrate/utils/frame/omni-bencher/src/command.rs +++ b/substrate/utils/frame/omni-bencher/src/command.rs @@ -36,7 +36,7 @@ use sp_runtime::traits::BlakeTwo256; /// Directly via crates.io: /// /// ```sh -/// cargo install --locked polkadot-omni-bencher +/// cargo install --locked frame-omni-bencher /// ``` /// /// or when the sources are locally checked out: @@ -48,7 +48,7 @@ use sp_runtime::traits::BlakeTwo256; /// Check the installed version and print the docs: /// /// ```sh -/// polkadot-omni-bencher --help +/// frame-omni-bencher --help /// ``` /// /// ## Usage @@ -63,7 +63,7 @@ use sp_runtime::traits::BlakeTwo256; /// Now as example we benchmark `pallet_balances`: /// /// ```sh -/// polkadot-omni-bencher v1 benchmark pallet \ +/// frame-omni-bencher v1 benchmark pallet \ /// --runtime target/release/wbuild/westend-runtime/westend-runtime.compact.compressed.wasm \ /// --pallet "pallet_balances" --extrinsic "" /// ``` diff --git a/substrate/utils/frame/omni-bencher/src/main.rs b/substrate/utils/frame/omni-bencher/src/main.rs index 138c66918dfd..c148403f2970 100644 --- a/substrate/utils/frame/omni-bencher/src/main.rs +++ b/substrate/utils/frame/omni-bencher/src/main.rs @@ -23,7 +23,7 @@ use sc_cli::Result; fn main() -> Result<()> { env_logger::Builder::from_env(Env::default().default_filter_or("info")).init(); - log::warn!("The Polkadot omni-bencher is not yet battle tested - double check the results.",); + log::warn!("The FRAME omni-bencher is not yet battle tested - double check the results.",); command::Command::parse().run() } From cbf5bd645f0c99e39cdc63b419d312550c36efff Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Fri, 5 Apr 2024 14:44:34 +0200 Subject: [PATCH 30/44] Update docs Signed-off-by: Oliver Tale-Yazdi --- polkadot/runtime/parachains/src/configuration.rs | 4 ++++ prdoc/pr_3512.prdoc | 2 +- substrate/utils/frame/benchmarking-cli/src/pallet/command.rs | 3 ++- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/polkadot/runtime/parachains/src/configuration.rs b/polkadot/runtime/parachains/src/configuration.rs index 8f0beb551c05..1328a60dd9f1 100644 --- a/polkadot/runtime/parachains/src/configuration.rs +++ b/polkadot/runtime/parachains/src/configuration.rs @@ -281,6 +281,10 @@ impl> Default for HostConfiguration> HostConfiguration { + /// Mutate the values of self to be good estimates for benchmarking. + /// + /// The values do not need to be worst-case, since the benchmarking logic extrapolates. They + /// should be a bit more than usually expected. fn with_benchmarking_default(mut self) -> Self { self.max_head_data_size = self.max_head_data_size.max(1 << 20); self.max_downward_message_size = self.max_downward_message_size.max(1 << 16); diff --git a/prdoc/pr_3512.prdoc b/prdoc/pr_3512.prdoc index 4211baa484ca..e6b16c5c7bf5 100644 --- a/prdoc/pr_3512.prdoc +++ b/prdoc/pr_3512.prdoc @@ -1,4 +1,4 @@ -title: "Introduce Runtime Omni Bencher" +title: "[FRAME] Introduce Runtime Omni Bencher" doc: - audience: Node Dev diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs index 7ca8ea4d8008..89afd717f494 100644 --- a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs +++ b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs @@ -162,7 +162,8 @@ impl PalletCmd { /// Runs the command and benchmarks a pallet. #[deprecated( note = "`run` will be removed after December 2024. Use `run_with_spec` instead or \ - completely remove the code and use the `frame-benchmarking-cli` instead." + completely remove the code and use the `frame-benchmarking-cli` instead (see \ + https://github.com/paritytech/polkadot-sdk/pull/3512)." )] pub fn run(&self, config: sc_service::Configuration) -> Result<()> where From 68d7e77db742c33500d72707ac10c5ecd3fbe874 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Fri, 5 Apr 2024 14:49:33 +0200 Subject: [PATCH 31/44] Update substrate/utils/frame/benchmarking-cli/src/pallet/mod.rs Co-authored-by: Liam Aharon --- substrate/utils/frame/benchmarking-cli/src/pallet/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/mod.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/mod.rs index 9a199582b969..afb7a5d229bd 100644 --- a/substrate/utils/frame/benchmarking-cli/src/pallet/mod.rs +++ b/substrate/utils/frame/benchmarking-cli/src/pallet/mod.rs @@ -177,7 +177,7 @@ pub struct PalletCmd { /// How to construct the genesis state. /// - /// Uses `GenesisBuilder::Spec` by default and `GenesisConstructor::Runtime` if `runtime` is + /// Uses [`GenesisBuilder::Spec`] by default and [`GenesisConstructor::Runtime`] if `runtime` is /// set. #[arg(long, value_enum)] pub genesis_builder: Option, From 9ab55478000ea848dfecc5edddac38838c3a684b Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Fri, 5 Apr 2024 16:06:05 +0200 Subject: [PATCH 32/44] Docs Signed-off-by: Oliver Tale-Yazdi --- .../utils/frame/benchmarking-cli/src/pallet/command.rs | 2 +- substrate/utils/frame/benchmarking-cli/src/pallet/mod.rs | 9 ++++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs index 89afd717f494..9ef1b7062bab 100644 --- a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs +++ b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs @@ -662,7 +662,7 @@ impl PalletCmd { .execute() .map_err(|e| format!("Could not call runtime API to {hint}: {}", e))?; let res = R::decode(&mut &res[..]) - .map_err(|e| format!("Failed to decode runtime API result: {:?}", e))?; + .map_err(|e| format!("Failed to decode runtime API result to {hint}: {:?}", e))?; Ok(res) } diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/mod.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/mod.rs index afb7a5d229bd..24784b6ce6cc 100644 --- a/substrate/utils/frame/benchmarking-cli/src/pallet/mod.rs +++ b/substrate/utils/frame/benchmarking-cli/src/pallet/mod.rs @@ -177,8 +177,8 @@ pub struct PalletCmd { /// How to construct the genesis state. /// - /// Uses [`GenesisBuilder::Spec`] by default and [`GenesisConstructor::Runtime`] if `runtime` is - /// set. + /// Uses [`GenesisBuilder::Spec`] by default and [`GenesisConstructor::Runtime`] if `runtime` + /// is set. #[arg(long, value_enum)] pub genesis_builder: Option, @@ -238,7 +238,10 @@ pub struct PalletCmd { #[arg(long)] pub unsafe_overwrite_results: bool, - /// Suppresses most prints. + /// Do not print a summary at the end of the run. + /// + /// These summaries can be very long when benchmarking multiple pallets at once. For CI + /// use-cases, this option reduces the noise. #[arg(long)] quiet: bool, } From 685e58231cda5d3ec04e0c58d4b3b0206b570018 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Fri, 5 Apr 2024 16:14:30 +0200 Subject: [PATCH 33/44] Add all crates to prdoc Signed-off-by: Oliver Tale-Yazdi --- prdoc/pr_3512.prdoc | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/prdoc/pr_3512.prdoc b/prdoc/pr_3512.prdoc index e6b16c5c7bf5..d44e1ca1e296 100644 --- a/prdoc/pr_3512.prdoc +++ b/prdoc/pr_3512.prdoc @@ -21,3 +21,27 @@ crates: bump: major - name: frame-benchmarking-cli bump: major + - name: polkadot-parachain-bin + bump: patch + - name: polkadot-cli + bump: patch + - name: polkadot-runtime-common + bump: patch + - name: polkadot-runtime-parachains + bump: patch + - name: rococo-runtime + bump: patch + - name: staging-node-cli + bump: patch + - name: kitchensink-runtime + bump: patch + - name: pallet-babe + bump: patch + - name: frame-benchmarking + bump: patch + - name: pallet-election-provider-multi-phase + bump: patch + - name: pallet-fast-unstake + bump: patch + - name: pallet-contracts + bump: patch From a4fef56fdde14fc164dc1233487e19cf6b6c17b6 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Fri, 5 Apr 2024 16:22:40 +0200 Subject: [PATCH 34/44] fixup merge Signed-off-by: Oliver Tale-Yazdi --- Cargo.lock | 10 ---------- .../utils/frame/benchmarking-cli/src/pallet/command.rs | 6 +++--- 2 files changed, 3 insertions(+), 13 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e05fa56bef78..2dbc0f610bc0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5009,16 +5009,6 @@ dependencies = [ "regex", ] -[[package]] -name = "env_logger" -version = "0.8.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a009aa4810eb158359dda09d0c87378e4bbb89b5a801f016885a4707ba24f7ea" -dependencies = [ - "log", - "regex", -] - [[package]] name = "env_logger" version = "0.8.4" diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs index 8fc862ec938e..1a517ce46101 100644 --- a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs +++ b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs @@ -341,7 +341,7 @@ impl PalletCmd { if !self.no_verify { let mut changes = genesis_changes.clone(); let state = &state_without_tracking; - // Dont use these results since verification code will add overhead. + // Don't use these results since verification code will add overhead. let _batch: Vec = match Self::exec_state_machine::< std::result::Result, String>, _, @@ -397,7 +397,7 @@ impl PalletCmd { pallet.as_bytes(), extrinsic.as_bytes(), &selected_components.clone(), - false, // dont run verification code for final values + false, // don't run verification code for final values self.repeat, ) .encode(), @@ -440,7 +440,7 @@ impl PalletCmd { pallet.as_bytes(), extrinsic.as_bytes(), &selected_components.clone(), - false, // dont run verification code for final values + false, // don't run verification code for final values self.repeat, ) .encode(), From 02f0514dba503fb99cda132f63cc682e49a6d997 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Fri, 5 Apr 2024 18:58:12 +0200 Subject: [PATCH 35/44] Cannot doc internal crate Signed-off-by: Oliver Tale-Yazdi --- substrate/utils/frame/benchmarking-cli/src/pallet/mod.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/mod.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/mod.rs index 24784b6ce6cc..d05b52f1ac87 100644 --- a/substrate/utils/frame/benchmarking-cli/src/pallet/mod.rs +++ b/substrate/utils/frame/benchmarking-cli/src/pallet/mod.rs @@ -177,8 +177,7 @@ pub struct PalletCmd { /// How to construct the genesis state. /// - /// Uses [`GenesisBuilder::Spec`] by default and [`GenesisConstructor::Runtime`] if `runtime` - /// is set. + /// Uses `GenesisBuilder::Spec` by default and `GenesisBuilder::Runtime` if `runtime` is set. #[arg(long, value_enum)] pub genesis_builder: Option, From b3507a79fab3c9bf4d67c9a477c4ba2b01f63b84 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Fri, 5 Apr 2024 19:41:19 +0200 Subject: [PATCH 36/44] Use new genesis builder api Signed-off-by: Oliver Tale-Yazdi --- Cargo.lock | 2 + .../utils/frame/benchmarking-cli/Cargo.toml | 2 + .../benchmarking-cli/src/pallet/command.rs | 81 ++++++++++++++++--- 3 files changed, 74 insertions(+), 11 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2dbc0f610bc0..c8460deaea56 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5503,6 +5503,7 @@ dependencies = [ "rand", "rand_pcg", "sc-block-builder", + "sc-chain-spec", "sc-cli", "sc-client-api", "sc-client-db", @@ -5516,6 +5517,7 @@ dependencies = [ "sp-core", "sp-database", "sp-externalities 0.25.0", + "sp-genesis-builder", "sp-inherents", "sp-io", "sp-keystore", diff --git a/substrate/utils/frame/benchmarking-cli/Cargo.toml b/substrate/utils/frame/benchmarking-cli/Cargo.toml index b7fbb24b1a09..92169484c928 100644 --- a/substrate/utils/frame/benchmarking-cli/Cargo.toml +++ b/substrate/utils/frame/benchmarking-cli/Cargo.toml @@ -37,6 +37,7 @@ frame-benchmarking = { path = "../../../frame/benchmarking" } frame-support = { path = "../../../frame/support" } frame-system = { path = "../../../frame/system" } sc-block-builder = { path = "../../../client/block-builder" } +sc-chain-spec = { path = "../../../client/chain-spec", default-features = false } sc-cli = { path = "../../../client/cli", default-features = false } sc-client-api = { path = "../../../client/api" } sc-client-db = { path = "../../../client/db", default-features = false } @@ -48,6 +49,7 @@ sp-blockchain = { path = "../../../primitives/blockchain" } sp-core = { path = "../../../primitives/core" } sp-database = { path = "../../../primitives/database" } sp-externalities = { path = "../../../primitives/externalities" } +sp-genesis-builder = { path = "../../../primitives/genesis-builder" } sp-inherents = { path = "../../../primitives/inherents" } sp-keystore = { path = "../../../primitives/keystore" } sp-runtime = { path = "../../../primitives/runtime" } diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs index 1a517ce46101..e14b3fbe17da 100644 --- a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs +++ b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs @@ -27,6 +27,7 @@ use frame_benchmarking::{ }; use frame_support::traits::StorageInfo; use linked_hash_map::LinkedHashMap; +use sc_chain_spec::json_patch::merge as json_merge; use sc_cli::{execution_method_from_cli, ChainSpec, CliConfiguration, Result, SharedParams}; use sc_client_db::BenchmarkingState; use sc_executor::{HeapAllocStrategy, WasmExecutor, DEFAULT_HEAP_ALLOC_STRATEGY}; @@ -38,6 +39,7 @@ use sp_core::{ traits::{CallContext, CodeExecutor, ReadRuntimeVersionExt, WrappedRuntimeCode}, }; use sp_externalities::Extensions; +use sp_genesis_builder::{PresetId, Result as GenesisBuildResult}; use sp_keystore::{testing::MemoryKeystore, KeystoreExt}; use sp_runtime::traits::Hash; use sp_state_machine::{OverlayedChanges, StateMachine}; @@ -158,6 +160,9 @@ generate the genesis state is deprecated. Please remove the `--chain`/`--dev`/`- point `--runtime` to your runtime blob and set `--genesis-builder=runtime`. This warning may \ become a hard error any time after December 2024."; +/// The preset that we expect to find in the GenesisBuilder runtime API. +const GENESIS_PRESET: &str = "development"; + impl PalletCmd { /// Runs the command and benchmarks a pallet. #[deprecated( @@ -612,13 +617,57 @@ impl PalletCmd { let runtime = self.runtime_blob(&state)?; let runtime_code = runtime.code()?; - let genesis_json: Vec = Self::exec_state_machine( + // We cannot use the `GenesisConfigBuilderRuntimeCaller` here since it returns the changes + // as `Storage` item, but we need it as `OverlayedChanges`. + let presets: Vec = Self::exec_state_machine( + StateMachine::new( + &state, + &mut Default::default(), + &executor, + "GenesisBuilder_preset_names", + &[], // no args + &mut Self::build_extensions(executor.clone()), + &runtime_code, + CallContext::Offchain, + ), + "get the genesis preset names", + )?; + + if !presets.contains(&"development".into()) { + log::error!("Available genesis presets: {:?}", presets); + return Err( + "GenesisBuilder::get_preset does not contain the `{GENESIS_PRESET}` preset".into() + ) + } + + let genesis_json: Option> = Self::exec_state_machine( + StateMachine::new( + &state, + &mut Default::default(), + &executor, + "GenesisBuilder_get_preset", + &None::.encode(), // Use the default preset + &mut Self::build_extensions(executor.clone()), + &runtime_code, + CallContext::Offchain, + ), + "build the genesis spec", + )?; + + let Some(base_genesis_json) = genesis_json else { + return Err("GenesisBuilder::get_preset returned no data".into()) + }; + + let base_genesis_json = serde_json::from_slice::(&base_genesis_json) + .map_err(|e| format!("GenesisBuilder::get_preset returned invalid JSON: {:?}", e))?; + + let genesis_json: Option> = Self::exec_state_machine( StateMachine::new( &state, &mut Default::default(), &executor, - "GenesisBuilder_create_default_config", - &[], // no args for this call + "GenesisBuilder_get_preset", + &Some::(GENESIS_PRESET.into()).encode(), // Use the default preset &mut Self::build_extensions(executor.clone()), &runtime_code, CallContext::Offchain, @@ -626,19 +675,29 @@ impl PalletCmd { "build the genesis spec", )?; + let Some(dev_genesis_json) = genesis_json else { + return Err("GenesisBuilder::get_preset returned no data".into()) + }; + // Sanity check that it is JSON before we plug it into the next call. - if !serde_json::from_slice::(&genesis_json).is_ok() { - log::warn!("GenesisBuilder::create_default_config returned invalid JSON"); - } + let dev_genesis_json = serde_json::from_slice::(&dev_genesis_json) + .map_err(|e| format!("GenesisBuilder::get_preset returned invalid JSON: {:?}", e))?; + + let mut genesis_json = serde_json::Value::default(); + json_merge(&mut genesis_json, base_genesis_json); + json_merge(&mut genesis_json, dev_genesis_json); + + let json_pretty_str = serde_json::to_string_pretty(&genesis_json) + .map_err(|e| format!("json to string failed: {e}"))?; let mut changes = Default::default(); - let build_config_ret: Vec = Self::exec_state_machine( + let build_res: GenesisBuildResult = Self::exec_state_machine( StateMachine::new( &state, &mut changes, &executor, - "GenesisBuilder_build_config", - &genesis_json.encode(), + "GenesisBuilder_build_state", + &json_pretty_str.encode(), &mut Extensions::default(), &runtime_code, CallContext::Offchain, @@ -646,8 +705,8 @@ impl PalletCmd { "populate the genesis state", )?; - if !build_config_ret.is_empty() { - log::warn!("GenesisBuilder::build_config returned unexpected data"); + if let Err(e) = build_res { + return Err(format!("GenesisBuilder::build_state failed: {}", e).into()) } Ok(changes) From a7717dbf2e5aa6c003156d624171972932229f17 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Fri, 5 Apr 2024 22:35:48 +0200 Subject: [PATCH 37/44] Test with rococo runtime Signed-off-by: Oliver Tale-Yazdi --- .gitlab/pipeline/test.yml | 4 ++-- .../utils/frame/benchmarking-cli/src/pallet/command.rs | 7 +++---- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/.gitlab/pipeline/test.yml b/.gitlab/pipeline/test.yml index f556289b59a5..63819abb294b 100644 --- a/.gitlab/pipeline/test.yml +++ b/.gitlab/pipeline/test.yml @@ -338,8 +338,8 @@ quick-benchmarks-omni: WASM_BUILD_NO_COLOR: 1 WASM_BUILD_RUSTFLAGS: "-C debug-assertions -D warnings" script: - - time cargo build --locked --quiet --release -p kitchensink-runtime --features runtime-benchmarks - - time cargo run --locked --release -p frame-omni-bencher --quiet -- v1 benchmark pallet --runtime target/release/wbuild/kitchensink-runtime/kitchensink_runtime.compact.compressed.wasm --all --steps 2 --repeat 1 --quiet + - time cargo build --locked --quiet --release -p rococo-runtime --features runtime-benchmarks + - time cargo run --locked --release -p frame-omni-bencher --quiet -- v1 benchmark pallet --runtime target/release/wbuild/rococo-runtime/rococo_runtime.compact.compressed.wasm --all --steps 2 --repeat 1 --quiet test-frame-examples-compile-to-wasm: # into one job diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs index e14b3fbe17da..751c11fe8d8d 100644 --- a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs +++ b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs @@ -634,10 +634,9 @@ impl PalletCmd { )?; if !presets.contains(&"development".into()) { - log::error!("Available genesis presets: {:?}", presets); - return Err( - "GenesisBuilder::get_preset does not contain the `{GENESIS_PRESET}` preset".into() - ) + log::warn!( + "Could not find genesis preset '{GENESIS_PRESET}'. Falling back to default." + ); } let genesis_json: Option> = Self::exec_state_machine( From 1a4db1a9065130e05f53c8c7fc1f944f5d933823 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Sat, 6 Apr 2024 00:23:35 +0200 Subject: [PATCH 38/44] Ignore rococo build warnings Signed-off-by: Oliver Tale-Yazdi --- .gitlab/pipeline/test.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.gitlab/pipeline/test.yml b/.gitlab/pipeline/test.yml index 63819abb294b..a4b419591699 100644 --- a/.gitlab/pipeline/test.yml +++ b/.gitlab/pipeline/test.yml @@ -333,10 +333,10 @@ quick-benchmarks-omni: variables: # Enable debug assertions since we are running optimized builds for testing # but still want to have debug assertions. - RUSTFLAGS: "-C debug-assertions -D warnings" + RUSTFLAGS: "-C debug-assertions" RUST_BACKTRACE: "full" WASM_BUILD_NO_COLOR: 1 - WASM_BUILD_RUSTFLAGS: "-C debug-assertions -D warnings" + WASM_BUILD_RUSTFLAGS: "-C debug-assertions" script: - time cargo build --locked --quiet --release -p rococo-runtime --features runtime-benchmarks - time cargo run --locked --release -p frame-omni-bencher --quiet -- v1 benchmark pallet --runtime target/release/wbuild/rococo-runtime/rococo_runtime.compact.compressed.wasm --all --steps 2 --repeat 1 --quiet From 86a40e47250a32cae52911352bab3063ca87dadb Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Mon, 8 Apr 2024 11:58:50 +0200 Subject: [PATCH 39/44] Disable forlift Signed-off-by: Oliver Tale-Yazdi --- .gitlab/pipeline/test.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitlab/pipeline/test.yml b/.gitlab/pipeline/test.yml index a4b419591699..79e35f75a67d 100644 --- a/.gitlab/pipeline/test.yml +++ b/.gitlab/pipeline/test.yml @@ -337,6 +337,7 @@ quick-benchmarks-omni: RUST_BACKTRACE: "full" WASM_BUILD_NO_COLOR: 1 WASM_BUILD_RUSTFLAGS: "-C debug-assertions" + FORKLIFT_BYPASS: true script: - time cargo build --locked --quiet --release -p rococo-runtime --features runtime-benchmarks - time cargo run --locked --release -p frame-omni-bencher --quiet -- v1 benchmark pallet --runtime target/release/wbuild/rococo-runtime/rococo_runtime.compact.compressed.wasm --all --steps 2 --repeat 1 --quiet From 76a8b0cece74af3a0c84c7fd720073da1ae7b2bd Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Mon, 8 Apr 2024 13:32:49 +0200 Subject: [PATCH 40/44] Revert "Disable forlift" This reverts commit 86a40e47250a32cae52911352bab3063ca87dadb. --- .gitlab/pipeline/test.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.gitlab/pipeline/test.yml b/.gitlab/pipeline/test.yml index 79e35f75a67d..a4b419591699 100644 --- a/.gitlab/pipeline/test.yml +++ b/.gitlab/pipeline/test.yml @@ -337,7 +337,6 @@ quick-benchmarks-omni: RUST_BACKTRACE: "full" WASM_BUILD_NO_COLOR: 1 WASM_BUILD_RUSTFLAGS: "-C debug-assertions" - FORKLIFT_BYPASS: true script: - time cargo build --locked --quiet --release -p rococo-runtime --features runtime-benchmarks - time cargo run --locked --release -p frame-omni-bencher --quiet -- v1 benchmark pallet --runtime target/release/wbuild/rococo-runtime/rococo_runtime.compact.compressed.wasm --all --steps 2 --repeat 1 --quiet From 95b0e36f3a4cb853185329301d7c5d91730be55d Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Mon, 8 Apr 2024 15:54:51 +0200 Subject: [PATCH 41/44] Enable assertions for short-benchmarks Signed-off-by: Oliver Tale-Yazdi --- .gitlab/pipeline/build.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.gitlab/pipeline/build.yml b/.gitlab/pipeline/build.yml index 8658e92efc8f..c890cdacd3b8 100644 --- a/.gitlab/pipeline/build.yml +++ b/.gitlab/pipeline/build.yml @@ -150,6 +150,10 @@ build-short-benchmark: - .common-refs - .run-immediately - .collect-artifacts + variables: + RUSTFLAGS: "-C debug-assertions" + WASM_BUILD_NO_COLOR: 1 + WASM_BUILD_RUSTFLAGS: "-C debug-assertions" script: - cargo build --profile release --locked --features=runtime-benchmarks,on-chain-release-build --bin polkadot --workspace - mkdir -p artifacts From f04b21e35279964617416d7d6281e5eae24eb4c8 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Mon, 8 Apr 2024 16:11:28 +0200 Subject: [PATCH 42/44] Revert "Enable assertions for short-benchmarks" This reverts commit 95b0e36f3a4cb853185329301d7c5d91730be55d. --- .gitlab/pipeline/build.yml | 4 ---- 1 file changed, 4 deletions(-) diff --git a/.gitlab/pipeline/build.yml b/.gitlab/pipeline/build.yml index c890cdacd3b8..8658e92efc8f 100644 --- a/.gitlab/pipeline/build.yml +++ b/.gitlab/pipeline/build.yml @@ -150,10 +150,6 @@ build-short-benchmark: - .common-refs - .run-immediately - .collect-artifacts - variables: - RUSTFLAGS: "-C debug-assertions" - WASM_BUILD_NO_COLOR: 1 - WASM_BUILD_RUSTFLAGS: "-C debug-assertions" script: - cargo build --profile release --locked --features=runtime-benchmarks,on-chain-release-build --bin polkadot --workspace - mkdir -p artifacts From 2e0e5e38c53584f2e0efafdb194501837afe4b86 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Mon, 8 Apr 2024 16:14:07 +0200 Subject: [PATCH 43/44] only run for pallet-balances Signed-off-by: Oliver Tale-Yazdi --- .gitlab/pipeline/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitlab/pipeline/test.yml b/.gitlab/pipeline/test.yml index a4b419591699..e62ea4153bf7 100644 --- a/.gitlab/pipeline/test.yml +++ b/.gitlab/pipeline/test.yml @@ -339,7 +339,7 @@ quick-benchmarks-omni: WASM_BUILD_RUSTFLAGS: "-C debug-assertions" script: - time cargo build --locked --quiet --release -p rococo-runtime --features runtime-benchmarks - - time cargo run --locked --release -p frame-omni-bencher --quiet -- v1 benchmark pallet --runtime target/release/wbuild/rococo-runtime/rococo_runtime.compact.compressed.wasm --all --steps 2 --repeat 1 --quiet + - time cargo run --locked --release -p frame-omni-bencher --quiet -- v1 benchmark pallet --runtime target/release/wbuild/rococo-runtime/rococo_runtime.compact.compressed.wasm --pallet="pallet_balances" --extrinsic="" --steps 2 --repeat 1 --quiet test-frame-examples-compile-to-wasm: # into one job From b420bdd8ba4d94b76fdbf717ff52bab931fe7344 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Mon, 8 Apr 2024 16:40:15 +0200 Subject: [PATCH 44/44] Use asset-hub-rococo to test omni-bencher Signed-off-by: Oliver Tale-Yazdi --- .gitlab/pipeline/test.yml | 4 +- .../benchmarking-cli/src/pallet/command.rs | 42 ++++++------------- 2 files changed, 14 insertions(+), 32 deletions(-) diff --git a/.gitlab/pipeline/test.yml b/.gitlab/pipeline/test.yml index e62ea4153bf7..af16f5d2de7f 100644 --- a/.gitlab/pipeline/test.yml +++ b/.gitlab/pipeline/test.yml @@ -338,8 +338,8 @@ quick-benchmarks-omni: WASM_BUILD_NO_COLOR: 1 WASM_BUILD_RUSTFLAGS: "-C debug-assertions" script: - - time cargo build --locked --quiet --release -p rococo-runtime --features runtime-benchmarks - - time cargo run --locked --release -p frame-omni-bencher --quiet -- v1 benchmark pallet --runtime target/release/wbuild/rococo-runtime/rococo_runtime.compact.compressed.wasm --pallet="pallet_balances" --extrinsic="" --steps 2 --repeat 1 --quiet + - time cargo build --locked --quiet --release -p asset-hub-westend-runtime --features runtime-benchmarks + - time cargo run --locked --release -p frame-omni-bencher --quiet -- v1 benchmark pallet --runtime target/release/wbuild/asset-hub-westend-runtime/asset_hub_westend_runtime.compact.compressed.wasm --all --steps 2 --repeat 1 --quiet test-frame-examples-compile-to-wasm: # into one job diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs index 751c11fe8d8d..305a9b960b98 100644 --- a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs +++ b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs @@ -619,26 +619,6 @@ impl PalletCmd { // We cannot use the `GenesisConfigBuilderRuntimeCaller` here since it returns the changes // as `Storage` item, but we need it as `OverlayedChanges`. - let presets: Vec = Self::exec_state_machine( - StateMachine::new( - &state, - &mut Default::default(), - &executor, - "GenesisBuilder_preset_names", - &[], // no args - &mut Self::build_extensions(executor.clone()), - &runtime_code, - CallContext::Offchain, - ), - "get the genesis preset names", - )?; - - if !presets.contains(&"development".into()) { - log::warn!( - "Could not find genesis preset '{GENESIS_PRESET}'. Falling back to default." - ); - } - let genesis_json: Option> = Self::exec_state_machine( StateMachine::new( &state, @@ -660,7 +640,7 @@ impl PalletCmd { let base_genesis_json = serde_json::from_slice::(&base_genesis_json) .map_err(|e| format!("GenesisBuilder::get_preset returned invalid JSON: {:?}", e))?; - let genesis_json: Option> = Self::exec_state_machine( + let dev_genesis_json: Option> = Self::exec_state_machine( StateMachine::new( &state, &mut Default::default(), @@ -674,17 +654,19 @@ impl PalletCmd { "build the genesis spec", )?; - let Some(dev_genesis_json) = genesis_json else { - return Err("GenesisBuilder::get_preset returned no data".into()) - }; - - // Sanity check that it is JSON before we plug it into the next call. - let dev_genesis_json = serde_json::from_slice::(&dev_genesis_json) - .map_err(|e| format!("GenesisBuilder::get_preset returned invalid JSON: {:?}", e))?; - let mut genesis_json = serde_json::Value::default(); json_merge(&mut genesis_json, base_genesis_json); - json_merge(&mut genesis_json, dev_genesis_json); + + if let Some(dev) = dev_genesis_json { + let dev: serde_json::Value = serde_json::from_slice(&dev).map_err(|e| { + format!("GenesisBuilder::get_preset returned invalid JSON: {:?}", e) + })?; + json_merge(&mut genesis_json, dev); + } else { + log::warn!( + "Could not find genesis preset '{GENESIS_PRESET}'. Falling back to default." + ); + } let json_pretty_str = serde_json::to_string_pretty(&genesis_json) .map_err(|e| format!("json to string failed: {e}"))?;