Co #11164: Sub-commands for benchmark

.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

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.
-	#[clap(name = "benchmark", about = "Benchmark runtime pallets.")]
+	/// Sub-commands concerned with benchmarking.
+	/// 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,

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,212 +460,79 @@ 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);
 
-			ensure_dev(chain_spec).map_err(Error::Other)?;
+			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();
 
-			#[cfg(feature = "kusama-native")]
-			if chain_spec.is_kusama() {
-				return Ok(runner.sync_run(|config| {
-					cmd.run::<service::kusama_runtime::Block, service::KusamaExecutorDispatch>(
-						config,
-					)
-					.map_err(|e| Error::SubstrateCli(e))
-				})?)
-			}
-
-			#[cfg(feature = "westend-native")]
-			if chain_spec.is_westend() {
-				return Ok(runner.sync_run(|config| {
-					cmd.run::<service::westend_runtime::Block, service::WestendExecutorDispatch>(
-						config,
-					)
-					.map_err(|e| Error::SubstrateCli(e))
-				})?)
-			}
-
-			// else we assume it is polkadot.
-			#[cfg(feature = "polkadot-native")]
-			{
-				return Ok(runner.sync_run(|config| {
-					cmd.run::<service::polkadot_runtime::Block, service::PolkadotExecutorDispatch>(
-						config,
+					unwrap_client!(
+						client,
+						cmd.run(config, client.clone(), db, storage).map_err(Error::SubstrateCli)
 					)
-					.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")
+				}),
+				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>(config)
+								.map_err(|e| Error::SubstrateCli(e))
+						})?)
 					}
-				})?)
-			}
-
-			#[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(feature = "westend-native")]
+					if chain_spec.is_westend() {
+						return Ok(runner.sync_run(|config| {
+							cmd.run::<service::westend_runtime::Block, service::WestendExecutorDispatch>(config)
+								.map_err(|e| Error::SubstrateCli(e))
+						})?)
 					}
-				})?)
-			}
 
-			#[cfg(not(feature = "polkadot-native"))]
-			unreachable!("No runtime feature (polkadot, kusama, westend, rococo) is enabled")
-		},
-		Some(Subcommand::BenchmarkOverhead(cmd)) => {
-			use polkadot_client::benchmark_inherent_data;
-
-			let runner = cli.create_runner(cmd)?;
-			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")
+					// else we assume it is polkadot.
+					#[cfg(feature = "polkadot-native")]
+					{
+						return Ok(runner.sync_run(|config| {
+							cmd.run::<service::polkadot_runtime::Block, service::PolkadotExecutorDispatch>(config)
+								.map_err(|e| Error::SubstrateCli(e))
+						})?)
 					}
-				})?)
-			}
 
-			#[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(not(feature = "polkadot-native"))]
+					#[allow(unreachable_code)]
+					Err(service::Error::NoRuntime.into())
+				},
+				// NOTE: this allows the Polkadot client to leniently implement
+				// new benchmark commands.
+				#[allow(unreachable_patterns)]
+				_ => Err(Error::CommandNotImplemented),
 			}
-
-			#[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("");

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),
 }