feat: pallet benchmarking interactive UI #420

chungquantin · 2025-02-19T05:46:17Z

Description

The PR is a part of a bigger PR #424. It implements the full flow of the pop bench pallet including new key features:

Interactive UI to manage parameters before running the benchmark.
Sourcing the frame-omni-bencher binary feat: add code to source omni bencher #428. This is required to list pallets and extrinsics. Explained here
Refactor to use our own version of PalletCmd (called BenchmarkPallet) instead of frame-benchmarking-cli::PalletCmd due to required parameters issue. Explained here

How to test the PR?

pop bench pallet

If arguments are provided like

pop bench pallet --runtime=path-to-runtime --pallet= --extrinsic=

We skip the runtime, the pallet and the extrinsic.

To skip the parameter menu, user can specify flag --skip then the parameter menu can be skipped and go directly to run the bencmarking.

pop bench pallet --skip

Test single line command

pop bench pallet --pallet=pallet_balances --extrinsic=burn_allow_death,burn_keep_alive --steps=50 --repeat=20 --external-repeat=1 --db-cache=1024 --map-size=1000000 --additional-trie-layers=2 --output-pov-analysis=median-slopes --runtime=/Users/chungquantin/Developer/pop-network/pop-cli/tests/runtimes/base_parachain_benchmark.wasm --genesis-builder=runtime --genesis-builder-preset=development

Help command

Full flow breakdown

Note: If --list or --json-output is provided, we run the command by frame-benchmarking-cli directly instead of go through the implementation in pop-cli.

Stage 1: Locate runtime or input runtime path manually

1.1. Run the command in a parachain project, which contains runtime or runtimes (Hyperbridge) folder. This falls to the first stage "Stage 1: Locating and building a runtime binary | Scenario 1" of the previous PR.

1.2. Run the command anywhere (no a parachain project), which does contains runtime or runtimes (Hyperbridge) folder. This falls to the first stage "Stage 1: Locating and building a runtime binary | Scenario 2" of the previous PR.

1.3. Inputting the runtime manually. User can input the path to parachain project or path to WASM binary file directly.

NOTE: After the path is located and there is no WASM file found in the target/<profile>/wbuild/<runtime-name>.wasm, we will build the runtime with runtime-benchmarks feature first.

Stage 2: Select genesis builder policy & preset

See full breakdown in #411

Stage 3: Select pallets (Single Select) & extrinsics (Multiple Select)

At this stage, we will check if the binary of frame-omni-bencher exists on the user's machine. Explained here

Note: We only call frame-omni-bencher to load pallets and extrinsics once a session.

3.1. "🔎 Search for pallets by name": User can specify the pallet name to search for the pallets (fuzzy searching).

For the extrinsic selection, we only allow user to select if there are pallet specified. If all pallet are selected (--pallet=*), we skip the next step.

3.2. "🔎 Search for extrinsics by name": User can specify the extrinsic name to search for the extrinsics (fuzzy searching).

Stage 4: Parameter menu

User can update the parameters by selecting an option in the parameter menu. Some arguments will be disabled depend on the status of other parameters. See is_disabled method. For example:

we don't allow selecting extrinsics if all pallets are selected.
we don't allow update the genesis builder preset if policy is equal to GenesisBuilderPolicy::None

Stage 5: Input the output path

This stage is optional, if no value is provided, we don't write to any file.

Stage 6: Print the single line command

[sc-2882]

…ld_runtime

* feat: add bench subcommand * feat: integrates frame-benchmarking-cli * refactor: PalletCmd run with spec * feat: set RUST_LOG=info for benchmarking display * feat: add CLI messages and tests * feat: add benchmark runtime wasm test * chore: fix comment * fix: operator cannot be applied to type * chore: display error and refactor test file * chore: rename `bench` file * chore: clippy warning * refactor: parachain feature in bench command * feat: add bench subcommand * feat: integrates frame-benchmarking-cli * refactor: PalletCmd run with spec * feat: add CLI messages and tests * feat: add benchmark runtime wasm test * chore: fix comment * fix: operator cannot be applied to type * chore: display error and refactor test file * chore: rename `bench` file * chore: clippy warning * refactor: parachain feature in bench command * chore: revert mod.rs * feat: add bench subcommand * chore: revert mod.rs * feat: add bench subcommand * feat: integrates frame-benchmarking-cli * refactor: PalletCmd run with spec * feat: set RUST_LOG=info for benchmarking display * feat: add CLI messages and tests * feat: add benchmark runtime wasm test * chore: fix comment * fix: operator cannot be applied to type * chore: display error and refactor test file * chore: rename `bench` file * chore: clippy warning * refactor: parachain feature in bench command * feat: add bench subcommand * feat: integrates frame-benchmarking-cli * refactor: PalletCmd run with spec * feat: add CLI messages and tests * feat: add benchmark runtime wasm test * chore: fix comment * fix: operator cannot be applied to type * chore: display error and refactor test file * chore: rename `bench` file * chore: clippy warning * refactor: parachain feature in bench command * chore: revert mod.rs * feat: add bench subcommand * chore: revert mod.rs * chore: revert changes * feat: benchmark existing runtime binary and select policy, presets (#411) * feat: add bench subcommand * feat: integrates frame-benchmarking-cli * refactor: PalletCmd run with spec * feat: set RUST_LOG=info for benchmarking display * feat: add CLI messages and tests * feat: add benchmark runtime wasm test * chore: fix comment * fix: operator cannot be applied to type * chore: display error and refactor test file * feat: auto detect wasm blob and build runtime * feat: build runtime wasm blob * refactor: build binary path method * fix: runtime path test * refactor: feature gating for benchmarking feature * chore: remove unused argument * fix: comment * chore: rename `bench` file * chore: rename `bench` file * feat: auto detect wasm blob and build runtime * feat: build runtime wasm blob * refactor: build binary path method * feat: list and select runtimes * chore: remove unused code * feat: auto detect wasm blob and build runtime * feat: build runtime wasm blob * refactor: build binary path method * chore: removed duplicate code * chore: remove spinner * chore: clippy warning * chore: clippy warning * refactor: parachain feature in bench command * chore: reorder imports * feat: select genesis builder * refactor: test helpers * chore: remove output display * chore: update parse genesis builder comment * fix: locate runtime and help command * chore: clippy warning * chore: display error message on binary check * fix: comment * feat: add genesis preset check (#422) * feat: add genesis preset check * feat: update_genesis_preset * refactor: constant name & add tests for preset * refactor: get_runtime_path * refactor: code order * refactor: test files * chore: clippy warning * refactor: list presets instead of manual input * feat: default to `none` if no presets found * refactor: build project test instead of runtime

…feat-pallet_bench_ui

* refactor: use filter_mode to fuzzy search pallets * chore: update cargo-deny-action@v2 * chore: rebase * chore: revert not relevant changes * chore: delete conflicting file * feat: add confirm input to extrinsics selection

…feat-pallet_bench_ui

chungquantin · 2025-03-05T05:00:38Z

crates/pop-parachains/src/bench/mod.rs

+		Some(temp_file.as_file()),
+		vec![
+			&format!("--runtime={}", runtime_path.display()),
+			"--genesis-builder=none",


We need this flag otherwise error thrown in the child process.

crates/pop-common/src/helpers.rs

chungquantin · 2025-03-05T05:06:53Z

crates/pop-parachains/Cargo.toml

@@ -37,6 +37,7 @@ zombienet-sdk.workspace = true
 # Benchmarking
 cumulus-primitives-proof-size-hostfunction.workspace = true
 frame-benchmarking-cli.workspace = true
+fuzzy-matcher = "0.3.7"


Removed this crate in #442

chungquantin · 2025-03-05T05:07:58Z

crates/pop-cli/src/common/bench.rs

+		.default_input("./runtime")
+		.placeholder("./runtime")
+		.interact()
+		.map(PathBuf::from)


chungquantin · 2025-03-05T05:08:03Z

crates/pop-cli/src/common/bench.rs

+		.input("Please provide the path to the runtime or parachain project.")
+		.required(true)
+		.default_input("./runtime")
+		.placeholder("./runtime")


Daanvdplas

Thanks for the super clear information provided! I was able to immediately start reviewing without any hurdles, fantastic!!!

Also well done on the PR and research that you have put into it!

I have a question and some commands that were failing for me, I have gone through all the code but haven't been able to test the functionality yet so that will come next time.

Why pop bench pallet and not pop bench?

Some commands that were failing:

pop bench pallet --list not so clear error:

Resolved by not throwing error anymore. Listing pallets for non runtime-benchmarks feature runtime binary works normally.

pop bench pallet:

@chungquantin Resolved:

pop bench pallet --runtime=runtime --pallet= --extrinsic= in pop-node folder:

@chungquantin Resolved:

crates/pop-cli/src/commands/bench/pallet.rs

Daanvdplas · 2025-03-05T11:37:39Z

crates/pop-cli/src/commands/bench/pallet.rs

+
+	/// Comma separated list of pallets that should be excluded from the benchmark.
+	#[arg(long, value_parser, num_args = 1.., value_delimiter = ',')]
+	pub exclude_pallets: Vec<String>,


I assume this one is used in combination with the below flag or pallet=* ?

Yes correct, this is only used when pallet=*

And with the flag all? If that is not allowed we could add the conflicts_with

crates/pop-cli/src/commands/bench/pallet.rs

crates/pop-cli/src/common/bench.rs

crates/pop-parachains/src/bench/mod.rs

AlexD10S

Great work, Tin! The searcher is running smoothly, so cool!

I left a few comments, but they’re all minor and not blockers. Approved! 🎉

AlexD10S · 2025-03-05T20:42:29Z

crates/pop-cli/src/commands/bench/pallet.rs

+				match option.update_arguments(self, &mut registry, cli).await {
+					Ok(true) => break,
+					Ok(false) => continue,
+					Err(e) => cli.info(e)?,


Not requesting a change, just a question, an info message here is better than an error?

Throws error here would panic and cancel the loop. The idea is whenever there's an error, we log the error and display the menu again.

I didn't mean to throw an error, it was more about the message cliclack::log::error or maybe a cliclack::log::warning instead of an info.

Ah I see, apologies I did not aware of the feature in cliclack

crates/pop-cli/src/commands/bench/pallet.rs

crates/pop-cli/src/common/bench.rs

crates/pop-parachains/src/bench/mod.rs

crates/pop-cli/src/commands/bench/pallet.rs

Daanvdplas · 2025-03-07T13:43:22Z

I did pop bench pallet and gave input to points 0 to 4, now in the parameter menu they are asked again:

If it is an easy fix, can we eliminate the things that are provided by the user from the menu?

Could we ask if the user would like to go through the parameter menu? The skip flag is already a great flag!
Loading the menu is also slow, do we know why?
Can we remove the word FRAME here?

Could we add a newline before outputting the generated command (great feature!):

When we can choose the extrinsics to benchmark, can we add (select with space)?

Daanvdplas

I left some UX improvements / suggestions, but wow what an amazing PR!

chungquantin added 30 commits February 10, 2025 15:51

feat: add bench subcommand

051d5ed

Merge branch 'r0gue-io:main' into chungquantin/feat-pop_bench

23ba99f

feat: integrates frame-benchmarking-cli

5987931

refactor: PalletCmd run with spec

0d5f5ef

feat: set RUST_LOG=info for benchmarking display

f6a8f06

feat: add CLI messages and tests

a3139e4

feat: add benchmark runtime wasm test

45c6ade

chore: fix comment

53258ac

fix: operator cannot be applied to type

8f82fde

chore: display error and refactor test file

fa2db46

feat: auto detect wasm blob and build runtime

907696a

feat: build runtime wasm blob

7011eab

refactor: build binary path method

28c637a

fix: runtime path test

dc63358

refactor: feature gating for benchmarking feature

6f17808

chore: remove unused argument

557cdaa

fix: comment

2306420

chore: rename bench file

667e997

chore: rename bench file

646d6f5

feat: auto detect wasm blob and build runtime

5863fd3

feat: build runtime wasm blob

b09800e

refactor: build binary path method

299b60e

feat: list and select runtimes

f879739

chore: remove unused code

5c9c4b4

feat: auto detect wasm blob and build runtime

c343380

feat: build runtime wasm blob

81d93e2

refactor: build binary path method

aa4cbb5

chore: removed duplicate code

1b5dae9

chore: remove spinner

42d134a

Merge branch 'chungquantin/feat-pop_bench' into chugnquantin/feat-bui…

bdcfd06

…ld_runtime

chungquantin added 13 commits March 5, 2025 10:03

chore: rebase

aedf8bc

chore: rebase

16525fb

chore: revert cargo.lock and add more ignore

7385186

chore: update cargo-deny-action@v2

ecc9849

chore: revert relay.rs changes

93bb2fd

chore: remove vulnerabilities by upgrade zombienet

6961279

chore: rebase

0621a08

chore: revert not relevant changes

fef73a2

Merge branch 'chungquantin/feat-pop_bench_command' into chungquantin/…

3acdb69

…feat-pallet_bench_ui

Merge branch 'chungquantin/feat-pop_bench_command' into chungquantin/…

4d96402

…feat-pallet_bench_ui

chore: test for get_relative_or_absolute_path

7641a8a

chungquantin commented Mar 5, 2025

View reviewed changes

chungquantin requested a review from AlexD10S March 5, 2025 05:10

refactor: remove methods only used once

5181c5a

chungquantin force-pushed the chungquantin/feat-pallet_bench_ui branch from 2fb9adf to 5181c5a Compare March 5, 2025 06:05

Daanvdplas reviewed Mar 5, 2025

View reviewed changes

AlexD10S approved these changes Mar 5, 2025

View reviewed changes

chungquantin added 6 commits March 6, 2025 13:23

refactor: get arguments ok_or_else

e9553d7

refactor: TryFrom<String> for genesis builder

89791b8

chore: comments

40a80c6

chore: return error if is not provided

ff2f39d

fix: unit tests adapt to new changes

9030238

chore: ensure runtime binary exists test

8e3fee3

chungquantin requested a review from Daanvdplas March 6, 2025 12:08

chore: clippy warning

0a40834

chungquantin mentioned this pull request Mar 7, 2025

feat: implement filter mode for pop call #445

Open

Daanvdplas approved these changes Mar 7, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: pallet benchmarking interactive UI #420

feat: pallet benchmarking interactive UI #420

chungquantin commented Feb 19, 2025 •

edited

Loading

chungquantin Mar 5, 2025

chungquantin Mar 5, 2025

chungquantin Mar 5, 2025

chungquantin Mar 5, 2025

Daanvdplas left a comment •

edited by chungquantin

Loading

Daanvdplas Mar 5, 2025

chungquantin Mar 5, 2025

Daanvdplas Mar 7, 2025

AlexD10S left a comment

AlexD10S Mar 5, 2025

chungquantin Mar 6, 2025

AlexD10S Mar 7, 2025 •

edited

Loading

chungquantin Mar 7, 2025

Daanvdplas commented Mar 7, 2025

Daanvdplas left a comment

feat: pallet benchmarking interactive UI #420

Are you sure you want to change the base?

feat: pallet benchmarking interactive UI #420

Conversation

chungquantin commented Feb 19, 2025 • edited Loading

Description

How to test the PR?

Help command

Full flow breakdown

Stage 1: Locate runtime or input runtime path manually

Stage 2: Select genesis builder policy & preset

Stage 3: Select pallets (Single Select) & extrinsics (Multiple Select)

Stage 4: Parameter menu

Stage 5: Input the output path

Stage 6: Print the single line command

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Daanvdplas left a comment • edited by chungquantin Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlexD10S left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlexD10S Mar 7, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Daanvdplas commented Mar 7, 2025

Daanvdplas left a comment

Choose a reason for hiding this comment

chungquantin commented Feb 19, 2025 •

edited

Loading

Daanvdplas left a comment •

edited by chungquantin

Loading

AlexD10S Mar 7, 2025 •

edited

Loading