Skip to content

Commit

Permalink
Update submit finality proof weight formula (#981)
Browse files Browse the repository at this point in the history 
* updated weight formula for submit_finality_proof

* remove common prefix traces

* update docs

* single benchmark

* Re-generate weights.

* Update delivery transaction limits

Co-authored-by: Tomasz Drwięga <tomasz@parity.io>
Co-authored-by: Hernando Castano <hernando@hcastano.com>
  • Loading branch information
@svyatonik @tomusdrw @HCastano
3 people authored Jun 7, 2021
1 parent 69df513 commit 5a9862f
Show file tree
Hide file tree
Showing 9 changed files with 113 additions and 320 deletions.
231 changes: 35 additions & 196 deletions modules/grandpa/src/benchmarking.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,24 +38,18 @@
//!
//! Note that the worst case scenario here would be a justification where each validator has it's
//! own fork which is `SESSION_LENGTH` blocks long.
//!
//! As far as benchmarking results go, the only benchmark that should be used in
//! `pallet-bridge-grandpa` to annotate weights is the `submit_finality_proof` one. The others are
//! looking at the effects of specific code paths and do not actually reflect the overall worst case
//! scenario.
use crate::*;

use bp_test_utils::{
accounts, authority_list, make_justification_for_header, test_keyring, JustificationGeneratorParams, ALICE,
TEST_GRANDPA_ROUND, TEST_GRANDPA_SET_ID,
accounts, make_justification_for_header, JustificationGeneratorParams, TEST_GRANDPA_ROUND, TEST_GRANDPA_SET_ID,
};
use frame_benchmarking::{benchmarks_instance_pallet, whitelisted_caller};
use frame_support::traits::Get;
use frame_system::RawOrigin;
use sp_finality_grandpa::AuthorityId;
use sp_runtime::traits::Zero;
use sp_std::{vec, vec::Vec};
use sp_std::vec::Vec;

// The maximum number of vote ancestries to include in a justification.
//
Expand All @@ -75,124 +69,46 @@ fn header_number<T: Config<I>, I: 'static, N: From<u32>>() -> N {
(T::HeadersToKeep::get() + 1).into()
}

/// Prepare header and its justification to submit using `submit_finality_proof`.
fn prepare_benchmark_data<T: Config<I>, I: 'static>(
precommits: u32,
ancestors: u32,
) -> (BridgedHeader<T, I>, GrandpaJustification<BridgedHeader<T, I>>) {
let authority_list = accounts(precommits as u16)
.iter()
.map(|id| (AuthorityId::from(*id), 1))
.collect::<Vec<_>>();

let init_data = InitializationData {
header: bp_test_utils::test_header(Zero::zero()),
authority_list,
set_id: TEST_GRANDPA_SET_ID,
is_halted: false,
};

bootstrap_bridge::<T, I>(init_data);

let header: BridgedHeader<T, I> = bp_test_utils::test_header(header_number::<T, I, _>());
let params = JustificationGeneratorParams {
header: header.clone(),
round: TEST_GRANDPA_ROUND,
set_id: TEST_GRANDPA_SET_ID,
authorities: accounts(precommits as u16).iter().map(|k| (*k, 1)).collect::<Vec<_>>(),
ancestors,
forks: 1,
};
let justification = make_justification_for_header(params);
(header, justification)
}

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.
//
// The other benchmarks related to `submit_finality_proof` are looking at the effect of specific
// parameters and are there mostly for seeing how specific codepaths behave.
submit_finality_proof {
let v in 1..MAX_VOTE_ANCESTRIES;
let p in 1..MAX_VALIDATOR_SET_SIZE;

let caller: T::AccountId = whitelisted_caller();

let authority_list = accounts(p as u16)
.iter()
.map(|id| (AuthorityId::from(*id), 1))
.collect::<Vec<_>>();

let init_data = InitializationData {
header: bp_test_utils::test_header(Zero::zero()),
authority_list,
set_id: TEST_GRANDPA_SET_ID,
is_halted: false,
};

bootstrap_bridge::<T, I>(init_data);

let header: BridgedHeader<T, I> = bp_test_utils::test_header(header_number::<T, I, _>());
let params = JustificationGeneratorParams {
header: header.clone(),
round: TEST_GRANDPA_ROUND,
set_id: TEST_GRANDPA_SET_ID,
authorities: accounts(p as u16).iter().map(|k| (*k, 1)).collect::<Vec<_>>(),
votes: v,
forks: 1,
};

let justification = make_justification_for_header(params);

}: _(RawOrigin::Signed(caller), header, justification)
verify {
let header: BridgedHeader<T, I> = bp_test_utils::test_header(header_number::<T, I, _>());
let expected_hash = header.hash();

assert_eq!(<BestFinalized<T, I>>::get(), expected_hash);
assert!(<ImportedHeaders<T, I>>::contains_key(expected_hash));
}

// What we want to check here is the effect of vote ancestries on justification verification
// do this by varying the number of headers between `finality_target` and `header_of_chain`.
submit_finality_proof_on_single_fork {
let v in 1..MAX_VOTE_ANCESTRIES;

let caller: T::AccountId = whitelisted_caller();

let init_data = InitializationData {
header: bp_test_utils::test_header(Zero::zero()),
authority_list: authority_list(),
set_id: TEST_GRANDPA_SET_ID,
is_halted: false,
};

bootstrap_bridge::<T, I>(init_data);
let header: BridgedHeader<T, I> = bp_test_utils::test_header(header_number::<T, I, _>());

let params = JustificationGeneratorParams {
header: header.clone(),
round: TEST_GRANDPA_ROUND,
set_id: TEST_GRANDPA_SET_ID,
authorities: test_keyring(),
votes: v,
forks: 1,
};

let justification = make_justification_for_header(params);

}: submit_finality_proof(RawOrigin::Signed(caller), header, justification)
verify {
let header: BridgedHeader<T, I> = bp_test_utils::test_header(header_number::<T, I, _>());
let expected_hash = header.hash();

assert_eq!(<BestFinalized<T, I>>::get(), expected_hash);
assert!(<ImportedHeaders<T, I>>::contains_key(expected_hash));
}

// What we want to check here is the effect of many pre-commits on justification verification.
// We do this by creating many forks, whose head will be used as a signed pre-commit in the
// final justification.
submit_finality_proof_on_many_forks {
let p in 1..MAX_VALIDATOR_SET_SIZE;

let caller: T::AccountId = whitelisted_caller();

let authority_list = accounts(p as u16)
.iter()
.map(|id| (AuthorityId::from(*id), 1))
.collect::<Vec<_>>();

let init_data = InitializationData {
header: bp_test_utils::test_header(Zero::zero()),
authority_list,
set_id: TEST_GRANDPA_SET_ID,
is_halted: false,
};

bootstrap_bridge::<T, I>(init_data);
let header: BridgedHeader<T, I> = bp_test_utils::test_header(header_number::<T, I, _>());

let params = JustificationGeneratorParams {
header: header.clone(),
round: TEST_GRANDPA_ROUND,
set_id: TEST_GRANDPA_SET_ID,
authorities: accounts(p as u16).iter().map(|k| (*k, 1)).collect::<Vec<_>>(),
votes: p,
forks: p,
};

let justification = make_justification_for_header(params);

let (header, justification) = prepare_benchmark_data::<T, I>(p, v);
}: submit_finality_proof(RawOrigin::Signed(caller), header, justification)
verify {
let header: BridgedHeader<T, I> = bp_test_utils::test_header(header_number::<T, I, _>());
Expand All @@ -201,81 +117,4 @@ benchmarks_instance_pallet! {
assert_eq!(<BestFinalized<T, I>>::get(), expected_hash);
assert!(<ImportedHeaders<T, I>>::contains_key(expected_hash));
}

// Here we want to find out the overheaded of looking through consensus digests found in a
// header. As the number of logs in a header grows, how much more work do we require to look
// through them?
//
// Note that this should be the same for looking through scheduled changes and forces changes,
// which is why we only have one benchmark for this.
find_scheduled_change {
// Not really sure what a good bound for this is.
let n in 1..1000;

let mut logs = vec![];
for i in 0..n {
// We chose a non-consensus log on purpose since that way we have to look through all
// the logs in the header
logs.push(sp_runtime::DigestItem::Other(vec![]));
}

let mut header: BridgedHeader<T, I> = bp_test_utils::test_header(Zero::zero());
let digest = header.digest_mut();
*digest = sp_runtime::Digest {
logs,
};

}: {
crate::find_scheduled_change(&header)
}

// What we want to check here is how long it takes to read and write the authority set tracked
// by the pallet as the number of authorities grows.
read_write_authority_sets {
// The current max target number of validators on Polkadot/Kusama
let n in 1..1000;

let mut authorities = vec![];
for i in 0..n {
authorities.push((ALICE, 1));
}

let authority_set = bp_header_chain::AuthoritySet {
authorities: authorities.iter().map(|(id, w)| (AuthorityId::from(*id), *w)).collect(),
set_id: 0
};

<CurrentAuthoritySet<T, I>>::put(&authority_set);

}: {
let authority_set = <CurrentAuthoritySet<T, I>>::get();
<CurrentAuthoritySet<T, I>>::put(&authority_set);
}
}

#[cfg(test)]
mod tests {
use super::*;
use frame_support::assert_ok;

#[test]
fn finality_proof_is_valid() {
mock::run_test(|| {
assert_ok!(test_benchmark_submit_finality_proof::<mock::TestRuntime>());
});
}

#[test]
fn single_fork_finality_proof_is_valid() {
mock::run_test(|| {
assert_ok!(test_benchmark_submit_finality_proof_on_single_fork::<mock::TestRuntime>());
});
}

#[test]
fn multi_fork_finality_proof_is_valid() {
mock::run_test(|| {
assert_ok!(test_benchmark_submit_finality_proof_on_many_forks::<mock::TestRuntime>());
});
}
}
5 changes: 3 additions & 2 deletions modules/grandpa/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ use frame_support::{ensure, fail};
use frame_system::{ensure_signed, RawOrigin};
use sp_finality_grandpa::{ConsensusLog, GRANDPA_ENGINE_ID};
use sp_runtime::traits::{BadOrigin, Header as HeaderT, Zero};
use sp_std::convert::TryInto;

#[cfg(test)]
mod mock;
Expand Down Expand Up @@ -124,8 +125,8 @@ pub mod pallet {
/// If successful in verification, it will write the target header to the underlying storage
/// pallet.
#[pallet::weight(T::WeightInfo::submit_finality_proof(
justification.votes_ancestries.len() as u32,
justification.commit.precommits.len() as u32,
justification.commit.precommits.len().try_into().unwrap_or(u32::MAX),
justification.votes_ancestries.len().try_into().unwrap_or(u32::MAX),
))]
pub fn submit_finality_proof(
origin: OriginFor<T>,
Expand Down
62 changes: 8 additions & 54 deletions modules/grandpa/src/weights.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
//! Autogenerated weights for pallet_bridge_grandpa
//!
//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 3.0.0
//! DATE: 2021-04-21, STEPS: [50, ], REPEAT: 20
//! DATE: 2021-06-03, STEPS: [50, ], REPEAT: 20
//! LOW RANGE: [], HIGH RANGE: []
//! EXECUTION: Some(Wasm), WASM-EXECUTION: Compiled
//! CHAIN: Some("dev"), DB CACHE: 128
Expand Down Expand Up @@ -48,74 +48,28 @@ use sp_std::marker::PhantomData;

/// Weight functions needed for pallet_bridge_grandpa.
pub trait WeightInfo {
fn submit_finality_proof(v: u32, p: u32) -> Weight;
fn submit_finality_proof_on_single_fork(v: u32) -> Weight;
fn submit_finality_proof_on_many_forks(p: u32) -> Weight;
fn find_scheduled_change(n: u32) -> Weight;
fn read_write_authority_sets(n: u32) -> Weight;
fn submit_finality_proof(p: u32, v: u32) -> Weight;
}

/// Weights for pallet_bridge_grandpa using the Rialto node and recommended hardware.
pub struct RialtoWeight<T>(PhantomData<T>);
impl<T: frame_system::Config> WeightInfo for RialtoWeight<T> {
fn submit_finality_proof(v: u32, p: u32) -> Weight {
fn submit_finality_proof(p: u32, v: u32) -> Weight {
(0 as Weight)
.saturating_add((756_462_000 as Weight).saturating_mul(v as Weight))
.saturating_add((791_236_000 as Weight).saturating_mul(p as Weight))
.saturating_add((59_692_000 as Weight).saturating_mul(p as Weight))
.saturating_add((6_876_000 as Weight).saturating_mul(v as Weight))
.saturating_add(T::DbWeight::get().reads(7 as Weight))
.saturating_add(T::DbWeight::get().writes(6 as Weight))
}
fn submit_finality_proof_on_single_fork(v: u32) -> Weight {
(280_121_000 as Weight)
.saturating_add((14_098_000 as Weight).saturating_mul(v as Weight))
.saturating_add(T::DbWeight::get().reads(7 as Weight))
.saturating_add(T::DbWeight::get().writes(6 as Weight))
}
fn submit_finality_proof_on_many_forks(p: u32) -> Weight {
(10_370_940_000 as Weight)
.saturating_add((96_902_000 as Weight).saturating_mul(p as Weight))
.saturating_add(T::DbWeight::get().reads(7 as Weight))
.saturating_add(T::DbWeight::get().writes(6 as Weight))
}
fn find_scheduled_change(n: u32) -> Weight {
(479_000 as Weight).saturating_add((11_000 as Weight).saturating_mul(n as Weight))
}
fn read_write_authority_sets(n: u32) -> Weight {
(8_030_000 as Weight)
.saturating_add((232_000 as Weight).saturating_mul(n as Weight))
.saturating_add(T::DbWeight::get().reads(1 as Weight))
.saturating_add(T::DbWeight::get().writes(1 as Weight))
}
}

// For backwards compatibility and tests
impl WeightInfo for () {
fn submit_finality_proof(v: u32, p: u32) -> Weight {
fn submit_finality_proof(p: u32, v: u32) -> Weight {
(0 as Weight)
.saturating_add((756_462_000 as Weight).saturating_mul(v as Weight))
.saturating_add((791_236_000 as Weight).saturating_mul(p as Weight))
.saturating_add((59_692_000 as Weight).saturating_mul(p as Weight))
.saturating_add((6_876_000 as Weight).saturating_mul(v as Weight))
.saturating_add(RocksDbWeight::get().reads(7 as Weight))
.saturating_add(RocksDbWeight::get().writes(6 as Weight))
}
fn submit_finality_proof_on_single_fork(v: u32) -> Weight {
(280_121_000 as Weight)
.saturating_add((14_098_000 as Weight).saturating_mul(v as Weight))
.saturating_add(RocksDbWeight::get().reads(7 as Weight))
.saturating_add(RocksDbWeight::get().writes(6 as Weight))
}
fn submit_finality_proof_on_many_forks(p: u32) -> Weight {
(10_370_940_000 as Weight)
.saturating_add((96_902_000 as Weight).saturating_mul(p as Weight))
.saturating_add(RocksDbWeight::get().reads(7 as Weight))
.saturating_add(RocksDbWeight::get().writes(6 as Weight))
}
fn find_scheduled_change(n: u32) -> Weight {
(479_000 as Weight).saturating_add((11_000 as Weight).saturating_mul(n as Weight))
}
fn read_write_authority_sets(n: u32) -> Weight {
(8_030_000 as Weight)
.saturating_add((232_000 as Weight).saturating_mul(n as Weight))
.saturating_add(RocksDbWeight::get().reads(1 as Weight))
.saturating_add(RocksDbWeight::get().writes(1 as Weight))
}
}
Loading

0 comments on commit 5a9862f

Please sign in to comment.