From 47b7edde686b59560230e2e6c21cb8e5f8f3a56e Mon Sep 17 00:00:00 2001 From: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Date: Mon, 12 Jul 2021 16:35:57 +0200 Subject: [PATCH] Store election snapshot in a more memory-friendly way. (#9275) * Store election snapshot in a more memory-friendly way. * fix * re-order benchmarks * Update frame/election-provider-multi-phase/src/lib.rs Co-authored-by: Guillaume Thiolliere * cargo run --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_election_provider_multi_phase --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/election-provider-multi-phase/src/weights.rs --template=./.maintain/frame-weight-template.hbs * manually fix the weights * remove todo Co-authored-by: Guillaume Thiolliere Co-authored-by: Parity Bot --- .../src/benchmarking.rs | 24 +++++++-------- .../election-provider-multi-phase/src/lib.rs | 29 +++++++++++++++---- 2 files changed, 35 insertions(+), 18 deletions(-) diff --git a/substrate/frame/election-provider-multi-phase/src/benchmarking.rs b/substrate/frame/election-provider-multi-phase/src/benchmarking.rs index f73ead376d..6cf581135f 100644 --- a/substrate/frame/election-provider-multi-phase/src/benchmarking.rs +++ b/substrate/frame/election-provider-multi-phase/src/benchmarking.rs @@ -199,6 +199,18 @@ frame_benchmarking::benchmarks! { assert!(>::current_phase().is_unsigned()); } + on_initialize_open_unsigned_without_snapshot { + // need to assume signed phase was open before + >::on_initialize_open_signed().unwrap(); + assert!(>::snapshot().is_some()); + assert!(>::current_phase().is_signed()); + }: { + >::on_initialize_open_unsigned(false, true, 1u32.into()).unwrap(); + } verify { + assert!(>::snapshot().is_some()); + assert!(>::current_phase().is_unsigned()); + } + finalize_signed_phase_accept_solution { let receiver = account("receiver", 0, SEED); let initial_balance = T::Currency::minimum_balance() * 10u32.into(); @@ -232,18 +244,6 @@ frame_benchmarking::benchmarks! { assert_eq!(T::Currency::reserved_balance(&receiver), 0u32.into()); } - on_initialize_open_unsigned_without_snapshot { - // need to assume signed phase was open before - >::on_initialize_open_signed().unwrap(); - assert!(>::snapshot().is_some()); - assert!(>::current_phase().is_signed()); - }: { - >::on_initialize_open_unsigned(false, true, 1u32.into()).unwrap(); - } verify { - assert!(>::snapshot().is_some()); - assert!(>::current_phase().is_unsigned()); - } - // a call to `::elect` where we only return the queued solution. elect_queued { // number of votes in snapshot. diff --git a/substrate/frame/election-provider-multi-phase/src/lib.rs b/substrate/frame/election-provider-multi-phase/src/lib.rs index b41db2a42c..65a31e8ee9 100644 --- a/substrate/frame/election-provider-multi-phase/src/lib.rs +++ b/substrate/frame/election-provider-multi-phase/src/lib.rs @@ -327,7 +327,7 @@ impl BenchmarkingConfig for () { } /// Current phase of the pallet. -#[derive(PartialEq, Eq, Clone, Copy, Encode, Decode, RuntimeDebug)] +#[derive(PartialEq, Eq, Clone, Copy, Encode, Decode, Debug)] pub enum Phase { /// Nothing, the election is not happening. Off, @@ -402,7 +402,7 @@ pub enum FallbackStrategy { } /// The type of `Computation` that provided this election data. -#[derive(PartialEq, Eq, Clone, Copy, Encode, Decode, RuntimeDebug)] +#[derive(PartialEq, Eq, Clone, Copy, Encode, Decode, Debug)] pub enum ElectionCompute { /// Election was computed on-chain. OnChain, @@ -476,7 +476,7 @@ pub struct RoundSnapshot { /// This is stored automatically on-chain, and it contains the **size of the entire snapshot**. /// This is also used in dispatchables as weight witness data and should **only contain the size of /// the presented solution**, not the entire snapshot. -#[derive(PartialEq, Eq, Clone, Copy, Encode, Decode, RuntimeDebug, Default)] +#[derive(PartialEq, Eq, Clone, Copy, Encode, Decode, Debug, Default)] pub struct SolutionOrSnapshotSize { /// The length of voters. #[codec(compact)] @@ -1308,12 +1308,29 @@ impl Pallet { } // Only write snapshot if all existed. - >::put(SolutionOrSnapshotSize { + let metadata = SolutionOrSnapshotSize { voters: voters.len() as u32, targets: targets.len() as u32, - }); + }; + log!(debug, "creating a snapshot with metadata {:?}", metadata); + + >::put(metadata); >::put(desired_targets); - >::put(RoundSnapshot { voters, targets }); + + // instead of using storage APIs, we do a manual encoding into a fixed-size buffer. + // `encoded_size` encodes it without storing it anywhere, this should not cause any allocation. + let snapshot = RoundSnapshot { voters, targets }; + let size = snapshot.encoded_size(); + log!(info, "snapshot pre-calculated size {:?}", size); + let mut buffer = Vec::with_capacity(size); + snapshot.encode_to(&mut buffer); + + // do some checks. + debug_assert_eq!(buffer, snapshot.encode()); + // buffer should have not re-allocated since. + debug_assert!(buffer.len() == size && size == buffer.capacity()); + + sp_io::storage::set(&>::hashed_key(), &buffer); Ok(w1.saturating_add(w2).saturating_add(w3).saturating_add(T::DbWeight::get().writes(3))) }