update some weights

frame/identity/src/benchmarking.rs

@@ -157,7 +157,7 @@ benchmarks! {
 
 	// We need to split `set_subs` into two benchmarks to accurately isolate the potential
 	// writes caused by new or old sub accounts. The actual weight should simply be
-	// the greater of these two weights.
+	// the sum of these two weights.
 	set_subs_new {
 		let caller: T::AccountId = whitelisted_caller();
 		// Create a new subs vec with s sub accounts
@@ -344,7 +344,8 @@ mod tests {
 		new_test_ext().execute_with(|| {
 			assert_ok!(test_benchmark_add_registrar::<Test>());
 			assert_ok!(test_benchmark_set_identity::<Test>());
-			assert_ok!(test_benchmark_set_subs::<Test>());
+			assert_ok!(test_benchmark_set_subs_new::<Test>());
+			assert_ok!(test_benchmark_set_subs_old::<Test>());
 			assert_ok!(test_benchmark_clear_identity::<Test>());
 			assert_ok!(test_benchmark_request_judgement::<Test>());
 			assert_ok!(test_benchmark_cancel_request::<Test>());

frame/identity/src/default_weights.rs

@@ -24,110 +24,110 @@ use frame_support::weights::{Weight, constants::RocksDbWeight as DbWeight};
 
 impl crate::WeightInfo for () {
 	fn add_registrar(r: u32, ) -> Weight {
-		(38525000 as Weight)
-			.saturating_add((427000 as Weight).saturating_mul(r as Weight))
+		(38_700_000 as Weight)
+			.saturating_add((432_000 as Weight).saturating_mul(r as Weight))
 			.saturating_add(DbWeight::get().reads(1 as Weight))
 			.saturating_add(DbWeight::get().writes(1 as Weight))
 	}
 	fn set_identity(r: u32, x: u32, ) -> Weight {
-		(107872000 as Weight)
-			.saturating_add((419000 as Weight).saturating_mul(r as Weight))
-			.saturating_add((3013000 as Weight).saturating_mul(x as Weight))
+		(108_344_000 as Weight)
+			.saturating_add((394_000 as Weight).saturating_mul(r as Weight))
+			.saturating_add((2_997_000 as Weight).saturating_mul(x as Weight))
 			.saturating_add(DbWeight::get().reads(1 as Weight))
 			.saturating_add(DbWeight::get().writes(1 as Weight))
 	}
 	fn set_subs_new(s: u32, ) -> Weight {
-		(76452000 as Weight)
-			.saturating_add((14836000 as Weight).saturating_mul(s as Weight))
+		(76_128_000 as Weight)
+			.saturating_add((14_854_000 as Weight).saturating_mul(s as Weight))
 			.saturating_add(DbWeight::get().reads(2 as Weight))
 			.saturating_add(DbWeight::get().reads((1 as Weight).saturating_mul(s as Weight)))
 			.saturating_add(DbWeight::get().writes(1 as Weight))
 			.saturating_add(DbWeight::get().writes((1 as Weight).saturating_mul(s as Weight)))
 	}
 	fn set_subs_old(p: u32, ) -> Weight {
-		(70274000 as Weight)
-			.saturating_add((5518000 as Weight).saturating_mul(p as Weight))
+		(69_632_000 as Weight)
+			.saturating_add((5_532_000 as Weight).saturating_mul(p as Weight))
 			.saturating_add(DbWeight::get().reads(2 as Weight))
 			.saturating_add(DbWeight::get().writes(1 as Weight))
 			.saturating_add(DbWeight::get().writes((1 as Weight).saturating_mul(p as Weight)))
 	}
 	fn add_sub(p: u32, ) -> Weight {
-		(108119000 as Weight)
-			.saturating_add((245000 as Weight).saturating_mul(p as Weight))
+		(107_628_000 as Weight)
+			.saturating_add((240_000 as Weight).saturating_mul(p as Weight))
 			.saturating_add(DbWeight::get().reads(3 as Weight))
 			.saturating_add(DbWeight::get().writes(2 as Weight))
 	}
 	fn rename_sub(p: u32, ) -> Weight {
-		(35829000 as Weight)
-			.saturating_add((67000 as Weight).saturating_mul(p as Weight))
+		(35_566_000 as Weight)
+			.saturating_add((70_000 as Weight).saturating_mul(p as Weight))
 			.saturating_add(DbWeight::get().reads(2 as Weight))
 			.saturating_add(DbWeight::get().writes(1 as Weight))
 	}
 	fn remove_sub(p: u32, ) -> Weight {
-		(101562000 as Weight)
-			.saturating_add((216000 as Weight).saturating_mul(p as Weight))
+		(100_689_000 as Weight)
+			.saturating_add((220_000 as Weight).saturating_mul(p as Weight))
 			.saturating_add(DbWeight::get().reads(3 as Weight))
 			.saturating_add(DbWeight::get().writes(2 as Weight))
 	}
 	fn quit_sub(p: u32, ) -> Weight {
-		(64745000 as Weight)
-			.saturating_add((210000 as Weight).saturating_mul(p as Weight))
+		(64_411_000 as Weight)
+			.saturating_add((208_000 as Weight).saturating_mul(p as Weight))
 			.saturating_add(DbWeight::get().reads(2 as Weight))
 			.saturating_add(DbWeight::get().writes(2 as Weight))
 	}
 	fn clear_identity(r: u32, s: u32, x: u32, ) -> Weight {
-		(89640000 as Weight)
-			.saturating_add((270000 as Weight).saturating_mul(r as Weight))
-			.saturating_add((5488000 as Weight).saturating_mul(s as Weight))
-			.saturating_add((1647000 as Weight).saturating_mul(x as Weight))
+		(89_185_000 as Weight)
+			.saturating_add((250_000 as Weight).saturating_mul(r as Weight))
+			.saturating_add((5_552_000 as Weight).saturating_mul(s as Weight))
+			.saturating_add((1_663_000 as Weight).saturating_mul(x as Weight))
 			.saturating_add(DbWeight::get().reads(2 as Weight))
 			.saturating_add(DbWeight::get().writes(2 as Weight))
 			.saturating_add(DbWeight::get().writes((1 as Weight).saturating_mul(s as Weight)))
 	}
 	fn request_judgement(r: u32, x: u32, ) -> Weight {
-		(108673000 as Weight)
-			.saturating_add((491000 as Weight).saturating_mul(r as Weight))
-			.saturating_add((3236000 as Weight).saturating_mul(x as Weight))
+		(107_531_000 as Weight)
+			.saturating_add((536_000 as Weight).saturating_mul(r as Weight))
+			.saturating_add((3_237_000 as Weight).saturating_mul(x as Weight))
 			.saturating_add(DbWeight::get().reads(2 as Weight))
 			.saturating_add(DbWeight::get().writes(1 as Weight))
 	}
 	fn cancel_request(r: u32, x: u32, ) -> Weight {
-		(94605000 as Weight)
-			.saturating_add((341000 as Weight).saturating_mul(r as Weight))
-			.saturating_add((3205000 as Weight).saturating_mul(x as Weight))
+		(93_368_000 as Weight)
+			.saturating_add((371_000 as Weight).saturating_mul(r as Weight))
+			.saturating_add((3_225_000 as Weight).saturating_mul(x as Weight))
 			.saturating_add(DbWeight::get().reads(1 as Weight))
 			.saturating_add(DbWeight::get().writes(1 as Weight))
 	}
 	fn set_fee(r: u32, ) -> Weight {
-		(15928000 as Weight)
-			.saturating_add((377000 as Weight).saturating_mul(r as Weight))
+		(15_942_000 as Weight)
+			.saturating_add((390_000 as Weight).saturating_mul(r as Weight))
 			.saturating_add(DbWeight::get().reads(1 as Weight))
 			.saturating_add(DbWeight::get().writes(1 as Weight))
 	}
 	fn set_account_id(r: u32, ) -> Weight {
-		(17940000 as Weight)
-			.saturating_add((388000 as Weight).saturating_mul(r as Weight))
+		(18_135_000 as Weight)
+			.saturating_add((385_000 as Weight).saturating_mul(r as Weight))
 			.saturating_add(DbWeight::get().reads(1 as Weight))
 			.saturating_add(DbWeight::get().writes(1 as Weight))
 	}
 	fn set_fields(r: u32, ) -> Weight {
-		(15781000 as Weight)
-			.saturating_add((391000 as Weight).saturating_mul(r as Weight))
+		(15_962_000 as Weight)
+			.saturating_add((398_000 as Weight).saturating_mul(r as Weight))
 			.saturating_add(DbWeight::get().reads(1 as Weight))
 			.saturating_add(DbWeight::get().writes(1 as Weight))
 	}
 	fn provide_judgement(r: u32, x: u32, ) -> Weight {
-		(71415000 as Weight)
-			.saturating_add((445000 as Weight).saturating_mul(r as Weight))
-			.saturating_add((3212000 as Weight).saturating_mul(x as Weight))
+		(71_698_000 as Weight)
+			.saturating_add((423_000 as Weight).saturating_mul(r as Weight))
+			.saturating_add((3_215_000 as Weight).saturating_mul(x as Weight))
 			.saturating_add(DbWeight::get().reads(2 as Weight))
 			.saturating_add(DbWeight::get().writes(1 as Weight))
 	}
 	fn kill_identity(r: u32, s: u32, x: u32, ) -> Weight {
-		(115969000 as Weight)
-			.saturating_add((161000 as Weight).saturating_mul(r as Weight))
-			.saturating_add((5509000 as Weight).saturating_mul(s as Weight))
-			.saturating_add((1635000 as Weight).saturating_mul(x as Weight))
+		(111_774_000 as Weight)
+			.saturating_add((249_000 as Weight).saturating_mul(r as Weight))
+			.saturating_add((5_571_000 as Weight).saturating_mul(s as Weight))
+			.saturating_add((1_662_000 as Weight).saturating_mul(x as Weight))
 			.saturating_add(DbWeight::get().reads(2 as Weight))
 			.saturating_add(DbWeight::get().writes(2 as Weight))
 			.saturating_add(DbWeight::get().writes((1 as Weight).saturating_mul(s as Weight)))

frame/identity/src/lib.rs

@@ -516,49 +516,6 @@ mod weight_for {
 	use frame_support::{traits::Get, weights::Weight};
 	use super::Trait;
 
-	/// Weight calculation for `add_registrar`.
-	///
-	/// Based on benchmark:
-	/// 22.24 + R * 0.371 µs (min squares analysis)
-	pub(crate) fn add_registrar<T: Trait>(
-		registrars: Weight
-	) -> Weight {
-		T::DbWeight::get().reads_writes(1, 1)
-			+ 23_000_000 // constant
-			+ 380_000 * registrars // R
-	}
-
-	/// Weight calculation for `set_identity`.
-	///
-	/// Based on benchmark:
-	/// 50.64 + R * 0.215 + X * 1.424 µs (min squares analysis)
-	pub(crate) fn set_identity<T: Trait>(
-		judgements: Weight,
-		extra_fields: Weight
-	) -> Weight {
-		T::DbWeight::get().reads_writes(1, 1)
-			+ 51_000_000 // constant
-			+ 220_000 * judgements // R
-			+ 1_500_000 * extra_fields // X
-	}
-
-	/// Weight calculation for `set_subs`.
-	///
-	/// Based on benchmark:
-	/// 36.21 + P * 2.481 + S * 3.633 µs (min squares analysis)
-	pub(crate) fn set_subs<T: Trait>(
-		old_subs: Weight,
-		subs: Weight
-	) -> Weight {
-		let db = T::DbWeight::get();
-		db.reads(1) // storage-exists (`IdentityOf::contains_key`)
-			.saturating_add(db.reads_writes(1, old_subs)) // `SubsOf::get` read + P old DB deletions
-			.saturating_add(db.writes(subs + 1)) // S + 1 new DB writes
-			.saturating_add(37_000_000) // constant
-			.saturating_add(2_500_000 * old_subs) // P
-			.saturating_add(subs.saturating_mul(3_700_000)) // S
-	}
-
 	/// Weight calculation for `clear_identity`.
 	///
 	/// Based on benchmark:
@@ -708,7 +665,7 @@ decl_module! {
 		/// - One storage mutation (codec `O(R)`).
 		/// - One event.
 		/// # </weight>
-		#[weight = weight_for::add_registrar::<T>(T::MaxRegistrars::get().into()) ]
+		#[weight = T::WeightInfo::add_registrar(T::MaxRegistrars::get()) ]
 		fn add_registrar(origin, account: T::AccountId) -> DispatchResultWithPostInfo {
 			T::RegistrarOrigin::ensure_origin(origin)?;
 
@@ -724,7 +681,7 @@ decl_module! {
 
 			Self::deposit_event(RawEvent::RegistrarAdded(i));
 
-			Ok(Some(weight_for::add_registrar::<T>(registrar_count as Weight)).into())
+			Ok(Some(T::WeightInfo::add_registrar(registrar_count as u32)).into())
 		}
 
 		/// Set an account's identity information and reserve the appropriate deposit.
@@ -746,7 +703,7 @@ decl_module! {
 		/// - One storage mutation (codec-read `O(X' + R)`, codec-write `O(X + R)`).
 		/// - One event.
 		/// # </weight>
-		#[weight =  weight_for::set_identity::<T>(
+		#[weight =  T::WeightInfo::set_identity(
 			T::MaxRegistrars::get().into(), // R
 			T::MaxAdditionalFields::get().into(), // X
 		)]
@@ -775,13 +732,13 @@ decl_module! {
 				let _ = T::Currency::unreserve(&sender, old_deposit - id.deposit);
 			}
 
-			let judgements = id.judgements.len() as Weight;
+			let judgements = id.judgements.len();
 			<IdentityOf<T>>::insert(&sender, id);
 			Self::deposit_event(RawEvent::IdentitySet(sender));
 
-			Ok(Some(weight_for::set_identity::<T>(
-				judgements, // R
-				extra_fields as Weight // X
+			Ok(Some(T::WeightInfo::set_identity(
+				judgements as u32, // R
+				extra_fields // X
 			)).into())
 		}
 
@@ -806,10 +763,9 @@ decl_module! {
 		///   - One storage write (codec complexity `O(S)`).
 		///   - One storage-exists (`IdentityOf::contains_key`).
 		/// # </weight>
-		#[weight = weight_for::set_subs::<T>(
-			T::MaxSubAccounts::get().into(), // P
-			subs.len() as Weight // S
-		)]
+		#[weight = T::WeightInfo::set_subs_old(T::MaxSubAccounts::get()) // P: Assume max sub accounts removed.
+			.saturating_add(T::WeightInfo::set_subs_new(subs.len() as u32)) // S: Assume all subs are new.
+		]
 		fn set_subs(origin, subs: Vec<(T::AccountId, Data)>) -> DispatchResultWithPostInfo {
 			let sender = ensure_signed(origin)?;
 			ensure!(<IdentityOf<T>>::contains_key(&sender), Error::<T>::NotFound);
@@ -823,11 +779,15 @@ decl_module! {
 
 			if old_deposit < new_deposit {
 				T::Currency::reserve(&sender, new_deposit - old_deposit)?;
-			}
-			// do nothing if they're equal.
-			if old_deposit > new_deposit {
+			} else if old_deposit > new_deposit {
 				let _ = T::Currency::unreserve(&sender, old_deposit - new_deposit);
 			}
+			// do nothing if they're equal.
+
+			// TODO: This whole extrinsic screams "not optimized". For example we could
+			// filter any overlap between new and old subs, and avoid reading/writing
+			// to those values... We could also ideally avoid needing to write to
+			// N storage items for N sub accounts.
 
 			for s in old_ids.iter() {
 				<SuperOf<T>>::remove(s);
@@ -836,18 +796,18 @@ decl_module! {
 				<SuperOf<T>>::insert(&id, (sender.clone(), name));
 				id
 			}).collect::<Vec<_>>();
-			let new_subs = ids.len() as Weight;
+			let new_subs = ids.len();
 
 			if ids.is_empty() {
 				<SubsOf<T>>::remove(&sender);
 			} else {
 				<SubsOf<T>>::insert(&sender, (new_deposit, ids));
 			}
 
-			Ok(Some(weight_for::set_subs::<T>(
-				old_ids.len() as Weight, // P
-				new_subs // S
-			)).into())
+			Ok(Some(
+				T::WeightInfo::set_subs_old(old_ids.len() as u32) // P: Real number of old accounts removed.
+				.saturating_add(T::WeightInfo::set_subs_new(new_subs as u32)) // S: New subs added.
+			).into())
 		}
 
 		/// Clear an account's identity info and all sub-accounts and return all deposits.