diff --git a/frame/alliance/src/benchmarking.rs b/frame/alliance/src/benchmarking.rs index e07d7c44a97ff..e2e1579fcc9b4 100644 --- a/frame/alliance/src/benchmarking.rs +++ b/frame/alliance/src/benchmarking.rs @@ -832,8 +832,8 @@ benchmarks_instance_pallet! { } add_unscrupulous_items { - let n in 1 .. T::MaxUnscrupulousItems::get(); - let l in 1 .. T::MaxWebsiteUrlLength::get(); + let n in 0 .. T::MaxUnscrupulousItems::get(); + let l in 0 .. T::MaxWebsiteUrlLength::get(); set_members::(); @@ -856,8 +856,8 @@ benchmarks_instance_pallet! { } remove_unscrupulous_items { - let n in 1 .. T::MaxUnscrupulousItems::get(); - let l in 1 .. T::MaxWebsiteUrlLength::get(); + let n in 0 .. T::MaxUnscrupulousItems::get(); + let l in 0 .. T::MaxWebsiteUrlLength::get(); set_members::(); diff --git a/frame/benchmarking/src/baseline.rs b/frame/benchmarking/src/baseline.rs index 6a310330cafb1..5fd845551daca 100644 --- a/frame/benchmarking/src/baseline.rs +++ b/frame/benchmarking/src/baseline.rs @@ -90,7 +90,7 @@ benchmarks! { } sr25519_verification { - let i in 1 .. 100; + let i in 0 .. 100; let public = SignerId::generate_pair(None); diff --git a/frame/bounties/src/benchmarking.rs b/frame/bounties/src/benchmarking.rs index 6ccd587cebc10..07dd781c29af3 100644 --- a/frame/bounties/src/benchmarking.rs +++ b/frame/bounties/src/benchmarking.rs @@ -197,7 +197,7 @@ benchmarks_instance_pallet! { } spend_funds { - let b in 1 .. 100; + let b in 0 .. 100; setup_pot_account::(); create_approved_bounties::(b)?; @@ -214,9 +214,13 @@ benchmarks_instance_pallet! { ); } verify { - ensure!(budget_remaining < BalanceOf::::max_value(), "Budget not used"); ensure!(missed_any == false, "Missed some"); - assert_last_event::(Event::BountyBecameActive { index: b - 1 }.into()) + if b > 0 { + ensure!(budget_remaining < BalanceOf::::max_value(), "Budget not used"); + assert_last_event::(Event::BountyBecameActive { index: b - 1 }.into()) + } else { + ensure!(budget_remaining == BalanceOf::::max_value(), "Budget used"); + } } impl_benchmark_test_suite!(Bounties, crate::tests::new_test_ext(), crate::tests::Test) diff --git a/frame/collective/src/benchmarking.rs b/frame/collective/src/benchmarking.rs index 7c444dbaa7b6a..fcebacf5762e7 100644 --- a/frame/collective/src/benchmarking.rs +++ b/frame/collective/src/benchmarking.rs @@ -36,68 +36,69 @@ fn assert_last_event, I: 'static>(generic_event: >:: benchmarks_instance_pallet! { set_members { - let m in 1 .. T::MaxMembers::get(); - let n in 1 .. T::MaxMembers::get(); - let p in 1 .. T::MaxProposals::get(); + let m in 0 .. T::MaxMembers::get(); + let n in 0 .. T::MaxMembers::get(); + let p in 0 .. T::MaxProposals::get(); // Set old members. // We compute the difference of old and new members, so it should influence timing. let mut old_members = vec![]; - let mut last_old_member = account::("old member", 0, SEED); for i in 0 .. m { - last_old_member = account::("old member", i, SEED); - old_members.push(last_old_member.clone()); + let old_member = account::("old member", i, SEED); + old_members.push(old_member); } let old_members_count = old_members.len() as u32; Collective::::set_members( SystemOrigin::Root.into(), old_members.clone(), - Some(last_old_member.clone()), + old_members.last().cloned(), T::MaxMembers::get(), )?; - // Set a high threshold for proposals passing so that they stay around. - let threshold = m.max(2); - // Length of the proposals should be irrelevant to `set_members`. - let length = 100; - for i in 0 .. p { - // Proposals should be different so that different proposal hashes are generated - let proposal: T::Proposal = SystemCall::::remark { - remark: vec![i as u8; length] - }.into(); - Collective::::propose( - SystemOrigin::Signed(last_old_member.clone()).into(), - threshold, - Box::new(proposal.clone()), - MAX_BYTES, - )?; - let hash = T::Hashing::hash_of(&proposal); - // Vote on the proposal to increase state relevant for `set_members`. - // Not voting for `last_old_member` because they proposed and not voting for the first member - // to keep the proposal from passing. - for j in 2 .. m - 1 { - let voter = &old_members[j as usize]; - let approve = true; - Collective::::vote( - SystemOrigin::Signed(voter.clone()).into(), - hash, - i, - approve, + // If there were any old members generate a bunch of proposals. + if m > 0 { + // Set a high threshold for proposals passing so that they stay around. + let threshold = m.max(2); + // Length of the proposals should be irrelevant to `set_members`. + let length = 100; + for i in 0 .. p { + // Proposals should be different so that different proposal hashes are generated + let proposal: T::Proposal = SystemCall::::remark { + remark: vec![i as u8; length] + }.into(); + Collective::::propose( + SystemOrigin::Signed(old_members.last().unwrap().clone()).into(), + threshold, + Box::new(proposal.clone()), + MAX_BYTES, )?; + let hash = T::Hashing::hash_of(&proposal); + // Vote on the proposal to increase state relevant for `set_members`. + // Not voting for last old member because they proposed and not voting for the first member + // to keep the proposal from passing. + for j in 2 .. m - 1 { + let voter = &old_members[j as usize]; + let approve = true; + Collective::::vote( + SystemOrigin::Signed(voter.clone()).into(), + hash, + i, + approve, + )?; + } } } // Construct `new_members`. // It should influence timing since it will sort this vector. let mut new_members = vec![]; - let mut last_member = account::("member", 0, SEED); for i in 0 .. n { - last_member = account::("member", i, SEED); - new_members.push(last_member.clone()); + let member = account::("member", i, SEED); + new_members.push(member); } - }: _(SystemOrigin::Root, new_members.clone(), Some(last_member), T::MaxMembers::get()) + }: _(SystemOrigin::Root, new_members.clone(), new_members.last().cloned(), T::MaxMembers::get()) verify { new_members.sort(); assert_eq!(Collective::::members(), new_members); diff --git a/frame/elections-phragmen/src/benchmarking.rs b/frame/elections-phragmen/src/benchmarking.rs index 22d00a912a4f7..06ac8d7c60162 100644 --- a/frame/elections-phragmen/src/benchmarking.rs +++ b/frame/elections-phragmen/src/benchmarking.rs @@ -362,7 +362,7 @@ benchmarks! { // total number of voters. let v in (T::MaxVoters::get() / 2) .. T::MaxVoters::get(); // those that are defunct and need removal. - let d in 1 .. (T::MaxVoters::get() / 2); + let d in 0 .. (T::MaxVoters::get() / 2); // remove any previous stuff. clean::(); diff --git a/frame/examples/basic/src/benchmarking.rs b/frame/examples/basic/src/benchmarking.rs index 93e14f358208e..4d1659af46460 100644 --- a/frame/examples/basic/src/benchmarking.rs +++ b/frame/examples/basic/src/benchmarking.rs @@ -34,22 +34,22 @@ use frame_system::RawOrigin; // Details on using the benchmarks macro can be seen at: // https://paritytech.github.io/substrate/master/frame_benchmarking/trait.Benchmarking.html#tymethod.benchmarks benchmarks! { - // This will measure the execution time of `set_dummy` for b in [1..1000] range. + // This will measure the execution time of `set_dummy` for b in [0..1000] range. set_dummy_benchmark { // This is the benchmark setup phase - let b in 1 .. 1000; + let b in 0 .. 1000; }: set_dummy(RawOrigin::Root, b.into()) // The execution phase is just running `set_dummy` extrinsic call verify { // This is the optional benchmark verification phase, asserting certain states. assert_eq!(Pallet::::dummy(), Some(b.into())) } - // This will measure the execution time of `accumulate_dummy` for b in [1..1000] range. + // This will measure the execution time of `accumulate_dummy` for b in [0..1000] range. // The benchmark execution phase is shorthanded. When the name of the benchmark case is the same // as the extrinsic call. `_(...)` is used to represent the extrinsic name. // The benchmark verification phase is omitted. accumulate_dummy { - let b in 1 .. 1000; + let b in 0 .. 1000; // The caller account is whitelisted for DB reads/write by the benchmarking macro. let caller: T::AccountId = whitelisted_caller(); }: _(RawOrigin::Signed(caller), b.into()) diff --git a/frame/gilt/src/benchmarking.rs b/frame/gilt/src/benchmarking.rs index 1767a8da4def0..92ebf81854f23 100644 --- a/frame/gilt/src/benchmarking.rs +++ b/frame/gilt/src/benchmarking.rs @@ -97,7 +97,7 @@ benchmarks! { pursue_target_per_item { // bids taken - let b in 1..T::MaxQueueLen::get(); + let b in 0..T::MaxQueueLen::get(); let caller: T::AccountId = whitelisted_caller(); T::Currency::make_free_balance_be(&caller, T::MinFreeze::get() * BalanceOf::::from(b + 1)); @@ -113,7 +113,7 @@ benchmarks! { pursue_target_per_queue { // total queues hit - let q in 1..T::QueueCount::get(); + let q in 0..T::QueueCount::get(); let caller: T::AccountId = whitelisted_caller(); T::Currency::make_free_balance_be(&caller, T::MinFreeze::get() * BalanceOf::::from(q + 1)); diff --git a/frame/identity/src/benchmarking.rs b/frame/identity/src/benchmarking.rs index c66658b92b0d2..c628387a4d22e 100644 --- a/frame/identity/src/benchmarking.rs +++ b/frame/identity/src/benchmarking.rs @@ -130,7 +130,7 @@ benchmarks! { set_identity { let r in 1 .. T::MaxRegistrars::get() => add_registrars::(r)?; - let x in 1 .. T::MaxAdditionalFields::get(); + let x in 0 .. T::MaxAdditionalFields::get(); let caller = { // The target user let caller: T::AccountId = whitelisted_caller(); @@ -166,7 +166,7 @@ benchmarks! { set_subs_new { let caller: T::AccountId = whitelisted_caller(); // Create a new subs vec with s sub accounts - let s in 1 .. T::MaxSubAccounts::get() => (); + let s in 0 .. T::MaxSubAccounts::get() => (); let subs = create_sub_accounts::(&caller, s)?; ensure!(SubsOf::::get(&caller).1.len() == 0, "Caller already has subs"); }: set_subs(RawOrigin::Signed(caller.clone()), subs) @@ -177,7 +177,7 @@ benchmarks! { set_subs_old { let caller: T::AccountId = whitelisted_caller(); // Give them p many previous sub accounts. - let p in 1 .. T::MaxSubAccounts::get() => { + let p in 0 .. T::MaxSubAccounts::get() => { let _ = add_sub_accounts::(&caller, p)?; }; // Remove all subs. @@ -198,12 +198,12 @@ benchmarks! { let _ = T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); let r in 1 .. T::MaxRegistrars::get() => add_registrars::(r)?; - let s in 1 .. T::MaxSubAccounts::get() => { + let s in 0 .. T::MaxSubAccounts::get() => { // Give them s many sub accounts let caller: T::AccountId = whitelisted_caller(); let _ = add_sub_accounts::(&caller, s)?; }; - let x in 1 .. T::MaxAdditionalFields::get(); + let x in 0 .. T::MaxAdditionalFields::get(); // Create their main identity with x additional fields let info = create_identity_info::(x); @@ -233,7 +233,7 @@ benchmarks! { let _ = T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); let r in 1 .. T::MaxRegistrars::get() => add_registrars::(r)?; - let x in 1 .. T::MaxAdditionalFields::get() => { + let x in 0 .. T::MaxAdditionalFields::get() => { // Create their main identity with x additional fields let info = create_identity_info::(x); let caller: T::AccountId = whitelisted_caller(); @@ -251,7 +251,7 @@ benchmarks! { let _ = T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); let r in 1 .. T::MaxRegistrars::get() => add_registrars::(r)?; - let x in 1 .. T::MaxAdditionalFields::get() => { + let x in 0 .. T::MaxAdditionalFields::get() => { // Create their main identity with x additional fields let info = create_identity_info::(x); let caller: T::AccountId = whitelisted_caller(); @@ -332,7 +332,7 @@ benchmarks! { let _ = T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); let r in 1 .. T::MaxRegistrars::get() - 1 => add_registrars::(r)?; - let x in 1 .. T::MaxAdditionalFields::get(); + let x in 0 .. T::MaxAdditionalFields::get(); let info = create_identity_info::(x); let info_hash = T::Hashing::hash_of(&info); @@ -348,8 +348,8 @@ benchmarks! { kill_identity { let r in 1 .. T::MaxRegistrars::get() => add_registrars::(r)?; - let s in 1 .. T::MaxSubAccounts::get(); - let x in 1 .. T::MaxAdditionalFields::get(); + let s in 0 .. T::MaxSubAccounts::get(); + let x in 0 .. T::MaxAdditionalFields::get(); let target: T::AccountId = account("target", 0, SEED); let target_origin: ::RuntimeOrigin = RawOrigin::Signed(target.clone()).into(); @@ -379,7 +379,7 @@ benchmarks! { } add_sub { - let s in 1 .. T::MaxSubAccounts::get() - 1; + let s in 0 .. T::MaxSubAccounts::get() - 1; let caller: T::AccountId = whitelisted_caller(); let _ = add_sub_accounts::(&caller, s)?; @@ -415,7 +415,7 @@ benchmarks! { } quit_sub { - let s in 1 .. T::MaxSubAccounts::get() - 1; + let s in 0 .. T::MaxSubAccounts::get() - 1; let caller: T::AccountId = whitelisted_caller(); let sup = account("super", 0, SEED); diff --git a/frame/ranked-collective/src/benchmarking.rs b/frame/ranked-collective/src/benchmarking.rs index a4d074450e836..eb629b330abb2 100644 --- a/frame/ranked-collective/src/benchmarking.rs +++ b/frame/ranked-collective/src/benchmarking.rs @@ -138,7 +138,7 @@ benchmarks_instance_pallet! { } cleanup_poll { - let n in 1 .. 100; + let n in 0 .. 100; // Create a poll let class = T::Polls::classes().into_iter().next().unwrap(); diff --git a/frame/staking/src/benchmarking.rs b/frame/staking/src/benchmarking.rs index c7e6936ac75d8..dcb861e2ce419 100644 --- a/frame/staking/src/benchmarking.rs +++ b/frame/staking/src/benchmarking.rs @@ -88,6 +88,7 @@ pub fn create_validator_with_nominators( points_total += 10; points_individual.push((v_stash.clone(), 10)); + let original_nominator_count = Nominators::::count(); let mut nominators = Vec::new(); // Give the validator n nominators, but keep total users in the system the same. @@ -114,7 +115,7 @@ pub fn create_validator_with_nominators( assert_eq!(new_validators.len(), 1); assert_eq!(new_validators[0], v_stash, "Our validator was not selected!"); assert_ne!(Validators::::count(), 0); - assert_ne!(Nominators::::count(), 0); + assert_eq!(Nominators::::count(), original_nominator_count + nominators.len() as u32); // Give Era Points let reward = EraRewardPoints:: { @@ -544,7 +545,7 @@ benchmarks! { } payout_stakers_dead_controller { - let n in 1 .. T::MaxNominatorRewardedPerValidator::get() as u32; + let n in 0 .. T::MaxNominatorRewardedPerValidator::get() as u32; let (validator, nominators) = create_validator_with_nominators::( n, T::MaxNominatorRewardedPerValidator::get() as u32, @@ -577,7 +578,7 @@ benchmarks! { } payout_stakers_alive_staked { - let n in 1 .. T::MaxNominatorRewardedPerValidator::get() as u32; + let n in 0 .. T::MaxNominatorRewardedPerValidator::get() as u32; let (validator, nominators) = create_validator_with_nominators::( n, T::MaxNominatorRewardedPerValidator::get() as u32, @@ -695,7 +696,7 @@ benchmarks! { new_era { let v in 1 .. 10; - let n in 1 .. 100; + let n in 0 .. 100; create_validators_with_nominators_for_era::( v, @@ -714,7 +715,7 @@ benchmarks! { #[extra] payout_all { let v in 1 .. 10; - let n in 1 .. 100; + let n in 0 .. 100; create_validators_with_nominators_for_era::( v, n, diff --git a/frame/system/benchmarking/src/lib.rs b/frame/system/benchmarking/src/lib.rs index dbe38da5775a7..0f7603fe1dd9f 100644 --- a/frame/system/benchmarking/src/lib.rs +++ b/frame/system/benchmarking/src/lib.rs @@ -64,7 +64,7 @@ benchmarks! { #[skip_meta] set_storage { - let i in 1 .. 1000; + let i in 0 .. 1000; // Set up i items to add let mut items = Vec::new(); @@ -72,56 +72,69 @@ benchmarks! { let hash = (i, j).using_encoded(T::Hashing::hash).as_ref().to_vec(); items.push((hash.clone(), hash.clone())); } + + let items_to_verify = items.clone(); }: _(RawOrigin::Root, items) verify { - let last_hash = (i, i - 1).using_encoded(T::Hashing::hash); - let value = storage::unhashed::get_raw(last_hash.as_ref()).ok_or("No value stored")?; - assert_eq!(value, last_hash.as_ref().to_vec()); + // Verify that they're actually in the storage. + for (item, _) in items_to_verify { + let value = storage::unhashed::get_raw(&item).ok_or("No value stored")?; + assert_eq!(value, *item); + } } #[skip_meta] kill_storage { - let i in 1 .. 1000; + let i in 0 .. 1000; // Add i items to storage - let mut items = Vec::new(); + let mut items = Vec::with_capacity(i as usize); for j in 0 .. i { let hash = (i, j).using_encoded(T::Hashing::hash).as_ref().to_vec(); storage::unhashed::put_raw(&hash, &hash); items.push(hash); } - // We will verify this value is removed - let last_hash = (i, i - 1).using_encoded(T::Hashing::hash); - let value = storage::unhashed::get_raw(last_hash.as_ref()).ok_or("No value stored")?; - assert_eq!(value, last_hash.as_ref().to_vec()); + // Verify that they're actually in the storage. + for item in &items { + let value = storage::unhashed::get_raw(item).ok_or("No value stored")?; + assert_eq!(value, *item); + } + let items_to_verify = items.clone(); }: _(RawOrigin::Root, items) verify { - assert_eq!(storage::unhashed::get_raw(last_hash.as_ref()), None); + // Verify that they're not in the storage anymore. + for item in items_to_verify { + assert!(storage::unhashed::get_raw(&item).is_none()); + } } #[skip_meta] kill_prefix { - let p in 1 .. 1000; + let p in 0 .. 1000; let prefix = p.using_encoded(T::Hashing::hash).as_ref().to_vec(); + let mut items = Vec::with_capacity(p as usize); // add p items that share a prefix for i in 0 .. p { let hash = (p, i).using_encoded(T::Hashing::hash).as_ref().to_vec(); let key = [&prefix[..], &hash[..]].concat(); storage::unhashed::put_raw(&key, &key); + items.push(key); } - // We will verify this value is removed - let last_hash = (p, p - 1).using_encoded(T::Hashing::hash).as_ref().to_vec(); - let last_key = [&prefix[..], &last_hash[..]].concat(); - let value = storage::unhashed::get_raw(&last_key).ok_or("No value stored")?; - assert_eq!(value, last_key); - + // Verify that they're actually in the storage. + for item in &items { + let value = storage::unhashed::get_raw(item).ok_or("No value stored")?; + assert_eq!(value, *item); + } }: _(RawOrigin::Root, prefix, p) verify { - assert_eq!(storage::unhashed::get_raw(&last_key), None); + // Verify that they're not in the storage anymore. + for item in items { + assert!(storage::unhashed::get_raw(&item).is_none()); + } } impl_benchmark_test_suite!(Pallet, crate::mock::new_test_ext(), crate::mock::Test);