Skip to content

Commit

Permalink
Fix HeadersToKeep and MaxBridgedAuthorities in Millau benchmarks (p…
Browse files Browse the repository at this point in the history 
…aritytech#1851)

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

* typo

* impl review suggestion
  • Loading branch information
@svyatonik @serban300
svyatonik authored and serban300 committed Apr 9, 2024
1 parent ddb2da7 commit 5b85ebf
Show file tree
Hide file tree
Showing 6 changed files with 65 additions and 63 deletions.
22 changes: 2 additions & 20 deletions bridges/bin/millau/runtime/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
4 changes: 2 additions & 2 deletions bridges/bin/rialto-parachain/runtime/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions bridges/bin/rialto/runtime/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
33 changes: 18 additions & 15 deletions bridges/modules/grandpa/src/benchmarking.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Expand Down
21 changes: 19 additions & 2 deletions bridges/modules/grandpa/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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);
Expand Down Expand Up @@ -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;
Expand Down
44 changes: 22 additions & 22 deletions bridges/modules/grandpa/src/weights.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
///
Expand All @@ -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))
}
Expand All @@ -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)
///
Expand All @@ -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))
}
Expand Down

0 comments on commit 5b85ebf

Please sign in to comment.