Fix HeadersToKeep and MaxBridgedAuthorities in Millau benchmarks (#1851)

* fix `HeadersToKeep` and `MaxBridgedAuthorities` in Millau benchmarks

* typo

* impl review suggestion

Author: svyatonik

bridges/bin/millau/runtime/src/lib.rs

@@ -387,30 +387,12 @@ impl pallet_bridge_relayers::Config for Runtime {
 	type WeightInfo = ();
 }
 
-#[cfg(feature = "runtime-benchmarks")]
-parameter_types! {
-	/// Number of headers to keep in benchmarks.
-	///
-	/// In benchmarks we always populate with full number of `HeadersToKeep` to make sure that
-	/// pruning is taken into account.
-	///
-	/// Note: This is lower than regular value, to speed up benchmarking setup.
-	pub const HeadersToKeep: u32 = 1024;
-	/// Maximal number of authorities at Rialto.
-	///
-	/// In benchmarks we're using sets of up to `1024` authorities to prepare for possible
-	/// upgrades in the future and see if performance degrades when number of authorities
-	/// grow.
-	pub const MaxAuthoritiesAtRialto: u32 = pallet_bridge_grandpa::benchmarking::MAX_VALIDATOR_SET_SIZE;
-}
-
-#[cfg(not(feature = "runtime-benchmarks"))]
 parameter_types! {
 	/// Number of headers to keep.
 	///
 	/// Assuming the worst case of every header being finalized, we will keep headers at least for a
-	/// week.
-	pub const HeadersToKeep: u32 = 7 * bp_rialto::DAYS;
+	/// day.
+	pub const HeadersToKeep: u32 = bp_rialto::DAYS;
 	/// Maximal number of authorities at Rialto.
 	pub const MaxAuthoritiesAtRialto: u32 = bp_rialto::MAX_AUTHORITIES_COUNT;
 }

bridges/bin/rialto-parachain/runtime/src/lib.rs

@@ -533,8 +533,8 @@ parameter_types! {
 	/// Number of headers to keep.
 	///
 	/// Assuming the worst case of every header being finalized, we will keep headers at least for a
-	/// week.
-	pub const HeadersToKeep: u32 = 7 * bp_millau::DAYS as u32;
+	/// day.
+	pub const HeadersToKeep: u32 = bp_millau::DAYS as u32;
 
 	/// Maximal number of authorities at Millau.
 	pub const MaxAuthoritiesAtMillau: u32 = bp_millau::MAX_AUTHORITIES_COUNT;

bridges/bin/rialto/runtime/src/lib.rs

@@ -401,8 +401,8 @@ parameter_types! {
 	/// Number of headers to keep.
 	///
 	/// Assuming the worst case of every header being finalized, we will keep headers at least for a
-	/// week.
-	pub const HeadersToKeep: u32 = 7 * bp_rialto::DAYS;
+	/// day.
+	pub const HeadersToKeep: u32 = bp_rialto::DAYS;
 
 	/// Maximal number of authorities at Millau.
 	pub const MaxAuthoritiesAtMillau: u32 = bp_millau::MAX_AUTHORITIES_COUNT;

bridges/modules/grandpa/src/benchmarking.rs

@@ -53,26 +53,29 @@ use sp_finality_grandpa::AuthorityId;
 use sp_runtime::traits::Zero;
 use sp_std::vec::Vec;
 
-// The maximum number of vote ancestries to include in a justification.
-//
-// In practice this would be limited by the session length (number of blocks a single authority set
-// can produce) of a given chain.
+/// The maximum number of vote ancestries to include in a justification.
+///
+/// In practice this would be limited by the session length (number of blocks a single authority set
+/// can produce) of a given chain.
 const MAX_VOTE_ANCESTRIES: u32 = 1000;
 
-// The maximum number of pre-commits to include in a justification. In practice this scales with the
-// number of validators.
-pub const MAX_VALIDATOR_SET_SIZE: u32 = 1024;
-
-// `1..MAX_VALIDATOR_SET_SIZE` and `1..MAX_VOTE_ANCESTRIES` are too large && benchmarks are
-// running for almost 40m (steps=50, repeat=20) on a decent laptop, which is too much. Since
-// we're building linear function here, let's just select some limited subrange for benchmarking.
-const VALIDATOR_SET_SIZE_RANGE_BEGIN: u32 = MAX_VALIDATOR_SET_SIZE / 20;
-const VALIDATOR_SET_SIZE_RANGE_END: u32 =
-	VALIDATOR_SET_SIZE_RANGE_BEGIN + VALIDATOR_SET_SIZE_RANGE_BEGIN;
+// `1..MAX_VOTE_ANCESTRIES` is too large && benchmarks are running for almost 40m (steps=50,
+// repeat=20) on a decent laptop, which is too much. Since we're building linear function here,
+// let's just select some limited subrange for benchmarking.
 const MAX_VOTE_ANCESTRIES_RANGE_BEGIN: u32 = MAX_VOTE_ANCESTRIES / 20;
 const MAX_VOTE_ANCESTRIES_RANGE_END: u32 =
 	MAX_VOTE_ANCESTRIES_RANGE_BEGIN + MAX_VOTE_ANCESTRIES_RANGE_BEGIN;
 
+// the same with validators - if there are too much validators, let's run benchmarks on subrange
+fn validator_set_range_end<T: Config<I>, I: 'static>() -> u32 {
+	let max_bridged_authorities = T::MaxBridgedAuthorities::get();
+	if max_bridged_authorities > 128 {
+		sp_std::cmp::max(128, max_bridged_authorities / 5)
+	} else {
+		max_bridged_authorities
+	}
+}
+
 /// Returns number of first header to be imported.
 ///
 /// Since we bootstrap the pallet with `HeadersToKeep` already imported headers,
@@ -117,7 +120,7 @@ benchmarks_instance_pallet! {
 	// This is the "gold standard" benchmark for this extrinsic, and it's what should be used to
 	// annotate the weight in the pallet.
 	submit_finality_proof {
-		let p in VALIDATOR_SET_SIZE_RANGE_BEGIN..VALIDATOR_SET_SIZE_RANGE_END;
+		let p in 1 .. validator_set_range_end::<T, I>();
 		let v in MAX_VOTE_ANCESTRIES_RANGE_BEGIN..MAX_VOTE_ANCESTRIES_RANGE_END;
 		let caller: T::AccountId = whitelisted_caller();
 		let (header, justification) = prepare_benchmark_data::<T, I>(p, v);

bridges/modules/grandpa/src/lib.rs

@@ -104,6 +104,9 @@ pub mod pallet {
 		#[pallet::constant]
 		type MaxRequests: Get<u32>;
 
+		// Avoid using `HeadersToKeep` directly in the pallet code. Use `headers_to_keep` function
+		// instead.
+
 		/// Maximal number of finalized headers to keep in the storage.
 		///
 		/// The setting is there to prevent growing the on-chain state indefinitely. Note
@@ -464,6 +467,20 @@ pub mod pallet {
 		})?)
 	}
 
+	/// Return number of headers to keep in the runtime storage.
+	#[cfg(not(feature = "runtime-benchmarks"))]
+	pub(crate) fn headers_to_keep<T: Config<I>, I: 'static>() -> u32 {
+		T::HeadersToKeep::get()
+	}
+
+	/// Return number of headers to keep in the runtime storage.
+	#[cfg(feature = "runtime-benchmarks")]
+	pub(crate) fn headers_to_keep<T: Config<I>, I: 'static>() -> u32 {
+		// every db operation (significantly) slows down benchmarks, so let's keep as min as
+		// possible
+		2
+	}
+
 	/// Import a previously verified header to the storage.
 	///
 	/// Note this function solely takes care of updating the storage and pruning old entries,
@@ -479,7 +496,7 @@ pub mod pallet {
 		<ImportedHashes<T, I>>::insert(index, hash);
 
 		// Update ring buffer pointer and remove old header.
-		<ImportedHashesPointer<T, I>>::put((index + 1) % T::HeadersToKeep::get());
+		<ImportedHashesPointer<T, I>>::put((index + 1) % headers_to_keep::<T, I>());
 		if let Ok(hash) = pruning {
 			log::debug!(target: LOG_TARGET, "Pruning old header: {:?}.", hash);
 			<ImportedHeaders<T, I>>::remove(hash);
@@ -523,7 +540,7 @@ pub mod pallet {
 		init_params: super::InitializationData<BridgedHeader<T, I>>,
 	) {
 		let start_number = *init_params.header.number();
-		let end_number = start_number + T::HeadersToKeep::get().into();
+		let end_number = start_number + headers_to_keep::<T, I>().into();
 		initialize_bridge::<T, I>(init_params).expect("benchmarks are correct");
 
 		let mut number = start_number;

bridges/modules/grandpa/src/weights.rs

@@ -75,8 +75,8 @@ impl<T: frame_system::Config> WeightInfo for BridgeWeight<T> {
 	///
 	/// Storage: BridgeRialtoGrandpa CurrentAuthoritySet (r:1 w:0)
 	///
-	/// Proof: BridgeRialtoGrandpa CurrentAuthoritySet (max_values: Some(1), max_size: Some(40970),
-	/// added: 41465, mode: MaxEncodedLen)
+	/// Proof: BridgeRialtoGrandpa CurrentAuthoritySet (max_values: Some(1), max_size: Some(209),
+	/// added: 704, mode: MaxEncodedLen)
 	///
 	/// Storage: BridgeRialtoGrandpa ImportedHashesPointer (r:1 w:1)
 	///
@@ -93,19 +93,19 @@ impl<T: frame_system::Config> WeightInfo for BridgeWeight<T> {
 	/// Proof: BridgeRialtoGrandpa ImportedHeaders (max_values: None, max_size: Some(68), added:
 	/// 2543, mode: MaxEncodedLen)
 	///
-	/// The range of component `p` is `[51, 102]`.
+	/// The range of component `p` is `[1, 5]`.
 	///
 	/// The range of component `v` is `[50, 100]`.
 	fn submit_finality_proof(p: u32, v: u32) -> Weight {
 		// Proof Size summary in bytes:
-		//  Measured:  `2524 + p * (40 ±0)`
-		//  Estimated: `46001`
-		// Minimum execution time: 2_282_140 nanoseconds.
-		Weight::from_parts(142_496_714, 46001)
-			// Standard Error: 32_796
-			.saturating_add(Weight::from_ref_time(40_232_935).saturating_mul(p.into()))
-			// Standard Error: 33_574
-			.saturating_add(Weight::from_ref_time(1_185_407).saturating_mul(v.into()))
+		//  Measured:  `459 + p * (40 ±0)`
+		//  Estimated: `5240`
+		// Minimum execution time: 368_734 nanoseconds.
+		Weight::from_parts(64_214_587, 5240)
+			// Standard Error: 226_504
+			.saturating_add(Weight::from_ref_time(41_231_918).saturating_mul(p.into()))
+			// Standard Error: 20_667
+			.saturating_add(Weight::from_ref_time(2_770_962).saturating_mul(v.into()))
 			.saturating_add(T::DbWeight::get().reads(6_u64))
 			.saturating_add(T::DbWeight::get().writes(6_u64))
 	}
@@ -130,8 +130,8 @@ impl WeightInfo for () {
 	///
 	/// Storage: BridgeRialtoGrandpa CurrentAuthoritySet (r:1 w:0)
 	///
-	/// Proof: BridgeRialtoGrandpa CurrentAuthoritySet (max_values: Some(1), max_size: Some(40970),
-	/// added: 41465, mode: MaxEncodedLen)
+	/// Proof: BridgeRialtoGrandpa CurrentAuthoritySet (max_values: Some(1), max_size: Some(209),
+	/// added: 704, mode: MaxEncodedLen)
 	///
 	/// Storage: BridgeRialtoGrandpa ImportedHashesPointer (r:1 w:1)
 	///
@@ -148,19 +148,19 @@ impl WeightInfo for () {
 	/// Proof: BridgeRialtoGrandpa ImportedHeaders (max_values: None, max_size: Some(68), added:
 	/// 2543, mode: MaxEncodedLen)
 	///
-	/// The range of component `p` is `[51, 102]`.
+	/// The range of component `p` is `[1, 5]`.
 	///
 	/// The range of component `v` is `[50, 100]`.
 	fn submit_finality_proof(p: u32, v: u32) -> Weight {
 		// Proof Size summary in bytes:
-		//  Measured:  `2524 + p * (40 ±0)`
-		//  Estimated: `46001`
-		// Minimum execution time: 2_282_140 nanoseconds.
-		Weight::from_parts(142_496_714, 46001)
-			// Standard Error: 32_796
-			.saturating_add(Weight::from_ref_time(40_232_935).saturating_mul(p.into()))
-			// Standard Error: 33_574
-			.saturating_add(Weight::from_ref_time(1_185_407).saturating_mul(v.into()))
+		//  Measured:  `459 + p * (40 ±0)`
+		//  Estimated: `5240`
+		// Minimum execution time: 368_734 nanoseconds.
+		Weight::from_parts(64_214_587, 5240)
+			// Standard Error: 226_504
+			.saturating_add(Weight::from_ref_time(41_231_918).saturating_mul(p.into()))
+			// Standard Error: 20_667
+			.saturating_add(Weight::from_ref_time(2_770_962).saturating_mul(v.into()))
 			.saturating_add(RocksDbWeight::get().reads(6_u64))
 			.saturating_add(RocksDbWeight::get().writes(6_u64))
 	}