Sub-commands for benchmark

bin/node-template/node/src/cli.rs

@@ -36,8 +36,8 @@ pub enum Subcommand {
 	/// Revert the chain to a previous state.
 	Revert(sc_cli::RevertCmd),
 
-	/// The custom benchmark subcommand benchmarking runtime pallets.
-	#[clap(name = "benchmark", about = "Benchmark runtime pallets.")]
+	/// Sub-commands concerned with benchmarking.
+	#[clap(subcommand)]
 	Benchmark(frame_benchmarking_cli::BenchmarkCmd),
 
 	/// Try some command against runtime state.

bin/node-template/node/src/command.rs

@@ -3,6 +3,7 @@ use crate::{
 	cli::{Cli, Subcommand},
 	service,
 };
+use frame_benchmarking_cli::BenchmarkCmd;
 use node_template_runtime::Block;
 use sc_cli::{ChainSpec, RuntimeVersion, SubstrateCli};
 use sc_service::PartialComponents;
@@ -98,16 +99,21 @@ pub fn run() -> sc_cli::Result<()> {
 				Ok((cmd.run(client, backend, None), task_manager))
 			})
 		},
-		Some(Subcommand::Benchmark(cmd)) =>
-			if cfg!(feature = "runtime-benchmarks") {
-				let runner = cli.create_runner(cmd)?;
+		Some(Subcommand::Benchmark(cmd)) => {
+			let runner = cli.create_runner(cmd)?;
 
-				runner.sync_run(|config| cmd.run::<Block, service::ExecutorDispatch>(config))
-			} else {
-				Err("Benchmarking wasn't enabled when building the node. You can enable it with \
-				     `--features runtime-benchmarks`."
-					.into())
-			},
+			runner.sync_run(|config| match cmd {
+				BenchmarkCmd::Pallet(cmd) =>
+					if cfg!(feature = "runtime-benchmarks") {
+						cmd.run::<Block, service::ExecutorDispatch>(config)
+					} else {
+						Err("Runtime benchmarking wasn't enabled when building the node. \
+							You can enable it with `--features runtime-benchmarks`."
+							.into())
+					},
+				_ => Err("Sub-command is not supported".into()),
+			})
+		},
 		#[cfg(feature = "try-runtime")]
 		Some(Subcommand::TryRuntime(cmd)) => {
 			let runner = cli.create_runner(cmd)?;

bin/node/cli/src/cli.rs

@@ -38,28 +38,11 @@ pub enum Subcommand {
 	)]
 	Inspect(node_inspect::cli::InspectCmd),
 
-	/// The custom benchmark subcommmand 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),
-
 	/// Try some command against runtime state.
 	#[cfg(feature = "try-runtime")]
 	TryRuntime(try_runtime_cli::TryRuntimeCmd),

bin/node/cli/src/command.rs

@@ -16,7 +16,9 @@
 // You should have received a copy of the GNU General Public License
 // along with this program. If not, see <https://www.gnu.org/licenses/>.
 
+use super::command_helper::{benchmark_inherent_data, ExtrinsicBuilder};
 use crate::{chain_spec, service, service::new_partial, Cli, Subcommand};
+use frame_benchmarking_cli::*;
 use node_executor::ExecutorDispatch;
 use node_primitives::Block;
 use node_runtime::RuntimeApi;
@@ -88,45 +90,39 @@ pub fn run() -> Result<()> {
 
 			runner.sync_run(|config| cmd.run::<Block, RuntimeApi, ExecutorDispatch>(config))
 		},
-		Some(Subcommand::Benchmark(cmd)) =>
-			if cfg!(feature = "runtime-benchmarks") {
-				let runner = cli.create_runner(cmd)?;
-
-				runner.sync_run(|config| cmd.run::<Block, ExecutorDispatch>(config))
-			} else {
-				Err("Benchmarking wasn't enabled when building the node. \
-				You can enable it with `--features runtime-benchmarks`."
-					.into())
-			},
-		Some(Subcommand::BenchmarkBlock(cmd)) => {
+		Some(Subcommand::Benchmark(cmd)) => {
 			let runner = cli.create_runner(cmd)?;
-			runner.async_run(|config| {
-				let PartialComponents { client, task_manager, .. } = new_partial(&config)?;
-				Ok((cmd.run(client), task_manager))
-			})
-		},
-		Some(Subcommand::BenchmarkOverhead(cmd)) => {
-			let runner = cli.create_runner(cmd)?;
-			runner.async_run(|mut config| {
-				use super::command_helper::{inherent_data, ExtrinsicBuilder};
-				// We don't use the authority role since that would start producing blocks
-				// in the background which would mess with our benchmark.
-				config.role = sc_service::Role::Full;
-
-				let PartialComponents { client, task_manager, .. } = new_partial(&config)?;
-				let ext_builder = ExtrinsicBuilder::new(client.clone());
-
-				Ok((cmd.run(config, client, inherent_data()?, Arc::new(ext_builder)), task_manager))
-			})
-		},
-		Some(Subcommand::BenchmarkStorage(cmd)) => {
-			let runner = cli.create_runner(cmd)?;
-			runner.async_run(|config| {
-				let PartialComponents { client, task_manager, backend, .. } = new_partial(&config)?;
-				let db = backend.expose_db();
-				let storage = backend.expose_storage();
 
-				Ok((cmd.run(config, client, db, storage), task_manager))
+			runner.sync_run(|config| {
+				let PartialComponents { client, backend, .. } = new_partial(&config)?;
+
+				// This switch needs to be in the client, since the client decides
+				// which sub-commands it wants to support.
+				match cmd {
+					BenchmarkCmd::Pallet(cmd) => {
+						if !cfg!(feature = "runtime-benchmarks") {
+							return Err(
+								"Runtime benchmarking wasn't enabled when building the node. \
+							You can enable it with `--features runtime-benchmarks`."
+									.into(),
+							)
+						}
+
+						cmd.run::<Block, ExecutorDispatch>(config)
+					},
+					BenchmarkCmd::Block(cmd) => cmd.run(client),
+					BenchmarkCmd::Storage(cmd) => {
+						let db = backend.expose_db();
+						let storage = backend.expose_storage();
+
+						cmd.run(config, client, db, storage)
+					},
+					BenchmarkCmd::Overhead(cmd) => {
+						let ext_builder = ExtrinsicBuilder::new(client.clone());
+
+						cmd.run(config, client, benchmark_inherent_data()?, Arc::new(ext_builder))
+					},
+				}
 			})
 		},
 		Some(Subcommand::Key(cmd)) => cmd.run(&cli),

bin/node/cli/src/command_helper.rs

@@ -57,7 +57,7 @@ impl frame_benchmarking_cli::ExtrinsicBuilder for ExtrinsicBuilder {
 }
 
 /// Generates inherent data for the `benchmark-overhead` command.
-pub fn inherent_data() -> Result<InherentData> {
+pub fn benchmark_inherent_data() -> Result<InherentData> {
 	let mut inherent_data = InherentData::new();
 	let d = Duration::from_millis(0);
 	let timestamp = sp_timestamp::InherentDataProvider::new(d.into());

bin/node/cli/tests/benchmark_block_works.rs

@@ -0,0 +1,48 @@
+// 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 <https://www.gnu.org/licenses/>.
+
+// Unix only since it uses signals from [`common::run_node_for_a_while`].
+#![cfg(unix)]
+
+use assert_cmd::cargo::cargo_bin;
+use std::process::Command;
+use tempfile::tempdir;
+
+pub mod common;
+
+/// `benchmark block` works for the dev runtime using the wasm executor.
+#[tokio::test]
+async fn benchmark_block_works() {
+	let base_dir = tempdir().expect("could not create a temp dir");
+
+	common::run_node_for_a_while(base_dir.path(), &["--dev"]).await;
+
+	// Invoke `benchmark block` with all options to make sure that they are valid.
+	let status = Command::new(cargo_bin("substrate"))
+		.args(["benchmark", "block", "--dev"])
+		.arg("-d")
+		.arg(base_dir.path())
+		.args(["--pruning", "archive"])
+		.args(["--from", "1", "--to", "1"])
+		.args(["--repeat", "1"])
+		.args(["--execution", "wasm", "--wasm-execution", "compiled"])
+		.status()
+		.unwrap();
+
+	assert!(status.success())
+}

bin/node/cli/tests/benchmark_overhead_works.rs

@@ -20,7 +20,7 @@ use assert_cmd::cargo::cargo_bin;
 use std::process::Command;
 use tempfile::tempdir;
 
-/// Tests that the `benchmark-overhead` command works for the substrate dev runtime.
+/// Tests that the `benchmark overhead` command works for the substrate dev runtime.
 #[test]
 fn benchmark_overhead_works() {
 	let tmp_dir = tempdir().expect("could not create a temp dir");
@@ -29,7 +29,7 @@ fn benchmark_overhead_works() {
 	// Only put 10 extrinsics into the block otherwise it takes forever to build it
 	// especially for a non-release build.
 	let status = Command::new(cargo_bin("substrate"))
-		.args(&["benchmark-overhead", "--dev", "-d"])
+		.args(&["benchmark", "overhead", "--dev", "-d"])
 		.arg(base_path)
 		.arg("--weight-path")
 		.arg(base_path)

bin/node/cli/tests/benchmark_storage_works.rs

@@ -23,7 +23,7 @@ use std::{
 };
 use tempfile::tempdir;
 
-/// Tests that the `benchmark-storage` command works for the dev runtime.
+/// Tests that 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");
@@ -39,7 +39,7 @@ fn benchmark_storage_works() {
 
 fn benchmark_storage(db: &str, base_path: &Path) -> ExitStatus {
 	Command::new(cargo_bin("substrate"))
-		.args(&["benchmark-storage", "--dev"])
+		.args(&["benchmark", "storage", "--dev"])
 		.arg("--db")
 		.arg(db)
 		.arg("--weight-path")

utils/frame/benchmarking-cli/src/block/bench.rs

@@ -34,7 +34,7 @@ use serde::Serialize;
 use std::{fmt::Debug, marker::PhantomData, sync::Arc, time::Instant};
 use thousands::Separable;
 
-use crate::storage::record::{StatSelect, Stats};
+use crate::shared::{StatSelect, Stats};
 
 /// Log target for printing block weight info.
 const LOG_TARGET: &'static str = "benchmark::block::weight";

utils/frame/benchmarking-cli/src/block/cmd.rs

@@ -29,7 +29,7 @@ use std::{fmt::Debug, sync::Arc};
 
 use super::bench::{Benchmark, BenchmarkParams};
 
-/// Benchmark the execution time historic blocks.
+/// Benchmark the execution time of historic blocks.
 ///
 /// This can be used to verify that blocks do not use more weight than they consumed
 /// in their `WeightInfo`. Example:
@@ -73,7 +73,7 @@ impl BlockCmd {
 	/// Benchmark the execution time of historic blocks and compare it to their consumed weight.
 	///
 	/// Output will be printed to console.
-	pub async fn run<Block, BA, C>(&self, client: Arc<C>) -> Result<()>
+	pub fn run<Block, BA, C>(&self, client: Arc<C>) -> Result<()>
 	where
 		Block: BlockT<Extrinsic = OpaqueExtrinsic>,
 		BA: ClientBackend<Block>,