From e746521651b53ce7cc7d8ad2ab6cea5fe6f45e2f Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Mon, 5 Jun 2023 10:06:53 +0300 Subject: [PATCH] Improve receive_messages_proof benchmarks accuracy (#2176) * Adjust messages pallet benchmarks * Address comment --- bridges/modules/messages/src/benchmarking.rs | 24 +-- bridges/modules/messages/src/weights.rs | 162 ++++++++++--------- bridges/modules/messages/src/weights_ext.rs | 33 +--- bridges/modules/xcm-bridge-hub/src/mock.rs | 14 +- 4 files changed, 108 insertions(+), 125 deletions(-) diff --git a/bridges/modules/messages/src/benchmarking.rs b/bridges/modules/messages/src/benchmarking.rs index 5006b55573b05..ee969d727ddc8 100644 --- a/bridges/modules/messages/src/benchmarking.rs +++ b/bridges/modules/messages/src/benchmarking.rs @@ -33,7 +33,10 @@ use codec::Decode; use frame_benchmarking::{account, v2::*}; use frame_support::weights::Weight; use frame_system::RawOrigin; -use sp_runtime::{traits::TrailingZeroInput, BoundedVec}; +use sp_runtime::{ + traits::{Get, TrailingZeroInput}, + BoundedVec, +}; use sp_std::{ops::RangeInclusive, prelude::*}; const SEED: u32 = 0; @@ -186,14 +189,17 @@ mod benchmarks { // Benchmarks that are used directly by the runtime calls weight formulae. // + fn max_msgs, I: 'static>() -> u32 { + T::MaxUnconfirmedMessagesAtInboundLane::get() as u32 - + ReceiveMessagesProofSetup::::LATEST_RECEIVED_NONCE as u32 + } + // Benchmark `receive_messages_proof` extrinsic with single minimal-weight message and following // conditions: // * proof does not include outbound lane state proof; // * inbound lane already has state, so it needs to be read and decoded; // * message is dispatched (reminder: dispatch weight should be minimal); // * message requires all heavy checks done by dispatcher. - // - // This is base benchmark for all other message delivery benchmarks. #[benchmark] fn receive_single_message_proof() { // setup code @@ -219,21 +225,16 @@ mod benchmarks { setup.check_last_nonce(); } - // Benchmark `receive_messages_proof` extrinsic with two minimal-weight messages and following + // Benchmark `receive_messages_proof` extrinsic with `n` minimal-weight messages and following // conditions: // * proof does not include outbound lane state proof; // * inbound lane already has state, so it needs to be read and decoded; // * message is dispatched (reminder: dispatch weight should be minimal); // * message requires all heavy checks done by dispatcher. - // - // The weight of single message delivery could be approximated as - // `weight(receive_two_messages_proof) - weight(receive_single_message_proof)`. - // This won't be super-accurate if message has non-zero dispatch weight, but estimation should - // be close enough to real weight. #[benchmark] - fn receive_two_messages_proof() { + fn receive_n_messages_proof(n: Linear<1, { max_msgs::() }>) { // setup code - let setup = ReceiveMessagesProofSetup::::new(2); + let setup = ReceiveMessagesProofSetup::::new(n); let (proof, dispatch_weight) = T::prepare_message_proof(MessageProofParams { lane: T::bench_lane_id(), message_nonces: setup.nonces(), @@ -489,6 +490,7 @@ mod benchmarks { // * inbound lane already has state, so it needs to be read and decoded; // * message is **SUCCESSFULLY** dispatched; // * message requires all heavy checks done by dispatcher. + // #[benchmark(extra)] #[benchmark] fn receive_single_message_n_bytes_proof_with_dispatch( /// Proof size in bytes diff --git a/bridges/modules/messages/src/weights.rs b/bridges/modules/messages/src/weights.rs index 39b372fd219a6..660a6d4aa9e43 100644 --- a/bridges/modules/messages/src/weights.rs +++ b/bridges/modules/messages/src/weights.rs @@ -51,7 +51,7 @@ use sp_std::marker::PhantomData; /// Weight functions needed for pallet_bridge_messages. pub trait WeightInfo { fn receive_single_message_proof() -> Weight; - fn receive_two_messages_proof() -> Weight; + fn receive_n_messages_proof(n: u32) -> Weight; fn receive_single_message_proof_with_outbound_lane_state() -> Weight; fn receive_single_message_n_kb_proof(n: u32) -> Weight; fn receive_delivery_proof_for_single_message() -> Weight; @@ -81,33 +81,39 @@ impl WeightInfo for BridgeWeight { /// 51655, mode: MaxEncodedLen) fn receive_single_message_proof() -> Weight { // Proof Size summary in bytes: - // Measured: `428` + // Measured: `490` // Estimated: `52645` - // Minimum execution time: 32_573 nanoseconds. - Weight::from_parts(35_227_000, 52645) + // Minimum execution time: 34_644 nanoseconds. + Weight::from_parts(36_135_000, 52645) .saturating_add(T::DbWeight::get().reads(3_u64)) .saturating_add(T::DbWeight::get().writes(1_u64)) } - /// Storage: BridgeUnknownMessages PalletOperatingMode (r:1 w:0) + /// Storage: BridgeRialtoMessages PalletOperatingMode (r:1 w:0) /// - /// Proof: BridgeUnknownMessages PalletOperatingMode (max_values: Some(1), max_size: Some(2), + /// Proof: BridgeRialtoMessages PalletOperatingMode (max_values: Some(1), max_size: Some(2), /// added: 497, mode: MaxEncodedLen) /// - /// Storage: BridgeUnknownGrandpa ImportedHeaders (r:1 w:0) + /// Storage: BridgeRialtoGrandpa ImportedHeaders (r:1 w:0) /// - /// Proof: BridgeUnknownGrandpa ImportedHeaders (max_values: Some(14400), max_size: Some(68), + /// Proof: BridgeRialtoGrandpa ImportedHeaders (max_values: Some(14400), max_size: Some(68), /// added: 2048, mode: MaxEncodedLen) /// - /// Storage: BridgeUnknownMessages InboundLanes (r:1 w:1) + /// Storage: BridgeRialtoMessages InboundLanes (r:1 w:1) /// - /// Proof: BridgeUnknownMessages InboundLanes (max_values: None, max_size: Some(49180), added: + /// Proof: BridgeRialtoMessages InboundLanes (max_values: None, max_size: Some(49180), added: /// 51655, mode: MaxEncodedLen) - fn receive_two_messages_proof() -> Weight { + /// + /// The range of component `n` is `[1, 1004]`. + /// + /// The range of component `n` is `[1, 1004]`. + fn receive_n_messages_proof(n: u32) -> Weight { // Proof Size summary in bytes: - // Measured: `428` + // Measured: `490` // Estimated: `52645` - // Minimum execution time: 43_726 nanoseconds. - Weight::from_parts(47_518_000, 52645) + // Minimum execution time: 35_330 nanoseconds. + Weight::from_parts(27_526_047, 52645) + // Standard Error: 2_681 + .saturating_add(Weight::from_parts(7_412_923, 0).saturating_mul(n.into())) .saturating_add(T::DbWeight::get().reads(3_u64)) .saturating_add(T::DbWeight::get().writes(1_u64)) } @@ -127,10 +133,10 @@ impl WeightInfo for BridgeWeight { /// 51655, mode: MaxEncodedLen) fn receive_single_message_proof_with_outbound_lane_state() -> Weight { // Proof Size summary in bytes: - // Measured: `428` + // Measured: `490` // Estimated: `52645` - // Minimum execution time: 40_091 nanoseconds. - Weight::from_parts(43_639_000, 52645) + // Minimum execution time: 41_123 nanoseconds. + Weight::from_parts(43_023_000, 52645) .saturating_add(T::DbWeight::get().reads(3_u64)) .saturating_add(T::DbWeight::get().writes(1_u64)) } @@ -146,20 +152,20 @@ impl WeightInfo for BridgeWeight { /// /// Storage: BridgeUnknownMessages InboundLanes (r:1 w:1) /// - /// Proof: BridgeRialtoParachainMessages InboundLanes (max_values: None, max_size: Some(49180), - /// added: 51655, mode: MaxEncodedLen) + /// Proof: BridgeUnknownMessages InboundLanes (max_values: None, max_size: Some(49180), added: + /// 51655, mode: MaxEncodedLen) /// /// The range of component `n` is `[1, 16]`. /// /// The range of component `n` is `[1, 16]`. fn receive_single_message_n_kb_proof(n: u32) -> Weight { // Proof Size summary in bytes: - // Measured: `428` + // Measured: `490` // Estimated: `52645` - // Minimum execution time: 34_578 nanoseconds. - Weight::from_parts(36_723_095, 52645) - // Standard Error: 8_197 - .saturating_add(Weight::from_parts(1_317_904, 0).saturating_mul(n.into())) + // Minimum execution time: 36_301 nanoseconds. + Weight::from_parts(37_103_459, 52645) + // Standard Error: 4_645 + .saturating_add(Weight::from_parts(1_172_720, 0).saturating_mul(n.into())) .saturating_add(T::DbWeight::get().reads(3_u64)) .saturating_add(T::DbWeight::get().writes(1_u64)) } @@ -184,10 +190,10 @@ impl WeightInfo for BridgeWeight { /// mode: MaxEncodedLen) fn receive_delivery_proof_for_single_message() -> Weight { // Proof Size summary in bytes: - // Measured: `453` + // Measured: `515` // Estimated: `3530` - // Minimum execution time: 32_635 nanoseconds. - Weight::from_parts(33_711_000, 3530) + // Minimum execution time: 33_941 nanoseconds. + Weight::from_parts(35_252_000, 3530) .saturating_add(T::DbWeight::get().reads(4_u64)) .saturating_add(T::DbWeight::get().writes(2_u64)) } @@ -212,10 +218,10 @@ impl WeightInfo for BridgeWeight { /// mode: MaxEncodedLen) fn receive_delivery_proof_for_two_messages_by_single_relayer() -> Weight { // Proof Size summary in bytes: - // Measured: `470` + // Measured: `532` // Estimated: `3530` - // Minimum execution time: 31_808 nanoseconds. - Weight::from_parts(33_071_000, 3530) + // Minimum execution time: 33_259 nanoseconds. + Weight::from_parts(34_558_000, 3530) .saturating_add(T::DbWeight::get().reads(4_u64)) .saturating_add(T::DbWeight::get().writes(2_u64)) } @@ -240,10 +246,10 @@ impl WeightInfo for BridgeWeight { /// mode: MaxEncodedLen) fn receive_delivery_proof_for_two_messages_by_two_relayers() -> Weight { // Proof Size summary in bytes: - // Measured: `470` + // Measured: `532` // Estimated: `6070` - // Minimum execution time: 34_171 nanoseconds. - Weight::from_parts(35_591_000, 6070) + // Minimum execution time: 35_199 nanoseconds. + Weight::from_parts(36_989_000, 6070) .saturating_add(T::DbWeight::get().reads(5_u64)) .saturating_add(T::DbWeight::get().writes(3_u64)) } @@ -265,12 +271,12 @@ impl WeightInfo for BridgeWeight { /// The range of component `n` is `[128, 2048]`. fn receive_single_message_n_bytes_proof_with_dispatch(n: u32) -> Weight { // Proof Size summary in bytes: - // Measured: `428` + // Measured: `490` // Estimated: `52645` - // Minimum execution time: 76_504 nanoseconds. - Weight::from_parts(75_331_522, 52645) - // Standard Error: 1_440 - .saturating_add(Weight::from_parts(302_158, 0).saturating_mul(n.into())) + // Minimum execution time: 75_228 nanoseconds. + Weight::from_parts(62_255_691, 52645) + // Standard Error: 2_005 + .saturating_add(Weight::from_parts(353_141, 0).saturating_mul(n.into())) .saturating_add(T::DbWeight::get().reads(3_u64)) .saturating_add(T::DbWeight::get().writes(1_u64)) } @@ -294,33 +300,39 @@ impl WeightInfo for () { /// 51655, mode: MaxEncodedLen) fn receive_single_message_proof() -> Weight { // Proof Size summary in bytes: - // Measured: `428` + // Measured: `490` // Estimated: `52645` - // Minimum execution time: 32_573 nanoseconds. - Weight::from_parts(35_227_000, 52645) + // Minimum execution time: 34_644 nanoseconds. + Weight::from_parts(36_135_000, 52645) .saturating_add(RocksDbWeight::get().reads(3_u64)) .saturating_add(RocksDbWeight::get().writes(1_u64)) } - /// Storage: BridgeUnknownMessages PalletOperatingMode (r:1 w:0) + /// Storage: BridgeRialtoMessages PalletOperatingMode (r:1 w:0) /// - /// Proof: BridgeUnknownMessages PalletOperatingMode (max_values: Some(1), max_size: Some(2), + /// Proof: BridgeRialtoMessages PalletOperatingMode (max_values: Some(1), max_size: Some(2), /// added: 497, mode: MaxEncodedLen) /// - /// Storage: BridgeUnknownGrandpa ImportedHeaders (r:1 w:0) + /// Storage: BridgeRialtoGrandpa ImportedHeaders (r:1 w:0) /// - /// Proof: BridgeUnknownGrandpa ImportedHeaders (max_values: Some(14400), max_size: Some(68), + /// Proof: BridgeRialtoGrandpa ImportedHeaders (max_values: Some(14400), max_size: Some(68), /// added: 2048, mode: MaxEncodedLen) /// - /// Storage: BridgeUnknownMessages InboundLanes (r:1 w:1) + /// Storage: BridgeRialtoMessages InboundLanes (r:1 w:1) /// - /// Proof: BridgeUnknownMessages InboundLanes (max_values: None, max_size: Some(49180), added: + /// Proof: BridgeRialtoMessages InboundLanes (max_values: None, max_size: Some(49180), added: /// 51655, mode: MaxEncodedLen) - fn receive_two_messages_proof() -> Weight { + /// + /// The range of component `n` is `[1, 1004]`. + /// + /// The range of component `n` is `[1, 1004]`. + fn receive_n_messages_proof(n: u32) -> Weight { // Proof Size summary in bytes: - // Measured: `428` + // Measured: `490` // Estimated: `52645` - // Minimum execution time: 43_726 nanoseconds. - Weight::from_parts(47_518_000, 52645) + // Minimum execution time: 35_330 nanoseconds. + Weight::from_parts(27_526_047, 52645) + // Standard Error: 2_681 + .saturating_add(Weight::from_parts(7_412_923, 0).saturating_mul(n.into())) .saturating_add(RocksDbWeight::get().reads(3_u64)) .saturating_add(RocksDbWeight::get().writes(1_u64)) } @@ -340,10 +352,10 @@ impl WeightInfo for () { /// 51655, mode: MaxEncodedLen) fn receive_single_message_proof_with_outbound_lane_state() -> Weight { // Proof Size summary in bytes: - // Measured: `428` + // Measured: `490` // Estimated: `52645` - // Minimum execution time: 40_091 nanoseconds. - Weight::from_parts(43_639_000, 52645) + // Minimum execution time: 41_123 nanoseconds. + Weight::from_parts(43_023_000, 52645) .saturating_add(RocksDbWeight::get().reads(3_u64)) .saturating_add(RocksDbWeight::get().writes(1_u64)) } @@ -359,20 +371,20 @@ impl WeightInfo for () { /// /// Storage: BridgeUnknownMessages InboundLanes (r:1 w:1) /// - /// Proof: BridgeRialtoParachainMessages InboundLanes (max_values: None, max_size: Some(49180), - /// added: 51655, mode: MaxEncodedLen) + /// Proof: BridgeRialtoMessages InboundLanes (max_values: None, max_size: Some(49180), added: + /// 51655, mode: MaxEncodedLen) /// /// The range of component `n` is `[1, 16]`. /// /// The range of component `n` is `[1, 16]`. fn receive_single_message_n_kb_proof(n: u32) -> Weight { // Proof Size summary in bytes: - // Measured: `428` + // Measured: `490` // Estimated: `52645` - // Minimum execution time: 34_578 nanoseconds. - Weight::from_parts(36_723_095, 52645) - // Standard Error: 8_197 - .saturating_add(Weight::from_parts(1_317_904, 0).saturating_mul(n.into())) + // Minimum execution time: 36_301 nanoseconds. + Weight::from_parts(37_103_459, 52645) + // Standard Error: 4_645 + .saturating_add(Weight::from_parts(1_172_720, 0).saturating_mul(n.into())) .saturating_add(RocksDbWeight::get().reads(3_u64)) .saturating_add(RocksDbWeight::get().writes(1_u64)) } @@ -397,10 +409,10 @@ impl WeightInfo for () { /// mode: MaxEncodedLen) fn receive_delivery_proof_for_single_message() -> Weight { // Proof Size summary in bytes: - // Measured: `453` + // Measured: `515` // Estimated: `3530` - // Minimum execution time: 32_635 nanoseconds. - Weight::from_parts(33_711_000, 3530) + // Minimum execution time: 33_941 nanoseconds. + Weight::from_parts(35_252_000, 3530) .saturating_add(RocksDbWeight::get().reads(4_u64)) .saturating_add(RocksDbWeight::get().writes(2_u64)) } @@ -425,10 +437,10 @@ impl WeightInfo for () { /// mode: MaxEncodedLen) fn receive_delivery_proof_for_two_messages_by_single_relayer() -> Weight { // Proof Size summary in bytes: - // Measured: `470` + // Measured: `532` // Estimated: `3530` - // Minimum execution time: 31_808 nanoseconds. - Weight::from_parts(33_071_000, 3530) + // Minimum execution time: 33_259 nanoseconds. + Weight::from_parts(34_558_000, 3530) .saturating_add(RocksDbWeight::get().reads(4_u64)) .saturating_add(RocksDbWeight::get().writes(2_u64)) } @@ -453,10 +465,10 @@ impl WeightInfo for () { /// mode: MaxEncodedLen) fn receive_delivery_proof_for_two_messages_by_two_relayers() -> Weight { // Proof Size summary in bytes: - // Measured: `470` + // Measured: `532` // Estimated: `6070` - // Minimum execution time: 34_171 nanoseconds. - Weight::from_parts(35_591_000, 6070) + // Minimum execution time: 35_199 nanoseconds. + Weight::from_parts(36_989_000, 6070) .saturating_add(RocksDbWeight::get().reads(5_u64)) .saturating_add(RocksDbWeight::get().writes(3_u64)) } @@ -478,12 +490,12 @@ impl WeightInfo for () { /// The range of component `n` is `[128, 2048]`. fn receive_single_message_n_bytes_proof_with_dispatch(n: u32) -> Weight { // Proof Size summary in bytes: - // Measured: `428` + // Measured: `490` // Estimated: `52645` - // Minimum execution time: 76_504 nanoseconds. - Weight::from_parts(75_331_522, 52645) - // Standard Error: 1_440 - .saturating_add(Weight::from_parts(302_158, 0).saturating_mul(n.into())) + // Minimum execution time: 75_228 nanoseconds. + Weight::from_parts(62_255_691, 52645) + // Standard Error: 2_005 + .saturating_add(Weight::from_parts(353_141, 0).saturating_mul(n.into())) .saturating_add(RocksDbWeight::get().reads(3_u64)) .saturating_add(RocksDbWeight::get().writes(1_u64)) } diff --git a/bridges/modules/messages/src/weights_ext.rs b/bridges/modules/messages/src/weights_ext.rs index 01058d19ccb94..3fc23b3ba7403 100644 --- a/bridges/modules/messages/src/weights_ext.rs +++ b/bridges/modules/messages/src/weights_ext.rs @@ -40,13 +40,6 @@ pub fn ensure_weights_are_correct() { // benchmarked using `MaxEncodedLen` approach and there are no components that cause additional // db reads - // verify `receive_messages_proof` weight components - assert_ne!(W::receive_messages_proof_overhead().ref_time(), 0); - assert_ne!(W::receive_messages_proof_overhead().proof_size(), 0); - // W::receive_messages_proof_messages_overhead(1).ref_time() may be zero because: - // the message processing code (`InboundLane::receive_message`) is minimal and may not be - // accounted by our benchmarks - assert_eq!(W::receive_messages_proof_messages_overhead(1).proof_size(), 0); // W::receive_messages_proof_outbound_lane_state_overhead().ref_time() may be zero because: // the outbound lane state processing code (`InboundLane::receive_state_update`) is minimal and // may not be accounted by our benchmarks @@ -297,13 +290,11 @@ pub trait WeightInfoExt: WeightInfo { dispatch_weight: Weight, ) -> Weight { // basic components of extrinsic weight - let transaction_overhead = Self::receive_messages_proof_overhead(); + let base_weight = Self::receive_n_messages_proof(messages_count); let transaction_overhead_from_runtime = Self::receive_messages_proof_overhead_from_runtime(); let outbound_state_delivery_weight = Self::receive_messages_proof_outbound_lane_state_overhead(); - let messages_delivery_weight = - Self::receive_messages_proof_messages_overhead(MessageNonce::from(messages_count)); let messages_dispatch_weight = dispatch_weight; // proof size overhead weight @@ -315,10 +306,9 @@ pub trait WeightInfoExt: WeightInfo { actual_proof_size.saturating_sub(expected_proof_size), ); - transaction_overhead + base_weight .saturating_add(transaction_overhead_from_runtime) .saturating_add(outbound_state_delivery_weight) - .saturating_add(messages_delivery_weight) .saturating_add(messages_dispatch_weight) .saturating_add(proof_size_overhead) } @@ -354,25 +344,6 @@ pub trait WeightInfoExt: WeightInfo { // Functions that are used by extrinsics weights formulas. - /// Returns weight overhead of message delivery transaction (`receive_messages_proof`). - fn receive_messages_proof_overhead() -> Weight { - let weight_of_two_messages_and_two_tx_overheads = - Self::receive_single_message_proof().saturating_mul(2); - let weight_of_two_messages_and_single_tx_overhead = Self::receive_two_messages_proof(); - weight_of_two_messages_and_two_tx_overheads - .saturating_sub(weight_of_two_messages_and_single_tx_overhead) - } - - /// Returns weight that needs to be accounted when receiving given a number of messages with - /// message delivery transaction (`receive_messages_proof`). - fn receive_messages_proof_messages_overhead(messages: MessageNonce) -> Weight { - let weight_of_two_messages_and_single_tx_overhead = Self::receive_two_messages_proof(); - let weight_of_single_message_and_single_tx_overhead = Self::receive_single_message_proof(); - weight_of_two_messages_and_single_tx_overhead - .saturating_sub(weight_of_single_message_and_single_tx_overhead) - .saturating_mul(messages as _) - } - /// Returns weight that needs to be accounted when message delivery transaction /// (`receive_messages_proof`) is carrying outbound lane state proof. fn receive_messages_proof_outbound_lane_state_overhead() -> Weight { diff --git a/bridges/modules/xcm-bridge-hub/src/mock.rs b/bridges/modules/xcm-bridge-hub/src/mock.rs index 20dd60c67ac3c..bfbcc4c31a843 100644 --- a/bridges/modules/xcm-bridge-hub/src/mock.rs +++ b/bridges/modules/xcm-bridge-hub/src/mock.rs @@ -107,24 +107,22 @@ impl pallet_bridge_messages::WeightInfo for TestMessagesWeights { fn receive_single_message_proof() -> Weight { Weight::zero() } - fn receive_single_message_proof_with_outbound_lane_state() -> Weight { + fn receive_n_messages_proof(_: u32) -> Weight { Weight::zero() } - fn receive_delivery_proof_for_single_message() -> Weight { + fn receive_single_message_proof_with_outbound_lane_state() -> Weight { Weight::zero() } - fn receive_delivery_proof_for_two_messages_by_single_relayer() -> Weight { + fn receive_single_message_n_kb_proof(_: u32) -> Weight { Weight::zero() } - fn receive_delivery_proof_for_two_messages_by_two_relayers() -> Weight { + fn receive_delivery_proof_for_single_message() -> Weight { Weight::zero() } - - fn receive_two_messages_proof() -> Weight { + fn receive_delivery_proof_for_two_messages_by_single_relayer() -> Weight { Weight::zero() } - - fn receive_single_message_n_kb_proof(_: u32) -> Weight { + fn receive_delivery_proof_for_two_messages_by_two_relayers() -> Weight { Weight::zero() }