Skip to content

Commit 64569fe

Browse files
ggwpezark0f
authored andcommitted
Add Weightless benchmark bailing (paritytech#12829)
* 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>
1 parent eafd914 commit 64569fe

File tree

4 files changed

+49
-14
lines changed

4 files changed

+49
-14
lines changed

frame/benchmarking/src/lib.rs

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -881,6 +881,13 @@ macro_rules! impl_bench_name_tests {
881881
"WARNING: benchmark error skipped - {}",
882882
stringify!($name),
883883
);
884+
},
885+
$crate::BenchmarkError::Weightless => {
886+
// This is considered a success condition.
887+
$crate::log::error!(
888+
"WARNING: benchmark weightless skipped - {}",
889+
stringify!($name),
890+
);
884891
}
885892
}
886893
},
@@ -1640,6 +1647,14 @@ macro_rules! impl_test_function {
16401647
.expect("benchmark name is always a valid string!"),
16411648
);
16421649
}
1650+
$crate::BenchmarkError::Weightless => {
1651+
// This is considered a success condition.
1652+
$crate::log::error!(
1653+
"WARNING: benchmark weightless skipped - {}",
1654+
$crate::str::from_utf8(benchmark_name)
1655+
.expect("benchmark name is always a valid string!"),
1656+
);
1657+
}
16431658
}
16441659
},
16451660
Ok(Ok(())) => (),
@@ -1792,6 +1807,17 @@ macro_rules! add_benchmark {
17921807
.expect("benchmark name is always a valid string!")
17931808
);
17941809
None
1810+
},
1811+
Err($crate::BenchmarkError::Weightless) => {
1812+
$crate::log::error!(
1813+
"WARNING: benchmark weightless skipped - {}",
1814+
$crate::str::from_utf8(benchmark)
1815+
.expect("benchmark name is always a valid string!")
1816+
);
1817+
Some(vec![$crate::BenchmarkResult {
1818+
components: selected_components.clone(),
1819+
.. Default::default()
1820+
}])
17951821
}
17961822
};
17971823

frame/benchmarking/src/utils.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,11 @@ pub enum BenchmarkError {
162162
/// The benchmarking pipeline is allowed to fail here, and we should simply
163163
/// skip processing these results.
164164
Skip,
165+
/// No weight can be determined; set the weight of this call to zero.
166+
///
167+
/// You can also use `Override` instead, but this is easier to use since `Override` expects the
168+
/// correct components to be present.
169+
Weightless,
165170
}
166171

167172
impl From<BenchmarkError> for &'static str {
@@ -170,6 +175,7 @@ impl From<BenchmarkError> for &'static str {
170175
BenchmarkError::Stop(s) => s,
171176
BenchmarkError::Override(_) => "benchmark override",
172177
BenchmarkError::Skip => "benchmark skip",
178+
BenchmarkError::Weightless => "benchmark weightless",
173179
}
174180
}
175181
}

frame/bounties/src/benchmarking.rs

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121

2222
use super::*;
2323

24-
use frame_benchmarking::{account, benchmarks_instance_pallet, whitelisted_caller};
24+
use frame_benchmarking::{account, benchmarks_instance_pallet, whitelisted_caller, BenchmarkError};
2525
use frame_system::RawOrigin;
2626
use sp_runtime::traits::Bounded;
2727

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

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

6465
fn create_bounty<T: Config<I>, I: 'static>(
65-
) -> Result<(AccountIdLookupOf<T>, BountyIndex), &'static str> {
66+
) -> Result<(AccountIdLookupOf<T>, BountyIndex), BenchmarkError> {
6667
let (caller, curator, fee, value, reason) =
6768
setup_bounty::<T, I>(0, T::MaximumReasonLength::get());
6869
let curator_lookup = T::Lookup::unlookup(curator.clone());
6970
Bounties::<T, I>::propose_bounty(RawOrigin::Signed(caller).into(), value, reason)?;
7071
let bounty_id = BountyCount::<T, I>::get() - 1;
71-
let approve_origin = T::ApproveOrigin::successful_origin();
72+
let approve_origin =
73+
T::SpendOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?;
7274
Bounties::<T, I>::approve_bounty(approve_origin.clone(), bounty_id)?;
7375
Treasury::<T, I>::on_initialize(T::BlockNumber::zero());
7476
Bounties::<T, I>::propose_curator(approve_origin, bounty_id, curator_lookup.clone(), fee)?;
@@ -97,7 +99,7 @@ benchmarks_instance_pallet! {
9799
let (caller, curator, fee, value, reason) = setup_bounty::<T, I>(0, T::MaximumReasonLength::get());
98100
Bounties::<T, I>::propose_bounty(RawOrigin::Signed(caller).into(), value, reason)?;
99101
let bounty_id = BountyCount::<T, I>::get() - 1;
100-
let approve_origin = T::SpendOrigin::successful_origin();
102+
let approve_origin = T::SpendOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?;
101103
}: _<T::RuntimeOrigin>(approve_origin, bounty_id)
102104

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

115116
// Worst case when curator is inactive and any sender unassigns the curator.
@@ -128,7 +129,7 @@ benchmarks_instance_pallet! {
128129
let curator_lookup = T::Lookup::unlookup(curator.clone());
129130
Bounties::<T, I>::propose_bounty(RawOrigin::Signed(caller).into(), value, reason)?;
130131
let bounty_id = BountyCount::<T, I>::get() - 1;
131-
let approve_origin = T::SpendOrigin::successful_origin();
132+
let approve_origin = T::SpendOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?;
132133
Bounties::<T, I>::approve_bounty(approve_origin.clone(), bounty_id)?;
133134
Treasury::<T, I>::on_initialize(T::BlockNumber::zero());
134135
Bounties::<T, I>::propose_curator(approve_origin, bounty_id, curator_lookup, fee)?;

frame/child-bounties/src/benchmarking.rs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121

2222
use super::*;
2323

24-
use frame_benchmarking::{account, benchmarks, whitelisted_caller};
24+
use frame_benchmarking::{account, benchmarks, whitelisted_caller, BenchmarkError};
2525
use frame_system::RawOrigin;
2626

2727
use crate::Pallet as ChildBounties;
@@ -94,7 +94,7 @@ fn setup_child_bounty<T: Config>(user: u32, description: u32) -> BenchmarkChildB
9494
fn activate_bounty<T: Config>(
9595
user: u32,
9696
description: u32,
97-
) -> Result<BenchmarkChildBounty<T>, &'static str> {
97+
) -> Result<BenchmarkChildBounty<T>, BenchmarkError> {
9898
let mut child_bounty_setup = setup_child_bounty::<T>(user, description);
9999
let curator_lookup = T::Lookup::unlookup(child_bounty_setup.curator.clone());
100100
Bounties::<T>::propose_bounty(
@@ -105,7 +105,9 @@ fn activate_bounty<T: Config>(
105105

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

108-
Bounties::<T>::approve_bounty(RawOrigin::Root.into(), child_bounty_setup.bounty_id)?;
108+
let approve_origin =
109+
T::SpendOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?;
110+
Bounties::<T>::approve_bounty(approve_origin, child_bounty_setup.bounty_id)?;
109111
Treasury::<T>::on_initialize(T::BlockNumber::zero());
110112
Bounties::<T>::propose_curator(
111113
RawOrigin::Root.into(),
@@ -124,7 +126,7 @@ fn activate_bounty<T: Config>(
124126
fn activate_child_bounty<T: Config>(
125127
user: u32,
126128
description: u32,
127-
) -> Result<BenchmarkChildBounty<T>, &'static str> {
129+
) -> Result<BenchmarkChildBounty<T>, BenchmarkError> {
128130
let mut bounty_setup = activate_bounty::<T>(user, description)?;
129131
let child_curator_lookup = T::Lookup::unlookup(bounty_setup.child_curator.clone());
130132

0 commit comments

Comments
 (0)