diff --git a/prdoc/pr_3395.prdoc b/prdoc/pr_3395.prdoc new file mode 100644 index 0000000000..a10fb84fd7 --- /dev/null +++ b/prdoc/pr_3395.prdoc @@ -0,0 +1,14 @@ +# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0 +# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json + +title: "`benchmarking-cli` `pallet` subcommand: refactor `--list` and add `--all` option" + +doc: + - audience: Node Dev + description: | + `pallet` subcommand's `--list` now accepts two values: "all" and "pallets". The former will list all available benchmarks, the latter will list only pallets. + Also adds `--all` to run all the available benchmarks and `--no-csv-header` to omit the csv-style header in the output. + NOTE: changes are backward compatible. + +crates: + - name: frame-benchmarking-cli diff --git a/substrate/scripts/run_all_benchmarks.sh b/substrate/scripts/run_all_benchmarks.sh index 83848100a7..6dd7cede31 100755 --- a/substrate/scripts/run_all_benchmarks.sh +++ b/substrate/scripts/run_all_benchmarks.sh @@ -59,11 +59,12 @@ done if [ "$skip_build" != true ] then echo "[+] Compiling Substrate benchmarks..." - cargo build --profile=production --locked --features=runtime-benchmarks --bin substrate + cargo build --profile=production --locked --features=runtime-benchmarks --bin substrate-node fi # The executable to use. -SUBSTRATE=./target/production/substrate +# Parent directory because of the monorepo structure. +SUBSTRATE=../target/production/substrate-node # Manually exclude some pallets. EXCLUDED_PALLETS=( @@ -80,11 +81,7 @@ EXCLUDED_PALLETS=( # Load all pallet names in an array. ALL_PALLETS=($( - $SUBSTRATE benchmark pallet --list --chain=dev |\ - tail -n+2 |\ - cut -d',' -f1 |\ - sort |\ - uniq + $SUBSTRATE benchmark pallet --list=pallets --no-csv-header --chain=dev )) # Filter out the excluded pallets by concatenating the arrays and discarding duplicates. diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs index dce49db15f..d5dc6226ab 100644 --- a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs +++ b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs @@ -15,7 +15,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use super::{writer, PalletCmd}; +use super::{writer, ListOutput, PalletCmd}; use codec::{Decode, Encode}; use frame_benchmarking::{ Analysis, BenchmarkBatch, BenchmarkBatchSplitResults, BenchmarkList, BenchmarkParameter, @@ -39,7 +39,13 @@ 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 std::{ + collections::{BTreeMap, BTreeSet, HashMap}, + fmt::Debug, + fs, + str::FromStr, + time, +}; /// Logging target const LOG_TARGET: &'static str = "frame::benchmark::pallet"; @@ -191,6 +197,7 @@ impl PalletCmd { 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(); @@ -309,9 +316,8 @@ impl PalletCmd { return Err("No benchmarks found which match your input.".into()) } - if self.list { - // List benchmarks instead of running them - list_benchmark(benchmarks_to_run); + if let Some(list_output) = self.list { + list_benchmark(benchmarks_to_run, list_output, self.no_csv_header); return Ok(()) } @@ -755,19 +761,43 @@ impl CliConfiguration for PalletCmd { /// List the benchmarks available in the runtime, in a CSV friendly format. fn list_benchmark( - mut benchmarks_to_run: Vec<( + benchmarks_to_run: Vec<( Vec, Vec, Vec<(BenchmarkParameter, u32, u32)>, Vec<(String, String)>, )>, + list_output: ListOutput, + no_csv_header: bool, ) { - // Sort and de-dub by pallet and function name. - benchmarks_to_run.sort_by(|(pa, sa, _, _), (pb, sb, _, _)| (pa, sa).cmp(&(pb, sb))); - benchmarks_to_run.dedup_by(|(pa, sa, _, _), (pb, sb, _, _)| (pa, sa) == (pb, sb)); + let mut benchmarks = BTreeMap::new(); - println!("pallet, benchmark"); - for (pallet, extrinsic, _, _) in benchmarks_to_run { - println!("{}, {}", String::from_utf8_lossy(&pallet), String::from_utf8_lossy(&extrinsic)); + // Sort and de-dub by pallet and function name. + benchmarks_to_run.iter().for_each(|(pallet, extrinsic, _, _)| { + benchmarks + .entry(String::from_utf8_lossy(pallet).to_string()) + .or_insert_with(BTreeSet::new) + .insert(String::from_utf8_lossy(extrinsic).to_string()); + }); + + match list_output { + ListOutput::All => { + if !no_csv_header { + println!("pallet,extrinsic"); + } + for (pallet, extrinsics) in benchmarks { + for extrinsic in extrinsics { + println!("{pallet},{extrinsic}"); + } + } + }, + ListOutput::Pallets => { + if !no_csv_header { + println!("pallet"); + }; + for pallet in benchmarks.keys() { + println!("{pallet}"); + } + }, } } diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/mod.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/mod.rs index c69ce1765f..6dc56c0724 100644 --- a/substrate/utils/frame/benchmarking-cli/src/pallet/mod.rs +++ b/substrate/utils/frame/benchmarking-cli/src/pallet/mod.rs @@ -19,6 +19,7 @@ mod command; mod writer; use crate::shared::HostInfoParams; +use clap::ValueEnum; use sc_cli::{ WasmExecutionMethod, WasmtimeInstantiationStrategy, DEFAULT_WASMTIME_INSTANTIATION_STRATEGY, DEFAULT_WASM_EXECUTION_METHOD, @@ -31,17 +32,32 @@ fn parse_pallet_name(pallet: &str) -> std::result::Result { Ok(pallet.replace("-", "_")) } +/// List options for available benchmarks. +#[derive(Debug, Clone, Copy, ValueEnum)] +pub enum ListOutput { + /// List all available pallets and extrinsics. + All, + /// List all available pallets only. + Pallets, +} + /// Benchmark the extrinsic weight of FRAME Pallets. #[derive(Debug, clap::Parser)] pub struct PalletCmd { /// Select a FRAME Pallet to benchmark, or `*` for all (in which case `extrinsic` must be `*`). - #[arg(short, long, value_parser = parse_pallet_name, required_unless_present_any = ["list", "json_input"])] + #[arg(short, long, value_parser = parse_pallet_name, required_unless_present_any = ["list", "json_input", "all"], default_value_if("all", "true", Some("*".into())))] pub pallet: Option, /// Select an extrinsic inside the pallet to benchmark, or `*` for all. - #[arg(short, long, required_unless_present_any = ["list", "json_input"])] + #[arg(short, long, required_unless_present_any = ["list", "json_input", "all"], default_value_if("all", "true", Some("*".into())))] pub extrinsic: Option, + /// Run benchmarks for all pallets and extrinsics. + /// + /// This is equivalent to running `--pallet * --extrinsic *`. + #[arg(long)] + pub all: bool, + /// Select how many samples we should take across the variable components. #[arg(short, long, default_value_t = 50)] pub steps: u32, @@ -158,11 +174,15 @@ pub struct PalletCmd { #[arg(long = "db-cache", value_name = "MiB", default_value_t = 1024)] pub database_cache_size: u32, - /// List the benchmarks that match your query rather than running them. + /// List and print available benchmarks in a csv-friendly format. /// - /// When nothing is provided, we list all benchmarks. - #[arg(long)] - pub list: bool, + /// NOTE: `num_args` and `require_equals` are required to allow `--list` + #[arg(long, value_enum, ignore_case = true, num_args = 0..=1, require_equals = true, default_missing_value("All"))] + pub list: Option, + + /// Don't include csv header when listing benchmarks. + #[arg(long, requires("list"))] + pub no_csv_header: bool, /// If enabled, the storage info is not displayed in the output next to the analysis. ///