Add verify block to benchmark tests (#5551)

* Add verify block to benchmarks macro.

* Update all benchmarks.

* Add tests, add params.

* Should panic.

* ups, add closures

* Update tests.rs

* update macro syntax

* Revert benchmark syntax change

* verify only in tests

* Update tests.rs

* Uncomment staking

* Fix tests for benchmark instance

* Add docs

* Update frame/benchmarking/src/lib.rs

* add trace logs to benchmarks

* Verify returns closure

Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>
This commit is contained in:
Marcio Diaz
2020-04-08 12:55:06 +02:00
committed by GitHub
parent c73398ab03
commit 7cc095ec9f
6 changed files with 332 additions and 70 deletions
+225 -30
View File
@@ -28,7 +28,7 @@ pub use utils::*;
pub use analysis::Analysis;
#[doc(hidden)]
pub use sp_io::storage::root as storage_root;
pub use sp_runtime::traits::Dispatchable;
pub use sp_runtime::traits::{Dispatchable, Zero};
pub use paste;
/// Construct pallet benchmarks for weighing dispatchables.
@@ -132,6 +132,25 @@ pub use paste;
/// benchmark just like a regular benchmark, but only testing at the lowest and highest values for
/// each component. The function will return `Ok(())` if the benchmarks return no errors.
///
/// You can optionally add a `verify` code block at the end of a benchmark to test any final state
/// of your benchmark in a unit test. For example:
///
/// ```ignore
/// sort_vector {
/// let x in 1 .. 10000;
/// let mut m = Vec::<u32>::new();
/// for i in (0..x).rev() {
/// m.push(i);
/// }
/// }: {
/// m.sort();
/// } verify {
/// ensure!(m[0] == 0, "You forgot to sort!")
/// }
/// ```
///
/// These `verify` blocks will not execute when running your actual benchmarks!
///
/// You can construct benchmark tests like so:
///
/// ```ignore
@@ -192,6 +211,7 @@ macro_rules! benchmarks_iter {
{ $( $common:tt )* }
( $( $names:ident )* )
$name:ident { $( $code:tt )* }: _ ( $origin:expr $( , $arg:expr )* )
verify $postcode:block
$( $rest:tt )*
) => {
$crate::benchmarks_iter! {
@@ -199,6 +219,7 @@ macro_rules! benchmarks_iter {
{ $( $common )* }
( $( $names )* )
$name { $( $code )* }: $name ( $origin $( , $arg )* )
verify $postcode
$( $rest )*
}
};
@@ -208,6 +229,7 @@ macro_rules! benchmarks_iter {
{ $( $common:tt )* }
( $( $names:ident )* )
$name:ident { $( $code:tt )* }: $dispatch:ident ( $origin:expr $( , $arg:expr )* )
verify $postcode:block
$( $rest:tt )*
) => {
$crate::benchmarks_iter! {
@@ -217,6 +239,7 @@ macro_rules! benchmarks_iter {
$name { $( $code )* }: {
<Call<T> as $crate::Dispatchable>::dispatch(Call::<T>::$dispatch($($arg),*), $origin.into())?;
}
verify $postcode
$( $rest )*
}
};
@@ -226,6 +249,7 @@ macro_rules! benchmarks_iter {
{ $( $common:tt )* }
( $( $names:ident )* )
$name:ident { $( $code:tt )* }: $dispatch:ident ( $origin:expr $( , $arg:expr )* )
verify $postcode:block
$( $rest:tt )*
) => {
$crate::benchmarks_iter! {
@@ -235,6 +259,7 @@ macro_rules! benchmarks_iter {
$name { $( $code )* }: {
<Call<T, I> as $crate::Dispatchable>::dispatch(Call::<T, I>::$dispatch($($arg),*), $origin.into())?;
}
verify $postcode
$( $rest )*
}
};
@@ -244,6 +269,7 @@ macro_rules! benchmarks_iter {
{ $( $common:tt )* }
( $( $names:ident )* )
$name:ident { $( $code:tt )* }: $eval:block
verify $postcode:block
$( $rest:tt )*
) => {
$crate::benchmark_backend! {
@@ -253,6 +279,7 @@ macro_rules! benchmarks_iter {
{ }
{ $eval }
{ $( $code )* }
$postcode
}
$crate::benchmarks_iter!(
$instance
@@ -266,8 +293,59 @@ macro_rules! benchmarks_iter {
$crate::selected_benchmark!( $instance $( $names ),* );
$crate::impl_benchmark!( $instance $( $names ),* );
#[cfg(test)]
$crate::impl_benchmark_tests!( $( $names ),* );
}
$crate::impl_benchmark_tests!( $instance $( $names ),* );
};
// add verify block to _() format
(
$instance:ident
{ $( $common:tt )* }
( $( $names:ident )* )
$name:ident { $( $code:tt )* }: _ ( $origin:expr $( , $arg:expr )* )
$( $rest:tt )*
) => {
$crate::benchmarks_iter! {
$instance
{ $( $common )* }
( $( $names )* )
$name { $( $code )* }: _ ( $origin $( , $arg )* )
verify { }
$( $rest )*
}
};
// add verify block to name() format
(
$instance:ident
{ $( $common:tt )* }
( $( $names:ident )* )
$name:ident { $( $code:tt )* }: $dispatch:ident ( $origin:expr $( , $arg:expr )* )
$( $rest:tt )*
) => {
$crate::benchmarks_iter! {
$instance
{ $( $common )* }
( $( $names )* )
$name { $( $code )* }: $dispatch ( $origin $( , $arg )* )
verify { }
$( $rest )*
}
};
// add verify block to {} format
(
$instance:ident
{ $( $common:tt )* }
( $( $names:ident )* )
$name:ident { $( $code:tt )* }: $eval:block
$( $rest:tt )*
) => {
$crate::benchmarks_iter!(
$instance
{ $( $common )* }
( $( $names )* )
$name { $( $code )* }: $eval
verify { }
$( $rest )*
);
};
}
#[macro_export]
@@ -281,12 +359,12 @@ macro_rules! benchmark_backend {
} { $eval:block } {
let $pre_id:tt : $pre_ty:ty = $pre_ex:expr;
$( $rest:tt )*
} ) => {
} $postcode:block) => {
$crate::benchmark_backend! {
$instance $name { $( $common )* } {
$( PRE { $( $pre_parsed )* } )*
PRE { $pre_id , $pre_ty , $pre_ex }
} { $eval } { $( $rest )* }
} { $eval } { $( $rest )* } $postcode
}
};
($instance:ident $name:ident {
@@ -296,12 +374,12 @@ macro_rules! benchmark_backend {
} { $eval:block } {
let $param:ident in ( $param_from:expr ) .. $param_to:expr => $param_instancer:expr;
$( $rest:tt )*
}) => {
} $postcode:block) => {
$crate::benchmark_backend! {
$instance $name { $( $common )* } {
$( $parsed )*
PARAM { $param , $param_from , $param_to , $param_instancer }
} { $eval } { $( $rest )* }
} { $eval } { $( $rest )* } $postcode
}
};
// mutation arm to look after defaulting to a common param
@@ -312,7 +390,7 @@ macro_rules! benchmark_backend {
} { $eval:block } {
let $param:ident in ...;
$( $rest:tt )*
}) => {
} $postcode:block) => {
$crate::benchmark_backend! {
$instance $name {
$( { $common , $common_from , $common_to , $common_instancer } )*
@@ -324,7 +402,7 @@ macro_rules! benchmark_backend {
.. ({ $( let $common = $common_to; )* $param })
=> ({ $( let $common = || -> Result<(), &'static str> { $common_instancer ; Ok(()) }; )* $param()? });
$( $rest )*
}
} $postcode
}
};
// mutation arm to look after defaulting only the range to common param
@@ -335,7 +413,7 @@ macro_rules! benchmark_backend {
} { $eval:block } {
let $param:ident in _ .. _ => $param_instancer:expr ;
$( $rest:tt )*
}) => {
} $postcode:block) => {
$crate::benchmark_backend! {
$instance $name {
$( { $common , $common_from , $common_to , $common_instancer } )*
@@ -347,7 +425,7 @@ macro_rules! benchmark_backend {
.. ({ $( let $common = $common_to; )* $param })
=> $param_instancer ;
$( $rest )*
}
} $postcode
}
};
// mutation arm to look after a single tt for param_from.
@@ -358,12 +436,12 @@ macro_rules! benchmark_backend {
} { $eval:block } {
let $param:ident in $param_from:tt .. $param_to:expr => $param_instancer:expr ;
$( $rest:tt )*
}) => {
} $postcode:block) => {
$crate::benchmark_backend! {
$instance $name { $( $common )* } { $( $parsed )* } { $eval } {
let $param in ( $param_from ) .. $param_to => $param_instancer;
$( $rest )*
}
} $postcode
}
};
// mutation arm to look after the default tail of `=> ()`
@@ -374,12 +452,12 @@ macro_rules! benchmark_backend {
} { $eval:block } {
let $param:ident in $param_from:tt .. $param_to:expr;
$( $rest:tt )*
}) => {
} $postcode:block) => {
$crate::benchmark_backend! {
$instance $name { $( $common )* } { $( $parsed )* } { $eval } {
let $param in $param_from .. $param_to => ();
$( $rest )*
}
} $postcode
}
};
// mutation arm to look after `let _ =`
@@ -390,12 +468,12 @@ macro_rules! benchmark_backend {
} { $eval:block } {
let $pre_id:tt = $pre_ex:expr;
$( $rest:tt )*
}) => {
} $postcode:block) => {
$crate::benchmark_backend! {
$instance $name { $( $common )* } { $( $parsed )* } { $eval } {
let $pre_id : _ = $pre_ex;
$( $rest )*
}
} $postcode
}
};
// no instance actioning arm
@@ -404,7 +482,7 @@ macro_rules! benchmark_backend {
} {
$( PRE { $pre_id:tt , $pre_ty:ty , $pre_ex:expr } )*
$( PARAM { $param:ident , $param_from:expr , $param_to:expr , $param_instancer:expr } )*
} { $eval:block } { $( $post:tt )* } ) => {
} { $eval:block } { $( $post:tt )* } $postcode:block) => {
#[allow(non_camel_case_types)]
struct $name;
#[allow(unused_variables)]
@@ -435,6 +513,25 @@ macro_rules! benchmark_backend {
Ok(Box::new(move || -> Result<(), &'static str> { $eval; Ok(()) }))
}
fn verify(&self, components: &[($crate::BenchmarkParameter, u32)])
-> Result<Box<dyn FnOnce() -> Result<(), &'static str>>, &'static str>
{
$(
let $common = $common_from;
)*
$(
// Prepare instance
let $param = components.iter().find(|&c| c.0 == $crate::BenchmarkParameter::$param).unwrap().1;
)*
$(
let $pre_id : $pre_ty = $pre_ex;
)*
$( $param_instancer ; )*
$( $post )*
Ok(Box::new(move || -> Result<(), &'static str> { $eval; $postcode; Ok(()) }))
}
}
};
// instance actioning arm
@@ -443,7 +540,7 @@ macro_rules! benchmark_backend {
} {
$( PRE { $pre_id:tt , $pre_ty:ty , $pre_ex:expr } )*
$( PARAM { $param:ident , $param_from:expr , $param_to:expr , $param_instancer:expr } )*
} { $eval:block } { $( $post:tt )* } ) => {
} { $eval:block } { $( $post:tt )* } $postcode:block) => {
#[allow(non_camel_case_types)]
struct $name;
#[allow(unused_variables)]
@@ -474,6 +571,25 @@ macro_rules! benchmark_backend {
Ok(Box::new(move || -> Result<(), &'static str> { $eval; Ok(()) }))
}
fn verify(&self, components: &[($crate::BenchmarkParameter, u32)])
-> Result<Box<dyn FnOnce() -> Result<(), &'static str>>, &'static str>
{
$(
let $common = $common_from;
)*
$(
// Prepare instance
let $param = components.iter().find(|&c| c.0 == $crate::BenchmarkParameter::$param).unwrap().1;
)*
$(
let $pre_id : $pre_ty = $pre_ex;
)*
$( $param_instancer ; )*
$( $post )*
Ok(Box::new(move || -> Result<(), &'static str> { $eval; $postcode; Ok(()) }))
}
}
}
}
@@ -518,6 +634,14 @@ macro_rules! selected_benchmark {
$( Self::$bench => <$bench as $crate::BenchmarkingSetup<T>>::instance(&$bench, components), )*
}
}
fn verify(&self, components: &[($crate::BenchmarkParameter, u32)])
-> Result<Box<dyn FnOnce() -> Result<(), &'static str>>, &'static str>
{
match self {
$( Self::$bench => <$bench as $crate::BenchmarkingSetup<T>>::verify(&$bench, components), )*
}
}
}
};
(
@@ -544,6 +668,14 @@ macro_rules! selected_benchmark {
$( Self::$bench => <$bench as $crate::BenchmarkingSetupInstance<T, I>>::instance(&$bench, components), )*
}
}
fn verify(&self, components: &[($crate::BenchmarkParameter, u32)])
-> Result<Box<dyn FnOnce() -> Result<(), &'static str>>, &'static str>
{
match self {
$( Self::$bench => <$bench as $crate::BenchmarkingSetupInstance<T, I>>::verify(&$bench, components), )*
}
}
}
}
}
@@ -618,20 +750,25 @@ macro_rules! impl_benchmark {
// Run the benchmark `repeat` times.
for _ in 0..repeat {
// Set the block number to 1 so events are deposited.
frame_system::Module::<T>::set_block_number(1.into());
// Set up the externalities environment for the setup we want to benchmark.
let closure_to_benchmark = <SelectedBenchmark as $crate::BenchmarkingSetup<T>>::instance(&selected_benchmark, &c)?;
// Set the block number to at least 1 so events are deposited.
if $crate::Zero::is_zero(&frame_system::Module::<T>::block_number()) {
frame_system::Module::<T>::set_block_number(1.into());
}
// Commit the externalities to the database, flushing the DB cache.
// This will enable worst case scenario for reading from the database.
$crate::benchmarking::commit_db();
// Time the extrinsic logic.
frame_support::debug::trace!(target: "benchmark", "Start Benchmark: {:?} {:?}", name, component_value);
let start_extrinsic = $crate::benchmarking::current_time();
closure_to_benchmark()?;
let finish_extrinsic = $crate::benchmarking::current_time();
let elapsed_extrinsic = finish_extrinsic - start_extrinsic;
frame_support::debug::trace!(target: "benchmark", "End Benchmark: {} ns", elapsed_extrinsic);
// Time the storage root recalculation.
let start_storage_root = $crate::benchmarking::current_time();
@@ -718,20 +855,25 @@ macro_rules! impl_benchmark {
// Run the benchmark `repeat` times.
for _ in 0..repeat {
// Set the block number to 1 so events are deposited.
frame_system::Module::<T>::set_block_number(1.into());
// Set up the externalities environment for the setup we want to benchmark.
let closure_to_benchmark = <SelectedBenchmark as $crate::BenchmarkingSetupInstance<T, I>>::instance(&selected_benchmark, &c)?;
// Set the block number to at least 1 so events are deposited.
if $crate::Zero::is_zero(&frame_system::Module::<T>::block_number()) {
frame_system::Module::<T>::set_block_number(1.into());
}
// Commit the externalities to the database, flushing the DB cache.
// This will enable worst case scenario for reading from the database.
$crate::benchmarking::commit_db();
// Time the extrinsic logic.
frame_support::debug::trace!(target: "benchmark", "Start Benchmark: {:?} {:?}", name, component_value);
let start_extrinsic = $crate::benchmarking::current_time();
closure_to_benchmark()?;
let finish_extrinsic = $crate::benchmarking::current_time();
let elapsed_extrinsic = finish_extrinsic - start_extrinsic;
frame_support::debug::trace!(target: "benchmark", "End Benchmark: {} ns", elapsed_extrinsic);
// Time the storage root recalculation.
let start_storage_root = $crate::benchmarking::current_time();
@@ -758,6 +900,7 @@ macro_rules! impl_benchmark {
#[macro_export]
macro_rules! impl_benchmark_tests {
(
NO_INSTANCE
$( $name:ident ),*
) => {
$(
@@ -783,12 +926,17 @@ macro_rules! impl_benchmark_tests {
)
.collect();
// Set the block number to 1 so events are deposited.
frame_system::Module::<T>::set_block_number(1.into());
// Set up the externalities environment for the setup we want to benchmark.
let closure_to_benchmark = <SelectedBenchmark as $crate::BenchmarkingSetup<T>>::instance(&selected_benchmark, &c)?;
// Run the benchmark
closure_to_benchmark()?;
// Set up the verification state
let closure_to_verify = <SelectedBenchmark as $crate::BenchmarkingSetup<T>>::verify(&selected_benchmark, &c)?;
// Set the block number to at least 1 so events are deposited.
if $crate::Zero::is_zero(&frame_system::Module::<T>::block_number()) {
frame_system::Module::<T>::set_block_number(1.into());
}
// Run verification
closure_to_verify()?;
// Reset the state
$crate::benchmarking::wipe_db();
}
@@ -797,7 +945,54 @@ macro_rules! impl_benchmark_tests {
}
}
)*
}
};
(
INSTANCE
$( $name:ident ),*
) => {
$(
$crate::paste::item! {
fn [<test_benchmark_ $name>] <T: Trait> () -> Result<(), &'static str>
where T: frame_system::Trait
{
let selected_benchmark = SelectedBenchmark::$name;
let components = <SelectedBenchmark as $crate::BenchmarkingSetupInstance<T, _>>::components(&selected_benchmark);
for (_, (name, low, high)) in components.iter().enumerate() {
// Test only the low and high value, assuming values in the middle won't break
for component_value in vec![low, high] {
// Select the max value for all the other components.
let c: Vec<($crate::BenchmarkParameter, u32)> = components.iter()
.enumerate()
.map(|(_, (n, _, h))|
if n == name {
(*n, *component_value)
} else {
(*n, *h)
}
)
.collect();
// Set up the verification state
let closure_to_verify = <SelectedBenchmark as $crate::BenchmarkingSetupInstance<T, _>>::verify(&selected_benchmark, &c)?;
// Set the block number to at least 1 so events are deposited.
if $crate::Zero::is_zero(&frame_system::Module::<T>::block_number()) {
frame_system::Module::<T>::set_block_number(1.into());
}
// Run verification
closure_to_verify()?;
// Reset the state
$crate::benchmarking::wipe_db();
}
}
Ok(())
}
}
)*
};
}