Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FRAME] Runtime Omni Bencher #3512

Merged
merged 51 commits into from
Apr 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
51 commits
Select commit Hold shift + click to select a range
eedbe77
Do the stuff
ggwpez Feb 28, 2024
35a203a
Merge remote-tracking branch 'origin/master' into oty-benchmark-from-…
ggwpez Feb 28, 2024
1bab526
Make stuff work
ggwpez Feb 28, 2024
a0d27b8
Remove test code
ggwpez Feb 28, 2024
b165650
Cleanup
ggwpez Feb 28, 2024
e952e87
Rococo and Westend work
ggwpez Mar 1, 2024
be0d71f
Fix overlay rollback
ggwpez Mar 1, 2024
ca35646
Refactor
ggwpez Mar 4, 2024
9e52ca1
Revert old code
ggwpez Mar 4, 2024
f9b3f13
Merge remote-tracking branch 'origin/master' into oty-benchmark-from-…
ggwpez Mar 5, 2024
b797ea2
Clippy
ggwpez Mar 5, 2024
9b00f0b
fixup
ggwpez Mar 14, 2024
59cf796
Reset script to master version
ggwpez Mar 21, 2024
351fa91
Merge remote-tracking branch 'origin/master' into oty-benchmark-from-…
ggwpez Mar 21, 2024
44c037f
Cleanup
ggwpez Mar 21, 2024
8f66e72
lockfile
ggwpez Mar 21, 2024
96cdb1d
WIP
ggwpez Mar 21, 2024
982ab5a
Add BenchmarkSetup to NIS pallet
ggwpez Mar 21, 2024
10cdffd
Remove FAIL-CI
ggwpez Mar 21, 2024
8cbd538
CI: Add dedicated omni runner job
ggwpez Mar 21, 2024
71e98bf
Fix
ggwpez Mar 21, 2024
4d1dfbc
CI: Build runtime
ggwpez Mar 21, 2024
5ee9425
Keep --dev working
ggwpez Mar 21, 2024
d3762e8
Benchmarking default
ggwpez Mar 22, 2024
746c9b8
what
ggwpez Mar 22, 2024
04133e6
Fix
ggwpez Mar 22, 2024
ccdb8ff
Docs
ggwpez Mar 23, 2024
59bddd1
Cleanup shit code
ggwpez Mar 23, 2024
b2e069d
Merge remote-tracking branch 'origin/master' into oty-benchmark-from-…
ggwpez Mar 23, 2024
29f2583
Fixup
ggwpez Mar 25, 2024
e7f4c21
Cleanup
ggwpez Mar 25, 2024
7eeeee0
Add PRdoc
ggwpez Mar 25, 2024
2d8203b
Rename to frame-omni-bencher
ggwpez Apr 5, 2024
cbf5bd6
Update docs
ggwpez Apr 5, 2024
68d7e77
Update substrate/utils/frame/benchmarking-cli/src/pallet/mod.rs
ggwpez Apr 5, 2024
e0515a6
Merge remote-tracking branch 'origin/oty-benchmark-from-wasm' into ot…
ggwpez Apr 5, 2024
9ab5547
Docs
ggwpez Apr 5, 2024
685e582
Add all crates to prdoc
ggwpez Apr 5, 2024
542d500
Merge remote-tracking branch 'origin/master' into oty-benchmark-from-…
ggwpez Apr 5, 2024
a4fef56
fixup merge
ggwpez Apr 5, 2024
02f0514
Cannot doc internal crate
ggwpez Apr 5, 2024
b3507a7
Use new genesis builder api
ggwpez Apr 5, 2024
a7717db
Test with rococo runtime
ggwpez Apr 5, 2024
1a4db1a
Ignore rococo build warnings
ggwpez Apr 5, 2024
86a40e4
Disable forlift
ggwpez Apr 8, 2024
10af924
Merge branch 'master' into oty-benchmark-from-wasm
ggwpez Apr 8, 2024
76a8b0c
Revert "Disable forlift"
ggwpez Apr 8, 2024
95b0e36
Enable assertions for short-benchmarks
ggwpez Apr 8, 2024
f04b21e
Revert "Enable assertions for short-benchmarks"
ggwpez Apr 8, 2024
2e0e5e3
only run for pallet-balances
ggwpez Apr 8, 2024
b420bdd
Use asset-hub-rococo to test omni-bencher
ggwpez Apr 8, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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)))
ggwpez marked this conversation as resolved.
Show resolved Hide resolved
} 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);
}
ggwpez marked this conversation as resolved.
Show resolved Hide resolved
}

}: {
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
Comment on lines +288 to +296
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a comment about how this was derived, and how to keep it correclty updated?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also why and how was this not an issue so far? in the old code we didn't specialize any of these for benchmarks.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these values are set properly in the chain spec. But not in the runtime genesis.

}
}

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:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems this isn't a complete list of changed 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>>();
ggwpez marked this conversation as resolved.
Show resolved Hide resolved
#[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
Loading