Add execution overhead benchmarking (#10977)

* Add benchmark-block Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Remove first approach This reverts commit 9ce7f32. * Add block and extrinsic benchmarks * Doc Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Fix template Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Beauty fixes Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Check for non-empty chain Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Add tests for Stats Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Review fixes Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Review fixes Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Apply suggestions from code review Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com> * Review fixes Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Review fixes Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Push first version again Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Push first version again Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Cleanup Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Cleanup Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Cleanup Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Beauty fixes Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Apply suggestions from code review Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com> * Update utils/frame/benchmarking-cli/src/overhead/template.rs Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com> * Review fixes Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Doc + Template fixes Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Review fixes Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Comment fix Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Add test Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Pust merge fixup Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Fixup Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Move code to better place Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com> Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
paritytech · Mar 17, 2022 · 89682c1 · 89682c1
1 parent cf69667
commit 89682c1
Show file tree

Hide file tree

Showing 17 changed files with 852 additions and 53 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/bin/node/cli/src/cli.rs b/bin/node/cli/src/cli.rs
@@ -42,6 +42,13 @@ pub enum Subcommand {
 	#[clap(name = "benchmark", about = "Benchmark runtime pallets.")]
 	Benchmark(frame_benchmarking_cli::BenchmarkCmd),
 
+	/// 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),

diff --git a/bin/node/cli/src/command.rs b/bin/node/cli/src/command.rs
@@ -18,10 +18,13 @@
 
 use crate::{chain_spec, service, service::new_partial, Cli, Subcommand};
 use node_executor::ExecutorDispatch;
-use node_runtime::{Block, RuntimeApi};
+use node_primitives::Block;
+use node_runtime::RuntimeApi;
 use sc_cli::{ChainSpec, Result, RuntimeVersion, SubstrateCli};
 use sc_service::PartialComponents;
 
+use std::sync::Arc;
+
 impl SubstrateCli for Cli {
 	fn impl_name() -> String {
 		"Substrate Node".into()
@@ -95,13 +98,21 @@ pub fn run() -> Result<()> {
 				You can enable it with `--features runtime-benchmarks`."
 					.into())
 			},
-		Some(Subcommand::BenchmarkStorage(cmd)) => {
-			if !cfg!(feature = "runtime-benchmarks") {
-				return Err("Benchmarking wasn't enabled when building the node. \
-				You can enable it with `--features runtime-benchmarks`."
-					.into())
-			}
+		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)?;

diff --git a/bin/node/cli/src/command_helper.rs b/bin/node/cli/src/command_helper.rs
@@ -0,0 +1,69 @@
+// 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/>.
+
+//! Contains code to setup the command invocations in [`super::command`] which would
+//! otherwise bloat that module.
+
+use crate::service::{create_extrinsic, FullClient};
+
+use node_runtime::SystemCall;
+use sc_cli::Result;
+use sp_inherents::{InherentData, InherentDataProvider};
+use sp_keyring::Sr25519Keyring;
+use sp_runtime::OpaqueExtrinsic;
+
+use std::{sync::Arc, time::Duration};
+
+/// Generates extrinsics for the `benchmark-overhead` command.
+pub struct ExtrinsicBuilder {
+	client: Arc<FullClient>,
+}
+
+impl ExtrinsicBuilder {
+	/// Creates a new [`Self`] from the given client.
+	pub fn new(client: Arc<FullClient>) -> Self {
+		Self { client }
+	}
+}
+
+impl frame_benchmarking_cli::ExtrinsicBuilder for ExtrinsicBuilder {
+	fn remark(&self, nonce: u32) -> std::result::Result<OpaqueExtrinsic, &'static str> {
+		let acc = Sr25519Keyring::Bob.pair();
+		let extrinsic: OpaqueExtrinsic = create_extrinsic(
+			self.client.as_ref(),
+			acc,
+			SystemCall::remark { remark: vec![] },
+			Some(nonce),
+		)
+		.into();
+
+		Ok(extrinsic)
+	}
+}
+
+/// Generates inherent data for the `benchmark-overhead` command.
+pub fn inherent_data() -> Result<InherentData> {
+	let mut inherent_data = InherentData::new();
+	let d = Duration::from_millis(0);
+	let timestamp = sp_timestamp::InherentDataProvider::new(d.into());
+
+	timestamp
+		.provide_inherent_data(&mut inherent_data)
+		.map_err(|e| format!("creating inherent data: {:?}", e))?;
+	Ok(inherent_data)
+}
diff --git a/bin/node/cli/src/lib.rs b/bin/node/cli/src/lib.rs
@@ -38,6 +38,8 @@ pub mod service;
 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_overhead_works.rs b/bin/node/cli/tests/benchmark_overhead_works.rs
@@ -0,0 +1,46 @@
+// 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/>.
+
+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.
+#[test]
+fn benchmark_overhead_works() {
+	let tmp_dir = tempdir().expect("could not create a temp dir");
+	let base_path = tmp_dir.path();
+
+	// 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"])
+		.arg(base_path)
+		.arg("--weight-path")
+		.arg(base_path)
+		.args(["--warmup", "10", "--repeat", "10"])
+		.args(["--add", "100", "--mul", "1.2", "--metric", "p75"])
+		.args(["--max-ext-per-block", "10"])
+		.status()
+		.unwrap();
+	assert!(status.success());
+
+	// Weight files have been created.
+	assert!(base_path.join("block_weights.rs").exists());
+	assert!(base_path.join("extrinsic_weights.rs").exists());
+}
diff --git a/utils/frame/benchmarking-cli/Cargo.toml b/utils/frame/benchmarking-cli/Cargo.toml
@@ -16,6 +16,7 @@ targets = ["x86_64-unknown-linux-gnu"]
 frame-benchmarking = { version = "4.0.0-dev", path = "../../../frame/benchmarking" }
 frame-support = { version = "4.0.0-dev", path = "../../../frame/support" }
 sp-core = { version = "6.0.0", path = "../../../primitives/core" }
+sc-block-builder = { version = "0.10.0-dev", path = "../../../client/block-builder" }
 sc-service = { version = "0.10.0-dev", default-features = false, path = "../../../client/service" }
 sc-client-api = { version = "4.0.0-dev", path = "../../../client/api" }
 sc-cli = { version = "0.10.0-dev", path = "../../../client/cli" }
@@ -26,6 +27,7 @@ sp-api = { version = "4.0.0-dev", path = "../../../primitives/api" }
 sp-externalities = { version = "0.12.0", path = "../../../primitives/externalities" }
 sp-database = { version = "4.0.0-dev", path = "../../../primitives/database" }
 sp-blockchain = { version = "4.0.0-dev", path = "../../../primitives/blockchain" }
+sp-inherents = { version = "4.0.0-dev", path = "../../../primitives/inherents" }
 sp-keystore = { version = "0.12.0", path = "../../../primitives/keystore" }
 sp-storage = { version = "6.0.0", path = "../../../primitives/storage" }
 sp-runtime = { version = "6.0.0", path = "../../../primitives/runtime" }

diff --git a/utils/frame/benchmarking-cli/src/lib.rs b/utils/frame/benchmarking-cli/src/lib.rs
@@ -16,12 +16,15 @@
 // limitations under the License.
 
 mod command;
+pub mod overhead;
+mod post_processing;
 mod storage;
 mod writer;
 
 use sc_cli::{ExecutionStrategy, WasmExecutionMethod, DEFAULT_WASM_EXECUTION_METHOD};
 use std::{fmt::Debug, path::PathBuf};
 
+pub use overhead::{ExtrinsicBuilder, OverheadCmd};
 pub use storage::StorageCmd;
 
 // Add a more relaxed parsing for pallet names by allowing pallet directory names with `-` to be