Skip to content

Commit

Permalink
[benchmarking] Reset to genesis storage after each run (#5655)
Browse files Browse the repository at this point in the history
The genesis state is currently partially provided via
`OverlayedChanges`, but these changes are reset by the runtime after the
first repetition, causing the second repitition to use an invalid
genesis state.

Changes:
- Provide the genesis state as a `Storage` without any
`OverlayedChanges` to make it work correctly with repetitions.
- Add `--genesis-builder-preset` option to use different genesis preset
names.
- Improve error messages.

---------

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: ggwpez <ggwpez@users.noreply.github.com>
Co-authored-by: Bastian Köcher <git@kchr.de>
Co-authored-by: Branislav Kontur <bkontur@gmail.com>
  • Loading branch information
4 people authored Sep 22, 2024
1 parent 128f6c7 commit 2e4e5bf
Show file tree
Hide file tree
Showing 7 changed files with 197 additions and 157 deletions.
15 changes: 15 additions & 0 deletions prdoc/pr_5655.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
title: '[benchmarking] Reset to genesis storage after each run'
doc:
- audience: Runtime Dev
description: |-
The genesis state is currently partially provided via `OverlayedChanges`, but these changes are reset by the runtime after the first repetition, causing the second repitition to use an invalid genesis state.

Changes:
- Provide the genesis state as a `Storage` without any `OverlayedChanges` to make it work correctly with repetitions.
- Add `--genesis-builder-preset` option to use different genesis preset names.
- Improve error messages.
crates:
- name: frame-benchmarking-cli
bump: major
- name: frame-benchmarking-pallet-pov
bump: patch
35 changes: 35 additions & 0 deletions substrate/bin/node/cli/tests/benchmark_pallet_works.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,31 @@ fn benchmark_pallet_works() {
benchmark_pallet(20, 50, true);
}

#[test]
fn benchmark_pallet_args_work() {
benchmark_pallet_args(&["--list", "--pallet=pallet_balances"], true);
benchmark_pallet_args(&["--list", "--pallet=pallet_balances"], true);
benchmark_pallet_args(
&["--list", "--pallet=pallet_balances", "--genesis-builder=spec-genesis"],
true,
);
benchmark_pallet_args(
&["--list", "--pallet=pallet_balances", "--chain=dev", "--genesis-builder=spec-genesis"],
true,
);

// Error because the genesis runtime does not have any presets in it:
benchmark_pallet_args(
&["--list", "--pallet=pallet_balances", "--chain=dev", "--genesis-builder=spec-runtime"],
false,
);
// Error because no runtime is provided:
benchmark_pallet_args(
&["--list", "--pallet=pallet_balances", "--chain=dev", "--genesis-builder=runtime"],
false,
);
}

fn benchmark_pallet(steps: u32, repeat: u32, should_work: bool) {
let status = Command::new(cargo_bin("substrate-node"))
.args(["benchmark", "pallet", "--dev"])
Expand All @@ -51,3 +76,13 @@ fn benchmark_pallet(steps: u32, repeat: u32, should_work: bool) {

assert_eq!(status.success(), should_work);
}

fn benchmark_pallet_args(args: &[&str], should_work: bool) {
let status = Command::new(cargo_bin("substrate-node"))
.args(["benchmark", "pallet"])
.args(args)
.status()
.unwrap();

assert_eq!(status.success(), should_work);
}
31 changes: 31 additions & 0 deletions substrate/frame/benchmarking/pov/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ use frame_support::traits::UnfilteredDispatchable;
use frame_system::{Pallet as System, RawOrigin};
use sp_runtime::traits::Hash;

#[cfg(feature = "std")]
frame_support::parameter_types! {
pub static StorageRootHash: Option<alloc::vec::Vec<u8>> = None;
}

#[benchmarks]
mod benchmarks {
use super::*;
Expand Down Expand Up @@ -392,6 +397,32 @@ mod benchmarks {
}
}

#[benchmark]
fn storage_root_is_the_same_every_time(i: Linear<0, 10>) {
#[cfg(feature = "std")]
let root = sp_io::storage::root(sp_runtime::StateVersion::V1);

#[cfg(feature = "std")]
match (i, StorageRootHash::get()) {
(0, Some(_)) => panic!("StorageRootHash should be None initially"),
(0, None) => StorageRootHash::set(Some(root)),
(_, Some(r)) if r == root => {},
(_, Some(r)) =>
panic!("StorageRootHash should be the same every time: {:?} vs {:?}", r, root),
(_, None) => panic!("StorageRootHash should be Some after the first iteration"),
}

// Also test that everything is reset correctly:
sp_io::storage::set(b"key1", b"value");

#[block]
{
sp_io::storage::set(b"key2", b"value");
}

sp_io::storage::set(b"key3", b"value");
}

impl_benchmark_test_suite!(Pallet, super::mock::new_test_ext(), super::mock::Test,);
}

Expand Down
Loading

0 comments on commit 2e4e5bf

Please sign in to comment.