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

Draft benchmark sanity weight test #1493

Closed
wants to merge 16 commits into from
5 changes: 2 additions & 3 deletions .gitlab/pipeline/short-benchmarks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ short-benchmark-westend: &short-bench
tags:
- benchmark
script:
- ./artifacts/polkadot benchmark pallet --chain $RUNTIME-dev --pallet "*" --extrinsic "*" --steps 2 --repeat 1
- ./artifacts/polkadot benchmark pallet --chain $RUNTIME-dev --pallet "*" --extrinsic "*" --steps 2 --repeat 1 --sanity-weight-check ignore

# run short-benchmarks for system parachain runtimes from cumulus

Expand All @@ -47,8 +47,7 @@ short-benchmark-westend: &short-bench
tags:
- benchmark
script:
- ./artifacts/polkadot-parachain benchmark pallet --chain $RUNTIME_CHAIN --pallet "*" --extrinsic "*" --steps 2 --repeat 1

- ./artifacts/polkadot-parachain benchmark pallet --chain $RUNTIME_CHAIN --pallet "*" --extrinsic "*" --steps 2 --repeat 1 --sanity-weight-check ignore
short-benchmark-asset-hub-polkadot:
<<: *short-bench-cumulus
variables:
Expand Down
4 changes: 1 addition & 3 deletions .gitlab/pipeline/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ test-linux-stable-runtime-benchmarks:
RUSTFLAGS: "-Cdebug-assertions=y -Dwarnings"
script:
- time cargo nextest run --workspace --features runtime-benchmarks benchmark --locked --cargo-profile testnet

# can be used to run all tests
# test-linux-stable-all:
# stage: test
Expand Down Expand Up @@ -352,8 +351,7 @@ quick-benchmarks:
WASM_BUILD_NO_COLOR: 1
WASM_BUILD_RUSTFLAGS: "-C debug-assertions -D warnings"
script:
- time cargo run --locked --release -p 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 node-cli --bin substrate-node --features runtime-benchmarks -- benchmark pallet --execution wasm --wasm-execution compiled --chain dev --pallet "*" --extrinsic "*" --steps 2 --repeat 1 --sanity-weight-check ignore
test-frame-examples-compile-to-wasm:
# into one job
stage: test
Expand Down
6 changes: 5 additions & 1 deletion cumulus/parachain-template/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -702,6 +702,8 @@ impl_runtime_apis! {
fn benchmark_metadata(extra: bool) -> (
Vec<frame_benchmarking::BenchmarkList>,
Vec<frame_support::traits::StorageInfo>,
Weight,
frame_support::weights::RuntimeDbWeight,
) {
use frame_benchmarking::{Benchmarking, BenchmarkList};
use frame_support::traits::StorageInfoTrait;
Expand All @@ -712,7 +714,9 @@ impl_runtime_apis! {
list_benchmarks!(list, extra);

let storage_info = AllPalletsWithSystem::storage_info();
(list, storage_info)
let max_extrinsic_weight = RuntimeBlockWeights::get().per_class.get(DispatchClass::Normal).max_extrinsic.unwrap();
let db_weight: frame_support::weights::RuntimeDbWeight = <Self as frame_system::Config>::DbWeight::get();
(list, storage_info, max_extrinsic_weight, db_weight)
}

fn dispatch_benchmark(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1162,6 +1162,8 @@ impl_runtime_apis! {
fn benchmark_metadata(extra: bool) -> (
Vec<frame_benchmarking::BenchmarkList>,
Vec<frame_support::traits::StorageInfo>,
Weight,
frame_support::weights::RuntimeDbWeight,
) {
use frame_benchmarking::{Benchmarking, BenchmarkList};
use frame_support::traits::StorageInfoTrait;
Expand All @@ -1186,7 +1188,9 @@ impl_runtime_apis! {
list_benchmarks!(list, extra);

let storage_info = AllPalletsWithSystem::storage_info();
(list, storage_info)
let max_extrinsic_weight = RuntimeBlockWeights::get().per_class.get(DispatchClass::Normal).max_extrinsic.unwrap();
let db_weight: frame_support::weights::RuntimeDbWeight = <Self as frame_system::Config>::DbWeight::get();
(list, storage_info, max_extrinsic_weight, db_weight)
}

fn dispatch_benchmark(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1042,6 +1042,8 @@ impl_runtime_apis! {
fn benchmark_metadata(extra: bool) -> (
Vec<frame_benchmarking::BenchmarkList>,
Vec<frame_support::traits::StorageInfo>,
Weight,
frame_support::weights::RuntimeDbWeight,
) {
use frame_benchmarking::{Benchmarking, BenchmarkList};
use frame_support::traits::StorageInfoTrait;
Expand All @@ -1065,7 +1067,9 @@ impl_runtime_apis! {
list_benchmarks!(list, extra);

let storage_info = AllPalletsWithSystem::storage_info();
(list, storage_info)
let max_extrinsic_weight = RuntimeBlockWeights::get().per_class.get(DispatchClass::Normal).max_extrinsic.unwrap();
let db_weight: frame_support::weights::RuntimeDbWeight = <Self as frame_system::Config>::DbWeight::get();
(list, storage_info, max_extrinsic_weight, db_weight)
}

fn dispatch_benchmark(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1241,6 +1241,8 @@ impl_runtime_apis! {
fn benchmark_metadata(extra: bool) -> (
Vec<frame_benchmarking::BenchmarkList>,
Vec<frame_support::traits::StorageInfo>,
Weight,
frame_support::weights::RuntimeDbWeight,
) {
use frame_benchmarking::{Benchmarking, BenchmarkList};
use frame_support::traits::StorageInfoTrait;
Expand Down Expand Up @@ -1269,7 +1271,9 @@ impl_runtime_apis! {
list_benchmarks!(list, extra);

let storage_info = AllPalletsWithSystem::storage_info();
(list, storage_info)
let max_extrinsic_weight = RuntimeBlockWeights::get().per_class.get(DispatchClass::Normal).max_extrinsic.unwrap();
let db_weight: frame_support::weights::RuntimeDbWeight = <Self as frame_system::Config>::DbWeight::get();
(list, storage_info, max_extrinsic_weight, db_weight)
}

fn dispatch_benchmark(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1226,6 +1226,8 @@ impl_runtime_apis! {
fn benchmark_metadata(extra: bool) -> (
Vec<frame_benchmarking::BenchmarkList>,
Vec<frame_support::traits::StorageInfo>,
Weight,
frame_support::weights::RuntimeDbWeight,
) {
use frame_benchmarking::{Benchmarking, BenchmarkList};
use frame_support::traits::StorageInfoTrait;
Expand All @@ -1250,7 +1252,9 @@ impl_runtime_apis! {
list_benchmarks!(list, extra);

let storage_info = AllPalletsWithSystem::storage_info();
(list, storage_info)
let max_extrinsic_weight = RuntimeBlockWeights::get().per_class.get(DispatchClass::Normal).max_extrinsic.unwrap();
let db_weight: frame_support::weights::RuntimeDbWeight = <Self as frame_system::Config>::DbWeight::get();
(list, storage_info, max_extrinsic_weight, db_weight)
}

fn dispatch_benchmark(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -631,6 +631,8 @@ impl_runtime_apis! {
fn benchmark_metadata(extra: bool) -> (
Vec<frame_benchmarking::BenchmarkList>,
Vec<frame_support::traits::StorageInfo>,
Weight,
frame_support::weights::RuntimeDbWeight,
) {
use frame_benchmarking::{Benchmarking, BenchmarkList};
use frame_support::traits::StorageInfoTrait;
Expand All @@ -647,7 +649,9 @@ impl_runtime_apis! {
list_benchmarks!(list, extra);

let storage_info = AllPalletsWithSystem::storage_info();
(list, storage_info)
let max_extrinsic_weight = RuntimeBlockWeights::get().per_class.get(DispatchClass::Normal).max_extrinsic.unwrap();
let db_weight: frame_support::weights::RuntimeDbWeight = <Self as frame_system::Config>::DbWeight::get();
(list, storage_info, max_extrinsic_weight, db_weight)
}

fn dispatch_benchmark(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -631,6 +631,8 @@ impl_runtime_apis! {
fn benchmark_metadata(extra: bool) -> (
Vec<frame_benchmarking::BenchmarkList>,
Vec<frame_support::traits::StorageInfo>,
Weight,
frame_support::weights::RuntimeDbWeight,
) {
use frame_benchmarking::{Benchmarking, BenchmarkList};
use frame_support::traits::StorageInfoTrait;
Expand All @@ -647,7 +649,9 @@ impl_runtime_apis! {
list_benchmarks!(list, extra);

let storage_info = AllPalletsWithSystem::storage_info();
(list, storage_info)
let max_extrinsic_weight = RuntimeBlockWeights::get().per_class.get(DispatchClass::Normal).max_extrinsic.unwrap();
let db_weight: frame_support::weights::RuntimeDbWeight = <Self as frame_system::Config>::DbWeight::get();
(list, storage_info, max_extrinsic_weight, db_weight)
}

fn dispatch_benchmark(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -811,6 +811,8 @@ impl_runtime_apis! {
fn benchmark_metadata(extra: bool) -> (
Vec<frame_benchmarking::BenchmarkList>,
Vec<frame_support::traits::StorageInfo>,
Weight,
frame_support::weights::RuntimeDbWeight,
) {
use frame_benchmarking::{Benchmarking, BenchmarkList};
use frame_support::traits::StorageInfoTrait;
Expand All @@ -836,7 +838,9 @@ impl_runtime_apis! {
list_benchmarks!(list, extra);

let storage_info = AllPalletsWithSystem::storage_info();
(list, storage_info)
let max_extrinsic_weight = RuntimeBlockWeights::get().per_class.get(DispatchClass::Normal).max_extrinsic.unwrap();
let db_weight: frame_support::weights::RuntimeDbWeight = <Self as frame_system::Config>::DbWeight::get();
(list, storage_info, max_extrinsic_weight, db_weight)
}

fn dispatch_benchmark(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -902,6 +902,8 @@ impl_runtime_apis! {
fn benchmark_metadata(extra: bool) -> (
Vec<frame_benchmarking::BenchmarkList>,
Vec<frame_support::traits::StorageInfo>,
Weight,
frame_support::weights::RuntimeDbWeight,
) {
use frame_benchmarking::{Benchmarking, BenchmarkList};
use frame_support::traits::StorageInfoTrait;
Expand All @@ -912,7 +914,9 @@ impl_runtime_apis! {
list_benchmarks!(list, extra);

let storage_info = AllPalletsWithSystem::storage_info();
(list, storage_info)
let max_extrinsic_weight = RuntimeBlockWeights::get().per_class.get(DispatchClass::Normal).max_extrinsic.unwrap();
let db_weight: frame_support::weights::RuntimeDbWeight = <Self as frame_system::Config>::DbWeight::get();
(list, storage_info, max_extrinsic_weight, db_weight)
}

fn dispatch_benchmark(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -645,6 +645,8 @@ impl_runtime_apis! {
fn benchmark_metadata(extra: bool) -> (
Vec<frame_benchmarking::BenchmarkList>,
Vec<frame_support::traits::StorageInfo>,
Weight,
frame_support::weights::RuntimeDbWeight,
) {
use frame_benchmarking::{Benchmarking, BenchmarkList};
use frame_support::traits::StorageInfoTrait;
Expand All @@ -655,7 +657,9 @@ impl_runtime_apis! {
list_benchmarks!(list, extra);

let storage_info = AllPalletsWithSystem::storage_info();
(list, storage_info)
let max_extrinsic_weight = RuntimeBlockWeights::get().per_class.get(DispatchClass::Normal).max_extrinsic.unwrap();
let db_weight: frame_support::weights::RuntimeDbWeight = <Self as frame_system::Config>::DbWeight::get();
(list, storage_info, max_extrinsic_weight, db_weight)
}

fn dispatch_benchmark(
Expand Down
8 changes: 6 additions & 2 deletions cumulus/parachains/runtimes/glutton/glutton-kusama/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -437,17 +437,21 @@ impl_runtime_apis! {
fn benchmark_metadata(extra: bool) -> (
Vec<frame_benchmarking::BenchmarkList>,
Vec<frame_support::traits::StorageInfo>,
Weight,
frame_support::weights::RuntimeDbWeight,
) {
use frame_benchmarking::{Benchmarking, BenchmarkList};
use frame_support::traits::StorageInfoTrait;
use frame_system_benchmarking::Pallet as SystemBench;
use sp_core::Get;

let mut list = Vec::<BenchmarkList>::new();
list_benchmarks!(list, extra);

let storage_info = AllPalletsWithSystem::storage_info();

(list, storage_info)
let max_extrinsic_weight = RuntimeBlockWeights::get().per_class.get(DispatchClass::Normal).max_extrinsic.unwrap();
let db_weight: frame_support::weights::RuntimeDbWeight = <Self as frame_system::Config>::DbWeight::get();
(list, storage_info, max_extrinsic_weight, db_weight)
}

fn dispatch_benchmark(
Expand Down
6 changes: 5 additions & 1 deletion cumulus/parachains/runtimes/testing/penpal/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -790,6 +790,8 @@ impl_runtime_apis! {
fn benchmark_metadata(extra: bool) -> (
Vec<frame_benchmarking::BenchmarkList>,
Vec<frame_support::traits::StorageInfo>,
Weight,
frame_support::weights::RuntimeDbWeight,
) {
use frame_benchmarking::{Benchmarking, BenchmarkList};
use frame_support::traits::StorageInfoTrait;
Expand All @@ -800,7 +802,9 @@ impl_runtime_apis! {
list_benchmarks!(list, extra);

let storage_info = AllPalletsWithSystem::storage_info();
(list, storage_info)
let max_extrinsic_weight = RuntimeBlockWeights::get().per_class.get(DispatchClass::Normal).max_extrinsic.unwrap();
let db_weight: frame_support::weights::RuntimeDbWeight = <Self as frame_system::Config>::DbWeight::get();
(list, storage_info, max_extrinsic_weight, db_weight)
}

fn dispatch_benchmark(
Expand Down
6 changes: 5 additions & 1 deletion polkadot/runtime/rococo/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2041,6 +2041,8 @@ sp_api::impl_runtime_apis! {
fn benchmark_metadata(extra: bool) -> (
Vec<frame_benchmarking::BenchmarkList>,
Vec<frame_support::traits::StorageInfo>,
Weight,
frame_support::weights::RuntimeDbWeight,
) {
use frame_benchmarking::{Benchmarking, BenchmarkList};
use frame_support::traits::StorageInfoTrait;
Expand All @@ -2052,7 +2054,9 @@ sp_api::impl_runtime_apis! {
list_benchmarks!(list, extra);

let storage_info = AllPalletsWithSystem::storage_info();
return (list, storage_info)
let max_extrinsic_weight = BlockWeights::get().per_class.get(DispatchClass::Normal).max_extrinsic.unwrap();
let db_weight: frame_support::weights::RuntimeDbWeight = <Self as frame_system::Config>::DbWeight::get();
(list, storage_info, max_extrinsic_weight, db_weight)
Copy link
Member

Choose a reason for hiding this comment

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

I hoped that we could somehow around doing a breaking change for all runtimes, but it seems unavoidable, unless we want to squeeze this into into any of the vectors.
Maybe you can create a single struct that holds the Weight and RuntimeDbWeight. Then we can extend that in the future and avoid more breaking changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Creating a single struct for Weight and RuntimeDbWeight is either way a better solution. Would you prefer to have it included in BenchmarkList / StorageInfo and make it backwards compatible or add an additional parameter which results in a breaking change?

Copy link
Member

@ggwpez ggwpez Feb 5, 2024

Choose a reason for hiding this comment

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

I think it would need one breaking change, yes. But from then on we can avoid further breaking changes by using the following pattern:

struct AllBenchmarkData { // not a good name
  list: Vec<frame_benchmarking::BenchmarkList>,
  info: Vec<frame_support::traits::StorageInfo>,
  max_ext_weight: Option<..>,
  db_weights: Option<..>
}

and having Default on that thing. The runtime devs can use ..Default::default() to stay forward compatible and we can keep adding Options without directly breaking their code.

}

fn dispatch_benchmark(
Expand Down
6 changes: 5 additions & 1 deletion polkadot/runtime/westend/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2124,6 +2124,8 @@ sp_api::impl_runtime_apis! {
fn benchmark_metadata(extra: bool) -> (
Vec<frame_benchmarking::BenchmarkList>,
Vec<frame_support::traits::StorageInfo>,
Weight,
frame_support::weights::RuntimeDbWeight,
) {
use frame_benchmarking::{Benchmarking, BenchmarkList};
use frame_support::traits::StorageInfoTrait;
Expand All @@ -2141,7 +2143,9 @@ sp_api::impl_runtime_apis! {
list_benchmarks!(list, extra);

let storage_info = AllPalletsWithSystem::storage_info();
return (list, storage_info)
let max_extrinsic_weight = BlockWeights::get().per_class.get(DispatchClass::Normal).max_extrinsic.unwrap();
let db_weight: frame_support::weights::RuntimeDbWeight = <Self as frame_system::Config>::DbWeight::get();
(list, storage_info, max_extrinsic_weight, db_weight)
}

fn dispatch_benchmark(
Expand Down
11 changes: 8 additions & 3 deletions substrate/bin/node-template/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@ use sp_version::RuntimeVersion;
use frame_support::genesis_builder_helper::{build_config, create_default_config};
// A few exports that help ease life for downstream crates.
pub use frame_support::{
construct_runtime, parameter_types,
construct_runtime,
dispatch::DispatchClass,
parameter_types,
traits::{
ConstBool, ConstU128, ConstU32, ConstU64, ConstU8, KeyOwnerProofSystem, Randomness,
StorageInfo,
Expand Down Expand Up @@ -516,6 +518,8 @@ impl_runtime_apis! {
fn benchmark_metadata(extra: bool) -> (
Vec<frame_benchmarking::BenchmarkList>,
Vec<frame_support::traits::StorageInfo>,
Weight,
frame_support::weights::RuntimeDbWeight,
) {
use frame_benchmarking::{baseline, Benchmarking, BenchmarkList};
use frame_support::traits::StorageInfoTrait;
Expand All @@ -526,8 +530,9 @@ impl_runtime_apis! {
list_benchmarks!(list, extra);

let storage_info = AllPalletsWithSystem::storage_info();

(list, storage_info)
let max_extrinsic_weight = BlockWeights::get().per_class.get(DispatchClass::Normal).max_extrinsic.unwrap();
let db_weight: frame_support::weights::RuntimeDbWeight = <Self as frame_system::Config>::DbWeight::get();
(list, storage_info, max_extrinsic_weight, db_weight)
}

fn dispatch_benchmark(
Expand Down
6 changes: 5 additions & 1 deletion substrate/bin/node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2748,6 +2748,8 @@ impl_runtime_apis! {
fn benchmark_metadata(extra: bool) -> (
Vec<frame_benchmarking::BenchmarkList>,
Vec<frame_support::traits::StorageInfo>,
Weight,
frame_support::weights::RuntimeDbWeight,
) {
use frame_benchmarking::{baseline, Benchmarking, BenchmarkList};
use frame_support::traits::StorageInfoTrait;
Expand All @@ -2766,8 +2768,10 @@ impl_runtime_apis! {
list_benchmarks!(list, extra);

let storage_info = AllPalletsWithSystem::storage_info();
let max_extrinsic_weight = RuntimeBlockWeights::get().per_class.get(DispatchClass::Normal).max_extrinsic.unwrap();
let db_weight: frame_support::weights::RuntimeDbWeight = <Self as frame_system::Config>::DbWeight::get();

(list, storage_info)
(list, storage_info, max_extrinsic_weight, db_weight)
}

fn dispatch_benchmark(
Expand Down
13 changes: 13 additions & 0 deletions substrate/client/cli/src/arg_enums.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,19 @@ impl std::fmt::Display for WasmExecutionMethod {
}
}

/// How to output the result of the sanity weight check.
#[allow(missing_docs)]
#[derive(Debug, Clone, Copy, ValueEnum, PartialEq)]
#[value(rename_all = "kebab-case")]
pub enum SanityWeightCheck {
Error,
Warning,
Ignore,
Daanvdplas marked this conversation as resolved.
Show resolved Hide resolved
}

/// The default [`SanityWeightCheck`].
pub const DEFAULT_SANITY_WEIGHT_CHECK: SanityWeightCheck = SanityWeightCheck::Error;
ggwpez marked this conversation as resolved.
Show resolved Hide resolved

/// Converts the execution method and instantiation strategy command line arguments
/// into an execution method which can be used internally.
pub fn execution_method_from_cli(
Expand Down
Loading
Loading