Skip to content

Commit

Permalink
[FRAME] Runtime Omni Bencher (paritytech#3512)
Browse files Browse the repository at this point in the history
This MR contains two major changes and some maintenance cleanup.  

## 1. Free Standing Pallet Benchmark Runner

Closes paritytech#3045, depends
on your runtime exposing the `GenesisBuilderApi` (like
paritytech#1492).

Introduces a new binary crate: `frame-omni-bencher`.  
It allows to directly benchmark a WASM blob - without needing a node or
chain spec.

This makes it much easier to generate pallet weights and should allow us
to remove bloaty code from the node.
It should work for all FRAME runtimes that dont use 3rd party host calls
or non `BlakeTwo256` block hashing (basically all polkadot parachains
should work).

It is 100% backwards compatible with the old CLI args, when the `v1`
compatibility command is used. This is done to allow for forwards
compatible addition of new commands.

### Example (full example in the Rust docs)

Installing the CLI:
```sh
cargo install --locked --path substrate/utils/frame/omni-bencher
frame-omni-bencher --help
```

Building the Westend runtime:
```sh
cargo build -p westend-runtime --release --features runtime-benchmarks
```

Benchmarking the runtime:
```sh
frame-omni-bencher v1 benchmark pallet --runtime target/release/wbuild/westend-runtime/westend_runtime.compact.compressed.wasm --all
```

## 2. Building the Benchmark Genesis State in the Runtime

Closes paritytech#2664

This adds `--runtime` and `--genesis-builder=none|runtime|spec`
arguments to the `benchmark pallet` command to make it possible to
generate the genesis storage by the runtime. This can be used with both
the node and the freestanding benchmark runners. It utilizes the new
`GenesisBuilder` RA and depends on having
paritytech#3412 deployed.

## 3. Simpler args for `PalletCmd::run`

You can do three things here to integrate the changes into your node:
- nothing: old code keeps working as before but emits a deprecated
warning
- delete: remove the pallet benchmarking code from your node and use the
omni-bencher instead
- patch: apply the patch below and keep using as currently. This emits a
deprecated warning at runtime, since it uses the old way to generate a
genesis state, but is the smallest change.

```patch
runner.sync_run(|config| cmd
-    .run::<HashingFor<Block>, ReclaimHostFunctions>(config)
+    .run_with_spec::<HashingFor<Block>, ReclaimHostFunctions>(Some(config.chain_spec))
)
```

## 4. Maintenance Change
- `pallet-nis` get a `BenchmarkSetup` config item to prepare its
counterparty asset.
- Add percent progress print when running benchmarks.
- Dont immediately exit on benchmark error but try to run as many as
possible and print errors last.

---------

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: Liam Aharon <liam.aharon@hotmail.com>
  • Loading branch information
2 people authored and dharjeezy committed Apr 9, 2024
1 parent 387c044 commit 63a20c3
Show file tree
Hide file tree
Showing 33 changed files with 1,064 additions and 307 deletions.
4 changes: 2 additions & 2 deletions .gitlab/pipeline/check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ cargo-clippy:
variables:
RUSTFLAGS: "-D warnings"
script:
- SKIP_WASM_BUILD=1 cargo clippy --all-targets --locked --workspace
- SKIP_WASM_BUILD=1 cargo clippy --all-targets --all-features --locked --workspace
- SKIP_WASM_BUILD=1 cargo clippy --all-targets --locked --workspace --quiet
- SKIP_WASM_BUILD=1 cargo clippy --all-targets --all-features --locked --workspace --quiet

check-try-runtime:
stage: check
Expand Down
19 changes: 18 additions & 1 deletion .gitlab/pipeline/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,24 @@ quick-benchmarks:
WASM_BUILD_NO_COLOR: 1
WASM_BUILD_RUSTFLAGS: "-C debug-assertions -D warnings"
script:
- time cargo run --locked --release -p staging-node-cli --bin substrate-node --features runtime-benchmarks -- benchmark pallet --execution wasm --wasm-execution compiled --chain dev --pallet "*" --extrinsic "*" --steps 2 --repeat 1
- time cargo run --locked --release -p staging-node-cli --bin substrate-node --features runtime-benchmarks --quiet -- benchmark pallet --chain dev --pallet "*" --extrinsic "*" --steps 2 --repeat 1 --quiet

quick-benchmarks-omni:
stage: test
extends:
- .docker-env
- .common-refs
- .run-immediately
variables:
# Enable debug assertions since we are running optimized builds for testing
# but still want to have debug assertions.
RUSTFLAGS: "-C debug-assertions"
RUST_BACKTRACE: "full"
WASM_BUILD_NO_COLOR: 1
WASM_BUILD_RUSTFLAGS: "-C debug-assertions"
script:
- time cargo build --locked --quiet --release -p asset-hub-westend-runtime --features runtime-benchmarks
- time cargo run --locked --release -p frame-omni-bencher --quiet -- v1 benchmark pallet --runtime target/release/wbuild/asset-hub-westend-runtime/asset_hub_westend_runtime.compact.compressed.wasm --all --steps 2 --repeat 1 --quiet

test-frame-examples-compile-to-wasm:
# into one job
Expand Down
16 changes: 16 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -492,6 +492,7 @@ members = [
"substrate/utils/frame/frame-utilities-cli",
"substrate/utils/frame/generate-bags",
"substrate/utils/frame/generate-bags/node-runtime",
"substrate/utils/frame/omni-bencher",
"substrate/utils/frame/remote-externalities",
"substrate/utils/frame/rpc/client",
"substrate/utils/frame/rpc/state-trie-migration-rpc",
Expand Down
2 changes: 1 addition & 1 deletion cumulus/polkadot-parachain/src/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -568,7 +568,7 @@ pub fn run() -> Result<()> {
match cmd {
BenchmarkCmd::Pallet(cmd) =>
if cfg!(feature = "runtime-benchmarks") {
runner.sync_run(|config| cmd.run::<sp_runtime::traits::HashingFor<Block>, ReclaimHostFunctions>(config))
runner.sync_run(|config| cmd.run_with_spec::<sp_runtime::traits::HashingFor<Block>, ReclaimHostFunctions>(Some(config.chain_spec)))
} else {
Err("Benchmarking wasn't enabled when building the node. \
You can enable it with `--features runtime-benchmarks`."
Expand Down
6 changes: 4 additions & 2 deletions polkadot/cli/src/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -451,8 +451,10 @@ pub fn run() -> Result<()> {

if cfg!(feature = "runtime-benchmarks") {
runner.sync_run(|config| {
cmd.run::<sp_runtime::traits::HashingFor<service::Block>, ()>(config)
.map_err(|e| Error::SubstrateCli(e))
cmd.run_with_spec::<sp_runtime::traits::HashingFor<service::Block>, ()>(
Some(config.chain_spec),
)
.map_err(|e| Error::SubstrateCli(e))
})
} else {
Err(sc_cli::Error::Input(
Expand Down
7 changes: 5 additions & 2 deletions polkadot/runtime/common/src/auctions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1899,10 +1899,13 @@ mod benchmarking {

// Trigger epoch change for new random number value:
{
pallet_babe::EpochStart::<T>::set((Zero::zero(), u32::MAX.into()));
pallet_babe::Pallet::<T>::on_initialize(duration + now + T::EndingPeriod::get());
let authorities = pallet_babe::Pallet::<T>::authorities();
let next_authorities = authorities.clone();
pallet_babe::Pallet::<T>::enact_epoch_change(authorities, next_authorities, None);
// Check for non empty authority set since it otherwise emits a No-OP warning.
if !authorities.is_empty() {
pallet_babe::Pallet::<T>::enact_epoch_change(authorities.clone(), authorities, None);
}
}

}: {
Expand Down
27 changes: 25 additions & 2 deletions polkadot/runtime/parachains/src/configuration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ pub struct HostConfiguration<BlockNumber> {

impl<BlockNumber: Default + From<u32>> Default for HostConfiguration<BlockNumber> {
fn default() -> Self {
Self {
let ret = Self {
async_backing_params: AsyncBackingParams {
max_candidate_depth: 0,
allowed_ancestry_len: 0,
Expand Down Expand Up @@ -270,7 +270,30 @@ impl<BlockNumber: Default + From<u32>> Default for HostConfiguration<BlockNumber
minimum_backing_votes: LEGACY_MIN_BACKING_VOTES,
node_features: NodeFeatures::EMPTY,
scheduler_params: Default::default(),
}
};

#[cfg(feature = "runtime-benchmarks")]
let ret = ret.with_benchmarking_default();
ret
}
}

#[cfg(feature = "runtime-benchmarks")]
impl<BlockNumber: Default + From<u32>> HostConfiguration<BlockNumber> {
/// Mutate the values of self to be good estimates for benchmarking.
///
/// The values do not need to be worst-case, since the benchmarking logic extrapolates. They
/// should be a bit more than usually expected.
fn with_benchmarking_default(mut self) -> Self {
self.max_head_data_size = self.max_head_data_size.max(1 << 20);
self.max_downward_message_size = self.max_downward_message_size.max(1 << 16);
self.hrmp_channel_max_capacity = self.hrmp_channel_max_capacity.max(1000);
self.hrmp_channel_max_message_size = self.hrmp_channel_max_message_size.max(1 << 16);
self.hrmp_max_parachain_inbound_channels =
self.hrmp_max_parachain_inbound_channels.max(100);
self.hrmp_max_parachain_outbound_channels =
self.hrmp_max_parachain_outbound_channels.max(100);
self
}
}

Expand Down
2 changes: 2 additions & 0 deletions polkadot/runtime/parachains/src/hrmp/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ where
let deposit: BalanceOf<T> = config.hrmp_sender_deposit.unique_saturated_into();
let capacity = config.hrmp_channel_max_capacity;
let message_size = config.hrmp_channel_max_message_size;
assert!(message_size > 0, "Invalid genesis for benchmarking");
assert!(capacity > 0, "Invalid genesis for benchmarking");

let sender: ParaId = from.into();
let sender_origin: crate::Origin = from.into();
Expand Down
12 changes: 6 additions & 6 deletions polkadot/runtime/parachains/src/paras/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ fn generate_disordered_actions_queue<T: Config>() {

benchmarks! {
force_set_current_code {
let c in 1 .. MAX_CODE_SIZE;
let c in MIN_CODE_SIZE .. MAX_CODE_SIZE;
let new_code = ValidationCode(vec![0; c as usize]);
let para_id = ParaId::from(c as u32);
CurrentCodeHash::<T>::insert(&para_id, new_code.hash());
Expand All @@ -92,7 +92,7 @@ benchmarks! {
assert_last_event::<T>(Event::CurrentCodeUpdated(para_id).into());
}
force_set_current_head {
let s in 1 .. MAX_HEAD_DATA_SIZE;
let s in MIN_CODE_SIZE .. MAX_HEAD_DATA_SIZE;
let new_head = HeadData(vec![0; s as usize]);
let para_id = ParaId::from(1000);
}: _(RawOrigin::Root, para_id, new_head)
Expand All @@ -104,7 +104,7 @@ benchmarks! {
let context = BlockNumberFor::<T>::from(1000u32);
}: _(RawOrigin::Root, para_id, context)
force_schedule_code_upgrade {
let c in 1 .. MAX_CODE_SIZE;
let c in MIN_CODE_SIZE .. MAX_CODE_SIZE;
let new_code = ValidationCode(vec![0; c as usize]);
let para_id = ParaId::from(c as u32);
let block = BlockNumberFor::<T>::from(c);
Expand All @@ -114,7 +114,7 @@ benchmarks! {
assert_last_event::<T>(Event::CodeUpgradeScheduled(para_id).into());
}
force_note_new_head {
let s in 1 .. MAX_HEAD_DATA_SIZE;
let s in MIN_CODE_SIZE .. MAX_HEAD_DATA_SIZE;
let para_id = ParaId::from(1000);
let new_head = HeadData(vec![0; s as usize]);
let old_code_hash = ValidationCode(vec![0]).hash();
Expand All @@ -126,7 +126,7 @@ benchmarks! {
generate_disordered_pruning::<T>();
Pallet::<T>::schedule_code_upgrade(
para_id,
ValidationCode(vec![0]),
ValidationCode(vec![0u8; MIN_CODE_SIZE as usize]),
expired,
&config,
UpgradeStrategy::SetGoAheadSignal,
Expand All @@ -145,7 +145,7 @@ benchmarks! {
}

add_trusted_validation_code {
let c in 1 .. MAX_CODE_SIZE;
let c in MIN_CODE_SIZE .. MAX_CODE_SIZE;
let new_code = ValidationCode(vec![0; c as usize]);

pvf_check::prepare_bypassing_bench::<T>(new_code.clone());
Expand Down
2 changes: 2 additions & 0 deletions polkadot/runtime/rococo/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1154,6 +1154,8 @@ impl pallet_nis::Config for Runtime {
type MaxIntakeWeight = MaxIntakeWeight;
type ThawThrottle = ThawThrottle;
type RuntimeHoldReason = RuntimeHoldReason;
#[cfg(feature = "runtime-benchmarks")]
type BenchmarkSetup = ();
}

parameter_types! {
Expand Down
47 changes: 47 additions & 0 deletions prdoc/pr_3512.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
title: "[FRAME] Introduce Runtime Omni Bencher"

doc:
- audience: Node Dev
description: |
Introduces a new freestanding binary; the `frame-omni-bencher`. This can be used as alternative to the node-integrated `benchmark pallet` command. It currently only supports pallet benchmarking.

The optional change to integrate this MR is in the node. The `run` function is now deprecated in favour or `run_with_spec`. This should be rather easy to integrate:

```patch
runner.sync_run(|config| cmd
- .run::<HashingFor<Block>, ReclaimHostFunctions>(config)
+ .run_with_spec::<HashingFor<Block>, ReclaimHostFunctions>(Some(config.chain_spec))
)
```

Additionally, a new `--runtime` CLI arg was introduced to the `benchmark pallet` command. It allows to generate the genesis state directly by the runtime, instead of using a spec file.

crates:
- name: pallet-nis
bump: major
- name: frame-benchmarking-cli
bump: major
- name: polkadot-parachain-bin
bump: patch
- name: polkadot-cli
bump: patch
- name: polkadot-runtime-common
bump: patch
- name: polkadot-runtime-parachains
bump: patch
- name: rococo-runtime
bump: patch
- name: staging-node-cli
bump: patch
- name: kitchensink-runtime
bump: patch
- name: pallet-babe
bump: patch
- name: frame-benchmarking
bump: patch
- name: pallet-election-provider-multi-phase
bump: patch
- name: pallet-fast-unstake
bump: patch
- name: pallet-contracts
bump: patch
2 changes: 1 addition & 1 deletion substrate/bin/node/cli/src/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ pub fn run() -> Result<()> {
)
}

cmd.run::<HashingFor<Block>, sp_statement_store::runtime_api::HostFunctions>(config)
cmd.run_with_spec::<HashingFor<Block>, sp_statement_store::runtime_api::HostFunctions>(Some(config.chain_spec))
},
BenchmarkCmd::Block(cmd) => {
// ensure that we keep the task manager alive
Expand Down
19 changes: 19 additions & 0 deletions substrate/bin/node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1759,6 +1759,25 @@ impl pallet_nis::Config for Runtime {
type MaxIntakeWeight = MaxIntakeWeight;
type ThawThrottle = ThawThrottle;
type RuntimeHoldReason = RuntimeHoldReason;
#[cfg(feature = "runtime-benchmarks")]
type BenchmarkSetup = SetupAsset;
}

#[cfg(feature = "runtime-benchmarks")]
pub struct SetupAsset;
#[cfg(feature = "runtime-benchmarks")]
impl pallet_nis::BenchmarkSetup for SetupAsset {
fn create_counterpart_asset() {
let owner = AccountId::from([0u8; 32]);
// this may or may not fail depending on if the chain spec or runtime genesis is used.
let _ = Assets::force_create(
RuntimeOrigin::root(),
9u32.into(),
sp_runtime::MultiAddress::Id(owner),
true,
1,
);
}
}

parameter_types! {
Expand Down
2 changes: 1 addition & 1 deletion substrate/frame/babe/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ pub mod pallet {
/// entropy was fixed (i.e. it was known to chain observers). Since epochs are defined in
/// slots, which may be skipped, the block numbers may not line up with the slot numbers.
#[pallet::storage]
pub(super) type EpochStart<T: Config> =
pub type EpochStart<T: Config> =
StorageValue<_, (BlockNumberFor<T>, BlockNumberFor<T>), ValueQuery>;

/// How late the current block is compared to its parent.
Expand Down
14 changes: 7 additions & 7 deletions substrate/frame/benchmarking/src/v1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -850,8 +850,8 @@ macro_rules! impl_bench_name_tests {
if !($extra) {
let disabled = $crate::__private::vec![ $( stringify!($names_extra).as_ref() ),* ];
if disabled.contains(&stringify!($name)) {
$crate::__private::log::error!(
"INFO: extra benchmark skipped - {}",
$crate::__private::log::debug!(
"extra benchmark skipped - {}",
stringify!($name),
);
return ();
Expand All @@ -874,21 +874,21 @@ macro_rules! impl_bench_name_tests {
$crate::BenchmarkError::Override(_) => {
// This is still considered a success condition.
$crate::__private::log::error!(
"WARNING: benchmark error overridden - {}",
"benchmark error overridden - {}",
stringify!($name),
);
},
$crate::BenchmarkError::Skip => {
// This is considered a success condition.
$crate::__private::log::error!(
"WARNING: benchmark error skipped - {}",
$crate::__private::log::debug!(
"benchmark skipped - {}",
stringify!($name),
);
},
$crate::BenchmarkError::Weightless => {
// This is considered a success condition.
$crate::__private::log::error!(
"WARNING: benchmark weightless skipped - {}",
$crate::__private::log::debug!(
"benchmark weightless skipped - {}",
stringify!($name),
);
}
Expand Down
4 changes: 2 additions & 2 deletions substrate/frame/contracts/src/benchmarking/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ mod benchmarks {
#[benchmark(pov_mode = Measured)]
fn migration_noop() {
let version = LATEST_MIGRATION_VERSION;
assert_eq!(StorageVersion::get::<Pallet<T>>(), version);
StorageVersion::new(version).put::<Pallet<T>>();
#[block]
{
Migration::<T>::migrate(Weight::MAX);
Expand All @@ -358,7 +358,7 @@ mod benchmarks {
#[benchmark(pov_mode = Measured)]
fn on_runtime_upgrade_noop() {
let latest_version = LATEST_MIGRATION_VERSION;
assert_eq!(StorageVersion::get::<Pallet<T>>(), latest_version);
StorageVersion::new(latest_version).put::<Pallet<T>>();
#[block]
{
<Migration<T, false> as frame_support::traits::OnRuntimeUpgrade>::on_runtime_upgrade();
Expand Down
2 changes: 1 addition & 1 deletion substrate/frame/election-provider-multi-phase/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -931,7 +931,7 @@ pub mod pallet {
.expect(error_message);

// Store the newly received solution.
log!(info, "queued unsigned solution with score {:?}", ready.score);
log!(debug, "queued unsigned solution with score {:?}", ready.score);
let ejected_a_solution = <QueuedSolution<T>>::exists();
<QueuedSolution<T>>::put(ready);
Self::deposit_event(Event::SolutionStored {
Expand Down
2 changes: 1 addition & 1 deletion substrate/frame/fast-unstake/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -560,7 +560,7 @@ pub mod pallet {
if !remaining.is_zero() {
Self::halt("not enough balance to unreserve");
} else {
log!(info, "unstaked {:?}, outcome: {:?}", stash, result);
log!(debug, "unstaked {:?}, outcome: {:?}", stash, result);
Self::deposit_event(Event::<T>::Unstaked { stash, result });
}
};
Expand Down
Loading

0 comments on commit 63a20c3

Please sign in to comment.