Add benchmarks for pallet-membership (#8596)

* Add benchmakrs for membership * Update frame/membership/src/lib.rs Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com> * Make it all work * Add mock weights * Update frame/membership/src/lib.rs * Update frame/membership/src/lib.rs Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com> * cargo run --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_membership --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/membership/src/weights.rs --template=./.maintain/frame-weight-template.hbs * cargo run --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_membership --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/membership/src/weights.rs --template=./.maintain/frame-weight-template.hbs Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com> Co-authored-by: Parity Benchmarking Bot <admin@parity.io>
2026-06-13 21:01:05 +00:00 · 2021-04-18 06:52:05 +02:00
parent 92630df143
commit b6b107030d
9 changed files with 392 additions and 25 deletions
@@ -18,7 +18,7 @@
 //! # Membership Module
 //!
 //! Allows control of membership of a set of `AccountId`s, useful for managing membership of of a
-//! collective. A prime member may be set.
+//! collective. A prime member may be set

 // Ensure we're `no_std` when compiling for Wasm.
 #![cfg_attr(not(feature = "std"), no_std)]
@@ -26,11 +26,14 @@
 use sp_std::prelude::*;
 use frame_support::{
 	decl_module, decl_storage, decl_event, decl_error,
-	traits::{ChangeMembers, InitializeMembers, EnsureOrigin, Contains, SortedMembers},
+	traits::{ChangeMembers, InitializeMembers, EnsureOrigin, Contains, SortedMembers, Get},
 };
 use frame_system::ensure_signed;

-pub trait Config<I=DefaultInstance>: frame_system::Config {
+pub mod weights;
+pub use weights::WeightInfo;
+
+pub trait Config<I = DefaultInstance>: frame_system::Config {
 	/// The overarching event type.
 	type Event: From<Event<Self, I>> + Into<<Self as frame_system::Config>::Event>;

@@ -56,6 +59,16 @@ pub trait Config<I=DefaultInstance>: frame_system::Config {

 	/// The receiver of the signal for when the membership has changed.
 	type MembershipChanged: ChangeMembers<Self::AccountId>;
+
+	/// The maximum number of members that this membership can have.
+	///
+	/// This is used for benchmarking. Re-run the benchmarks if this changes.
+	///
+	/// This is not enforced in the code; the membership size can exceed this limit.
+	type MaxMembers: Get<u32>;
+
+	/// Weight information for extrinsics in this pallet.
+	type WeightInfo: WeightInfo;
 }

 decl_storage! {
@@ -127,6 +140,8 @@ decl_module! {
 			let mut members = <Members<T, I>>::get();
 			let location = members.binary_search(&who).err().ok_or(Error::<T, I>::AlreadyMember)?;
 			members.insert(location, who.clone());
+
+			Self::maybe_warn_max_members(&members);
 			<Members<T, I>>::put(&members);

 			T::MembershipChanged::change_members_sorted(&[who], &[], &members[..]);
@@ -144,6 +159,8 @@ decl_module! {
 			let mut members = <Members<T, I>>::get();
 			let location = members.binary_search(&who).ok().ok_or(Error::<T, I>::NotMember)?;
 			members.remove(location);
+
+			Self::maybe_warn_max_members(&members);
 			<Members<T, I>>::put(&members);

 			T::MembershipChanged::change_members_sorted(&[], &[who], &members[..]);
@@ -168,6 +185,8 @@ decl_module! {
 			let _ = members.binary_search(&add).err().ok_or(Error::<T, I>::AlreadyMember)?;
 			members[location] = add.clone();
 			members.sort();
+
+			Self::maybe_warn_max_members(&members);
 			<Members<T, I>>::put(&members);

 			T::MembershipChanged::change_members_sorted(
@@ -193,10 +212,10 @@ decl_module! {
 			<Members<T, I>>::mutate(|m| {
 				T::MembershipChanged::set_members_sorted(&members[..], m);
 				Self::rejig_prime(&members);
+				Self::maybe_warn_max_members(&members);
 				*m = members;
 			});

-
 			Self::deposit_event(RawEvent::MembersReset);
 		}

@@ -215,6 +234,8 @@ decl_module! {
 				let _ = members.binary_search(&new).err().ok_or(Error::<T, I>::AlreadyMember)?;
 				members[location] = new.clone();
 				members.sort();
+
+				Self::maybe_warn_max_members(&members);
 				<Members<T, I>>::put(&members);

 				T::MembershipChanged::change_members_sorted(
@@ -264,6 +285,17 @@ impl<T: Config<I>, I: Instance> Module<T, I> {
 			}
 		}
 	}
+
+	fn maybe_warn_max_members(members: &[T::AccountId]) {
+		if members.len() as u32 > T::MaxMembers::get() {
+			log::error!(
+				target: "runtime::membership",
+				"maximum number of members used for weight is exceeded, weights can be underestimated [{} > {}].",
+				members.len(),
+				T::MaxMembers::get(),
+			)
+		}
+	}
 }

 impl<T: Config<I>, I: Instance> Contains<T::AccountId> for Module<T, I> {
@@ -282,6 +314,149 @@ impl<T: Config<I>, I: Instance> SortedMembers<T::AccountId> for Module<T, I> {
 	}
 }

+#[cfg(feature = "runtime-benchmarks")]
+mod benchmark {
+	use super::{*, Module as Membership};
+	use frame_system::RawOrigin;
+	use frame_support::{traits::EnsureOrigin, assert_ok};
+	use frame_benchmarking::{benchmarks_instance, whitelist, account, impl_benchmark_test_suite};
+
+	const SEED: u32 = 0;
+
+	fn set_members<T: Config<I>, I: Instance>(members: Vec<T::AccountId>, prime: Option<usize>) {
+		let reset_origin = T::ResetOrigin::successful_origin();
+		let prime_origin = T::PrimeOrigin::successful_origin();
+
+		assert_ok!(<Membership<T, _>>::reset_members(reset_origin, members.clone()));
+		if let Some(prime) = prime.map(|i| members[i].clone()) {
+			assert_ok!(<Membership<T, _>>::set_prime(prime_origin, prime));
+		} else {
+			assert_ok!(<Membership<T, _>>::clear_prime(prime_origin));
+		}
+	}
+
+	benchmarks_instance! {
+		add_member {
+			let m in 1 .. T::MaxMembers::get();
+
+			let members = (0..m).map(|i| account("member", i, SEED)).collect::<Vec<T::AccountId>>();
+			set_members::<T, I>(members.clone(), None);
+			let new_member = account::<T::AccountId>("add", m, SEED);
+		}: {
+			assert_ok!(<Membership<T, _>>::add_member(T::AddOrigin::successful_origin(), new_member.clone()));
+		}
+		verify {
+			assert!(<Members<T, _>>::get().contains(&new_member));
+			#[cfg(test)] crate::tests::clean();
+		}
+
+		// the case of no prime or the prime being removed is surely cheaper than the case of
+		// reporting a new prime via `MembershipChanged`.
+		remove_member {
+			let m in 2 .. T::MaxMembers::get();
+
+			let members = (0..m).map(|i| account("member", i, SEED)).collect::<Vec<T::AccountId>>();
+			set_members::<T, I>(members.clone(), Some(members.len() - 1));
+
+			let to_remove = members.first().cloned().unwrap();
+		}: {
+			assert_ok!(<Membership<T, _>>::remove_member(T::RemoveOrigin::successful_origin(), to_remove.clone()));
+		} verify {
+			assert!(!<Members<T, _>>::get().contains(&to_remove));
+			// prime is rejigged
+			assert!(<Prime<T, _>>::get().is_some() && T::MembershipChanged::get_prime().is_some());
+			#[cfg(test)] crate::tests::clean();
+		}
+
+		// we remove a non-prime to make sure it needs to be set again.
+		swap_member {
+			let m in 2 .. T::MaxMembers::get();
+
+			let members = (0..m).map(|i| account("member", i, SEED)).collect::<Vec<T::AccountId>>();
+			set_members::<T, I>(members.clone(), Some(members.len() - 1));
+			let add = account::<T::AccountId>("member", m, SEED);
+			let remove = members.first().cloned().unwrap();
+		}: {
+			assert_ok!(<Membership<T, _>>::swap_member(
+				T::SwapOrigin::successful_origin(),
+				remove.clone(),
+				add.clone(),
+			));
+		} verify {
+			assert!(!<Members<T, _>>::get().contains(&remove));
+			assert!(<Members<T, _>>::get().contains(&add));
+			// prime is rejigged
+			assert!(<Prime<T, _>>::get().is_some() && T::MembershipChanged::get_prime().is_some());
+			#[cfg(test)] crate::tests::clean();
+		}
+
+		// er keep the prime common between incoming and outgoing to make sure it is rejigged.
+		reset_member {
+			let m in 1 .. T::MaxMembers::get();
+
+			let members = (1..m+1).map(|i| account("member", i, SEED)).collect::<Vec<T::AccountId>>();
+			set_members::<T, I>(members.clone(), Some(members.len() - 1));
+			let mut new_members = (m..2*m).map(|i| account("member", i, SEED)).collect::<Vec<T::AccountId>>();
+		}: {
+			assert_ok!(<Membership<T, _>>::reset_members(T::ResetOrigin::successful_origin(), new_members.clone()));
+		} verify {
+			new_members.sort();
+			assert_eq!(<Members<T, _>>::get(), new_members);
+			// prime is rejigged
+			assert!(<Prime<T, _>>::get().is_some() && T::MembershipChanged::get_prime().is_some());
+			#[cfg(test)] crate::tests::clean();
+		}
+
+		change_key {
+			let m in 1 .. T::MaxMembers::get();
+
+			// worse case would be to change the prime
+			let members = (0..m).map(|i| account("member", i, SEED)).collect::<Vec<T::AccountId>>();
+			let prime = members.last().cloned().unwrap();
+			set_members::<T, I>(members.clone(), Some(members.len() - 1));
+
+			let add = account::<T::AccountId>("member", m, SEED);
+			whitelist!(prime);
+		}: {
+			assert_ok!(<Membership<T, _>>::change_key(RawOrigin::Signed(prime.clone()).into(), add.clone()));
+		} verify {
+			assert!(!<Members<T, _>>::get().contains(&prime));
+			assert!(<Members<T, _>>::get().contains(&add));
+			// prime is rejigged
+			assert_eq!(<Prime<T, _>>::get().unwrap(), add);
+			#[cfg(test)] crate::tests::clean();
+		}
+
+		set_prime {
+			let m in 1 .. T::MaxMembers::get();
+			let members = (0..m).map(|i| account("member", i, SEED)).collect::<Vec<T::AccountId>>();
+			let prime = members.last().cloned().unwrap();
+			set_members::<T, I>(members, None);
+		}: {
+			assert_ok!(<Membership<T, _>>::set_prime(T::PrimeOrigin::successful_origin(), prime));
+		} verify {
+			assert!(<Prime<T, _>>::get().is_some());
+			assert!(<T::MembershipChanged>::get_prime().is_some());
+			#[cfg(test)] crate::tests::clean();
+		}
+
+		clear_prime {
+			let m in 1 .. T::MaxMembers::get();
+			let members = (0..m).map(|i| account("member", i, SEED)).collect::<Vec<T::AccountId>>();
+			let prime = members.last().cloned().unwrap();
+			set_members::<T, I>(members, None);
+		}: {
+			assert_ok!(<Membership<T, _>>::clear_prime(T::PrimeOrigin::successful_origin()));
+		} verify {
+			assert!(<Prime<T, _>>::get().is_none());
+			assert!(<T::MembershipChanged>::get_prime().is_none());
+			#[cfg(test)] crate::tests::clean();
+		}
+	}
+
+	impl_benchmark_test_suite!(Membership, crate::tests::new_bench_ext(), crate::tests::Test,);
+}
+
 #[cfg(test)]
 mod tests {
 	use super::*;
@@ -308,11 +483,13 @@ mod tests {

 	parameter_types! {
 		pub const BlockHashCount: u64 = 250;
+		pub const MaxMembers: u32 = 10;
 		pub BlockWeights: frame_system::limits::BlockWeights =
 			frame_system::limits::BlockWeights::simple_max(1024);
 		pub static Members: Vec<u64> = vec![];
 		pub static Prime: Option<u64> = None;
 	}
+
 	impl frame_system::Config for Test {
 		type BaseCallFilter = ();
 		type BlockWeights = ();
@@ -349,7 +526,7 @@ mod tests {
 	pub struct TestChangeMembers;
 	impl ChangeMembers<u64> for TestChangeMembers {
 		fn change_members_sorted(incoming: &[u64], outgoing: &[u64], new: &[u64]) {
-			let mut old_plus_incoming = MEMBERS.with(|m| m.borrow().to_vec());
+			let mut old_plus_incoming = Members::get();
 			old_plus_incoming.extend_from_slice(incoming);
 			old_plus_incoming.sort();
 			let mut new_plus_outgoing = new.to_vec();
@@ -357,13 +534,17 @@ mod tests {
 			new_plus_outgoing.sort();
 			assert_eq!(old_plus_incoming, new_plus_outgoing);

-			MEMBERS.with(|m| *m.borrow_mut() = new.to_vec());
-			PRIME.with(|p| *p.borrow_mut() = None);
+			Members::set(new.to_vec());
+			Prime::set(None);
 		}
 		fn set_prime(who: Option<u64>) {
-			PRIME.with(|p| *p.borrow_mut() = who);
+			Prime::set(who);
+		}
+		fn get_prime() -> Option<u64> {
+			Prime::get()
 		}
 	}
+
 	impl InitializeMembers<u64> for TestChangeMembers {
 		fn initialize_members(members: &[u64]) {
 			MEMBERS.with(|m| *m.borrow_mut() = members.to_vec());
@@ -379,9 +560,11 @@ mod tests {
 		type PrimeOrigin = EnsureSignedBy<Five, u64>;
 		type MembershipInitialized = TestChangeMembers;
 		type MembershipChanged = TestChangeMembers;
+		type MaxMembers = MaxMembers;
+		type WeightInfo = ();
 	}

-	fn new_test_ext() -> sp_io::TestExternalities {
+	pub(crate) fn new_test_ext() -> sp_io::TestExternalities {
 		let mut t = frame_system::GenesisConfig::default().build_storage::<Test>().unwrap();
 		// We use default for brevity, but you can configure as desired if needed.
 		pallet_membership::GenesisConfig::<Test>{
@@ -391,6 +574,17 @@ mod tests {
 		t.into()
 	}

+	#[cfg(feature = "runtime-benchmarks")]
+	pub(crate) fn new_bench_ext() -> sp_io::TestExternalities {
+		frame_system::GenesisConfig::default().build_storage::<Test>().unwrap().into()
+	}
+
+	#[cfg(feature = "runtime-benchmarks")]
+	pub(crate) fn clean() {
+		Members::set(vec![]);
+		Prime::set(None);
+	}
+
 	#[test]
 	fn query_membership_works() {
 		new_test_ext().execute_with(|| {
@@ -0,0 +1,159 @@
+// This file is part of Substrate.
+
+// Copyright (C) 2021 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.
+
+//! Autogenerated weights for pallet_membership
+//!
+//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 3.0.0
+//! DATE: 2021-04-17, STEPS: `[50, ]`, REPEAT: 20, LOW RANGE: `[]`, HIGH RANGE: `[]`
+//! EXECUTION: Some(Wasm), WASM-EXECUTION: Compiled, CHAIN: Some("dev"), DB CACHE: 128
+
+// Executed Command:
+// target/release/substrate
+// benchmark
+// --chain=dev
+// --steps=50
+// --repeat=20
+// --pallet=pallet_membership
+// --extrinsic=*
+// --execution=wasm
+// --wasm-execution=compiled
+// --heap-pages=4096
+// --output=./frame/membership/src/weights.rs
+// --template=./.maintain/frame-weight-template.hbs
+
+
+#![allow(unused_parens)]
+#![allow(unused_imports)]
+
+use frame_support::{traits::Get, weights::{Weight, constants::RocksDbWeight}};
+use sp_std::marker::PhantomData;
+
+/// Weight functions needed for pallet_membership.
+pub trait WeightInfo {
+	fn add_member(m: u32, ) -> Weight;
+	fn remove_member(m: u32, ) -> Weight;
+	fn swap_member(m: u32, ) -> Weight;
+	fn reset_member(m: u32, ) -> Weight;
+	fn change_key(m: u32, ) -> Weight;
+	fn set_prime(m: u32, ) -> Weight;
+	fn clear_prime(m: u32, ) -> Weight;
+}
+
+/// Weights for pallet_membership using the Substrate node and recommended hardware.
+pub struct SubstrateWeight<T>(PhantomData<T>);
+impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
+	fn add_member(m: u32, ) -> Weight {
+		(25_448_000 as Weight)
+			// Standard Error: 3_000
+			.saturating_add((257_000 as Weight).saturating_mul(m as Weight))
+			.saturating_add(T::DbWeight::get().reads(2 as Weight))
+			.saturating_add(T::DbWeight::get().writes(3 as Weight))
+	}
+	fn remove_member(m: u32, ) -> Weight {
+		(31_317_000 as Weight)
+			// Standard Error: 0
+			.saturating_add((215_000 as Weight).saturating_mul(m as Weight))
+			.saturating_add(T::DbWeight::get().reads(3 as Weight))
+			.saturating_add(T::DbWeight::get().writes(3 as Weight))
+	}
+	fn swap_member(m: u32, ) -> Weight {
+		(31_208_000 as Weight)
+			// Standard Error: 0
+			.saturating_add((229_000 as Weight).saturating_mul(m as Weight))
+			.saturating_add(T::DbWeight::get().reads(3 as Weight))
+			.saturating_add(T::DbWeight::get().writes(3 as Weight))
+	}
+	fn reset_member(m: u32, ) -> Weight {
+		(31_673_000 as Weight)
+			// Standard Error: 1_000
+			.saturating_add((455_000 as Weight).saturating_mul(m as Weight))
+			.saturating_add(T::DbWeight::get().reads(3 as Weight))
+			.saturating_add(T::DbWeight::get().writes(3 as Weight))
+	}
+	fn change_key(m: u32, ) -> Weight {
+		(33_499_000 as Weight)
+			// Standard Error: 0
+			.saturating_add((226_000 as Weight).saturating_mul(m as Weight))
+			.saturating_add(T::DbWeight::get().reads(3 as Weight))
+			.saturating_add(T::DbWeight::get().writes(4 as Weight))
+	}
+	fn set_prime(m: u32, ) -> Weight {
+		(8_865_000 as Weight)
+			// Standard Error: 0
+			.saturating_add((124_000 as Weight).saturating_mul(m as Weight))
+			.saturating_add(T::DbWeight::get().reads(1 as Weight))
+			.saturating_add(T::DbWeight::get().writes(2 as Weight))
+	}
+	fn clear_prime(m: u32, ) -> Weight {
+		(3_397_000 as Weight)
+			// Standard Error: 0
+			.saturating_add((1_000 as Weight).saturating_mul(m as Weight))
+			.saturating_add(T::DbWeight::get().writes(2 as Weight))
+	}
+}
+
+// For backwards compatibility and tests
+impl WeightInfo for () {
+	fn add_member(m: u32, ) -> Weight {
+		(25_448_000 as Weight)
+			// Standard Error: 3_000
+			.saturating_add((257_000 as Weight).saturating_mul(m as Weight))
+			.saturating_add(RocksDbWeight::get().reads(2 as Weight))
+			.saturating_add(RocksDbWeight::get().writes(3 as Weight))
+	}
+	fn remove_member(m: u32, ) -> Weight {
+		(31_317_000 as Weight)
+			// Standard Error: 0
+			.saturating_add((215_000 as Weight).saturating_mul(m as Weight))
+			.saturating_add(RocksDbWeight::get().reads(3 as Weight))
+			.saturating_add(RocksDbWeight::get().writes(3 as Weight))
+	}
+	fn swap_member(m: u32, ) -> Weight {
+		(31_208_000 as Weight)
+			// Standard Error: 0
+			.saturating_add((229_000 as Weight).saturating_mul(m as Weight))
+			.saturating_add(RocksDbWeight::get().reads(3 as Weight))
+			.saturating_add(RocksDbWeight::get().writes(3 as Weight))
+	}
+	fn reset_member(m: u32, ) -> Weight {
+		(31_673_000 as Weight)
+			// Standard Error: 1_000
+			.saturating_add((455_000 as Weight).saturating_mul(m as Weight))
+			.saturating_add(RocksDbWeight::get().reads(3 as Weight))
+			.saturating_add(RocksDbWeight::get().writes(3 as Weight))
+	}
+	fn change_key(m: u32, ) -> Weight {
+		(33_499_000 as Weight)
+			// Standard Error: 0
+			.saturating_add((226_000 as Weight).saturating_mul(m as Weight))
+			.saturating_add(RocksDbWeight::get().reads(3 as Weight))
+			.saturating_add(RocksDbWeight::get().writes(4 as Weight))
+	}
+	fn set_prime(m: u32, ) -> Weight {
+		(8_865_000 as Weight)
+			// Standard Error: 0
+			.saturating_add((124_000 as Weight).saturating_mul(m as Weight))
+			.saturating_add(RocksDbWeight::get().reads(1 as Weight))
+			.saturating_add(RocksDbWeight::get().writes(2 as Weight))
+	}
+	fn clear_prime(m: u32, ) -> Weight {
+		(3_397_000 as Weight)
+			// Standard Error: 0
+			.saturating_add((1_000 as Weight).saturating_mul(m as Weight))
+			.saturating_add(RocksDbWeight::get().writes(2 as Weight))
+	}
+}