Skip to content

Commit

Permalink
Add Weightless benchmark bailing (paritytech#12829)
Browse files Browse the repository at this point in the history 
* Calls can be 'Weightless'

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Fix (child)-bounties benches

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Just use one dummy value

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* 🤦

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
  • Loading branch information
@ggwpez
ggwpez authored Dec 2, 2022
1 parent c17c7d8 commit 34900ca
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 14 deletions.
26 changes: 26 additions & 0 deletions frame/benchmarking/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -881,6 +881,13 @@ macro_rules! impl_bench_name_tests {
"WARNING: benchmark error skipped - {}",
stringify!($name),
);
},
$crate::BenchmarkError::Weightless => {
// This is considered a success condition.
$crate::log::error!(
"WARNING: benchmark weightless skipped - {}",
stringify!($name),
);
}
}
},
Expand Down Expand Up @@ -1640,6 +1647,14 @@ macro_rules! impl_test_function {
.expect("benchmark name is always a valid string!"),
);
}
$crate::BenchmarkError::Weightless => {
// This is considered a success condition.
$crate::log::error!(
"WARNING: benchmark weightless skipped - {}",
$crate::str::from_utf8(benchmark_name)
.expect("benchmark name is always a valid string!"),
);
}
}
},
Ok(Ok(())) => (),
Expand Down Expand Up @@ -1792,6 +1807,17 @@ macro_rules! add_benchmark {
.expect("benchmark name is always a valid string!")
);
None
},
Err($crate::BenchmarkError::Weightless) => {
$crate::log::error!(
"WARNING: benchmark weightless skipped - {}",
$crate::str::from_utf8(benchmark)
.expect("benchmark name is always a valid string!")
);
Some(vec![$crate::BenchmarkResult {
components: selected_components.clone(),
.. Default::default()
}])
}
};

Expand Down
6 changes: 6 additions & 0 deletions frame/benchmarking/src/utils.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,11 @@ pub enum BenchmarkError {
/// The benchmarking pipeline is allowed to fail here, and we should simply
/// skip processing these results.
Skip,
/// No weight can be determined; set the weight of this call to zero.
///
/// You can also use `Override` instead, but this is easier to use since `Override` expects the
/// correct components to be present.
Weightless,
}

impl From<BenchmarkError> for &'static str {
Expand All @@ -170,6 +175,7 @@ impl From<BenchmarkError> for &'static str {
BenchmarkError::Stop(s) => s,
BenchmarkError::Override(_) => "benchmark override",
BenchmarkError::Skip => "benchmark skip",
BenchmarkError::Weightless => "benchmark weightless",
}
}
}
Expand Down
21 changes: 11 additions & 10 deletions frame/bounties/src/benchmarking.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

use super::*;

use frame_benchmarking::{account, benchmarks_instance_pallet, whitelisted_caller};
use frame_benchmarking::{account, benchmarks_instance_pallet, whitelisted_caller, BenchmarkError};
use frame_system::RawOrigin;
use sp_runtime::traits::Bounded;

Expand All @@ -31,13 +31,14 @@ use pallet_treasury::Pallet as Treasury;
const SEED: u32 = 0;

// Create bounties that are approved for use in `on_initialize`.
fn create_approved_bounties<T: Config<I>, I: 'static>(n: u32) -> Result<(), &'static str> {
fn create_approved_bounties<T: Config<I>, I: 'static>(n: u32) -> Result<(), BenchmarkError> {
for i in 0..n {
let (caller, _curator, _fee, value, reason) =
setup_bounty::<T, I>(i, T::MaximumReasonLength::get());
Bounties::<T, I>::propose_bounty(RawOrigin::Signed(caller).into(), value, reason)?;
let bounty_id = BountyCount::<T, I>::get() - 1;
let approve_origin = T::ApproveOrigin::successful_origin();
let approve_origin =
T::SpendOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?;
Bounties::<T, I>::approve_bounty(approve_origin, bounty_id)?;
}
ensure!(BountyApprovals::<T, I>::get().len() == n as usize, "Not all bounty approved");
Expand All @@ -62,13 +63,14 @@ fn setup_bounty<T: Config<I>, I: 'static>(
}

fn create_bounty<T: Config<I>, I: 'static>(
) -> Result<(AccountIdLookupOf<T>, BountyIndex), &'static str> {
) -> Result<(AccountIdLookupOf<T>, BountyIndex), BenchmarkError> {
let (caller, curator, fee, value, reason) =
setup_bounty::<T, I>(0, T::MaximumReasonLength::get());
let curator_lookup = T::Lookup::unlookup(curator.clone());
Bounties::<T, I>::propose_bounty(RawOrigin::Signed(caller).into(), value, reason)?;
let bounty_id = BountyCount::<T, I>::get() - 1;
let approve_origin = T::ApproveOrigin::successful_origin();
let approve_origin =
T::SpendOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?;
Bounties::<T, I>::approve_bounty(approve_origin.clone(), bounty_id)?;
Treasury::<T, I>::on_initialize(T::BlockNumber::zero());
Bounties::<T, I>::propose_curator(approve_origin, bounty_id, curator_lookup.clone(), fee)?;
Expand Down Expand Up @@ -97,7 +99,7 @@ benchmarks_instance_pallet! {
let (caller, curator, fee, value, reason) = setup_bounty::<T, I>(0, T::MaximumReasonLength::get());
Bounties::<T, I>::propose_bounty(RawOrigin::Signed(caller).into(), value, reason)?;
let bounty_id = BountyCount::<T, I>::get() - 1;
let approve_origin = T::SpendOrigin::successful_origin();
let approve_origin = T::SpendOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?;
}: _<T::RuntimeOrigin>(approve_origin, bounty_id)

propose_curator {
Expand All @@ -106,10 +108,9 @@ benchmarks_instance_pallet! {
let curator_lookup = T::Lookup::unlookup(curator);
Bounties::<T, I>::propose_bounty(RawOrigin::Signed(caller).into(), value, reason)?;
let bounty_id = BountyCount::<T, I>::get() - 1;
let approve_origin = T::SpendOrigin::successful_origin();
Bounties::<T, I>::approve_bounty(approve_origin, bounty_id)?;
let approve_origin = T::SpendOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?;
Bounties::<T, I>::approve_bounty(approve_origin.clone(), bounty_id)?;
Treasury::<T, I>::on_initialize(T::BlockNumber::zero());
let approve_origin = T::SpendOrigin::successful_origin();
}: _<T::RuntimeOrigin>(approve_origin, bounty_id, curator_lookup, fee)

// Worst case when curator is inactive and any sender unassigns the curator.
Expand All @@ -128,7 +129,7 @@ benchmarks_instance_pallet! {
let curator_lookup = T::Lookup::unlookup(curator.clone());
Bounties::<T, I>::propose_bounty(RawOrigin::Signed(caller).into(), value, reason)?;
let bounty_id = BountyCount::<T, I>::get() - 1;
let approve_origin = T::SpendOrigin::successful_origin();
let approve_origin = T::SpendOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?;
Bounties::<T, I>::approve_bounty(approve_origin.clone(), bounty_id)?;
Treasury::<T, I>::on_initialize(T::BlockNumber::zero());
Bounties::<T, I>::propose_curator(approve_origin, bounty_id, curator_lookup, fee)?;
Expand Down
10 changes: 6 additions & 4 deletions frame/child-bounties/src/benchmarking.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

use super::*;

use frame_benchmarking::{account, benchmarks, whitelisted_caller};
use frame_benchmarking::{account, benchmarks, whitelisted_caller, BenchmarkError};
use frame_system::RawOrigin;

use crate::Pallet as ChildBounties;
Expand Down Expand Up @@ -94,7 +94,7 @@ fn setup_child_bounty<T: Config>(user: u32, description: u32) -> BenchmarkChildB
fn activate_bounty<T: Config>(
user: u32,
description: u32,
) -> Result<BenchmarkChildBounty<T>, &'static str> {
) -> Result<BenchmarkChildBounty<T>, BenchmarkError> {
let mut child_bounty_setup = setup_child_bounty::<T>(user, description);
let curator_lookup = T::Lookup::unlookup(child_bounty_setup.curator.clone());
Bounties::<T>::propose_bounty(
Expand All @@ -105,7 +105,9 @@ fn activate_bounty<T: Config>(

child_bounty_setup.bounty_id = Bounties::<T>::bounty_count() - 1;

Bounties::<T>::approve_bounty(RawOrigin::Root.into(), child_bounty_setup.bounty_id)?;
let approve_origin =
T::SpendOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?;
Bounties::<T>::approve_bounty(approve_origin, child_bounty_setup.bounty_id)?;
Treasury::<T>::on_initialize(T::BlockNumber::zero());
Bounties::<T>::propose_curator(
RawOrigin::Root.into(),
Expand All @@ -124,7 +126,7 @@ fn activate_bounty<T: Config>(
fn activate_child_bounty<T: Config>(
user: u32,
description: u32,
) -> Result<BenchmarkChildBounty<T>, &'static str> {
) -> Result<BenchmarkChildBounty<T>, BenchmarkError> {
let mut bounty_setup = activate_bounty::<T>(user, description)?;
let child_curator_lookup = T::Lookup::unlookup(bounty_setup.child_curator.clone());

Expand Down

0 comments on commit 34900ca

Please sign in to comment.