From 51bf026b1b751b541602178921caeb575b67f6ad Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Tue, 27 Aug 2024 12:05:15 +0200 Subject: [PATCH] frame-omni-bencher maintenance (#5466) Changes: - Set default level to `Info` again. Seems like a dependency update set it to something higher. - Fix docs to not use `--locked` since we rely on dependency bumps via cargo. - Add README with rust docs. - Fix bug where the node ignored `--heap-pages` argument. You can test the `--heap-pages` bug by running this command on master and then on this branch. Note that it should fail because of the very low heap pages arg: `cargo run --release --bin polkadot --features=runtime-benchmarks -- benchmark pallet --chain=dev --steps=10 --repeat=30 --wasm-execution=compiled --heap-pages=8 --pallet=frame-system --extrinsic="*"` --------- Signed-off-by: Oliver Tale-Yazdi Co-authored-by: ggwpez (cherry picked from commit 7e7c33453eeb14f47c6c4d0f98cc982e485edc77) --- Cargo.lock | 1 + prdoc/pr_5466.prdoc | 14 +++++ .../benchmarking-cli/src/pallet/command.rs | 8 +-- substrate/utils/frame/omni-bencher/Cargo.toml | 2 + substrate/utils/frame/omni-bencher/README.md | 60 +++++++++++++++++++ .../utils/frame/omni-bencher/src/command.rs | 14 +++-- .../utils/frame/omni-bencher/src/main.rs | 14 ++++- 7 files changed, 104 insertions(+), 9 deletions(-) create mode 100644 prdoc/pr_5466.prdoc create mode 100644 substrate/utils/frame/omni-bencher/README.md diff --git a/Cargo.lock b/Cargo.lock index 5384966db3a6..7319c2dedb7f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5788,6 +5788,7 @@ dependencies = [ "sc-cli", "sp-runtime", "sp-statement-store", + "tracing-subscriber 0.3.18", ] [[package]] diff --git a/prdoc/pr_5466.prdoc b/prdoc/pr_5466.prdoc new file mode 100644 index 000000000000..57f20b3585b4 --- /dev/null +++ b/prdoc/pr_5466.prdoc @@ -0,0 +1,14 @@ +crates: +- bump: patch + name: frame-omni-bencher +- bump: patch + name: frame-benchmarking-cli +doc: +- audience: Runtime Dev + description: | + Changes: + - Set default level to `Info` again. Seems like a dependency update set it to something higher. + - Fix docs to not use `--locked` since we rely on dependency bumps via cargo. + - Add README with rust docs. + - Fix bug where the node ignored `--heap-pages` argument. +title: frame-omni-bencher maintenance diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs index 305a9b960b98..15f0d6a1e4f3 100644 --- a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs +++ b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs @@ -240,7 +240,7 @@ impl PalletCmd { let state = &state_without_tracking; let runtime = self.runtime_blob(&state_without_tracking)?; let runtime_code = runtime.code()?; - let alloc_strategy = Self::alloc_strategy(runtime_code.heap_pages); + let alloc_strategy = self.alloc_strategy(runtime_code.heap_pages); let executor = WasmExecutor::<( sp_io::SubstrateHostFunctions, @@ -744,9 +744,9 @@ impl PalletCmd { } /// Allocation strategy for pallet benchmarking. - fn alloc_strategy(heap_pages: Option) -> HeapAllocStrategy { - heap_pages.map_or(DEFAULT_HEAP_ALLOC_STRATEGY, |p| HeapAllocStrategy::Static { - extra_pages: p as _, + fn alloc_strategy(&self, runtime_heap_pages: Option) -> HeapAllocStrategy { + self.heap_pages.or(runtime_heap_pages).map_or(DEFAULT_HEAP_ALLOC_STRATEGY, |p| { + HeapAllocStrategy::Static { extra_pages: p as _ } }) } diff --git a/substrate/utils/frame/omni-bencher/Cargo.toml b/substrate/utils/frame/omni-bencher/Cargo.toml index 2d1e690eb285..7a9a76d2386a 100644 --- a/substrate/utils/frame/omni-bencher/Cargo.toml +++ b/substrate/utils/frame/omni-bencher/Cargo.toml @@ -6,6 +6,7 @@ authors.workspace = true edition.workspace = true repository.workspace = true license.workspace = true +readme = "README.md" [lints] workspace = true @@ -21,5 +22,6 @@ sp-runtime.workspace = true sp-runtime.default-features = true sp-statement-store.workspace = true sp-statement-store.default-features = true +tracing-subscriber = { workspace = true } env_logger = { workspace = true } log = { workspace = true } diff --git a/substrate/utils/frame/omni-bencher/README.md b/substrate/utils/frame/omni-bencher/README.md new file mode 100644 index 000000000000..29bfaeb6450b --- /dev/null +++ b/substrate/utils/frame/omni-bencher/README.md @@ -0,0 +1,60 @@ +# Polkadot Omni Benchmarking CLI + +The Polkadot Omni benchmarker allows to benchmark the extrinsics of any Polkadot runtime. It is +meant to replace the current manual integration of the `benchmark pallet` into every parachain node. +This reduces duplicate code and makes maintenance for builders easier. The CLI is currently only +able to benchmark extrinsics. In the future it is planned to extend this to some other areas. + +General FRAME runtimes could also be used with this benchmarker, as long as they don't utilize any +host functions that are not part of the Polkadot host specification. + +## Installation + +Directly via crates.io: + +```sh +cargo install frame-omni-bencher --profile=production +``` + +from GitHub: + +```sh +cargo install --git https://github.com/paritytech/polkadot-sdk frame-omni-bencher --profile=production +``` + +or locally from the sources: + +```sh +cargo install --path substrate/utils/frame/omni-bencher --profile=production +``` + +Check the installed version and print the docs: + +```sh +frame-omni-bencher --help +``` + +## Usage + +First we need to ensure that there is a runtime available. As example we will build the Westend +runtime: + +```sh +cargo build -p westend-runtime --profile production --features runtime-benchmarks +``` + +Now as an example, we benchmark the `balances` pallet: + +```sh +frame-omni-bencher v1 benchmark pallet \ +--runtime target/release/wbuild/westend-runtime/westend-runtime.compact.compressed.wasm \ +--pallet "pallet_balances" --extrinsic "" +``` + +The `--steps`, `--repeat`, `--heap-pages` and `--wasm-execution` arguments have sane defaults and do +not need be passed explicitly anymore. + +## Backwards Compatibility + +The exposed pallet sub-command is identical as the node-integrated CLI. The only difference is that +it needs to be prefixed with a `v1` to ensure drop-in compatibility. diff --git a/substrate/utils/frame/omni-bencher/src/command.rs b/substrate/utils/frame/omni-bencher/src/command.rs index f0159f4307d6..19177ed549b7 100644 --- a/substrate/utils/frame/omni-bencher/src/command.rs +++ b/substrate/utils/frame/omni-bencher/src/command.rs @@ -36,13 +36,19 @@ use sp_runtime::traits::BlakeTwo256; /// Directly via crates.io: /// /// ```sh -/// cargo install --locked frame-omni-bencher +/// cargo install frame-omni-bencher --profile=production /// ``` /// -/// or when the sources are locally checked out: +/// from GitHub: /// /// ```sh -/// cargo install --locked --path substrate/utils/frame/omni-bencher --profile=production +/// cargo install --git https://github.com/paritytech/polkadot-sdk frame-omni-bencher --profile=production +/// ``` +/// +/// or locally from the sources: +/// +/// ```sh +/// cargo install --path substrate/utils/frame/omni-bencher --profile=production /// ``` /// /// Check the installed version and print the docs: @@ -60,7 +66,7 @@ use sp_runtime::traits::BlakeTwo256; /// cargo build -p westend-runtime --profile production --features runtime-benchmarks /// ``` /// -/// Now as example we benchmark `pallet_balances`: +/// Now as an example, we benchmark the `balances` pallet: /// /// ```sh /// frame-omni-bencher v1 benchmark pallet \ diff --git a/substrate/utils/frame/omni-bencher/src/main.rs b/substrate/utils/frame/omni-bencher/src/main.rs index c148403f2970..3199a59aaa6d 100644 --- a/substrate/utils/frame/omni-bencher/src/main.rs +++ b/substrate/utils/frame/omni-bencher/src/main.rs @@ -20,10 +20,22 @@ mod command; use clap::Parser; use env_logger::Env; use sc_cli::Result; +use tracing_subscriber::EnvFilter; fn main() -> Result<()> { - env_logger::Builder::from_env(Env::default().default_filter_or("info")).init(); + setup_logger(); + log::warn!("The FRAME omni-bencher is not yet battle tested - double check the results.",); command::Command::parse().run() } + +/// Setup logging with `info` as default level. Can be set via `RUST_LOG` env. +fn setup_logger() { + let env_filter = EnvFilter::try_from_default_env().unwrap_or_else(|_| EnvFilter::new("info")); + + tracing_subscriber::fmt() + .with_env_filter(env_filter) + .with_writer(std::io::stderr) + .init(); +}