From b81d0eaef16e18f55ed91e46db6c0769a0911834 Mon Sep 17 00:00:00 2001 From: Alexander Popiak Date: Fri, 31 Jul 2020 18:08:01 +0200 Subject: [PATCH 01/14] add benchmark for disapprove_proposal --- frame/collective/src/benchmarking.rs | 55 ++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/frame/collective/src/benchmarking.rs b/frame/collective/src/benchmarking.rs index b9558d8c8ce1c..85759e929008d 100644 --- a/frame/collective/src/benchmarking.rs +++ b/frame/collective/src/benchmarking.rs @@ -579,6 +579,54 @@ benchmarks_instance! { assert_eq!(Collective::::proposals().len(), (p - 1) as usize); assert_last_event::(RawEvent::Executed(last_hash, Err(DispatchError::BadOrigin)).into()); } + + disapprove_proposal { + let p in 1 .. T::MaxProposals::get(); + + let m = 3; + let b = MAX_BYTES; + let bytes_in_storage = b + size_of::() as u32; + + // Construct `members`. + let mut members = vec![]; + for i in 0 .. m - 1 { + let member = account("member", i, SEED); + members.push(member); + } + let caller: T::AccountId = account("caller", 0, SEED); + members.push(caller.clone()); + Collective::::set_members( + SystemOrigin::Root.into(), + members.clone(), + Some(caller.clone()), + MAX_MEMBERS, + )?; + + // Threshold is one less than total members so that two nays will disapprove the vote + let threshold = m - 1; + + // Add proposals + let mut last_hash = T::Hash::default(); + for i in 0 .. p { + // Proposals should be different so that different proposal hashes are generated + let proposal: T::Proposal = SystemCall::::remark(vec![i as u8; b as usize]).into(); + Collective::::propose( + SystemOrigin::Signed(caller.clone()).into(), + threshold, + Box::new(proposal.clone()), + bytes_in_storage, + )?; + last_hash = T::Hashing::hash_of(&proposal); + } + + System::::set_block_number(T::BlockNumber::max_value()); + assert_eq!(Collective::::proposals().len(), p as usize); + + }: _(SystemOrigin::Root, last_hash) + verify { + assert_eq!(Collective::::proposals().len(), (p - 1) as usize); + assert_last_event::(RawEvent::Disapproved(last_hash).into()); + } } #[cfg(test)] @@ -649,4 +697,11 @@ mod tests { assert_ok!(test_benchmark_close_approved::()); }); } + + #[test] + fn disapprove_proposal() { + new_test_ext().execute_with(|| { + assert_ok!(test_benchmark_disapprove_proposal::()); + }); + } } From a6822b03bdc8a8cfb7024055539851fb0561e2be Mon Sep 17 00:00:00 2001 From: Alexander Popiak Date: Fri, 31 Jul 2020 18:47:18 +0200 Subject: [PATCH 02/14] use generated WeightInfo for pallet-collective weights --- bin/node/runtime/src/lib.rs | 2 +- bin/node/runtime/src/weights/mod.rs | 1 + .../runtime/src/weights/pallet_collective.rs | 96 ++++++ frame/collective/src/lib.rs | 301 ++++++------------ 4 files changed, 197 insertions(+), 203 deletions(-) create mode 100644 bin/node/runtime/src/weights/pallet_collective.rs diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index acc1b07281834..d425977216ac3 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -510,7 +510,7 @@ impl pallet_collective::Trait for Runtime { type Event = Event; type MotionDuration = CouncilMotionDuration; type MaxProposals = CouncilMaxProposals; - type WeightInfo = (); + type WeightInfo = weights::pallet_collective::WeightInfo; } parameter_types! { diff --git a/bin/node/runtime/src/weights/mod.rs b/bin/node/runtime/src/weights/mod.rs index 70bae879ce05c..5bf198b067efe 100644 --- a/bin/node/runtime/src/weights/mod.rs +++ b/bin/node/runtime/src/weights/mod.rs @@ -16,4 +16,5 @@ //! A list of the different weight modules for our runtime. pub mod pallet_balances; +pub mod pallet_collective; pub mod pallet_democracy; diff --git a/bin/node/runtime/src/weights/pallet_collective.rs b/bin/node/runtime/src/weights/pallet_collective.rs new file mode 100644 index 0000000000000..1ac8132868616 --- /dev/null +++ b/bin/node/runtime/src/weights/pallet_collective.rs @@ -0,0 +1,96 @@ +// Copyright (C) 2020 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 2.0.0-rc5 + +use frame_support::weights::{Weight, constants::RocksDbWeight as DbWeight}; + +pub struct WeightInfo; +impl pallet_collective::WeightInfo for WeightInfo { + fn set_members(m: u32, n: u32, p: u32, ) -> Weight { + (0 as Weight) + .saturating_add((20267000 as Weight).saturating_mul(m as Weight)) + .saturating_add((176000 as Weight).saturating_mul(n as Weight)) + .saturating_add((26230000 as Weight).saturating_mul(p as Weight)) + .saturating_add(DbWeight::get().reads(2 as Weight)) + .saturating_add(DbWeight::get().reads((1 as Weight).saturating_mul(p as Weight))) + .saturating_add(DbWeight::get().writes(2 as Weight)) + .saturating_add(DbWeight::get().writes((1 as Weight).saturating_mul(p as Weight))) + } + fn execute(b: u32, m: u32, ) -> Weight { + (26278000 as Weight) + .saturating_add((111000 as Weight).saturating_mul(m as Weight)) + .saturating_add((4000 as Weight).saturating_mul(b as Weight)) + .saturating_add(DbWeight::get().reads(1 as Weight)) + } + fn propose_execute(b: u32, m: u32, ) -> Weight { + (31972000 as Weight) + .saturating_add((218000 as Weight).saturating_mul(m as Weight)) + .saturating_add((4000 as Weight).saturating_mul(b as Weight)) + .saturating_add(DbWeight::get().reads(2 as Weight)) + } + fn propose_proposed(b: u32, m: u32, p: u32, ) -> Weight { + (53435000 as Weight) + .saturating_add((158000 as Weight).saturating_mul(m as Weight)) + .saturating_add((476000 as Weight).saturating_mul(p as Weight)) + .saturating_add((2000 as Weight).saturating_mul(b as Weight)) + .saturating_add(DbWeight::get().reads(4 as Weight)) + .saturating_add(DbWeight::get().writes(4 as Weight)) + } + fn vote(m: u32, ) -> Weight { + (46165000 as Weight) + .saturating_add((264000 as Weight).saturating_mul(m as Weight)) + .saturating_add(DbWeight::get().reads(2 as Weight)) + .saturating_add(DbWeight::get().writes(1 as Weight)) + } + // WARNING! Some components were not used: ["b"] + fn close_early_disapproved(m: u32, p: u32, ) -> Weight { + (52896000 as Weight) + .saturating_add((242000 as Weight).saturating_mul(m as Weight)) + .saturating_add((450000 as Weight).saturating_mul(p as Weight)) + .saturating_add(DbWeight::get().reads(3 as Weight)) + .saturating_add(DbWeight::get().writes(3 as Weight)) + } + fn close_early_approved(b: u32, m: u32, p: u32, ) -> Weight { + (73235000 as Weight) + .saturating_add((247000 as Weight).saturating_mul(m as Weight)) + .saturating_add((456000 as Weight).saturating_mul(p as Weight)) + .saturating_add((1000 as Weight).saturating_mul(b as Weight)) + .saturating_add(DbWeight::get().reads(4 as Weight)) + .saturating_add(DbWeight::get().writes(3 as Weight)) + } + // WARNING! Some components were not used: ["b"] + fn close_disapproved(m: u32, p: u32, ) -> Weight { + (56171000 as Weight) + .saturating_add((250000 as Weight).saturating_mul(m as Weight)) + .saturating_add((462000 as Weight).saturating_mul(p as Weight)) + .saturating_add(DbWeight::get().reads(4 as Weight)) + .saturating_add(DbWeight::get().writes(3 as Weight)) + } + fn close_approved(b: u32, m: u32, p: u32, ) -> Weight { + (77594000 as Weight) + .saturating_add((246000 as Weight).saturating_mul(m as Weight)) + .saturating_add((453000 as Weight).saturating_mul(p as Weight)) + .saturating_add((2000 as Weight).saturating_mul(b as Weight)) + .saturating_add(DbWeight::get().reads(5 as Weight)) + .saturating_add(DbWeight::get().writes(3 as Weight)) + } + fn disapprove_proposal(p: u32, ) -> Weight { + (0 as Weight) + .saturating_add((490000 as Weight).saturating_mul(p as Weight)) + .saturating_add(DbWeight::get().reads(1 as Weight)) + .saturating_add(DbWeight::get().writes(3 as Weight)) + } +} diff --git a/frame/collective/src/lib.rs b/frame/collective/src/lib.rs index 1edd8c75b90b0..dcdc366d6edb9 100644 --- a/frame/collective/src/lib.rs +++ b/frame/collective/src/lib.rs @@ -78,26 +78,28 @@ pub const MAX_MEMBERS: MemberCount = 100; pub trait WeightInfo { fn set_members(m: u32, n: u32, p: u32, ) -> Weight; - fn execute(m: u32, b: u32, ) -> Weight; - fn propose_execute(m: u32, b: u32, ) -> Weight; - fn propose_proposed(m: u32, p: u32, b: u32, ) -> Weight; + fn execute(b: u32, m: u32, ) -> Weight; + fn propose_execute(b: u32, m: u32, ) -> Weight; + fn propose_proposed(b: u32, m: u32, p: u32, ) -> Weight; fn vote(m: u32, ) -> Weight; - fn close_early_disapproved(m: u32, p: u32, b: u32, ) -> Weight; - fn close_early_approved(m: u32, p: u32, b: u32, ) -> Weight; - fn close_disapproved(m: u32, p: u32, b: u32, ) -> Weight; - fn close_approved(m: u32, p: u32, b: u32, ) -> Weight; + fn close_early_disapproved(m: u32, p: u32, ) -> Weight; + fn close_early_approved(b: u32, m: u32, p: u32, ) -> Weight; + fn close_disapproved(m: u32, p: u32, ) -> Weight; + fn close_approved(b: u32, m: u32, p: u32, ) -> Weight; + fn disapprove_proposal(p: u32, ) -> Weight; } impl WeightInfo for () { fn set_members(_m: u32, _n: u32, _p: u32, ) -> Weight { 1_000_000_000 } - fn execute(_m: u32, _b: u32, ) -> Weight { 1_000_000_000 } - fn propose_execute(_m: u32, _b: u32, ) -> Weight { 1_000_000_000 } - fn propose_proposed(_m: u32, _p: u32, _b: u32, ) -> Weight { 1_000_000_000 } + fn execute(_b: u32, _m: u32, ) -> Weight { 1_000_000_000 } + fn propose_execute(_b: u32, _m: u32, ) -> Weight { 1_000_000_000 } + fn propose_proposed(_b: u32, _m: u32, _p: u32, ) -> Weight { 1_000_000_000 } fn vote(_m: u32, ) -> Weight { 1_000_000_000 } - fn close_early_disapproved(_m: u32, _p: u32, _b: u32, ) -> Weight { 1_000_000_000 } - fn close_early_approved(_m: u32, _p: u32, _b: u32, ) -> Weight { 1_000_000_000 } - fn close_disapproved(_m: u32, _p: u32, _b: u32, ) -> Weight { 1_000_000_000 } - fn close_approved(_m: u32, _p: u32, _b: u32, ) -> Weight { 1_000_000_000 } + fn close_early_disapproved(_m: u32, _p: u32, ) -> Weight { 1_000_000_000 } + fn close_early_approved(_b: u32, _m: u32, _p: u32, ) -> Weight { 1_000_000_000 } + fn close_disapproved(_m: u32, _p: u32, ) -> Weight { 1_000_000_000 } + fn close_approved(_b: u32, _m: u32, _p: u32, ) -> Weight { 1_000_000_000 } + fn disapprove_proposal(_p: u32, ) -> Weight { 1_000_000_000 } } pub trait Trait: frame_system::Trait { @@ -235,126 +237,21 @@ decl_error! { /// Functions for calcuating the weight of dispatchables. mod weight_for { - use frame_support::{traits::Get, weights::Weight}; - use super::{Trait, Instance}; - - /// Calculate the weight for `set_members`. - /// - /// Based on benchmark: - /// 0 + M * 20.47 + N * 0.109 + P * 26.29 µs (min squares analysis) - /// - /// Note: The complexity of `set_members` is quadratic (`O(MP + N)`), so the linear approximation - /// of the benchmark is not always permissible. It is here, though, because the linear approximation - /// covered the range of possible values and we estimate weight via the worst case (max paramter - /// values) before execution so we can be sure that we are only overestimating. - pub(crate) fn set_members, I: Instance>( - old_count: Weight, - new_count: Weight, - proposals: Weight, - ) -> Weight { - let db = T::DbWeight::get(); - db.reads_writes(1, 1) // mutate `Members` - .saturating_add(db.writes(1)) // set `Prime` - .saturating_add(db.reads(1)) // read `Proposals` - .saturating_add(db.reads_writes(proposals, proposals)) // update votes (`Voting`) - .saturating_add(old_count.saturating_mul(21_000_000)) // M - .saturating_add(new_count.saturating_mul(110_000)) // N - .saturating_add(proposals.saturating_mul(27_000_000)) // P - } - - /// Calculate the weight for `execute`. - /// - /// Based on benchmark: - /// 22.62 + M * 0.115 + B * 0.003 µs (min squares analysis) - pub(crate) fn execute, I: Instance>( - members: Weight, - proposal: Weight, - length: Weight, - ) -> Weight { - T::DbWeight::get().reads(1) // read members for `is_member` - .saturating_add(23_000_000) // constant - .saturating_add(length.saturating_mul(4_000)) // B - .saturating_add(members.saturating_mul(120_000)) // M - .saturating_add(proposal) // P - } - - /// Calculate the weight for `propose` if the proposal is executed straight away (`threshold < 2`). - /// - /// Based on benchmark: - /// 28.12 + M * 0.218 + B * 0.003 µs (min squares analysis) - pub(crate) fn propose_execute, I: Instance>( - members: Weight, - proposal: Weight, - length: Weight, - ) -> Weight { - T::DbWeight::get().reads(2) // `is_member` + `contains_key` - .saturating_add(29_000_000) // constant - .saturating_add(length.saturating_mul(3_000)) // B - .saturating_add(members.saturating_mul(220_000)) // M - .saturating_add(proposal) // P1 - } - - /// Calculate the weight for `propose` if the proposal is put up for a vote (`threshold >= 2`). - /// - /// Based on benchmark: - /// 49.75 + M * 0.105 + P2 0.502 + B * 0.006 µs (min squares analysis) - pub(crate) fn propose_proposed, I: Instance>( - members: Weight, - proposals: Weight, - length: Weight, - ) -> Weight { - T::DbWeight::get().reads(2) // `is_member` + `contains_key` - .saturating_add(T::DbWeight::get().reads_writes(2, 4)) // `proposal` insertion - .saturating_add(50_000_000) // constant - .saturating_add(length.saturating_mul(6_000)) // B - .saturating_add(members.saturating_mul(110_000)) // M - .saturating_add(proposals.saturating_mul(510_000)) // P2 - } - - /// Calculate the weight for `vote`. - /// - /// Based on benchmark: - /// 24.03 + M * 0.349 + P * 0.119 + B * 0.003 µs (min squares analysis) - pub(crate) fn vote, I: Instance>( - members: Weight, - ) -> Weight { - T::DbWeight::get().reads(1) // read `Members` - .saturating_add(T::DbWeight::get().reads_writes(1, 1)) // mutate `Voting` - .saturating_add(30_000_000) // constant - .saturating_add(members.saturating_mul(500_000)) // M - } + use frame_support::weights::Weight; + use super::{Trait, Instance, WeightInfo}; /// Calculate the weight for `close`. - /// - /// Based on benchmarks: - /// - early disapproved: 37.21 + M * 0.239 + P2 * 0.466 + B * 0.002 µs (min squares analysis) - /// - early approved: 50.82 + M * 0.211 + P2 * 0.478 + B * 0.008 µs (min squares analysis) - /// - disapproved: 51.08 + M * 0.224 + P2 * 0.475 + B * 0.003 µs (min squares analysis) - /// - approved: 65.95 + M * 0.226 + P2 * 0.487 + B * 0.005 µs (min squares analysis) pub(crate) fn close, I: Instance>( - members: Weight, - proposal_weight: Weight, - proposals: Weight, - length: Weight, + length: u32, // B + members: u32, // M + proposal_weight: Weight, // P1 + proposals: u32, // P2 ) -> Weight { - let db = T::DbWeight::get(); - close_without_finalize::(members, length) - .saturating_add(db.reads(1)) // `Prime` - .saturating_add(db.writes(1)) // `Proposals` - .saturating_add(db.writes(1)) // `Voting` - .saturating_add(proposal_weight) // P1 - .saturating_add(proposals.saturating_mul(490_000)) // P2 - } - - /// Calculate the weight for `close` without the call to `approve/disapprove_proposal`. - pub(crate) fn close_without_finalize, I: Instance>( - members: Weight, - length: Weight, - ) -> Weight { - T::DbWeight::get().reads(3) // `Members`, `Voting`, `ProposalOf` - .saturating_add(66_000_000) // constant - .saturating_add(length.saturating_mul(8_000)) // B - .saturating_add(members.saturating_mul(250_000)) // M + T::WeightInfo::close_early_approved(length, members, proposals) + .max(T::WeightInfo::close_early_disapproved(members, proposals)) + .max(T::WeightInfo::close_approved(length, members, proposals)) + .max(T::WeightInfo::close_disapproved(members, proposals)) + .saturating_add(proposal_weight) } } @@ -401,10 +298,10 @@ decl_module! { /// - 1 storage write (codec `O(1)`) for deleting the old `prime` and setting the new one /// # #[weight = ( - weight_for::set_members::( - (*old_count).into(), // M - new_members.len() as Weight, // N - T::MaxProposals::get().into(), // P + T::WeightInfo::set_members( + *old_count, // M + new_members.len() as u32, // N + T::MaxProposals::get() // P ), DispatchClass::Operational )] @@ -435,10 +332,10 @@ decl_module! { >::set_members_sorted(&new_members, &old); Prime::::set(prime); - Ok(Some(weight_for::set_members::( - old.len() as Weight, // M - new_members.len() as Weight, // N - T::MaxProposals::get().into(), // P + Ok(Some(T::WeightInfo::set_members( + old.len() as u32, // M + new_members.len() as u32, // N + T::MaxProposals::get(), // P )).into()) } @@ -453,11 +350,10 @@ decl_module! { /// - 1 event /// # #[weight = ( - weight_for::execute::( - MAX_MEMBERS.into(), - proposal.get_dispatch_info().weight, - *length_bound as Weight, - ), + T::WeightInfo::execute( + *length_bound as u32, // B + MAX_MEMBERS, // M + ).saturating_add(proposal.get_dispatch_info().weight), // P DispatchClass::Operational )] fn execute(origin, @@ -476,11 +372,12 @@ decl_module! { RawEvent::MemberExecuted(proposal_hash, result.map(|_| ()).map_err(|e| e.error)) ); - Ok(get_result_weight(result).map(|w| weight_for::execute::( - members.len() as Weight, - w, - proposal_len as Weight - )).into()) + Ok(get_result_weight(result).map(|w| { + T::WeightInfo::execute( + proposal_len as u32, // B + members.len() as u32, // M + ).saturating_add(w) // P + }).into()) } /// Add a new proposal to either be voted on or executed directly. @@ -512,16 +409,15 @@ decl_module! { /// # #[weight = ( if *threshold < 2 { - weight_for::propose_execute::( - MAX_MEMBERS.into(), // M - proposal.get_dispatch_info().weight, // P1 - *length_bound as Weight, // B - ) + T::WeightInfo::propose_execute( + *length_bound, // B + MAX_MEMBERS, // M + ).saturating_add(proposal.get_dispatch_info().weight) // P1 } else { - weight_for::propose_proposed::( - MAX_MEMBERS.into(), // M - T::MaxProposals::get().into(), // P2 - *length_bound as Weight, // B + T::WeightInfo::propose_proposed( + *length_bound, // B + MAX_MEMBERS, // M + T::MaxProposals::get(), // P2 ) }, DispatchClass::Operational @@ -547,11 +443,12 @@ decl_module! { RawEvent::Executed(proposal_hash, result.map(|_| ()).map_err(|e| e.error)) ); - Ok(get_result_weight(result).map(|w| weight_for::propose_execute::( - members.len() as Weight, // M - w, // P1 - proposal_len as Weight, // B - )).into()) + Ok(get_result_weight(result).map(|w| { + T::WeightInfo::propose_execute( + proposal_len as u32, // B + members.len() as u32, // M + ).saturating_add(w) // P1 + }).into()) } else { let active_proposals = >::try_mutate(|proposals| -> Result { @@ -571,10 +468,10 @@ decl_module! { Self::deposit_event(RawEvent::Proposed(who, index, proposal_hash, threshold)); - Ok(Some(weight_for::propose_proposed::( - members.len() as Weight, // M - active_proposals as Weight, // P2 - proposal_len as Weight, // B + Ok(Some(T::WeightInfo::propose_proposed( + proposal_len as u32, // B + members.len() as u32, // M + active_proposals as u32, // P2 )).into()) } } @@ -591,8 +488,11 @@ decl_module! { /// - 1 storage mutation `Voting` (codec `O(M)`) /// - 1 event /// # + // NOTE: surprising difference between old manual weight and new generated weight: + // (49155000 as Weight) + // .saturating_add((295000 as Weight).saturating_mul(m as Weight)) #[weight = ( - weight_for::vote::(MAX_MEMBERS.into()), + T::WeightInfo::vote(MAX_MEMBERS), DispatchClass::Operational )] fn vote(origin, @@ -636,7 +536,7 @@ decl_module! { Voting::::insert(&proposal, voting); - Ok(Some(weight_for::vote::(members.len() as Weight)).into()) + Ok(Some(T::WeightInfo::vote(members.len() as u32)).into()) } /// Close a vote that is either approved, disapproved or whose voting period has ended. @@ -666,12 +566,17 @@ decl_module! { /// - any mutations done while executing `proposal` (`P1`) /// - up to 3 events /// # + // NOTE: surprising difference between old manual weight and new generated weight: + // early disapproved: 37.21 vs 47.412 + // early approved: 50.82 vs 69.466 and B * 0.008 vs 0.004 * b + // disapproved: B * 0.003 vs 0 * b + // approved: 65.95 vs 77.033 #[weight = ( weight_for::close::( - MAX_MEMBERS.into(), // `M` + *length_bound, // B + MAX_MEMBERS, // `M` *proposal_weight_bound, // `P1` - T::MaxProposals::get().into(), // `P2` - *length_bound as Weight, // B + T::MaxProposals::get(), // `P2` ), DispatchClass::Operational )] @@ -699,17 +604,17 @@ decl_module! { proposal_weight_bound )?; Self::deposit_event(RawEvent::Closed(proposal_hash, yes_votes, no_votes)); - let approve_weight = Self::do_approve_proposal(seats, voting, proposal_hash, proposal); + let (proposal_weight, proposal_count) = + Self::do_approve_proposal(seats, voting, proposal_hash, proposal); return Ok(Some( - weight_for::close_without_finalize::(seats.into(), len as Weight) - .saturating_add(approve_weight) + T::WeightInfo::close_early_approved(len as u32, seats, proposal_count) + .saturating_add(proposal_weight) ).into()); } else if disapproved { Self::deposit_event(RawEvent::Closed(proposal_hash, yes_votes, no_votes)); - let disapprove_weight = Self::do_disapprove_proposal(proposal_hash); + let proposal_count = Self::do_disapprove_proposal(proposal_hash); return Ok(Some( - weight_for::close_without_finalize::(seats.into(), 0) - .saturating_add(disapprove_weight) + T::WeightInfo::close_early_disapproved(seats, proposal_count) ).into()); } @@ -733,19 +638,17 @@ decl_module! { proposal_weight_bound )?; Self::deposit_event(RawEvent::Closed(proposal_hash, yes_votes, no_votes)); - let approve_weight = Self::do_approve_proposal(seats, voting, proposal_hash, proposal); + let (proposal_weight, proposal_count) = + Self::do_approve_proposal(seats, voting, proposal_hash, proposal); return Ok(Some( - weight_for::close_without_finalize::(seats.into(), len as Weight) - .saturating_add(T::DbWeight::get().reads(1)) // read `Prime` - .saturating_add(approve_weight) + T::WeightInfo::close_approved(len as u32, seats, proposal_count) + .saturating_add(proposal_weight) ).into()); } else { Self::deposit_event(RawEvent::Closed(proposal_hash, yes_votes, no_votes)); - let disapprove_weight = Self::do_disapprove_proposal(proposal_hash); + let proposal_count = Self::do_disapprove_proposal(proposal_hash); return Ok(Some( - weight_for::close_without_finalize::(seats.into(), 0) - .saturating_add(T::DbWeight::get().reads(1)) // read `Prime` - .saturating_add(disapprove_weight) + T::WeightInfo::close_disapproved(seats, proposal_count) ).into()); } } @@ -764,13 +667,11 @@ decl_module! { /// * Reads: Proposals /// * Writes: Voting, Proposals, ProposalOf /// # - #[weight = T::DbWeight::get().reads_writes(1, 3) // `Voting`, `Proposals`, `ProposalOf` - .saturating_add(490_000 * Weight::from(T::MaxProposals::get())) // P2 - ] + #[weight = T::WeightInfo::disapprove_proposal(T::MaxProposals::get())] fn disapprove_proposal(origin, proposal_hash: T::Hash) -> DispatchResultWithPostInfo { ensure_root(origin)?; - let actual_weight = Self::do_disapprove_proposal(proposal_hash); - Ok(Some(actual_weight).into()) + let proposal_count = Self::do_disapprove_proposal(proposal_hash); + Ok(Some(T::WeightInfo::disapprove_proposal(proposal_count)).into()) } } } @@ -822,8 +723,7 @@ impl, I: Instance> Module { voting: Votes, proposal_hash: T::Hash, proposal: >::Proposal, - ) -> Weight { - let mut weight: Weight = 0; + ) -> (Weight, u32) { Self::deposit_event(RawEvent::Approved(proposal_hash)); let dispatch_weight = proposal.get_dispatch_info().weight; @@ -832,23 +732,21 @@ impl, I: Instance> Module { Self::deposit_event( RawEvent::Executed(proposal_hash, result.map(|_| ()).map_err(|e| e.error)) ); - weight = weight.saturating_add( - // default to the dispatch info weight for safety - get_result_weight(result).unwrap_or(dispatch_weight) // P1 - ); + // default to the dispatch info weight for safety + let proposal_weight = get_result_weight(result).unwrap_or(dispatch_weight); // P1 - let remove_proposal_weight = Self::remove_proposal(proposal_hash); - weight.saturating_add(remove_proposal_weight) + let proposal_count = Self::remove_proposal(proposal_hash); + (proposal_weight, proposal_count) } - fn do_disapprove_proposal(proposal_hash: T::Hash) -> Weight { + fn do_disapprove_proposal(proposal_hash: T::Hash) -> u32 { // disapproved Self::deposit_event(RawEvent::Disapproved(proposal_hash)); Self::remove_proposal(proposal_hash) } // Removes a proposal from the pallet, cleaning up votes and the vector of proposals. - fn remove_proposal(proposal_hash: T::Hash) -> Weight { + fn remove_proposal(proposal_hash: T::Hash) -> u32 { // remove proposal and vote ProposalOf::::remove(&proposal_hash); Voting::::remove(&proposal_hash); @@ -856,8 +754,7 @@ impl, I: Instance> Module { proposals.retain(|h| h != &proposal_hash); proposals.len() + 1 // calculate weight based on original length }); - T::DbWeight::get().reads_writes(1, 3) // `Voting`, `Proposals`, `ProposalOf` - .saturating_add(490_000 * num_proposals as Weight) // P2 + num_proposals as u32 } } From 199a630b3e4053985ac3152bd1a0faec5362c57c Mon Sep 17 00:00:00 2001 From: Alexander Popiak Date: Mon, 3 Aug 2020 15:28:12 +0200 Subject: [PATCH 03/14] order collective benchmark params alphabetically to get a consistent ordering --- frame/collective/src/benchmarking.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/frame/collective/src/benchmarking.rs b/frame/collective/src/benchmarking.rs index 85759e929008d..dbef1bf7cbb0c 100644 --- a/frame/collective/src/benchmarking.rs +++ b/frame/collective/src/benchmarking.rs @@ -111,8 +111,8 @@ benchmarks_instance! { } execute { - let m in 1 .. MAX_MEMBERS; let b in 1 .. MAX_BYTES; + let m in 1 .. MAX_MEMBERS; let bytes_in_storage = b + size_of::() as u32; @@ -141,8 +141,8 @@ benchmarks_instance! { // This tests when execution would happen immediately after proposal propose_execute { - let m in 1 .. MAX_MEMBERS; let b in 1 .. MAX_BYTES; + let m in 1 .. MAX_MEMBERS; let bytes_in_storage = b + size_of::() as u32; @@ -172,9 +172,9 @@ benchmarks_instance! { // This tests when proposal is created and queued as "proposed" propose_proposed { + let b in 1 .. MAX_BYTES; let m in 2 .. MAX_MEMBERS; let p in 1 .. T::MaxProposals::get(); - let b in 1 .. MAX_BYTES; let bytes_in_storage = b + size_of::() as u32; @@ -287,10 +287,10 @@ benchmarks_instance! { } close_early_disapproved { + let b in 1 .. MAX_BYTES; // We choose 4 as a minimum so we always trigger a vote in the voting loop (`for j in ...`) let m in 4 .. MAX_MEMBERS; let p in 1 .. T::MaxProposals::get(); - let b in 1 .. MAX_BYTES; let bytes_in_storage = b + size_of::() as u32; @@ -364,10 +364,10 @@ benchmarks_instance! { } close_early_approved { + let b in 1 .. MAX_BYTES; // We choose 4 as a minimum so we always trigger a vote in the voting loop (`for j in ...`) let m in 4 .. MAX_MEMBERS; let p in 1 .. T::MaxProposals::get(); - let b in 1 .. MAX_BYTES; let bytes_in_storage = b + size_of::() as u32; @@ -445,10 +445,10 @@ benchmarks_instance! { } close_disapproved { + let b in 1 .. MAX_BYTES; // We choose 4 as a minimum so we always trigger a vote in the voting loop (`for j in ...`) let m in 4 .. MAX_MEMBERS; let p in 1 .. T::MaxProposals::get(); - let b in 1 .. MAX_BYTES; let bytes_in_storage = b + size_of::() as u32; @@ -517,10 +517,10 @@ benchmarks_instance! { } close_approved { + let b in 1 .. MAX_BYTES; // We choose 4 as a minimum so we always trigger a vote in the voting loop (`for j in ...`) let m in 4 .. MAX_MEMBERS; let p in 1 .. T::MaxProposals::get(); - let b in 1 .. MAX_BYTES; let bytes_in_storage = b + size_of::() as u32; From 768a363e9dbab775636051d2c001605f7ae81665 Mon Sep 17 00:00:00 2001 From: Alexander Popiak Date: Fri, 7 Aug 2020 10:56:02 +0200 Subject: [PATCH 04/14] address review comments --- frame/collective/src/benchmarking.rs | 12 ++++----- frame/collective/src/lib.rs | 37 +++++++++------------------- 2 files changed, 17 insertions(+), 32 deletions(-) diff --git a/frame/collective/src/benchmarking.rs b/frame/collective/src/benchmarking.rs index dbef1bf7cbb0c..1d3a4ca7799af 100644 --- a/frame/collective/src/benchmarking.rs +++ b/frame/collective/src/benchmarking.rs @@ -287,12 +287,12 @@ benchmarks_instance! { } close_early_disapproved { - let b in 1 .. MAX_BYTES; // We choose 4 as a minimum so we always trigger a vote in the voting loop (`for j in ...`) let m in 4 .. MAX_MEMBERS; let p in 1 .. T::MaxProposals::get(); - let bytes_in_storage = b + size_of::() as u32; + let bytes = 100; + let bytes_in_storage = bytes + size_of::() as u32; // Construct `members`. let mut members = vec![]; @@ -313,7 +313,7 @@ benchmarks_instance! { let mut last_hash = T::Hash::default(); for i in 0 .. p { // Proposals should be different so that different proposal hashes are generated - let proposal: T::Proposal = SystemCall::::remark(vec![i as u8; b as usize]).into(); + let proposal: T::Proposal = SystemCall::::remark(vec![i as u8; bytes as usize]).into(); Collective::::propose( SystemOrigin::Signed(proposer.clone()).into(), threshold, @@ -445,12 +445,12 @@ benchmarks_instance! { } close_disapproved { - let b in 1 .. MAX_BYTES; // We choose 4 as a minimum so we always trigger a vote in the voting loop (`for j in ...`) let m in 4 .. MAX_MEMBERS; let p in 1 .. T::MaxProposals::get(); - let bytes_in_storage = b + size_of::() as u32; + let bytes = 100; + let bytes_in_storage = bytes + size_of::() as u32; // Construct `members`. let mut members = vec![]; @@ -474,7 +474,7 @@ benchmarks_instance! { let mut last_hash = T::Hash::default(); for i in 0 .. p { // Proposals should be different so that different proposal hashes are generated - let proposal: T::Proposal = SystemCall::::remark(vec![i as u8; b as usize]).into(); + let proposal: T::Proposal = SystemCall::::remark(vec![i as u8; bytes as usize]).into(); Collective::::propose( SystemOrigin::Signed(caller.clone()).into(), threshold, diff --git a/frame/collective/src/lib.rs b/frame/collective/src/lib.rs index dcdc366d6edb9..6a8aef826b919 100644 --- a/frame/collective/src/lib.rs +++ b/frame/collective/src/lib.rs @@ -235,26 +235,6 @@ decl_error! { } } -/// Functions for calcuating the weight of dispatchables. -mod weight_for { - use frame_support::weights::Weight; - use super::{Trait, Instance, WeightInfo}; - - /// Calculate the weight for `close`. - pub(crate) fn close, I: Instance>( - length: u32, // B - members: u32, // M - proposal_weight: Weight, // P1 - proposals: u32, // P2 - ) -> Weight { - T::WeightInfo::close_early_approved(length, members, proposals) - .max(T::WeightInfo::close_early_disapproved(members, proposals)) - .max(T::WeightInfo::close_approved(length, members, proposals)) - .max(T::WeightInfo::close_disapproved(members, proposals)) - .saturating_add(proposal_weight) - } -} - /// Return the weight of a dispatch call result as an `Option`. /// /// Will return the weight regardless of what the state of the result is. @@ -572,12 +552,17 @@ decl_module! { // disapproved: B * 0.003 vs 0 * b // approved: 65.95 vs 77.033 #[weight = ( - weight_for::close::( - *length_bound, // B - MAX_MEMBERS, // `M` - *proposal_weight_bound, // `P1` - T::MaxProposals::get(), // `P2` - ), + { + let b = *length_bound; + let m = MAX_MEMBERS; + let p1 = *proposal_weight_bound; + let p2 = T::MaxProposals::get(); + T::WeightInfo::close_early_approved(b, m, p2) + .max(T::WeightInfo::close_early_disapproved(m, p2)) + .max(T::WeightInfo::close_approved(b, m, p2)) + .max(T::WeightInfo::close_disapproved(m, p2)) + .saturating_add(p1) + }, DispatchClass::Operational )] fn close(origin, From ed62a89efc549ec4f46d7563f90dea9334f9e17c Mon Sep 17 00:00:00 2001 From: Alexander Popiak Date: Fri, 7 Aug 2020 11:02:36 +0200 Subject: [PATCH 05/14] remove default impl of WeightInfo for () --- frame/collective/src/lib.rs | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/frame/collective/src/lib.rs b/frame/collective/src/lib.rs index 6a8aef826b919..166b5ab83b286 100644 --- a/frame/collective/src/lib.rs +++ b/frame/collective/src/lib.rs @@ -89,19 +89,6 @@ pub trait WeightInfo { fn disapprove_proposal(p: u32, ) -> Weight; } -impl WeightInfo for () { - fn set_members(_m: u32, _n: u32, _p: u32, ) -> Weight { 1_000_000_000 } - fn execute(_b: u32, _m: u32, ) -> Weight { 1_000_000_000 } - fn propose_execute(_b: u32, _m: u32, ) -> Weight { 1_000_000_000 } - fn propose_proposed(_b: u32, _m: u32, _p: u32, ) -> Weight { 1_000_000_000 } - fn vote(_m: u32, ) -> Weight { 1_000_000_000 } - fn close_early_disapproved(_m: u32, _p: u32, ) -> Weight { 1_000_000_000 } - fn close_early_approved(_b: u32, _m: u32, _p: u32, ) -> Weight { 1_000_000_000 } - fn close_disapproved(_m: u32, _p: u32, ) -> Weight { 1_000_000_000 } - fn close_approved(_b: u32, _m: u32, _p: u32, ) -> Weight { 1_000_000_000 } - fn disapprove_proposal(_p: u32, ) -> Weight { 1_000_000_000 } -} - pub trait Trait: frame_system::Trait { /// The outer origin type. type Origin: From>; From ec01d1fa2df5ce374b3711ed1e9dcb8633302b32 Mon Sep 17 00:00:00 2001 From: Alexander Popiak Date: Fri, 14 Aug 2020 17:02:48 +0200 Subject: [PATCH 06/14] remove comments about weight changes --- frame/collective/src/lib.rs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/frame/collective/src/lib.rs b/frame/collective/src/lib.rs index 166b5ab83b286..9ff1ba42f22bf 100644 --- a/frame/collective/src/lib.rs +++ b/frame/collective/src/lib.rs @@ -455,9 +455,6 @@ decl_module! { /// - 1 storage mutation `Voting` (codec `O(M)`) /// - 1 event /// # - // NOTE: surprising difference between old manual weight and new generated weight: - // (49155000 as Weight) - // .saturating_add((295000 as Weight).saturating_mul(m as Weight)) #[weight = ( T::WeightInfo::vote(MAX_MEMBERS), DispatchClass::Operational @@ -533,11 +530,6 @@ decl_module! { /// - any mutations done while executing `proposal` (`P1`) /// - up to 3 events /// # - // NOTE: surprising difference between old manual weight and new generated weight: - // early disapproved: 37.21 vs 47.412 - // early approved: 50.82 vs 69.466 and B * 0.008 vs 0.004 * b - // disapproved: B * 0.003 vs 0 * b - // approved: 65.95 vs 77.033 #[weight = ( { let b = *length_bound; From 1b8c73fe833dab4c94e461cc2f9cfd3733770a8a Mon Sep 17 00:00:00 2001 From: Alexander Popiak Date: Mon, 17 Aug 2020 15:24:43 +0200 Subject: [PATCH 07/14] add default weights --- frame/collective/src/default_weight.rs | 96 ++++++++++++++++++++++++++ frame/collective/src/lib.rs | 2 + 2 files changed, 98 insertions(+) create mode 100644 frame/collective/src/default_weight.rs diff --git a/frame/collective/src/default_weight.rs b/frame/collective/src/default_weight.rs new file mode 100644 index 0000000000000..06525a31ba03e --- /dev/null +++ b/frame/collective/src/default_weight.rs @@ -0,0 +1,96 @@ +// Copyright (C) 2020 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Default weights for the Collective Pallet +//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 2.0.0-rc5 + +use frame_support::weights::{Weight, constants::RocksDbWeight as DbWeight}; + +impl crate::WeightInfo for () { + fn set_members(m: u32, n: u32, p: u32, ) -> Weight { + (0 as Weight) + .saturating_add((20267000 as Weight).saturating_mul(m as Weight)) + .saturating_add((176000 as Weight).saturating_mul(n as Weight)) + .saturating_add((26230000 as Weight).saturating_mul(p as Weight)) + .saturating_add(DbWeight::get().reads(2 as Weight)) + .saturating_add(DbWeight::get().reads((1 as Weight).saturating_mul(p as Weight))) + .saturating_add(DbWeight::get().writes(2 as Weight)) + .saturating_add(DbWeight::get().writes((1 as Weight).saturating_mul(p as Weight))) + } + fn execute(b: u32, m: u32, ) -> Weight { + (26278000 as Weight) + .saturating_add((111000 as Weight).saturating_mul(m as Weight)) + .saturating_add((4000 as Weight).saturating_mul(b as Weight)) + .saturating_add(DbWeight::get().reads(1 as Weight)) + } + fn propose_execute(b: u32, m: u32, ) -> Weight { + (31972000 as Weight) + .saturating_add((218000 as Weight).saturating_mul(m as Weight)) + .saturating_add((4000 as Weight).saturating_mul(b as Weight)) + .saturating_add(DbWeight::get().reads(2 as Weight)) + } + fn propose_proposed(b: u32, m: u32, p: u32, ) -> Weight { + (53435000 as Weight) + .saturating_add((158000 as Weight).saturating_mul(m as Weight)) + .saturating_add((476000 as Weight).saturating_mul(p as Weight)) + .saturating_add((2000 as Weight).saturating_mul(b as Weight)) + .saturating_add(DbWeight::get().reads(4 as Weight)) + .saturating_add(DbWeight::get().writes(4 as Weight)) + } + fn vote(m: u32, ) -> Weight { + (46165000 as Weight) + .saturating_add((264000 as Weight).saturating_mul(m as Weight)) + .saturating_add(DbWeight::get().reads(2 as Weight)) + .saturating_add(DbWeight::get().writes(1 as Weight)) + } + // WARNING! Some components were not used: ["b"] + fn close_early_disapproved(m: u32, p: u32, ) -> Weight { + (52896000 as Weight) + .saturating_add((242000 as Weight).saturating_mul(m as Weight)) + .saturating_add((450000 as Weight).saturating_mul(p as Weight)) + .saturating_add(DbWeight::get().reads(3 as Weight)) + .saturating_add(DbWeight::get().writes(3 as Weight)) + } + fn close_early_approved(b: u32, m: u32, p: u32, ) -> Weight { + (73235000 as Weight) + .saturating_add((247000 as Weight).saturating_mul(m as Weight)) + .saturating_add((456000 as Weight).saturating_mul(p as Weight)) + .saturating_add((1000 as Weight).saturating_mul(b as Weight)) + .saturating_add(DbWeight::get().reads(4 as Weight)) + .saturating_add(DbWeight::get().writes(3 as Weight)) + } + // WARNING! Some components were not used: ["b"] + fn close_disapproved(m: u32, p: u32, ) -> Weight { + (56171000 as Weight) + .saturating_add((250000 as Weight).saturating_mul(m as Weight)) + .saturating_add((462000 as Weight).saturating_mul(p as Weight)) + .saturating_add(DbWeight::get().reads(4 as Weight)) + .saturating_add(DbWeight::get().writes(3 as Weight)) + } + fn close_approved(b: u32, m: u32, p: u32, ) -> Weight { + (77594000 as Weight) + .saturating_add((246000 as Weight).saturating_mul(m as Weight)) + .saturating_add((453000 as Weight).saturating_mul(p as Weight)) + .saturating_add((2000 as Weight).saturating_mul(b as Weight)) + .saturating_add(DbWeight::get().reads(5 as Weight)) + .saturating_add(DbWeight::get().writes(3 as Weight)) + } + fn disapprove_proposal(p: u32, ) -> Weight { + (0 as Weight) + .saturating_add((490000 as Weight).saturating_mul(p as Weight)) + .saturating_add(DbWeight::get().reads(1 as Weight)) + .saturating_add(DbWeight::get().writes(3 as Weight)) + } +} diff --git a/frame/collective/src/lib.rs b/frame/collective/src/lib.rs index 9ff1ba42f22bf..740bf3f081d82 100644 --- a/frame/collective/src/lib.rs +++ b/frame/collective/src/lib.rs @@ -60,6 +60,8 @@ use frame_system::{self as system, ensure_signed, ensure_root}; #[cfg(feature = "runtime-benchmarks")] mod benchmarking; +mod default_weight; + /// Simple index type for proposal counting. pub type ProposalIndex = u32; From cc6d91a65ebc3dc3b2fc61ad11dc037e45b9535c Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Wed, 26 Aug 2020 15:28:27 +0200 Subject: [PATCH 08/14] Apply suggestions from code review Co-authored-by: Guillaume Thiolliere --- frame/collective/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/collective/src/lib.rs b/frame/collective/src/lib.rs index 740bf3f081d82..e296be98ba9f8 100644 --- a/frame/collective/src/lib.rs +++ b/frame/collective/src/lib.rs @@ -320,7 +320,7 @@ decl_module! { /// # #[weight = ( T::WeightInfo::execute( - *length_bound as u32, // B + *length_bound, // B MAX_MEMBERS, // M ).saturating_add(proposal.get_dispatch_info().weight), // P DispatchClass::Operational From fd52a032fc3ad9ad230683bdee79eb4cd90e48f1 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Wed, 26 Aug 2020 22:22:17 +0200 Subject: [PATCH 09/14] whitelist voter account in benchmark --- frame/collective/src/benchmarking.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/frame/collective/src/benchmarking.rs b/frame/collective/src/benchmarking.rs index 50682441668b0..fe040170da58e 100644 --- a/frame/collective/src/benchmarking.rs +++ b/frame/collective/src/benchmarking.rs @@ -356,6 +356,9 @@ benchmarks_instance! { approve, )?; + // Whitelist voter account from further DB operations. + let voter_key = frame_system::Account::::hashed_key_for(&voter); + frame_benchmarking::benchmarking::add_to_whitelist(voter_key.into()); }: close(SystemOrigin::Signed(voter), last_hash.clone(), index, Weight::max_value(), bytes_in_storage) verify { // The last proposal is removed. From 8c7900c8fa93d1181e3c160d4fc79938fc2b8556 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Thu, 27 Aug 2020 03:04:41 +0200 Subject: [PATCH 10/14] update weights --- .../runtime/src/weights/pallet_collective.rs | 63 ++++++++++--------- frame/collective/src/default_weight.rs | 63 ++++++++++--------- 2 files changed, 64 insertions(+), 62 deletions(-) diff --git a/bin/node/runtime/src/weights/pallet_collective.rs b/bin/node/runtime/src/weights/pallet_collective.rs index 1ac8132868616..32b4ad02d7aa9 100644 --- a/bin/node/runtime/src/weights/pallet_collective.rs +++ b/bin/node/runtime/src/weights/pallet_collective.rs @@ -13,7 +13,10 @@ // See the License for the specific language governing permissions and // limitations under the License. -//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 2.0.0-rc5 +//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 2.0.0-rc6 + +#![allow(unused_parens)] +#![allow(unused_imports)] use frame_support::weights::{Weight, constants::RocksDbWeight as DbWeight}; @@ -21,75 +24,73 @@ pub struct WeightInfo; impl pallet_collective::WeightInfo for WeightInfo { fn set_members(m: u32, n: u32, p: u32, ) -> Weight { (0 as Weight) - .saturating_add((20267000 as Weight).saturating_mul(m as Weight)) - .saturating_add((176000 as Weight).saturating_mul(n as Weight)) - .saturating_add((26230000 as Weight).saturating_mul(p as Weight)) + .saturating_add((21040000 as Weight).saturating_mul(m as Weight)) + .saturating_add((173000 as Weight).saturating_mul(n as Weight)) + .saturating_add((31595000 as Weight).saturating_mul(p as Weight)) .saturating_add(DbWeight::get().reads(2 as Weight)) .saturating_add(DbWeight::get().reads((1 as Weight).saturating_mul(p as Weight))) .saturating_add(DbWeight::get().writes(2 as Weight)) .saturating_add(DbWeight::get().writes((1 as Weight).saturating_mul(p as Weight))) } fn execute(b: u32, m: u32, ) -> Weight { - (26278000 as Weight) - .saturating_add((111000 as Weight).saturating_mul(m as Weight)) + (43359000 as Weight) .saturating_add((4000 as Weight).saturating_mul(b as Weight)) + .saturating_add((123000 as Weight).saturating_mul(m as Weight)) .saturating_add(DbWeight::get().reads(1 as Weight)) } fn propose_execute(b: u32, m: u32, ) -> Weight { - (31972000 as Weight) - .saturating_add((218000 as Weight).saturating_mul(m as Weight)) + (54134000 as Weight) .saturating_add((4000 as Weight).saturating_mul(b as Weight)) + .saturating_add((239000 as Weight).saturating_mul(m as Weight)) .saturating_add(DbWeight::get().reads(2 as Weight)) } fn propose_proposed(b: u32, m: u32, p: u32, ) -> Weight { - (53435000 as Weight) - .saturating_add((158000 as Weight).saturating_mul(m as Weight)) - .saturating_add((476000 as Weight).saturating_mul(p as Weight)) - .saturating_add((2000 as Weight).saturating_mul(b as Weight)) + (90650000 as Weight) + .saturating_add((5000 as Weight).saturating_mul(b as Weight)) + .saturating_add((152000 as Weight).saturating_mul(m as Weight)) + .saturating_add((970000 as Weight).saturating_mul(p as Weight)) .saturating_add(DbWeight::get().reads(4 as Weight)) .saturating_add(DbWeight::get().writes(4 as Weight)) } fn vote(m: u32, ) -> Weight { - (46165000 as Weight) - .saturating_add((264000 as Weight).saturating_mul(m as Weight)) + (74460000 as Weight) + .saturating_add((290000 as Weight).saturating_mul(m as Weight)) .saturating_add(DbWeight::get().reads(2 as Weight)) .saturating_add(DbWeight::get().writes(1 as Weight)) } - // WARNING! Some components were not used: ["b"] fn close_early_disapproved(m: u32, p: u32, ) -> Weight { - (52896000 as Weight) - .saturating_add((242000 as Weight).saturating_mul(m as Weight)) - .saturating_add((450000 as Weight).saturating_mul(p as Weight)) + (86360000 as Weight) + .saturating_add((232000 as Weight).saturating_mul(m as Weight)) + .saturating_add((954000 as Weight).saturating_mul(p as Weight)) .saturating_add(DbWeight::get().reads(3 as Weight)) .saturating_add(DbWeight::get().writes(3 as Weight)) } fn close_early_approved(b: u32, m: u32, p: u32, ) -> Weight { - (73235000 as Weight) - .saturating_add((247000 as Weight).saturating_mul(m as Weight)) - .saturating_add((456000 as Weight).saturating_mul(p as Weight)) + (123653000 as Weight) .saturating_add((1000 as Weight).saturating_mul(b as Weight)) + .saturating_add((287000 as Weight).saturating_mul(m as Weight)) + .saturating_add((920000 as Weight).saturating_mul(p as Weight)) .saturating_add(DbWeight::get().reads(4 as Weight)) .saturating_add(DbWeight::get().writes(3 as Weight)) } - // WARNING! Some components were not used: ["b"] fn close_disapproved(m: u32, p: u32, ) -> Weight { - (56171000 as Weight) - .saturating_add((250000 as Weight).saturating_mul(m as Weight)) - .saturating_add((462000 as Weight).saturating_mul(p as Weight)) + (95395000 as Weight) + .saturating_add((236000 as Weight).saturating_mul(m as Weight)) + .saturating_add((965000 as Weight).saturating_mul(p as Weight)) .saturating_add(DbWeight::get().reads(4 as Weight)) .saturating_add(DbWeight::get().writes(3 as Weight)) } fn close_approved(b: u32, m: u32, p: u32, ) -> Weight { - (77594000 as Weight) - .saturating_add((246000 as Weight).saturating_mul(m as Weight)) - .saturating_add((453000 as Weight).saturating_mul(p as Weight)) - .saturating_add((2000 as Weight).saturating_mul(b as Weight)) + (135284000 as Weight) + .saturating_add((4000 as Weight).saturating_mul(b as Weight)) + .saturating_add((218000 as Weight).saturating_mul(m as Weight)) + .saturating_add((951000 as Weight).saturating_mul(p as Weight)) .saturating_add(DbWeight::get().reads(5 as Weight)) .saturating_add(DbWeight::get().writes(3 as Weight)) } fn disapprove_proposal(p: u32, ) -> Weight { - (0 as Weight) - .saturating_add((490000 as Weight).saturating_mul(p as Weight)) + (50500000 as Weight) + .saturating_add((966000 as Weight).saturating_mul(p as Weight)) .saturating_add(DbWeight::get().reads(1 as Weight)) .saturating_add(DbWeight::get().writes(3 as Weight)) } diff --git a/frame/collective/src/default_weight.rs b/frame/collective/src/default_weight.rs index 06525a31ba03e..bb6fe0ea25312 100644 --- a/frame/collective/src/default_weight.rs +++ b/frame/collective/src/default_weight.rs @@ -14,82 +14,83 @@ // limitations under the License. //! Default weights for the Collective Pallet -//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 2.0.0-rc5 +//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 2.0.0-rc6 + +#![allow(unused_parens)] +#![allow(unused_imports)] use frame_support::weights::{Weight, constants::RocksDbWeight as DbWeight}; impl crate::WeightInfo for () { fn set_members(m: u32, n: u32, p: u32, ) -> Weight { (0 as Weight) - .saturating_add((20267000 as Weight).saturating_mul(m as Weight)) - .saturating_add((176000 as Weight).saturating_mul(n as Weight)) - .saturating_add((26230000 as Weight).saturating_mul(p as Weight)) + .saturating_add((21040000 as Weight).saturating_mul(m as Weight)) + .saturating_add((173000 as Weight).saturating_mul(n as Weight)) + .saturating_add((31595000 as Weight).saturating_mul(p as Weight)) .saturating_add(DbWeight::get().reads(2 as Weight)) .saturating_add(DbWeight::get().reads((1 as Weight).saturating_mul(p as Weight))) .saturating_add(DbWeight::get().writes(2 as Weight)) .saturating_add(DbWeight::get().writes((1 as Weight).saturating_mul(p as Weight))) } fn execute(b: u32, m: u32, ) -> Weight { - (26278000 as Weight) - .saturating_add((111000 as Weight).saturating_mul(m as Weight)) + (43359000 as Weight) .saturating_add((4000 as Weight).saturating_mul(b as Weight)) + .saturating_add((123000 as Weight).saturating_mul(m as Weight)) .saturating_add(DbWeight::get().reads(1 as Weight)) } fn propose_execute(b: u32, m: u32, ) -> Weight { - (31972000 as Weight) - .saturating_add((218000 as Weight).saturating_mul(m as Weight)) + (54134000 as Weight) .saturating_add((4000 as Weight).saturating_mul(b as Weight)) + .saturating_add((239000 as Weight).saturating_mul(m as Weight)) .saturating_add(DbWeight::get().reads(2 as Weight)) } fn propose_proposed(b: u32, m: u32, p: u32, ) -> Weight { - (53435000 as Weight) - .saturating_add((158000 as Weight).saturating_mul(m as Weight)) - .saturating_add((476000 as Weight).saturating_mul(p as Weight)) - .saturating_add((2000 as Weight).saturating_mul(b as Weight)) + (90650000 as Weight) + .saturating_add((5000 as Weight).saturating_mul(b as Weight)) + .saturating_add((152000 as Weight).saturating_mul(m as Weight)) + .saturating_add((970000 as Weight).saturating_mul(p as Weight)) .saturating_add(DbWeight::get().reads(4 as Weight)) .saturating_add(DbWeight::get().writes(4 as Weight)) } fn vote(m: u32, ) -> Weight { - (46165000 as Weight) - .saturating_add((264000 as Weight).saturating_mul(m as Weight)) + (74460000 as Weight) + .saturating_add((290000 as Weight).saturating_mul(m as Weight)) .saturating_add(DbWeight::get().reads(2 as Weight)) .saturating_add(DbWeight::get().writes(1 as Weight)) } - // WARNING! Some components were not used: ["b"] fn close_early_disapproved(m: u32, p: u32, ) -> Weight { - (52896000 as Weight) - .saturating_add((242000 as Weight).saturating_mul(m as Weight)) - .saturating_add((450000 as Weight).saturating_mul(p as Weight)) + (86360000 as Weight) + .saturating_add((232000 as Weight).saturating_mul(m as Weight)) + .saturating_add((954000 as Weight).saturating_mul(p as Weight)) .saturating_add(DbWeight::get().reads(3 as Weight)) .saturating_add(DbWeight::get().writes(3 as Weight)) } fn close_early_approved(b: u32, m: u32, p: u32, ) -> Weight { - (73235000 as Weight) - .saturating_add((247000 as Weight).saturating_mul(m as Weight)) - .saturating_add((456000 as Weight).saturating_mul(p as Weight)) + (123653000 as Weight) .saturating_add((1000 as Weight).saturating_mul(b as Weight)) + .saturating_add((287000 as Weight).saturating_mul(m as Weight)) + .saturating_add((920000 as Weight).saturating_mul(p as Weight)) .saturating_add(DbWeight::get().reads(4 as Weight)) .saturating_add(DbWeight::get().writes(3 as Weight)) } - // WARNING! Some components were not used: ["b"] fn close_disapproved(m: u32, p: u32, ) -> Weight { - (56171000 as Weight) - .saturating_add((250000 as Weight).saturating_mul(m as Weight)) - .saturating_add((462000 as Weight).saturating_mul(p as Weight)) + (95395000 as Weight) + .saturating_add((236000 as Weight).saturating_mul(m as Weight)) + .saturating_add((965000 as Weight).saturating_mul(p as Weight)) .saturating_add(DbWeight::get().reads(4 as Weight)) .saturating_add(DbWeight::get().writes(3 as Weight)) } fn close_approved(b: u32, m: u32, p: u32, ) -> Weight { - (77594000 as Weight) - .saturating_add((246000 as Weight).saturating_mul(m as Weight)) - .saturating_add((453000 as Weight).saturating_mul(p as Weight)) - .saturating_add((2000 as Weight).saturating_mul(b as Weight)) + (135284000 as Weight) + .saturating_add((4000 as Weight).saturating_mul(b as Weight)) + .saturating_add((218000 as Weight).saturating_mul(m as Weight)) + .saturating_add((951000 as Weight).saturating_mul(p as Weight)) .saturating_add(DbWeight::get().reads(5 as Weight)) .saturating_add(DbWeight::get().writes(3 as Weight)) } fn disapprove_proposal(p: u32, ) -> Weight { - (0 as Weight) - .saturating_add((490000 as Weight).saturating_mul(p as Weight)) + (50500000 as Weight) + .saturating_add((966000 as Weight).saturating_mul(p as Weight)) .saturating_add(DbWeight::get().reads(1 as Weight)) .saturating_add(DbWeight::get().writes(3 as Weight)) } From bece618483ecaa74acc2c3eb08545a867a456e95 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Thu, 27 Aug 2020 18:03:40 +0200 Subject: [PATCH 11/14] MaxMembers configurable --- bin/node/runtime/src/lib.rs | 8 +++- frame/collective/src/benchmarking.rs | 42 +++++++++---------- frame/collective/src/lib.rs | 61 +++++++++++++++------------- 3 files changed, 59 insertions(+), 52 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index a7d4a180519d0..43838add675f9 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -509,6 +509,7 @@ impl pallet_democracy::Trait for Runtime { parameter_types! { pub const CouncilMotionDuration: BlockNumber = 5 * DAYS; pub const CouncilMaxProposals: u32 = 100; + pub const CouncilMaxMembers: u32 = 100; } type CouncilCollective = pallet_collective::Instance1; @@ -518,6 +519,7 @@ impl pallet_collective::Trait for Runtime { type Event = Event; type MotionDuration = CouncilMotionDuration; type MaxProposals = CouncilMaxProposals; + type MaxMembers = CouncilMaxMembers; type WeightInfo = weights::pallet_collective::WeightInfo; } @@ -530,8 +532,8 @@ parameter_types! { pub const ElectionsPhragmenModuleId: LockIdentifier = *b"phrelect"; } -// Make sure that there are no more than `MAX_MEMBERS` members elected via elections-phragmen. -const_assert!(DesiredMembers::get() <= pallet_collective::MAX_MEMBERS); +// Make sure that there are no more than `MaxMembers` members elected via elections-phragmen. +const_assert!(DesiredMembers::get() <= CouncilMaxMembers::get()); impl pallet_elections_phragmen::Trait for Runtime { type Event = Event; @@ -556,6 +558,7 @@ impl pallet_elections_phragmen::Trait for Runtime { parameter_types! { pub const TechnicalMotionDuration: BlockNumber = 5 * DAYS; pub const TechnicalMaxProposals: u32 = 100; + pub const TechnicalMaxMembers: u32 = 100; } type TechnicalCollective = pallet_collective::Instance2; @@ -565,6 +568,7 @@ impl pallet_collective::Trait for Runtime { type Event = Event; type MotionDuration = TechnicalMotionDuration; type MaxProposals = TechnicalMaxProposals; + type MaxMembers = TechnicalMaxMembers; type WeightInfo = (); } diff --git a/frame/collective/src/benchmarking.rs b/frame/collective/src/benchmarking.rs index fe040170da58e..fa71586a05956 100644 --- a/frame/collective/src/benchmarking.rs +++ b/frame/collective/src/benchmarking.rs @@ -45,8 +45,8 @@ benchmarks_instance! { _{ } set_members { - let m in 1 .. MAX_MEMBERS; - let n in 1 .. MAX_MEMBERS; + let m in 1 .. T::MaxMembers::get(); + let n in 1 .. T::MaxMembers::get(); let p in 1 .. T::MaxProposals::get(); // Set old members. @@ -63,7 +63,7 @@ benchmarks_instance! { SystemOrigin::Root.into(), old_members.clone(), Some(last_old_member.clone()), - MAX_MEMBERS, + T::MaxMembers::get(), )?; // Set a high threshold for proposals passing so that they stay around. @@ -104,7 +104,7 @@ benchmarks_instance! { new_members.push(last_member.clone()); } - }: _(SystemOrigin::Root, new_members.clone(), Some(last_member), MAX_MEMBERS) + }: _(SystemOrigin::Root, new_members.clone(), Some(last_member), T::MaxMembers::get()) verify { new_members.sort(); assert_eq!(Collective::::members(), new_members); @@ -112,7 +112,7 @@ benchmarks_instance! { execute { let b in 1 .. MAX_BYTES; - let m in 1 .. MAX_MEMBERS; + let m in 1 .. T::MaxMembers::get(); let bytes_in_storage = b + size_of::() as u32; @@ -126,7 +126,7 @@ benchmarks_instance! { let caller: T::AccountId = whitelisted_caller(); members.push(caller.clone()); - Collective::::set_members(SystemOrigin::Root.into(), members, None, MAX_MEMBERS)?; + Collective::::set_members(SystemOrigin::Root.into(), members, None, T::MaxMembers::get())?; let proposal: T::Proposal = SystemCall::::remark(vec![1; b as usize]).into(); @@ -142,7 +142,7 @@ benchmarks_instance! { // This tests when execution would happen immediately after proposal propose_execute { let b in 1 .. MAX_BYTES; - let m in 1 .. MAX_MEMBERS; + let m in 1 .. T::MaxMembers::get(); let bytes_in_storage = b + size_of::() as u32; @@ -156,7 +156,7 @@ benchmarks_instance! { let caller: T::AccountId = whitelisted_caller(); members.push(caller.clone()); - Collective::::set_members(SystemOrigin::Root.into(), members, None, MAX_MEMBERS)?; + Collective::::set_members(SystemOrigin::Root.into(), members, None, T::MaxMembers::get())?; let proposal: T::Proposal = SystemCall::::remark(vec![1; b as usize]).into(); let threshold = 1; @@ -173,7 +173,7 @@ benchmarks_instance! { // This tests when proposal is created and queued as "proposed" propose_proposed { let b in 1 .. MAX_BYTES; - let m in 2 .. MAX_MEMBERS; + let m in 2 .. T::MaxMembers::get(); let p in 1 .. T::MaxProposals::get(); let bytes_in_storage = b + size_of::() as u32; @@ -186,7 +186,7 @@ benchmarks_instance! { } let caller: T::AccountId = whitelisted_caller(); members.push(caller.clone()); - Collective::::set_members(SystemOrigin::Root.into(), members, None, MAX_MEMBERS)?; + Collective::::set_members(SystemOrigin::Root.into(), members, None, T::MaxMembers::get())?; let threshold = m; // Add previous proposals. @@ -215,7 +215,7 @@ benchmarks_instance! { vote { // We choose 5 as a minimum so we always trigger a vote in the voting loop (`for j in ...`) - let m in 5 .. MAX_MEMBERS; + let m in 5 .. T::MaxMembers::get(); let p = T::MaxProposals::get(); let b = MAX_BYTES; @@ -231,7 +231,7 @@ benchmarks_instance! { } let voter: T::AccountId = account("voter", 0, SEED); members.push(voter.clone()); - Collective::::set_members(SystemOrigin::Root.into(), members.clone(), None, MAX_MEMBERS)?; + Collective::::set_members(SystemOrigin::Root.into(), members.clone(), None, T::MaxMembers::get())?; // Threshold is 1 less than the number of members so that one person can vote nay let threshold = m - 1; @@ -288,7 +288,7 @@ benchmarks_instance! { close_early_disapproved { // We choose 4 as a minimum so we always trigger a vote in the voting loop (`for j in ...`) - let m in 4 .. MAX_MEMBERS; + let m in 4 .. T::MaxMembers::get(); let p in 1 .. T::MaxProposals::get(); let bytes = 100; @@ -304,7 +304,7 @@ benchmarks_instance! { } let voter: T::AccountId = account("voter", 0, SEED); members.push(voter.clone()); - Collective::::set_members(SystemOrigin::Root.into(), members.clone(), None, MAX_MEMBERS)?; + Collective::::set_members(SystemOrigin::Root.into(), members.clone(), None, T::MaxMembers::get())?; // Threshold is total members so that one nay will disapprove the vote let threshold = m; @@ -369,7 +369,7 @@ benchmarks_instance! { close_early_approved { let b in 1 .. MAX_BYTES; // We choose 4 as a minimum so we always trigger a vote in the voting loop (`for j in ...`) - let m in 4 .. MAX_MEMBERS; + let m in 4 .. T::MaxMembers::get(); let p in 1 .. T::MaxProposals::get(); let bytes_in_storage = b + size_of::() as u32; @@ -382,7 +382,7 @@ benchmarks_instance! { } let caller: T::AccountId = whitelisted_caller(); members.push(caller.clone()); - Collective::::set_members(SystemOrigin::Root.into(), members.clone(), None, MAX_MEMBERS)?; + Collective::::set_members(SystemOrigin::Root.into(), members.clone(), None, T::MaxMembers::get())?; // Threshold is 2 so any two ayes will approve the vote let threshold = 2; @@ -449,7 +449,7 @@ benchmarks_instance! { close_disapproved { // We choose 4 as a minimum so we always trigger a vote in the voting loop (`for j in ...`) - let m in 4 .. MAX_MEMBERS; + let m in 4 .. T::MaxMembers::get(); let p in 1 .. T::MaxProposals::get(); let bytes = 100; @@ -467,7 +467,7 @@ benchmarks_instance! { SystemOrigin::Root.into(), members.clone(), Some(caller.clone()), - MAX_MEMBERS, + T::MaxMembers::get(), )?; // Threshold is one less than total members so that two nays will disapprove the vote @@ -522,7 +522,7 @@ benchmarks_instance! { close_approved { let b in 1 .. MAX_BYTES; // We choose 4 as a minimum so we always trigger a vote in the voting loop (`for j in ...`) - let m in 4 .. MAX_MEMBERS; + let m in 4 .. T::MaxMembers::get(); let p in 1 .. T::MaxProposals::get(); let bytes_in_storage = b + size_of::() as u32; @@ -539,7 +539,7 @@ benchmarks_instance! { SystemOrigin::Root.into(), members.clone(), Some(caller.clone()), - MAX_MEMBERS, + T::MaxMembers::get(), )?; // Threshold is two, so any two ayes will pass the vote @@ -602,7 +602,7 @@ benchmarks_instance! { SystemOrigin::Root.into(), members.clone(), Some(caller.clone()), - MAX_MEMBERS, + T::MaxMembers::get(), )?; // Threshold is one less than total members so that two nays will disapprove the vote diff --git a/frame/collective/src/lib.rs b/frame/collective/src/lib.rs index e296be98ba9f8..3d22a9ab5afb4 100644 --- a/frame/collective/src/lib.rs +++ b/frame/collective/src/lib.rs @@ -20,7 +20,7 @@ //! //! The membership can be provided in one of two ways: either directly, using the Root-dispatchable //! function `set_members`, or indirectly, through implementing the `ChangeMembers`. -//! The pallet assumes that the amount of members stays at or below `MAX_MEMBERS` for its weight +//! The pallet assumes that the amount of members stays at or below `MaxMembers` for its weight //! calculations, but enforces this neither in `set_members` nor in `change_members_sorted`. //! //! A "prime" member may be set allowing their vote to act as the default vote in case of any @@ -71,13 +71,6 @@ pub type ProposalIndex = u32; /// vote exactly once, therefore also the number of votes for any given motion. pub type MemberCount = u32; -/// The maximum number of members supported by the pallet. Used for weight estimation. -/// -/// NOTE: -/// + Benchmarks will need to be re-run and weights adjusted if this changes. -/// + This pallet assumes that dependents keep to the limit without enforcing it. -pub const MAX_MEMBERS: MemberCount = 100; - pub trait WeightInfo { fn set_members(m: u32, n: u32, p: u32, ) -> Weight; fn execute(b: u32, m: u32, ) -> Weight; @@ -108,7 +101,14 @@ pub trait Trait: frame_system::Trait { type MotionDuration: Get; /// Maximum number of proposals allowed to be active in parallel. - type MaxProposals: Get; + type MaxProposals: Get; + + /// The maximum number of members supported by the pallet. Used for weight estimation. + /// + /// NOTE: + /// + Benchmarks will need to be re-run and weights adjusted if this changes. + /// + This pallet assumes that dependents keep to the limit without enforcing it. + type MaxMembers: Get; /// Weight information for extrinsics in this pallet. type WeightInfo: WeightInfo; @@ -251,7 +251,7 @@ decl_module! { /// /// Requires root origin. /// - /// NOTE: Does not enforce the expected `MAX_MEMBERS` limit on the amount of members, but + /// NOTE: Does not enforce the expected `MaxMembers` limit on the amount of members, but /// the weight estimations rely on it to estimate dispatchable weight. /// /// # @@ -280,10 +280,10 @@ decl_module! { old_count: MemberCount, ) -> DispatchResultWithPostInfo { ensure_root(origin)?; - if new_members.len() > MAX_MEMBERS as usize { + if new_members.len() > T::MaxMembers::get() as usize { debug::error!( "New members count exceeds maximum amount of members expected. (expected: {}, actual: {})", - MAX_MEMBERS, + T::MaxMembers::get(), new_members.len() ); } @@ -321,7 +321,7 @@ decl_module! { #[weight = ( T::WeightInfo::execute( *length_bound, // B - MAX_MEMBERS, // M + T::MaxMembers::get(), // M ).saturating_add(proposal.get_dispatch_info().weight), // P DispatchClass::Operational )] @@ -380,12 +380,12 @@ decl_module! { if *threshold < 2 { T::WeightInfo::propose_execute( *length_bound, // B - MAX_MEMBERS, // M + T::MaxMembers::get(), // M ).saturating_add(proposal.get_dispatch_info().weight) // P1 } else { T::WeightInfo::propose_proposed( *length_bound, // B - MAX_MEMBERS, // M + T::MaxMembers::get(), // M T::MaxProposals::get(), // P2 ) }, @@ -458,7 +458,7 @@ decl_module! { /// - 1 event /// # #[weight = ( - T::WeightInfo::vote(MAX_MEMBERS), + T::WeightInfo::vote(T::MaxMembers::get()), DispatchClass::Operational )] fn vote(origin, @@ -535,7 +535,7 @@ decl_module! { #[weight = ( { let b = *length_bound; - let m = MAX_MEMBERS; + let m = T::MaxMembers::get(); let p1 = *proposal_weight_bound; let p2 = T::MaxProposals::get(); T::WeightInfo::close_early_approved(b, m, p2) @@ -570,7 +570,7 @@ decl_module! { proposal_weight_bound )?; Self::deposit_event(RawEvent::Closed(proposal_hash, yes_votes, no_votes)); - let (proposal_weight, proposal_count) = + let (proposal_weight, proposal_count) = Self::do_approve_proposal(seats, voting, proposal_hash, proposal); return Ok(Some( T::WeightInfo::close_early_approved(len as u32, seats, proposal_count) @@ -604,7 +604,7 @@ decl_module! { proposal_weight_bound )?; Self::deposit_event(RawEvent::Closed(proposal_hash, yes_votes, no_votes)); - let (proposal_weight, proposal_count) = + let (proposal_weight, proposal_count) = Self::do_approve_proposal(seats, voting, proposal_hash, proposal); return Ok(Some( T::WeightInfo::close_approved(len as u32, seats, proposal_count) @@ -727,7 +727,7 @@ impl, I: Instance> Module { impl, I: Instance> ChangeMembers for Module { /// Update the members of the collective. Votes are updated and the prime is reset. /// - /// NOTE: Does not enforce the expected `MAX_MEMBERS` limit on the amount of members, but + /// NOTE: Does not enforce the expected `MaxMembers` limit on the amount of members, but /// the weight estimations rely on it to estimate dispatchable weight. /// /// # @@ -747,10 +747,10 @@ impl, I: Instance> ChangeMembers for Module { outgoing: &[T::AccountId], new: &[T::AccountId], ) { - if new.len() > MAX_MEMBERS as usize { + if new.len() > T::MaxMembers::get() as usize { debug::error!( "New members count exceeds maximum amount of members expected. (expected: {}, actual: {})", - MAX_MEMBERS, + T::MaxMembers::get(), new.len() ); } @@ -910,6 +910,7 @@ mod tests { pub const AvailableBlockRatio: Perbill = Perbill::one(); pub const MotionDuration: u64 = 3; pub const MaxProposals: u32 = 100; + pub const MaxMembers: u32 = 100; } impl frame_system::Trait for Test { type BaseCallFilter = (); @@ -944,6 +945,7 @@ mod tests { type Event = Event; type MotionDuration = MotionDuration; type MaxProposals = MaxProposals; + type MaxMembers = MaxMembers; type WeightInfo = (); } impl Trait for Test { @@ -952,6 +954,7 @@ mod tests { type Event = Event; type MotionDuration = MotionDuration; type MaxProposals = MaxProposals; + type MaxMembers = MaxMembers; type WeightInfo = (); } @@ -1027,7 +1030,7 @@ mod tests { #[test] fn proposal_weight_limit_works_on_approve() { new_test_ext().execute_with(|| { - let proposal = Call::Collective(crate::Call::set_members(vec![1, 2, 3], None, MAX_MEMBERS)); + let proposal = Call::Collective(crate::Call::set_members(vec![1, 2, 3], None, MaxMembers::get())); let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); let proposal_weight = proposal.get_dispatch_info().weight; let hash = BlakeTwo256::hash_of(&proposal); @@ -1047,7 +1050,7 @@ mod tests { #[test] fn proposal_weight_limit_ignored_on_disapprove() { new_test_ext().execute_with(|| { - let proposal = Call::Collective(crate::Call::set_members(vec![1, 2, 3], None, MAX_MEMBERS)); + let proposal = Call::Collective(crate::Call::set_members(vec![1, 2, 3], None, MaxMembers::get())); let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); let proposal_weight = proposal.get_dispatch_info().weight; let hash = BlakeTwo256::hash_of(&proposal); @@ -1068,7 +1071,7 @@ mod tests { let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); let proposal_weight = proposal.get_dispatch_info().weight; let hash = BlakeTwo256::hash_of(&proposal); - assert_ok!(Collective::set_members(Origin::root(), vec![1, 2, 3], Some(3), MAX_MEMBERS)); + assert_ok!(Collective::set_members(Origin::root(), vec![1, 2, 3], Some(3), MaxMembers::get())); assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); @@ -1093,7 +1096,7 @@ mod tests { let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); let proposal_weight = proposal.get_dispatch_info().weight; let hash = BlakeTwo256::hash_of(&proposal); - assert_ok!(Collective::set_members(Origin::root(), vec![1, 2, 3], Some(1), MAX_MEMBERS)); + assert_ok!(Collective::set_members(Origin::root(), vec![1, 2, 3], Some(1), MaxMembers::get())); assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); @@ -1161,7 +1164,7 @@ mod tests { Collective::voting(&hash), Some(Votes { index: 0, threshold: 3, ayes: vec![1, 2], nays: vec![], end }) ); - assert_ok!(Collective::set_members(Origin::root(), vec![2, 3, 4], None, MAX_MEMBERS)); + assert_ok!(Collective::set_members(Origin::root(), vec![2, 3, 4], None, MaxMembers::get())); assert_eq!( Collective::voting(&hash), Some(Votes { index: 0, threshold: 3, ayes: vec![2], nays: vec![], end }) @@ -1176,7 +1179,7 @@ mod tests { Collective::voting(&hash), Some(Votes { index: 1, threshold: 2, ayes: vec![2], nays: vec![3], end }) ); - assert_ok!(Collective::set_members(Origin::root(), vec![2, 4], None, MAX_MEMBERS)); + assert_ok!(Collective::set_members(Origin::root(), vec![2, 4], None, MaxMembers::get())); assert_eq!( Collective::voting(&hash), Some(Votes { index: 1, threshold: 2, ayes: vec![2], nays: vec![], end }) @@ -1234,7 +1237,7 @@ mod tests { #[test] fn correct_validate_and_get_proposal() { new_test_ext().execute_with(|| { - let proposal = Call::Collective(crate::Call::set_members(vec![1, 2, 3], None, MAX_MEMBERS)); + let proposal = Call::Collective(crate::Call::set_members(vec![1, 2, 3], None, MaxMembers::get())); let length = proposal.encode().len() as u32; assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), length)); From 68f74c2d6b60493d469200023718335d51886f67 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Thu, 27 Aug 2020 20:19:03 +0200 Subject: [PATCH 12/14] remove base weight comment --- frame/collective/src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/frame/collective/src/lib.rs b/frame/collective/src/lib.rs index 3d22a9ab5afb4..949484a5957b4 100644 --- a/frame/collective/src/lib.rs +++ b/frame/collective/src/lib.rs @@ -628,7 +628,6 @@ decl_module! { /// /// # /// Complexity: O(P) where P is the number of max proposals - /// Base Weight: .49 * P /// DB Weight: /// * Reads: Proposals /// * Writes: Voting, Proposals, ProposalOf From 4d04b4a291b436591295681497d0198042ab9fc3 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Thu, 27 Aug 2020 20:21:43 +0200 Subject: [PATCH 13/14] add weight to technical collective --- bin/node/runtime/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 43838add675f9..54dea704bd7f6 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -569,7 +569,7 @@ impl pallet_collective::Trait for Runtime { type MotionDuration = TechnicalMotionDuration; type MaxProposals = TechnicalMaxProposals; type MaxMembers = TechnicalMaxMembers; - type WeightInfo = (); + type WeightInfo = weights::pallet_collective::WeightInfo; } type EnsureRootOrHalfCouncil = EnsureOneOf< From 78001208cccc90b5c6583f23a312bf773799e596 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Thu, 27 Aug 2020 21:48:32 +0200 Subject: [PATCH 14/14] another DB whitelist optimization --- frame/collective/src/benchmarking.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/frame/collective/src/benchmarking.rs b/frame/collective/src/benchmarking.rs index fa71586a05956..d4e80d515941f 100644 --- a/frame/collective/src/benchmarking.rs +++ b/frame/collective/src/benchmarking.rs @@ -277,6 +277,9 @@ benchmarks_instance! { // Voter switches vote to nay, but does not kill the vote, just updates + inserts let approve = false; + // Whitelist voter account from further DB operations. + let voter_key = frame_system::Account::::hashed_key_for(&voter); + frame_benchmarking::benchmarking::add_to_whitelist(voter_key.into()); }: _(SystemOrigin::Signed(voter), last_hash.clone(), index, approve) verify { // All proposals exist and the last proposal has just been updated.