From 051fa6ed0a5bb4244c7a7e543ae9841842747845 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Wed, 18 May 2022 14:17:57 +0200 Subject: [PATCH 01/20] Benchmark extrinsic Signed-off-by: Oliver Tale-Yazdi --- .../src/{overhead => extrinsic}/bench.rs | 64 ++++--------- .../benchmarking-cli/src/extrinsic/cmd.rs | 96 +++++++++++++++++++ .../src/extrinsic/extrinsic_factory.rs | 71 ++++++++++++++ .../benchmarking-cli/src/extrinsic/mod.rs | 27 ++++++ utils/frame/benchmarking-cli/src/lib.rs | 4 +- .../benchmarking-cli/src/overhead/cmd.rs | 46 ++++++--- .../benchmarking-cli/src/overhead/mod.rs | 5 +- .../benchmarking-cli/src/overhead/template.rs | 3 +- 8 files changed, 252 insertions(+), 64 deletions(-) rename utils/frame/benchmarking-cli/src/{overhead => extrinsic}/bench.rs (75%) create mode 100644 utils/frame/benchmarking-cli/src/extrinsic/cmd.rs create mode 100644 utils/frame/benchmarking-cli/src/extrinsic/extrinsic_factory.rs create mode 100644 utils/frame/benchmarking-cli/src/extrinsic/mod.rs diff --git a/utils/frame/benchmarking-cli/src/overhead/bench.rs b/utils/frame/benchmarking-cli/src/extrinsic/bench.rs similarity index 75% rename from utils/frame/benchmarking-cli/src/overhead/bench.rs rename to utils/frame/benchmarking-cli/src/extrinsic/bench.rs index 68f3f6597b466..4a81adc398216 100644 --- a/utils/frame/benchmarking-cli/src/overhead/bench.rs +++ b/utils/frame/benchmarking-cli/src/extrinsic/bench.rs @@ -36,8 +36,8 @@ use log::info; use serde::Serialize; use std::{marker::PhantomData, sync::Arc, time::Instant}; -use super::cmd::ExtrinsicBuilder; use crate::shared::Stats; +use super::ExtrinsicBuilder; /// Parameters to configure an *overhead* benchmark. #[derive(Debug, Default, Serialize, Clone, PartialEq, Args)] @@ -60,21 +60,11 @@ pub struct BenchmarkParams { /// The results of multiple runs in nano seconds. pub(crate) type BenchRecord = Vec; -/// Type of a benchmark. -#[derive(Serialize, Clone, PartialEq, Copy)] -pub(crate) enum BenchmarkType { - /// Measure the per-extrinsic execution overhead. - Extrinsic, - /// Measure the per-block execution overhead. - Block, -} - /// Holds all objects needed to run the *overhead* benchmarks. pub(crate) struct Benchmark { client: Arc, params: BenchmarkParams, inherent_data: sp_inherents::InherentData, - ext_builder: Arc, _p: PhantomData<(Block, BA)>, } @@ -90,15 +80,14 @@ where client: Arc, params: BenchmarkParams, inherent_data: sp_inherents::InherentData, - ext_builder: Arc, ) -> Self { - Self { client, params, inherent_data, ext_builder, _p: PhantomData } + Self { client, params, inherent_data, _p: PhantomData } } /// Run the specified benchmark. - pub fn bench(&self, bench_type: BenchmarkType) -> Result { - let (block, num_ext) = self.build_block(bench_type)?; - let record = self.measure_block(&block, num_ext, bench_type)?; + pub fn bench(&self, ext_builder: Option<&dyn ExtrinsicBuilder>) -> Result { + let (block, num_ext) = self.build_block(ext_builder)?; + let record = self.measure_block(&block, num_ext)?; Stats::new(&record) } @@ -106,7 +95,7 @@ where /// /// Returns the block and the number of extrinsics in the block /// that are not inherents. - fn build_block(&self, bench_type: BenchmarkType) -> Result<(Block, u64)> { + fn build_block(&self, ext_builder: Option<&dyn ExtrinsicBuilder>) -> Result<(Block, Option)> { let mut builder = self.client.new_block(Default::default())?; // Create and insert the inherents. let inherents = builder.create_inherents(self.inherent_data.clone())?; @@ -114,16 +103,18 @@ where builder.push(inherent)?; } - // Return early if we just want a block with inherents and no additional extrinsics. - if bench_type == BenchmarkType::Block { - return Ok((builder.build()?.block, 0)) - } + let ext_builder = if let Some(ext_builder) = ext_builder { + ext_builder + } else { + // Return early if we just want a block with inherents and no additional extrinsics. + return Ok((builder.build()?.block, None)) + }; // Put as many extrinsics into the block as possible and count them. info!("Building block, this takes some time..."); let mut num_ext = 0; for nonce in 0..self.max_ext_per_block() { - let ext = self.ext_builder.remark(nonce)?; + let ext = ext_builder.build(nonce)?; match builder.push(ext.clone()) { Ok(()) => {}, Err(ApplyExtrinsicFailed(Validity(TransactionValidityError::Invalid( @@ -139,19 +130,18 @@ where info!("Extrinsics per block: {}", num_ext); let block = builder.build()?.block; - Ok((block, num_ext)) + Ok((block, Some(num_ext))) } /// Measures the time that it take to execute a block or an extrinsic. fn measure_block( &self, block: &Block, - num_ext: u64, - bench_type: BenchmarkType, + num_ext: Option, ) -> Result { let mut record = BenchRecord::new(); - if bench_type == BenchmarkType::Extrinsic && num_ext == 0 { - return Err("Cannot measure the extrinsic time of an empty block".into()) + if num_ext == Some(0) { + return Err("Cannot divide by zero".into()) } let genesis = BlockId::Number(Zero::zero()); @@ -176,7 +166,7 @@ where .map_err(|e| Error::Client(RuntimeApiError(e)))?; let elapsed = start.elapsed().as_nanos(); - if bench_type == BenchmarkType::Extrinsic { + if let Some(num_ext) = num_ext { // Checked for non-zero div above. record.push((elapsed as f64 / num_ext as f64).ceil() as u64); } else { @@ -191,21 +181,3 @@ where self.params.max_ext_per_block.unwrap_or(u32::MAX) } } - -impl BenchmarkType { - /// Short name of the benchmark type. - pub(crate) fn short_name(&self) -> &'static str { - match self { - Self::Extrinsic => "extrinsic", - Self::Block => "block", - } - } - - /// Long name of the benchmark type. - pub(crate) fn long_name(&self) -> &'static str { - match self { - Self::Extrinsic => "ExtrinsicBase", - Self::Block => "BlockExecution", - } - } -} diff --git a/utils/frame/benchmarking-cli/src/extrinsic/cmd.rs b/utils/frame/benchmarking-cli/src/extrinsic/cmd.rs new file mode 100644 index 0000000000000..8417a0f1a0498 --- /dev/null +++ b/utils/frame/benchmarking-cli/src/extrinsic/cmd.rs @@ -0,0 +1,96 @@ +// This file is part of Substrate. + +// Copyright (C) 2022 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 sc_cli::CliConfiguration; +use sc_cli::{ImportParams, SharedParams}; +use sc_block_builder::{BlockBuilderApi, BlockBuilderProvider}; +use sc_cli::{Result}; +use sc_client_api::Backend as ClientBackend; +use sp_api::{ApiExt, ProvideRuntimeApi}; +use sp_runtime::{traits::Block as BlockT, OpaqueExtrinsic}; + +use clap::{Args, Parser}; +use log::info; +use serde::Serialize; +use std::{fmt::Debug, sync::Arc}; + +use super::bench::Benchmark; +use super::bench::BenchmarkParams; +use super::extrinsic_factory::ExtrinsicFactory; + +#[derive(Debug, Parser)] +pub struct ExtrinsicCmd { + #[allow(missing_docs)] + #[clap(flatten)] + pub shared_params: SharedParams, + + #[allow(missing_docs)] + #[clap(flatten)] + pub import_params: ImportParams, + + #[allow(missing_docs)] + #[clap(flatten)] + pub params: ExtrinsicParams, +} + +#[derive(Debug, Default, Serialize, Clone, PartialEq, Args)] +pub struct ExtrinsicParams { + #[clap(flatten)] + pub bench: BenchmarkParams, + + /// Pallet name of the extrinsic. + #[clap(short, long, value_name = "PALLET", index = 1)] + pub pallet: String, + + /// Extrinsic name. + #[clap(short, long, value_name = "EXTRINSIC", index = 2)] + pub extrinsic: String, +} + +impl ExtrinsicCmd { + pub fn run( + &self, + client: Arc, + inherent_data: sp_inherents::InherentData, + ext_factory: &ExtrinsicFactory, + ) -> Result<()> + where + Block: BlockT, + BA: ClientBackend, + C: BlockBuilderProvider + ProvideRuntimeApi, + C::Api: ApiExt + BlockBuilderApi, + { + let ext_builder = ext_factory.try_get(&self.params.pallet, &self.params.extrinsic).expect("TODO"); + let bench = Benchmark::new(client, self.params.bench.clone(), inherent_data); + + let stats = bench.bench(Some(ext_builder))?; + info!("Executing a {} extrinsic takes[ns]:\n{:?}", &ext_builder, stats); + + Ok(()) + } +} + +// Boilerplate +impl CliConfiguration for ExtrinsicCmd { + fn shared_params(&self) -> &SharedParams { + &self.shared_params + } + + fn import_params(&self) -> Option<&ImportParams> { + Some(&self.import_params) + } +} diff --git a/utils/frame/benchmarking-cli/src/extrinsic/extrinsic_factory.rs b/utils/frame/benchmarking-cli/src/extrinsic/extrinsic_factory.rs new file mode 100644 index 0000000000000..d1a3e10bf97a7 --- /dev/null +++ b/utils/frame/benchmarking-cli/src/extrinsic/extrinsic_factory.rs @@ -0,0 +1,71 @@ +// This file is part of Substrate. + +// Copyright (C) 2022 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. + +//! Provides the [`ExtrinsicFactory`] and the [`ExtrinsicBuilder`] types. +//! Is used by the *overhead* and *extrinsic* benchmarking to build extrinsics. + +use sp_runtime::OpaqueExtrinsic; + +#[derive(Default)] +/// Helper to manage [`ExtrinsicBuilder`] instances. +pub struct ExtrinsicFactory(pub Vec>); + +impl ExtrinsicFactory { + /// Adds a builder to the list and consumes [`Self`]. + pub fn with_builder(mut self, builder: Box) -> Self { + self.0.push(builder); + self + } + + /// Returns an optional builder for an extrinsic. + pub fn try_get(&self, pallet: &str, extrinsic: &str) -> Option<&dyn ExtrinsicBuilder> { + self.0 + .iter() + .find(|b| { + b.pallet() == pallet.to_lowercase() && b.extrinsic() == extrinsic.to_lowercase() + }) + .map(|b| b.as_ref()) + } + + /// Formats the builders in the standard `Pallet::Extrinsic` scheme. + pub fn as_str_vec(&self) -> Vec { + self.0.iter().map(|b| format!("{}", b.as_ref())).collect() + } +} + +/// Used by the benchmark to build signed extrinsics. +/// +/// The built extrinsics only need to be valid in the first block +/// who's parent block is the genesis block. +/// This assumption simplifies the generation of the extrinsics. +pub trait ExtrinsicBuilder { + /// Pallet of the extrinsic. + fn pallet(&self) -> &'static str; + /// Name of the extrinsic. + fn extrinsic(&self) -> &'static str; + + /// Builds an extrinsic. + /// + /// Will be called multiple times with increasing nonces. + fn build(&self, nonce: u32) -> std::result::Result; +} + +impl std::fmt::Display for &dyn ExtrinsicBuilder { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + write!(f, "{}::{}", self.pallet().to_uppercase(), self.extrinsic().to_uppercase()) + } +} diff --git a/utils/frame/benchmarking-cli/src/extrinsic/mod.rs b/utils/frame/benchmarking-cli/src/extrinsic/mod.rs new file mode 100644 index 0000000000000..2dc9179729b14 --- /dev/null +++ b/utils/frame/benchmarking-cli/src/extrinsic/mod.rs @@ -0,0 +1,27 @@ +// This file is part of Substrate. + +// Copyright (C) 2022 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. + +//! Benchmark the time it takes to execute a specific extrinsic. +//! This is a generalization of the *overhead* benchmark which can only measure `System::Remark` +//! extrinsics. + +pub mod cmd; +pub mod extrinsic_factory; +pub mod bench; + +pub use cmd::ExtrinsicCmd; +pub use extrinsic_factory::{ExtrinsicBuilder, ExtrinsicFactory}; diff --git a/utils/frame/benchmarking-cli/src/lib.rs b/utils/frame/benchmarking-cli/src/lib.rs index d0eee3d2939fc..301a64a5f0443 100644 --- a/utils/frame/benchmarking-cli/src/lib.rs +++ b/utils/frame/benchmarking-cli/src/lib.rs @@ -18,6 +18,7 @@ //! Contains the root [`BenchmarkCmd`] command and exports its sub-commands. mod block; +mod extrinsic; mod machine; mod overhead; mod pallet; @@ -25,8 +26,9 @@ mod shared; mod storage; pub use block::BlockCmd; +pub use extrinsic::ExtrinsicCmd; pub use machine::{MachineCmd, Requirements, SUBSTRATE_REFERENCE_HARDWARE}; -pub use overhead::{ExtrinsicBuilder, OverheadCmd}; +pub use overhead::OverheadCmd; pub use pallet::PalletCmd; pub use storage::StorageCmd; diff --git a/utils/frame/benchmarking-cli/src/overhead/cmd.rs b/utils/frame/benchmarking-cli/src/overhead/cmd.rs index 3cf281986861f..69aaae48866f8 100644 --- a/utils/frame/benchmarking-cli/src/overhead/cmd.rs +++ b/utils/frame/benchmarking-cli/src/overhead/cmd.rs @@ -31,11 +31,12 @@ use serde::Serialize; use std::{fmt::Debug, sync::Arc}; use crate::{ + extrinsic::bench::{Benchmark, BenchmarkParams}, overhead::{ - bench::{Benchmark, BenchmarkParams, BenchmarkType}, template::TemplateData, }, - shared::WeightParams, + extrinsic::ExtrinsicFactory, + shared::{ WeightParams}, }; /// Benchmark the execution overhead per-block and per-extrinsic. @@ -66,13 +67,13 @@ pub struct OverheadParams { pub bench: BenchmarkParams, } -/// Used by the benchmark to build signed extrinsics. -/// -/// The built extrinsics only need to be valid in the first block -/// who's parent block is the genesis block. -pub trait ExtrinsicBuilder { - /// Build a `System::remark` extrinsic. - fn remark(&self, nonce: u32) -> std::result::Result; +/// Type of a benchmark. +#[derive(Serialize, Clone, PartialEq, Copy)] +pub(crate) enum BenchmarkType { + /// Measure the per-extrinsic execution overhead. + Extrinsic, + /// Measure the per-block execution overhead. + Block, } impl OverheadCmd { @@ -85,7 +86,7 @@ impl OverheadCmd { cfg: Configuration, client: Arc, inherent_data: sp_inherents::InherentData, - ext_builder: Arc, + ext_factory: &ExtrinsicFactory, ) -> Result<()> where Block: BlockT, @@ -93,18 +94,19 @@ impl OverheadCmd { C: BlockBuilderProvider + ProvideRuntimeApi, C::Api: ApiExt + BlockBuilderApi, { - let bench = Benchmark::new(client, self.params.bench.clone(), inherent_data, ext_builder); + let ext_builder = ext_factory.try_get("system", "remark").expect("TODO"); + let bench = Benchmark::new(client, self.params.bench.clone(), inherent_data); // per-block execution overhead { - let stats = bench.bench(BenchmarkType::Block)?; + let stats = bench.bench(Some(ext_builder))?; info!("Per-block execution overhead [ns]:\n{:?}", stats); let template = TemplateData::new(BenchmarkType::Block, &cfg, &self.params, &stats)?; template.write(&self.params.weight.weight_path)?; } // per-extrinsic execution overhead { - let stats = bench.bench(BenchmarkType::Extrinsic)?; + let stats = bench.bench(Some(ext_builder))?; info!("Per-extrinsic execution overhead [ns]:\n{:?}", stats); let template = TemplateData::new(BenchmarkType::Extrinsic, &cfg, &self.params, &stats)?; template.write(&self.params.weight.weight_path)?; @@ -114,6 +116,24 @@ impl OverheadCmd { } } +impl BenchmarkType { + /// Short name of the benchmark type. + pub(crate) fn short_name(&self) -> &'static str { + match self { + Self::Extrinsic => "extrinsic", + Self::Block => "block", + } + } + + /// Long name of the benchmark type. + pub(crate) fn long_name(&self) -> &'static str { + match self { + Self::Extrinsic => "ExtrinsicBase", + Self::Block => "BlockExecution", + } + } +} + // Boilerplate impl CliConfiguration for OverheadCmd { fn shared_params(&self) -> &SharedParams { diff --git a/utils/frame/benchmarking-cli/src/overhead/mod.rs b/utils/frame/benchmarking-cli/src/overhead/mod.rs index abdeac22b7898..fc3db912f7a30 100644 --- a/utils/frame/benchmarking-cli/src/overhead/mod.rs +++ b/utils/frame/benchmarking-cli/src/overhead/mod.rs @@ -15,8 +15,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -mod bench; pub mod cmd; -mod template; +pub mod template; -pub use cmd::{ExtrinsicBuilder, OverheadCmd}; +pub use cmd::OverheadCmd; diff --git a/utils/frame/benchmarking-cli/src/overhead/template.rs b/utils/frame/benchmarking-cli/src/overhead/template.rs index 44e2c1f02e30f..70aee3d12fc8b 100644 --- a/utils/frame/benchmarking-cli/src/overhead/template.rs +++ b/utils/frame/benchmarking-cli/src/overhead/template.rs @@ -27,7 +27,8 @@ use serde::Serialize; use std::{env, fs, path::PathBuf}; use crate::{ - overhead::{bench::BenchmarkType, cmd::OverheadParams}, + overhead::cmd::BenchmarkType, + overhead::{cmd::OverheadParams}, shared::{Stats, UnderscoreHelper}, }; From 43cedac16832216773be518ad67cb3c01b5834de Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Wed, 18 May 2022 15:54:23 +0200 Subject: [PATCH 02/20] Reduce warmup and repeat MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Running this 1000 times with a full block takes ~33 minutes đŸ™ˆ. Reducing it to ~3 minutes per default. Signed-off-by: Oliver Tale-Yazdi --- utils/frame/benchmarking-cli/src/extrinsic/bench.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/utils/frame/benchmarking-cli/src/extrinsic/bench.rs b/utils/frame/benchmarking-cli/src/extrinsic/bench.rs index 4a81adc398216..32d379b86ef16 100644 --- a/utils/frame/benchmarking-cli/src/extrinsic/bench.rs +++ b/utils/frame/benchmarking-cli/src/extrinsic/bench.rs @@ -43,11 +43,11 @@ use super::ExtrinsicBuilder; #[derive(Debug, Default, Serialize, Clone, PartialEq, Args)] pub struct BenchmarkParams { /// Rounds of warmups before measuring. - #[clap(long, default_value = "100")] + #[clap(long, default_value = "10")] pub warmup: u32, /// How many times the benchmark should be repeated. - #[clap(long, default_value = "1000")] + #[clap(long, default_value = "100")] pub repeat: u32, /// Maximal number of extrinsics that should be put into a block. From 5af2d41479c5ea7d35e5b7904702e836ab0487b1 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Wed, 18 May 2022 17:13:48 +0200 Subject: [PATCH 03/20] Make ExistentialDeposit public Signed-off-by: Oliver Tale-Yazdi --- bin/node-template/runtime/src/lib.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/bin/node-template/runtime/src/lib.rs b/bin/node-template/runtime/src/lib.rs index 0145cacef8f7d..3622fb7eb04fe 100644 --- a/bin/node-template/runtime/src/lib.rs +++ b/bin/node-template/runtime/src/lib.rs @@ -234,6 +234,12 @@ impl pallet_timestamp::Config for Runtime { type WeightInfo = (); } +parameter_types! { + /// This constant can also be written in-place as `ConstU128<500>`. + /// However, in this case its needed in another place as well. + pub const ExistentialDeposit: u128 = 500; +} + impl pallet_balances::Config for Runtime { type MaxLocks = ConstU32<50>; type MaxReserves = (); @@ -243,7 +249,7 @@ impl pallet_balances::Config for Runtime { /// The ubiquitous event type. type Event = Event; type DustRemoval = (); - type ExistentialDeposit = ConstU128<500>; + type ExistentialDeposit = ExistentialDeposit; type AccountStore = System; type WeightInfo = pallet_balances::weights::SubstrateWeight; } From 6d25d105974cad517fc6403f5487453adc036609 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Wed, 18 May 2022 17:14:30 +0200 Subject: [PATCH 04/20] Add 'bechmark extrinsic' command Signed-off-by: Oliver Tale-Yazdi --- .../{command_helper.rs => benchmarking.rs} | 58 ++++++++++++++--- bin/node-template/node/src/command.rs | 26 ++++++-- bin/node-template/node/src/main.rs | 2 +- .../{command_helper.rs => benchmarking.rs} | 62 ++++++++++++++++--- bin/node/cli/src/command.rs | 23 +++++-- bin/node/cli/src/lib.rs | 4 +- .../cli/tests/benchmark_extrinsic_works.rs | 43 +++++++++++++ .../benchmarking-cli/src/extrinsic/bench.rs | 20 +++--- .../benchmarking-cli/src/extrinsic/cmd.rs | 31 +++++++--- .../src/extrinsic/extrinsic_factory.rs | 27 ++++---- .../benchmarking-cli/src/extrinsic/mod.rs | 2 +- utils/frame/benchmarking-cli/src/lib.rs | 5 +- .../benchmarking-cli/src/overhead/cmd.rs | 18 +++--- .../benchmarking-cli/src/overhead/template.rs | 3 +- 14 files changed, 247 insertions(+), 77 deletions(-) rename bin/node-template/node/src/{command_helper.rs => benchmarking.rs} (72%) rename bin/node/cli/src/{command_helper.rs => benchmarking.rs} (53%) create mode 100644 bin/node/cli/tests/benchmark_extrinsic_works.rs diff --git a/bin/node-template/node/src/command_helper.rs b/bin/node-template/node/src/benchmarking.rs similarity index 72% rename from bin/node-template/node/src/command_helper.rs rename to bin/node-template/node/src/benchmarking.rs index 287e81b1e96bd..c32cf9022e8a5 100644 --- a/bin/node-template/node/src/command_helper.rs +++ b/bin/node-template/node/src/benchmarking.rs @@ -16,13 +16,14 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . -//! Contains code to setup the command invocations in [`super::command`] which would -//! otherwise bloat that module. +//! Setup code for [`super::command`] which would otherwise bloat that module. +//! +//! Should only be used for benchmarking as it may break in other contexts. use crate::service::FullClient; use node_template_runtime as runtime; -use runtime::SystemCall; +use runtime::{AccountId, Balance, BalancesCall, SystemCall}; use sc_cli::Result; use sc_client_api::BlockBackend; use sp_core::{Encode, Pair}; @@ -35,19 +36,39 @@ use std::{sync::Arc, time::Duration}; /// Generates extrinsics for the `benchmark overhead` command. /// /// Note: Should only be used for benchmarking. -pub struct BenchmarkExtrinsicBuilder { +pub struct RemarkBuilder { client: Arc, } -impl BenchmarkExtrinsicBuilder { +/// Generates `Balances::TransferKeepAlive` extrinsics for the benchmarks. +/// +/// Note: Should only be used for benchmarking. +pub struct TransferKeepAliveBuilder { + client: Arc, + dest: AccountId, + value: Balance, +} + +impl RemarkBuilder { /// Creates a new [`Self`] from the given client. pub fn new(client: Arc) -> Self { Self { client } } } -impl frame_benchmarking_cli::ExtrinsicBuilder for BenchmarkExtrinsicBuilder { - fn remark(&self, nonce: u32) -> std::result::Result { +impl TransferKeepAliveBuilder { + /// Creates a new [`Self`] from the given client. + pub fn new(client: Arc, dest: AccountId, value: Balance) -> Self { + Self { client, dest, value } + } +} + +impl frame_benchmarking_cli::ExtrinsicBuilder for RemarkBuilder { + fn name(&self) -> (&str, &str) { + ("system", "remark") + } + + fn build(&self, nonce: u32) -> std::result::Result { let acc = Sr25519Keyring::Bob.pair(); let extrinsic: OpaqueExtrinsic = create_benchmark_extrinsic( self.client.as_ref(), @@ -61,6 +82,29 @@ impl frame_benchmarking_cli::ExtrinsicBuilder for BenchmarkExtrinsicBuilder { } } +impl frame_benchmarking_cli::ExtrinsicBuilder for TransferKeepAliveBuilder { + fn name(&self) -> (&str, &str) { + ("balances", "transfer_keep_alive") + } + + fn build(&self, nonce: u32) -> std::result::Result { + let acc = Sr25519Keyring::Bob.pair(); + let extrinsic: OpaqueExtrinsic = create_benchmark_extrinsic( + self.client.as_ref(), + acc, + BalancesCall::transfer_keep_alive { + dest: self.dest.clone().into(), + value: self.value.into(), + } + .into(), + nonce, + ) + .into(); + + Ok(extrinsic) + } +} + /// Create a transaction using the given `call`. /// /// Note: Should only be used for benchmarking. diff --git a/bin/node-template/node/src/command.rs b/bin/node-template/node/src/command.rs index e3e10007929e6..f47a2e38dd8f8 100644 --- a/bin/node-template/node/src/command.rs +++ b/bin/node-template/node/src/command.rs @@ -1,14 +1,14 @@ use crate::{ + benchmarking::{inherent_benchmark_data, RemarkBuilder, TransferKeepAliveBuilder}, chain_spec, cli::{Cli, Subcommand}, - command_helper::{inherent_benchmark_data, BenchmarkExtrinsicBuilder}, service, }; -use frame_benchmarking_cli::{BenchmarkCmd, SUBSTRATE_REFERENCE_HARDWARE}; -use node_template_runtime::Block; +use frame_benchmarking_cli::{BenchmarkCmd, ExtrinsicFactory, SUBSTRATE_REFERENCE_HARDWARE}; +use node_template_runtime::{ExistentialDeposit, Block}; use sc_cli::{ChainSpec, RuntimeVersion, SubstrateCli}; use sc_service::PartialComponents; -use std::sync::Arc; +use sp_keyring::Sr25519Keyring; impl SubstrateCli for Cli { fn impl_name() -> String { @@ -137,9 +137,23 @@ pub fn run() -> sc_cli::Result<()> { }, BenchmarkCmd::Overhead(cmd) => { let PartialComponents { client, .. } = service::new_partial(&config)?; - let ext_builder = BenchmarkExtrinsicBuilder::new(client.clone()); + let ext_builder = RemarkBuilder::new(client.clone()); - cmd.run(config, client, inherent_benchmark_data()?, Arc::new(ext_builder)) + cmd.run(config, client, inherent_benchmark_data()?, &ext_builder) + }, + BenchmarkCmd::Extrinsic(cmd) => { + let PartialComponents { client, .. } = service::new_partial(&config)?; + // Register the *Remark* and *TKA* builders. + let ext_factory = ExtrinsicFactory(vec![ + Box::new(RemarkBuilder::new(client.clone())), + Box::new(TransferKeepAliveBuilder::new( + client.clone(), + Sr25519Keyring::Alice.to_account_id(), + ExistentialDeposit::get(), + )), + ]); + + cmd.run(client, inherent_benchmark_data()?, &ext_factory) }, BenchmarkCmd::Machine(cmd) => cmd.run(&config, SUBSTRATE_REFERENCE_HARDWARE.clone()), diff --git a/bin/node-template/node/src/main.rs b/bin/node-template/node/src/main.rs index 0f2fbd5a909c6..426cbabb6fbf7 100644 --- a/bin/node-template/node/src/main.rs +++ b/bin/node-template/node/src/main.rs @@ -4,9 +4,9 @@ mod chain_spec; #[macro_use] mod service; +mod benchmarking; mod cli; mod command; -mod command_helper; mod rpc; fn main() -> sc_cli::Result<()> { diff --git a/bin/node/cli/src/command_helper.rs b/bin/node/cli/src/benchmarking.rs similarity index 53% rename from bin/node/cli/src/command_helper.rs rename to bin/node/cli/src/benchmarking.rs index 84d85ee367cab..393f60ddb6215 100644 --- a/bin/node/cli/src/command_helper.rs +++ b/bin/node/cli/src/benchmarking.rs @@ -16,12 +16,14 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . -//! Contains code to setup the command invocations in [`super::command`] which would -//! otherwise bloat that module. +//! Setup code for [`super::command`] which would otherwise bloat that module. +//! +//! Should only be used for benchmarking as it may break in other contexts. use crate::service::{create_extrinsic, FullClient}; -use node_runtime::SystemCall; +use node_primitives::{Balance, AccountId}; +use node_runtime::{BalancesCall, SystemCall}; use sc_cli::Result; use sp_inherents::{InherentData, InherentDataProvider}; use sp_keyring::Sr25519Keyring; @@ -29,20 +31,42 @@ use sp_runtime::OpaqueExtrinsic; use std::{sync::Arc, time::Duration}; -/// Generates extrinsics for the `benchmark overhead` command. -pub struct BenchmarkExtrinsicBuilder { +/// Generates `System::Remark` extrinsics for the benchmarks. +/// +/// Note: Should only be used for benchmarking. +pub struct RemarkBuilder { client: Arc, } -impl BenchmarkExtrinsicBuilder { +/// Generates `Balances::TransferKeepAlive` extrinsics for the benchmarks. +/// +/// Note: Should only be used for benchmarking. +pub struct TransferKeepAliveBuilder { + client: Arc, + dest: AccountId, + value: Balance, +} + +impl RemarkBuilder { /// Creates a new [`Self`] from the given client. pub fn new(client: Arc) -> Self { Self { client } } } -impl frame_benchmarking_cli::ExtrinsicBuilder for BenchmarkExtrinsicBuilder { - fn remark(&self, nonce: u32) -> std::result::Result { +impl TransferKeepAliveBuilder { + /// Creates a new [`Self`] from the given client. + pub fn new(client: Arc, dest: AccountId, value: Balance) -> Self { + Self { client, dest, value } + } +} + +impl frame_benchmarking_cli::ExtrinsicBuilder for RemarkBuilder { + fn name(&self) -> (&str, &str) { + ("system", "remark") + } + + fn build(&self, nonce: u32) -> std::result::Result { let acc = Sr25519Keyring::Bob.pair(); let extrinsic: OpaqueExtrinsic = create_extrinsic( self.client.as_ref(), @@ -56,6 +80,28 @@ impl frame_benchmarking_cli::ExtrinsicBuilder for BenchmarkExtrinsicBuilder { } } +impl frame_benchmarking_cli::ExtrinsicBuilder for TransferKeepAliveBuilder { + fn name(&self) -> (&str, &str) { + ("balances", "non_endow_non_reap_transfer_keep_alive") + } + + fn build(&self, nonce: u32) -> std::result::Result { + let acc = Sr25519Keyring::Bob.pair(); + let extrinsic: OpaqueExtrinsic = create_extrinsic( + self.client.as_ref(), + acc, + BalancesCall::transfer_keep_alive { + dest: self.dest.clone().into(), + value: self.value.into(), + }, + Some(nonce), + ) + .into(); + + Ok(extrinsic) + } +} + /// Generates inherent data for the `benchmark overhead` command. pub fn inherent_benchmark_data() -> Result { let mut inherent_data = InherentData::new(); diff --git a/bin/node/cli/src/command.rs b/bin/node/cli/src/command.rs index b17a26fa02935..df3d5a891e2ab 100644 --- a/bin/node/cli/src/command.rs +++ b/bin/node/cli/src/command.rs @@ -16,7 +16,7 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . -use super::command_helper::{inherent_benchmark_data, BenchmarkExtrinsicBuilder}; +use super::benchmarking::{inherent_benchmark_data, RemarkBuilder, TransferKeepAliveBuilder}; use crate::{ chain_spec, service, service::{new_partial, FullClient}, @@ -25,9 +25,10 @@ use crate::{ use frame_benchmarking_cli::*; use node_executor::ExecutorDispatch; use node_primitives::Block; -use node_runtime::RuntimeApi; +use node_runtime::{ExistentialDeposit, RuntimeApi}; use sc_cli::{ChainSpec, Result, RuntimeVersion, SubstrateCli}; use sc_service::PartialComponents; +use sp_keyring::Sr25519Keyring; use std::sync::Arc; @@ -126,9 +127,23 @@ pub fn run() -> Result<()> { }, BenchmarkCmd::Overhead(cmd) => { let PartialComponents { client, .. } = new_partial(&config)?; - let ext_builder = BenchmarkExtrinsicBuilder::new(client.clone()); + let ext_builder = RemarkBuilder::new(client.clone()); - cmd.run(config, client, inherent_benchmark_data()?, Arc::new(ext_builder)) + cmd.run(config, client, inherent_benchmark_data()?, &ext_builder) + }, + BenchmarkCmd::Extrinsic(cmd) => { + let PartialComponents { client, .. } = service::new_partial(&config)?; + // Register the *Remark* and *TKA* builders. + let ext_factory = ExtrinsicFactory(vec![ + Box::new(RemarkBuilder::new(client.clone())), + Box::new(TransferKeepAliveBuilder::new( + client.clone(), + Sr25519Keyring::Alice.to_account_id(), + ExistentialDeposit::get(), + )), + ]); + + cmd.run(client, inherent_benchmark_data()?, &ext_factory) }, BenchmarkCmd::Machine(cmd) => cmd.run(&config, SUBSTRATE_REFERENCE_HARDWARE.clone()), diff --git a/bin/node/cli/src/lib.rs b/bin/node/cli/src/lib.rs index 06c0bcccbc296..13c074268e50f 100644 --- a/bin/node/cli/src/lib.rs +++ b/bin/node/cli/src/lib.rs @@ -35,11 +35,11 @@ pub mod chain_spec; #[macro_use] pub mod service; #[cfg(feature = "cli")] +mod benchmarking; +#[cfg(feature = "cli")] mod cli; #[cfg(feature = "cli")] mod command; -#[cfg(feature = "cli")] -mod command_helper; #[cfg(feature = "cli")] pub use cli::*; diff --git a/bin/node/cli/tests/benchmark_extrinsic_works.rs b/bin/node/cli/tests/benchmark_extrinsic_works.rs new file mode 100644 index 0000000000000..fd60ed1f31d1e --- /dev/null +++ b/bin/node/cli/tests/benchmark_extrinsic_works.rs @@ -0,0 +1,43 @@ +// This file is part of Substrate. + +// Copyright (C) 2022 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: GPL-3.0-or-later WITH Classpath-exception-2.0 + +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . + +use assert_cmd::cargo::cargo_bin; +use std::process::Command; +use std::path::Path; +use tempfile::tempdir; + +/// Tests that the `benchmark extrinsic` command works for +/// remark and transfer_keep_alive within the substrate dev runtime. +#[test] +fn benchmark_extrinsic_works() { + benchmark_extrinsic("system", "remark"); + benchmark_extrinsic("balances", "transfer_keep_alive"); +} + +/// Checks that the `benchmark extrinsic` command works for the given pallet and extrinsic. +fn benchmark_extrinsic(pallet: &str, extrinsic: &str) { + let status = Command::new(cargo_bin("substrate")) + .args(&["benchmark", "extrinsic", "--dev"]) + .args([pallet, extrinsic]) + // Run with low repeats for faster execution. + .args(["--warmup=10", "--repeat=10", "--max-ext-per-block=10"]) + .status() + .unwrap(); + + assert!(status.success()); +} diff --git a/utils/frame/benchmarking-cli/src/extrinsic/bench.rs b/utils/frame/benchmarking-cli/src/extrinsic/bench.rs index 32d379b86ef16..e3f3c5318fa66 100644 --- a/utils/frame/benchmarking-cli/src/extrinsic/bench.rs +++ b/utils/frame/benchmarking-cli/src/extrinsic/bench.rs @@ -36,8 +36,8 @@ use log::info; use serde::Serialize; use std::{marker::PhantomData, sync::Arc, time::Instant}; -use crate::shared::Stats; use super::ExtrinsicBuilder; +use crate::shared::Stats; /// Parameters to configure an *overhead* benchmark. #[derive(Debug, Default, Serialize, Clone, PartialEq, Args)] @@ -91,11 +91,15 @@ where Stats::new(&record) } - /// Builds a block for the given benchmark type. + /// Builds a block which with some optional extrinsics. /// /// Returns the block and the number of extrinsics in the block /// that are not inherents. - fn build_block(&self, ext_builder: Option<&dyn ExtrinsicBuilder>) -> Result<(Block, Option)> { + /// Returns a block with only inherents if `ext_builder` is `None`. + fn build_block( + &self, + ext_builder: Option<&dyn ExtrinsicBuilder>, + ) -> Result<(Block, Option)> { let mut builder = self.client.new_block(Default::default())?; // Create and insert the inherents. let inherents = builder.create_inherents(self.inherent_data.clone())?; @@ -103,10 +107,10 @@ where builder.push(inherent)?; } + // Return early if `ext_builder` is `None`. let ext_builder = if let Some(ext_builder) = ext_builder { ext_builder } else { - // Return early if we just want a block with inherents and no additional extrinsics. return Ok((builder.build()?.block, None)) }; @@ -134,14 +138,10 @@ where } /// Measures the time that it take to execute a block or an extrinsic. - fn measure_block( - &self, - block: &Block, - num_ext: Option, - ) -> Result { + fn measure_block(&self, block: &Block, num_ext: Option) -> Result { let mut record = BenchRecord::new(); if num_ext == Some(0) { - return Err("Cannot divide by zero".into()) + return Err("Num extrinsics was zero".into()) } let genesis = BlockId::Number(Zero::zero()); diff --git a/utils/frame/benchmarking-cli/src/extrinsic/cmd.rs b/utils/frame/benchmarking-cli/src/extrinsic/cmd.rs index 8417a0f1a0498..b49fbdb5c4959 100644 --- a/utils/frame/benchmarking-cli/src/extrinsic/cmd.rs +++ b/utils/frame/benchmarking-cli/src/extrinsic/cmd.rs @@ -15,10 +15,8 @@ // See the License for the specific language governing permissions and // limitations under the License. -use sc_cli::CliConfiguration; -use sc_cli::{ImportParams, SharedParams}; use sc_block_builder::{BlockBuilderApi, BlockBuilderProvider}; -use sc_cli::{Result}; +use sc_cli::{CliConfiguration, ImportParams, Result, SharedParams}; use sc_client_api::Backend as ClientBackend; use sp_api::{ApiExt, ProvideRuntimeApi}; use sp_runtime::{traits::Block as BlockT, OpaqueExtrinsic}; @@ -28,10 +26,18 @@ use log::info; use serde::Serialize; use std::{fmt::Debug, sync::Arc}; -use super::bench::Benchmark; -use super::bench::BenchmarkParams; -use super::extrinsic_factory::ExtrinsicFactory; - +use super::{ + bench::{Benchmark, BenchmarkParams}, + extrinsic_factory::ExtrinsicFactory, +}; + +/// Benchmark the execution time of different extrinsics. +/// +/// This is calculated by filling a block with a specific extrinsic and executing the block. +/// The result time is then divided by the number of extrinsics in that block. +/// +/// NOTE: The [`frame_support::BlockExecutionWeight`] is ignored +/// in this case since it is very small compared to the total block execution time. #[derive(Debug, Parser)] pub struct ExtrinsicCmd { #[allow(missing_docs)] @@ -47,21 +53,25 @@ pub struct ExtrinsicCmd { pub params: ExtrinsicParams, } +/// The params for the [`ExtrinsicCmd`]. #[derive(Debug, Default, Serialize, Clone, PartialEq, Args)] pub struct ExtrinsicParams { #[clap(flatten)] pub bench: BenchmarkParams, /// Pallet name of the extrinsic. - #[clap(short, long, value_name = "PALLET", index = 1)] + #[clap(value_name = "PALLET", index = 1)] pub pallet: String, /// Extrinsic name. - #[clap(short, long, value_name = "EXTRINSIC", index = 2)] + #[clap(value_name = "EXTRINSIC", index = 2)] pub extrinsic: String, } impl ExtrinsicCmd { + /// Benchmark the execution time of a specific type of extrinsic. + /// + /// The output will be printed to console. pub fn run( &self, client: Arc, @@ -74,7 +84,8 @@ impl ExtrinsicCmd { C: BlockBuilderProvider + ProvideRuntimeApi, C::Api: ApiExt + BlockBuilderApi, { - let ext_builder = ext_factory.try_get(&self.params.pallet, &self.params.extrinsic).expect("TODO"); + let ext_builder = + ext_factory.try_get(&self.params.pallet, &self.params.extrinsic).expect("TODO"); let bench = Benchmark::new(client, self.params.bench.clone(), inherent_data); let stats = bench.bench(Some(ext_builder))?; diff --git a/utils/frame/benchmarking-cli/src/extrinsic/extrinsic_factory.rs b/utils/frame/benchmarking-cli/src/extrinsic/extrinsic_factory.rs index d1a3e10bf97a7..18f0a3192d8f2 100644 --- a/utils/frame/benchmarking-cli/src/extrinsic/extrinsic_factory.rs +++ b/utils/frame/benchmarking-cli/src/extrinsic/extrinsic_factory.rs @@ -16,28 +16,22 @@ // limitations under the License. //! Provides the [`ExtrinsicFactory`] and the [`ExtrinsicBuilder`] types. -//! Is used by the *overhead* and *extrinsic* benchmarking to build extrinsics. +//! Is used by the *overhead* and *extrinsic* benchmarks to build extrinsics. use sp_runtime::OpaqueExtrinsic; -#[derive(Default)] /// Helper to manage [`ExtrinsicBuilder`] instances. +#[derive(Default)] pub struct ExtrinsicFactory(pub Vec>); impl ExtrinsicFactory { - /// Adds a builder to the list and consumes [`Self`]. - pub fn with_builder(mut self, builder: Box) -> Self { - self.0.push(builder); - self - } - - /// Returns an optional builder for an extrinsic. + /// Returns a builder for an extrinsic. + /// + /// Is case in-sensitive. pub fn try_get(&self, pallet: &str, extrinsic: &str) -> Option<&dyn ExtrinsicBuilder> { self.0 .iter() - .find(|b| { - b.pallet() == pallet.to_lowercase() && b.extrinsic() == extrinsic.to_lowercase() - }) + .find(|b| b.name() == (&pallet.to_lowercase(), &extrinsic.to_lowercase())) .map(|b| b.as_ref()) } @@ -52,11 +46,10 @@ impl ExtrinsicFactory { /// The built extrinsics only need to be valid in the first block /// who's parent block is the genesis block. /// This assumption simplifies the generation of the extrinsics. +/// The signer should be one of the pre-funded dev accounts. pub trait ExtrinsicBuilder { /// Pallet of the extrinsic. - fn pallet(&self) -> &'static str; - /// Name of the extrinsic. - fn extrinsic(&self) -> &'static str; + fn name(&self /* TODO self not needed… */) -> (&str, &str); /// Builds an extrinsic. /// @@ -65,7 +58,9 @@ pub trait ExtrinsicBuilder { } impl std::fmt::Display for &dyn ExtrinsicBuilder { + /// Formats the builder in the standard `Pallet::Extrinsic` scheme. fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { - write!(f, "{}::{}", self.pallet().to_uppercase(), self.extrinsic().to_uppercase()) + let (pallet, extrinsic) = self.name(); + write!(f, "{}::{}", pallet.to_uppercase(), extrinsic.to_uppercase()) } } diff --git a/utils/frame/benchmarking-cli/src/extrinsic/mod.rs b/utils/frame/benchmarking-cli/src/extrinsic/mod.rs index 2dc9179729b14..12a09c3b1af63 100644 --- a/utils/frame/benchmarking-cli/src/extrinsic/mod.rs +++ b/utils/frame/benchmarking-cli/src/extrinsic/mod.rs @@ -19,9 +19,9 @@ //! This is a generalization of the *overhead* benchmark which can only measure `System::Remark` //! extrinsics. +pub mod bench; pub mod cmd; pub mod extrinsic_factory; -pub mod bench; pub use cmd::ExtrinsicCmd; pub use extrinsic_factory::{ExtrinsicBuilder, ExtrinsicFactory}; diff --git a/utils/frame/benchmarking-cli/src/lib.rs b/utils/frame/benchmarking-cli/src/lib.rs index 301a64a5f0443..96c7a997895d4 100644 --- a/utils/frame/benchmarking-cli/src/lib.rs +++ b/utils/frame/benchmarking-cli/src/lib.rs @@ -26,7 +26,7 @@ mod shared; mod storage; pub use block::BlockCmd; -pub use extrinsic::ExtrinsicCmd; +pub use extrinsic::{ExtrinsicBuilder, ExtrinsicCmd, ExtrinsicFactory}; pub use machine::{MachineCmd, Requirements, SUBSTRATE_REFERENCE_HARDWARE}; pub use overhead::OverheadCmd; pub use pallet::PalletCmd; @@ -43,8 +43,8 @@ pub enum BenchmarkCmd { Storage(StorageCmd), Overhead(OverheadCmd), Block(BlockCmd), - #[clap(hide = true)] // Hidden until fully completed. Machine(MachineCmd), + Extrinsic(ExtrinsicCmd), } /// Unwraps a [`BenchmarkCmd`] into its concrete sub-command. @@ -60,6 +60,7 @@ macro_rules! unwrap_cmd { BenchmarkCmd::Overhead($cmd) => $code, BenchmarkCmd::Block($cmd) => $code, BenchmarkCmd::Machine($cmd) => $code, + BenchmarkCmd::Extrinsic($cmd) => $code, } } } diff --git a/utils/frame/benchmarking-cli/src/overhead/cmd.rs b/utils/frame/benchmarking-cli/src/overhead/cmd.rs index 69aaae48866f8..e5a53eead5fbe 100644 --- a/utils/frame/benchmarking-cli/src/overhead/cmd.rs +++ b/utils/frame/benchmarking-cli/src/overhead/cmd.rs @@ -31,12 +31,12 @@ use serde::Serialize; use std::{fmt::Debug, sync::Arc}; use crate::{ - extrinsic::bench::{Benchmark, BenchmarkParams}, - overhead::{ - template::TemplateData, + extrinsic::{ + bench::{Benchmark, BenchmarkParams as ExtrinsicBenchmarkParams}, + ExtrinsicBuilder, }, - extrinsic::ExtrinsicFactory, - shared::{ WeightParams}, + overhead::template::TemplateData, + shared::WeightParams, }; /// Benchmark the execution overhead per-block and per-extrinsic. @@ -64,7 +64,7 @@ pub struct OverheadParams { #[allow(missing_docs)] #[clap(flatten)] - pub bench: BenchmarkParams, + pub bench: ExtrinsicBenchmarkParams, } /// Type of a benchmark. @@ -86,7 +86,7 @@ impl OverheadCmd { cfg: Configuration, client: Arc, inherent_data: sp_inherents::InherentData, - ext_factory: &ExtrinsicFactory, + ext_builder: &dyn ExtrinsicBuilder, ) -> Result<()> where Block: BlockT, @@ -94,7 +94,9 @@ impl OverheadCmd { C: BlockBuilderProvider + ProvideRuntimeApi, C::Api: ApiExt + BlockBuilderApi, { - let ext_builder = ext_factory.try_get("system", "remark").expect("TODO"); + if ext_builder.name() != ("system", "remark") { + panic!("The extrinsic builder is required to build `System::Remark` extrinsics instead of {}", &ext_builder); + } let bench = Benchmark::new(client, self.params.bench.clone(), inherent_data); // per-block execution overhead diff --git a/utils/frame/benchmarking-cli/src/overhead/template.rs b/utils/frame/benchmarking-cli/src/overhead/template.rs index 70aee3d12fc8b..b2964032b6a13 100644 --- a/utils/frame/benchmarking-cli/src/overhead/template.rs +++ b/utils/frame/benchmarking-cli/src/overhead/template.rs @@ -27,8 +27,7 @@ use serde::Serialize; use std::{env, fs, path::PathBuf}; use crate::{ - overhead::cmd::BenchmarkType, - overhead::{cmd::OverheadParams}, + overhead::cmd::{BenchmarkType, OverheadParams}, shared::{Stats, UnderscoreHelper}, }; From b1d07ee182da4d5b207aa78801c84d5c0cc9192b Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Wed, 18 May 2022 17:14:43 +0200 Subject: [PATCH 05/20] fmt Signed-off-by: Oliver Tale-Yazdi --- bin/node-template/node/src/command.rs | 2 +- bin/node/cli/src/benchmarking.rs | 2 +- bin/node/cli/tests/benchmark_extrinsic_works.rs | 3 +-- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/bin/node-template/node/src/command.rs b/bin/node-template/node/src/command.rs index f47a2e38dd8f8..43b6b67a2da81 100644 --- a/bin/node-template/node/src/command.rs +++ b/bin/node-template/node/src/command.rs @@ -5,7 +5,7 @@ use crate::{ service, }; use frame_benchmarking_cli::{BenchmarkCmd, ExtrinsicFactory, SUBSTRATE_REFERENCE_HARDWARE}; -use node_template_runtime::{ExistentialDeposit, Block}; +use node_template_runtime::{Block, ExistentialDeposit}; use sc_cli::{ChainSpec, RuntimeVersion, SubstrateCli}; use sc_service::PartialComponents; use sp_keyring::Sr25519Keyring; diff --git a/bin/node/cli/src/benchmarking.rs b/bin/node/cli/src/benchmarking.rs index 393f60ddb6215..384410050585e 100644 --- a/bin/node/cli/src/benchmarking.rs +++ b/bin/node/cli/src/benchmarking.rs @@ -22,7 +22,7 @@ use crate::service::{create_extrinsic, FullClient}; -use node_primitives::{Balance, AccountId}; +use node_primitives::{AccountId, Balance}; use node_runtime::{BalancesCall, SystemCall}; use sc_cli::Result; use sp_inherents::{InherentData, InherentDataProvider}; diff --git a/bin/node/cli/tests/benchmark_extrinsic_works.rs b/bin/node/cli/tests/benchmark_extrinsic_works.rs index fd60ed1f31d1e..f2c3df9ad1560 100644 --- a/bin/node/cli/tests/benchmark_extrinsic_works.rs +++ b/bin/node/cli/tests/benchmark_extrinsic_works.rs @@ -17,8 +17,7 @@ // along with this program. If not, see . use assert_cmd::cargo::cargo_bin; -use std::process::Command; -use std::path::Path; +use std::{path::Path, process::Command}; use tempfile::tempdir; /// Tests that the `benchmark extrinsic` command works for From d8e2cdc5347aa688561b12d50cf8a579d2c804b2 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Tue, 31 May 2022 20:11:08 +0200 Subject: [PATCH 06/20] Add --list and cleanup Signed-off-by: Oliver Tale-Yazdi --- bin/node-template/node/src/benchmarking.rs | 16 +++++-- bin/node/cli/src/benchmarking.rs | 16 +++++-- .../benchmarking-cli/src/extrinsic/cmd.rs | 47 +++++++++++++++---- .../src/extrinsic/extrinsic_factory.rs | 31 ++++++------ .../benchmarking-cli/src/overhead/cmd.rs | 4 +- 5 files changed, 80 insertions(+), 34 deletions(-) diff --git a/bin/node-template/node/src/benchmarking.rs b/bin/node-template/node/src/benchmarking.rs index c32cf9022e8a5..9954e805d7534 100644 --- a/bin/node-template/node/src/benchmarking.rs +++ b/bin/node-template/node/src/benchmarking.rs @@ -64,8 +64,12 @@ impl TransferKeepAliveBuilder { } impl frame_benchmarking_cli::ExtrinsicBuilder for RemarkBuilder { - fn name(&self) -> (&str, &str) { - ("system", "remark") + fn pallet(&self) -> &str { + "system" + } + + fn extrinsic(&self) -> &str { + "remark" } fn build(&self, nonce: u32) -> std::result::Result { @@ -83,8 +87,12 @@ impl frame_benchmarking_cli::ExtrinsicBuilder for RemarkBuilder { } impl frame_benchmarking_cli::ExtrinsicBuilder for TransferKeepAliveBuilder { - fn name(&self) -> (&str, &str) { - ("balances", "transfer_keep_alive") + fn pallet(&self) -> &str { + "balances" + } + + fn extrinsic(&self) -> &str { + "transfer_keep_alive" } fn build(&self, nonce: u32) -> std::result::Result { diff --git a/bin/node/cli/src/benchmarking.rs b/bin/node/cli/src/benchmarking.rs index 384410050585e..2fe54e34207e3 100644 --- a/bin/node/cli/src/benchmarking.rs +++ b/bin/node/cli/src/benchmarking.rs @@ -62,8 +62,12 @@ impl TransferKeepAliveBuilder { } impl frame_benchmarking_cli::ExtrinsicBuilder for RemarkBuilder { - fn name(&self) -> (&str, &str) { - ("system", "remark") + fn pallet(&self) -> &str { + "system" + } + + fn extrinsic(&self) -> &str { + "remark" } fn build(&self, nonce: u32) -> std::result::Result { @@ -81,8 +85,12 @@ impl frame_benchmarking_cli::ExtrinsicBuilder for RemarkBuilder { } impl frame_benchmarking_cli::ExtrinsicBuilder for TransferKeepAliveBuilder { - fn name(&self) -> (&str, &str) { - ("balances", "non_endow_non_reap_transfer_keep_alive") + fn pallet(&self) -> &str { + "balances" + } + + fn extrinsic(&self) -> &str { + "transfer_keep_alive" } fn build(&self, nonce: u32) -> std::result::Result { diff --git a/utils/frame/benchmarking-cli/src/extrinsic/cmd.rs b/utils/frame/benchmarking-cli/src/extrinsic/cmd.rs index b49fbdb5c4959..6941b2e2e7790 100644 --- a/utils/frame/benchmarking-cli/src/extrinsic/cmd.rs +++ b/utils/frame/benchmarking-cli/src/extrinsic/cmd.rs @@ -59,13 +59,19 @@ pub struct ExtrinsicParams { #[clap(flatten)] pub bench: BenchmarkParams, - /// Pallet name of the extrinsic. - #[clap(value_name = "PALLET", index = 1)] - pub pallet: String, + /// List all available pallets and extrinsics. + /// + /// The format is CSV with header `pallet, extrinsic`. + #[clap(long)] + pub list: bool, + + /// Pallet name of the extrinsic to benchmark. + #[clap(long, value_name = "PALLET", required_unless_present = "list")] + pub pallet: Option, - /// Extrinsic name. - #[clap(value_name = "EXTRINSIC", index = 2)] - pub extrinsic: String, + /// Extrinsic to benchmark. + #[clap(long, value_name = "EXTRINSIC", required_unless_present = "list")] + pub extrinsic: Option, } impl ExtrinsicCmd { @@ -84,12 +90,33 @@ impl ExtrinsicCmd { C: BlockBuilderProvider + ProvideRuntimeApi, C::Api: ApiExt + BlockBuilderApi, { - let ext_builder = - ext_factory.try_get(&self.params.pallet, &self.params.extrinsic).expect("TODO"); - let bench = Benchmark::new(client, self.params.bench.clone(), inherent_data); + // Short circuit if --list was specified. + if self.params.list { + let list: Vec = ext_factory.0.iter().map(|b| b.name()).collect(); + info!( + "Listing available extrinsics ({}):\npallet, extrinsic\n{}", + list.len(), + list.join("\n") + ); + return Ok(()) + } + + let pallet = self.params.pallet.clone().unwrap_or_default(); + let extrinsic = self.params.extrinsic.clone().unwrap_or_default(); + let ext_builder = match ext_factory.try_get(&pallet, &extrinsic) { + Some(ext_builder) => ext_builder, + None => + return Err("Unknown pallet or extrinsic. Use --list for a complete list.".into()), + }; + let bench = Benchmark::new(client, self.params.bench.clone(), inherent_data); let stats = bench.bench(Some(ext_builder))?; - info!("Executing a {} extrinsic takes[ns]:\n{:?}", &ext_builder, stats); + info!( + "Executing a {}::{} extrinsic takes[ns]:\n{:?}", + ext_builder.pallet(), + ext_builder.extrinsic(), + stats + ); Ok(()) } diff --git a/utils/frame/benchmarking-cli/src/extrinsic/extrinsic_factory.rs b/utils/frame/benchmarking-cli/src/extrinsic/extrinsic_factory.rs index 18f0a3192d8f2..0e89aac15ea60 100644 --- a/utils/frame/benchmarking-cli/src/extrinsic/extrinsic_factory.rs +++ b/utils/frame/benchmarking-cli/src/extrinsic/extrinsic_factory.rs @@ -25,20 +25,18 @@ use sp_runtime::OpaqueExtrinsic; pub struct ExtrinsicFactory(pub Vec>); impl ExtrinsicFactory { - /// Returns a builder for an extrinsic. + /// Returns a builder for a pallet and extrinsic name. /// /// Is case in-sensitive. pub fn try_get(&self, pallet: &str, extrinsic: &str) -> Option<&dyn ExtrinsicBuilder> { self.0 .iter() - .find(|b| b.name() == (&pallet.to_lowercase(), &extrinsic.to_lowercase())) + .find(|b| { + b.pallet().to_lowercase() == pallet.to_lowercase() && + b.extrinsic().to_lowercase() == extrinsic.to_lowercase() + }) .map(|b| b.as_ref()) } - - /// Formats the builders in the standard `Pallet::Extrinsic` scheme. - pub fn as_str_vec(&self) -> Vec { - self.0.iter().map(|b| format!("{}", b.as_ref())).collect() - } } /// Used by the benchmark to build signed extrinsics. @@ -48,8 +46,14 @@ impl ExtrinsicFactory { /// This assumption simplifies the generation of the extrinsics. /// The signer should be one of the pre-funded dev accounts. pub trait ExtrinsicBuilder { - /// Pallet of the extrinsic. - fn name(&self /* TODO self not needed… */) -> (&str, &str); + /// Name of the pallet this builder is for. + /// + /// Should be all lowercase. + fn pallet(&self) -> &str; + /// Name of the extrinsic this builder is for. + /// + /// Should be all lowercase. + fn extrinsic(&self) -> &str; /// Builds an extrinsic. /// @@ -57,10 +61,9 @@ pub trait ExtrinsicBuilder { fn build(&self, nonce: u32) -> std::result::Result; } -impl std::fmt::Display for &dyn ExtrinsicBuilder { - /// Formats the builder in the standard `Pallet::Extrinsic` scheme. - fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { - let (pallet, extrinsic) = self.name(); - write!(f, "{}::{}", pallet.to_uppercase(), extrinsic.to_uppercase()) +impl dyn ExtrinsicBuilder + '_ { + /// Name of this builder in CSV format `pallet, extrinsic`. + pub fn name(&self) -> String { + format!("{}, {}", self.pallet(), self.extrinsic()) } } diff --git a/utils/frame/benchmarking-cli/src/overhead/cmd.rs b/utils/frame/benchmarking-cli/src/overhead/cmd.rs index e5a53eead5fbe..4d1934b48705e 100644 --- a/utils/frame/benchmarking-cli/src/overhead/cmd.rs +++ b/utils/frame/benchmarking-cli/src/overhead/cmd.rs @@ -94,8 +94,8 @@ impl OverheadCmd { C: BlockBuilderProvider + ProvideRuntimeApi, C::Api: ApiExt + BlockBuilderApi, { - if ext_builder.name() != ("system", "remark") { - panic!("The extrinsic builder is required to build `System::Remark` extrinsics instead of {}", &ext_builder); + if ext_builder.pallet() != "system" || ext_builder.extrinsic() != "remark" { + return Err(format!("The extrinsic builder is required to build `System::Remark` extrinsics but builds `{}` extrinsics instead", ext_builder.name()).into()); } let bench = Benchmark::new(client, self.params.bench.clone(), inherent_data); From 4ecc0d3e51f74c79f071d3d80308ad9a877bf771 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Wed, 1 Jun 2022 15:00:37 +0200 Subject: [PATCH 07/20] Unrelated Clippy Signed-off-by: Oliver Tale-Yazdi --- frame/ranked-collective/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/ranked-collective/src/lib.rs b/frame/ranked-collective/src/lib.rs index 521dfb6aad4e3..61686f1efeb2f 100644 --- a/frame/ranked-collective/src/lib.rs +++ b/frame/ranked-collective/src/lib.rs @@ -444,7 +444,7 @@ pub mod pallet { MemberCount::::insert(rank, index.checked_add(1).ok_or(Overflow)?); IdToIndex::::insert(rank, &who, index); IndexToId::::insert(rank, index, &who); - Members::::insert(&who, MemberRecord { rank, ..record }); + Members::::insert(&who, MemberRecord { rank }); Self::deposit_event(Event::RankChanged { who, rank }); Ok(()) From 603d0ae2ac12c2a8b673ef594356de28b66b47f8 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Wed, 1 Jun 2022 15:11:53 +0200 Subject: [PATCH 08/20] Clippy Signed-off-by: Oliver Tale-Yazdi --- bin/node/cli/tests/benchmark_extrinsic_works.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/bin/node/cli/tests/benchmark_extrinsic_works.rs b/bin/node/cli/tests/benchmark_extrinsic_works.rs index f2c3df9ad1560..1d3a393e80b48 100644 --- a/bin/node/cli/tests/benchmark_extrinsic_works.rs +++ b/bin/node/cli/tests/benchmark_extrinsic_works.rs @@ -17,8 +17,7 @@ // along with this program. If not, see . use assert_cmd::cargo::cargo_bin; -use std::{path::Path, process::Command}; -use tempfile::tempdir; +use std::process::Command; /// Tests that the `benchmark extrinsic` command works for /// remark and transfer_keep_alive within the substrate dev runtime. From a336902215bbca9ebd5a80312ab6e12fd7837b18 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Wed, 1 Jun 2022 15:53:52 +0200 Subject: [PATCH 09/20] Fix tests and doc Signed-off-by: Oliver Tale-Yazdi --- bin/node/cli/tests/benchmark_extrinsic_works.rs | 2 +- utils/frame/benchmarking-cli/src/extrinsic/cmd.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/bin/node/cli/tests/benchmark_extrinsic_works.rs b/bin/node/cli/tests/benchmark_extrinsic_works.rs index 1d3a393e80b48..7abe54287b02f 100644 --- a/bin/node/cli/tests/benchmark_extrinsic_works.rs +++ b/bin/node/cli/tests/benchmark_extrinsic_works.rs @@ -31,7 +31,7 @@ fn benchmark_extrinsic_works() { fn benchmark_extrinsic(pallet: &str, extrinsic: &str) { let status = Command::new(cargo_bin("substrate")) .args(&["benchmark", "extrinsic", "--dev"]) - .args([pallet, extrinsic]) + .args(&["--pallet", pallet, "--extrinsic", extrinsic]) // Run with low repeats for faster execution. .args(["--warmup=10", "--repeat=10", "--max-ext-per-block=10"]) .status() diff --git a/utils/frame/benchmarking-cli/src/extrinsic/cmd.rs b/utils/frame/benchmarking-cli/src/extrinsic/cmd.rs index 6941b2e2e7790..4ce92c3467514 100644 --- a/utils/frame/benchmarking-cli/src/extrinsic/cmd.rs +++ b/utils/frame/benchmarking-cli/src/extrinsic/cmd.rs @@ -36,8 +36,8 @@ use super::{ /// This is calculated by filling a block with a specific extrinsic and executing the block. /// The result time is then divided by the number of extrinsics in that block. /// -/// NOTE: The [`frame_support::BlockExecutionWeight`] is ignored -/// in this case since it is very small compared to the total block execution time. +/// NOTE: The BlockExecutionWeight is ignored in this case since it +// is very small compared to the total block execution time. #[derive(Debug, Parser)] pub struct ExtrinsicCmd { #[allow(missing_docs)] From 5efa7c17db5917b114a0f673b94c1d005466534f Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Fri, 17 Jun 2022 15:08:24 +0200 Subject: [PATCH 10/20] Move implementations up + fmt Signed-off-by: Oliver Tale-Yazdi --- bin/node-template/node/src/benchmarking.rs | 32 +++---- bin/node/cli/src/benchmarking.rs | 32 +++---- .../basic-authorship/src/basic_authorship.rs | 84 +++++++++++-------- 3 files changed, 80 insertions(+), 68 deletions(-) diff --git a/bin/node-template/node/src/benchmarking.rs b/bin/node-template/node/src/benchmarking.rs index 9954e805d7534..f0e32104cd3ee 100644 --- a/bin/node-template/node/src/benchmarking.rs +++ b/bin/node-template/node/src/benchmarking.rs @@ -40,15 +40,6 @@ pub struct RemarkBuilder { client: Arc, } -/// Generates `Balances::TransferKeepAlive` extrinsics for the benchmarks. -/// -/// Note: Should only be used for benchmarking. -pub struct TransferKeepAliveBuilder { - client: Arc, - dest: AccountId, - value: Balance, -} - impl RemarkBuilder { /// Creates a new [`Self`] from the given client. pub fn new(client: Arc) -> Self { @@ -56,13 +47,6 @@ impl RemarkBuilder { } } -impl TransferKeepAliveBuilder { - /// Creates a new [`Self`] from the given client. - pub fn new(client: Arc, dest: AccountId, value: Balance) -> Self { - Self { client, dest, value } - } -} - impl frame_benchmarking_cli::ExtrinsicBuilder for RemarkBuilder { fn pallet(&self) -> &str { "system" @@ -86,6 +70,22 @@ impl frame_benchmarking_cli::ExtrinsicBuilder for RemarkBuilder { } } +/// Generates `Balances::TransferKeepAlive` extrinsics for the benchmarks. +/// +/// Note: Should only be used for benchmarking. +pub struct TransferKeepAliveBuilder { + client: Arc, + dest: AccountId, + value: Balance, +} + +impl TransferKeepAliveBuilder { + /// Creates a new [`Self`] from the given client. + pub fn new(client: Arc, dest: AccountId, value: Balance) -> Self { + Self { client, dest, value } + } +} + impl frame_benchmarking_cli::ExtrinsicBuilder for TransferKeepAliveBuilder { fn pallet(&self) -> &str { "balances" diff --git a/bin/node/cli/src/benchmarking.rs b/bin/node/cli/src/benchmarking.rs index 2fe54e34207e3..52657016c046a 100644 --- a/bin/node/cli/src/benchmarking.rs +++ b/bin/node/cli/src/benchmarking.rs @@ -38,15 +38,6 @@ pub struct RemarkBuilder { client: Arc, } -/// Generates `Balances::TransferKeepAlive` extrinsics for the benchmarks. -/// -/// Note: Should only be used for benchmarking. -pub struct TransferKeepAliveBuilder { - client: Arc, - dest: AccountId, - value: Balance, -} - impl RemarkBuilder { /// Creates a new [`Self`] from the given client. pub fn new(client: Arc) -> Self { @@ -54,13 +45,6 @@ impl RemarkBuilder { } } -impl TransferKeepAliveBuilder { - /// Creates a new [`Self`] from the given client. - pub fn new(client: Arc, dest: AccountId, value: Balance) -> Self { - Self { client, dest, value } - } -} - impl frame_benchmarking_cli::ExtrinsicBuilder for RemarkBuilder { fn pallet(&self) -> &str { "system" @@ -84,6 +68,22 @@ impl frame_benchmarking_cli::ExtrinsicBuilder for RemarkBuilder { } } +/// Generates `Balances::TransferKeepAlive` extrinsics for the benchmarks. +/// +/// Note: Should only be used for benchmarking. +pub struct TransferKeepAliveBuilder { + client: Arc, + dest: AccountId, + value: Balance, +} + +impl TransferKeepAliveBuilder { + /// Creates a new [`Self`] from the given client. + pub fn new(client: Arc, dest: AccountId, value: Balance) -> Self { + Self { client, dest, value } + } +} + impl frame_benchmarking_cli::ExtrinsicBuilder for TransferKeepAliveBuilder { fn pallet(&self) -> &str { "balances" diff --git a/client/basic-authorship/src/basic_authorship.rs b/client/basic-authorship/src/basic_authorship.rs index 5a020ee81050e..57418266373c5 100644 --- a/client/basic-authorship/src/basic_authorship.rs +++ b/client/basic-authorship/src/basic_authorship.rs @@ -629,12 +629,14 @@ mod tests { .unwrap(); block_on( - txpool.maintain(chain_event( - client - .header(&BlockId::Number(0u64)) - .expect("header get error") - .expect("there should be header"), - )), + txpool.maintain( + chain_event( + client + .header(&BlockId::Number(0u64)) + .expect("header get error") + .expect("there should be header"), + ), + ), ); let mut proposer_factory = @@ -724,12 +726,14 @@ mod tests { block_on(txpool.submit_at(&BlockId::number(0), SOURCE, vec![extrinsic(0)])).unwrap(); block_on( - txpool.maintain(chain_event( - client - .header(&BlockId::Number(0u64)) - .expect("header get error") - .expect("there should be header"), - )), + txpool.maintain( + chain_event( + client + .header(&BlockId::Number(0u64)) + .expect("header get error") + .expect("there should be header"), + ), + ), ); let mut proposer_factory = @@ -825,12 +829,14 @@ mod tests { }; block_on( - txpool.maintain(chain_event( - client - .header(&BlockId::Number(0u64)) - .expect("header get error") - .expect("there should be header"), - )), + txpool.maintain( + chain_event( + client + .header(&BlockId::Number(0u64)) + .expect("header get error") + .expect("there should be header"), + ), + ), ); assert_eq!(txpool.ready().count(), 7); @@ -839,12 +845,14 @@ mod tests { block_on(client.import(BlockOrigin::Own, block)).unwrap(); block_on( - txpool.maintain(chain_event( - client - .header(&BlockId::Number(1)) - .expect("header get error") - .expect("there should be header"), - )), + txpool.maintain( + chain_event( + client + .header(&BlockId::Number(1)) + .expect("header get error") + .expect("there should be header"), + ), + ), ); assert_eq!(txpool.ready().count(), 5); @@ -969,12 +977,14 @@ mod tests { .unwrap(); block_on( - txpool.maintain(chain_event( - client - .header(&BlockId::Number(0u64)) - .expect("header get error") - .expect("there should be header"), - )), + txpool.maintain( + chain_event( + client + .header(&BlockId::Number(0u64)) + .expect("header get error") + .expect("there should be header"), + ), + ), ); assert_eq!(txpool.ready().count(), MAX_SKIPPED_TRANSACTIONS * 3); @@ -1032,12 +1042,14 @@ mod tests { .unwrap(); block_on( - txpool.maintain(chain_event( - client - .header(&BlockId::Number(0u64)) - .expect("header get error") - .expect("there should be header"), - )), + txpool.maintain( + chain_event( + client + .header(&BlockId::Number(0u64)) + .expect("header get error") + .expect("there should be header"), + ), + ), ); assert_eq!(txpool.ready().count(), MAX_SKIPPED_TRANSACTIONS * 2 + 2); From fba7975d49b5dd265c5f439a0e1ed9d853590cf9 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Fri, 17 Jun 2022 15:10:53 +0200 Subject: [PATCH 11/20] Dont use parameter_types macro Signed-off-by: Oliver Tale-Yazdi --- bin/node-template/node/src/command.rs | 2 +- bin/node-template/runtime/src/lib.rs | 9 +++------ bin/node/cli/src/command.rs | 2 +- 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/bin/node-template/node/src/command.rs b/bin/node-template/node/src/command.rs index 43b6b67a2da81..e687710c92580 100644 --- a/bin/node-template/node/src/command.rs +++ b/bin/node-template/node/src/command.rs @@ -149,7 +149,7 @@ pub fn run() -> sc_cli::Result<()> { Box::new(TransferKeepAliveBuilder::new( client.clone(), Sr25519Keyring::Alice.to_account_id(), - ExistentialDeposit::get(), + ExistentialDeposit, )), ]); diff --git a/bin/node-template/runtime/src/lib.rs b/bin/node-template/runtime/src/lib.rs index 3622fb7eb04fe..5fee98ae419c0 100644 --- a/bin/node-template/runtime/src/lib.rs +++ b/bin/node-template/runtime/src/lib.rs @@ -234,11 +234,8 @@ impl pallet_timestamp::Config for Runtime { type WeightInfo = (); } -parameter_types! { - /// This constant can also be written in-place as `ConstU128<500>`. - /// However, in this case its needed in another place as well. - pub const ExistentialDeposit: u128 = 500; -} +/// Existential deposit. +pub const ExistentialDeposit: u128 = 500; impl pallet_balances::Config for Runtime { type MaxLocks = ConstU32<50>; @@ -249,7 +246,7 @@ impl pallet_balances::Config for Runtime { /// The ubiquitous event type. type Event = Event; type DustRemoval = (); - type ExistentialDeposit = ExistentialDeposit; + type ExistentialDeposit = ConstU32; type AccountStore = System; type WeightInfo = pallet_balances::weights::SubstrateWeight; } diff --git a/bin/node/cli/src/command.rs b/bin/node/cli/src/command.rs index df3d5a891e2ab..42aeb4b4bc03b 100644 --- a/bin/node/cli/src/command.rs +++ b/bin/node/cli/src/command.rs @@ -139,7 +139,7 @@ pub fn run() -> Result<()> { Box::new(TransferKeepAliveBuilder::new( client.clone(), Sr25519Keyring::Alice.to_account_id(), - ExistentialDeposit::get(), + ExistentialDeposit, )), ]); From 83f8fcf2086182e144a122c984373c3f2f2f84d9 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Fri, 17 Jun 2022 15:13:14 +0200 Subject: [PATCH 12/20] Cache to_lowercase() call The .to_lowercase() on the builder is actually not needes since its already documented to only return lower case. Signed-off-by: Oliver Tale-Yazdi --- .../benchmarking-cli/src/extrinsic/extrinsic_factory.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/utils/frame/benchmarking-cli/src/extrinsic/extrinsic_factory.rs b/utils/frame/benchmarking-cli/src/extrinsic/extrinsic_factory.rs index 0e89aac15ea60..e245036bcb502 100644 --- a/utils/frame/benchmarking-cli/src/extrinsic/extrinsic_factory.rs +++ b/utils/frame/benchmarking-cli/src/extrinsic/extrinsic_factory.rs @@ -29,12 +29,12 @@ impl ExtrinsicFactory { /// /// Is case in-sensitive. pub fn try_get(&self, pallet: &str, extrinsic: &str) -> Option<&dyn ExtrinsicBuilder> { + let pallet = pallet.to_lowercase(); + let extrinsic = extrinsic.to_lowercase(); + self.0 .iter() - .find(|b| { - b.pallet().to_lowercase() == pallet.to_lowercase() && - b.extrinsic().to_lowercase() == extrinsic.to_lowercase() - }) + .find(|b| b.pallet() == pallet && b.extrinsic() == extrinsic) .map(|b| b.as_ref()) } } From 3688b131274c70bbdacaa02e987759df6a3c604a Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Fri, 17 Jun 2022 15:15:09 +0200 Subject: [PATCH 13/20] Spelling Signed-off-by: Oliver Tale-Yazdi --- utils/frame/benchmarking-cli/src/extrinsic/bench.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utils/frame/benchmarking-cli/src/extrinsic/bench.rs b/utils/frame/benchmarking-cli/src/extrinsic/bench.rs index e3f3c5318fa66..a9a8cdc0be6bd 100644 --- a/utils/frame/benchmarking-cli/src/extrinsic/bench.rs +++ b/utils/frame/benchmarking-cli/src/extrinsic/bench.rs @@ -91,7 +91,7 @@ where Stats::new(&record) } - /// Builds a block which with some optional extrinsics. + /// Builds a block with some optional extrinsics. /// /// Returns the block and the number of extrinsics in the block /// that are not inherents. From 58d7d3fb77905595a4a163ce13eb2f2646819976 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Fri, 17 Jun 2022 15:46:15 +0200 Subject: [PATCH 14/20] Use correct nightly for fmt... Signed-off-by: Oliver Tale-Yazdi --- .../basic-authorship/src/basic_authorship.rs | 84 ++++++++----------- 1 file changed, 36 insertions(+), 48 deletions(-) diff --git a/client/basic-authorship/src/basic_authorship.rs b/client/basic-authorship/src/basic_authorship.rs index 57418266373c5..5a020ee81050e 100644 --- a/client/basic-authorship/src/basic_authorship.rs +++ b/client/basic-authorship/src/basic_authorship.rs @@ -629,14 +629,12 @@ mod tests { .unwrap(); block_on( - txpool.maintain( - chain_event( - client - .header(&BlockId::Number(0u64)) - .expect("header get error") - .expect("there should be header"), - ), - ), + txpool.maintain(chain_event( + client + .header(&BlockId::Number(0u64)) + .expect("header get error") + .expect("there should be header"), + )), ); let mut proposer_factory = @@ -726,14 +724,12 @@ mod tests { block_on(txpool.submit_at(&BlockId::number(0), SOURCE, vec![extrinsic(0)])).unwrap(); block_on( - txpool.maintain( - chain_event( - client - .header(&BlockId::Number(0u64)) - .expect("header get error") - .expect("there should be header"), - ), - ), + txpool.maintain(chain_event( + client + .header(&BlockId::Number(0u64)) + .expect("header get error") + .expect("there should be header"), + )), ); let mut proposer_factory = @@ -829,14 +825,12 @@ mod tests { }; block_on( - txpool.maintain( - chain_event( - client - .header(&BlockId::Number(0u64)) - .expect("header get error") - .expect("there should be header"), - ), - ), + txpool.maintain(chain_event( + client + .header(&BlockId::Number(0u64)) + .expect("header get error") + .expect("there should be header"), + )), ); assert_eq!(txpool.ready().count(), 7); @@ -845,14 +839,12 @@ mod tests { block_on(client.import(BlockOrigin::Own, block)).unwrap(); block_on( - txpool.maintain( - chain_event( - client - .header(&BlockId::Number(1)) - .expect("header get error") - .expect("there should be header"), - ), - ), + txpool.maintain(chain_event( + client + .header(&BlockId::Number(1)) + .expect("header get error") + .expect("there should be header"), + )), ); assert_eq!(txpool.ready().count(), 5); @@ -977,14 +969,12 @@ mod tests { .unwrap(); block_on( - txpool.maintain( - chain_event( - client - .header(&BlockId::Number(0u64)) - .expect("header get error") - .expect("there should be header"), - ), - ), + txpool.maintain(chain_event( + client + .header(&BlockId::Number(0u64)) + .expect("header get error") + .expect("there should be header"), + )), ); assert_eq!(txpool.ready().count(), MAX_SKIPPED_TRANSACTIONS * 3); @@ -1042,14 +1032,12 @@ mod tests { .unwrap(); block_on( - txpool.maintain( - chain_event( - client - .header(&BlockId::Number(0u64)) - .expect("header get error") - .expect("there should be header"), - ), - ), + txpool.maintain(chain_event( + client + .header(&BlockId::Number(0u64)) + .expect("header get error") + .expect("there should be header"), + )), ); assert_eq!(txpool.ready().count(), MAX_SKIPPED_TRANSACTIONS * 2 + 2); From 7aea64b49a24bf65a0ea7f864e623dff28bf96e3 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Fri, 17 Jun 2022 16:39:47 +0200 Subject: [PATCH 15/20] Rename ED Signed-off-by: Oliver Tale-Yazdi --- bin/node-template/node/src/command.rs | 4 ++-- bin/node-template/runtime/src/lib.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/bin/node-template/node/src/command.rs b/bin/node-template/node/src/command.rs index e687710c92580..142f0b40c325e 100644 --- a/bin/node-template/node/src/command.rs +++ b/bin/node-template/node/src/command.rs @@ -5,7 +5,7 @@ use crate::{ service, }; use frame_benchmarking_cli::{BenchmarkCmd, ExtrinsicFactory, SUBSTRATE_REFERENCE_HARDWARE}; -use node_template_runtime::{Block, ExistentialDeposit}; +use node_template_runtime::{Block, EXISTENTIAL_DEPOSIT}; use sc_cli::{ChainSpec, RuntimeVersion, SubstrateCli}; use sc_service::PartialComponents; use sp_keyring::Sr25519Keyring; @@ -149,7 +149,7 @@ pub fn run() -> sc_cli::Result<()> { Box::new(TransferKeepAliveBuilder::new( client.clone(), Sr25519Keyring::Alice.to_account_id(), - ExistentialDeposit, + EXISTENTIAL_DEPOSIT, )), ]); diff --git a/bin/node-template/runtime/src/lib.rs b/bin/node-template/runtime/src/lib.rs index 5fee98ae419c0..15ffe211083a4 100644 --- a/bin/node-template/runtime/src/lib.rs +++ b/bin/node-template/runtime/src/lib.rs @@ -235,7 +235,7 @@ impl pallet_timestamp::Config for Runtime { } /// Existential deposit. -pub const ExistentialDeposit: u128 = 500; +pub const EXISTENTIAL_DEPOSIT: u128 = 500; impl pallet_balances::Config for Runtime { type MaxLocks = ConstU32<50>; @@ -246,7 +246,7 @@ impl pallet_balances::Config for Runtime { /// The ubiquitous event type. type Event = Event; type DustRemoval = (); - type ExistentialDeposit = ConstU32; + type ExistentialDeposit = ConstU128; type AccountStore = System; type WeightInfo = pallet_balances::weights::SubstrateWeight; } From b5a193e044d0fd95888163dc3da33ce71e4f4014 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Fri, 17 Jun 2022 17:24:46 +0200 Subject: [PATCH 16/20] Fix compile Signed-off-by: Oliver Tale-Yazdi --- bin/node/cli/src/command.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/node/cli/src/command.rs b/bin/node/cli/src/command.rs index 42aeb4b4bc03b..df3d5a891e2ab 100644 --- a/bin/node/cli/src/command.rs +++ b/bin/node/cli/src/command.rs @@ -139,7 +139,7 @@ pub fn run() -> Result<()> { Box::new(TransferKeepAliveBuilder::new( client.clone(), Sr25519Keyring::Alice.to_account_id(), - ExistentialDeposit, + ExistentialDeposit::get(), )), ]); From a6eaec540d67a198d732eba0092b5475b139d9fc Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Thu, 14 Jul 2022 16:26:57 +0200 Subject: [PATCH 17/20] Subtract block base weight Signed-off-by: Oliver Tale-Yazdi --- .../benchmarking-cli/src/extrinsic/bench.rs | 48 +++++++++++++------ .../benchmarking-cli/src/extrinsic/cmd.rs | 2 +- .../benchmarking-cli/src/overhead/cmd.rs | 4 +- 3 files changed, 36 insertions(+), 18 deletions(-) diff --git a/utils/frame/benchmarking-cli/src/extrinsic/bench.rs b/utils/frame/benchmarking-cli/src/extrinsic/bench.rs index a9a8cdc0be6bd..fbdeb8cb477b6 100644 --- a/utils/frame/benchmarking-cli/src/extrinsic/bench.rs +++ b/utils/frame/benchmarking-cli/src/extrinsic/bench.rs @@ -37,7 +37,7 @@ use serde::Serialize; use std::{marker::PhantomData, sync::Arc, time::Instant}; use super::ExtrinsicBuilder; -use crate::shared::Stats; +use crate::shared::{StatSelect, Stats}; /// Parameters to configure an *overhead* benchmark. #[derive(Debug, Default, Serialize, Clone, PartialEq, Args)] @@ -84,13 +84,39 @@ where Self { client, params, inherent_data, _p: PhantomData } } - /// Run the specified benchmark. - pub fn bench(&self, ext_builder: Option<&dyn ExtrinsicBuilder>) -> Result { - let (block, num_ext) = self.build_block(ext_builder)?; - let record = self.measure_block(&block, num_ext)?; + /// Benchmark a block with only inherents. + pub fn bench_block(&self) -> Result { + let (block, _) = self.build_block(None)?; + let record = self.measure_block(&block)?; Stats::new(&record) } + /// Benchmark the time of an extrinsic in a full block. + /// + /// First benchmarks an empty block, analogous to `bench_block` and use it as baseline. + /// Then benchmarks a full block built with the given `ext_builder` and subtract the baseline + /// from the result. + /// This is necessary to account for the time the inherents use. + pub fn bench_extrinsic(&self, ext_builder: &dyn ExtrinsicBuilder) -> Result { + let (block, _) = self.build_block(None)?; + let base = self.measure_block(&block)?; + let base_time = Stats::new(&base)?.select(StatSelect::Average); + + let (block, num_ext) = self.build_block(Some(ext_builder))?; + // option to result + let num_ext = num_ext.ok_or_else(|| Error::Input("".into()))?; + let mut records = self.measure_block(&block)?; + + for r in &mut records { + // Subtract the base time. + *r = r.saturating_sub(base_time); + // Divide by the number of extrinsics in the block. + *r = ((*r as f64) / (num_ext as f64)).ceil() as u64; + } + + Stats::new(&records) + } + /// Builds a block with some optional extrinsics. /// /// Returns the block and the number of extrinsics in the block @@ -138,11 +164,8 @@ where } /// Measures the time that it take to execute a block or an extrinsic. - fn measure_block(&self, block: &Block, num_ext: Option) -> Result { + fn measure_block(&self, block: &Block) -> Result { let mut record = BenchRecord::new(); - if num_ext == Some(0) { - return Err("Num extrinsics was zero".into()) - } let genesis = BlockId::Number(Zero::zero()); info!("Running {} warmups...", self.params.warmup); @@ -166,12 +189,7 @@ where .map_err(|e| Error::Client(RuntimeApiError(e)))?; let elapsed = start.elapsed().as_nanos(); - if let Some(num_ext) = num_ext { - // Checked for non-zero div above. - record.push((elapsed as f64 / num_ext as f64).ceil() as u64); - } else { - record.push(elapsed as u64); - } + record.push(elapsed as u64); } Ok(record) diff --git a/utils/frame/benchmarking-cli/src/extrinsic/cmd.rs b/utils/frame/benchmarking-cli/src/extrinsic/cmd.rs index 4ce92c3467514..6b4fd0fad6638 100644 --- a/utils/frame/benchmarking-cli/src/extrinsic/cmd.rs +++ b/utils/frame/benchmarking-cli/src/extrinsic/cmd.rs @@ -110,7 +110,7 @@ impl ExtrinsicCmd { }; let bench = Benchmark::new(client, self.params.bench.clone(), inherent_data); - let stats = bench.bench(Some(ext_builder))?; + let stats = bench.bench_extrinsic(ext_builder)?; info!( "Executing a {}::{} extrinsic takes[ns]:\n{:?}", ext_builder.pallet(), diff --git a/utils/frame/benchmarking-cli/src/overhead/cmd.rs b/utils/frame/benchmarking-cli/src/overhead/cmd.rs index a73906a3090f9..28ceea63f1572 100644 --- a/utils/frame/benchmarking-cli/src/overhead/cmd.rs +++ b/utils/frame/benchmarking-cli/src/overhead/cmd.rs @@ -105,14 +105,14 @@ impl OverheadCmd { // per-block execution overhead { - let stats = bench.bench(Some(ext_builder))?; + let stats = bench.bench_block()?; info!("Per-block execution overhead [ns]:\n{:?}", stats); let template = TemplateData::new(BenchmarkType::Block, &cfg, &self.params, &stats)?; template.write(&self.params.weight.weight_path)?; } // per-extrinsic execution overhead { - let stats = bench.bench(Some(ext_builder))?; + let stats = bench.bench_extrinsic(ext_builder)?; info!("Per-extrinsic execution overhead [ns]:\n{:?}", stats); let template = TemplateData::new(BenchmarkType::Extrinsic, &cfg, &self.params, &stats)?; template.write(&self.params.weight.weight_path)?; From d94db3db368fc6d04af42fc345bbdf608120a861 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Thu, 14 Jul 2022 16:53:41 +0200 Subject: [PATCH 18/20] Fixes Signed-off-by: Oliver Tale-Yazdi --- utils/frame/benchmarking-cli/src/extrinsic/bench.rs | 5 ++--- .../benchmarking-cli/src/extrinsic/extrinsic_factory.rs | 3 ++- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/utils/frame/benchmarking-cli/src/extrinsic/bench.rs b/utils/frame/benchmarking-cli/src/extrinsic/bench.rs index fbdeb8cb477b6..27fc40fb34774 100644 --- a/utils/frame/benchmarking-cli/src/extrinsic/bench.rs +++ b/utils/frame/benchmarking-cli/src/extrinsic/bench.rs @@ -94,7 +94,7 @@ where /// Benchmark the time of an extrinsic in a full block. /// /// First benchmarks an empty block, analogous to `bench_block` and use it as baseline. - /// Then benchmarks a full block built with the given `ext_builder` and subtract the baseline + /// Then benchmarks a full block built with the given `ext_builder` and subtracts the baseline /// from the result. /// This is necessary to account for the time the inherents use. pub fn bench_extrinsic(&self, ext_builder: &dyn ExtrinsicBuilder) -> Result { @@ -103,8 +103,7 @@ where let base_time = Stats::new(&base)?.select(StatSelect::Average); let (block, num_ext) = self.build_block(Some(ext_builder))?; - // option to result - let num_ext = num_ext.ok_or_else(|| Error::Input("".into()))?; + let num_ext = num_ext.ok_or_else(|| Error::Input("Block was empty".into()))?; let mut records = self.measure_block(&block)?; for r in &mut records { diff --git a/utils/frame/benchmarking-cli/src/extrinsic/extrinsic_factory.rs b/utils/frame/benchmarking-cli/src/extrinsic/extrinsic_factory.rs index e245036bcb502..7e1a22ccd1419 100644 --- a/utils/frame/benchmarking-cli/src/extrinsic/extrinsic_factory.rs +++ b/utils/frame/benchmarking-cli/src/extrinsic/extrinsic_factory.rs @@ -50,6 +50,7 @@ pub trait ExtrinsicBuilder { /// /// Should be all lowercase. fn pallet(&self) -> &str; + /// Name of the extrinsic this builder is for. /// /// Should be all lowercase. @@ -62,7 +63,7 @@ pub trait ExtrinsicBuilder { } impl dyn ExtrinsicBuilder + '_ { - /// Name of this builder in CSV format `pallet, extrinsic`. + /// Name of this builder in CSV format: `pallet, extrinsic`. pub fn name(&self) -> String { format!("{}, {}", self.pallet(), self.extrinsic()) } From 929eaa00484e6f4122714a8bfded130283472ce5 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Mon, 18 Jul 2022 16:45:36 +0200 Subject: [PATCH 19/20] Use tmp folder for test This should already be the case since --dev is passed but somehow not... Signed-off-by: Oliver Tale-Yazdi --- bin/node/cli/tests/benchmark_extrinsic_works.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/bin/node/cli/tests/benchmark_extrinsic_works.rs b/bin/node/cli/tests/benchmark_extrinsic_works.rs index 7abe54287b02f..8a901b936eab3 100644 --- a/bin/node/cli/tests/benchmark_extrinsic_works.rs +++ b/bin/node/cli/tests/benchmark_extrinsic_works.rs @@ -29,8 +29,10 @@ fn benchmark_extrinsic_works() { /// Checks that the `benchmark extrinsic` command works for the given pallet and extrinsic. fn benchmark_extrinsic(pallet: &str, extrinsic: &str) { + let base_dir = tempdir().expect("could not create a temp dir"); + let status = Command::new(cargo_bin("substrate")) - .args(&["benchmark", "extrinsic", "--dev"]) + .args(&["benchmark", "extrinsic", "--dev"].arg("-d").arg(base_dir.path())) .args(&["--pallet", pallet, "--extrinsic", extrinsic]) // Run with low repeats for faster execution. .args(["--warmup=10", "--repeat=10", "--max-ext-per-block=10"]) From f603a06e21336c74b2c8cc283aa5b58c6d4bc94a Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Mon, 18 Jul 2022 17:40:42 +0200 Subject: [PATCH 20/20] Fix test Signed-off-by: Oliver Tale-Yazdi --- bin/node/cli/tests/benchmark_extrinsic_works.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/bin/node/cli/tests/benchmark_extrinsic_works.rs b/bin/node/cli/tests/benchmark_extrinsic_works.rs index 8a901b936eab3..69800ad3c6c13 100644 --- a/bin/node/cli/tests/benchmark_extrinsic_works.rs +++ b/bin/node/cli/tests/benchmark_extrinsic_works.rs @@ -18,6 +18,7 @@ use assert_cmd::cargo::cargo_bin; use std::process::Command; +use tempfile::tempdir; /// Tests that the `benchmark extrinsic` command works for /// remark and transfer_keep_alive within the substrate dev runtime. @@ -32,7 +33,9 @@ fn benchmark_extrinsic(pallet: &str, extrinsic: &str) { let base_dir = tempdir().expect("could not create a temp dir"); let status = Command::new(cargo_bin("substrate")) - .args(&["benchmark", "extrinsic", "--dev"].arg("-d").arg(base_dir.path())) + .args(&["benchmark", "extrinsic", "--dev"]) + .arg("-d") + .arg(base_dir.path()) .args(&["--pallet", pallet, "--extrinsic", extrinsic]) // Run with low repeats for faster execution. .args(["--warmup=10", "--repeat=10", "--max-ext-per-block=10"])