From 9ce7f32cae12ce637bd3b85d255dd55ba7198f9d Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Wed, 2 Mar 2022 11:17:19 +0000 Subject: [PATCH 01/29] Add benchmark-block Signed-off-by: Oliver Tale-Yazdi --- Cargo.lock | 14 + bin/node-template/node/Cargo.toml | 10 + bin/node-template/node/src/cli.rs | 4 + bin/node-template/node/src/command.rs | 75 ++++- bin/node-template/node/src/service.rs | 2 +- bin/node/cli/Cargo.toml | 6 + bin/node/cli/src/cli.rs | 4 + bin/node/cli/src/command.rs | 70 +++- frame/aura/src/lib.rs | 5 +- frame/babe/src/lib.rs | 11 +- utils/frame/benchmarking-cli/Cargo.toml | 6 + utils/frame/benchmarking-cli/src/block/cmd.rs | 303 ++++++++++++++++++ utils/frame/benchmarking-cli/src/block/mod.rs | 20 ++ .../benchmarking-cli/src/block/weights.hbs | 87 +++++ utils/frame/benchmarking-cli/src/lib.rs | 2 + .../benchmarking-cli/src/storage/record.rs | 2 +- .../benchmarking-cli/src/storage/write.rs | 1 + 17 files changed, 610 insertions(+), 12 deletions(-) create mode 100644 utils/frame/benchmarking-cli/src/block/cmd.rs create mode 100644 utils/frame/benchmarking-cli/src/block/mod.rs create mode 100644 utils/frame/benchmarking-cli/src/block/weights.hbs diff --git a/Cargo.lock b/Cargo.lock index 5c1b6ed88fb4a..5739ced35f011 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2126,6 +2126,7 @@ dependencies = [ "clap 3.0.7", "frame-benchmarking", "frame-support", + "futures 0.3.19", "handlebars", "hash-db", "hex", @@ -2136,9 +2137,11 @@ dependencies = [ "memory-db", "parity-scale-codec", "rand 0.8.4", + "sc-block-builder", "sc-cli", "sc-client-api", "sc-client-db", + "sc-consensus", "sc-executor", "sc-service", "serde", @@ -2146,9 +2149,11 @@ dependencies = [ "serde_nanos", "sp-api", "sp-blockchain", + "sp-consensus", "sp-core", "sp-database", "sp-externalities", + "sp-inherents", "sp-keystore", "sp-runtime", "sp-state-machine", @@ -4787,6 +4792,7 @@ version = "3.0.0-dev" dependencies = [ "assert_cmd", "async-std", + "chrono", "clap 3.0.7", "clap_complete", "criterion", @@ -4845,6 +4851,7 @@ dependencies = [ "sp-blockchain", "sp-consensus", "sp-consensus-babe", + "sp-consensus-slots", "sp-core", "sp-finality-grandpa", "sp-inherents", @@ -5059,6 +5066,9 @@ dependencies = [ "frame-benchmarking", "frame-benchmarking-cli", "jsonrpc-core", + "log 0.4.14", + "node-cli", + "node-runtime", "node-template-runtime", "pallet-transaction-payment-rpc", "sc-basic-authorship", @@ -5080,8 +5090,12 @@ dependencies = [ "sp-blockchain", "sp-consensus", "sp-consensus-aura", + "sp-consensus-babe", + "sp-consensus-slots", "sp-core", "sp-finality-grandpa", + "sp-inherents", + "sp-keyring", "sp-runtime", "sp-timestamp", "substrate-build-script-utils", diff --git a/bin/node-template/node/Cargo.toml b/bin/node-template/node/Cargo.toml index 98e8af96d3f8d..6e0d8bbc46d34 100644 --- a/bin/node-template/node/Cargo.toml +++ b/bin/node-template/node/Cargo.toml @@ -37,6 +37,12 @@ sc-client-api = { version = "4.0.0-dev", path = "../../../client/api" } sp-runtime = { version = "6.0.0", path = "../../../primitives/runtime" } sp-timestamp = { version = "4.0.0-dev", path = "../../../primitives/timestamp" } +# OTY added +sp-consensus-slots = { version = "0.10.0-dev", path = "../../../primitives/consensus/slots" } +sp-consensus-babe = { version = "0.10.0-dev", path = "../../../primitives/consensus/babe" } +sp-inherents = { version = "4.0.0-dev", path = "../../../primitives/inherents" } +log = "0.4.8" + # These dependencies are used for the node template's RPCs jsonrpc-core = "18.0.0" sc-rpc = { version = "4.0.0-dev", path = "../../../client/rpc" } @@ -57,6 +63,10 @@ node-template-runtime = { version = "4.0.0-dev", path = "../runtime" } # CLI-specific dependencies try-runtime-cli = { version = "0.10.0-dev", optional = true, path = "../../../utils/frame/try-runtime/cli" } +# OTY added +sp-keyring = { version = "6.0.0", path = "../../../primitives/keyring" } +node-runtime = { version = "3.0.0-dev", path = "../../node/runtime" } +node-cli = { version = "3.0.0-dev", path = "../../node/cli" } [build-dependencies] substrate-build-script-utils = { version = "3.0.0", path = "../../../utils/build-script-utils" } diff --git a/bin/node-template/node/src/cli.rs b/bin/node-template/node/src/cli.rs index c4d27b71e4994..9db00ccdb5dd9 100644 --- a/bin/node-template/node/src/cli.rs +++ b/bin/node-template/node/src/cli.rs @@ -40,6 +40,10 @@ pub enum Subcommand { #[clap(name = "benchmark", about = "Benchmark runtime pallets.")] Benchmark(frame_benchmarking_cli::BenchmarkCmd), + /// The custom benchmark subcommand benchmarking runtime pallets. + #[clap(name = "benchmark-block", about = "Benchmark runtime pallets.")] + BenchmarkBlock(frame_benchmarking_cli::BlockCmd), + /// Try some command against runtime state. #[cfg(feature = "try-runtime")] TryRuntime(try_runtime_cli::TryRuntimeCmd), diff --git a/bin/node-template/node/src/command.rs b/bin/node-template/node/src/command.rs index 72c7a75b387bb..346c139ebb1ab 100644 --- a/bin/node-template/node/src/command.rs +++ b/bin/node-template/node/src/command.rs @@ -3,10 +3,13 @@ use crate::{ cli::{Cli, Subcommand}, service, }; -use node_template_runtime::Block; +use node_cli::service::create_extrinsic; +use node_runtime::SystemCall; use sc_cli::{ChainSpec, RuntimeVersion, SubstrateCli}; use sc_service::PartialComponents; +use std::sync::Arc; + impl SubstrateCli for Cli { fn impl_name() -> String { "Substrate Node".into() @@ -46,6 +49,47 @@ impl SubstrateCli for Cli { } } +use super::service::FullClient; +struct ExtrinsicGen { + client: Arc, +} +use node_runtime::UncheckedExtrinsic; +use node_template_runtime::Block; +use sp_keyring::Sr25519Keyring; +use sp_runtime::{traits::Block as BlockT, OpaqueExtrinsic}; +/* +impl frame_benchmarking_cli::block::cmd::ExtrinsicGenerator for ExtrinsicGen { + fn noop(&self, nonce: u32) -> Option { + let src = Sr25519Keyring::Alice.pair(); + + let extrinsic: OpaqueExtrinsic = create_extrinsic( + self.client.as_ref(), + src.clone(), + SystemCall::remark { remark: vec![] }, + Some(nonce), + ) + .into(); + None + } +}*/ + +#[derive(Default)] +struct InherentProv {} + +impl frame_benchmarking_cli::block::cmd::BlockInherentDataProvider for InherentProv { + fn providers( + &self, + block: u64, + ) -> std::result::Result>, sp_inherents::Error> + { + log::info!("Creating inherents for block #{}", block); + let d = std::time::Duration::from_millis(0); + let timestamp = sp_timestamp::InherentDataProvider::new(d.into()); + + Ok(vec![Arc::new(timestamp)]) + } +} + /// Parse and run command line arguments pub fn run() -> sc_cli::Result<()> { let cli = Cli::from_args(); @@ -98,6 +142,35 @@ pub fn run() -> sc_cli::Result<()> { Ok((cmd.run(client, backend), task_manager)) }) }, + Some(Subcommand::BenchmarkBlock(cmd)) => { + unimplemented!(); + }, + /*let runner = cli.create_runner(cmd)?; + runner.async_run(|mut config| { + config.role = sc_service::Role::Full; + + let PartialComponents { client, task_manager, backend, import_queue, .. } = + service::new_partial(&config)?; + let db = backend.expose_db(); + let storage = backend.expose_storage(); + let ext_gen = ExtrinsicGen { client: client.clone() }; + let provs: InherentProv = Default::default(); + + Ok(( + cmd.run( + config, + client.clone(), + import_queue, + db, + storage, + client.clone(), + Arc::new(provs), + Arc::new(ext_gen), + ), + task_manager, + )) + }) + },*/ Some(Subcommand::Benchmark(cmd)) => if cfg!(feature = "runtime-benchmarks") { let runner = cli.create_runner(cmd)?; diff --git a/bin/node-template/node/src/service.rs b/bin/node-template/node/src/service.rs index fc7dc9b978df3..e2a8cb4ed834b 100644 --- a/bin/node-template/node/src/service.rs +++ b/bin/node-template/node/src/service.rs @@ -31,7 +31,7 @@ impl sc_executor::NativeExecutionDispatch for ExecutorDispatch { } } -type FullClient = +pub(crate) type FullClient = sc_service::TFullClient>; type FullBackend = sc_service::TFullBackend; type FullSelectChain = sc_consensus::LongestChain; diff --git a/bin/node/cli/Cargo.toml b/bin/node/cli/Cargo.toml index b4a91712fd16c..a5c5c0530b854 100644 --- a/bin/node/cli/Cargo.toml +++ b/bin/node/cli/Cargo.toml @@ -96,6 +96,12 @@ frame-benchmarking-cli = { version = "4.0.0-dev", optional = true, path = "../.. node-inspect = { version = "0.9.0-dev", optional = true, path = "../inspect" } try-runtime-cli = { version = "0.10.0-dev", optional = true, path = "../../../utils/frame/try-runtime/cli" } +# OTY added +sp-consensus-slots = { version = "0.10.0-dev", path = "../../../primitives/consensus/slots" } +# OTY +sp-blockchain = { version = "4.0.0-dev", path = "../../../primitives/blockchain" } +chrono = "0.4.19" + [target.'cfg(any(target_arch="x86_64", target_arch="aarch64"))'.dependencies] node-executor = { version = "3.0.0-dev", path = "../executor", features = ["wasmtime"] } sc-cli = { version = "0.10.0-dev", optional = true, path = "../../../client/cli", features = ["wasmtime"] } diff --git a/bin/node/cli/src/cli.rs b/bin/node/cli/src/cli.rs index 386215854b963..b6c6da310822d 100644 --- a/bin/node/cli/src/cli.rs +++ b/bin/node/cli/src/cli.rs @@ -42,6 +42,10 @@ pub enum Subcommand { #[clap(name = "benchmark", about = "Benchmark runtime pallets.")] Benchmark(frame_benchmarking_cli::BenchmarkCmd), + /// Sub command for benchmarking the storage speed. + #[clap(name = "benchmark-block", about = "Benchmark TODO.")] + BenchmarkBlock(frame_benchmarking_cli::BlockCmd), + /// 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 index cc6480bb90d55..0a181f3b9b44f 100644 --- a/bin/node/cli/src/command.rs +++ b/bin/node/cli/src/command.rs @@ -18,10 +18,12 @@ use crate::{chain_spec, service, service::new_partial, Cli, Subcommand}; use node_executor::ExecutorDispatch; -use node_runtime::{Block, RuntimeApi}; +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() @@ -68,6 +70,46 @@ impl SubstrateCli for Cli { &node_runtime::VERSION } } +struct ExtrinsicGen { + client: Arc, +} +use crate::service::{create_extrinsic, FullClient}; +use node_primitives::Block; +use node_runtime::{BalancesCall, SystemCall}; +use sp_keyring::Sr25519Keyring; +use sp_runtime::AccountId32; +use sp_runtime::{traits::Block as BlockT, MultiAddress, OpaqueExtrinsic}; +impl frame_benchmarking_cli::block::cmd::ExtrinsicGenerator for ExtrinsicGen { + fn noop(&self, nonce: u32) -> Option { + let src = Sr25519Keyring::Alice.pair(); + + let extrinsic: OpaqueExtrinsic = create_extrinsic( + self.client.as_ref(), + src.clone(), + SystemCall::remark { remark: vec![] }, + Some(nonce), + ) + .into(); + Some(extrinsic) + } +} + +#[derive(Default)] +struct InherentProv {} + +impl frame_benchmarking_cli::block::cmd::BlockInherentDataProvider for InherentProv { + fn providers( + &self, + block: u64, + ) -> std::result::Result>, sp_inherents::Error> + { + log::info!("Creating inherents for block #{}", block); + let d = std::time::Duration::from_millis(0); + let timestamp = sp_timestamp::InherentDataProvider::new(d.into()); + + Ok(vec![Arc::new(timestamp)]) + } +} /// Parse command line arguments into service configuration. pub fn run() -> Result<()> { @@ -95,6 +137,32 @@ pub fn run() -> Result<()> { You can enable it with `--features runtime-benchmarks`." .into()) }, + Some(Subcommand::BenchmarkBlock(cmd)) => { + let runner = cli.create_runner(cmd)?; + runner.async_run(|mut config| { + config.role = sc_service::Role::Full; + + let PartialComponents { client, task_manager, backend, .. } = new_partial(&config)?; + let db = backend.expose_db(); + let storage = backend.expose_storage(); + let ext_gen = ExtrinsicGen { client: client.clone() }; + + let provs: InherentProv = Default::default(); + Ok(( + cmd.run( + config, + client.clone(), + client.clone(), + db, + storage, + client.clone(), + Arc::new(provs), + Arc::new(ext_gen), + ), + task_manager, + )) + }) + }, Some(Subcommand::BenchmarkStorage(cmd)) => { if !cfg!(feature = "runtime-benchmarks") { return Err("Benchmarking wasn't enabled when building the node. \ diff --git a/frame/aura/src/lib.rs b/frame/aura/src/lib.rs index 657965c60a3f1..83fb6cb392a27 100644 --- a/frame/aura/src/lib.rs +++ b/frame/aura/src/lib.rs @@ -282,9 +282,6 @@ impl OnTimestampSet for Pallet { let timestamp_slot = moment / slot_duration; let timestamp_slot = Slot::from(timestamp_slot.saturated_into::()); - assert!( - CurrentSlot::::get() == timestamp_slot, - "Timestamp slot must match `CurrentSlot`" - ); + assert!(CurrentSlot::::get() == timestamp_slot, "sdaf`"); } } diff --git a/frame/babe/src/lib.rs b/frame/babe/src/lib.rs index f673c8b43bee0..22d0fd62891a6 100644 --- a/frame/babe/src/lib.rs +++ b/frame/babe/src/lib.rs @@ -842,10 +842,13 @@ impl OnTimestampSet for Pallet { let timestamp_slot = moment / slot_duration; let timestamp_slot = Slot::from(timestamp_slot.saturated_into::()); - assert!( - CurrentSlot::::get() == timestamp_slot, - "Timestamp slot must match `CurrentSlot`" - ); + let c = CurrentSlot::::get(); + if c != timestamp_slot { + panic!( + "d: {:?}, moment: {:?}, want: {:?}, got: {:?}", + slot_duration, moment, c, timestamp_slot + ); + } } } diff --git a/utils/frame/benchmarking-cli/Cargo.toml b/utils/frame/benchmarking-cli/Cargo.toml index 20122c4279366..46c7b985c3dbd 100644 --- a/utils/frame/benchmarking-cli/Cargo.toml +++ b/utils/frame/benchmarking-cli/Cargo.toml @@ -49,6 +49,12 @@ hex = "0.4.3" memory-db = "0.29.0" rand = { version = "0.8.4", features = ["small_rng"] } +sc-block-builder = { version = "0.10.0-dev", path = "../../../client/block-builder" } +sp-inherents = { version = "4.0.0-dev", path = "../../../primitives/inherents" } +futures = "0.3.19" +sp-consensus = { version = "0.10.0-dev", path = "../../../primitives/consensus/common" } +sc-consensus = { version = "0.10.0-dev", path = "../../../client/consensus/common" } + [features] default = ["db", "sc-client-db/runtime-benchmarks"] db = ["sc-client-db/with-kvdb-rocksdb", "sc-client-db/with-parity-db"] diff --git a/utils/frame/benchmarking-cli/src/block/cmd.rs b/utils/frame/benchmarking-cli/src/block/cmd.rs new file mode 100644 index 0000000000000..4a6a9dfb33f37 --- /dev/null +++ b/utils/frame/benchmarking-cli/src/block/cmd.rs @@ -0,0 +1,303 @@ +// This file is part of Substrate. + +// Copyright (C) 2022 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +use sc_cli::{CliConfiguration, DatabaseParams, PruningParams, Result, SharedParams}; +use sc_client_api::{Backend as ClientBackend, StorageProvider, UsageProvider}; +use sc_client_db::DbHash; +use sc_service::Configuration; +use sp_blockchain::HeaderBackend; +use sp_database::{ColumnId, Database}; +use sp_runtime::{ + OpaqueExtrinsic, + traits::{Block as BlockT, HashFor}, + transaction_validity::{InvalidTransaction, TransactionValidityError}, +}; +use sp_state_machine::Storage; +use sp_storage::StateVersion; +use sp_inherents::InherentData; +use sc_block_builder::{BlockBuilderProvider, BlockBuilder}; +use sc_block_builder::BlockBuilderApi; +use sp_api::{ApiExt, ProvideRuntimeApi}; +use sc_consensus::{ + block_import::{BlockImportParams, ForkChoiceStrategy}, + BlockImport, StateAction, +}; +use sp_blockchain::{ApplyExtrinsicFailed::Validity, Error::ApplyExtrinsicFailed}; +use sp_consensus::BlockOrigin; +use sp_api::BlockId; + +use clap::{Args, Parser}; +use log::info; +use rand::prelude::*; +use serde::Serialize; +use std::{boxed::Box, fmt::Debug, sync::Arc, time::Instant}; + +use crate::storage::{record::{Stats, StatSelect}, template::TemplateData}; + +#[derive(Debug, Parser)] +pub struct BlockCmd { + #[allow(missing_docs)] + #[clap(flatten)] + pub shared_params: SharedParams, + + #[allow(missing_docs)] + #[clap(flatten)] + pub database_params: DatabaseParams, + + #[allow(missing_docs)] + #[clap(flatten)] + pub pruning_params: PruningParams, + + #[allow(missing_docs)] + #[clap(flatten)] + pub params: BlockParams, +} + +#[derive(Debug, Default, Serialize, Clone, PartialEq, Args)] +pub struct BlockParams { + /// Path to write the *weight* file to. Can be a file or directory. + /// For substrate this should be `frame/support/src/weights`. + #[clap(long, default_value = ".")] + pub weight_path: String, + + /// Select a specific metric to calculate the final weight output. + #[clap(long = "metric", default_value = "average")] + pub weight_metric: StatSelect, + + /// Multiply the resulting weight with the given factor. Must be positive. + /// Is calculated before `weight_add`. + #[clap(long = "mul", default_value = "1")] + pub weight_mul: f64, + + /// Add the given offset to the resulting weight. + /// Is calculated after `weight_mul`. + #[clap(long = "add", default_value = "0")] + pub weight_add: u64, + + /// Skip the `read` benchmark. + #[clap(long)] + pub skip_read: bool, + + /// Skip the `write` benchmark. + #[clap(long)] + pub skip_write: bool, + + /// Rounds of warmups before measuring. + /// Only supported for `read` benchmarks. + #[clap(long, default_value = "1")] + pub warmups: u32, + + /// State cache size. + #[clap(long, default_value = "0")] + pub state_cache_size: usize, + + /// Limit them number of extrinsics to put in a block. + #[clap(long)] + pub max_ext_per_block: Option, + + /// Repeats for measuring the time of a an empty block execution. + #[clap(long, default_value = "1000")] + pub repeat_empty_block: u32, + + /// Repeats for measuring the time of a a full block execution. + #[clap(long, default_value = "100")] + pub repeat_full_block: u32, +} + +/// The results of multiple runs in ns. +type BenchRecord = Vec; + +pub trait ExtrinsicGenerator { + fn noop(&self, nonce: u32) -> Option; +} + +impl BlockCmd { + pub async fn run( + &self, + cfg: Configuration, + client: Arc, + mut iq: IQ, + db: (Arc>, ColumnId), + storage: Arc>>, + ra: Arc, + idps: Arc, + ext_gen: Arc, + ) -> Result<()> + where + // TODO remove the Extrinsic = OpaqueExtrinsic + Block: BlockT, + BA: ClientBackend, + C: BlockBuilderProvider + + UsageProvider + + StorageProvider + + HeaderBackend, + RA: ProvideRuntimeApi, + IQ: sc_consensus::BlockImport, + API: ApiExt + BlockBuilderApi, + { + let genesis = BlockId::Number(client.info().best_number); + //let mut template = TemplateData::new(&cfg, &self.params); + info!("Creating empty block"); + // Block builders are not cloneable, so we need two. + let mut build_empty = client.new_block(Default::default()).unwrap(); + let mut build_full = client.new_block(Default::default()).unwrap(); + let mut data = sp_inherents::InherentData::new(); + idps.provide_inherent_data(&mut data).unwrap(); + info!("Creating inherents from {} inherent datas", data.len()); + let inherents = build_empty.create_inherents(data).unwrap(); + info!("Inserting {} inherent extrinsic", inherents.len()); + for ext in inherents.clone() { + build_empty.push(ext.clone())?; + build_full.push(ext.clone())?; + } + // Build a block with only inherents aka empty. + let empty_block = build_empty.build()?.block; + // Interesting part here: + // Benchmark 1; Execution time of a an empty block. + let mut rec_empty = BenchRecord::new(); + info!("Executing an empty block {} times", self.params.repeat_empty_block); + for i in 0..self.params.repeat_empty_block { + let block = empty_block.clone(); + let start = Instant::now(); + ra.runtime_api().execute_block(&genesis, block).unwrap(); + rec_empty.push(start.elapsed().as_nanos() as u64); + } + + info!("Estimating max NO-OPs per block, capped at {}", self.max_ext_per_block()); + let mut noops = Vec::new(); + for nonce in 0..self.max_ext_per_block() { + let ext = ext_gen.noop(nonce).expect("Need noop"); + match build_full.push(ext.clone()) { + Ok(_) => {}, + Err(ApplyExtrinsicFailed(Validity(TransactionValidityError::Invalid( + InvalidTransaction::ExhaustsResources, + )))) => break, + Err(error) => panic!("{}", error), + } + noops.push(ext); + } + info!("Max NO-OPs per block {}, continuing with benchmark.", noops.len()); + let full_block = build_full.build()?.block; + + // Interesting part here: + // Benchmark 2; Execution time of a block filled with NO-OPs. + let mut rec_full = BenchRecord::new(); + info!("Executing a full block {} times", self.params.repeat_full_block); + for i in 0..self.params.repeat_full_block { + let block = full_block.clone(); + let start = Instant::now(); + ra.runtime_api().execute_block(&genesis, block).unwrap(); + let took = start.elapsed().as_nanos(); + // Divide by the number of extrinsics in a block. + rec_full.push(took as u64 / noops.len() as u64); + } + + let start = Instant::now(); + Self::must_import(full_block, &mut iq); + let speed = noops.len() as f32 / start.elapsed().as_secs_f32(); + info!("Imported block {:.2} NO-OP/s", speed); + + let stats = Stats::new(&rec_empty).unwrap(); + info!("Executing an empty block [ns]:\n{:?}", stats); + + let stats = Stats::new(&rec_full).unwrap(); + info!("Executing NO-OP extrinsic [ns]:\n{:?}", stats); + + Ok(()) + } + + /*fn new_block<'a, Block, BA, RA, C>(client: Arc, idps: Arc,) -> Result> + where Block: BlockT, + C: BlockBuilderProvider + 'a, + RA: sp_api::ProvideRuntimeApi, + BA: sc_client_api::Backend { + let builder = client.new_block(Default::default()).unwrap(); + Ok(builder) + }*/ + + fn must_import(block: B, iq: &mut IQ) + where + B: BlockT, + IQ: sc_consensus::BlockImport, + { + let mut params = BlockImportParams::new(BlockOrigin::File, block.header().clone()); + params.body = Some(block.extrinsics().to_vec()); + params.fork_choice = Some(ForkChoiceStrategy::LongestChain); + let block_hash = block.hash(); + let import = + futures::executor::block_on(iq.import_block(params, Default::default())).unwrap(); + if !matches!(import, sc_consensus::ImportResult::Imported(_)) { + panic!("Could not import block: {:?}", import) + } + } + + /// Creates an rng from a random seed. + pub(crate) fn setup_rng() -> impl rand::Rng { + let seed = rand::thread_rng().gen::(); + info!("Using seed {}", seed); + StdRng::seed_from_u64(seed) + } + + fn max_ext_per_block(&self) -> u32 { + self.params.max_ext_per_block.unwrap_or(u32::MAX) + } +} + +// Boilerplate +impl CliConfiguration for BlockCmd { + fn shared_params(&self) -> &SharedParams { + &self.shared_params + } + + fn database_params(&self) -> Option<&DatabaseParams> { + Some(&self.database_params) + } + + fn pruning_params(&self) -> Option<&PruningParams> { + Some(&self.pruning_params) + } + + fn state_cache_size(&self) -> Result { + Ok(self.params.state_cache_size) + } +} + + + + /*info!("Importing empty block as warmup"); + let block = block_builder.build()?.block; + info!("Executing block"); + ra.runtime_api().execute_block(&BlockId::Number(genesis), block.clone()).expect("Must execute"); + info!("Importing block"); + Self::must_import(block, &mut iq); + + info!("Creating empty block"); + let mut block_builder = client.new_block(Default::default()).unwrap(); + + let idps = block_inherents.providers(1).unwrap(); + info!("Creating inherent data from {} providers", idps.len()); + + let mut data = sp_inherents::InherentData::new(); + for idp in idps.clone() { + idp.provide_inherent_data(&mut data).unwrap(); + } + info!("Creating inherents from {} inherent datas", data.len()); + let inherents = block_builder.create_inherents(data).unwrap(); + info!("Inserting {} extrinsic", inherents.len()); + for ext in inherents.clone() { + block_builder.push(ext.clone())?; + }*/ diff --git a/utils/frame/benchmarking-cli/src/block/mod.rs b/utils/frame/benchmarking-cli/src/block/mod.rs new file mode 100644 index 0000000000000..4e33e24202b1c --- /dev/null +++ b/utils/frame/benchmarking-cli/src/block/mod.rs @@ -0,0 +1,20 @@ +// This file is part of Substrate. + +// Copyright (C) 2022 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +pub mod cmd; + +pub use cmd::BlockCmd; diff --git a/utils/frame/benchmarking-cli/src/block/weights.hbs b/utils/frame/benchmarking-cli/src/block/weights.hbs new file mode 100644 index 0000000000000..eafdd1114e766 --- /dev/null +++ b/utils/frame/benchmarking-cli/src/block/weights.hbs @@ -0,0 +1,87 @@ +// This file is part of Substrate. + +// Copyright (C) 2022 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION {{version}} +//! DATE: {{date}} +//! +//! DATABASE: `{{db_name}}`, RUNTIME: `{{runtime_name}}` +//! SKIP-WRITE: `{{params.skip_write}}`, SKIP-READ: `{{params.skip_read}}`, WARMUPS: `{{params.warmups}}` +//! STATE-VERSION: `V{{params.state_version}}`, STATE-CACHE-SIZE: `{{params.state_cache_size}}` +//! WEIGHT-PATH: `{{params.weight_path}}` +//! METRIC: `{{params.weight_metric}}`, WEIGHT-MUL: `{{params.weight_mul}}`, WEIGHT-ADD: `{{params.weight_add}}` + +// Executed Command: +{{#each args as |arg|}} +// {{arg}} +{{/each}} + +pub mod constants { + use frame_support::{ + parameter_types, + weights::{Weight, constants, RuntimeDbWeight}, + }; + + parameter_types! { + /// Importing a block with 0 txs. + pub const BlockExecutionWeight: Weight = {{underscore block}} * constants::WEIGHT_PER_NANOS; + /// Executing a System remarks (no-op) txs. + pub const ExtrinsicBaseWeight: Weight = {{underscore extrinsic}} * constants::WEIGHT_PER_NANOS; + } + + #[cfg(test)] + mod test_base_weights { + use frame_support::weights::constants; + + /// Checks that the block weight exist and is sane. + // NOTE: If this test fails but you are sure that the generated values are fine, + // you can delete it. + #[test] + fn sane_block() { + use super::constants::BlockExecutionWeight as W; + + // At least 100 µs. + assert!( + W::get() >= 100 * constants::WEIGHT_PER_MICROS, + "Weight should be at least 100 µs." + ); + // At most 50 ms. + assert!( + W::get() <= 10 * constants::WEIGHT_PER_MILLIS, + "Weight should be at most 10 ms." + ); + } + + /// Checks that the extrinsic weight exist and is sane. + // NOTE: If this test fails but you are sure that the generated values are fine, + // you can delete it. + #[test] + fn sane_extrinsic() { + use super::constants::ExtrinsicBaseWeight as W; + + // At least 10 µs. + assert!( + W::get() >= 10 * constants::WEIGHT_PER_MICROS, + "Weight should be at least 100 µs." + ); + // At most 1 ms. + assert!( + W::get() <= constants::WEIGHT_PER_MILLIS, + "Weight should be at most 10 ms." + ); + } + } +} diff --git a/utils/frame/benchmarking-cli/src/lib.rs b/utils/frame/benchmarking-cli/src/lib.rs index 56aab0321ccd0..7de6929b2eafb 100644 --- a/utils/frame/benchmarking-cli/src/lib.rs +++ b/utils/frame/benchmarking-cli/src/lib.rs @@ -15,6 +15,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +pub mod block; mod command; mod storage; mod writer; @@ -22,6 +23,7 @@ mod writer; use sc_cli::{ExecutionStrategy, WasmExecutionMethod}; use std::{fmt::Debug, path::PathBuf}; +pub use block::BlockCmd; pub use storage::StorageCmd; // Add a more relaxed parsing for pallet names by allowing pallet directory names with `-` to be diff --git a/utils/frame/benchmarking-cli/src/storage/record.rs b/utils/frame/benchmarking-cli/src/storage/record.rs index 00a613c713007..3bb1ec21965fb 100644 --- a/utils/frame/benchmarking-cli/src/storage/record.rs +++ b/utils/frame/benchmarking-cli/src/storage/record.rs @@ -154,7 +154,7 @@ impl Stats { fn percentile(mut xs: Vec, p: f64) -> u64 { xs.sort(); let index = (xs.len() as f64 * p).ceil() as usize; - xs[index] + xs[index-1] } } diff --git a/utils/frame/benchmarking-cli/src/storage/write.rs b/utils/frame/benchmarking-cli/src/storage/write.rs index eb9ba11f30696..49ebbcc2349a9 100644 --- a/utils/frame/benchmarking-cli/src/storage/write.rs +++ b/utils/frame/benchmarking-cli/src/storage/write.rs @@ -51,6 +51,7 @@ impl StorageCmd { let mut record = BenchRecord::default(); let supports_rc = db.supports_ref_counting(); + // TODO use HeaderBackend.info() let block = BlockId::Number(client.usage_info().chain.best_number); let header = client.header(block)?.ok_or("Header not found")?; let original_root = *header.state_root(); From f07941f282ee46c124b806c9ac0e7aff43a1828e Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Mon, 7 Mar 2022 15:59:11 +0100 Subject: [PATCH 02/29] Remove first approach This reverts commit 9ce7f32cae12ce637bd3b85d255dd55ba7198f9d. --- Cargo.lock | 14 - bin/node-template/node/Cargo.toml | 10 - bin/node-template/node/src/cli.rs | 4 - bin/node-template/node/src/command.rs | 75 +---- bin/node-template/node/src/service.rs | 2 +- bin/node/cli/Cargo.toml | 6 - bin/node/cli/src/cli.rs | 4 - bin/node/cli/src/command.rs | 70 +--- frame/aura/src/lib.rs | 5 +- frame/babe/src/lib.rs | 11 +- utils/frame/benchmarking-cli/Cargo.toml | 6 - utils/frame/benchmarking-cli/src/block/cmd.rs | 303 ------------------ utils/frame/benchmarking-cli/src/block/mod.rs | 20 -- .../benchmarking-cli/src/block/weights.hbs | 87 ----- utils/frame/benchmarking-cli/src/lib.rs | 2 - .../benchmarking-cli/src/storage/record.rs | 2 +- .../benchmarking-cli/src/storage/write.rs | 1 - 17 files changed, 12 insertions(+), 610 deletions(-) delete mode 100644 utils/frame/benchmarking-cli/src/block/cmd.rs delete mode 100644 utils/frame/benchmarking-cli/src/block/mod.rs delete mode 100644 utils/frame/benchmarking-cli/src/block/weights.hbs diff --git a/Cargo.lock b/Cargo.lock index 5739ced35f011..5c1b6ed88fb4a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2126,7 +2126,6 @@ dependencies = [ "clap 3.0.7", "frame-benchmarking", "frame-support", - "futures 0.3.19", "handlebars", "hash-db", "hex", @@ -2137,11 +2136,9 @@ dependencies = [ "memory-db", "parity-scale-codec", "rand 0.8.4", - "sc-block-builder", "sc-cli", "sc-client-api", "sc-client-db", - "sc-consensus", "sc-executor", "sc-service", "serde", @@ -2149,11 +2146,9 @@ dependencies = [ "serde_nanos", "sp-api", "sp-blockchain", - "sp-consensus", "sp-core", "sp-database", "sp-externalities", - "sp-inherents", "sp-keystore", "sp-runtime", "sp-state-machine", @@ -4792,7 +4787,6 @@ version = "3.0.0-dev" dependencies = [ "assert_cmd", "async-std", - "chrono", "clap 3.0.7", "clap_complete", "criterion", @@ -4851,7 +4845,6 @@ dependencies = [ "sp-blockchain", "sp-consensus", "sp-consensus-babe", - "sp-consensus-slots", "sp-core", "sp-finality-grandpa", "sp-inherents", @@ -5066,9 +5059,6 @@ dependencies = [ "frame-benchmarking", "frame-benchmarking-cli", "jsonrpc-core", - "log 0.4.14", - "node-cli", - "node-runtime", "node-template-runtime", "pallet-transaction-payment-rpc", "sc-basic-authorship", @@ -5090,12 +5080,8 @@ dependencies = [ "sp-blockchain", "sp-consensus", "sp-consensus-aura", - "sp-consensus-babe", - "sp-consensus-slots", "sp-core", "sp-finality-grandpa", - "sp-inherents", - "sp-keyring", "sp-runtime", "sp-timestamp", "substrate-build-script-utils", diff --git a/bin/node-template/node/Cargo.toml b/bin/node-template/node/Cargo.toml index 6e0d8bbc46d34..98e8af96d3f8d 100644 --- a/bin/node-template/node/Cargo.toml +++ b/bin/node-template/node/Cargo.toml @@ -37,12 +37,6 @@ sc-client-api = { version = "4.0.0-dev", path = "../../../client/api" } sp-runtime = { version = "6.0.0", path = "../../../primitives/runtime" } sp-timestamp = { version = "4.0.0-dev", path = "../../../primitives/timestamp" } -# OTY added -sp-consensus-slots = { version = "0.10.0-dev", path = "../../../primitives/consensus/slots" } -sp-consensus-babe = { version = "0.10.0-dev", path = "../../../primitives/consensus/babe" } -sp-inherents = { version = "4.0.0-dev", path = "../../../primitives/inherents" } -log = "0.4.8" - # These dependencies are used for the node template's RPCs jsonrpc-core = "18.0.0" sc-rpc = { version = "4.0.0-dev", path = "../../../client/rpc" } @@ -63,10 +57,6 @@ node-template-runtime = { version = "4.0.0-dev", path = "../runtime" } # CLI-specific dependencies try-runtime-cli = { version = "0.10.0-dev", optional = true, path = "../../../utils/frame/try-runtime/cli" } -# OTY added -sp-keyring = { version = "6.0.0", path = "../../../primitives/keyring" } -node-runtime = { version = "3.0.0-dev", path = "../../node/runtime" } -node-cli = { version = "3.0.0-dev", path = "../../node/cli" } [build-dependencies] substrate-build-script-utils = { version = "3.0.0", path = "../../../utils/build-script-utils" } diff --git a/bin/node-template/node/src/cli.rs b/bin/node-template/node/src/cli.rs index 9db00ccdb5dd9..c4d27b71e4994 100644 --- a/bin/node-template/node/src/cli.rs +++ b/bin/node-template/node/src/cli.rs @@ -40,10 +40,6 @@ pub enum Subcommand { #[clap(name = "benchmark", about = "Benchmark runtime pallets.")] Benchmark(frame_benchmarking_cli::BenchmarkCmd), - /// The custom benchmark subcommand benchmarking runtime pallets. - #[clap(name = "benchmark-block", about = "Benchmark runtime pallets.")] - BenchmarkBlock(frame_benchmarking_cli::BlockCmd), - /// Try some command against runtime state. #[cfg(feature = "try-runtime")] TryRuntime(try_runtime_cli::TryRuntimeCmd), diff --git a/bin/node-template/node/src/command.rs b/bin/node-template/node/src/command.rs index 346c139ebb1ab..72c7a75b387bb 100644 --- a/bin/node-template/node/src/command.rs +++ b/bin/node-template/node/src/command.rs @@ -3,13 +3,10 @@ use crate::{ cli::{Cli, Subcommand}, service, }; -use node_cli::service::create_extrinsic; -use node_runtime::SystemCall; +use node_template_runtime::Block; use sc_cli::{ChainSpec, RuntimeVersion, SubstrateCli}; use sc_service::PartialComponents; -use std::sync::Arc; - impl SubstrateCli for Cli { fn impl_name() -> String { "Substrate Node".into() @@ -49,47 +46,6 @@ impl SubstrateCli for Cli { } } -use super::service::FullClient; -struct ExtrinsicGen { - client: Arc, -} -use node_runtime::UncheckedExtrinsic; -use node_template_runtime::Block; -use sp_keyring::Sr25519Keyring; -use sp_runtime::{traits::Block as BlockT, OpaqueExtrinsic}; -/* -impl frame_benchmarking_cli::block::cmd::ExtrinsicGenerator for ExtrinsicGen { - fn noop(&self, nonce: u32) -> Option { - let src = Sr25519Keyring::Alice.pair(); - - let extrinsic: OpaqueExtrinsic = create_extrinsic( - self.client.as_ref(), - src.clone(), - SystemCall::remark { remark: vec![] }, - Some(nonce), - ) - .into(); - None - } -}*/ - -#[derive(Default)] -struct InherentProv {} - -impl frame_benchmarking_cli::block::cmd::BlockInherentDataProvider for InherentProv { - fn providers( - &self, - block: u64, - ) -> std::result::Result>, sp_inherents::Error> - { - log::info!("Creating inherents for block #{}", block); - let d = std::time::Duration::from_millis(0); - let timestamp = sp_timestamp::InherentDataProvider::new(d.into()); - - Ok(vec![Arc::new(timestamp)]) - } -} - /// Parse and run command line arguments pub fn run() -> sc_cli::Result<()> { let cli = Cli::from_args(); @@ -142,35 +98,6 @@ pub fn run() -> sc_cli::Result<()> { Ok((cmd.run(client, backend), task_manager)) }) }, - Some(Subcommand::BenchmarkBlock(cmd)) => { - unimplemented!(); - }, - /*let runner = cli.create_runner(cmd)?; - runner.async_run(|mut config| { - config.role = sc_service::Role::Full; - - let PartialComponents { client, task_manager, backend, import_queue, .. } = - service::new_partial(&config)?; - let db = backend.expose_db(); - let storage = backend.expose_storage(); - let ext_gen = ExtrinsicGen { client: client.clone() }; - let provs: InherentProv = Default::default(); - - Ok(( - cmd.run( - config, - client.clone(), - import_queue, - db, - storage, - client.clone(), - Arc::new(provs), - Arc::new(ext_gen), - ), - task_manager, - )) - }) - },*/ Some(Subcommand::Benchmark(cmd)) => if cfg!(feature = "runtime-benchmarks") { let runner = cli.create_runner(cmd)?; diff --git a/bin/node-template/node/src/service.rs b/bin/node-template/node/src/service.rs index e2a8cb4ed834b..fc7dc9b978df3 100644 --- a/bin/node-template/node/src/service.rs +++ b/bin/node-template/node/src/service.rs @@ -31,7 +31,7 @@ impl sc_executor::NativeExecutionDispatch for ExecutorDispatch { } } -pub(crate) type FullClient = +type FullClient = sc_service::TFullClient>; type FullBackend = sc_service::TFullBackend; type FullSelectChain = sc_consensus::LongestChain; diff --git a/bin/node/cli/Cargo.toml b/bin/node/cli/Cargo.toml index a5c5c0530b854..b4a91712fd16c 100644 --- a/bin/node/cli/Cargo.toml +++ b/bin/node/cli/Cargo.toml @@ -96,12 +96,6 @@ frame-benchmarking-cli = { version = "4.0.0-dev", optional = true, path = "../.. node-inspect = { version = "0.9.0-dev", optional = true, path = "../inspect" } try-runtime-cli = { version = "0.10.0-dev", optional = true, path = "../../../utils/frame/try-runtime/cli" } -# OTY added -sp-consensus-slots = { version = "0.10.0-dev", path = "../../../primitives/consensus/slots" } -# OTY -sp-blockchain = { version = "4.0.0-dev", path = "../../../primitives/blockchain" } -chrono = "0.4.19" - [target.'cfg(any(target_arch="x86_64", target_arch="aarch64"))'.dependencies] node-executor = { version = "3.0.0-dev", path = "../executor", features = ["wasmtime"] } sc-cli = { version = "0.10.0-dev", optional = true, path = "../../../client/cli", features = ["wasmtime"] } diff --git a/bin/node/cli/src/cli.rs b/bin/node/cli/src/cli.rs index b6c6da310822d..386215854b963 100644 --- a/bin/node/cli/src/cli.rs +++ b/bin/node/cli/src/cli.rs @@ -42,10 +42,6 @@ pub enum Subcommand { #[clap(name = "benchmark", about = "Benchmark runtime pallets.")] Benchmark(frame_benchmarking_cli::BenchmarkCmd), - /// Sub command for benchmarking the storage speed. - #[clap(name = "benchmark-block", about = "Benchmark TODO.")] - BenchmarkBlock(frame_benchmarking_cli::BlockCmd), - /// 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 index 0a181f3b9b44f..cc6480bb90d55 100644 --- a/bin/node/cli/src/command.rs +++ b/bin/node/cli/src/command.rs @@ -18,12 +18,10 @@ use crate::{chain_spec, service, service::new_partial, Cli, Subcommand}; use node_executor::ExecutorDispatch; -use node_runtime::RuntimeApi; +use node_runtime::{Block, 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() @@ -70,46 +68,6 @@ impl SubstrateCli for Cli { &node_runtime::VERSION } } -struct ExtrinsicGen { - client: Arc, -} -use crate::service::{create_extrinsic, FullClient}; -use node_primitives::Block; -use node_runtime::{BalancesCall, SystemCall}; -use sp_keyring::Sr25519Keyring; -use sp_runtime::AccountId32; -use sp_runtime::{traits::Block as BlockT, MultiAddress, OpaqueExtrinsic}; -impl frame_benchmarking_cli::block::cmd::ExtrinsicGenerator for ExtrinsicGen { - fn noop(&self, nonce: u32) -> Option { - let src = Sr25519Keyring::Alice.pair(); - - let extrinsic: OpaqueExtrinsic = create_extrinsic( - self.client.as_ref(), - src.clone(), - SystemCall::remark { remark: vec![] }, - Some(nonce), - ) - .into(); - Some(extrinsic) - } -} - -#[derive(Default)] -struct InherentProv {} - -impl frame_benchmarking_cli::block::cmd::BlockInherentDataProvider for InherentProv { - fn providers( - &self, - block: u64, - ) -> std::result::Result>, sp_inherents::Error> - { - log::info!("Creating inherents for block #{}", block); - let d = std::time::Duration::from_millis(0); - let timestamp = sp_timestamp::InherentDataProvider::new(d.into()); - - Ok(vec![Arc::new(timestamp)]) - } -} /// Parse command line arguments into service configuration. pub fn run() -> Result<()> { @@ -137,32 +95,6 @@ pub fn run() -> Result<()> { You can enable it with `--features runtime-benchmarks`." .into()) }, - Some(Subcommand::BenchmarkBlock(cmd)) => { - let runner = cli.create_runner(cmd)?; - runner.async_run(|mut config| { - config.role = sc_service::Role::Full; - - let PartialComponents { client, task_manager, backend, .. } = new_partial(&config)?; - let db = backend.expose_db(); - let storage = backend.expose_storage(); - let ext_gen = ExtrinsicGen { client: client.clone() }; - - let provs: InherentProv = Default::default(); - Ok(( - cmd.run( - config, - client.clone(), - client.clone(), - db, - storage, - client.clone(), - Arc::new(provs), - Arc::new(ext_gen), - ), - task_manager, - )) - }) - }, Some(Subcommand::BenchmarkStorage(cmd)) => { if !cfg!(feature = "runtime-benchmarks") { return Err("Benchmarking wasn't enabled when building the node. \ diff --git a/frame/aura/src/lib.rs b/frame/aura/src/lib.rs index 83fb6cb392a27..657965c60a3f1 100644 --- a/frame/aura/src/lib.rs +++ b/frame/aura/src/lib.rs @@ -282,6 +282,9 @@ impl OnTimestampSet for Pallet { let timestamp_slot = moment / slot_duration; let timestamp_slot = Slot::from(timestamp_slot.saturated_into::()); - assert!(CurrentSlot::::get() == timestamp_slot, "sdaf`"); + assert!( + CurrentSlot::::get() == timestamp_slot, + "Timestamp slot must match `CurrentSlot`" + ); } } diff --git a/frame/babe/src/lib.rs b/frame/babe/src/lib.rs index 22d0fd62891a6..f673c8b43bee0 100644 --- a/frame/babe/src/lib.rs +++ b/frame/babe/src/lib.rs @@ -842,13 +842,10 @@ impl OnTimestampSet for Pallet { let timestamp_slot = moment / slot_duration; let timestamp_slot = Slot::from(timestamp_slot.saturated_into::()); - let c = CurrentSlot::::get(); - if c != timestamp_slot { - panic!( - "d: {:?}, moment: {:?}, want: {:?}, got: {:?}", - slot_duration, moment, c, timestamp_slot - ); - } + assert!( + CurrentSlot::::get() == timestamp_slot, + "Timestamp slot must match `CurrentSlot`" + ); } } diff --git a/utils/frame/benchmarking-cli/Cargo.toml b/utils/frame/benchmarking-cli/Cargo.toml index 46c7b985c3dbd..20122c4279366 100644 --- a/utils/frame/benchmarking-cli/Cargo.toml +++ b/utils/frame/benchmarking-cli/Cargo.toml @@ -49,12 +49,6 @@ hex = "0.4.3" memory-db = "0.29.0" rand = { version = "0.8.4", features = ["small_rng"] } -sc-block-builder = { version = "0.10.0-dev", path = "../../../client/block-builder" } -sp-inherents = { version = "4.0.0-dev", path = "../../../primitives/inherents" } -futures = "0.3.19" -sp-consensus = { version = "0.10.0-dev", path = "../../../primitives/consensus/common" } -sc-consensus = { version = "0.10.0-dev", path = "../../../client/consensus/common" } - [features] default = ["db", "sc-client-db/runtime-benchmarks"] db = ["sc-client-db/with-kvdb-rocksdb", "sc-client-db/with-parity-db"] diff --git a/utils/frame/benchmarking-cli/src/block/cmd.rs b/utils/frame/benchmarking-cli/src/block/cmd.rs deleted file mode 100644 index 4a6a9dfb33f37..0000000000000 --- a/utils/frame/benchmarking-cli/src/block/cmd.rs +++ /dev/null @@ -1,303 +0,0 @@ -// This file is part of Substrate. - -// Copyright (C) 2022 Parity Technologies (UK) Ltd. -// SPDX-License-Identifier: Apache-2.0 - -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -use sc_cli::{CliConfiguration, DatabaseParams, PruningParams, Result, SharedParams}; -use sc_client_api::{Backend as ClientBackend, StorageProvider, UsageProvider}; -use sc_client_db::DbHash; -use sc_service::Configuration; -use sp_blockchain::HeaderBackend; -use sp_database::{ColumnId, Database}; -use sp_runtime::{ - OpaqueExtrinsic, - traits::{Block as BlockT, HashFor}, - transaction_validity::{InvalidTransaction, TransactionValidityError}, -}; -use sp_state_machine::Storage; -use sp_storage::StateVersion; -use sp_inherents::InherentData; -use sc_block_builder::{BlockBuilderProvider, BlockBuilder}; -use sc_block_builder::BlockBuilderApi; -use sp_api::{ApiExt, ProvideRuntimeApi}; -use sc_consensus::{ - block_import::{BlockImportParams, ForkChoiceStrategy}, - BlockImport, StateAction, -}; -use sp_blockchain::{ApplyExtrinsicFailed::Validity, Error::ApplyExtrinsicFailed}; -use sp_consensus::BlockOrigin; -use sp_api::BlockId; - -use clap::{Args, Parser}; -use log::info; -use rand::prelude::*; -use serde::Serialize; -use std::{boxed::Box, fmt::Debug, sync::Arc, time::Instant}; - -use crate::storage::{record::{Stats, StatSelect}, template::TemplateData}; - -#[derive(Debug, Parser)] -pub struct BlockCmd { - #[allow(missing_docs)] - #[clap(flatten)] - pub shared_params: SharedParams, - - #[allow(missing_docs)] - #[clap(flatten)] - pub database_params: DatabaseParams, - - #[allow(missing_docs)] - #[clap(flatten)] - pub pruning_params: PruningParams, - - #[allow(missing_docs)] - #[clap(flatten)] - pub params: BlockParams, -} - -#[derive(Debug, Default, Serialize, Clone, PartialEq, Args)] -pub struct BlockParams { - /// Path to write the *weight* file to. Can be a file or directory. - /// For substrate this should be `frame/support/src/weights`. - #[clap(long, default_value = ".")] - pub weight_path: String, - - /// Select a specific metric to calculate the final weight output. - #[clap(long = "metric", default_value = "average")] - pub weight_metric: StatSelect, - - /// Multiply the resulting weight with the given factor. Must be positive. - /// Is calculated before `weight_add`. - #[clap(long = "mul", default_value = "1")] - pub weight_mul: f64, - - /// Add the given offset to the resulting weight. - /// Is calculated after `weight_mul`. - #[clap(long = "add", default_value = "0")] - pub weight_add: u64, - - /// Skip the `read` benchmark. - #[clap(long)] - pub skip_read: bool, - - /// Skip the `write` benchmark. - #[clap(long)] - pub skip_write: bool, - - /// Rounds of warmups before measuring. - /// Only supported for `read` benchmarks. - #[clap(long, default_value = "1")] - pub warmups: u32, - - /// State cache size. - #[clap(long, default_value = "0")] - pub state_cache_size: usize, - - /// Limit them number of extrinsics to put in a block. - #[clap(long)] - pub max_ext_per_block: Option, - - /// Repeats for measuring the time of a an empty block execution. - #[clap(long, default_value = "1000")] - pub repeat_empty_block: u32, - - /// Repeats for measuring the time of a a full block execution. - #[clap(long, default_value = "100")] - pub repeat_full_block: u32, -} - -/// The results of multiple runs in ns. -type BenchRecord = Vec; - -pub trait ExtrinsicGenerator { - fn noop(&self, nonce: u32) -> Option; -} - -impl BlockCmd { - pub async fn run( - &self, - cfg: Configuration, - client: Arc, - mut iq: IQ, - db: (Arc>, ColumnId), - storage: Arc>>, - ra: Arc, - idps: Arc, - ext_gen: Arc, - ) -> Result<()> - where - // TODO remove the Extrinsic = OpaqueExtrinsic - Block: BlockT, - BA: ClientBackend, - C: BlockBuilderProvider - + UsageProvider - + StorageProvider - + HeaderBackend, - RA: ProvideRuntimeApi, - IQ: sc_consensus::BlockImport, - API: ApiExt + BlockBuilderApi, - { - let genesis = BlockId::Number(client.info().best_number); - //let mut template = TemplateData::new(&cfg, &self.params); - info!("Creating empty block"); - // Block builders are not cloneable, so we need two. - let mut build_empty = client.new_block(Default::default()).unwrap(); - let mut build_full = client.new_block(Default::default()).unwrap(); - let mut data = sp_inherents::InherentData::new(); - idps.provide_inherent_data(&mut data).unwrap(); - info!("Creating inherents from {} inherent datas", data.len()); - let inherents = build_empty.create_inherents(data).unwrap(); - info!("Inserting {} inherent extrinsic", inherents.len()); - for ext in inherents.clone() { - build_empty.push(ext.clone())?; - build_full.push(ext.clone())?; - } - // Build a block with only inherents aka empty. - let empty_block = build_empty.build()?.block; - // Interesting part here: - // Benchmark 1; Execution time of a an empty block. - let mut rec_empty = BenchRecord::new(); - info!("Executing an empty block {} times", self.params.repeat_empty_block); - for i in 0..self.params.repeat_empty_block { - let block = empty_block.clone(); - let start = Instant::now(); - ra.runtime_api().execute_block(&genesis, block).unwrap(); - rec_empty.push(start.elapsed().as_nanos() as u64); - } - - info!("Estimating max NO-OPs per block, capped at {}", self.max_ext_per_block()); - let mut noops = Vec::new(); - for nonce in 0..self.max_ext_per_block() { - let ext = ext_gen.noop(nonce).expect("Need noop"); - match build_full.push(ext.clone()) { - Ok(_) => {}, - Err(ApplyExtrinsicFailed(Validity(TransactionValidityError::Invalid( - InvalidTransaction::ExhaustsResources, - )))) => break, - Err(error) => panic!("{}", error), - } - noops.push(ext); - } - info!("Max NO-OPs per block {}, continuing with benchmark.", noops.len()); - let full_block = build_full.build()?.block; - - // Interesting part here: - // Benchmark 2; Execution time of a block filled with NO-OPs. - let mut rec_full = BenchRecord::new(); - info!("Executing a full block {} times", self.params.repeat_full_block); - for i in 0..self.params.repeat_full_block { - let block = full_block.clone(); - let start = Instant::now(); - ra.runtime_api().execute_block(&genesis, block).unwrap(); - let took = start.elapsed().as_nanos(); - // Divide by the number of extrinsics in a block. - rec_full.push(took as u64 / noops.len() as u64); - } - - let start = Instant::now(); - Self::must_import(full_block, &mut iq); - let speed = noops.len() as f32 / start.elapsed().as_secs_f32(); - info!("Imported block {:.2} NO-OP/s", speed); - - let stats = Stats::new(&rec_empty).unwrap(); - info!("Executing an empty block [ns]:\n{:?}", stats); - - let stats = Stats::new(&rec_full).unwrap(); - info!("Executing NO-OP extrinsic [ns]:\n{:?}", stats); - - Ok(()) - } - - /*fn new_block<'a, Block, BA, RA, C>(client: Arc, idps: Arc,) -> Result> - where Block: BlockT, - C: BlockBuilderProvider + 'a, - RA: sp_api::ProvideRuntimeApi, - BA: sc_client_api::Backend { - let builder = client.new_block(Default::default()).unwrap(); - Ok(builder) - }*/ - - fn must_import(block: B, iq: &mut IQ) - where - B: BlockT, - IQ: sc_consensus::BlockImport, - { - let mut params = BlockImportParams::new(BlockOrigin::File, block.header().clone()); - params.body = Some(block.extrinsics().to_vec()); - params.fork_choice = Some(ForkChoiceStrategy::LongestChain); - let block_hash = block.hash(); - let import = - futures::executor::block_on(iq.import_block(params, Default::default())).unwrap(); - if !matches!(import, sc_consensus::ImportResult::Imported(_)) { - panic!("Could not import block: {:?}", import) - } - } - - /// Creates an rng from a random seed. - pub(crate) fn setup_rng() -> impl rand::Rng { - let seed = rand::thread_rng().gen::(); - info!("Using seed {}", seed); - StdRng::seed_from_u64(seed) - } - - fn max_ext_per_block(&self) -> u32 { - self.params.max_ext_per_block.unwrap_or(u32::MAX) - } -} - -// Boilerplate -impl CliConfiguration for BlockCmd { - fn shared_params(&self) -> &SharedParams { - &self.shared_params - } - - fn database_params(&self) -> Option<&DatabaseParams> { - Some(&self.database_params) - } - - fn pruning_params(&self) -> Option<&PruningParams> { - Some(&self.pruning_params) - } - - fn state_cache_size(&self) -> Result { - Ok(self.params.state_cache_size) - } -} - - - - /*info!("Importing empty block as warmup"); - let block = block_builder.build()?.block; - info!("Executing block"); - ra.runtime_api().execute_block(&BlockId::Number(genesis), block.clone()).expect("Must execute"); - info!("Importing block"); - Self::must_import(block, &mut iq); - - info!("Creating empty block"); - let mut block_builder = client.new_block(Default::default()).unwrap(); - - let idps = block_inherents.providers(1).unwrap(); - info!("Creating inherent data from {} providers", idps.len()); - - let mut data = sp_inherents::InherentData::new(); - for idp in idps.clone() { - idp.provide_inherent_data(&mut data).unwrap(); - } - info!("Creating inherents from {} inherent datas", data.len()); - let inherents = block_builder.create_inherents(data).unwrap(); - info!("Inserting {} extrinsic", inherents.len()); - for ext in inherents.clone() { - block_builder.push(ext.clone())?; - }*/ diff --git a/utils/frame/benchmarking-cli/src/block/mod.rs b/utils/frame/benchmarking-cli/src/block/mod.rs deleted file mode 100644 index 4e33e24202b1c..0000000000000 --- a/utils/frame/benchmarking-cli/src/block/mod.rs +++ /dev/null @@ -1,20 +0,0 @@ -// This file is part of Substrate. - -// Copyright (C) 2022 Parity Technologies (UK) Ltd. -// SPDX-License-Identifier: Apache-2.0 - -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -pub mod cmd; - -pub use cmd::BlockCmd; diff --git a/utils/frame/benchmarking-cli/src/block/weights.hbs b/utils/frame/benchmarking-cli/src/block/weights.hbs deleted file mode 100644 index eafdd1114e766..0000000000000 --- a/utils/frame/benchmarking-cli/src/block/weights.hbs +++ /dev/null @@ -1,87 +0,0 @@ -// This file is part of Substrate. - -// Copyright (C) 2022 Parity Technologies (UK) Ltd. -// SPDX-License-Identifier: Apache-2.0 - -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION {{version}} -//! DATE: {{date}} -//! -//! DATABASE: `{{db_name}}`, RUNTIME: `{{runtime_name}}` -//! SKIP-WRITE: `{{params.skip_write}}`, SKIP-READ: `{{params.skip_read}}`, WARMUPS: `{{params.warmups}}` -//! STATE-VERSION: `V{{params.state_version}}`, STATE-CACHE-SIZE: `{{params.state_cache_size}}` -//! WEIGHT-PATH: `{{params.weight_path}}` -//! METRIC: `{{params.weight_metric}}`, WEIGHT-MUL: `{{params.weight_mul}}`, WEIGHT-ADD: `{{params.weight_add}}` - -// Executed Command: -{{#each args as |arg|}} -// {{arg}} -{{/each}} - -pub mod constants { - use frame_support::{ - parameter_types, - weights::{Weight, constants, RuntimeDbWeight}, - }; - - parameter_types! { - /// Importing a block with 0 txs. - pub const BlockExecutionWeight: Weight = {{underscore block}} * constants::WEIGHT_PER_NANOS; - /// Executing a System remarks (no-op) txs. - pub const ExtrinsicBaseWeight: Weight = {{underscore extrinsic}} * constants::WEIGHT_PER_NANOS; - } - - #[cfg(test)] - mod test_base_weights { - use frame_support::weights::constants; - - /// Checks that the block weight exist and is sane. - // NOTE: If this test fails but you are sure that the generated values are fine, - // you can delete it. - #[test] - fn sane_block() { - use super::constants::BlockExecutionWeight as W; - - // At least 100 µs. - assert!( - W::get() >= 100 * constants::WEIGHT_PER_MICROS, - "Weight should be at least 100 µs." - ); - // At most 50 ms. - assert!( - W::get() <= 10 * constants::WEIGHT_PER_MILLIS, - "Weight should be at most 10 ms." - ); - } - - /// Checks that the extrinsic weight exist and is sane. - // NOTE: If this test fails but you are sure that the generated values are fine, - // you can delete it. - #[test] - fn sane_extrinsic() { - use super::constants::ExtrinsicBaseWeight as W; - - // At least 10 µs. - assert!( - W::get() >= 10 * constants::WEIGHT_PER_MICROS, - "Weight should be at least 100 µs." - ); - // At most 1 ms. - assert!( - W::get() <= constants::WEIGHT_PER_MILLIS, - "Weight should be at most 10 ms." - ); - } - } -} diff --git a/utils/frame/benchmarking-cli/src/lib.rs b/utils/frame/benchmarking-cli/src/lib.rs index 7de6929b2eafb..56aab0321ccd0 100644 --- a/utils/frame/benchmarking-cli/src/lib.rs +++ b/utils/frame/benchmarking-cli/src/lib.rs @@ -15,7 +15,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -pub mod block; mod command; mod storage; mod writer; @@ -23,7 +22,6 @@ mod writer; use sc_cli::{ExecutionStrategy, WasmExecutionMethod}; use std::{fmt::Debug, path::PathBuf}; -pub use block::BlockCmd; pub use storage::StorageCmd; // Add a more relaxed parsing for pallet names by allowing pallet directory names with `-` to be diff --git a/utils/frame/benchmarking-cli/src/storage/record.rs b/utils/frame/benchmarking-cli/src/storage/record.rs index 3bb1ec21965fb..00a613c713007 100644 --- a/utils/frame/benchmarking-cli/src/storage/record.rs +++ b/utils/frame/benchmarking-cli/src/storage/record.rs @@ -154,7 +154,7 @@ impl Stats { fn percentile(mut xs: Vec, p: f64) -> u64 { xs.sort(); let index = (xs.len() as f64 * p).ceil() as usize; - xs[index-1] + xs[index] } } diff --git a/utils/frame/benchmarking-cli/src/storage/write.rs b/utils/frame/benchmarking-cli/src/storage/write.rs index 49ebbcc2349a9..eb9ba11f30696 100644 --- a/utils/frame/benchmarking-cli/src/storage/write.rs +++ b/utils/frame/benchmarking-cli/src/storage/write.rs @@ -51,7 +51,6 @@ impl StorageCmd { let mut record = BenchRecord::default(); let supports_rc = db.supports_ref_counting(); - // TODO use HeaderBackend.info() let block = BlockId::Number(client.usage_info().chain.best_number); let header = client.header(block)?.ok_or("Header not found")?; let original_root = *header.state_root(); From 60d2ebdd3dc2d136a086092907b027c7bdaeb0ae Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Mon, 7 Mar 2022 14:55:33 +0000 Subject: [PATCH 03/29] Add block and extrinsic benchmarks --- bin/node-template/node/src/cli.rs | 3 + bin/node-template/node/src/command.rs | 9 + bin/node/cli/src/cli.rs | 4 + bin/node/cli/src/command.rs | 9 + utils/frame/benchmarking-cli/Cargo.toml | 4 +- .../frame/benchmarking-cli/src/block/bench.rs | 233 ++++++++++++++++++ utils/frame/benchmarking-cli/src/block/cmd.rs | 121 +++++++++ utils/frame/benchmarking-cli/src/block/mod.rs | 22 ++ .../benchmarking-cli/src/block/template.rs | 103 ++++++++ .../benchmarking-cli/src/block/weights.hbs | 82 ++++++ utils/frame/benchmarking-cli/src/lib.rs | 3 + .../src/post_processing/mod.rs | 42 ++++ .../frame/benchmarking-cli/src/storage/cmd.rs | 41 ++- .../benchmarking-cli/src/storage/record.rs | 2 +- .../benchmarking-cli/src/storage/template.rs | 16 +- 15 files changed, 655 insertions(+), 39 deletions(-) create mode 100644 utils/frame/benchmarking-cli/src/block/bench.rs create mode 100644 utils/frame/benchmarking-cli/src/block/cmd.rs create mode 100644 utils/frame/benchmarking-cli/src/block/mod.rs create mode 100644 utils/frame/benchmarking-cli/src/block/template.rs create mode 100644 utils/frame/benchmarking-cli/src/block/weights.hbs create mode 100644 utils/frame/benchmarking-cli/src/post_processing/mod.rs diff --git a/bin/node-template/node/src/cli.rs b/bin/node-template/node/src/cli.rs index c4d27b71e4994..36f0f5e93e81b 100644 --- a/bin/node-template/node/src/cli.rs +++ b/bin/node-template/node/src/cli.rs @@ -36,6 +36,9 @@ pub enum Subcommand { /// Revert the chain to a previous state. Revert(sc_cli::RevertCmd), + #[clap(name = "benchmark")] + BenchmarkBlock(frame_benchmarking_cli::block::cmd::BlockCmd), + /// The custom benchmark subcommand benchmarking runtime pallets. #[clap(name = "benchmark", about = "Benchmark runtime pallets.")] Benchmark(frame_benchmarking_cli::BenchmarkCmd), diff --git a/bin/node-template/node/src/command.rs b/bin/node-template/node/src/command.rs index 72c7a75b387bb..e81e86c7e1194 100644 --- a/bin/node-template/node/src/command.rs +++ b/bin/node-template/node/src/command.rs @@ -98,6 +98,15 @@ pub fn run() -> sc_cli::Result<()> { Ok((cmd.run(client, backend), task_manager)) }) }, + Some(Subcommand::BenchmarkBlock(cmd)) => { + // TODO feature check + let runner = cli.create_runner(cmd)?; + runner.async_run(|config| { + let PartialComponents { client, task_manager, .. } = service::new_partial(&config)?; + + Ok((cmd.run(config, client), task_manager)) + }) + }, Some(Subcommand::Benchmark(cmd)) => if cfg!(feature = "runtime-benchmarks") { let runner = cli.create_runner(cmd)?; diff --git a/bin/node/cli/src/cli.rs b/bin/node/cli/src/cli.rs index 386215854b963..7053390c74861 100644 --- a/bin/node/cli/src/cli.rs +++ b/bin/node/cli/src/cli.rs @@ -42,6 +42,10 @@ pub enum Subcommand { #[clap(name = "benchmark", about = "Benchmark runtime pallets.")] Benchmark(frame_benchmarking_cli::BenchmarkCmd), + /// Sub command for benchmarking block and extrinsic base weight. + #[clap(name = "benchmark-block", about = "Benchmark block and extrinsic bas weight.")] + BenchmarkBlock(frame_benchmarking_cli::block::cmd::BlockCmd), + /// 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 index cc6480bb90d55..1cc132fa757b3 100644 --- a/bin/node/cli/src/command.rs +++ b/bin/node/cli/src/command.rs @@ -95,6 +95,15 @@ pub fn run() -> Result<()> { You can enable it with `--features runtime-benchmarks`." .into()) }, + Some(Subcommand::BenchmarkBlock(cmd)) => { + // TODO feature check + let runner = cli.create_runner(cmd)?; + runner.async_run(|config| { + let PartialComponents { client, task_manager, .. } = new_partial(&config)?; + + Ok((cmd.run(config, client), task_manager)) + }) + }, Some(Subcommand::BenchmarkStorage(cmd)) => { if !cfg!(feature = "runtime-benchmarks") { return Err("Benchmarking wasn't enabled when building the node. \ diff --git a/utils/frame/benchmarking-cli/Cargo.toml b/utils/frame/benchmarking-cli/Cargo.toml index 20122c4279366..131bb5638a34c 100644 --- a/utils/frame/benchmarking-cli/Cargo.toml +++ b/utils/frame/benchmarking-cli/Cargo.toml @@ -15,7 +15,8 @@ targets = ["x86_64-unknown-linux-gnu"] [dependencies] 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" } +futures = "0.3.19" 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" } @@ -23,6 +24,7 @@ sc-client-db = { version = "0.10.0-dev", path = "../../../client/db" } sc-executor = { version = "0.10.0-dev", path = "../../../client/executor" } sp-api = { version = "4.0.0-dev", path = "../../../primitives/api" } +sp-core = { version = "6.0.0", path = "../../../primitives/core" } 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" } diff --git a/utils/frame/benchmarking-cli/src/block/bench.rs b/utils/frame/benchmarking-cli/src/block/bench.rs new file mode 100644 index 0000000000000..6730f1c6b0e2c --- /dev/null +++ b/utils/frame/benchmarking-cli/src/block/bench.rs @@ -0,0 +1,233 @@ +// This file is part of Substrate. + +// Copyright (C) 2022 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +use sc_block_builder::BlockBuilderApi; +use sc_cli::Result; +use sc_client_api::{BlockBackend, UsageProvider}; +use sc_client_db::DbHash; +use sp_api::{ApiExt, BlockId, HeaderT, ProvideRuntimeApi}; +use sp_blockchain::HeaderBackend; +use sp_runtime::{traits::Block as BlockT, DigestItem}; + +use clap::Args; +use log::{info, warn}; +use serde::Serialize; +use std::{fmt, marker::PhantomData, sync::Arc, time::Instant}; + +/// Parameters to configure a block benchmark. +#[derive(Debug, Default, Serialize, Clone, PartialEq, Args)] +pub struct BenchmarkParams { + /// Skip benchmarking NO-OP extrinsics. + #[clap(long)] + pub skip_extrinsic: bool, + + /// Skip benchmark an empty block. + #[clap(long)] + pub skip_block: bool, + + /// Specify the number of a full block. 0 is treated as last block. + /// The block should be filled with `System::remark` extrinsics. + #[clap(long, default_value = "0")] + pub full_block: u32, + + /// Specify the number of an empty block. 0 is treated as last block. + /// The block should not contains any user extrinsics but only inherents. + #[clap(long, default_value = "0")] + pub empty_block: u32, + + /// How many executions should be measured. + #[clap(long, default_value = "100")] + pub repeat: u32, + + /// How many executions should be run as warmup. + #[clap(long, default_value = "100")] + pub warmup: u32, + + /// Maximum number of inherents that can be present in a block + /// such that the block will still be considered empty. + /// + /// Default is 1 since that is the case for Substrate. + /// This check exists to make sure that a non-empty block is not + /// accidentally counted as empty. + #[clap(long, default_value = "1")] + pub max_inherents: u32, + + /// Minimum number of extrinsics that must be present in a block + /// such that the block will still be considered full. + /// + /// Default is 12_000 since in Substrate a block can hold that many NO-OPs. + /// To benchmark the speed of a NO-OP extrinsic, there should be + /// as many in the block as possible, for Substrate this is 12_000. + #[clap(long, default_value = "12000")] + pub min_extrinsics: u32, +} + +/// The results of multiple runs in ns. +pub(crate) type BenchRecord = Vec; + +/// Type of +#[derive(Serialize, Clone)] +pub(crate) enum BenchmarkType { + Extrinsic, + Block, +} + +/// Benchmarks the time it takes to execute an empty block or a NO-OP extrinsic. +pub(crate) struct Benchmark { + client: Arc, + params: BenchmarkParams, + no_check: bool, + _p: PhantomData<(B, API)>, +} + +impl Benchmark +where + B: BlockT, + C: UsageProvider + HeaderBackend + BlockBackend + ProvideRuntimeApi, + API: ApiExt + BlockBuilderApi, +{ + pub fn new(client: Arc, params: BenchmarkParams, no_check: bool) -> Self { + Self { client, params, no_check, _p: PhantomData } + } + + /// Benchmarks the execution time of a block. + /// `empty_block` defines if the block should be empty. + pub fn bench(&self, which: BenchmarkType) -> Result { + let block_empty: bool; + let block = match which { + BenchmarkType::Block => { + block_empty = true; + self.load_block(self.params.empty_block)? + }, + BenchmarkType::Extrinsic => { + block_empty = false; + self.load_block(self.params.full_block)? + }, + }; + let parent = BlockId::Hash(*block.header().parent_hash()); + + self.check_block(&block, block_empty)?; + let block = self.unseal(block)?; + self.measure_block(&block, &parent, !block_empty) + } + + /// Loads a block. 0 loads the latest block. + fn load_block(&self, num: u32) -> Result { + let mut num = BlockId::Number(num.into()); + if num == BlockId::Number(0u32.into()) { + num = BlockId::Number(self.client.info().best_number); + } + info!("Loading block {}", num); + + self.client.block(&num)?.map(|b| b.block).ok_or("Could not find block".into()) + } + + /// Checks if the passed block is empty. + /// The resulting error can be demoted to a warning via `--no-check`. + fn check_block(&self, block: &B, want_empty: bool) -> Result<()> { + let num_ext = block.extrinsics().len() as u32; + let is_empty = num_ext <= self.params.max_inherents; + let is_full = num_ext >= self.params.min_extrinsics; + info!("Block contains {} extrinsics", num_ext); + + if want_empty { + match (is_empty, self.no_check) { + (true, _) => {}, + (false, false) => return Err("Block should be empty but was not".into()), + (false, true) => warn!("Treating non-empty block as empty because of --no-check"), + } + } else { + match (is_full, self.no_check) { + (true, _) => {}, + (false, true) => warn!("Treating non-full block as full because of --no-check"), + (false, false) => return Err("Block should be full but was not".into()), + } + } + + Ok(()) + } + + /// Removes the consensus seal from the block if there is any. + fn unseal(&self, block: B) -> Result { + let (mut header, extrinsics) = block.deconstruct(); + match header.digest_mut().pop() { + Some(DigestItem::Seal(_, _)) => {}, + Some(other) => header.digest_mut().push(other), + _ => {}, + } + Ok(B::new(header, extrinsics)) + } + + /// Measures the time that it take to execute a block. + /// `per_ext` specifies if the result should be divided + /// by the number of extrinsics in the block. + /// This is useful for the case that you only want to know + /// how long it takes to execute one extrinsic. + fn measure_block(&self, block: &B, before: &BlockId, per_ext: bool) -> Result { + let num_ext = block.extrinsics().len() as u64; + info!("Executing block with {} extrinsics {} times", num_ext, self.params.repeat); + let mut record = BenchRecord::new(); + + info!("Running warmup..."); + for _ in 0..self.params.warmup { + self.client + .runtime_api() + .execute_block(before, block.clone()) + .expect("Old blocks must execute"); + } + + info!("Measuring execution time"); + // Interesting part here: + // Execute a block multiple times and record each execution time. + for _ in 0..self.params.repeat { + let block = block.clone(); + let start = Instant::now(); + self.client + .runtime_api() + .execute_block(before, block) + .expect("Old blocks must execute"); + + let elapsed = start.elapsed().as_nanos(); + if per_ext { + // Zero div here can only happen with --no-check. + record.push(elapsed as u64 / num_ext); + } else { + record.push(elapsed as u64); + } + } + + Ok(record) + } +} + +impl fmt::Debug for BenchmarkType { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Extrinsic => write!(f, "extrinsic"), + Self::Block => write!(f, "block"), + } + } +} + +impl BenchmarkType { + pub(crate) fn name(&self) -> &'static str { + match self { + Self::Extrinsic => "ExtrinsicBase", + Self::Block => "BlockExecution", + } + } +} diff --git a/utils/frame/benchmarking-cli/src/block/cmd.rs b/utils/frame/benchmarking-cli/src/block/cmd.rs new file mode 100644 index 0000000000000..ba6d3131ac81a --- /dev/null +++ b/utils/frame/benchmarking-cli/src/block/cmd.rs @@ -0,0 +1,121 @@ +// This file is part of Substrate. + +// Copyright (C) 2022 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +use sc_block_builder::BlockBuilderApi; +use sc_cli::{CliConfiguration, DatabaseParams, PruningParams, Result, SharedParams}; +use sc_client_api::{BlockBackend, UsageProvider}; +use sc_client_db::DbHash; +use sc_service::Configuration; +use sp_api::{ApiExt, ProvideRuntimeApi}; +use sp_blockchain::HeaderBackend; +use sp_runtime::traits::Block as BlockT; + +use clap::{Args, Parser}; +use log::info; +use serde::Serialize; +use std::{fmt::Debug, sync::Arc}; + +use super::bench::{Benchmark, BenchmarkParams, BenchmarkType}; +use crate::{block::template::TemplateData, post_processing::WeightParams, storage::record::Stats}; + +/// General parameters for a [`BlockCmd`]. +#[derive(Debug, Parser)] +pub struct BlockCmd { + #[allow(missing_docs)] + #[clap(flatten)] + pub shared_params: SharedParams, + + #[allow(missing_docs)] + #[clap(flatten)] + pub database_params: DatabaseParams, + + #[allow(missing_docs)] + #[clap(flatten)] + pub pruning_params: PruningParams, + + #[allow(missing_docs)] + #[clap(flatten)] + pub params: BlockParams, +} + +/// Specific parameters for a [`BlockCmd`]. +#[derive(Debug, Default, Serialize, Clone, PartialEq, Args)] +pub struct BlockParams { + #[allow(missing_docs)] + #[clap(flatten)] + pub weight: WeightParams, + + #[allow(missing_docs)] + #[clap(flatten)] + pub bench: BenchmarkParams, + + /// Ignore safety checks. Only useful for debugging. + #[clap(long)] + pub no_check: bool, +} + +impl BlockCmd { + /// Run the block and extrinsic benchmark. + pub async fn run(&self, cfg: Configuration, client: Arc) -> Result<()> + where + Block: BlockT, + C: UsageProvider + + HeaderBackend + + BlockBackend + + ProvideRuntimeApi, + API: ApiExt + BlockBuilderApi, + { + let bench = Benchmark::new(client, self.params.bench.clone(), self.params.no_check); + + if !self.params.bench.skip_block { + let record = bench.bench(BenchmarkType::Block)?; + // TODO save json + let stats = Stats::new(&record)?; + info!("Executing an empty block [ns]:\n{:?}", stats); + // TODO weight + let template = TemplateData::new(BenchmarkType::Block, &cfg, &self.params, &stats)?; + template.write(&self.params.weight.weight_path)?; + } + + if !self.params.bench.skip_extrinsic { + let record = bench.bench(BenchmarkType::Extrinsic)?; + // TODO save json + let stats = Stats::new(&record)?; + info!("Executing a NO-OP extrinsic [ns]:\n{:?}", stats); + // TODO weight + let template = TemplateData::new(BenchmarkType::Extrinsic, &cfg, &self.params, &stats)?; + template.write(&self.params.weight.weight_path)?; + } + + Ok(()) + } +} + +// Boilerplate +impl CliConfiguration for BlockCmd { + fn shared_params(&self) -> &SharedParams { + &self.shared_params + } + + fn database_params(&self) -> Option<&DatabaseParams> { + Some(&self.database_params) + } + + fn pruning_params(&self) -> Option<&PruningParams> { + Some(&self.pruning_params) + } +} diff --git a/utils/frame/benchmarking-cli/src/block/mod.rs b/utils/frame/benchmarking-cli/src/block/mod.rs new file mode 100644 index 0000000000000..d68a515ba85b2 --- /dev/null +++ b/utils/frame/benchmarking-cli/src/block/mod.rs @@ -0,0 +1,22 @@ +// This file is part of Substrate. + +// Copyright (C) 2022 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +mod bench; +pub mod cmd; +mod template; + +pub use cmd::BlockCmd; diff --git a/utils/frame/benchmarking-cli/src/block/template.rs b/utils/frame/benchmarking-cli/src/block/template.rs new file mode 100644 index 0000000000000..28b24251e4971 --- /dev/null +++ b/utils/frame/benchmarking-cli/src/block/template.rs @@ -0,0 +1,103 @@ +// This file is part of Substrate. + +// Copyright (C) 2022 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +use sc_cli::Result; +use sc_service::Configuration; + +use log::info; +use serde::Serialize; +use std::{env, fs, path::PathBuf}; + +use crate::{ + block::{bench::BenchmarkType, cmd::BlockParams}, + storage::record::Stats, +}; + +static VERSION: &'static str = env!("CARGO_PKG_VERSION"); +static TEMPLATE: &str = include_str!("./weights.hbs"); + +/// Data consumed by Handlebar to fill out the `weights.hbs` template. +#[derive(Serialize, Debug, Clone)] +pub(crate) struct TemplateData { + template_type: BenchmarkType, + template_name: String, + /// Name of the runtime. Taken from the chain spec. + runtime_name: String, + /// Version of the benchmarking CLI used. + version: String, + /// Date that the template was filled out. + date: String, + /// Command line arguments that were passed to the CLI. + args: Vec, + /// Params of the executed command. + params: BlockParams, + /// Stats about a benchmark result. + stats: Stats, + /// The weight for one `read`. + weight: u64, +} + +impl TemplateData { + /// Returns a new [`Self`] from the given configuration. + pub(crate) fn new( + t: BenchmarkType, + cfg: &Configuration, + params: &BlockParams, + stats: &Stats, + ) -> Result { + let weight = params.weight.calc_weight(stats)?; + + Ok(TemplateData { + template_name: t.name().into(), + template_type: t, + runtime_name: cfg.chain_spec.name().into(), + version: VERSION.into(), + date: chrono::Utc::now().format("%Y-%m-%d (Y/M/D)").to_string(), + args: env::args().collect::>(), + params: params.clone(), + stats: stats.clone(), + weight, + }) + } + + /// Filles out the `weights.hbs` HBS template with its own data. + /// Writes the result to `path` which can be a directory or file. + pub fn write(&self, path: &str) -> Result<()> { + let mut handlebars = handlebars::Handlebars::new(); + // Format large integers with underscore. + handlebars.register_helper("underscore", Box::new(crate::writer::UnderscoreHelper)); + // Don't HTML escape any characters. + handlebars.register_escape_fn(|s| -> String { s.to_string() }); + + let out_path = self.build_path(path); + let mut fd = fs::File::create(&out_path)?; + info!("Writing weights to {:?}", fs::canonicalize(&out_path)?); + handlebars + .render_template_to_write(&TEMPLATE, &self, &mut fd) + .map_err(|e| format!("HBS template write: {:?}", e).into()) + } + + /// Builds a path for the weight file. + fn build_path(&self, weight_out: &str) -> PathBuf { + let mut path = PathBuf::from(weight_out); + if path.is_dir() { + path.push(format!("{:?}_weights.rs", self.template_type).to_lowercase()); + path.set_extension("rs"); + } + path + } +} diff --git a/utils/frame/benchmarking-cli/src/block/weights.hbs b/utils/frame/benchmarking-cli/src/block/weights.hbs new file mode 100644 index 0000000000000..c88f43c6c2066 --- /dev/null +++ b/utils/frame/benchmarking-cli/src/block/weights.hbs @@ -0,0 +1,82 @@ +// This file is part of Substrate. + +// Copyright (C) 2022 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION {{version}} +//! DATE: {{date}} +//! +//! TYPE: `{{template_type}}`, RUNTIME: `{{runtime_name}}` +//! WARMUPS: `{{params.bench.warmup}}`, REPEAT: `{{params.bench.repeat}}` +//! WEIGHT-PATH: `{{params.weight.weight_path}}` +//! WEIGHT-METRIC: `{{params.weight.weight_metric}}`, WEIGHT-MUL: `{{params.weight.weight_mul}}`, WEIGHT-ADD: `{{params.weight.weight_add}}` + +// Executed Command: +{{#each args as |arg|}} +// {{arg}} +{{/each}} + +pub mod constants { + use frame_support::{ + parameter_types, + weights::{constants, Weight}, + }; + + parameter_types! { + {{#if (eq template_type "block")}} + /// Time to execute an empty block. + {{else}} + /// Time to execute a NO-OP extrinsic eg. `System::remark`. + {{/if}} + /// Calculated by multiplying the *{{params.weight.weight_metric}}* with `{{params.weight.weight_mul}}` and adding `{{params.weight.weight_add}}`. + /// + /// Stats [ns]: + /// Min, Max: {{underscore stats.min}}, {{underscore stats.max}} + /// Average: {{underscore stats.avg}} + /// Median: {{underscore stats.median}} + /// StdDev: {{stats.stddev}} + /// + /// Percentiles [ns]: + /// 99th: {{underscore stats.p99}} + /// 95th: {{underscore stats.p95}} + /// 75th: {{underscore stats.p75}} + pub const {{template_name}}Weight: Weight = {{underscore weight}} * constants::WEIGHT_PER_NANOS; + } + + #[cfg(test)] + mod test_weights { + use frame_support::weights::constants; + + /// Checks that the weight exists and is sane. + // NOTE: If this test fails but you are sure that the generated values are fine, + // you can delete it. + #[test] + fn sane() { + let w = super::constants::{{template_name}}Weight::get(); + + {{#if (eq template_type "block")}} + // At least 100 µs. + assert!(w >= 100 * constants::WEIGHT_PER_MICROS, "Weight should be at least 100 µs."); + // At most 50 ms. + assert!(w <= 50 * constants::WEIGHT_PER_MILLIS, "Weight should be at most 50 ms."); + {{else}} + // At least 10 µs. + assert!(w >= 10 * constants::WEIGHT_PER_MICROS, "Weight should be at least 10 µs."); + // At most 1 ms. + assert!(w <= constants::WEIGHT_PER_MILLIS, "Weight should be at most 1 ms."); + {{/if}} + } + } +} diff --git a/utils/frame/benchmarking-cli/src/lib.rs b/utils/frame/benchmarking-cli/src/lib.rs index 56aab0321ccd0..ae937b6b3dad4 100644 --- a/utils/frame/benchmarking-cli/src/lib.rs +++ b/utils/frame/benchmarking-cli/src/lib.rs @@ -15,13 +15,16 @@ // See the License for the specific language governing permissions and // limitations under the License. +pub mod block; mod command; +mod post_processing; mod storage; mod writer; use sc_cli::{ExecutionStrategy, WasmExecutionMethod}; use std::{fmt::Debug, path::PathBuf}; +pub use block::BlockCmd; pub use storage::StorageCmd; // Add a more relaxed parsing for pallet names by allowing pallet directory names with `-` to be diff --git a/utils/frame/benchmarking-cli/src/post_processing/mod.rs b/utils/frame/benchmarking-cli/src/post_processing/mod.rs new file mode 100644 index 0000000000000..00892365493ab --- /dev/null +++ b/utils/frame/benchmarking-cli/src/post_processing/mod.rs @@ -0,0 +1,42 @@ +use sc_cli::Result; + +use clap::Args; +use serde::Serialize; + +use crate::storage::record::{StatSelect, Stats}; + +#[derive(Debug, Default, Serialize, Clone, PartialEq, Args)] +pub struct WeightParams { + /// Path to write the *weight* file to. Can be a file or directory. + /// For substrate this should be `frame/support/src/weights`. + #[clap(long, default_value = ".")] + pub weight_path: String, + + /// Select a specific metric to calculate the final weight output. + #[clap(long = "metric", default_value = "average")] + pub weight_metric: StatSelect, + + /// Multiply the resulting weight with the given factor. Must be positive. + /// Is calculated before `weight_add`. + #[clap(long = "mul", default_value = "1")] + pub weight_mul: f64, + + /// Add the given offset to the resulting weight. + /// Is calculated after `weight_mul`. + #[clap(long = "add", default_value = "0")] + pub weight_add: u64, +} + +/// Calculates the final weight by multiplying the selected metric with +/// `mul` and adding `add`. +/// Does not use safe casts and can overflow. +impl WeightParams { + pub(crate) fn calc_weight(&self, stat: &Stats) -> Result { + if self.weight_mul.is_sign_negative() || !self.weight_mul.is_normal() { + return Err("invalid floating number for `weight_mul`".into()) + } + let s = stat.select(self.weight_metric) as f64; + let w = s.mul_add(self.weight_mul, self.weight_add as f64).ceil(); + Ok(w as u64) // No safe cast here since there is no `From` for `u64`. + } +} diff --git a/utils/frame/benchmarking-cli/src/storage/cmd.rs b/utils/frame/benchmarking-cli/src/storage/cmd.rs index 4376b616286a4..300149c1a03e3 100644 --- a/utils/frame/benchmarking-cli/src/storage/cmd.rs +++ b/utils/frame/benchmarking-cli/src/storage/cmd.rs @@ -15,10 +15,12 @@ // See the License for the specific language governing permissions and // limitations under the License. -use sc_cli::{CliConfiguration, DatabaseParams, PruningParams, Result, SharedParams}; +use sc_cli::{ + CliConfiguration, DatabaseParams, PruningParams, Result, SharedParams, TransactionPoolParams, +}; use sc_client_api::{Backend as ClientBackend, StorageProvider, UsageProvider}; use sc_client_db::DbHash; -use sc_service::Configuration; +use sc_service::{config::TransactionPoolOptions, Configuration}; use sp_blockchain::HeaderBackend; use sp_database::{ColumnId, Database}; use sp_runtime::traits::{Block as BlockT, HashFor}; @@ -31,8 +33,8 @@ use rand::prelude::*; use serde::Serialize; use std::{fmt::Debug, sync::Arc}; -use super::{record::StatSelect, template::TemplateData}; - +use super::template::TemplateData; +use crate::post_processing::WeightParams; /// Benchmark the storage of a Substrate node with a live chain snapshot. #[derive(Debug, Parser)] pub struct StorageCmd { @@ -48,6 +50,10 @@ pub struct StorageCmd { #[clap(flatten)] pub pruning_params: PruningParams, + #[allow(missing_docs)] + #[clap(flatten)] + pub pool_params: TransactionPoolParams, + #[allow(missing_docs)] #[clap(flatten)] pub params: StorageParams, @@ -56,24 +62,9 @@ pub struct StorageCmd { /// Parameters for modifying the benchmark behaviour and the post processing of the results. #[derive(Debug, Default, Serialize, Clone, PartialEq, Args)] pub struct StorageParams { - /// Path to write the *weight* file to. Can be a file or directory. - /// For substrate this should be `frame/support/src/weights`. - #[clap(long, default_value = ".")] - pub weight_path: String, - - /// Select a specific metric to calculate the final weight output. - #[clap(long = "metric", default_value = "average")] - pub weight_metric: StatSelect, - - /// Multiply the resulting weight with the given factor. Must be positive. - /// Is calculated before `weight_add`. - #[clap(long = "mul", default_value = "1")] - pub weight_mul: f64, - - /// Add the given offset to the resulting weight. - /// Is calculated after `weight_mul`. - #[clap(long = "add", default_value = "0")] - pub weight_add: u64, + #[allow(missing_docs)] + #[clap(flatten)] + pub weight_params: WeightParams, /// Skip the `read` benchmark. #[clap(long)] @@ -131,7 +122,7 @@ impl StorageCmd { template.set_stats(None, Some(stats))?; } - template.write(&self.params.weight_path) + template.write(&self.params.weight_params.weight_path) } /// Returns the specified state version. @@ -165,6 +156,10 @@ impl CliConfiguration for StorageCmd { Some(&self.pruning_params) } + fn transaction_pool(&self) -> Result { + Ok(self.pool_params.transaction_pool()) + } + fn state_cache_size(&self) -> Result { Ok(self.params.state_cache_size) } diff --git a/utils/frame/benchmarking-cli/src/storage/record.rs b/utils/frame/benchmarking-cli/src/storage/record.rs index 00a613c713007..710c54bc54c2d 100644 --- a/utils/frame/benchmarking-cli/src/storage/record.rs +++ b/utils/frame/benchmarking-cli/src/storage/record.rs @@ -154,7 +154,7 @@ impl Stats { fn percentile(mut xs: Vec, p: f64) -> u64 { xs.sort(); let index = (xs.len() as f64 * p).ceil() as usize; - xs[index] + xs[index - 1] } } diff --git a/utils/frame/benchmarking-cli/src/storage/template.rs b/utils/frame/benchmarking-cli/src/storage/template.rs index 56e0869a914a1..5e31eb7619fce 100644 --- a/utils/frame/benchmarking-cli/src/storage/template.rs +++ b/utils/frame/benchmarking-cli/src/storage/template.rs @@ -75,11 +75,11 @@ impl TemplateData { write: Option<(Stats, Stats)>, ) -> Result<()> { if let Some(read) = read { - self.read_weight = calc_weight(&read.0, &self.params)?; + self.read_weight = self.params.weight_params.calc_weight(&read.0)?; self.read = Some(read); } if let Some(write) = write { - self.write_weight = calc_weight(&write.0, &self.params)?; + self.write_weight = self.params.weight_params.calc_weight(&write.0)?; self.write = Some(write); } Ok(()) @@ -112,15 +112,3 @@ impl TemplateData { path } } - -/// Calculates the final weight by multiplying the selected metric with -/// `mul` and adding `add`. -/// Does not use safe casts and can overflow. -fn calc_weight(stat: &Stats, params: &StorageParams) -> Result { - if params.weight_mul.is_sign_negative() || !params.weight_mul.is_normal() { - return Err("invalid floating number for `weight_mul`".into()) - } - let s = stat.select(params.weight_metric) as f64; - let w = s.mul_add(params.weight_mul, params.weight_add as f64).ceil(); - Ok(w as u64) // No safe cast here since there is no `From` for `u64`. -} From 8a3a8a80af28db7d3e59776d8eb4d7a5ac46a138 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Mon, 7 Mar 2022 16:29:13 +0100 Subject: [PATCH 04/29] Doc Signed-off-by: Oliver Tale-Yazdi --- Cargo.lock | 2 ++ utils/frame/benchmarking-cli/src/block/bench.rs | 17 ++++++++++------- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index db6edfa81d539..6f8515a54c096 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2129,6 +2129,7 @@ dependencies = [ "clap 3.0.7", "frame-benchmarking", "frame-support", + "futures 0.3.19", "handlebars", "hash-db", "hex", @@ -2139,6 +2140,7 @@ dependencies = [ "memory-db", "parity-scale-codec", "rand 0.8.4", + "sc-block-builder", "sc-cli", "sc-client-api", "sc-client-db", diff --git a/utils/frame/benchmarking-cli/src/block/bench.rs b/utils/frame/benchmarking-cli/src/block/bench.rs index 6730f1c6b0e2c..28eb5282b440a 100644 --- a/utils/frame/benchmarking-cli/src/block/bench.rs +++ b/utils/frame/benchmarking-cli/src/block/bench.rs @@ -100,6 +100,7 @@ where C: UsageProvider + HeaderBackend + BlockBackend + ProvideRuntimeApi, API: ApiExt + BlockBuilderApi, { + /// Create a new benchmark object. `no_check` will ignore some safety checks. pub fn new(client: Arc, params: BenchmarkParams, no_check: bool) -> Self { Self { client, params, no_check, _p: PhantomData } } @@ -178,19 +179,21 @@ where /// This is useful for the case that you only want to know /// how long it takes to execute one extrinsic. fn measure_block(&self, block: &B, before: &BlockId, per_ext: bool) -> Result { - let num_ext = block.extrinsics().len() as u64; - info!("Executing block with {} extrinsics {} times", num_ext, self.params.repeat); let mut record = BenchRecord::new(); + let num_ext = block.extrinsics().len() as u64; + if per_ext && num_ext == 0 { + return Err("Cannot measure extrinsic time of empty block".into()); + } - info!("Running warmup..."); + info!("Running {} warmups...", self.params.warmup); for _ in 0..self.params.warmup { self.client .runtime_api() .execute_block(before, block.clone()) - .expect("Old blocks must execute"); + .expect("Past blocks must execute"); } - info!("Measuring execution time"); + info!("Executing block {} times", self.params.repeat); // Interesting part here: // Execute a block multiple times and record each execution time. for _ in 0..self.params.repeat { @@ -199,11 +202,11 @@ where self.client .runtime_api() .execute_block(before, block) - .expect("Old blocks must execute"); + .expect("Past blocks must execute"); let elapsed = start.elapsed().as_nanos(); if per_ext { - // Zero div here can only happen with --no-check. + // non zero checked above record.push(elapsed as u64 / num_ext); } else { record.push(elapsed as u64); From 8928d9f6760fdc3acae853a443eb3c0d00793fe2 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Mon, 7 Mar 2022 16:57:39 +0100 Subject: [PATCH 05/29] Fix template Signed-off-by: Oliver Tale-Yazdi --- .../benchmarking-cli/src/block/weights.hbs | 28 +++++++++++++------ 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/utils/frame/benchmarking-cli/src/block/weights.hbs b/utils/frame/benchmarking-cli/src/block/weights.hbs index c88f43c6c2066..691c9f32c0e94 100644 --- a/utils/frame/benchmarking-cli/src/block/weights.hbs +++ b/utils/frame/benchmarking-cli/src/block/weights.hbs @@ -31,11 +31,11 @@ pub mod constants { use frame_support::{ parameter_types, - weights::{constants, Weight}, + weights::{constants::WEIGHT_PER_NANOS, Weight}, }; parameter_types! { - {{#if (eq template_type "block")}} + {{#if (eq template_type "Block")}} /// Time to execute an empty block. {{else}} /// Time to execute a NO-OP extrinsic eg. `System::remark`. @@ -52,7 +52,7 @@ pub mod constants { /// 99th: {{underscore stats.p99}} /// 95th: {{underscore stats.p95}} /// 75th: {{underscore stats.p75}} - pub const {{template_name}}Weight: Weight = {{underscore weight}} * constants::WEIGHT_PER_NANOS; + pub const {{template_name}}Weight: Weight = {{underscore weight}} * WEIGHT_PER_NANOS; } #[cfg(test)] @@ -66,16 +66,28 @@ pub mod constants { fn sane() { let w = super::constants::{{template_name}}Weight::get(); - {{#if (eq template_type "block")}} + {{#if (eq template_type "Block")}} // At least 100 µs. - assert!(w >= 100 * constants::WEIGHT_PER_MICROS, "Weight should be at least 100 µs."); + assert!( + w >= 100 * constants::WEIGHT_PER_MICROS, + "Weight should be at least 100 µs." + ); // At most 50 ms. - assert!(w <= 50 * constants::WEIGHT_PER_MILLIS, "Weight should be at most 50 ms."); + assert!( + w <= 50 * constants::WEIGHT_PER_MILLIS, + "Weight should be at most 50 ms." + ); {{else}} // At least 10 µs. - assert!(w >= 10 * constants::WEIGHT_PER_MICROS, "Weight should be at least 10 µs."); + assert!( + w >= 10 * constants::WEIGHT_PER_MICROS, + "Weight should be at least 10 µs." + ); // At most 1 ms. - assert!(w <= constants::WEIGHT_PER_MILLIS, "Weight should be at most 1 ms."); + assert!( + w <= constants::WEIGHT_PER_MILLIS, + "Weight should be at most 1 ms." + ); {{/if}} } } From 6631602818c22dd3c7ae0878241f1cd65df426ed Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Mon, 7 Mar 2022 17:29:46 +0100 Subject: [PATCH 06/29] Beauty fixes Signed-off-by: Oliver Tale-Yazdi --- bin/node/cli/src/cli.rs | 2 +- bin/node/cli/src/command.rs | 7 +- .../frame/benchmarking-cli/src/block/bench.rs | 70 +++++++++---------- utils/frame/benchmarking-cli/src/block/cmd.rs | 25 ++++--- .../benchmarking-cli/src/block/template.rs | 18 ++--- .../benchmarking-cli/src/block/weights.hbs | 4 +- .../src/post_processing/mod.rs | 18 +++++ .../frame/benchmarking-cli/src/storage/cmd.rs | 14 +--- 8 files changed, 84 insertions(+), 74 deletions(-) diff --git a/bin/node/cli/src/cli.rs b/bin/node/cli/src/cli.rs index 7053390c74861..a52bb0d7c2de3 100644 --- a/bin/node/cli/src/cli.rs +++ b/bin/node/cli/src/cli.rs @@ -43,7 +43,7 @@ pub enum Subcommand { Benchmark(frame_benchmarking_cli::BenchmarkCmd), /// Sub command for benchmarking block and extrinsic base weight. - #[clap(name = "benchmark-block", about = "Benchmark block and extrinsic bas weight.")] + #[clap(name = "benchmark-block", about = "Benchmark block and extrinsic base weight.")] BenchmarkBlock(frame_benchmarking_cli::block::cmd::BlockCmd), /// Sub command for benchmarking the storage speed. diff --git a/bin/node/cli/src/command.rs b/bin/node/cli/src/command.rs index 1cc132fa757b3..89ef2c3d4f1e5 100644 --- a/bin/node/cli/src/command.rs +++ b/bin/node/cli/src/command.rs @@ -96,7 +96,12 @@ pub fn run() -> Result<()> { .into()) }, Some(Subcommand::BenchmarkBlock(cmd)) => { - // TODO feature check + if !cfg!(feature = "runtime-benchmarks") { + return Err("Benchmarking wasn't enabled when building the node. \ + You can enable it with `--features runtime-benchmarks`." + .into()) + } + let runner = cli.create_runner(cmd)?; runner.async_run(|config| { let PartialComponents { client, task_manager, .. } = new_partial(&config)?; diff --git a/utils/frame/benchmarking-cli/src/block/bench.rs b/utils/frame/benchmarking-cli/src/block/bench.rs index 28eb5282b440a..a4c22473c1b45 100644 --- a/utils/frame/benchmarking-cli/src/block/bench.rs +++ b/utils/frame/benchmarking-cli/src/block/bench.rs @@ -26,7 +26,9 @@ use sp_runtime::{traits::Block as BlockT, DigestItem}; use clap::Args; use log::{info, warn}; use serde::Serialize; -use std::{fmt, marker::PhantomData, sync::Arc, time::Instant}; +use std::{marker::PhantomData, sync::Arc, time::Instant}; + +use crate::storage::record::Stats; /// Parameters to configure a block benchmark. #[derive(Debug, Default, Serialize, Clone, PartialEq, Args)] @@ -39,12 +41,12 @@ pub struct BenchmarkParams { #[clap(long)] pub skip_block: bool, - /// Specify the number of a full block. 0 is treated as last block. + /// Specify the id of a full block. 0 is treated as last block. /// The block should be filled with `System::remark` extrinsics. #[clap(long, default_value = "0")] pub full_block: u32, - /// Specify the number of an empty block. 0 is treated as last block. + /// Specify the id of an empty block. 0 is treated as last block. /// The block should not contains any user extrinsics but only inherents. #[clap(long, default_value = "0")] pub empty_block: u32, @@ -67,11 +69,9 @@ pub struct BenchmarkParams { pub max_inherents: u32, /// Minimum number of extrinsics that must be present in a block - /// such that the block will still be considered full. + /// such that the block will be considered full. /// /// Default is 12_000 since in Substrate a block can hold that many NO-OPs. - /// To benchmark the speed of a NO-OP extrinsic, there should be - /// as many in the block as possible, for Substrate this is 12_000. #[clap(long, default_value = "12000")] pub min_extrinsics: u32, } @@ -79,10 +79,12 @@ pub struct BenchmarkParams { /// The results of multiple runs in ns. pub(crate) type BenchRecord = Vec; -/// Type of +/// Type of a benchmark. #[derive(Serialize, Clone)] pub(crate) enum BenchmarkType { + /// Extrinsic execution speed was measured. Extrinsic, + /// Empty block execution speed was measured. Block, } @@ -106,24 +108,18 @@ where } /// Benchmarks the execution time of a block. - /// `empty_block` defines if the block should be empty. - pub fn bench(&self, which: BenchmarkType) -> Result { - let block_empty: bool; - let block = match which { - BenchmarkType::Block => { - block_empty = true; - self.load_block(self.params.empty_block)? - }, - BenchmarkType::Extrinsic => { - block_empty = false; - self.load_block(self.params.full_block)? - }, + pub fn bench(&self, which: BenchmarkType) -> Result { + let (id, empty) = match which { + BenchmarkType::Block => (self.params.empty_block, true), + BenchmarkType::Extrinsic => (self.params.full_block, false), }; - let parent = BlockId::Hash(*block.header().parent_hash()); + let block = self.load_block(id)?; + self.check_block(&block, empty)?; + let block = self.unsealed(block)?; - self.check_block(&block, block_empty)?; - let block = self.unseal(block)?; - self.measure_block(&block, &parent, !block_empty) + let parent = BlockId::Hash(*block.header().parent_hash()); + let rec = self.measure_block(&block, &parent, !empty)?; + Stats::new(&rec) } /// Loads a block. 0 loads the latest block. @@ -134,7 +130,7 @@ where } info!("Loading block {}", num); - self.client.block(&num)?.map(|b| b.block).ok_or("Could not find block".into()) + self.client.block(&num)?.map(|b| b.block).ok_or("Could not load block".into()) } /// Checks if the passed block is empty. @@ -154,16 +150,16 @@ where } else { match (is_full, self.no_check) { (true, _) => {}, - (false, true) => warn!("Treating non-full block as full because of --no-check"), (false, false) => return Err("Block should be full but was not".into()), + (false, true) => warn!("Treating non-full block as full because of --no-check"), } } Ok(()) } - /// Removes the consensus seal from the block if there is any. - fn unseal(&self, block: B) -> Result { + /// Removes the consensus seal from a block if there is any. + fn unsealed(&self, block: B) -> Result { let (mut header, extrinsics) = block.deconstruct(); match header.digest_mut().pop() { Some(DigestItem::Seal(_, _)) => {}, @@ -176,13 +172,13 @@ where /// Measures the time that it take to execute a block. /// `per_ext` specifies if the result should be divided /// by the number of extrinsics in the block. - /// This is useful for the case that you only want to know + /// This is useful for the case that you want to know /// how long it takes to execute one extrinsic. fn measure_block(&self, block: &B, before: &BlockId, per_ext: bool) -> Result { let mut record = BenchRecord::new(); let num_ext = block.extrinsics().len() as u64; if per_ext && num_ext == 0 { - return Err("Cannot measure extrinsic time of empty block".into()); + return Err("Cannot measure extrinsic time of empty block".into()) } info!("Running {} warmups...", self.params.warmup); @@ -206,7 +202,7 @@ where let elapsed = start.elapsed().as_nanos(); if per_ext { - // non zero checked above + // non zero checked above. record.push(elapsed as u64 / num_ext); } else { record.push(elapsed as u64); @@ -217,17 +213,17 @@ where } } -impl fmt::Debug for BenchmarkType { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { +impl BenchmarkType { + /// Short name of the benchmark type. + pub(crate) fn short_name(&self) -> &'static str { match self { - Self::Extrinsic => write!(f, "extrinsic"), - Self::Block => write!(f, "block"), + Self::Extrinsic => "extrinsic", + Self::Block => "block", } } -} -impl BenchmarkType { - pub(crate) fn name(&self) -> &'static str { + /// Long name of the benchmark type. + pub(crate) fn long_name(&self) -> &'static str { match self { Self::Extrinsic => "ExtrinsicBase", Self::Block => "BlockExecution", diff --git a/utils/frame/benchmarking-cli/src/block/cmd.rs b/utils/frame/benchmarking-cli/src/block/cmd.rs index ba6d3131ac81a..272388a7bc6a7 100644 --- a/utils/frame/benchmarking-cli/src/block/cmd.rs +++ b/utils/frame/benchmarking-cli/src/block/cmd.rs @@ -29,10 +29,15 @@ use log::info; use serde::Serialize; use std::{fmt::Debug, sync::Arc}; -use super::bench::{Benchmark, BenchmarkParams, BenchmarkType}; -use crate::{block::template::TemplateData, post_processing::WeightParams, storage::record::Stats}; - -/// General parameters for a [`BlockCmd`]. +use crate::{ + block::{ + bench::{Benchmark, BenchmarkParams, BenchmarkType}, + template::TemplateData, + }, + post_processing::WeightParams, +}; + +/// Parameters for a [`BlockCmd`]. #[derive(Debug, Parser)] pub struct BlockCmd { #[allow(missing_docs)] @@ -52,7 +57,7 @@ pub struct BlockCmd { pub params: BlockParams, } -/// Specific parameters for a [`BlockCmd`]. +/// Parameter for a block benchmark and post processing. #[derive(Debug, Default, Serialize, Clone, PartialEq, Args)] pub struct BlockParams { #[allow(missing_docs)] @@ -82,21 +87,15 @@ impl BlockCmd { let bench = Benchmark::new(client, self.params.bench.clone(), self.params.no_check); if !self.params.bench.skip_block { - let record = bench.bench(BenchmarkType::Block)?; - // TODO save json - let stats = Stats::new(&record)?; + let stats = bench.bench(BenchmarkType::Block)?; info!("Executing an empty block [ns]:\n{:?}", stats); - // TODO weight let template = TemplateData::new(BenchmarkType::Block, &cfg, &self.params, &stats)?; template.write(&self.params.weight.weight_path)?; } if !self.params.bench.skip_extrinsic { - let record = bench.bench(BenchmarkType::Extrinsic)?; - // TODO save json - let stats = Stats::new(&record)?; + let stats = bench.bench(BenchmarkType::Extrinsic)?; info!("Executing a NO-OP extrinsic [ns]:\n{:?}", stats); - // TODO weight let template = TemplateData::new(BenchmarkType::Extrinsic, &cfg, &self.params, &stats)?; template.write(&self.params.weight.weight_path)?; } diff --git a/utils/frame/benchmarking-cli/src/block/template.rs b/utils/frame/benchmarking-cli/src/block/template.rs index 28b24251e4971..5711340d63ab2 100644 --- a/utils/frame/benchmarking-cli/src/block/template.rs +++ b/utils/frame/benchmarking-cli/src/block/template.rs @@ -33,8 +33,10 @@ static TEMPLATE: &str = include_str!("./weights.hbs"); /// Data consumed by Handlebar to fill out the `weights.hbs` template. #[derive(Serialize, Debug, Clone)] pub(crate) struct TemplateData { - template_type: BenchmarkType, + /// Name of the benchmark. String version of `template_type`. template_name: String, + /// Path prefix that will be used to save the resulting file. + template_path: String, /// Name of the runtime. Taken from the chain spec. runtime_name: String, /// Version of the benchmarking CLI used. @@ -45,9 +47,9 @@ pub(crate) struct TemplateData { args: Vec, /// Params of the executed command. params: BlockParams, - /// Stats about a benchmark result. + /// Stats about the benchmark result. stats: Stats, - /// The weight for one `read`. + /// The resulting weight in ns. weight: u64, } @@ -62,8 +64,8 @@ impl TemplateData { let weight = params.weight.calc_weight(stats)?; Ok(TemplateData { - template_name: t.name().into(), - template_type: t, + template_name: t.short_name().into(), + template_path: t.long_name().into(), runtime_name: cfg.chain_spec.name().into(), version: VERSION.into(), date: chrono::Utc::now().format("%Y-%m-%d (Y/M/D)").to_string(), @@ -75,10 +77,10 @@ impl TemplateData { } /// Filles out the `weights.hbs` HBS template with its own data. - /// Writes the result to `path` which can be a directory or file. + /// Writes the result to `path` which can be a directory or a file. pub fn write(&self, path: &str) -> Result<()> { let mut handlebars = handlebars::Handlebars::new(); - // Format large integers with underscore. + // Format large integers with underscores. handlebars.register_helper("underscore", Box::new(crate::writer::UnderscoreHelper)); // Don't HTML escape any characters. handlebars.register_escape_fn(|s| -> String { s.to_string() }); @@ -95,7 +97,7 @@ impl TemplateData { fn build_path(&self, weight_out: &str) -> PathBuf { let mut path = PathBuf::from(weight_out); if path.is_dir() { - path.push(format!("{:?}_weights.rs", self.template_type).to_lowercase()); + path.push(format!("{:?}_weights.rs", self.template_name)); path.set_extension("rs"); } path diff --git a/utils/frame/benchmarking-cli/src/block/weights.hbs b/utils/frame/benchmarking-cli/src/block/weights.hbs index 691c9f32c0e94..1377e189db703 100644 --- a/utils/frame/benchmarking-cli/src/block/weights.hbs +++ b/utils/frame/benchmarking-cli/src/block/weights.hbs @@ -35,7 +35,7 @@ pub mod constants { }; parameter_types! { - {{#if (eq template_type "Block")}} + {{#if (eq template_name "block")}} /// Time to execute an empty block. {{else}} /// Time to execute a NO-OP extrinsic eg. `System::remark`. @@ -66,7 +66,7 @@ pub mod constants { fn sane() { let w = super::constants::{{template_name}}Weight::get(); - {{#if (eq template_type "Block")}} + {{#if (eq template_name "block")}} // At least 100 µs. assert!( w >= 100 * constants::WEIGHT_PER_MICROS, diff --git a/utils/frame/benchmarking-cli/src/post_processing/mod.rs b/utils/frame/benchmarking-cli/src/post_processing/mod.rs index 00892365493ab..5beb338e1dcbd 100644 --- a/utils/frame/benchmarking-cli/src/post_processing/mod.rs +++ b/utils/frame/benchmarking-cli/src/post_processing/mod.rs @@ -1,3 +1,20 @@ +// This file is part of Substrate. + +// Copyright (C) 2022 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + use sc_cli::Result; use clap::Args; @@ -5,6 +22,7 @@ use serde::Serialize; use crate::storage::record::{StatSelect, Stats}; +/// Parameters to configure the post processing of the weights. #[derive(Debug, Default, Serialize, Clone, PartialEq, Args)] pub struct WeightParams { /// Path to write the *weight* file to. Can be a file or directory. diff --git a/utils/frame/benchmarking-cli/src/storage/cmd.rs b/utils/frame/benchmarking-cli/src/storage/cmd.rs index 300149c1a03e3..03c19187f6114 100644 --- a/utils/frame/benchmarking-cli/src/storage/cmd.rs +++ b/utils/frame/benchmarking-cli/src/storage/cmd.rs @@ -15,12 +15,10 @@ // See the License for the specific language governing permissions and // limitations under the License. -use sc_cli::{ - CliConfiguration, DatabaseParams, PruningParams, Result, SharedParams, TransactionPoolParams, -}; +use sc_cli::{CliConfiguration, DatabaseParams, PruningParams, Result, SharedParams}; use sc_client_api::{Backend as ClientBackend, StorageProvider, UsageProvider}; use sc_client_db::DbHash; -use sc_service::{config::TransactionPoolOptions, Configuration}; +use sc_service::Configuration; use sp_blockchain::HeaderBackend; use sp_database::{ColumnId, Database}; use sp_runtime::traits::{Block as BlockT, HashFor}; @@ -50,10 +48,6 @@ pub struct StorageCmd { #[clap(flatten)] pub pruning_params: PruningParams, - #[allow(missing_docs)] - #[clap(flatten)] - pub pool_params: TransactionPoolParams, - #[allow(missing_docs)] #[clap(flatten)] pub params: StorageParams, @@ -156,10 +150,6 @@ impl CliConfiguration for StorageCmd { Some(&self.pruning_params) } - fn transaction_pool(&self) -> Result { - Ok(self.pool_params.transaction_pool()) - } - fn state_cache_size(&self) -> Result { Ok(self.params.state_cache_size) } From e683edbb4e1d2dd22eaa9a228a3593689b160815 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Tue, 8 Mar 2022 14:02:07 +0000 Subject: [PATCH 07/29] Check for non-empty chain Signed-off-by: Oliver Tale-Yazdi --- .../frame/benchmarking-cli/src/block/bench.rs | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/utils/frame/benchmarking-cli/src/block/bench.rs b/utils/frame/benchmarking-cli/src/block/bench.rs index a4c22473c1b45..2311ddc5ebff3 100644 --- a/utils/frame/benchmarking-cli/src/block/bench.rs +++ b/utils/frame/benchmarking-cli/src/block/bench.rs @@ -113,24 +113,33 @@ where BenchmarkType::Block => (self.params.empty_block, true), BenchmarkType::Extrinsic => (self.params.full_block, false), }; - let block = self.load_block(id)?; + let (block, parent) = self.load_block(id)?; self.check_block(&block, empty)?; let block = self.unsealed(block)?; - let parent = BlockId::Hash(*block.header().parent_hash()); let rec = self.measure_block(&block, &parent, !empty)?; Stats::new(&rec) } - /// Loads a block. 0 loads the latest block. - fn load_block(&self, num: u32) -> Result { + /// Loads a block and its parent hash. 0 loads the latest block. + fn load_block(&self, num: u32) -> Result<(B, BlockId)> { let mut num = BlockId::Number(num.into()); if num == BlockId::Number(0u32.into()) { num = BlockId::Number(self.client.info().best_number); + + if num == BlockId::Number(0u32.into()) { + return Err("Chain must have some blocks but was empty".into()) + } } info!("Loading block {}", num); - self.client.block(&num)?.map(|b| b.block).ok_or("Could not load block".into()) + let block = self + .client + .block(&num)? + .map(|b| b.block) + .ok_or::("Could not load block".into())?; + let parent = BlockId::Hash(*block.header().parent_hash()); + Ok((block, parent)) } /// Checks if the passed block is empty. From 3f2eaf492045899923054d64d6d26c23283ab127 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Wed, 9 Mar 2022 17:34:28 +0100 Subject: [PATCH 08/29] Add tests for Stats Signed-off-by: Oliver Tale-Yazdi --- .../benchmarking-cli/src/storage/record.rs | 43 ++++++++++++++++++- 1 file changed, 41 insertions(+), 2 deletions(-) diff --git a/utils/frame/benchmarking-cli/src/storage/record.rs b/utils/frame/benchmarking-cli/src/storage/record.rs index 710c54bc54c2d..da6a34d80f6cc 100644 --- a/utils/frame/benchmarking-cli/src/storage/record.rs +++ b/utils/frame/benchmarking-cli/src/storage/record.rs @@ -107,6 +107,7 @@ impl BenchRecord { } } +// TODO refactor once https://doc.rust-lang.org/test/stats/trait.Stats.html is stabilized. impl Stats { /// Calculates statistics and returns them. pub fn new(xs: &Vec) -> Result { @@ -153,8 +154,8 @@ impl Stats { /// This is best effort since it ignores the interpolation case. fn percentile(mut xs: Vec, p: f64) -> u64 { xs.sort(); - let index = (xs.len() as f64 * p).ceil() as usize; - xs[index - 1] + let index = (xs.len() as f64 * p).ceil() as usize - 1; + xs[index.clamp(0, xs.len() - 1)] } } @@ -189,3 +190,41 @@ impl FromStr for StatSelect { } } } + +#[cfg(test)] +mod test_stats { + use super::Stats; + use rand::{seq::SliceRandom, thread_rng}; + + #[test] + fn stats_correct() { + let mut data: Vec = (1..=100).collect(); + data.shuffle(&mut thread_rng()); + let stats = Stats::new(&data).unwrap(); + + assert_eq!(stats.sum, 5050); + assert_eq!(stats.min, 1); + assert_eq!(stats.max, 100); + + assert_eq!(stats.avg, 50); + assert_eq!(stats.median, 50); // 50.5 to be exact. + // Rounded with 1/100 precision. + assert_eq!(stats.stddev, 28.87); + + assert_eq!(stats.p99, 99); + assert_eq!(stats.p95, 95); + assert_eq!(stats.p75, 75); + } + + #[test] + fn no_panic_different_lengths() { + // No input errors. + assert!(Stats::new(&vec![]).is_err()); + + // Different small input lengths are fine. + for l in 1..10 { + let data = (0..=l).collect(); + assert!(Stats::new(&data).is_ok()); + } + } +} From a4ec657ab4ae9844220d211be735bb1a1c7dfcaf Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Wed, 9 Mar 2022 23:44:49 +0100 Subject: [PATCH 09/29] Review fixes Signed-off-by: Oliver Tale-Yazdi --- bin/node-template/node/src/cli.rs | 5 +++-- bin/node-template/node/src/command.rs | 1 - bin/node/cli/src/cli.rs | 2 +- bin/node/cli/src/command.rs | 6 ------ utils/frame/benchmarking-cli/src/block/template.rs | 14 +++++++------- utils/frame/benchmarking-cli/src/block/weights.hbs | 10 +++++----- 6 files changed, 16 insertions(+), 22 deletions(-) diff --git a/bin/node-template/node/src/cli.rs b/bin/node-template/node/src/cli.rs index 36f0f5e93e81b..86261e44ca142 100644 --- a/bin/node-template/node/src/cli.rs +++ b/bin/node-template/node/src/cli.rs @@ -36,8 +36,9 @@ pub enum Subcommand { /// Revert the chain to a previous state. Revert(sc_cli::RevertCmd), - #[clap(name = "benchmark")] - BenchmarkBlock(frame_benchmarking_cli::block::cmd::BlockCmd), + /// Sub command for benchmarking block and extrinsic base weight. + #[clap(name = "benchmark-block", about = "Benchmark block and extrinsic base weight.")] + BenchmarkBlock(frame_benchmarking_cli::BlockCmd), /// The custom benchmark subcommand benchmarking runtime pallets. #[clap(name = "benchmark", about = "Benchmark runtime pallets.")] diff --git a/bin/node-template/node/src/command.rs b/bin/node-template/node/src/command.rs index e81e86c7e1194..a85aa5f0896b6 100644 --- a/bin/node-template/node/src/command.rs +++ b/bin/node-template/node/src/command.rs @@ -99,7 +99,6 @@ pub fn run() -> sc_cli::Result<()> { }) }, Some(Subcommand::BenchmarkBlock(cmd)) => { - // TODO feature check let runner = cli.create_runner(cmd)?; runner.async_run(|config| { let PartialComponents { client, task_manager, .. } = service::new_partial(&config)?; diff --git a/bin/node/cli/src/cli.rs b/bin/node/cli/src/cli.rs index a52bb0d7c2de3..fd10c12cd5a44 100644 --- a/bin/node/cli/src/cli.rs +++ b/bin/node/cli/src/cli.rs @@ -44,7 +44,7 @@ pub enum Subcommand { /// Sub command for benchmarking block and extrinsic base weight. #[clap(name = "benchmark-block", about = "Benchmark block and extrinsic base weight.")] - BenchmarkBlock(frame_benchmarking_cli::block::cmd::BlockCmd), + BenchmarkBlock(frame_benchmarking_cli::BlockCmd), /// Sub command for benchmarking the storage speed. #[clap(name = "benchmark-storage", about = "Benchmark storage speed.")] diff --git a/bin/node/cli/src/command.rs b/bin/node/cli/src/command.rs index 89ef2c3d4f1e5..9c50854f021b0 100644 --- a/bin/node/cli/src/command.rs +++ b/bin/node/cli/src/command.rs @@ -96,12 +96,6 @@ pub fn run() -> Result<()> { .into()) }, Some(Subcommand::BenchmarkBlock(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()) - } - let runner = cli.create_runner(cmd)?; runner.async_run(|config| { let PartialComponents { client, task_manager, .. } = new_partial(&config)?; diff --git a/utils/frame/benchmarking-cli/src/block/template.rs b/utils/frame/benchmarking-cli/src/block/template.rs index 5711340d63ab2..e3206085defc7 100644 --- a/utils/frame/benchmarking-cli/src/block/template.rs +++ b/utils/frame/benchmarking-cli/src/block/template.rs @@ -33,10 +33,10 @@ static TEMPLATE: &str = include_str!("./weights.hbs"); /// Data consumed by Handlebar to fill out the `weights.hbs` template. #[derive(Serialize, Debug, Clone)] pub(crate) struct TemplateData { - /// Name of the benchmark. String version of `template_type`. - template_name: String, - /// Path prefix that will be used to save the resulting file. - template_path: String, + /// Short name of the benchmark. Can be "block" or "extrinsic". + long_name: String, + /// Long name of the benchmark. Can be "BlockExecution" "ExtrinsicBase". + short_name: String, /// Name of the runtime. Taken from the chain spec. runtime_name: String, /// Version of the benchmarking CLI used. @@ -64,8 +64,8 @@ impl TemplateData { let weight = params.weight.calc_weight(stats)?; Ok(TemplateData { - template_name: t.short_name().into(), - template_path: t.long_name().into(), + short_name: t.short_name().into(), + long_name: t.long_name().into(), runtime_name: cfg.chain_spec.name().into(), version: VERSION.into(), date: chrono::Utc::now().format("%Y-%m-%d (Y/M/D)").to_string(), @@ -97,7 +97,7 @@ impl TemplateData { fn build_path(&self, weight_out: &str) -> PathBuf { let mut path = PathBuf::from(weight_out); if path.is_dir() { - path.push(format!("{:?}_weights.rs", self.template_name)); + path.push(format!("{}_weights.rs", self.short_name)); path.set_extension("rs"); } path diff --git a/utils/frame/benchmarking-cli/src/block/weights.hbs b/utils/frame/benchmarking-cli/src/block/weights.hbs index 1377e189db703..e0772e3a915a6 100644 --- a/utils/frame/benchmarking-cli/src/block/weights.hbs +++ b/utils/frame/benchmarking-cli/src/block/weights.hbs @@ -18,7 +18,7 @@ //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION {{version}} //! DATE: {{date}} //! -//! TYPE: `{{template_type}}`, RUNTIME: `{{runtime_name}}` +//! SHORT-NAME: `{{short_name}}`, LONG-NAME: `{{long_name}}`, RUNTIME: `{{runtime_name}}` //! WARMUPS: `{{params.bench.warmup}}`, REPEAT: `{{params.bench.repeat}}` //! WEIGHT-PATH: `{{params.weight.weight_path}}` //! WEIGHT-METRIC: `{{params.weight.weight_metric}}`, WEIGHT-MUL: `{{params.weight.weight_mul}}`, WEIGHT-ADD: `{{params.weight.weight_add}}` @@ -35,7 +35,7 @@ pub mod constants { }; parameter_types! { - {{#if (eq template_name "block")}} + {{#if (eq short_name "block")}} /// Time to execute an empty block. {{else}} /// Time to execute a NO-OP extrinsic eg. `System::remark`. @@ -52,7 +52,7 @@ pub mod constants { /// 99th: {{underscore stats.p99}} /// 95th: {{underscore stats.p95}} /// 75th: {{underscore stats.p75}} - pub const {{template_name}}Weight: Weight = {{underscore weight}} * WEIGHT_PER_NANOS; + pub const {{long_name}}Weight: Weight = {{underscore weight}} * WEIGHT_PER_NANOS; } #[cfg(test)] @@ -64,9 +64,9 @@ pub mod constants { // you can delete it. #[test] fn sane() { - let w = super::constants::{{template_name}}Weight::get(); + let w = super::constants::{{long_name}}Weight::get(); - {{#if (eq template_name "block")}} + {{#if (eq short_name "block")}} // At least 100 µs. assert!( w >= 100 * constants::WEIGHT_PER_MICROS, From a73644bc4f895b8e3a16ca192599fb02afb46a60 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Thu, 10 Mar 2022 00:46:28 +0100 Subject: [PATCH 10/29] Review fixes Signed-off-by: Oliver Tale-Yazdi --- .../frame/benchmarking-cli/src/block/bench.rs | 7 ++--- utils/frame/benchmarking-cli/src/block/cmd.rs | 7 ++--- .../benchmarking-cli/src/block/template.rs | 3 +- .../src/post_processing/mod.rs | 29 +++++++++++++++++++ .../benchmarking-cli/src/storage/record.rs | 25 ++++++++-------- 5 files changed, 49 insertions(+), 22 deletions(-) diff --git a/utils/frame/benchmarking-cli/src/block/bench.rs b/utils/frame/benchmarking-cli/src/block/bench.rs index 2311ddc5ebff3..374894a507f14 100644 --- a/utils/frame/benchmarking-cli/src/block/bench.rs +++ b/utils/frame/benchmarking-cli/src/block/bench.rs @@ -18,7 +18,6 @@ use sc_block_builder::BlockBuilderApi; use sc_cli::Result; use sc_client_api::{BlockBackend, UsageProvider}; -use sc_client_db::DbHash; use sp_api::{ApiExt, BlockId, HeaderT, ProvideRuntimeApi}; use sp_blockchain::HeaderBackend; use sp_runtime::{traits::Block as BlockT, DigestItem}; @@ -98,7 +97,7 @@ pub(crate) struct Benchmark { impl Benchmark where - B: BlockT, + B: BlockT, C: UsageProvider + HeaderBackend + BlockBackend + ProvideRuntimeApi, API: ApiExt + BlockBuilderApi, { @@ -187,7 +186,7 @@ where let mut record = BenchRecord::new(); let num_ext = block.extrinsics().len() as u64; if per_ext && num_ext == 0 { - return Err("Cannot measure extrinsic time of empty block".into()) + return Err("Cannot measure the extrinsic time of an empty block".into()) } info!("Running {} warmups...", self.params.warmup); @@ -211,7 +210,7 @@ where let elapsed = start.elapsed().as_nanos(); if per_ext { - // non zero checked above. + // checked for non zero div above. record.push(elapsed as u64 / num_ext); } else { record.push(elapsed as u64); diff --git a/utils/frame/benchmarking-cli/src/block/cmd.rs b/utils/frame/benchmarking-cli/src/block/cmd.rs index 272388a7bc6a7..0879f7c71312d 100644 --- a/utils/frame/benchmarking-cli/src/block/cmd.rs +++ b/utils/frame/benchmarking-cli/src/block/cmd.rs @@ -18,7 +18,6 @@ use sc_block_builder::BlockBuilderApi; use sc_cli::{CliConfiguration, DatabaseParams, PruningParams, Result, SharedParams}; use sc_client_api::{BlockBackend, UsageProvider}; -use sc_client_db::DbHash; use sc_service::Configuration; use sp_api::{ApiExt, ProvideRuntimeApi}; use sp_blockchain::HeaderBackend; @@ -37,7 +36,7 @@ use crate::{ post_processing::WeightParams, }; -/// Parameters for a [`BlockCmd`]. +/// Command for running block and extrinsic benchmarks. #[derive(Debug, Parser)] pub struct BlockCmd { #[allow(missing_docs)] @@ -57,7 +56,7 @@ pub struct BlockCmd { pub params: BlockParams, } -/// Parameter for a block benchmark and post processing. +/// Parameters for a block benchmark and its result post processing. #[derive(Debug, Default, Serialize, Clone, PartialEq, Args)] pub struct BlockParams { #[allow(missing_docs)] @@ -77,7 +76,7 @@ impl BlockCmd { /// Run the block and extrinsic benchmark. pub async fn run(&self, cfg: Configuration, client: Arc) -> Result<()> where - Block: BlockT, + Block: BlockT, C: UsageProvider + HeaderBackend + BlockBackend diff --git a/utils/frame/benchmarking-cli/src/block/template.rs b/utils/frame/benchmarking-cli/src/block/template.rs index e3206085defc7..8eaa6e4aeecee 100644 --- a/utils/frame/benchmarking-cli/src/block/template.rs +++ b/utils/frame/benchmarking-cli/src/block/template.rs @@ -18,6 +18,7 @@ use sc_cli::Result; use sc_service::Configuration; +use handlebars::Handlebars; use log::info; use serde::Serialize; use std::{env, fs, path::PathBuf}; @@ -79,7 +80,7 @@ impl TemplateData { /// Filles out the `weights.hbs` HBS template with its own data. /// Writes the result to `path` which can be a directory or a file. pub fn write(&self, path: &str) -> Result<()> { - let mut handlebars = handlebars::Handlebars::new(); + let mut handlebars = Handlebars::new(); // Format large integers with underscores. handlebars.register_helper("underscore", Box::new(crate::writer::UnderscoreHelper)); // Don't HTML escape any characters. diff --git a/utils/frame/benchmarking-cli/src/post_processing/mod.rs b/utils/frame/benchmarking-cli/src/post_processing/mod.rs index 5beb338e1dcbd..363cd8a9f665c 100644 --- a/utils/frame/benchmarking-cli/src/post_processing/mod.rs +++ b/utils/frame/benchmarking-cli/src/post_processing/mod.rs @@ -58,3 +58,32 @@ impl WeightParams { Ok(w as u64) // No safe cast here since there is no `From` for `u64`. } } + +#[cfg(test)] +mod test_weight_params { + use super::WeightParams; + use crate::storage::record::{StatSelect, Stats}; + + #[test] + fn calc_weight_works() { + let stats = Stats { avg: 113, ..Default::default() }; + let params = WeightParams { + weight_metric: StatSelect::Average, + weight_mul: 0.75, + weight_add: 3, + ..Default::default() + }; + + let want = (113.0f64 * 0.75 + 3.0).ceil() as u64; // Ceil for overestimation. + let got = params.calc_weight(&stats).unwrap(); + assert_eq!(want, got); + } + + #[test] + fn calc_weight_detects_negative_mul() { + let stats = Stats::default(); + let params = WeightParams { weight_mul: -0.75, ..Default::default() }; + + assert!(params.calc_weight(&stats).is_err()); + } +} diff --git a/utils/frame/benchmarking-cli/src/storage/record.rs b/utils/frame/benchmarking-cli/src/storage/record.rs index da6a34d80f6cc..4d8a1462383b0 100644 --- a/utils/frame/benchmarking-cli/src/storage/record.rs +++ b/utils/frame/benchmarking-cli/src/storage/record.rs @@ -36,25 +36,25 @@ pub(crate) struct BenchRecord { #[derive(Serialize, Default, Clone)] pub(crate) struct Stats { /// Sum of all values. - sum: u64, + pub(crate) sum: u64, /// Minimal observed value. - min: u64, + pub(crate) min: u64, /// Maximal observed value. - max: u64, + pub(crate) max: u64, /// Average of all values. - avg: u64, + pub(crate) avg: u64, /// Median of all values. - median: u64, + pub(crate) median: u64, /// Standard derivation of all values. - stddev: f64, + pub(crate) stddev: f64, /// 99th percentile. At least 99% of all values are below this threshold. - p99: u64, + pub(crate) p99: u64, /// 95th percentile. At least 95% of all values are below this threshold. - p95: u64, + pub(crate) p95: u64, /// 75th percentile. At least 75% of all values are below this threshold. - p75: u64, + pub(crate) p75: u64, } /// Selects a specific field from a [`Stats`] object. @@ -208,8 +208,7 @@ mod test_stats { assert_eq!(stats.avg, 50); assert_eq!(stats.median, 50); // 50.5 to be exact. - // Rounded with 1/100 precision. - assert_eq!(stats.stddev, 28.87); + assert_eq!(stats.stddev, 28.87); // Rounded with 1/100 precision. assert_eq!(stats.p99, 99); assert_eq!(stats.p95, 95); @@ -217,8 +216,8 @@ mod test_stats { } #[test] - fn no_panic_different_lengths() { - // No input errors. + fn no_panic_short_lengths() { + // Empty input does error. assert!(Stats::new(&vec![]).is_err()); // Different small input lengths are fine. From e004a5db0cc59280e6845e65836787d694df2411 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Thu, 10 Mar 2022 12:37:36 +0100 Subject: [PATCH 11/29] Apply suggestions from code review Co-authored-by: Shawn Tabrizi --- utils/frame/benchmarking-cli/src/block/bench.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/utils/frame/benchmarking-cli/src/block/bench.rs b/utils/frame/benchmarking-cli/src/block/bench.rs index 374894a507f14..da0cf9f347412 100644 --- a/utils/frame/benchmarking-cli/src/block/bench.rs +++ b/utils/frame/benchmarking-cli/src/block/bench.rs @@ -81,9 +81,9 @@ pub(crate) type BenchRecord = Vec; /// Type of a benchmark. #[derive(Serialize, Clone)] pub(crate) enum BenchmarkType { - /// Extrinsic execution speed was measured. + /// Per-extrinsic execution overhead was measured. Extrinsic, - /// Empty block execution speed was measured. + /// Per-block execution overhead was measured. Block, } From 48bc8fdc6e7d1b8de8ccbdd46631c5ee26cd1ad8 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Thu, 10 Mar 2022 17:40:42 +0100 Subject: [PATCH 12/29] Review fixes Signed-off-by: Oliver Tale-Yazdi --- .../frame/benchmarking-cli/src/block/bench.rs | 52 ++++++++----------- utils/frame/benchmarking-cli/src/block/cmd.rs | 4 +- .../benchmarking-cli/src/block/template.rs | 2 +- .../benchmarking-cli/src/storage/record.rs | 1 - 4 files changed, 25 insertions(+), 34 deletions(-) diff --git a/utils/frame/benchmarking-cli/src/block/bench.rs b/utils/frame/benchmarking-cli/src/block/bench.rs index da0cf9f347412..f86cc8debdc63 100644 --- a/utils/frame/benchmarking-cli/src/block/bench.rs +++ b/utils/frame/benchmarking-cli/src/block/bench.rs @@ -34,20 +34,22 @@ use crate::storage::record::Stats; pub struct BenchmarkParams { /// Skip benchmarking NO-OP extrinsics. #[clap(long)] - pub skip_extrinsic: bool, + pub skip_extrinsic_benchmark: bool, /// Skip benchmark an empty block. #[clap(long)] - pub skip_block: bool, + pub skip_block_benchmark: bool, - /// Specify the id of a full block. 0 is treated as last block. + /// Specify the number of a full block. 0 is treated as last block. /// The block should be filled with `System::remark` extrinsics. - #[clap(long, default_value = "0")] + /// Set to one since that is the first block which could be full. + #[clap(long, default_value = "1")] pub full_block: u32, - /// Specify the id of an empty block. 0 is treated as last block. + /// Specify the number of an empty block. 0 is treated as last block. /// The block should not contains any user extrinsics but only inherents. - #[clap(long, default_value = "0")] + /// Set to one since that is the first block which could be empty. + #[clap(long, default_value = "1")] pub empty_block: u32, /// How many executions should be measured. @@ -58,21 +60,13 @@ pub struct BenchmarkParams { #[clap(long, default_value = "100")] pub warmup: u32, - /// Maximum number of inherents that can be present in a block - /// such that the block will still be considered empty. - /// - /// Default is 1 since that is the case for Substrate. - /// This check exists to make sure that a non-empty block is not - /// accidentally counted as empty. - #[clap(long, default_value = "1")] - pub max_inherents: u32, - - /// Minimum number of extrinsics that must be present in a block - /// such that the block will be considered full. - /// - /// Default is 12_000 since in Substrate a block can hold that many NO-OPs. - #[clap(long, default_value = "12000")] - pub min_extrinsics: u32, + /// Specify the number of inherents that are present per block. + /// Blocks with extrinsics besides the inherents will be counted + /// as full and can be used for an extrinsics benchmark. + /// Blocks with only the inherents will be considered empty and + /// can only be used for a block benchmark. + #[clap(long)] + pub num_inherents: u32, } /// The results of multiple runs in ns. @@ -145,9 +139,11 @@ where /// The resulting error can be demoted to a warning via `--no-check`. fn check_block(&self, block: &B, want_empty: bool) -> Result<()> { let num_ext = block.extrinsics().len() as u32; - let is_empty = num_ext <= self.params.max_inherents; - let is_full = num_ext >= self.params.min_extrinsics; - info!("Block contains {} extrinsics", num_ext); + info!("Block contains {} transactions", num_ext); + if num_ext < self.params.num_inherents { + return Err("A block cannot have less than `num_inherents` transactions".into()); + } + let is_empty = num_ext == self.params.num_inherents; if want_empty { match (is_empty, self.no_check) { @@ -156,7 +152,7 @@ where (false, true) => warn!("Treating non-empty block as empty because of --no-check"), } } else { - match (is_full, self.no_check) { + match (!is_empty, self.no_check) { (true, _) => {}, (false, false) => return Err("Block should be full but was not".into()), (false, true) => warn!("Treating non-full block as full because of --no-check"), @@ -169,11 +165,7 @@ where /// Removes the consensus seal from a block if there is any. fn unsealed(&self, block: B) -> Result { let (mut header, extrinsics) = block.deconstruct(); - match header.digest_mut().pop() { - Some(DigestItem::Seal(_, _)) => {}, - Some(other) => header.digest_mut().push(other), - _ => {}, - } + header.digest_mut().logs.retain(|item| !matches!(item, DigestItem::Seal(_, _))); Ok(B::new(header, extrinsics)) } diff --git a/utils/frame/benchmarking-cli/src/block/cmd.rs b/utils/frame/benchmarking-cli/src/block/cmd.rs index 0879f7c71312d..9175f7a419d7b 100644 --- a/utils/frame/benchmarking-cli/src/block/cmd.rs +++ b/utils/frame/benchmarking-cli/src/block/cmd.rs @@ -85,14 +85,14 @@ impl BlockCmd { { let bench = Benchmark::new(client, self.params.bench.clone(), self.params.no_check); - if !self.params.bench.skip_block { + if !self.params.bench.skip_block_benchmark { let stats = bench.bench(BenchmarkType::Block)?; info!("Executing an empty block [ns]:\n{:?}", stats); let template = TemplateData::new(BenchmarkType::Block, &cfg, &self.params, &stats)?; template.write(&self.params.weight.weight_path)?; } - if !self.params.bench.skip_extrinsic { + if !self.params.bench.skip_extrinsic_benchmark { let stats = bench.bench(BenchmarkType::Extrinsic)?; info!("Executing a NO-OP extrinsic [ns]:\n{:?}", stats); let template = TemplateData::new(BenchmarkType::Extrinsic, &cfg, &self.params, &stats)?; diff --git a/utils/frame/benchmarking-cli/src/block/template.rs b/utils/frame/benchmarking-cli/src/block/template.rs index 8eaa6e4aeecee..d37c4f28e7aca 100644 --- a/utils/frame/benchmarking-cli/src/block/template.rs +++ b/utils/frame/benchmarking-cli/src/block/template.rs @@ -98,7 +98,7 @@ impl TemplateData { fn build_path(&self, weight_out: &str) -> PathBuf { let mut path = PathBuf::from(weight_out); if path.is_dir() { - path.push(format!("{}_weights.rs", self.short_name)); + path.push(format!("{}_weights", self.short_name)); path.set_extension("rs"); } path diff --git a/utils/frame/benchmarking-cli/src/storage/record.rs b/utils/frame/benchmarking-cli/src/storage/record.rs index 4d8a1462383b0..7fc2a51c57f79 100644 --- a/utils/frame/benchmarking-cli/src/storage/record.rs +++ b/utils/frame/benchmarking-cli/src/storage/record.rs @@ -107,7 +107,6 @@ impl BenchRecord { } } -// TODO refactor once https://doc.rust-lang.org/test/stats/trait.Stats.html is stabilized. impl Stats { /// Calculates statistics and returns them. pub fn new(xs: &Vec) -> Result { From b935d34cf96301be18b2c546e5a8a414dfd78f12 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Thu, 10 Mar 2022 17:43:37 +0100 Subject: [PATCH 13/29] Review fixes Signed-off-by: Oliver Tale-Yazdi --- utils/frame/benchmarking-cli/src/block/bench.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utils/frame/benchmarking-cli/src/block/bench.rs b/utils/frame/benchmarking-cli/src/block/bench.rs index f86cc8debdc63..77ef74a06f60d 100644 --- a/utils/frame/benchmarking-cli/src/block/bench.rs +++ b/utils/frame/benchmarking-cli/src/block/bench.rs @@ -141,7 +141,7 @@ where let num_ext = block.extrinsics().len() as u32; info!("Block contains {} transactions", num_ext); if num_ext < self.params.num_inherents { - return Err("A block cannot have less than `num_inherents` transactions".into()); + return Err("A block cannot have less than `num_inherents` transactions".into()) } let is_empty = num_ext == self.params.num_inherents; From aede82768a0321233c518d5661ce7c7ab98913f8 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Mon, 14 Mar 2022 14:34:15 +0100 Subject: [PATCH 14/29] Push first version again Signed-off-by: Oliver Tale-Yazdi --- Cargo.lock | 12 ++ bin/node-template/node/Cargo.toml | 10 + bin/node-template/node/src/cli.rs | 8 +- bin/node-template/node/src/command.rs | 54 ++++- bin/node-template/node/src/service.rs | 2 +- bin/node/cli/Cargo.toml | 6 + bin/node/cli/src/cli.rs | 2 +- bin/node/cli/src/command.rs | 50 ++++- frame/aura/src/lib.rs | 5 +- frame/babe/src/lib.rs | 11 +- utils/frame/benchmarking-cli/Cargo.toml | 4 + .../frame/benchmarking-cli/src/block/bench.rs | 193 ++++++++---------- utils/frame/benchmarking-cli/src/block/cmd.rs | 94 ++++----- utils/frame/benchmarking-cli/src/block/mod.rs | 3 + .../benchmarking-cli/src/block/template.rs | 10 +- .../benchmarking-cli/src/block/weights.hbs | 25 +++ .../benchmarking-cli/src/storage/write.rs | 1 + 17 files changed, 308 insertions(+), 182 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6f8515a54c096..d0e689fc47220 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2144,6 +2144,7 @@ dependencies = [ "sc-cli", "sc-client-api", "sc-client-db", + "sc-consensus", "sc-executor", "sc-service", "serde", @@ -2151,9 +2152,11 @@ dependencies = [ "serde_nanos", "sp-api", "sp-blockchain", + "sp-consensus", "sp-core", "sp-database", "sp-externalities", + "sp-inherents", "sp-keystore", "sp-runtime", "sp-state-machine", @@ -4792,6 +4795,7 @@ version = "3.0.0-dev" dependencies = [ "assert_cmd", "async-std", + "chrono", "clap 3.0.7", "clap_complete", "criterion", @@ -4850,6 +4854,7 @@ dependencies = [ "sp-blockchain", "sp-consensus", "sp-consensus-babe", + "sp-consensus-slots", "sp-core", "sp-finality-grandpa", "sp-inherents", @@ -5065,6 +5070,9 @@ dependencies = [ "frame-benchmarking", "frame-benchmarking-cli", "jsonrpc-core", + "log 0.4.14", + "node-cli", + "node-runtime", "node-template-runtime", "pallet-transaction-payment-rpc", "sc-basic-authorship", @@ -5086,8 +5094,12 @@ dependencies = [ "sp-blockchain", "sp-consensus", "sp-consensus-aura", + "sp-consensus-babe", + "sp-consensus-slots", "sp-core", "sp-finality-grandpa", + "sp-inherents", + "sp-keyring", "sp-runtime", "sp-timestamp", "substrate-build-script-utils", diff --git a/bin/node-template/node/Cargo.toml b/bin/node-template/node/Cargo.toml index 98e8af96d3f8d..6e0d8bbc46d34 100644 --- a/bin/node-template/node/Cargo.toml +++ b/bin/node-template/node/Cargo.toml @@ -37,6 +37,12 @@ sc-client-api = { version = "4.0.0-dev", path = "../../../client/api" } sp-runtime = { version = "6.0.0", path = "../../../primitives/runtime" } sp-timestamp = { version = "4.0.0-dev", path = "../../../primitives/timestamp" } +# OTY added +sp-consensus-slots = { version = "0.10.0-dev", path = "../../../primitives/consensus/slots" } +sp-consensus-babe = { version = "0.10.0-dev", path = "../../../primitives/consensus/babe" } +sp-inherents = { version = "4.0.0-dev", path = "../../../primitives/inherents" } +log = "0.4.8" + # These dependencies are used for the node template's RPCs jsonrpc-core = "18.0.0" sc-rpc = { version = "4.0.0-dev", path = "../../../client/rpc" } @@ -57,6 +63,10 @@ node-template-runtime = { version = "4.0.0-dev", path = "../runtime" } # CLI-specific dependencies try-runtime-cli = { version = "0.10.0-dev", optional = true, path = "../../../utils/frame/try-runtime/cli" } +# OTY added +sp-keyring = { version = "6.0.0", path = "../../../primitives/keyring" } +node-runtime = { version = "3.0.0-dev", path = "../../node/runtime" } +node-cli = { version = "3.0.0-dev", path = "../../node/cli" } [build-dependencies] substrate-build-script-utils = { version = "3.0.0", path = "../../../utils/build-script-utils" } diff --git a/bin/node-template/node/src/cli.rs b/bin/node-template/node/src/cli.rs index 86261e44ca142..9db00ccdb5dd9 100644 --- a/bin/node-template/node/src/cli.rs +++ b/bin/node-template/node/src/cli.rs @@ -36,14 +36,14 @@ pub enum Subcommand { /// Revert the chain to a previous state. Revert(sc_cli::RevertCmd), - /// Sub command for benchmarking block and extrinsic base weight. - #[clap(name = "benchmark-block", about = "Benchmark block and extrinsic base weight.")] - BenchmarkBlock(frame_benchmarking_cli::BlockCmd), - /// The custom benchmark subcommand benchmarking runtime pallets. #[clap(name = "benchmark", about = "Benchmark runtime pallets.")] Benchmark(frame_benchmarking_cli::BenchmarkCmd), + /// The custom benchmark subcommand benchmarking runtime pallets. + #[clap(name = "benchmark-block", about = "Benchmark runtime pallets.")] + BenchmarkBlock(frame_benchmarking_cli::BlockCmd), + /// Try some command against runtime state. #[cfg(feature = "try-runtime")] TryRuntime(try_runtime_cli::TryRuntimeCmd), diff --git a/bin/node-template/node/src/command.rs b/bin/node-template/node/src/command.rs index a85aa5f0896b6..b70694935f56b 100644 --- a/bin/node-template/node/src/command.rs +++ b/bin/node-template/node/src/command.rs @@ -3,10 +3,13 @@ use crate::{ cli::{Cli, Subcommand}, service, }; -use node_template_runtime::Block; +use node_cli::service::create_extrinsic; +use node_runtime::SystemCall; use sc_cli::{ChainSpec, RuntimeVersion, SubstrateCli}; use sc_service::PartialComponents; +use std::sync::Arc; + impl SubstrateCli for Cli { fn impl_name() -> String { "Substrate Node".into() @@ -46,6 +49,47 @@ impl SubstrateCli for Cli { } } +use super::service::FullClient; +struct ExtrinsicGen { + client: Arc, +} +use node_runtime::UncheckedExtrinsic; +use node_template_runtime::Block; +use sp_keyring::Sr25519Keyring; +use sp_runtime::{traits::Block as BlockT, OpaqueExtrinsic}; +/* +impl frame_benchmarking_cli::block::cmd::ExtrinsicGenerator for ExtrinsicGen { + fn noop(&self, nonce: u32) -> Option { + let src = Sr25519Keyring::Alice.pair(); + + let extrinsic: OpaqueExtrinsic = create_extrinsic( + self.client.as_ref(), + src.clone(), + SystemCall::remark { remark: vec![] }, + Some(nonce), + ) + .into(); + None + } +}*/ +/* +#[derive(Default)] +struct InherentProv {} + +impl frame_benchmarking_cli::block::cmd::BlockInherentDataProvider for InherentProv { + fn providers( + &self, + block: u64, + ) -> std::result::Result>, sp_inherents::Error> + { + log::info!("Creating inherents for block #{}", block); + let d = std::time::Duration::from_millis(0); + let timestamp = sp_timestamp::InherentDataProvider::new(d.into()); + + Ok(vec![Arc::new(timestamp)]) + } +}*/ + /// Parse and run command line arguments pub fn run() -> sc_cli::Result<()> { let cli = Cli::from_args(); @@ -98,14 +142,6 @@ pub fn run() -> sc_cli::Result<()> { Ok((cmd.run(client, backend), task_manager)) }) }, - Some(Subcommand::BenchmarkBlock(cmd)) => { - let runner = cli.create_runner(cmd)?; - runner.async_run(|config| { - let PartialComponents { client, task_manager, .. } = service::new_partial(&config)?; - - Ok((cmd.run(config, client), task_manager)) - }) - }, Some(Subcommand::Benchmark(cmd)) => if cfg!(feature = "runtime-benchmarks") { let runner = cli.create_runner(cmd)?; diff --git a/bin/node-template/node/src/service.rs b/bin/node-template/node/src/service.rs index fc7dc9b978df3..e2a8cb4ed834b 100644 --- a/bin/node-template/node/src/service.rs +++ b/bin/node-template/node/src/service.rs @@ -31,7 +31,7 @@ impl sc_executor::NativeExecutionDispatch for ExecutorDispatch { } } -type FullClient = +pub(crate) type FullClient = sc_service::TFullClient>; type FullBackend = sc_service::TFullBackend; type FullSelectChain = sc_consensus::LongestChain; diff --git a/bin/node/cli/Cargo.toml b/bin/node/cli/Cargo.toml index b4a91712fd16c..a5c5c0530b854 100644 --- a/bin/node/cli/Cargo.toml +++ b/bin/node/cli/Cargo.toml @@ -96,6 +96,12 @@ frame-benchmarking-cli = { version = "4.0.0-dev", optional = true, path = "../.. node-inspect = { version = "0.9.0-dev", optional = true, path = "../inspect" } try-runtime-cli = { version = "0.10.0-dev", optional = true, path = "../../../utils/frame/try-runtime/cli" } +# OTY added +sp-consensus-slots = { version = "0.10.0-dev", path = "../../../primitives/consensus/slots" } +# OTY +sp-blockchain = { version = "4.0.0-dev", path = "../../../primitives/blockchain" } +chrono = "0.4.19" + [target.'cfg(any(target_arch="x86_64", target_arch="aarch64"))'.dependencies] node-executor = { version = "3.0.0-dev", path = "../executor", features = ["wasmtime"] } sc-cli = { version = "0.10.0-dev", optional = true, path = "../../../client/cli", features = ["wasmtime"] } diff --git a/bin/node/cli/src/cli.rs b/bin/node/cli/src/cli.rs index fd10c12cd5a44..830616a8b4541 100644 --- a/bin/node/cli/src/cli.rs +++ b/bin/node/cli/src/cli.rs @@ -45,7 +45,7 @@ pub enum Subcommand { /// Sub command for benchmarking block and extrinsic base weight. #[clap(name = "benchmark-block", about = "Benchmark block and extrinsic base weight.")] BenchmarkBlock(frame_benchmarking_cli::BlockCmd), - + /// 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 index 9c50854f021b0..dfbe747fe72c6 100644 --- a/bin/node/cli/src/command.rs +++ b/bin/node/cli/src/command.rs @@ -18,10 +18,12 @@ use crate::{chain_spec, service, service::new_partial, Cli, Subcommand}; use node_executor::ExecutorDispatch; -use node_runtime::{Block, RuntimeApi}; +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() @@ -68,6 +70,42 @@ impl SubstrateCli for Cli { &node_runtime::VERSION } } +struct ExtrinsicGen { + client: Arc, +} +use crate::service::{create_extrinsic, FullClient}; +use node_primitives::Block; +use node_runtime::{SystemCall}; +use sp_keyring::Sr25519Keyring; +use sp_runtime::{OpaqueExtrinsic}; +impl frame_benchmarking_cli::block::cmd::ExtrinsicGenerator for ExtrinsicGen { + fn remark(&self, nonce: u32) -> Option { + let src = Sr25519Keyring::Alice.pair(); + + // This `create_extrinsic` only exists for the node and has no + // equivalent in other runtimes. + let extrinsic: OpaqueExtrinsic = create_extrinsic( + self.client.as_ref(), + src.clone(), + SystemCall::remark { remark: vec![] }, + Some(nonce), + ) + .into(); + Some(extrinsic) + } +} + +use sp_inherents::InherentDataProvider; +fn inherent_data() -> Result { + let mut inherent_data = sp_inherents::InherentData::new(); + let d = std::time::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) +} /// Parse command line arguments into service configuration. pub fn run() -> Result<()> { @@ -97,10 +135,14 @@ pub fn run() -> Result<()> { }, Some(Subcommand::BenchmarkBlock(cmd)) => { let runner = cli.create_runner(cmd)?; - runner.async_run(|config| { - let PartialComponents { client, task_manager, .. } = new_partial(&config)?; + runner.async_run(|mut config| { + config.role = sc_service::Role::Full; + + let PartialComponents { client, task_manager, backend, .. } = new_partial(&config)?; + let ext_gen = ExtrinsicGen { client: client.clone() }; - Ok((cmd.run(config, client), task_manager)) + let inherents = inherent_data()?; + Ok((cmd.run(config, client.clone(), inherents, Arc::new(ext_gen)), task_manager)) }) }, Some(Subcommand::BenchmarkStorage(cmd)) => { diff --git a/frame/aura/src/lib.rs b/frame/aura/src/lib.rs index 657965c60a3f1..83fb6cb392a27 100644 --- a/frame/aura/src/lib.rs +++ b/frame/aura/src/lib.rs @@ -282,9 +282,6 @@ impl OnTimestampSet for Pallet { let timestamp_slot = moment / slot_duration; let timestamp_slot = Slot::from(timestamp_slot.saturated_into::()); - assert!( - CurrentSlot::::get() == timestamp_slot, - "Timestamp slot must match `CurrentSlot`" - ); + assert!(CurrentSlot::::get() == timestamp_slot, "sdaf`"); } } diff --git a/frame/babe/src/lib.rs b/frame/babe/src/lib.rs index f673c8b43bee0..22d0fd62891a6 100644 --- a/frame/babe/src/lib.rs +++ b/frame/babe/src/lib.rs @@ -842,10 +842,13 @@ impl OnTimestampSet for Pallet { let timestamp_slot = moment / slot_duration; let timestamp_slot = Slot::from(timestamp_slot.saturated_into::()); - assert!( - CurrentSlot::::get() == timestamp_slot, - "Timestamp slot must match `CurrentSlot`" - ); + let c = CurrentSlot::::get(); + if c != timestamp_slot { + panic!( + "d: {:?}, moment: {:?}, want: {:?}, got: {:?}", + slot_duration, moment, c, timestamp_slot + ); + } } } diff --git a/utils/frame/benchmarking-cli/Cargo.toml b/utils/frame/benchmarking-cli/Cargo.toml index 131bb5638a34c..79ca0c7117870 100644 --- a/utils/frame/benchmarking-cli/Cargo.toml +++ b/utils/frame/benchmarking-cli/Cargo.toml @@ -51,6 +51,10 @@ hex = "0.4.3" memory-db = "0.29.0" rand = { version = "0.8.4", features = ["small_rng"] } +sp-inherents = { version = "4.0.0-dev", path = "../../../primitives/inherents" } +sp-consensus = { version = "0.10.0-dev", path = "../../../primitives/consensus/common" } +sc-consensus = { version = "0.10.0-dev", path = "../../../client/consensus/common" } + [features] default = ["db", "sc-client-db/runtime-benchmarks"] db = ["sc-client-db/with-kvdb-rocksdb", "sc-client-db/with-parity-db"] diff --git a/utils/frame/benchmarking-cli/src/block/bench.rs b/utils/frame/benchmarking-cli/src/block/bench.rs index 77ef74a06f60d..303df23d7e20e 100644 --- a/utils/frame/benchmarking-cli/src/block/bench.rs +++ b/utils/frame/benchmarking-cli/src/block/bench.rs @@ -15,58 +15,54 @@ // See the License for the specific language governing permissions and // limitations under the License. -use sc_block_builder::BlockBuilderApi; -use sc_cli::Result; -use sc_client_api::{BlockBackend, UsageProvider}; -use sp_api::{ApiExt, BlockId, HeaderT, ProvideRuntimeApi}; -use sp_blockchain::HeaderBackend; -use sp_runtime::{traits::Block as BlockT, DigestItem}; +use sc_block_builder::{BlockBuilder, BlockBuilderApi, BlockBuilderProvider}; +use sc_cli::{CliConfiguration, DatabaseParams, PruningParams, Result, SharedParams}; +use sc_client_api::{Backend as ClientBackend, StorageProvider, UsageProvider}; +use sc_client_db::DbHash; +use sc_consensus::{ + block_import::{BlockImportParams, ForkChoiceStrategy}, + BlockImport, StateAction, +}; +use sc_service::Configuration; +use sp_api::{ApiExt, BlockId, Core, ProvideRuntimeApi}; +use sp_blockchain::{ApplyExtrinsicFailed::Validity, Error::ApplyExtrinsicFailed, HeaderBackend}; +use sp_consensus::BlockOrigin; +use sp_database::{ColumnId, Database}; +use sp_inherents::InherentData; +use sp_runtime::{ + traits::{Block as BlockT, HashFor, Zero}, + transaction_validity::{InvalidTransaction, TransactionValidityError}, + OpaqueExtrinsic, +}; +use sp_state_machine::Storage; +use sp_storage::StateVersion; + use clap::Args; use log::{info, warn}; use serde::Serialize; use std::{marker::PhantomData, sync::Arc, time::Instant}; +use super::cmd::{CommandParams, ExtrinsicGenerator}; use crate::storage::record::Stats; /// Parameters to configure a block benchmark. #[derive(Debug, Default, Serialize, Clone, PartialEq, Args)] pub struct BenchmarkParams { - /// Skip benchmarking NO-OP extrinsics. - #[clap(long)] - pub skip_extrinsic_benchmark: bool, - - /// Skip benchmark an empty block. - #[clap(long)] - pub skip_block_benchmark: bool, - - /// Specify the number of a full block. 0 is treated as last block. - /// The block should be filled with `System::remark` extrinsics. - /// Set to one since that is the first block which could be full. - #[clap(long, default_value = "1")] - pub full_block: u32, - - /// Specify the number of an empty block. 0 is treated as last block. - /// The block should not contains any user extrinsics but only inherents. - /// Set to one since that is the first block which could be empty. - #[clap(long, default_value = "1")] - pub empty_block: u32, - - /// How many executions should be measured. - #[clap(long, default_value = "100")] - pub repeat: u32, - - /// How many executions should be run as warmup. + /// Rounds of warmups before measuring. #[clap(long, default_value = "100")] pub warmup: u32, - /// Specify the number of inherents that are present per block. - /// Blocks with extrinsics besides the inherents will be counted - /// as full and can be used for an extrinsics benchmark. - /// Blocks with only the inherents will be considered empty and - /// can only be used for a block benchmark. + /// How many times the benchmark should be repeated. + #[clap(long, default_value = "1000")] + pub repeat: u32, + + /// Limit them number of extrinsics per block. + /// + /// Only useful for debugging since for a real benchmark you + /// want full blocks. #[clap(long)] - pub num_inherents: u32, + pub max_ext_per_block: Option, } /// The results of multiple runs in ns. @@ -82,91 +78,66 @@ pub(crate) enum BenchmarkType { } /// Benchmarks the time it takes to execute an empty block or a NO-OP extrinsic. -pub(crate) struct Benchmark { +pub(crate) struct Benchmark { client: Arc, params: BenchmarkParams, - no_check: bool, - _p: PhantomData<(B, API)>, + inherent_data: sp_inherents::InherentData, + ext_gen: Arc, + _p: PhantomData<(Block, BA)>, } -impl Benchmark +impl Benchmark where - B: BlockT, - C: UsageProvider + HeaderBackend + BlockBackend + ProvideRuntimeApi, - API: ApiExt + BlockBuilderApi, + Block: BlockT, + BA: ClientBackend, + C: BlockBuilderProvider + ProvideRuntimeApi, + C::Api: ApiExt + BlockBuilderApi, { - /// Create a new benchmark object. `no_check` will ignore some safety checks. - pub fn new(client: Arc, params: BenchmarkParams, no_check: bool) -> Self { - Self { client, params, no_check, _p: PhantomData } + /// Create a new benchmark object. + pub fn new(client: Arc,inherent_data: sp_inherents::InherentData, + ext_gen: Arc, params: BenchmarkParams) -> Self { + Self { client, params, inherent_data, ext_gen, _p: PhantomData } } /// Benchmarks the execution time of a block. pub fn bench(&self, which: BenchmarkType) -> Result { - let (id, empty) = match which { - BenchmarkType::Block => (self.params.empty_block, true), - BenchmarkType::Extrinsic => (self.params.full_block, false), - }; - let (block, parent) = self.load_block(id)?; - self.check_block(&block, empty)?; - let block = self.unsealed(block)?; - - let rec = self.measure_block(&block, &parent, !empty)?; + let has_extrinsics = which.has_extrinsics(); + let (block, extrinsics) = self.build_block(!has_extrinsics)?; + // TODO only divide by extrinsics.len not block.ext.len + let rec = self.measure_block(&block, has_extrinsics)?; Stats::new(&rec) } - /// Loads a block and its parent hash. 0 loads the latest block. - fn load_block(&self, num: u32) -> Result<(B, BlockId)> { - let mut num = BlockId::Number(num.into()); - if num == BlockId::Number(0u32.into()) { - num = BlockId::Number(self.client.info().best_number); - - if num == BlockId::Number(0u32.into()) { - return Err("Chain must have some blocks but was empty".into()) - } + fn build_block(&self, empty: bool) -> Result<(Block, Vec)> { + let mut build = self.client.new_block(Default::default()).unwrap(); + let inherents = build.create_inherents(self.inherent_data.clone()).unwrap(); + for inherent in inherents { + build.push(inherent)?; } - info!("Loading block {}", num); - - let block = self - .client - .block(&num)? - .map(|b| b.block) - .ok_or::("Could not load block".into())?; - let parent = BlockId::Hash(*block.header().parent_hash()); - Ok((block, parent)) - } - /// Checks if the passed block is empty. - /// The resulting error can be demoted to a warning via `--no-check`. - fn check_block(&self, block: &B, want_empty: bool) -> Result<()> { - let num_ext = block.extrinsics().len() as u32; - info!("Block contains {} transactions", num_ext); - if num_ext < self.params.num_inherents { - return Err("A block cannot have less than `num_inherents` transactions".into()) + // Return early if we just want an empty block. + if empty { + return Ok((build.build()?.block, vec![])); } - let is_empty = num_ext == self.params.num_inherents; - if want_empty { - match (is_empty, self.no_check) { - (true, _) => {}, - (false, false) => return Err("Block should be empty but was not".into()), - (false, true) => warn!("Treating non-empty block as empty because of --no-check"), - } - } else { - match (!is_empty, self.no_check) { - (true, _) => {}, - (false, false) => return Err("Block should be full but was not".into()), - (false, true) => warn!("Treating non-full block as full because of --no-check"), + info!("Counting max NO-OPs per block, capped at {}", self.max_ext_per_block()); + let mut remarks = Vec::new(); + for nonce in 0..self.max_ext_per_block() { + let ext = self.ext_gen.remark(nonce).expect("Need remark"); + match build.push(ext.clone()) { + Ok(_) => {}, + Err(ApplyExtrinsicFailed(Validity(TransactionValidityError::Invalid( + InvalidTransaction::ExhaustsResources, + )))) => break, + Err(error) => panic!("{}", error), } + remarks.push(ext); } + assert!(!remarks.is_empty(), "A Block must hold at least one extrinsic"); + info!("Max NO-OPs per block: {}", remarks.len()); + let block = build.build()?.block; - Ok(()) - } - - /// Removes the consensus seal from a block if there is any. - fn unsealed(&self, block: B) -> Result { - let (mut header, extrinsics) = block.deconstruct(); - header.digest_mut().logs.retain(|item| !matches!(item, DigestItem::Seal(_, _))); - Ok(B::new(header, extrinsics)) + Ok((block, remarks)) } /// Measures the time that it take to execute a block. @@ -174,18 +145,19 @@ where /// by the number of extrinsics in the block. /// This is useful for the case that you want to know /// how long it takes to execute one extrinsic. - fn measure_block(&self, block: &B, before: &BlockId, per_ext: bool) -> Result { + fn measure_block(&self, block: &Block, per_ext: bool) -> Result { let mut record = BenchRecord::new(); let num_ext = block.extrinsics().len() as u64; if per_ext && num_ext == 0 { return Err("Cannot measure the extrinsic time of an empty block".into()) } + let genesis = BlockId::Number(Zero::zero()); info!("Running {} warmups...", self.params.warmup); for _ in 0..self.params.warmup { self.client .runtime_api() - .execute_block(before, block.clone()) + .execute_block(&genesis, block.clone()) .expect("Past blocks must execute"); } @@ -197,7 +169,7 @@ where let start = Instant::now(); self.client .runtime_api() - .execute_block(before, block) + .execute_block(&genesis, block) .expect("Past blocks must execute"); let elapsed = start.elapsed().as_nanos(); @@ -211,9 +183,20 @@ where Ok(record) } + + fn max_ext_per_block(&self) -> u32 { + self.params.max_ext_per_block.unwrap_or(u32::MAX) + } } impl BenchmarkType { + pub(crate) fn has_extrinsics(&self) -> bool { + match self { + Self::Extrinsic => true, + Self::Block => false, + } + } + /// Short name of the benchmark type. pub(crate) fn short_name(&self) -> &'static str { match self { diff --git a/utils/frame/benchmarking-cli/src/block/cmd.rs b/utils/frame/benchmarking-cli/src/block/cmd.rs index 9175f7a419d7b..9fcd964e6c80b 100644 --- a/utils/frame/benchmarking-cli/src/block/cmd.rs +++ b/utils/frame/benchmarking-cli/src/block/cmd.rs @@ -15,28 +15,41 @@ // See the License for the specific language governing permissions and // limitations under the License. -use sc_block_builder::BlockBuilderApi; +use sc_block_builder::{BlockBuilder, BlockBuilderApi, BlockBuilderProvider}; use sc_cli::{CliConfiguration, DatabaseParams, PruningParams, Result, SharedParams}; -use sc_client_api::{BlockBackend, UsageProvider}; +use sc_client_api::{Backend as ClientBackend, StorageProvider, UsageProvider}; +use sc_client_db::DbHash; +use sc_consensus::{ + block_import::{BlockImportParams, ForkChoiceStrategy}, + BlockImport, StateAction, +}; use sc_service::Configuration; -use sp_api::{ApiExt, ProvideRuntimeApi}; -use sp_blockchain::HeaderBackend; -use sp_runtime::traits::Block as BlockT; +use sp_api::{ApiExt, BlockId, Core, ProvideRuntimeApi}; +use sp_blockchain::{ApplyExtrinsicFailed::Validity, Error::ApplyExtrinsicFailed, HeaderBackend}; +use sp_consensus::BlockOrigin; +use sp_database::{ColumnId, Database}; +use sp_inherents::InherentData; +use sp_runtime::{ + traits::{Block as BlockT, HashFor, Zero}, + transaction_validity::{InvalidTransaction, TransactionValidityError}, + OpaqueExtrinsic, +}; +use sp_state_machine::Storage; +use sp_storage::StateVersion; use clap::{Args, Parser}; use log::info; +use rand::prelude::*; use serde::Serialize; -use std::{fmt::Debug, sync::Arc}; +use std::{boxed::Box, fmt::Debug, sync::Arc, time::Instant}; use crate::{ - block::{ - bench::{Benchmark, BenchmarkParams, BenchmarkType}, - template::TemplateData, - }, post_processing::WeightParams, -}; + storage::{ + record::{StatSelect, Stats}, +}}; +use super::{template::TemplateData, bench::{Benchmark, BenchmarkType, BenchmarkParams}}; -/// Command for running block and extrinsic benchmarks. #[derive(Debug, Parser)] pub struct BlockCmd { #[allow(missing_docs)] @@ -45,20 +58,11 @@ pub struct BlockCmd { #[allow(missing_docs)] #[clap(flatten)] - pub database_params: DatabaseParams, - - #[allow(missing_docs)] - #[clap(flatten)] - pub pruning_params: PruningParams, - - #[allow(missing_docs)] - #[clap(flatten)] - pub params: BlockParams, + pub params: CommandParams, } -/// Parameters for a block benchmark and its result post processing. #[derive(Debug, Default, Serialize, Clone, PartialEq, Args)] -pub struct BlockParams { +pub struct CommandParams { #[allow(missing_docs)] #[clap(flatten)] pub weight: WeightParams, @@ -66,33 +70,39 @@ pub struct BlockParams { #[allow(missing_docs)] #[clap(flatten)] pub bench: BenchmarkParams, +} - /// Ignore safety checks. Only useful for debugging. - #[clap(long)] - pub no_check: bool, +/// Used by the benchmark to generate signed extrinsics. +pub trait ExtrinsicGenerator { + /// Generates a `System::remark` extrinsic. + fn remark(&self, nonce: u32) -> Option; } impl BlockCmd { - /// Run the block and extrinsic benchmark. - pub async fn run(&self, cfg: Configuration, client: Arc) -> Result<()> + pub async fn run( + &self, + cfg: Configuration, + client: Arc, + inherent_data: sp_inherents::InherentData, + ext_gen: Arc, + ) -> Result<()> where - Block: BlockT, - C: UsageProvider - + HeaderBackend - + BlockBackend - + ProvideRuntimeApi, - API: ApiExt + BlockBuilderApi, + Block: BlockT, + BA: ClientBackend, + C: BlockBuilderProvider + ProvideRuntimeApi, + C::Api: ApiExt + BlockBuilderApi, { - let bench = Benchmark::new(client, self.params.bench.clone(), self.params.no_check); + let bench = Benchmark::new(client, inherent_data, ext_gen, self.params.bench.clone()); - if !self.params.bench.skip_block_benchmark { + // Empty block benchmark. + { let stats = bench.bench(BenchmarkType::Block)?; info!("Executing an empty block [ns]:\n{:?}", stats); let template = TemplateData::new(BenchmarkType::Block, &cfg, &self.params, &stats)?; template.write(&self.params.weight.weight_path)?; } - - if !self.params.bench.skip_extrinsic_benchmark { + // NO-OP extrinsic benchmark. + { let stats = bench.bench(BenchmarkType::Extrinsic)?; info!("Executing a NO-OP extrinsic [ns]:\n{:?}", stats); let template = TemplateData::new(BenchmarkType::Extrinsic, &cfg, &self.params, &stats)?; @@ -108,12 +118,4 @@ impl CliConfiguration for BlockCmd { fn shared_params(&self) -> &SharedParams { &self.shared_params } - - fn database_params(&self) -> Option<&DatabaseParams> { - Some(&self.database_params) - } - - fn pruning_params(&self) -> Option<&PruningParams> { - Some(&self.pruning_params) - } } diff --git a/utils/frame/benchmarking-cli/src/block/mod.rs b/utils/frame/benchmarking-cli/src/block/mod.rs index d68a515ba85b2..d31a0302b0352 100644 --- a/utils/frame/benchmarking-cli/src/block/mod.rs +++ b/utils/frame/benchmarking-cli/src/block/mod.rs @@ -15,6 +15,9 @@ // See the License for the specific language governing permissions and // limitations under the License. +// TODO +#![allow(unused_imports)] + mod bench; pub mod cmd; mod template; diff --git a/utils/frame/benchmarking-cli/src/block/template.rs b/utils/frame/benchmarking-cli/src/block/template.rs index d37c4f28e7aca..7e06cd5e03439 100644 --- a/utils/frame/benchmarking-cli/src/block/template.rs +++ b/utils/frame/benchmarking-cli/src/block/template.rs @@ -24,7 +24,9 @@ use serde::Serialize; use std::{env, fs, path::PathBuf}; use crate::{ - block::{bench::BenchmarkType, cmd::BlockParams}, + block::{bench::{BenchmarkParams, BenchmarkType}}, + post_processing::WeightParams, + block::cmd::CommandParams, storage::record::Stats, }; @@ -46,8 +48,8 @@ pub(crate) struct TemplateData { date: String, /// Command line arguments that were passed to the CLI. args: Vec, - /// Params of the executed command. - params: BlockParams, + /// Params of the executed command TODO. + params: CommandParams, /// Stats about the benchmark result. stats: Stats, /// The resulting weight in ns. @@ -59,7 +61,7 @@ impl TemplateData { pub(crate) fn new( t: BenchmarkType, cfg: &Configuration, - params: &BlockParams, + params: &CommandParams, stats: &Stats, ) -> Result { let weight = params.weight.calc_weight(stats)?; diff --git a/utils/frame/benchmarking-cli/src/block/weights.hbs b/utils/frame/benchmarking-cli/src/block/weights.hbs index e0772e3a915a6..08388b12c659a 100644 --- a/utils/frame/benchmarking-cli/src/block/weights.hbs +++ b/utils/frame/benchmarking-cli/src/block/weights.hbs @@ -74,6 +74,7 @@ pub mod constants { ); // At most 50 ms. assert!( +<<<<<<< HEAD w <= 50 * constants::WEIGHT_PER_MILLIS, "Weight should be at most 50 ms." ); @@ -89,6 +90,30 @@ pub mod constants { "Weight should be at most 1 ms." ); {{/if}} +======= + W::get() <= 10 * constants::WEIGHT_PER_MILLIS, + "Weight should be at most 10 ms." + ); + } + + /// Checks that the extrinsic weight exist and is sane. + // NOTE: If this test fails but you are sure that the generated values are fine, + // you can delete it. + #[test] + fn sane_extrinsic() { + use super::constants::ExtrinsicBaseWeight as W; + + // At least 10 µs. + assert!( + W::get() >= 10 * constants::WEIGHT_PER_MICROS, + "Weight should be at least 100 µs." + ); + // At most 1 ms. + assert!( + W::get() <= constants::WEIGHT_PER_MILLIS, + "Weight should be at most 10 ms." + ); +>>>>>>> fd925a91ec7 (WIP) } } } diff --git a/utils/frame/benchmarking-cli/src/storage/write.rs b/utils/frame/benchmarking-cli/src/storage/write.rs index eb9ba11f30696..49ebbcc2349a9 100644 --- a/utils/frame/benchmarking-cli/src/storage/write.rs +++ b/utils/frame/benchmarking-cli/src/storage/write.rs @@ -51,6 +51,7 @@ impl StorageCmd { let mut record = BenchRecord::default(); let supports_rc = db.supports_ref_counting(); + // TODO use HeaderBackend.info() let block = BlockId::Number(client.usage_info().chain.best_number); let header = client.header(block)?.ok_or("Header not found")?; let original_root = *header.state_root(); From 902cae348997614e3355f2767339c3c9b07e916a Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Mon, 14 Mar 2022 14:34:15 +0100 Subject: [PATCH 15/29] Push first version again Signed-off-by: Oliver Tale-Yazdi --- utils/frame/benchmarking-cli/src/block/bench.rs | 2 +- utils/frame/benchmarking-cli/src/block/cmd.rs | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/utils/frame/benchmarking-cli/src/block/bench.rs b/utils/frame/benchmarking-cli/src/block/bench.rs index 303df23d7e20e..bc83cd745dff3 100644 --- a/utils/frame/benchmarking-cli/src/block/bench.rs +++ b/utils/frame/benchmarking-cli/src/block/bench.rs @@ -119,7 +119,7 @@ where if empty { return Ok((build.build()?.block, vec![])); } - + info!("Counting max NO-OPs per block, capped at {}", self.max_ext_per_block()); let mut remarks = Vec::new(); for nonce in 0..self.max_ext_per_block() { diff --git a/utils/frame/benchmarking-cli/src/block/cmd.rs b/utils/frame/benchmarking-cli/src/block/cmd.rs index 9fcd964e6c80b..f70c7ce3be078 100644 --- a/utils/frame/benchmarking-cli/src/block/cmd.rs +++ b/utils/frame/benchmarking-cli/src/block/cmd.rs @@ -94,14 +94,12 @@ impl BlockCmd { { let bench = Benchmark::new(client, inherent_data, ext_gen, self.params.bench.clone()); - // Empty block benchmark. { let stats = bench.bench(BenchmarkType::Block)?; info!("Executing an empty block [ns]:\n{:?}", stats); let template = TemplateData::new(BenchmarkType::Block, &cfg, &self.params, &stats)?; template.write(&self.params.weight.weight_path)?; } - // NO-OP extrinsic benchmark. { let stats = bench.bench(BenchmarkType::Extrinsic)?; info!("Executing a NO-OP extrinsic [ns]:\n{:?}", stats); From 316209b01dfcdd9d539d7bfb928fc4730002df0d Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Mon, 14 Mar 2022 22:13:49 +0100 Subject: [PATCH 16/29] Cleanup Signed-off-by: Oliver Tale-Yazdi --- bin/node-template/node/src/cli.rs | 4 - bin/node-template/node/src/command.rs | 46 +------ bin/node/cli/src/cli.rs | 11 +- bin/node/cli/src/command.rs | 14 ++- block_weights.rs | 79 ++++++++++++ extrinsic_weights.rs | 79 ++++++++++++ .../benchmarking-cli/src/block/weights.hbs | 119 ------------------ utils/frame/benchmarking-cli/src/lib.rs | 4 +- .../src/{block => overhead}/bench.rs | 109 ++++++++-------- .../src/{block => overhead}/cmd.rs | 63 +++++----- .../src/{block => overhead}/mod.rs | 5 +- .../src/{block => overhead}/template.rs | 8 +- .../benchmarking-cli/src/overhead/weights.hbs | 92 ++++++++++++++ 13 files changed, 352 insertions(+), 281 deletions(-) create mode 100644 block_weights.rs create mode 100644 extrinsic_weights.rs delete mode 100644 utils/frame/benchmarking-cli/src/block/weights.hbs rename utils/frame/benchmarking-cli/src/{block => overhead}/bench.rs (66%) rename utils/frame/benchmarking-cli/src/{block => overhead}/cmd.rs (62%) rename utils/frame/benchmarking-cli/src/{block => overhead}/mod.rs (92%) rename utils/frame/benchmarking-cli/src/{block => overhead}/template.rs (95%) create mode 100644 utils/frame/benchmarking-cli/src/overhead/weights.hbs diff --git a/bin/node-template/node/src/cli.rs b/bin/node-template/node/src/cli.rs index 9db00ccdb5dd9..c4d27b71e4994 100644 --- a/bin/node-template/node/src/cli.rs +++ b/bin/node-template/node/src/cli.rs @@ -40,10 +40,6 @@ pub enum Subcommand { #[clap(name = "benchmark", about = "Benchmark runtime pallets.")] Benchmark(frame_benchmarking_cli::BenchmarkCmd), - /// The custom benchmark subcommand benchmarking runtime pallets. - #[clap(name = "benchmark-block", about = "Benchmark runtime pallets.")] - BenchmarkBlock(frame_benchmarking_cli::BlockCmd), - /// Try some command against runtime state. #[cfg(feature = "try-runtime")] TryRuntime(try_runtime_cli::TryRuntimeCmd), diff --git a/bin/node-template/node/src/command.rs b/bin/node-template/node/src/command.rs index b70694935f56b..72c7a75b387bb 100644 --- a/bin/node-template/node/src/command.rs +++ b/bin/node-template/node/src/command.rs @@ -3,13 +3,10 @@ use crate::{ cli::{Cli, Subcommand}, service, }; -use node_cli::service::create_extrinsic; -use node_runtime::SystemCall; +use node_template_runtime::Block; use sc_cli::{ChainSpec, RuntimeVersion, SubstrateCli}; use sc_service::PartialComponents; -use std::sync::Arc; - impl SubstrateCli for Cli { fn impl_name() -> String { "Substrate Node".into() @@ -49,47 +46,6 @@ impl SubstrateCli for Cli { } } -use super::service::FullClient; -struct ExtrinsicGen { - client: Arc, -} -use node_runtime::UncheckedExtrinsic; -use node_template_runtime::Block; -use sp_keyring::Sr25519Keyring; -use sp_runtime::{traits::Block as BlockT, OpaqueExtrinsic}; -/* -impl frame_benchmarking_cli::block::cmd::ExtrinsicGenerator for ExtrinsicGen { - fn noop(&self, nonce: u32) -> Option { - let src = Sr25519Keyring::Alice.pair(); - - let extrinsic: OpaqueExtrinsic = create_extrinsic( - self.client.as_ref(), - src.clone(), - SystemCall::remark { remark: vec![] }, - Some(nonce), - ) - .into(); - None - } -}*/ -/* -#[derive(Default)] -struct InherentProv {} - -impl frame_benchmarking_cli::block::cmd::BlockInherentDataProvider for InherentProv { - fn providers( - &self, - block: u64, - ) -> std::result::Result>, sp_inherents::Error> - { - log::info!("Creating inherents for block #{}", block); - let d = std::time::Duration::from_millis(0); - let timestamp = sp_timestamp::InherentDataProvider::new(d.into()); - - Ok(vec![Arc::new(timestamp)]) - } -}*/ - /// Parse and run command line arguments pub fn run() -> sc_cli::Result<()> { let cli = Cli::from_args(); diff --git a/bin/node/cli/src/cli.rs b/bin/node/cli/src/cli.rs index 830616a8b4541..a911cc26ef87c 100644 --- a/bin/node/cli/src/cli.rs +++ b/bin/node/cli/src/cli.rs @@ -42,10 +42,13 @@ pub enum Subcommand { #[clap(name = "benchmark", about = "Benchmark runtime pallets.")] Benchmark(frame_benchmarking_cli::BenchmarkCmd), - /// Sub command for benchmarking block and extrinsic base weight. - #[clap(name = "benchmark-block", about = "Benchmark block and extrinsic base 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), diff --git a/bin/node/cli/src/command.rs b/bin/node/cli/src/command.rs index dfbe747fe72c6..daae897d87c31 100644 --- a/bin/node/cli/src/command.rs +++ b/bin/node/cli/src/command.rs @@ -75,12 +75,12 @@ struct ExtrinsicGen { } use crate::service::{create_extrinsic, FullClient}; use node_primitives::Block; -use node_runtime::{SystemCall}; +use node_runtime::SystemCall; use sp_keyring::Sr25519Keyring; -use sp_runtime::{OpaqueExtrinsic}; -impl frame_benchmarking_cli::block::cmd::ExtrinsicGenerator for ExtrinsicGen { +use sp_runtime::OpaqueExtrinsic; +impl frame_benchmarking_cli::overhead::cmd::ExtrinsicGenerator for ExtrinsicGen { fn remark(&self, nonce: u32) -> Option { - let src = Sr25519Keyring::Alice.pair(); + let src = Sr25519Keyring::Bob.pair(); // This `create_extrinsic` only exists for the node and has no // equivalent in other runtimes. @@ -133,12 +133,14 @@ pub fn run() -> Result<()> { You can enable it with `--features runtime-benchmarks`." .into()) }, - Some(Subcommand::BenchmarkBlock(cmd)) => { + Some(Subcommand::BenchmarkOverhead(cmd)) => { let runner = cli.create_runner(cmd)?; runner.async_run(|mut config| { + // TODO 1. is this needed? + // 2. should other stuff be disabled as well? config.role = sc_service::Role::Full; - let PartialComponents { client, task_manager, backend, .. } = new_partial(&config)?; + let PartialComponents { client, task_manager, .. } = new_partial(&config)?; let ext_gen = ExtrinsicGen { client: client.clone() }; let inherents = inherent_data()?; diff --git a/block_weights.rs b/block_weights.rs new file mode 100644 index 0000000000000..1a6bba8cd091a --- /dev/null +++ b/block_weights.rs @@ -0,0 +1,79 @@ +// This file is part of Substrate. + +// Copyright (C) 2022 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 4.0.0-dev +//! DATE: 2022-03-14 (Y/M/D) +//! +//! SHORT-NAME: `block`, LONG-NAME: `BlockExecution`, RUNTIME: `Development` +//! WARMUPS: `1`, REPEAT: `1` +//! WEIGHT-PATH: `.` +//! WEIGHT-METRIC: `Average`, WEIGHT-MUL: `1`, WEIGHT-ADD: `0` + +// Executed Command: +// ./target/release/substrate +// benchmark-overhead +// --dev +// --warmup +// 1 +// --repeat +// 1 + +use frame_support::{ + parameter_types, + weights::{constants::WEIGHT_PER_NANOS, Weight}, +}; + +parameter_types! { + /// Time to execute an empty block. + /// Calculated by multiplying the *Average* with `1` and adding `0`. + /// + /// Stats [ns]: + /// Min, Max: 5_172_175, 5_172_175 + /// Average: 5_172_175 + /// Median: 5_172_175 + /// StdDev: 0 + /// + /// Percentiles [ns]: + /// 99th: 5_172_175 + /// 95th: 5_172_175 + /// 75th: 5_172_175 + pub const BlockExecutionWeight: Weight = 5_172_175 * WEIGHT_PER_NANOS; +} + +#[cfg(test)] +mod test_weights { + use frame_support::weights::constants; + + /// Checks that the weight exists and is sane. + // NOTE: If this test fails but you are sure that the generated values are fine, + // you can delete it. + #[test] + fn sane() { + let w = super::constants::BlockExecutionWeight::get(); + + // At least 100 µs. + assert!( + w >= 100 * constants::WEIGHT_PER_MICROS, + "Weight should be at least 100 µs." + ); + // At most 50 ms. + assert!( + w <= 50 * constants::WEIGHT_PER_MILLIS, + "Weight should be at most 50 ms." + ); + } +} diff --git a/extrinsic_weights.rs b/extrinsic_weights.rs new file mode 100644 index 0000000000000..7ab24047252e4 --- /dev/null +++ b/extrinsic_weights.rs @@ -0,0 +1,79 @@ +// This file is part of Substrate. + +// Copyright (C) 2022 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 4.0.0-dev +//! DATE: 2022-03-14 (Y/M/D) +//! +//! SHORT-NAME: `extrinsic`, LONG-NAME: `Extrinsic`, RUNTIME: `Development` +//! WARMUPS: `1`, REPEAT: `1` +//! WEIGHT-PATH: `.` +//! WEIGHT-METRIC: `Average`, WEIGHT-MUL: `1`, WEIGHT-ADD: `0` + +// Executed Command: +// ./target/release/substrate +// benchmark-overhead +// --dev +// --warmup +// 1 +// --repeat +// 1 + +use frame_support::{ + parameter_types, + weights::{constants::WEIGHT_PER_NANOS, Weight}, +}; + +parameter_types! { + /// Time to execute a NO-OP extrinsic eg. `System::remark`. + /// Calculated by multiplying the *Average* with `1` and adding `0`. + /// + /// Stats [ns]: + /// Min, Max: 110_996, 110_996 + /// Average: 110_996 + /// Median: 110_996 + /// StdDev: 0 + /// + /// Percentiles [ns]: + /// 99th: 110_996 + /// 95th: 110_996 + /// 75th: 110_996 + pub const ExtrinsicWeight: Weight = 110_996 * WEIGHT_PER_NANOS; +} + +#[cfg(test)] +mod test_weights { + use frame_support::weights::constants; + + /// Checks that the weight exists and is sane. + // NOTE: If this test fails but you are sure that the generated values are fine, + // you can delete it. + #[test] + fn sane() { + let w = super::constants::ExtrinsicWeight::get(); + + // At least 10 µs. + assert!( + w >= 10 * constants::WEIGHT_PER_MICROS, + "Weight should be at least 10 µs." + ); + // At most 1 ms. + assert!( + w <= constants::WEIGHT_PER_MILLIS, + "Weight should be at most 1 ms." + ); + } +} diff --git a/utils/frame/benchmarking-cli/src/block/weights.hbs b/utils/frame/benchmarking-cli/src/block/weights.hbs deleted file mode 100644 index 08388b12c659a..0000000000000 --- a/utils/frame/benchmarking-cli/src/block/weights.hbs +++ /dev/null @@ -1,119 +0,0 @@ -// This file is part of Substrate. - -// Copyright (C) 2022 Parity Technologies (UK) Ltd. -// SPDX-License-Identifier: Apache-2.0 - -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION {{version}} -//! DATE: {{date}} -//! -//! SHORT-NAME: `{{short_name}}`, LONG-NAME: `{{long_name}}`, RUNTIME: `{{runtime_name}}` -//! WARMUPS: `{{params.bench.warmup}}`, REPEAT: `{{params.bench.repeat}}` -//! WEIGHT-PATH: `{{params.weight.weight_path}}` -//! WEIGHT-METRIC: `{{params.weight.weight_metric}}`, WEIGHT-MUL: `{{params.weight.weight_mul}}`, WEIGHT-ADD: `{{params.weight.weight_add}}` - -// Executed Command: -{{#each args as |arg|}} -// {{arg}} -{{/each}} - -pub mod constants { - use frame_support::{ - parameter_types, - weights::{constants::WEIGHT_PER_NANOS, Weight}, - }; - - parameter_types! { - {{#if (eq short_name "block")}} - /// Time to execute an empty block. - {{else}} - /// Time to execute a NO-OP extrinsic eg. `System::remark`. - {{/if}} - /// Calculated by multiplying the *{{params.weight.weight_metric}}* with `{{params.weight.weight_mul}}` and adding `{{params.weight.weight_add}}`. - /// - /// Stats [ns]: - /// Min, Max: {{underscore stats.min}}, {{underscore stats.max}} - /// Average: {{underscore stats.avg}} - /// Median: {{underscore stats.median}} - /// StdDev: {{stats.stddev}} - /// - /// Percentiles [ns]: - /// 99th: {{underscore stats.p99}} - /// 95th: {{underscore stats.p95}} - /// 75th: {{underscore stats.p75}} - pub const {{long_name}}Weight: Weight = {{underscore weight}} * WEIGHT_PER_NANOS; - } - - #[cfg(test)] - mod test_weights { - use frame_support::weights::constants; - - /// Checks that the weight exists and is sane. - // NOTE: If this test fails but you are sure that the generated values are fine, - // you can delete it. - #[test] - fn sane() { - let w = super::constants::{{long_name}}Weight::get(); - - {{#if (eq short_name "block")}} - // At least 100 µs. - assert!( - w >= 100 * constants::WEIGHT_PER_MICROS, - "Weight should be at least 100 µs." - ); - // At most 50 ms. - assert!( -<<<<<<< HEAD - w <= 50 * constants::WEIGHT_PER_MILLIS, - "Weight should be at most 50 ms." - ); - {{else}} - // At least 10 µs. - assert!( - w >= 10 * constants::WEIGHT_PER_MICROS, - "Weight should be at least 10 µs." - ); - // At most 1 ms. - assert!( - w <= constants::WEIGHT_PER_MILLIS, - "Weight should be at most 1 ms." - ); - {{/if}} -======= - W::get() <= 10 * constants::WEIGHT_PER_MILLIS, - "Weight should be at most 10 ms." - ); - } - - /// Checks that the extrinsic weight exist and is sane. - // NOTE: If this test fails but you are sure that the generated values are fine, - // you can delete it. - #[test] - fn sane_extrinsic() { - use super::constants::ExtrinsicBaseWeight as W; - - // At least 10 µs. - assert!( - W::get() >= 10 * constants::WEIGHT_PER_MICROS, - "Weight should be at least 100 µs." - ); - // At most 1 ms. - assert!( - W::get() <= constants::WEIGHT_PER_MILLIS, - "Weight should be at most 10 ms." - ); ->>>>>>> fd925a91ec7 (WIP) - } - } -} diff --git a/utils/frame/benchmarking-cli/src/lib.rs b/utils/frame/benchmarking-cli/src/lib.rs index ae937b6b3dad4..d75a2bc229810 100644 --- a/utils/frame/benchmarking-cli/src/lib.rs +++ b/utils/frame/benchmarking-cli/src/lib.rs @@ -15,8 +15,8 @@ // See the License for the specific language governing permissions and // limitations under the License. -pub mod block; mod command; +pub mod overhead; mod post_processing; mod storage; mod writer; @@ -24,7 +24,7 @@ mod writer; use sc_cli::{ExecutionStrategy, WasmExecutionMethod}; use std::{fmt::Debug, path::PathBuf}; -pub use block::BlockCmd; +pub use overhead::{ExtrinsicGenerator, OverheadCmd}; pub use storage::StorageCmd; // Add a more relaxed parsing for pallet names by allowing pallet directory names with `-` to be diff --git a/utils/frame/benchmarking-cli/src/block/bench.rs b/utils/frame/benchmarking-cli/src/overhead/bench.rs similarity index 66% rename from utils/frame/benchmarking-cli/src/block/bench.rs rename to utils/frame/benchmarking-cli/src/overhead/bench.rs index bc83cd745dff3..07a613e847ce7 100644 --- a/utils/frame/benchmarking-cli/src/block/bench.rs +++ b/utils/frame/benchmarking-cli/src/overhead/bench.rs @@ -15,36 +15,23 @@ // See the License for the specific language governing permissions and // limitations under the License. -use sc_block_builder::{BlockBuilder, BlockBuilderApi, BlockBuilderProvider}; -use sc_cli::{CliConfiguration, DatabaseParams, PruningParams, Result, SharedParams}; -use sc_client_api::{Backend as ClientBackend, StorageProvider, UsageProvider}; -use sc_client_db::DbHash; -use sc_consensus::{ - block_import::{BlockImportParams, ForkChoiceStrategy}, - BlockImport, StateAction, -}; -use sc_service::Configuration; +use sc_block_builder::{BlockBuilderApi, BlockBuilderProvider}; +use sc_cli::Result; +use sc_client_api::Backend as ClientBackend; use sp_api::{ApiExt, BlockId, Core, ProvideRuntimeApi}; -use sp_blockchain::{ApplyExtrinsicFailed::Validity, Error::ApplyExtrinsicFailed, HeaderBackend}; -use sp_consensus::BlockOrigin; -use sp_database::{ColumnId, Database}; -use sp_inherents::InherentData; +use sp_blockchain::{ApplyExtrinsicFailed::Validity, Error::ApplyExtrinsicFailed}; use sp_runtime::{ - traits::{Block as BlockT, HashFor, Zero}, + traits::{Block as BlockT, Zero}, transaction_validity::{InvalidTransaction, TransactionValidityError}, OpaqueExtrinsic, }; -use sp_state_machine::Storage; -use sp_storage::StateVersion; - use clap::Args; -use log::{info, warn}; +use log::info; use serde::Serialize; use std::{marker::PhantomData, sync::Arc, time::Instant}; -use super::cmd::{CommandParams, ExtrinsicGenerator}; -use crate::storage::record::Stats; +use crate::{overhead::cmd::ExtrinsicGenerator, storage::record::Stats}; /// Parameters to configure a block benchmark. #[derive(Debug, Default, Serialize, Clone, PartialEq, Args)] @@ -69,11 +56,11 @@ pub struct BenchmarkParams { pub(crate) type BenchRecord = Vec; /// Type of a benchmark. -#[derive(Serialize, Clone)] +#[derive(Serialize, Clone, PartialEq)] pub(crate) enum BenchmarkType { - /// Per-extrinsic execution overhead was measured. + /// Measure the per-extrinsic execution overhead. Extrinsic, - /// Per-block execution overhead was measured. + /// Measure the per-block execution overhead. Block, } @@ -93,35 +80,42 @@ where C: BlockBuilderProvider + ProvideRuntimeApi, C::Api: ApiExt + BlockBuilderApi, { - /// Create a new benchmark object. - pub fn new(client: Arc,inherent_data: sp_inherents::InherentData, - ext_gen: Arc, params: BenchmarkParams) -> Self { + /// Create a new [`Self`] from the arguments. + pub fn new( + client: Arc, + params: BenchmarkParams, + inherent_data: sp_inherents::InherentData, + ext_gen: Arc, + ) -> Self { Self { client, params, inherent_data, ext_gen, _p: PhantomData } } - /// Benchmarks the execution time of a block. - pub fn bench(&self, which: BenchmarkType) -> Result { - let has_extrinsics = which.has_extrinsics(); - let (block, extrinsics) = self.build_block(!has_extrinsics)?; - // TODO only divide by extrinsics.len not block.ext.len - let rec = self.measure_block(&block, has_extrinsics)?; - Stats::new(&rec) + /// Run the specified benchmark. + pub fn bench(&self, bench_type: BenchmarkType) -> Result { + let (block, num_ext) = self.build_block(bench_type.clone())?; + let record = self.measure_block(&block, num_ext, bench_type)?; + Stats::new(&record) } - fn build_block(&self, empty: bool) -> Result<(Block, Vec)> { + /// Builds a block for the given benchmark type. + /// + /// Returns the block and the number of extrinsics in the block. + fn build_block(&self, bench_type: BenchmarkType) -> Result<(Block, u64)> { let mut build = self.client.new_block(Default::default()).unwrap(); - let inherents = build.create_inherents(self.inherent_data.clone()).unwrap(); + // Create and insert the inherents. + let inherents = build.create_inherents(self.inherent_data.clone())?; for inherent in inherents { build.push(inherent)?; } // Return early if we just want an empty block. - if empty { - return Ok((build.build()?.block, vec![])); + if bench_type == BenchmarkType::Block { + return Ok((build.build()?.block, 0)) } - - info!("Counting max NO-OPs per block, capped at {}", self.max_ext_per_block()); - let mut remarks = Vec::new(); + + // Put as many extrinsics into the block as possible and count them. + info!("Building block..."); + let mut num_ext = 0; for nonce in 0..self.max_ext_per_block() { let ext = self.ext_gen.remark(nonce).expect("Need remark"); match build.push(ext.clone()) { @@ -131,24 +125,28 @@ where )))) => break, Err(error) => panic!("{}", error), } - remarks.push(ext); + num_ext += 1; } - assert!(!remarks.is_empty(), "A Block must hold at least one extrinsic"); - info!("Max NO-OPs per block: {}", remarks.len()); + assert!(num_ext != 0, "A Block must hold at least one extrinsic"); + info!("Remarks per block: {}", num_ext); let block = build.build()?.block; - Ok((block, remarks)) + Ok((block, num_ext)) } - /// Measures the time that it take to execute a block. + /// Measures the time that it take to execute a block or an extrinsic. /// `per_ext` specifies if the result should be divided /// by the number of extrinsics in the block. /// This is useful for the case that you want to know /// how long it takes to execute one extrinsic. - fn measure_block(&self, block: &Block, per_ext: bool) -> Result { + fn measure_block( + &self, + block: &Block, + num_ext: u64, + bench_type: BenchmarkType, + ) -> Result { let mut record = BenchRecord::new(); - let num_ext = block.extrinsics().len() as u64; - if per_ext && num_ext == 0 { + if bench_type == BenchmarkType::Extrinsic && num_ext == 0 { return Err("Cannot measure the extrinsic time of an empty block".into()) } let genesis = BlockId::Number(Zero::zero()); @@ -173,8 +171,8 @@ where .expect("Past blocks must execute"); let elapsed = start.elapsed().as_nanos(); - if per_ext { - // checked for non zero div above. + if bench_type == BenchmarkType::Extrinsic { + // Checked for non-zero div above. record.push(elapsed as u64 / num_ext); } else { record.push(elapsed as u64); @@ -184,19 +182,14 @@ where Ok(record) } + /// Returns the maximum number of extrinsics that should be put in + /// a block or `u32::MAX` if no option was passed. fn max_ext_per_block(&self) -> u32 { self.params.max_ext_per_block.unwrap_or(u32::MAX) } } impl BenchmarkType { - pub(crate) fn has_extrinsics(&self) -> bool { - match self { - Self::Extrinsic => true, - Self::Block => false, - } - } - /// Short name of the benchmark type. pub(crate) fn short_name(&self) -> &'static str { match self { @@ -208,7 +201,7 @@ impl BenchmarkType { /// Long name of the benchmark type. pub(crate) fn long_name(&self) -> &'static str { match self { - Self::Extrinsic => "ExtrinsicBase", + Self::Extrinsic => "Extrinsic", Self::Block => "BlockExecution", } } diff --git a/utils/frame/benchmarking-cli/src/block/cmd.rs b/utils/frame/benchmarking-cli/src/overhead/cmd.rs similarity index 62% rename from utils/frame/benchmarking-cli/src/block/cmd.rs rename to utils/frame/benchmarking-cli/src/overhead/cmd.rs index f70c7ce3be078..4892623ab6198 100644 --- a/utils/frame/benchmarking-cli/src/block/cmd.rs +++ b/utils/frame/benchmarking-cli/src/overhead/cmd.rs @@ -15,54 +15,41 @@ // See the License for the specific language governing permissions and // limitations under the License. -use sc_block_builder::{BlockBuilder, BlockBuilderApi, BlockBuilderProvider}; -use sc_cli::{CliConfiguration, DatabaseParams, PruningParams, Result, SharedParams}; -use sc_client_api::{Backend as ClientBackend, StorageProvider, UsageProvider}; -use sc_client_db::DbHash; -use sc_consensus::{ - block_import::{BlockImportParams, ForkChoiceStrategy}, - BlockImport, StateAction, -}; +use sc_block_builder::{BlockBuilderApi, BlockBuilderProvider}; +use sc_cli::{CliConfiguration, Result, SharedParams}; +use sc_client_api::Backend as ClientBackend; use sc_service::Configuration; -use sp_api::{ApiExt, BlockId, Core, ProvideRuntimeApi}; -use sp_blockchain::{ApplyExtrinsicFailed::Validity, Error::ApplyExtrinsicFailed, HeaderBackend}; -use sp_consensus::BlockOrigin; -use sp_database::{ColumnId, Database}; -use sp_inherents::InherentData; -use sp_runtime::{ - traits::{Block as BlockT, HashFor, Zero}, - transaction_validity::{InvalidTransaction, TransactionValidityError}, - OpaqueExtrinsic, -}; -use sp_state_machine::Storage; -use sp_storage::StateVersion; +use sp_api::{ApiExt, ProvideRuntimeApi}; +use sp_runtime::{traits::Block as BlockT, OpaqueExtrinsic}; use clap::{Args, Parser}; use log::info; -use rand::prelude::*; use serde::Serialize; -use std::{boxed::Box, fmt::Debug, sync::Arc, time::Instant}; +use std::{fmt::Debug, sync::Arc}; use crate::{ + overhead::{ + bench::{Benchmark, BenchmarkParams, BenchmarkType}, + template::TemplateData, + }, post_processing::WeightParams, - storage::{ - record::{StatSelect, Stats}, -}}; -use super::{template::TemplateData, bench::{Benchmark, BenchmarkType, BenchmarkParams}}; +}; +/// Benchmarks the per-block and per-extrinsic execution overhead. #[derive(Debug, Parser)] -pub struct BlockCmd { +pub struct OverheadCmd { #[allow(missing_docs)] #[clap(flatten)] pub shared_params: SharedParams, #[allow(missing_docs)] #[clap(flatten)] - pub params: CommandParams, + pub params: OverheadParams, } +/// Configures the benchmark and the post processing and weight generation. #[derive(Debug, Default, Serialize, Clone, PartialEq, Args)] -pub struct CommandParams { +pub struct OverheadParams { #[allow(missing_docs)] #[clap(flatten)] pub weight: WeightParams, @@ -73,12 +60,18 @@ pub struct CommandParams { } /// Used by the benchmark to generate signed extrinsics. +/// This must be implemented per-runtime since the signed extensions +/// depend on the runtime. pub trait ExtrinsicGenerator { /// Generates a `System::remark` extrinsic. fn remark(&self, nonce: u32) -> Option; } -impl BlockCmd { +impl OverheadCmd { + /// Measures the per-block and per-extrinsic execution overhead. + /// + /// Writes the results to console and into two instances of the + /// `weights.hbs` template. pub async fn run( &self, cfg: Configuration, @@ -92,17 +85,19 @@ impl BlockCmd { C: BlockBuilderProvider + ProvideRuntimeApi, C::Api: ApiExt + BlockBuilderApi, { - let bench = Benchmark::new(client, inherent_data, ext_gen, self.params.bench.clone()); + let bench = Benchmark::new(client, self.params.bench.clone(), inherent_data, ext_gen); + // per-block execution overhead { let stats = bench.bench(BenchmarkType::Block)?; - info!("Executing an empty block [ns]:\n{:?}", stats); + info!("Per-block execution overhead [ns]:\n{:?}", stats); let template = TemplateData::new(BenchmarkType::Block, &cfg, &self.params, &stats)?; template.write(&self.params.weight.weight_path)?; } + // per-extrinsic execution overhead { let stats = bench.bench(BenchmarkType::Extrinsic)?; - info!("Executing a NO-OP extrinsic [ns]:\n{:?}", stats); + info!("Per-extrinsic execution overhead [ns]:\n{:?}", stats); let template = TemplateData::new(BenchmarkType::Extrinsic, &cfg, &self.params, &stats)?; template.write(&self.params.weight.weight_path)?; } @@ -112,7 +107,7 @@ impl BlockCmd { } // Boilerplate -impl CliConfiguration for BlockCmd { +impl CliConfiguration for OverheadCmd { fn shared_params(&self) -> &SharedParams { &self.shared_params } diff --git a/utils/frame/benchmarking-cli/src/block/mod.rs b/utils/frame/benchmarking-cli/src/overhead/mod.rs similarity index 92% rename from utils/frame/benchmarking-cli/src/block/mod.rs rename to utils/frame/benchmarking-cli/src/overhead/mod.rs index d31a0302b0352..9bc5582eb9103 100644 --- a/utils/frame/benchmarking-cli/src/block/mod.rs +++ b/utils/frame/benchmarking-cli/src/overhead/mod.rs @@ -15,11 +15,8 @@ // See the License for the specific language governing permissions and // limitations under the License. -// TODO -#![allow(unused_imports)] - mod bench; pub mod cmd; mod template; -pub use cmd::BlockCmd; +pub use cmd::{ExtrinsicGenerator, OverheadCmd}; diff --git a/utils/frame/benchmarking-cli/src/block/template.rs b/utils/frame/benchmarking-cli/src/overhead/template.rs similarity index 95% rename from utils/frame/benchmarking-cli/src/block/template.rs rename to utils/frame/benchmarking-cli/src/overhead/template.rs index 7e06cd5e03439..5614512f62769 100644 --- a/utils/frame/benchmarking-cli/src/block/template.rs +++ b/utils/frame/benchmarking-cli/src/overhead/template.rs @@ -24,9 +24,7 @@ use serde::Serialize; use std::{env, fs, path::PathBuf}; use crate::{ - block::{bench::{BenchmarkParams, BenchmarkType}}, - post_processing::WeightParams, - block::cmd::CommandParams, + overhead::{bench::BenchmarkType, cmd::OverheadParams}, storage::record::Stats, }; @@ -49,7 +47,7 @@ pub(crate) struct TemplateData { /// Command line arguments that were passed to the CLI. args: Vec, /// Params of the executed command TODO. - params: CommandParams, + params: OverheadParams, /// Stats about the benchmark result. stats: Stats, /// The resulting weight in ns. @@ -61,7 +59,7 @@ impl TemplateData { pub(crate) fn new( t: BenchmarkType, cfg: &Configuration, - params: &CommandParams, + params: &OverheadParams, stats: &Stats, ) -> Result { let weight = params.weight.calc_weight(stats)?; diff --git a/utils/frame/benchmarking-cli/src/overhead/weights.hbs b/utils/frame/benchmarking-cli/src/overhead/weights.hbs new file mode 100644 index 0000000000000..9e02b023dfb0f --- /dev/null +++ b/utils/frame/benchmarking-cli/src/overhead/weights.hbs @@ -0,0 +1,92 @@ +// This file is part of Substrate. + +// Copyright (C) 2022 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION {{version}} +//! DATE: {{date}} +//! +//! SHORT-NAME: `{{short_name}}`, LONG-NAME: `{{long_name}}`, RUNTIME: `{{runtime_name}}` +//! WARMUPS: `{{params.bench.warmup}}`, REPEAT: `{{params.bench.repeat}}` +//! WEIGHT-PATH: `{{params.weight.weight_path}}` +//! WEIGHT-METRIC: `{{params.weight.weight_metric}}`, WEIGHT-MUL: `{{params.weight.weight_mul}}`, WEIGHT-ADD: `{{params.weight.weight_add}}` + +// Executed Command: +{{#each args as |arg|}} +// {{arg}} +{{/each}} + +use frame_support::{ + parameter_types, + weights::{constants::WEIGHT_PER_NANOS, Weight}, +}; + +parameter_types! { + {{#if (eq short_name "block")}} + /// Time to execute an empty block. + {{else}} + /// Time to execute a NO-OP extrinsic eg. `System::remark`. + {{/if}} + /// Calculated by multiplying the *{{params.weight.weight_metric}}* with `{{params.weight.weight_mul}}` and adding `{{params.weight.weight_add}}`. + /// + /// Stats [ns]: + /// Min, Max: {{underscore stats.min}}, {{underscore stats.max}} + /// Average: {{underscore stats.avg}} + /// Median: {{underscore stats.median}} + /// StdDev: {{stats.stddev}} + /// + /// Percentiles [ns]: + /// 99th: {{underscore stats.p99}} + /// 95th: {{underscore stats.p95}} + /// 75th: {{underscore stats.p75}} + pub const {{long_name}}Weight: Weight = {{underscore weight}} * WEIGHT_PER_NANOS; +} + +#[cfg(test)] +mod test_weights { + use frame_support::weights::constants; + + /// Checks that the weight exists and is sane. + // NOTE: If this test fails but you are sure that the generated values are fine, + // you can delete it. + #[test] + fn sane() { + let w = super::constants::{{long_name}}Weight::get(); + + {{#if (eq short_name "block")}} + // At least 100 µs. + assert!( + w >= 100 * constants::WEIGHT_PER_MICROS, + "Weight should be at least 100 µs." + ); + // At most 50 ms. + assert!( + w <= 50 * constants::WEIGHT_PER_MILLIS, + "Weight should be at most 50 ms." + ); + {{else}} + // At least 10 µs. + assert!( + w >= 10 * constants::WEIGHT_PER_MICROS, + "Weight should be at least 10 µs." + ); + // At most 1 ms. + assert!( + w <= constants::WEIGHT_PER_MILLIS, + "Weight should be at most 1 ms." + ); + {{/if}} + } +} From b40b650461dfed91082b96fbf1451863ac089870 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Mon, 14 Mar 2022 22:52:40 +0100 Subject: [PATCH 17/29] Cleanup Signed-off-by: Oliver Tale-Yazdi --- Cargo.lock | 5 +- bin/node-template/node/Cargo.toml | 10 --- bin/node-template/node/src/service.rs | 2 +- bin/node/cli/Cargo.toml | 6 -- bin/node/cli/src/command.rs | 41 ++++++---- block_weights.rs | 79 ------------------- extrinsic_weights.rs | 79 ------------------- frame/aura/src/lib.rs | 5 +- frame/babe/src/lib.rs | 11 +-- utils/frame/benchmarking-cli/Cargo.toml | 9 +-- utils/frame/benchmarking-cli/src/lib.rs | 2 +- .../benchmarking-cli/src/overhead/bench.rs | 56 ++++++------- .../benchmarking-cli/src/overhead/cmd.rs | 10 +-- .../benchmarking-cli/src/overhead/mod.rs | 2 +- .../benchmarking-cli/src/overhead/template.rs | 6 +- .../benchmarking-cli/src/storage/write.rs | 1 - 16 files changed, 73 insertions(+), 251 deletions(-) delete mode 100644 block_weights.rs delete mode 100644 extrinsic_weights.rs diff --git a/Cargo.lock b/Cargo.lock index 9f4d2f21cf9c1..0ea5b8d4f094b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2129,6 +2129,7 @@ dependencies = [ "clap 3.1.6", "frame-benchmarking", "frame-support", + "futures 0.3.19", "handlebars", "hash-db", "hex", @@ -2139,6 +2140,7 @@ dependencies = [ "memory-db", "parity-scale-codec", "rand 0.8.4", + "sc-block-builder", "sc-cli", "sc-client-api", "sc-client-db", @@ -2152,6 +2154,7 @@ dependencies = [ "sp-core", "sp-database", "sp-externalities", + "sp-inherents", "sp-keystore", "sp-runtime", "sp-state-machine", @@ -2196,7 +2199,7 @@ dependencies = [ name = "frame-election-solution-type-fuzzer" version = "2.0.0-alpha.5" dependencies = [ - "clap 3.0.7", + "clap 3.1.6", "frame-election-provider-solution-type", "honggfuzz", "parity-scale-codec", diff --git a/bin/node-template/node/Cargo.toml b/bin/node-template/node/Cargo.toml index e70843f6ece14..4549a5b613da2 100644 --- a/bin/node-template/node/Cargo.toml +++ b/bin/node-template/node/Cargo.toml @@ -37,12 +37,6 @@ sc-client-api = { version = "4.0.0-dev", path = "../../../client/api" } sp-runtime = { version = "6.0.0", path = "../../../primitives/runtime" } sp-timestamp = { version = "4.0.0-dev", path = "../../../primitives/timestamp" } -# OTY added -sp-consensus-slots = { version = "0.10.0-dev", path = "../../../primitives/consensus/slots" } -sp-consensus-babe = { version = "0.10.0-dev", path = "../../../primitives/consensus/babe" } -sp-inherents = { version = "4.0.0-dev", path = "../../../primitives/inherents" } -log = "0.4.8" - # These dependencies are used for the node template's RPCs jsonrpc-core = "18.0.0" sc-rpc = { version = "4.0.0-dev", path = "../../../client/rpc" } @@ -63,10 +57,6 @@ node-template-runtime = { version = "4.0.0-dev", path = "../runtime" } # CLI-specific dependencies try-runtime-cli = { version = "0.10.0-dev", optional = true, path = "../../../utils/frame/try-runtime/cli" } -# OTY added -sp-keyring = { version = "6.0.0", path = "../../../primitives/keyring" } -node-runtime = { version = "3.0.0-dev", path = "../../node/runtime" } -node-cli = { version = "3.0.0-dev", path = "../../node/cli" } [build-dependencies] substrate-build-script-utils = { version = "3.0.0", path = "../../../utils/build-script-utils" } diff --git a/bin/node-template/node/src/service.rs b/bin/node-template/node/src/service.rs index e2a8cb4ed834b..7509315a158ec 100644 --- a/bin/node-template/node/src/service.rs +++ b/bin/node-template/node/src/service.rs @@ -31,7 +31,7 @@ impl sc_executor::NativeExecutionDispatch for ExecutorDispatch { } } -pub(crate) type FullClient = +pub type FullClient = sc_service::TFullClient>; type FullBackend = sc_service::TFullBackend; type FullSelectChain = sc_consensus::LongestChain; diff --git a/bin/node/cli/Cargo.toml b/bin/node/cli/Cargo.toml index 0b50d114ce1bb..24e069d21f694 100644 --- a/bin/node/cli/Cargo.toml +++ b/bin/node/cli/Cargo.toml @@ -96,12 +96,6 @@ frame-benchmarking-cli = { version = "4.0.0-dev", optional = true, path = "../.. node-inspect = { version = "0.9.0-dev", optional = true, path = "../inspect" } try-runtime-cli = { version = "0.10.0-dev", optional = true, path = "../../../utils/frame/try-runtime/cli" } -# OTY added -sp-consensus-slots = { version = "0.10.0-dev", path = "../../../primitives/consensus/slots" } -# OTY -sp-blockchain = { version = "4.0.0-dev", path = "../../../primitives/blockchain" } -chrono = "0.4.19" - [target.'cfg(any(target_arch="x86_64", target_arch="aarch64"))'.dependencies] node-executor = { version = "3.0.0-dev", path = "../executor", features = ["wasmtime"] } sc-cli = { version = "0.10.0-dev", optional = true, path = "../../../client/cli", features = ["wasmtime"] } diff --git a/bin/node/cli/src/command.rs b/bin/node/cli/src/command.rs index daae897d87c31..6842fda70cf23 100644 --- a/bin/node/cli/src/command.rs +++ b/bin/node/cli/src/command.rs @@ -16,11 +16,19 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . -use crate::{chain_spec, service, service::new_partial, Cli, Subcommand}; +use crate::{ + chain_spec, service, + service::{create_extrinsic, new_partial, FullClient}, + Cli, Subcommand, +}; use node_executor::ExecutorDispatch; -use node_runtime::RuntimeApi; +use node_primitives::Block; +use node_runtime::{RuntimeApi, SystemCall}; use sc_cli::{ChainSpec, Result, RuntimeVersion, SubstrateCli}; use sc_service::PartialComponents; +use sp_inherents::InherentDataProvider; +use sp_keyring::Sr25519Keyring; +use sp_runtime::OpaqueExtrinsic; use std::sync::Arc; @@ -70,23 +78,19 @@ impl SubstrateCli for Cli { &node_runtime::VERSION } } -struct ExtrinsicGen { + +/// Generates extrinsics for the `overhead` benchmark. +struct ExtrinsicBuilder { client: Arc, } -use crate::service::{create_extrinsic, FullClient}; -use node_primitives::Block; -use node_runtime::SystemCall; -use sp_keyring::Sr25519Keyring; -use sp_runtime::OpaqueExtrinsic; -impl frame_benchmarking_cli::overhead::cmd::ExtrinsicGenerator for ExtrinsicGen { + +impl frame_benchmarking_cli::overhead::cmd::ExtrinsicBuilder for ExtrinsicBuilder { fn remark(&self, nonce: u32) -> Option { - let src = Sr25519Keyring::Bob.pair(); + let acc = Sr25519Keyring::Bob.pair(); - // This `create_extrinsic` only exists for the node and has no - // equivalent in other runtimes. let extrinsic: OpaqueExtrinsic = create_extrinsic( self.client.as_ref(), - src.clone(), + acc.clone(), SystemCall::remark { remark: vec![] }, Some(nonce), ) @@ -95,7 +99,7 @@ impl frame_benchmarking_cli::overhead::cmd::ExtrinsicGenerator for ExtrinsicGen } } -use sp_inherents::InherentDataProvider; +/// Creates the inherent data needed to build a block. fn inherent_data() -> Result { let mut inherent_data = sp_inherents::InherentData::new(); let d = std::time::Duration::from_millis(0); @@ -141,10 +145,13 @@ pub fn run() -> Result<()> { config.role = sc_service::Role::Full; let PartialComponents { client, task_manager, .. } = new_partial(&config)?; - let ext_gen = ExtrinsicGen { client: client.clone() }; - + let ext_builder = ExtrinsicBuilder { client: client.clone() }; let inherents = inherent_data()?; - Ok((cmd.run(config, client.clone(), inherents, Arc::new(ext_gen)), task_manager)) + + Ok(( + cmd.run(config, client.clone(), inherents, Arc::new(ext_builder)), + task_manager, + )) }) }, Some(Subcommand::BenchmarkStorage(cmd)) => { diff --git a/block_weights.rs b/block_weights.rs deleted file mode 100644 index 1a6bba8cd091a..0000000000000 --- a/block_weights.rs +++ /dev/null @@ -1,79 +0,0 @@ -// This file is part of Substrate. - -// Copyright (C) 2022 Parity Technologies (UK) Ltd. -// SPDX-License-Identifier: Apache-2.0 - -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 4.0.0-dev -//! DATE: 2022-03-14 (Y/M/D) -//! -//! SHORT-NAME: `block`, LONG-NAME: `BlockExecution`, RUNTIME: `Development` -//! WARMUPS: `1`, REPEAT: `1` -//! WEIGHT-PATH: `.` -//! WEIGHT-METRIC: `Average`, WEIGHT-MUL: `1`, WEIGHT-ADD: `0` - -// Executed Command: -// ./target/release/substrate -// benchmark-overhead -// --dev -// --warmup -// 1 -// --repeat -// 1 - -use frame_support::{ - parameter_types, - weights::{constants::WEIGHT_PER_NANOS, Weight}, -}; - -parameter_types! { - /// Time to execute an empty block. - /// Calculated by multiplying the *Average* with `1` and adding `0`. - /// - /// Stats [ns]: - /// Min, Max: 5_172_175, 5_172_175 - /// Average: 5_172_175 - /// Median: 5_172_175 - /// StdDev: 0 - /// - /// Percentiles [ns]: - /// 99th: 5_172_175 - /// 95th: 5_172_175 - /// 75th: 5_172_175 - pub const BlockExecutionWeight: Weight = 5_172_175 * WEIGHT_PER_NANOS; -} - -#[cfg(test)] -mod test_weights { - use frame_support::weights::constants; - - /// Checks that the weight exists and is sane. - // NOTE: If this test fails but you are sure that the generated values are fine, - // you can delete it. - #[test] - fn sane() { - let w = super::constants::BlockExecutionWeight::get(); - - // At least 100 µs. - assert!( - w >= 100 * constants::WEIGHT_PER_MICROS, - "Weight should be at least 100 µs." - ); - // At most 50 ms. - assert!( - w <= 50 * constants::WEIGHT_PER_MILLIS, - "Weight should be at most 50 ms." - ); - } -} diff --git a/extrinsic_weights.rs b/extrinsic_weights.rs deleted file mode 100644 index 7ab24047252e4..0000000000000 --- a/extrinsic_weights.rs +++ /dev/null @@ -1,79 +0,0 @@ -// This file is part of Substrate. - -// Copyright (C) 2022 Parity Technologies (UK) Ltd. -// SPDX-License-Identifier: Apache-2.0 - -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 4.0.0-dev -//! DATE: 2022-03-14 (Y/M/D) -//! -//! SHORT-NAME: `extrinsic`, LONG-NAME: `Extrinsic`, RUNTIME: `Development` -//! WARMUPS: `1`, REPEAT: `1` -//! WEIGHT-PATH: `.` -//! WEIGHT-METRIC: `Average`, WEIGHT-MUL: `1`, WEIGHT-ADD: `0` - -// Executed Command: -// ./target/release/substrate -// benchmark-overhead -// --dev -// --warmup -// 1 -// --repeat -// 1 - -use frame_support::{ - parameter_types, - weights::{constants::WEIGHT_PER_NANOS, Weight}, -}; - -parameter_types! { - /// Time to execute a NO-OP extrinsic eg. `System::remark`. - /// Calculated by multiplying the *Average* with `1` and adding `0`. - /// - /// Stats [ns]: - /// Min, Max: 110_996, 110_996 - /// Average: 110_996 - /// Median: 110_996 - /// StdDev: 0 - /// - /// Percentiles [ns]: - /// 99th: 110_996 - /// 95th: 110_996 - /// 75th: 110_996 - pub const ExtrinsicWeight: Weight = 110_996 * WEIGHT_PER_NANOS; -} - -#[cfg(test)] -mod test_weights { - use frame_support::weights::constants; - - /// Checks that the weight exists and is sane. - // NOTE: If this test fails but you are sure that the generated values are fine, - // you can delete it. - #[test] - fn sane() { - let w = super::constants::ExtrinsicWeight::get(); - - // At least 10 µs. - assert!( - w >= 10 * constants::WEIGHT_PER_MICROS, - "Weight should be at least 10 µs." - ); - // At most 1 ms. - assert!( - w <= constants::WEIGHT_PER_MILLIS, - "Weight should be at most 1 ms." - ); - } -} diff --git a/frame/aura/src/lib.rs b/frame/aura/src/lib.rs index 83fb6cb392a27..657965c60a3f1 100644 --- a/frame/aura/src/lib.rs +++ b/frame/aura/src/lib.rs @@ -282,6 +282,9 @@ impl OnTimestampSet for Pallet { let timestamp_slot = moment / slot_duration; let timestamp_slot = Slot::from(timestamp_slot.saturated_into::()); - assert!(CurrentSlot::::get() == timestamp_slot, "sdaf`"); + assert!( + CurrentSlot::::get() == timestamp_slot, + "Timestamp slot must match `CurrentSlot`" + ); } } diff --git a/frame/babe/src/lib.rs b/frame/babe/src/lib.rs index 22d0fd62891a6..f673c8b43bee0 100644 --- a/frame/babe/src/lib.rs +++ b/frame/babe/src/lib.rs @@ -842,13 +842,10 @@ impl OnTimestampSet for Pallet { let timestamp_slot = moment / slot_duration; let timestamp_slot = Slot::from(timestamp_slot.saturated_into::()); - let c = CurrentSlot::::get(); - if c != timestamp_slot { - panic!( - "d: {:?}, moment: {:?}, want: {:?}, got: {:?}", - slot_duration, moment, c, timestamp_slot - ); - } + assert!( + CurrentSlot::::get() == timestamp_slot, + "Timestamp slot must match `CurrentSlot`" + ); } } diff --git a/utils/frame/benchmarking-cli/Cargo.toml b/utils/frame/benchmarking-cli/Cargo.toml index f8e265355261e..0c7cfd791efeb 100644 --- a/utils/frame/benchmarking-cli/Cargo.toml +++ b/utils/frame/benchmarking-cli/Cargo.toml @@ -15,8 +15,8 @@ targets = ["x86_64-unknown-linux-gnu"] [dependencies] 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" } -futures = "0.3.19" 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" } @@ -24,10 +24,10 @@ sc-client-db = { version = "0.10.0-dev", path = "../../../client/db" } sc-executor = { version = "0.10.0-dev", path = "../../../client/executor" } sp-api = { version = "4.0.0-dev", path = "../../../primitives/api" } -sp-core = { version = "6.0.0", path = "../../../primitives/core" } 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" } @@ -50,10 +50,7 @@ hash-db = "0.15.2" hex = "0.4.3" memory-db = "0.29.0" rand = { version = "0.8.4", features = ["small_rng"] } - -sp-inherents = { version = "4.0.0-dev", path = "../../../primitives/inherents" } -sp-consensus = { version = "0.10.0-dev", path = "../../../primitives/consensus/common" } -sc-consensus = { version = "0.10.0-dev", path = "../../../client/consensus/common" } +futures = "0.3.19" [features] default = ["db", "sc-client-db/runtime-benchmarks"] diff --git a/utils/frame/benchmarking-cli/src/lib.rs b/utils/frame/benchmarking-cli/src/lib.rs index cc29ae5c01982..e8290ee273d22 100644 --- a/utils/frame/benchmarking-cli/src/lib.rs +++ b/utils/frame/benchmarking-cli/src/lib.rs @@ -24,7 +24,7 @@ mod writer; use sc_cli::{ExecutionStrategy, WasmExecutionMethod, DEFAULT_WASM_EXECUTION_METHOD}; use std::{fmt::Debug, path::PathBuf}; -pub use overhead::{ExtrinsicGenerator, OverheadCmd}; +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 diff --git a/utils/frame/benchmarking-cli/src/overhead/bench.rs b/utils/frame/benchmarking-cli/src/overhead/bench.rs index 07a613e847ce7..f191d6f9d877e 100644 --- a/utils/frame/benchmarking-cli/src/overhead/bench.rs +++ b/utils/frame/benchmarking-cli/src/overhead/bench.rs @@ -16,10 +16,13 @@ // limitations under the License. use sc_block_builder::{BlockBuilderApi, BlockBuilderProvider}; -use sc_cli::Result; +use sc_cli::{Error, Result}; use sc_client_api::Backend as ClientBackend; use sp_api::{ApiExt, BlockId, Core, ProvideRuntimeApi}; -use sp_blockchain::{ApplyExtrinsicFailed::Validity, Error::ApplyExtrinsicFailed}; +use sp_blockchain::{ + ApplyExtrinsicFailed::Validity, + Error::{ApplyExtrinsicFailed, RuntimeApiError}, +}; use sp_runtime::{ traits::{Block as BlockT, Zero}, transaction_validity::{InvalidTransaction, TransactionValidityError}, @@ -31,9 +34,9 @@ use log::info; use serde::Serialize; use std::{marker::PhantomData, sync::Arc, time::Instant}; -use crate::{overhead::cmd::ExtrinsicGenerator, storage::record::Stats}; +use crate::{overhead::cmd::ExtrinsicBuilder, storage::record::Stats}; -/// Parameters to configure a block benchmark. +/// Parameters to configure a *overhead* benchmark. #[derive(Debug, Default, Serialize, Clone, PartialEq, Args)] pub struct BenchmarkParams { /// Rounds of warmups before measuring. @@ -43,13 +46,6 @@ pub struct BenchmarkParams { /// How many times the benchmark should be repeated. #[clap(long, default_value = "1000")] pub repeat: u32, - - /// Limit them number of extrinsics per block. - /// - /// Only useful for debugging since for a real benchmark you - /// want full blocks. - #[clap(long)] - pub max_ext_per_block: Option, } /// The results of multiple runs in ns. @@ -64,12 +60,12 @@ pub(crate) enum BenchmarkType { Block, } -/// Benchmarks the time it takes to execute an empty block or a NO-OP extrinsic. +/// Holds all objects needed to run the *overhead* benchmarks. pub(crate) struct Benchmark { client: Arc, params: BenchmarkParams, inherent_data: sp_inherents::InherentData, - ext_gen: Arc, + ext_builder: Arc, _p: PhantomData<(Block, BA)>, } @@ -85,9 +81,9 @@ where client: Arc, params: BenchmarkParams, inherent_data: sp_inherents::InherentData, - ext_gen: Arc, + ext_builder: Arc, ) -> Self { - Self { client, params, inherent_data, ext_gen, _p: PhantomData } + Self { client, params, inherent_data, ext_builder, _p: PhantomData } } /// Run the specified benchmark. @@ -101,24 +97,24 @@ where /// /// Returns the block and the number of extrinsics in the block. fn build_block(&self, bench_type: BenchmarkType) -> Result<(Block, u64)> { - let mut build = self.client.new_block(Default::default()).unwrap(); + let mut builder = self.client.new_block(Default::default())?; // Create and insert the inherents. - let inherents = build.create_inherents(self.inherent_data.clone())?; + let inherents = builder.create_inherents(self.inherent_data.clone())?; for inherent in inherents { - build.push(inherent)?; + builder.push(inherent)?; } // Return early if we just want an empty block. if bench_type == BenchmarkType::Block { - return Ok((build.build()?.block, 0)) + return Ok((builder.build()?.block, 0)) } // Put as many extrinsics into the block as possible and count them. - info!("Building block..."); + info!("Building block, this takes some time..."); let mut num_ext = 0; - for nonce in 0..self.max_ext_per_block() { - let ext = self.ext_gen.remark(nonce).expect("Need remark"); - match build.push(ext.clone()) { + for nonce in 0.. { + let ext = self.ext_builder.remark(nonce).ok_or("Could not generate extrinsic")?; + match builder.push(ext.clone()) { Ok(_) => {}, Err(ApplyExtrinsicFailed(Validity(TransactionValidityError::Invalid( InvalidTransaction::ExhaustsResources, @@ -129,7 +125,7 @@ where } assert!(num_ext != 0, "A Block must hold at least one extrinsic"); info!("Remarks per block: {}", num_ext); - let block = build.build()?.block; + let block = builder.build()?.block; Ok((block, num_ext)) } @@ -156,7 +152,7 @@ where self.client .runtime_api() .execute_block(&genesis, block.clone()) - .expect("Past blocks must execute"); + .map_err(|e| Error::Client(RuntimeApiError(e)))?; } info!("Executing block {} times", self.params.repeat); @@ -168,7 +164,7 @@ where self.client .runtime_api() .execute_block(&genesis, block) - .expect("Past blocks must execute"); + .map_err(|e| Error::Client(RuntimeApiError(e)))?; let elapsed = start.elapsed().as_nanos(); if bench_type == BenchmarkType::Extrinsic { @@ -181,12 +177,6 @@ where Ok(record) } - - /// Returns the maximum number of extrinsics that should be put in - /// a block or `u32::MAX` if no option was passed. - fn max_ext_per_block(&self) -> u32 { - self.params.max_ext_per_block.unwrap_or(u32::MAX) - } } impl BenchmarkType { @@ -201,7 +191,7 @@ impl BenchmarkType { /// Long name of the benchmark type. pub(crate) fn long_name(&self) -> &'static str { match self { - Self::Extrinsic => "Extrinsic", + Self::Extrinsic => "ExtrinsicBase", Self::Block => "BlockExecution", } } diff --git a/utils/frame/benchmarking-cli/src/overhead/cmd.rs b/utils/frame/benchmarking-cli/src/overhead/cmd.rs index 4892623ab6198..075bde97631f5 100644 --- a/utils/frame/benchmarking-cli/src/overhead/cmd.rs +++ b/utils/frame/benchmarking-cli/src/overhead/cmd.rs @@ -59,11 +59,11 @@ pub struct OverheadParams { pub bench: BenchmarkParams, } -/// Used by the benchmark to generate signed extrinsics. +/// Used by the benchmark to build signed extrinsics. /// This must be implemented per-runtime since the signed extensions /// depend on the runtime. -pub trait ExtrinsicGenerator { - /// Generates a `System::remark` extrinsic. +pub trait ExtrinsicBuilder { + /// Build a `System::remark` extrinsic. fn remark(&self, nonce: u32) -> Option; } @@ -77,7 +77,7 @@ impl OverheadCmd { cfg: Configuration, client: Arc, inherent_data: sp_inherents::InherentData, - ext_gen: Arc, + ext_builder: Arc, ) -> Result<()> where Block: BlockT, @@ -85,7 +85,7 @@ impl OverheadCmd { C: BlockBuilderProvider + ProvideRuntimeApi, C::Api: ApiExt + BlockBuilderApi, { - let bench = Benchmark::new(client, self.params.bench.clone(), inherent_data, ext_gen); + let bench = Benchmark::new(client, self.params.bench.clone(), inherent_data, ext_builder); // per-block execution overhead { diff --git a/utils/frame/benchmarking-cli/src/overhead/mod.rs b/utils/frame/benchmarking-cli/src/overhead/mod.rs index 9bc5582eb9103..abdeac22b7898 100644 --- a/utils/frame/benchmarking-cli/src/overhead/mod.rs +++ b/utils/frame/benchmarking-cli/src/overhead/mod.rs @@ -19,4 +19,4 @@ mod bench; pub mod cmd; mod template; -pub use cmd::{ExtrinsicGenerator, OverheadCmd}; +pub use cmd::{ExtrinsicBuilder, OverheadCmd}; diff --git a/utils/frame/benchmarking-cli/src/overhead/template.rs b/utils/frame/benchmarking-cli/src/overhead/template.rs index 5614512f62769..c855662035d63 100644 --- a/utils/frame/benchmarking-cli/src/overhead/template.rs +++ b/utils/frame/benchmarking-cli/src/overhead/template.rs @@ -36,7 +36,7 @@ static TEMPLATE: &str = include_str!("./weights.hbs"); pub(crate) struct TemplateData { /// Short name of the benchmark. Can be "block" or "extrinsic". long_name: String, - /// Long name of the benchmark. Can be "BlockExecution" "ExtrinsicBase". + /// Long name of the benchmark. Can be "BlockExecution" or "ExtrinsicBase". short_name: String, /// Name of the runtime. Taken from the chain spec. runtime_name: String, @@ -77,7 +77,7 @@ impl TemplateData { }) } - /// Filles out the `weights.hbs` HBS template with its own data. + /// Fill out the `weights.hbs` HBS template with its own data. /// Writes the result to `path` which can be a directory or a file. pub fn write(&self, path: &str) -> Result<()> { let mut handlebars = Handlebars::new(); @@ -94,7 +94,7 @@ impl TemplateData { .map_err(|e| format!("HBS template write: {:?}", e).into()) } - /// Builds a path for the weight file. + /// Build a path for the weight file. fn build_path(&self, weight_out: &str) -> PathBuf { let mut path = PathBuf::from(weight_out); if path.is_dir() { diff --git a/utils/frame/benchmarking-cli/src/storage/write.rs b/utils/frame/benchmarking-cli/src/storage/write.rs index 49ebbcc2349a9..eb9ba11f30696 100644 --- a/utils/frame/benchmarking-cli/src/storage/write.rs +++ b/utils/frame/benchmarking-cli/src/storage/write.rs @@ -51,7 +51,6 @@ impl StorageCmd { let mut record = BenchRecord::default(); let supports_rc = db.supports_ref_counting(); - // TODO use HeaderBackend.info() let block = BlockId::Number(client.usage_info().chain.best_number); let header = client.header(block)?.ok_or("Header not found")?; let original_root = *header.state_root(); From 9f1659b54bf1cd77c691fa5362fa5c04badcb1bb Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Mon, 14 Mar 2022 23:15:30 +0100 Subject: [PATCH 18/29] Cleanup Signed-off-by: Oliver Tale-Yazdi --- Cargo.lock | 1 - bin/node-template/node/src/service.rs | 2 +- bin/node/cli/src/command.rs | 22 +++++++++---------- utils/frame/benchmarking-cli/Cargo.toml | 1 - .../benchmarking-cli/src/overhead/bench.rs | 2 ++ .../benchmarking-cli/src/overhead/cmd.rs | 7 ++++-- .../benchmarking-cli/src/overhead/template.rs | 3 +++ 7 files changed, 22 insertions(+), 16 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0ea5b8d4f094b..806b683f81743 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2129,7 +2129,6 @@ dependencies = [ "clap 3.1.6", "frame-benchmarking", "frame-support", - "futures 0.3.19", "handlebars", "hash-db", "hex", diff --git a/bin/node-template/node/src/service.rs b/bin/node-template/node/src/service.rs index 7509315a158ec..fc7dc9b978df3 100644 --- a/bin/node-template/node/src/service.rs +++ b/bin/node-template/node/src/service.rs @@ -31,7 +31,7 @@ impl sc_executor::NativeExecutionDispatch for ExecutorDispatch { } } -pub type FullClient = +type FullClient = sc_service::TFullClient>; type FullBackend = sc_service::TFullBackend; type FullSelectChain = sc_consensus::LongestChain; diff --git a/bin/node/cli/src/command.rs b/bin/node/cli/src/command.rs index 6842fda70cf23..694bf84d7be9b 100644 --- a/bin/node/cli/src/command.rs +++ b/bin/node/cli/src/command.rs @@ -26,11 +26,11 @@ use node_primitives::Block; use node_runtime::{RuntimeApi, SystemCall}; use sc_cli::{ChainSpec, Result, RuntimeVersion, SubstrateCli}; use sc_service::PartialComponents; -use sp_inherents::InherentDataProvider; +use sp_inherents::{InherentData, InherentDataProvider}; use sp_keyring::Sr25519Keyring; use sp_runtime::OpaqueExtrinsic; -use std::sync::Arc; +use std::{sync::Arc, time::Duration}; impl SubstrateCli for Cli { fn impl_name() -> String { @@ -80,29 +80,30 @@ impl SubstrateCli for Cli { } /// Generates extrinsics for the `overhead` benchmark. +/// TODO I will move this into a different file once we agreed on the structure. struct ExtrinsicBuilder { client: Arc, } -impl frame_benchmarking_cli::overhead::cmd::ExtrinsicBuilder for ExtrinsicBuilder { +impl frame_benchmarking_cli::ExtrinsicBuilder for ExtrinsicBuilder { fn remark(&self, nonce: u32) -> Option { let acc = Sr25519Keyring::Bob.pair(); - let extrinsic: OpaqueExtrinsic = create_extrinsic( self.client.as_ref(), - acc.clone(), + acc, SystemCall::remark { remark: vec![] }, Some(nonce), ) .into(); + Some(extrinsic) } } -/// Creates the inherent data needed to build a block. -fn inherent_data() -> Result { - let mut inherent_data = sp_inherents::InherentData::new(); - let d = std::time::Duration::from_millis(0); +/// Create the inherent data needed to build a block. +fn inherent_data() -> Result { + let mut inherent_data = InherentData::new(); + let d = Duration::from_millis(0); let timestamp = sp_timestamp::InherentDataProvider::new(d.into()); timestamp @@ -146,10 +147,9 @@ pub fn run() -> Result<()> { let PartialComponents { client, task_manager, .. } = new_partial(&config)?; let ext_builder = ExtrinsicBuilder { client: client.clone() }; - let inherents = inherent_data()?; Ok(( - cmd.run(config, client.clone(), inherents, Arc::new(ext_builder)), + cmd.run(config, client.clone(), inherent_data()?, Arc::new(ext_builder)), task_manager, )) }) diff --git a/utils/frame/benchmarking-cli/Cargo.toml b/utils/frame/benchmarking-cli/Cargo.toml index 0c7cfd791efeb..5575bb833ca77 100644 --- a/utils/frame/benchmarking-cli/Cargo.toml +++ b/utils/frame/benchmarking-cli/Cargo.toml @@ -50,7 +50,6 @@ hash-db = "0.15.2" hex = "0.4.3" memory-db = "0.29.0" rand = { version = "0.8.4", features = ["small_rng"] } -futures = "0.3.19" [features] default = ["db", "sc-client-db/runtime-benchmarks"] diff --git a/utils/frame/benchmarking-cli/src/overhead/bench.rs b/utils/frame/benchmarking-cli/src/overhead/bench.rs index f191d6f9d877e..bebc039631307 100644 --- a/utils/frame/benchmarking-cli/src/overhead/bench.rs +++ b/utils/frame/benchmarking-cli/src/overhead/bench.rs @@ -15,6 +15,8 @@ // See the License for the specific language governing permissions and // limitations under the License. +//! Contains the core benchmarking logic. + use sc_block_builder::{BlockBuilderApi, BlockBuilderProvider}; use sc_cli::{Error, Result}; use sc_client_api::Backend as ClientBackend; diff --git a/utils/frame/benchmarking-cli/src/overhead/cmd.rs b/utils/frame/benchmarking-cli/src/overhead/cmd.rs index 075bde97631f5..708da789a6853 100644 --- a/utils/frame/benchmarking-cli/src/overhead/cmd.rs +++ b/utils/frame/benchmarking-cli/src/overhead/cmd.rs @@ -15,6 +15,9 @@ // See the License for the specific language governing permissions and // limitations under the License. +//! Contains the [`OverheadCmd`] which the CLI uses as entry point +//! for the *overhead* benchmarks. + use sc_block_builder::{BlockBuilderApi, BlockBuilderProvider}; use sc_cli::{CliConfiguration, Result, SharedParams}; use sc_client_api::Backend as ClientBackend; @@ -60,8 +63,8 @@ pub struct OverheadParams { } /// Used by the benchmark to build signed extrinsics. -/// This must be implemented per-runtime since the signed extensions -/// depend on the runtime. +/// +/// This only has to work for the first block who's parent block is the genesis block. pub trait ExtrinsicBuilder { /// Build a `System::remark` extrinsic. fn remark(&self, nonce: u32) -> Option; diff --git a/utils/frame/benchmarking-cli/src/overhead/template.rs b/utils/frame/benchmarking-cli/src/overhead/template.rs index c855662035d63..8c47f1b4a53ea 100644 --- a/utils/frame/benchmarking-cli/src/overhead/template.rs +++ b/utils/frame/benchmarking-cli/src/overhead/template.rs @@ -15,6 +15,9 @@ // See the License for the specific language governing permissions and // limitations under the License. +//! Converts the benchmark results into [`TemplateData`] which gets +//! written into the `weights.hbs` template. + use sc_cli::Result; use sc_service::Configuration; From e6acb0c7d20d30a6bd1cf19935c98729ff038b4d Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Mon, 14 Mar 2022 23:30:44 +0100 Subject: [PATCH 19/29] Beauty fixes Signed-off-by: Oliver Tale-Yazdi --- bin/node/cli/src/command.rs | 5 +---- utils/frame/benchmarking-cli/src/overhead/bench.rs | 6 +----- utils/frame/benchmarking-cli/src/overhead/cmd.rs | 11 ++++++----- .../frame/benchmarking-cli/src/overhead/template.rs | 6 +++--- .../benchmarking-cli/src/post_processing/mod.rs | 12 +++++++----- 5 files changed, 18 insertions(+), 22 deletions(-) diff --git a/bin/node/cli/src/command.rs b/bin/node/cli/src/command.rs index 694bf84d7be9b..56131f8161b8c 100644 --- a/bin/node/cli/src/command.rs +++ b/bin/node/cli/src/command.rs @@ -148,10 +148,7 @@ pub fn run() -> Result<()> { let PartialComponents { client, task_manager, .. } = new_partial(&config)?; let ext_builder = ExtrinsicBuilder { client: client.clone() }; - Ok(( - cmd.run(config, client.clone(), inherent_data()?, Arc::new(ext_builder)), - task_manager, - )) + Ok((cmd.run(config, client, inherent_data()?, Arc::new(ext_builder)), task_manager)) }) }, Some(Subcommand::BenchmarkStorage(cmd)) => { diff --git a/utils/frame/benchmarking-cli/src/overhead/bench.rs b/utils/frame/benchmarking-cli/src/overhead/bench.rs index bebc039631307..10572b8614d2c 100644 --- a/utils/frame/benchmarking-cli/src/overhead/bench.rs +++ b/utils/frame/benchmarking-cli/src/overhead/bench.rs @@ -115,7 +115,7 @@ where info!("Building block, this takes some time..."); let mut num_ext = 0; for nonce in 0.. { - let ext = self.ext_builder.remark(nonce).ok_or("Could not generate extrinsic")?; + let ext = self.ext_builder.remark(nonce).ok_or("Could not build extrinsic")?; match builder.push(ext.clone()) { Ok(_) => {}, Err(ApplyExtrinsicFailed(Validity(TransactionValidityError::Invalid( @@ -133,10 +133,6 @@ where } /// Measures the time that it take to execute a block or an extrinsic. - /// `per_ext` specifies if the result should be divided - /// by the number of extrinsics in the block. - /// This is useful for the case that you want to know - /// how long it takes to execute one extrinsic. fn measure_block( &self, block: &Block, diff --git a/utils/frame/benchmarking-cli/src/overhead/cmd.rs b/utils/frame/benchmarking-cli/src/overhead/cmd.rs index 708da789a6853..6dc3a9d76d665 100644 --- a/utils/frame/benchmarking-cli/src/overhead/cmd.rs +++ b/utils/frame/benchmarking-cli/src/overhead/cmd.rs @@ -15,8 +15,8 @@ // See the License for the specific language governing permissions and // limitations under the License. -//! Contains the [`OverheadCmd`] which the CLI uses as entry point -//! for the *overhead* benchmarks. +//! Contains the [`OverheadCmd`] as entry point for the CLI to execute +//! the *overhead* benchmarks. use sc_block_builder::{BlockBuilderApi, BlockBuilderProvider}; use sc_cli::{CliConfiguration, Result, SharedParams}; @@ -50,7 +50,7 @@ pub struct OverheadCmd { pub params: OverheadParams, } -/// Configures the benchmark and the post processing and weight generation. +/// Configures the benchmark, the post-processing and weight generation. #[derive(Debug, Default, Serialize, Clone, PartialEq, Args)] pub struct OverheadParams { #[allow(missing_docs)] @@ -64,7 +64,8 @@ pub struct OverheadParams { /// Used by the benchmark to build signed extrinsics. /// -/// This only has to work for the first block who's parent block is the genesis block. +/// The built extrinsics only need to be valid in the first block +/// who's parent block is the genesis block. pub trait ExtrinsicBuilder { /// Build a `System::remark` extrinsic. fn remark(&self, nonce: u32) -> Option; @@ -74,7 +75,7 @@ impl OverheadCmd { /// Measures the per-block and per-extrinsic execution overhead. /// /// Writes the results to console and into two instances of the - /// `weights.hbs` template. + /// `weights.hbs` template, one for each benchmark. pub async fn run( &self, cfg: Configuration, diff --git a/utils/frame/benchmarking-cli/src/overhead/template.rs b/utils/frame/benchmarking-cli/src/overhead/template.rs index 8c47f1b4a53ea..1eb3d94082e6f 100644 --- a/utils/frame/benchmarking-cli/src/overhead/template.rs +++ b/utils/frame/benchmarking-cli/src/overhead/template.rs @@ -15,8 +15,8 @@ // See the License for the specific language governing permissions and // limitations under the License. -//! Converts the benchmark results into [`TemplateData`] which gets -//! written into the `weights.hbs` template. +//! Converts a benchmark result into [`TemplateData`] and writes +//! it into the `weights.hbs` template. use sc_cli::Result; use sc_service::Configuration; @@ -58,7 +58,7 @@ pub(crate) struct TemplateData { } impl TemplateData { - /// Returns a new [`Self`] from the given configuration. + /// Returns a new [`Self`] from the given params. pub(crate) fn new( t: BenchmarkType, cfg: &Configuration, diff --git a/utils/frame/benchmarking-cli/src/post_processing/mod.rs b/utils/frame/benchmarking-cli/src/post_processing/mod.rs index 363cd8a9f665c..134c50729a1c3 100644 --- a/utils/frame/benchmarking-cli/src/post_processing/mod.rs +++ b/utils/frame/benchmarking-cli/src/post_processing/mod.rs @@ -15,6 +15,8 @@ // See the License for the specific language governing permissions and // limitations under the License. +//! Calculates a weight from the statistics of a benchmark result. + use sc_cli::Result; use clap::Args; @@ -22,11 +24,11 @@ use serde::Serialize; use crate::storage::record::{StatSelect, Stats}; -/// Parameters to configure the post processing of the weights. +/// Configures the weight generation. #[derive(Debug, Default, Serialize, Clone, PartialEq, Args)] pub struct WeightParams { /// Path to write the *weight* file to. Can be a file or directory. - /// For substrate this should be `frame/support/src/weights`. + /// For Substrate this should be `frame/support/src/weights`. #[clap(long, default_value = ".")] pub weight_path: String, @@ -35,18 +37,18 @@ pub struct WeightParams { pub weight_metric: StatSelect, /// Multiply the resulting weight with the given factor. Must be positive. - /// Is calculated before `weight_add`. + /// Is applied before `weight_add`. #[clap(long = "mul", default_value = "1")] pub weight_mul: f64, /// Add the given offset to the resulting weight. - /// Is calculated after `weight_mul`. + /// Is applied after `weight_mul`. #[clap(long = "add", default_value = "0")] pub weight_add: u64, } /// Calculates the final weight by multiplying the selected metric with -/// `mul` and adding `add`. +/// `weight_mul` and adding `weight_add`. /// Does not use safe casts and can overflow. impl WeightParams { pub(crate) fn calc_weight(&self, stat: &Stats) -> Result { From 0ed83039a42d3a705844c15d104a72aa00589d29 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Tue, 15 Mar 2022 12:40:33 +0100 Subject: [PATCH 20/29] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bastian Köcher --- utils/frame/benchmarking-cli/src/overhead/bench.rs | 12 +++++++----- .../benchmarking-cli/src/post_processing/mod.rs | 3 +++ 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/utils/frame/benchmarking-cli/src/overhead/bench.rs b/utils/frame/benchmarking-cli/src/overhead/bench.rs index 10572b8614d2c..3f0a0c1446cbe 100644 --- a/utils/frame/benchmarking-cli/src/overhead/bench.rs +++ b/utils/frame/benchmarking-cli/src/overhead/bench.rs @@ -54,7 +54,7 @@ pub struct BenchmarkParams { pub(crate) type BenchRecord = Vec; /// Type of a benchmark. -#[derive(Serialize, Clone, PartialEq)] +#[derive(Serialize, Clone, PartialEq, Copy)] pub(crate) enum BenchmarkType { /// Measure the per-extrinsic execution overhead. Extrinsic, @@ -90,7 +90,7 @@ where /// Run the specified benchmark. pub fn bench(&self, bench_type: BenchmarkType) -> Result { - let (block, num_ext) = self.build_block(bench_type.clone())?; + let (block, num_ext) = self.build_block(bench_type)?; let record = self.measure_block(&block, num_ext, bench_type)?; Stats::new(&record) } @@ -117,7 +117,8 @@ where for nonce in 0.. { let ext = self.ext_builder.remark(nonce).ok_or("Could not build extrinsic")?; match builder.push(ext.clone()) { - Ok(_) => {}, + Ok(Ok(())) => {}, + Ok(r) => panic!("Failed to apply extrinsic: {:?}", r), Err(ApplyExtrinsicFailed(Validity(TransactionValidityError::Invalid( InvalidTransaction::ExhaustsResources, )))) => break, @@ -158,9 +159,10 @@ where // Execute a block multiple times and record each execution time. for _ in 0..self.params.repeat { let block = block.clone(); + let runtime_api = self.client.runtime_api(); let start = Instant::now(); - self.client - .runtime_api() + + runtime_api .execute_block(&genesis, block) .map_err(|e| Error::Client(RuntimeApiError(e)))?; diff --git a/utils/frame/benchmarking-cli/src/post_processing/mod.rs b/utils/frame/benchmarking-cli/src/post_processing/mod.rs index 134c50729a1c3..3f31070b4f0a6 100644 --- a/utils/frame/benchmarking-cli/src/post_processing/mod.rs +++ b/utils/frame/benchmarking-cli/src/post_processing/mod.rs @@ -28,6 +28,7 @@ use crate::storage::record::{StatSelect, Stats}; #[derive(Debug, Default, Serialize, Clone, PartialEq, Args)] pub struct WeightParams { /// Path to write the *weight* file to. Can be a file or directory. + /// /// For Substrate this should be `frame/support/src/weights`. #[clap(long, default_value = ".")] pub weight_path: String, @@ -37,11 +38,13 @@ pub struct WeightParams { pub weight_metric: StatSelect, /// Multiply the resulting weight with the given factor. Must be positive. + /// /// Is applied before `weight_add`. #[clap(long = "mul", default_value = "1")] pub weight_mul: f64, /// Add the given offset to the resulting weight. + /// /// Is applied after `weight_mul`. #[clap(long = "add", default_value = "0")] pub weight_add: u64, From a440311bb471c920efc045a50a7d103b82462599 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Tue, 15 Mar 2022 12:43:31 +0100 Subject: [PATCH 21/29] Update utils/frame/benchmarking-cli/src/overhead/template.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bastian Köcher --- utils/frame/benchmarking-cli/src/overhead/template.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/utils/frame/benchmarking-cli/src/overhead/template.rs b/utils/frame/benchmarking-cli/src/overhead/template.rs index 1eb3d94082e6f..d736e6a386606 100644 --- a/utils/frame/benchmarking-cli/src/overhead/template.rs +++ b/utils/frame/benchmarking-cli/src/overhead/template.rs @@ -101,8 +101,7 @@ impl TemplateData { fn build_path(&self, weight_out: &str) -> PathBuf { let mut path = PathBuf::from(weight_out); if path.is_dir() { - path.push(format!("{}_weights", self.short_name)); - path.set_extension("rs"); + path.push(format!("{}_weights.rs", self.short_name)); } path } From 0878563e3de2e1e160d46fd95c322f54052f4dfa Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Tue, 15 Mar 2022 12:04:25 +0000 Subject: [PATCH 22/29] Review fixes Signed-off-by: Oliver Tale-Yazdi --- bin/node/cli/benches/block_production.rs | 2 +- .../benchmarking-cli/src/overhead/bench.rs | 22 ++++++++++--------- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/bin/node/cli/benches/block_production.rs b/bin/node/cli/benches/block_production.rs index 77b51fa28dd1a..7cbcc828ea1e5 100644 --- a/bin/node/cli/benches/block_production.rs +++ b/bin/node/cli/benches/block_production.rs @@ -167,7 +167,7 @@ fn prepare_benchmark(client: &FullClient) -> (usize, Vec) { Err(ApplyExtrinsicFailed(Validity(TransactionValidityError::Invalid( InvalidTransaction::ExhaustsResources, )))) => break, - Err(error) => panic!("{}", error), + Err(e) => e, } extrinsics.push(extrinsic); diff --git a/utils/frame/benchmarking-cli/src/overhead/bench.rs b/utils/frame/benchmarking-cli/src/overhead/bench.rs index 3f0a0c1446cbe..b8a9a36233fc4 100644 --- a/utils/frame/benchmarking-cli/src/overhead/bench.rs +++ b/utils/frame/benchmarking-cli/src/overhead/bench.rs @@ -97,7 +97,8 @@ where /// Builds a block for the given benchmark type. /// - /// Returns the block and the number of extrinsics in the block. + /// Returns the block and the number of extrinsics in the block + /// that are not inherents. fn build_block(&self, bench_type: BenchmarkType) -> Result<(Block, u64)> { let mut builder = self.client.new_block(Default::default())?; // Create and insert the inherents. @@ -106,7 +107,7 @@ where builder.push(inherent)?; } - // Return early if we just want an empty block. + // Return early if we just want a block with inherents and no additional extrinsics. if bench_type == BenchmarkType::Block { return Ok((builder.build()?.block, 0)) } @@ -117,17 +118,18 @@ where for nonce in 0.. { let ext = self.ext_builder.remark(nonce).ok_or("Could not build extrinsic")?; match builder.push(ext.clone()) { - Ok(Ok(())) => {}, - Ok(r) => panic!("Failed to apply extrinsic: {:?}", r), + Ok(()) => {}, Err(ApplyExtrinsicFailed(Validity(TransactionValidityError::Invalid( InvalidTransaction::ExhaustsResources, - )))) => break, - Err(error) => panic!("{}", error), + )))) => break, // Block is full + Err(e) => return Err(Error::Client(e)), } num_ext += 1; } - assert!(num_ext != 0, "A Block must hold at least one extrinsic"); - info!("Remarks per block: {}", num_ext); + if num_ext == 0 { + return Err("A Block must hold at least one extrinsic".into()) + } + info!("Extrinsics per block: {}", num_ext); let block = builder.build()?.block; Ok((block, num_ext)) @@ -161,7 +163,7 @@ where let block = block.clone(); let runtime_api = self.client.runtime_api(); let start = Instant::now(); - + runtime_api .execute_block(&genesis, block) .map_err(|e| Error::Client(RuntimeApiError(e)))?; @@ -169,7 +171,7 @@ where let elapsed = start.elapsed().as_nanos(); if bench_type == BenchmarkType::Extrinsic { // Checked for non-zero div above. - record.push(elapsed as u64 / num_ext); + record.push((elapsed as f64 / num_ext as f64).ceil() as u64); } else { record.push(elapsed as u64); } From 60764281ea2423c5c59375d68f9064d7c51f30cc Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Tue, 15 Mar 2022 13:41:34 +0100 Subject: [PATCH 23/29] Doc + Template fixes Signed-off-by: Oliver Tale-Yazdi --- utils/frame/benchmarking-cli/src/overhead/bench.rs | 4 ++-- utils/frame/benchmarking-cli/src/overhead/weights.hbs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/utils/frame/benchmarking-cli/src/overhead/bench.rs b/utils/frame/benchmarking-cli/src/overhead/bench.rs index b8a9a36233fc4..48fce7cd45cb2 100644 --- a/utils/frame/benchmarking-cli/src/overhead/bench.rs +++ b/utils/frame/benchmarking-cli/src/overhead/bench.rs @@ -38,7 +38,7 @@ use std::{marker::PhantomData, sync::Arc, time::Instant}; use crate::{overhead::cmd::ExtrinsicBuilder, storage::record::Stats}; -/// Parameters to configure a *overhead* benchmark. +/// Parameters to configure an *overhead* benchmark. #[derive(Debug, Default, Serialize, Clone, PartialEq, Args)] pub struct BenchmarkParams { /// Rounds of warmups before measuring. @@ -50,7 +50,7 @@ pub struct BenchmarkParams { pub repeat: u32, } -/// The results of multiple runs in ns. +/// The results of multiple runs in nano seconds. pub(crate) type BenchRecord = Vec; /// Type of a benchmark. diff --git a/utils/frame/benchmarking-cli/src/overhead/weights.hbs b/utils/frame/benchmarking-cli/src/overhead/weights.hbs index 9e02b023dfb0f..0f6b7f3e9119f 100644 --- a/utils/frame/benchmarking-cli/src/overhead/weights.hbs +++ b/utils/frame/benchmarking-cli/src/overhead/weights.hbs @@ -63,7 +63,7 @@ mod test_weights { // you can delete it. #[test] fn sane() { - let w = super::constants::{{long_name}}Weight::get(); + let w = super::{{long_name}}Weight::get(); {{#if (eq short_name "block")}} // At least 100 µs. From 59f478e42236043fa86393b9aa14d92f0c8bce65 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Tue, 15 Mar 2022 13:46:07 +0100 Subject: [PATCH 24/29] Review fixes Signed-off-by: Oliver Tale-Yazdi --- bin/node/cli/benches/block_production.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/node/cli/benches/block_production.rs b/bin/node/cli/benches/block_production.rs index 7cbcc828ea1e5..77b51fa28dd1a 100644 --- a/bin/node/cli/benches/block_production.rs +++ b/bin/node/cli/benches/block_production.rs @@ -167,7 +167,7 @@ fn prepare_benchmark(client: &FullClient) -> (usize, Vec) { Err(ApplyExtrinsicFailed(Validity(TransactionValidityError::Invalid( InvalidTransaction::ExhaustsResources, )))) => break, - Err(e) => e, + Err(error) => panic!("{}", error), } extrinsics.push(extrinsic); From 533667c369af7445b0c2a20bd2e2edd8634e7809 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Tue, 15 Mar 2022 15:58:04 +0100 Subject: [PATCH 25/29] Comment fix Signed-off-by: Oliver Tale-Yazdi --- utils/frame/benchmarking-cli/src/overhead/template.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utils/frame/benchmarking-cli/src/overhead/template.rs b/utils/frame/benchmarking-cli/src/overhead/template.rs index d736e6a386606..726be23f11743 100644 --- a/utils/frame/benchmarking-cli/src/overhead/template.rs +++ b/utils/frame/benchmarking-cli/src/overhead/template.rs @@ -49,7 +49,7 @@ pub(crate) struct TemplateData { date: String, /// Command line arguments that were passed to the CLI. args: Vec, - /// Params of the executed command TODO. + /// Params of the executed command. params: OverheadParams, /// Stats about the benchmark result. stats: Stats, From a2c788adcf36aa7612a0971641e9af60542a2bac Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Tue, 15 Mar 2022 21:59:54 +0000 Subject: [PATCH 26/29] Add test Signed-off-by: Oliver Tale-Yazdi --- .../cli/tests/benchmark_overhead_works.rs | 46 +++++++++++++++++++ .../benchmarking-cli/src/overhead/bench.rs | 12 ++++- .../benchmarking-cli/src/overhead/template.rs | 11 +++-- .../src/post_processing/mod.rs | 2 +- 4 files changed, 64 insertions(+), 7 deletions(-) create mode 100644 bin/node/cli/tests/benchmark_overhead_works.rs diff --git a/bin/node/cli/tests/benchmark_overhead_works.rs b/bin/node/cli/tests/benchmark_overhead_works.rs new file mode 100644 index 0000000000000..550221ee2f70f --- /dev/null +++ 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 . + +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/src/overhead/bench.rs b/utils/frame/benchmarking-cli/src/overhead/bench.rs index 48fce7cd45cb2..26062ae9435ef 100644 --- a/utils/frame/benchmarking-cli/src/overhead/bench.rs +++ b/utils/frame/benchmarking-cli/src/overhead/bench.rs @@ -48,6 +48,12 @@ pub struct BenchmarkParams { /// How many times the benchmark should be repeated. #[clap(long, default_value = "1000")] pub repeat: u32, + + /// Maximal number of extrinsics that should be put into a block. + /// + /// Only useful for debugging. + #[clap(long)] + pub max_ext_per_block: Option, } /// The results of multiple runs in nano seconds. @@ -115,7 +121,7 @@ where // Put as many extrinsics into the block as possible and count them. info!("Building block, this takes some time..."); let mut num_ext = 0; - for nonce in 0.. { + for nonce in 0..self.max_ext_per_block() { let ext = self.ext_builder.remark(nonce).ok_or("Could not build extrinsic")?; match builder.push(ext.clone()) { Ok(()) => {}, @@ -179,6 +185,10 @@ where Ok(record) } + + fn max_ext_per_block(&self) -> u32 { + self.params.max_ext_per_block.unwrap_or(u32::MAX) + } } impl BenchmarkType { diff --git a/utils/frame/benchmarking-cli/src/overhead/template.rs b/utils/frame/benchmarking-cli/src/overhead/template.rs index 726be23f11743..0b9ca404cc725 100644 --- a/utils/frame/benchmarking-cli/src/overhead/template.rs +++ b/utils/frame/benchmarking-cli/src/overhead/template.rs @@ -89,7 +89,7 @@ impl TemplateData { // Don't HTML escape any characters. handlebars.register_escape_fn(|s| -> String { s.to_string() }); - let out_path = self.build_path(path); + let out_path = self.build_path(path)?; let mut fd = fs::File::create(&out_path)?; info!("Writing weights to {:?}", fs::canonicalize(&out_path)?); handlebars @@ -98,11 +98,12 @@ impl TemplateData { } /// Build a path for the weight file. - fn build_path(&self, weight_out: &str) -> PathBuf { + fn build_path(&self, weight_out: &str) -> Result { let mut path = PathBuf::from(weight_out); - if path.is_dir() { - path.push(format!("{}_weights.rs", self.short_name)); + if !path.is_dir() { + return Err("Need directory as --weight-path".into()) } - path + path.push(format!("{}_weights.rs", self.short_name)); + Ok(path) } } diff --git a/utils/frame/benchmarking-cli/src/post_processing/mod.rs b/utils/frame/benchmarking-cli/src/post_processing/mod.rs index 3f31070b4f0a6..e15f4004f94d4 100644 --- a/utils/frame/benchmarking-cli/src/post_processing/mod.rs +++ b/utils/frame/benchmarking-cli/src/post_processing/mod.rs @@ -27,7 +27,7 @@ use crate::storage::record::{StatSelect, Stats}; /// Configures the weight generation. #[derive(Debug, Default, Serialize, Clone, PartialEq, Args)] pub struct WeightParams { - /// Path to write the *weight* file to. Can be a file or directory. + /// Directory to write the *weight* files to. /// /// For Substrate this should be `frame/support/src/weights`. #[clap(long, default_value = ".")] From 7c6b2c01925ec9949ef1713336a654e051ca53c6 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Tue, 15 Mar 2022 23:39:30 +0100 Subject: [PATCH 27/29] Pust merge fixup Signed-off-by: Oliver Tale-Yazdi --- utils/frame/benchmarking-cli/src/overhead/template.rs | 7 ++++--- utils/frame/benchmarking-cli/src/post_processing/mod.rs | 1 + 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/utils/frame/benchmarking-cli/src/overhead/template.rs b/utils/frame/benchmarking-cli/src/overhead/template.rs index 0b9ca404cc725..1d781494cdcc7 100644 --- a/utils/frame/benchmarking-cli/src/overhead/template.rs +++ b/utils/frame/benchmarking-cli/src/overhead/template.rs @@ -82,7 +82,7 @@ impl TemplateData { /// Fill out the `weights.hbs` HBS template with its own data. /// Writes the result to `path` which can be a directory or a file. - pub fn write(&self, path: &str) -> Result<()> { + pub fn write(&self, path: &Option) -> Result<()> { let mut handlebars = Handlebars::new(); // Format large integers with underscores. handlebars.register_helper("underscore", Box::new(crate::writer::UnderscoreHelper)); @@ -98,8 +98,9 @@ impl TemplateData { } /// Build a path for the weight file. - fn build_path(&self, weight_out: &str) -> Result { - let mut path = PathBuf::from(weight_out); + fn build_path(&self, weight_out: &Option) -> Result { + let mut path = weight_out.clone().unwrap_or(PathBuf::new()); + if !path.is_dir() { return Err("Need directory as --weight-path".into()) } diff --git a/utils/frame/benchmarking-cli/src/post_processing/mod.rs b/utils/frame/benchmarking-cli/src/post_processing/mod.rs index 9e4f9a23c7e5b..fb20d9bd0c488 100644 --- a/utils/frame/benchmarking-cli/src/post_processing/mod.rs +++ b/utils/frame/benchmarking-cli/src/post_processing/mod.rs @@ -21,6 +21,7 @@ use sc_cli::Result; use clap::Args; use serde::Serialize; +use std::path::PathBuf; use crate::storage::record::{StatSelect, Stats}; From 2bca7bb2616da1229584e92a8d04090c2de80b99 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Tue, 15 Mar 2022 23:55:49 +0100 Subject: [PATCH 28/29] Fixup Signed-off-by: Oliver Tale-Yazdi --- utils/frame/benchmarking-cli/src/overhead/template.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utils/frame/benchmarking-cli/src/overhead/template.rs b/utils/frame/benchmarking-cli/src/overhead/template.rs index 1d781494cdcc7..f6fb8ed9d929e 100644 --- a/utils/frame/benchmarking-cli/src/overhead/template.rs +++ b/utils/frame/benchmarking-cli/src/overhead/template.rs @@ -99,7 +99,7 @@ impl TemplateData { /// Build a path for the weight file. fn build_path(&self, weight_out: &Option) -> Result { - let mut path = weight_out.clone().unwrap_or(PathBuf::new()); + let mut path = weight_out.clone().unwrap_or(PathBuf::from(".")); if !path.is_dir() { return Err("Need directory as --weight-path".into()) From cf2e763d4a6aa541f7366aefa0e6a732396552e4 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Wed, 16 Mar 2022 13:57:36 +0100 Subject: [PATCH 29/29] Move code to better place Signed-off-by: Oliver Tale-Yazdi --- bin/node/cli/src/command.rs | 59 ++-------------- bin/node/cli/src/command_helper.rs | 69 +++++++++++++++++++ bin/node/cli/src/lib.rs | 2 + .../benchmarking-cli/src/overhead/bench.rs | 2 +- .../benchmarking-cli/src/overhead/cmd.rs | 2 +- 5 files changed, 80 insertions(+), 54 deletions(-) create mode 100644 bin/node/cli/src/command_helper.rs diff --git a/bin/node/cli/src/command.rs b/bin/node/cli/src/command.rs index 56131f8161b8c..e208e324ee2aa 100644 --- a/bin/node/cli/src/command.rs +++ b/bin/node/cli/src/command.rs @@ -16,21 +16,14 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . -use crate::{ - chain_spec, service, - service::{create_extrinsic, new_partial, FullClient}, - Cli, Subcommand, -}; +use crate::{chain_spec, service, service::new_partial, Cli, Subcommand}; use node_executor::ExecutorDispatch; use node_primitives::Block; -use node_runtime::{RuntimeApi, SystemCall}; +use node_runtime::RuntimeApi; use sc_cli::{ChainSpec, Result, RuntimeVersion, SubstrateCli}; use sc_service::PartialComponents; -use sp_inherents::{InherentData, InherentDataProvider}; -use sp_keyring::Sr25519Keyring; -use sp_runtime::OpaqueExtrinsic; -use std::{sync::Arc, time::Duration}; +use std::sync::Arc; impl SubstrateCli for Cli { fn impl_name() -> String { @@ -79,39 +72,6 @@ impl SubstrateCli for Cli { } } -/// Generates extrinsics for the `overhead` benchmark. -/// TODO I will move this into a different file once we agreed on the structure. -struct ExtrinsicBuilder { - client: Arc, -} - -impl frame_benchmarking_cli::ExtrinsicBuilder for ExtrinsicBuilder { - fn remark(&self, nonce: u32) -> Option { - let acc = Sr25519Keyring::Bob.pair(); - let extrinsic: OpaqueExtrinsic = create_extrinsic( - self.client.as_ref(), - acc, - SystemCall::remark { remark: vec![] }, - Some(nonce), - ) - .into(); - - Some(extrinsic) - } -} - -/// Create the inherent data needed to build a block. -fn inherent_data() -> Result { - 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) -} - /// Parse command line arguments into service configuration. pub fn run() -> Result<()> { let cli = Cli::from_args(); @@ -141,23 +101,18 @@ pub fn run() -> Result<()> { Some(Subcommand::BenchmarkOverhead(cmd)) => { let runner = cli.create_runner(cmd)?; runner.async_run(|mut config| { - // TODO 1. is this needed? - // 2. should other stuff be disabled as well? + 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 { client: client.clone() }; + let ext_builder = ExtrinsicBuilder::new(client.clone()); Ok((cmd.run(config, client, inherent_data()?, Arc::new(ext_builder)), task_manager)) }) }, 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()) - } - 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 new file mode 100644 index 0000000000000..51fe7a5c5a7bf --- /dev/null +++ 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 . + +//! 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, +} + +impl ExtrinsicBuilder { + /// Creates a new [`Self`] from the given client. + pub fn new(client: Arc) -> Self { + Self { client } + } +} + +impl frame_benchmarking_cli::ExtrinsicBuilder for ExtrinsicBuilder { + fn remark(&self, nonce: u32) -> std::result::Result { + 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 { + 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 index 791140a25484d..06c0bcccbc296 100644 --- 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/utils/frame/benchmarking-cli/src/overhead/bench.rs b/utils/frame/benchmarking-cli/src/overhead/bench.rs index 26062ae9435ef..3e18c6a86db24 100644 --- a/utils/frame/benchmarking-cli/src/overhead/bench.rs +++ b/utils/frame/benchmarking-cli/src/overhead/bench.rs @@ -122,7 +122,7 @@ where info!("Building block, this takes some time..."); let mut num_ext = 0; for nonce in 0..self.max_ext_per_block() { - let ext = self.ext_builder.remark(nonce).ok_or("Could not build extrinsic")?; + let ext = self.ext_builder.remark(nonce)?; match builder.push(ext.clone()) { Ok(()) => {}, Err(ApplyExtrinsicFailed(Validity(TransactionValidityError::Invalid( diff --git a/utils/frame/benchmarking-cli/src/overhead/cmd.rs b/utils/frame/benchmarking-cli/src/overhead/cmd.rs index 6dc3a9d76d665..8c75627fe2462 100644 --- a/utils/frame/benchmarking-cli/src/overhead/cmd.rs +++ b/utils/frame/benchmarking-cli/src/overhead/cmd.rs @@ -68,7 +68,7 @@ pub struct OverheadParams { /// who's parent block is the genesis block. pub trait ExtrinsicBuilder { /// Build a `System::remark` extrinsic. - fn remark(&self, nonce: u32) -> Option; + fn remark(&self, nonce: u32) -> std::result::Result; } impl OverheadCmd {