Co #11164: Sub-commands for benchmark (#5247)

* Restructure benchmark commands

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Add benchmark storage test

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* cargo update -p sp-io

* Revert "cargo update -p sp-io"

This reverts commit c321b570cef806934acfb7e1a18f543a681b9927.

* cargo update -p sp-io

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* update lockfile for {"substrate"}

* remove `useless_attribute`

* Remove rococo-native cfg

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Make cumulus build

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Fix benchmark commands for the CI

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Make cumulus build

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Make cumulus build

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Disable new bench commands for rococo

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Make cumulus build

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>
Co-authored-by: parity-processbot <>

polkadot/.gitlab-ci.yml

@@ -820,7 +820,7 @@ short-benchmark-polkadot:          &short-bench
   variables:
     RUNTIME:                       polkadot
   script:
-    - ./artifacts/polkadot benchmark --execution wasm --wasm-execution compiled --chain $RUNTIME-dev --pallet "*" --extrinsic "*" --steps 1 --repeat 1
+    - ./artifacts/polkadot benchmark pallet --execution wasm --wasm-execution compiled --chain $RUNTIME-dev --pallet "*" --extrinsic "*" --steps 1 --repeat 1
 
 short-benchmark-kusama:
   <<:                              *short-bench

polkadot/cli/src/cli.rs

@@ -50,28 +50,11 @@ pub enum Subcommand {
 	#[clap(name = "execute-worker", hide = true)]
 	PvfExecuteWorker(ValidationWorkerCommand),
 
-	/// The custom benchmark subcommand benchmarking runtime pallets.
+	/// Sub-commands concerned with benchmarking.
-	#[clap(name = "benchmark", about = "Benchmark runtime pallets.")]
+	/// The pallet benchmarking moved to the `pallet` sub-command.
+	#[clap(subcommand)]
 	Benchmark(frame_benchmarking_cli::BenchmarkCmd),
 
-	/// Benchmark the execution time of historic blocks and compare it to their consumed weight.
-	#[clap(
-		name = "benchmark-block",
-		about = "Benchmark the execution time of historic blocks and compare it to their consumed weight."
-	)]
-	BenchmarkBlock(frame_benchmarking_cli::BlockCmd),
-
-	/// Sub command for benchmarking the per-block and per-extrinsic execution overhead.
-	#[clap(
-		name = "benchmark-overhead",
-		about = "Benchmark the per-block and per-extrinsic execution overhead."
-	)]
-	BenchmarkOverhead(frame_benchmarking_cli::OverheadCmd),
-
-	/// Sub command for benchmarking the storage speed.
-	#[clap(name = "benchmark-storage", about = "Benchmark storage speed.")]
-	BenchmarkStorage(frame_benchmarking_cli::StorageCmd),
-
 	/// Runs performance checks such as PVF compilation in order to measure machine
 	/// capabilities of running a validator.
 	HostPerfCheck,

polkadot/cli/src/command.rs

@@ -15,6 +15,7 @@
 // along with Polkadot.  If not, see <http://www.gnu.org/licenses/>.
 
 use crate::cli::{Cli, Subcommand};
+use frame_benchmarking_cli::BenchmarkCmd;
 use futures::future::TryFutureExt;
 use log::info;
 use sc_cli::{Role, RuntimeVersion, SubstrateCli};
@@ -216,6 +217,27 @@ fn ensure_dev(spec: &Box<dyn service::ChainSpec>) -> std::result::Result<(), Str
 	}
 }
 
+/// Unwraps a [`polkadot_client::Client`] into the concrete runtime client.
+macro_rules! unwrap_client {
+	(
+		$client:ident,
+		$code:expr
+	) => {
+		match $client.as_ref() {
+			#[cfg(feature = "polkadot-native")]
+			polkadot_client::Client::Polkadot($client) => $code,
+			#[cfg(feature = "westend-native")]
+			polkadot_client::Client::Westend($client) => $code,
+			#[cfg(feature = "kusama-native")]
+			polkadot_client::Client::Kusama($client) => $code,
+			#[cfg(feature = "rococo-native")]
+			polkadot_client::Client::Rococo($client) => $code,
+			#[allow(unreachable_patterns)]
+			_ => Err(Error::CommandNotImplemented),
+		}
+	};
+}
+
 /// Runs performance checks.
 /// Should only be used in release build since the check would take too much time otherwise.
 fn host_perf_check() -> Result<()> {
@@ -438,16 +460,49 @@ pub fn run() -> Result<()> {
 		Some(Subcommand::Benchmark(cmd)) => {
 			let runner = cli.create_runner(cmd)?;
 			let chain_spec = &runner.config().chain_spec;
-			set_default_ss58_version(chain_spec);
 
+			match cmd {
+				BenchmarkCmd::Storage(cmd) => runner.sync_run(|mut config| {
+					let (client, backend, _, _) = service::new_chain_ops(&mut config, None)?;
+					let db = backend.expose_db();
+					let storage = backend.expose_storage();
+
+					unwrap_client!(
+						client,
+						cmd.run(config, client.clone(), db, storage).map_err(Error::SubstrateCli)
+					)
+				}),
+				BenchmarkCmd::Block(cmd) => runner.sync_run(|mut config| {
+					let (client, _, _, _) = service::new_chain_ops(&mut config, None)?;
+
+					unwrap_client!(client, cmd.run(client.clone()).map_err(Error::SubstrateCli))
+				}),
+				BenchmarkCmd::Overhead(cmd) => {
+					ensure_dev(chain_spec).map_err(Error::Other)?;
+					runner.sync_run(|mut config| {
+						use polkadot_client::benchmark_inherent_data;
+						let (client, _, _, _) = service::new_chain_ops(&mut config, None)?;
+						let wrapped = client.clone();
+
+						let header = client.header(BlockId::Number(0_u32.into())).unwrap().unwrap();
+						let inherent_data = benchmark_inherent_data(header)
+							.map_err(|e| format!("generating inherent data: {:?}", e))?;
+
+						unwrap_client!(
+							client,
+							cmd.run(config, client.clone(), inherent_data, wrapped)
+								.map_err(Error::SubstrateCli)
+						)
+					})
+				},
+				BenchmarkCmd::Pallet(cmd) => {
+					set_default_ss58_version(chain_spec);
 					ensure_dev(chain_spec).map_err(Error::Other)?;
 
 					#[cfg(feature = "kusama-native")]
 					if chain_spec.is_kusama() {
 						return Ok(runner.sync_run(|config| {
-					cmd.run::<service::kusama_runtime::Block, service::KusamaExecutorDispatch>(
+							cmd.run::<service::kusama_runtime::Block, service::KusamaExecutorDispatch>(config)
-						config,
-					)
 								.map_err(|e| Error::SubstrateCli(e))
 						})?)
 					}
@@ -455,9 +510,7 @@ pub fn run() -> Result<()> {
 					#[cfg(feature = "westend-native")]
 					if chain_spec.is_westend() {
 						return Ok(runner.sync_run(|config| {
-					cmd.run::<service::westend_runtime::Block, service::WestendExecutorDispatch>(
+							cmd.run::<service::westend_runtime::Block, service::WestendExecutorDispatch>(config)
-						config,
-					)
 								.map_err(|e| Error::SubstrateCli(e))
 						})?)
 					}
@@ -466,184 +519,20 @@ pub fn run() -> Result<()> {
 					#[cfg(feature = "polkadot-native")]
 					{
 						return Ok(runner.sync_run(|config| {
-					cmd.run::<service::polkadot_runtime::Block, service::PolkadotExecutorDispatch>(
+							cmd.run::<service::polkadot_runtime::Block, service::PolkadotExecutorDispatch>(config)
-						config,
-					)
 								.map_err(|e| Error::SubstrateCli(e))
 						})?)
 					}
-			#[cfg(not(feature = "polkadot-native"))]
-			panic!("No runtime feature (polkadot, kusama, westend, rococo) is enabled")
-		},
-		Some(Subcommand::BenchmarkBlock(cmd)) => {
-			let runner = cli.create_runner(cmd)?;
-			let chain_spec = &runner.config().chain_spec;
-
-			#[cfg(feature = "rococo-native")]
-			if chain_spec.is_rococo() || chain_spec.is_wococo() || chain_spec.is_versi() {
-				return Ok(runner.async_run(|mut config| {
-					let (client, _, _, task_manager) = service::new_chain_ops(&mut config, None)?;
-
-					if let polkadot_client::Client::Rococo(pd) = &*client {
-						Ok((cmd.run(pd.clone()).map_err(Error::SubstrateCli), task_manager))
-					} else {
-						unreachable!("Checked above; qed")
-					}
-				})?)
-			}
-
-			#[cfg(feature = "kusama-native")]
-			if chain_spec.is_kusama() {
-				return Ok(runner.async_run(|mut config| {
-					let (client, _, _, task_manager) = service::new_chain_ops(&mut config, None)?;
-
-					if let polkadot_client::Client::Kusama(pd) = &*client {
-						Ok((cmd.run(pd.clone()).map_err(Error::SubstrateCli), task_manager))
-					} else {
-						unreachable!("Checked above; qed")
-					}
-				})?)
-			}
-
-			#[cfg(feature = "westend-native")]
-			if chain_spec.is_westend() {
-				return Ok(runner.async_run(|mut config| {
-					let (client, _, _, task_manager) = service::new_chain_ops(&mut config, None)?;
-
-					if let polkadot_client::Client::Westend(pd) = &*client {
-						Ok((cmd.run(pd.clone()).map_err(Error::SubstrateCli), task_manager))
-					} else {
-						unreachable!("Checked above; qed")
-					}
-				})?)
-			}
-
-			#[cfg(feature = "polkadot-native")]
-			{
-				return Ok(runner.async_run(|mut config| {
-					let (client, _, _, task_manager) = service::new_chain_ops(&mut config, None)?;
-
-					if let polkadot_client::Client::Polkadot(pd) = &*client {
-						Ok((cmd.run(pd.clone()).map_err(Error::SubstrateCli), task_manager))
-					} else {
-						unreachable!("Checked above; qed")
-					}
-				})?)
-			}
 
 					#[cfg(not(feature = "polkadot-native"))]
-			unreachable!("No runtime feature (polkadot, kusama, westend, rococo) is enabled")
+					#[allow(unreachable_code)]
+					Err(service::Error::NoRuntime.into())
 				},
-		Some(Subcommand::BenchmarkOverhead(cmd)) => {
+				// NOTE: this allows the Polkadot client to leniently implement
-			use polkadot_client::benchmark_inherent_data;
+				// new benchmark commands.
-
+				#[allow(unreachable_patterns)]
-			let runner = cli.create_runner(cmd)?;
+				_ => Err(Error::CommandNotImplemented),
-			let chain_spec = &runner.config().chain_spec;
-			set_default_ss58_version(chain_spec);
-			ensure_dev(chain_spec).map_err(Error::Other)?;
-
-			#[cfg(feature = "rococo-native")]
-			if chain_spec.is_rococo() || chain_spec.is_wococo() || chain_spec.is_versi() {
-				return Ok(runner.async_run(|mut config| {
-					let (client, _, _, task_manager) = service::new_chain_ops(&mut config, None)?;
-
-					let header = client.header(BlockId::Number(0_u32.into())).unwrap().unwrap();
-					let inherent_data = benchmark_inherent_data(header)
-						.map_err(|e| format!("generating inherent data: {:?}", e))?;
-
-					if let polkadot_client::Client::Rococo(pd) = &*client {
-						Ok((
-							cmd.run(config, pd.clone(), inherent_data, client)
-								.map_err(Error::SubstrateCli),
-							task_manager,
-						))
-					} else {
-						unreachable!("Checked above; qed")
 			}
-				})?)
-			}
-
-			#[cfg(feature = "kusama-native")]
-			if chain_spec.is_kusama() {
-				return Ok(runner.async_run(|mut config| {
-					let (client, _, _, task_manager) = service::new_chain_ops(&mut config, None)?;
-
-					let header = client.header(BlockId::Number(0_u32.into())).unwrap().unwrap();
-					let inherent_data = benchmark_inherent_data(header)
-						.map_err(|e| format!("generating inherent data: {:?}", e))?;
-
-					if let polkadot_client::Client::Kusama(pd) = &*client {
-						Ok((
-							cmd.run(config, pd.clone(), inherent_data, client)
-								.map_err(Error::SubstrateCli),
-							task_manager,
-						))
-					} else {
-						unreachable!("Checked above; qed")
-					}
-				})?)
-			}
-
-			#[cfg(feature = "westend-native")]
-			if chain_spec.is_westend() {
-				return Ok(runner.async_run(|mut config| {
-					let (client, _, _, task_manager) = service::new_chain_ops(&mut config, None)?;
-
-					let header = client.header(BlockId::Number(0_u32.into())).unwrap().unwrap();
-					let inherent_data = benchmark_inherent_data(header)
-						.map_err(|e| format!("generating inherent data: {:?}", e))?;
-
-					if let polkadot_client::Client::Westend(pd) = &*client {
-						Ok((
-							cmd.run(config, pd.clone(), inherent_data, client)
-								.map_err(Error::SubstrateCli),
-							task_manager,
-						))
-					} else {
-						unreachable!("Checked above; qed")
-					}
-				})?)
-			}
-
-			#[cfg(feature = "polkadot-native")]
-			{
-				return Ok(runner.async_run(|mut config| {
-					let (client, _, _, task_manager) = service::new_chain_ops(&mut config, None)?;
-
-					let header = client.header(BlockId::Number(0_u32.into())).unwrap().unwrap();
-					let inherent_data = benchmark_inherent_data(header)
-						.map_err(|e| format!("generating inherent data: {:?}", e))?;
-
-					if let polkadot_client::Client::Polkadot(pd) = &*client {
-						Ok((
-							cmd.run(config, pd.clone(), inherent_data, client)
-								.map_err(Error::SubstrateCli),
-							task_manager,
-						))
-					} else {
-						unreachable!("Checked above; qed")
-					}
-				})?)
-			}
-
-			#[cfg(not(feature = "polkadot-native"))]
-			unreachable!("No runtime feature (polkadot, kusama, westend, rococo) is enabled")
-		},
-		Some(Subcommand::BenchmarkStorage(cmd)) => {
-			let runner = cli.create_runner(cmd)?;
-			let chain_spec = &runner.config().chain_spec;
-			set_default_ss58_version(chain_spec);
-
-			Ok(runner.async_run(|mut config| {
-				let (client, backend, _, task_manager) = service::new_chain_ops(&mut config, None)?;
-				let db = backend.expose_db();
-				let storage = backend.expose_storage();
-
-				Ok((
-					cmd.run(config, client, db, storage).map_err(Error::SubstrateCli),
-					task_manager,
-				))
-			})?)
 		},
 		Some(Subcommand::HostPerfCheck) => {
 			let mut builder = sc_cli::LoggerBuilder::new("");

polkadot/cli/src/error.rs

@@ -45,6 +45,9 @@ pub enum Error {
 	#[error("URL did not resolve to anything")]
 	AddressResolutionMissing,
 
+	#[error("Command is not implemented")]
+	CommandNotImplemented,
+
 	#[error("Other: {0}")]
 	Other(String),
 }

polkadot/tests/benchmark_block_works.rs

@@ -1,20 +1,18 @@
-// This file is part of Substrate.
+// Copyright 2022 Parity Technologies (UK) Ltd.
+// This file is part of Polkadot.
 
-// Copyright (C) 2022 Parity Technologies (UK) Ltd.
+// Polkadot is free software: you can redistribute it and/or modify
-// 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,
+// Polkadot 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 <https://www.gnu.org/licenses/>.
+// along with Polkadot.  If not, see <http://www.gnu.org/licenses/>.
 
 // Unix only since it uses signals.
 #![cfg(unix)]
@@ -29,9 +27,9 @@ use tempfile::tempdir;
 
 pub mod common;
 
-static RUNTIMES: [&'static str; 6] = ["polkadot", "kusama", "westend", "rococo", "wococo", "versi"];
+static RUNTIMES: [&'static str; 3] = ["polkadot", "kusama", "westend"];
 
-/// `benchmark-block` works for all dev runtimes using the wasm executor.
+/// `benchmark block` works for all dev runtimes using the wasm executor.
 #[tokio::test]
 async fn benchmark_block_works() {
 	for runtime in RUNTIMES {
@@ -69,14 +67,14 @@ async fn build_chain(runtime: &str, base_path: &Path) -> Result<(), String> {
 
 /// Benchmarks the given block with the wasm executor.
 fn benchmark_block(runtime: &str, base_path: &Path, block: u32) -> Result<(), String> {
-	// Invoke `benchmark-block` with all options to make sure that they are valid.
+	// Invoke `benchmark block` with all options to make sure that they are valid.
 	let status = Command::new(cargo_bin("polkadot"))
-		.args(["benchmark-block", "--chain", &runtime])
+		.args(["benchmark", "block", "--chain", &runtime])
 		.arg("-d")
 		.arg(base_path)
 		.args(["--pruning", "archive"])
 		.args(["--from", &block.to_string(), "--to", &block.to_string()])
-		.args(["--repeat", "2"])
+		.args(["--repeat", "1"])
 		.args(["--execution", "wasm", "--wasm-execution", "compiled"])
 		.status()
 		.map_err(|e| format!("command failed: {:?}", e))?;

polkadot/tests/benchmark_overhead_works.rs

@@ -1,28 +1,26 @@
-// This file is part of Substrate.
+// Copyright 2022 Parity Technologies (UK) Ltd.
+// This file is part of Polkadot.
 
-// Copyright (C) 2022 Parity Technologies (UK) Ltd.
+// Polkadot is free software: you can redistribute it and/or modify
-// 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,
+// Polkadot 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 <https://www.gnu.org/licenses/>.
+// along with Polkadot.  If not, see <http://www.gnu.org/licenses/>.
 
 use assert_cmd::cargo::cargo_bin;
 use std::{process::Command, result::Result};
 use tempfile::tempdir;
 
-static RUNTIMES: [&'static str; 6] = ["polkadot", "kusama", "westend", "rococo", "wococo", "versi"];
+static RUNTIMES: [&'static str; 3] = ["polkadot", "kusama", "westend"];
 
-/// `benchmark-overhead` works for all dev runtimes.
+/// `benchmark overhead` works for all dev runtimes.
 #[test]
 fn benchmark_overhead_works() {
 	for runtime in RUNTIMES {
@@ -31,7 +29,7 @@ fn benchmark_overhead_works() {
 	}
 }
 
-/// `benchmark-overhead` rejects all non-dev runtimes.
+/// `benchmark overhead` rejects all non-dev runtimes.
 #[test]
 fn benchmark_overhead_rejects_non_dev_runtimes() {
 	for runtime in RUNTIMES {
@@ -43,9 +41,9 @@ fn benchmark_overhead(runtime: String) -> Result<(), String> {
 	let tmp_dir = tempdir().expect("could not create a temp dir");
 	let base_path = tmp_dir.path();
 
-	// Invoke `benchmark-overhead` with all options to make sure that they are valid.
+	// Invoke `benchmark overhead` with all options to make sure that they are valid.
 	let status = Command::new(cargo_bin("polkadot"))
-		.args(["benchmark-overhead", "--chain", &runtime])
+		.args(["benchmark", "overhead", "--chain", &runtime])
 		.arg("-d")
 		.arg(base_path)
 		.arg("--weight-path")

polkadot/tests/benchmark_storage_works.rs

@@ -0,0 +1,51 @@
+// Copyright 2022 Parity Technologies (UK) Ltd.
+// This file is part of Polkadot.
+
+// Polkadot 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.
+
+// Polkadot 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 Polkadot.  If not, see <http://www.gnu.org/licenses/>.
+
+use assert_cmd::cargo::cargo_bin;
+use std::{
+	path::Path,
+	process::{Command, ExitStatus},
+};
+use tempfile::tempdir;
+
+/// The `benchmark storage` command works for the dev runtime.
+#[test]
+fn benchmark_storage_works() {
+	let tmp_dir = tempdir().expect("could not create a temp dir");
+	let base_path = tmp_dir.path();
+
+	// Benchmarking the storage works and creates the weight file.
+	assert!(benchmark_storage("rocksdb", base_path).success());
+	assert!(base_path.join("rocksdb_weights.rs").exists());
+
+	assert!(benchmark_storage("paritydb", base_path).success());
+	assert!(base_path.join("paritydb_weights.rs").exists());
+}
+
+/// Invoke the `benchmark storage` sub-command.
+fn benchmark_storage(db: &str, base_path: &Path) -> ExitStatus {
+	Command::new(cargo_bin("polkadot"))
+		.args(&["benchmark", "storage", "--dev"])
+		.arg("--db")
+		.arg(db)
+		.arg("--weight-path")
+		.arg(base_path)
+		.args(["--state-version", "0"])
+		.args(["--warmups", "0"])
+		.args(["--add", "100", "--mul", "1.2", "--metric", "p75"])
+		.status()
+		.unwrap()
+}