Dynamic Benchmarking DB Whitelist (#6815)

* Add `get_whitelist` api

* add whitelisted caller

* Whitelist caller

* remove caller 0

* initial piping of origin (not actual value yet)

* remove attempt to pass origin around

* Add whitelist for `DidUpdate` storage on `pallet_timestamp`

* fix traits

* only add to whitelist if !contains

* PassBy not implemented error

* Whitelist read/writes explicitly per key

* update docs

* reduce trait constraint

* copy pasta

* Apply suggestions from code review

Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com>
Co-authored-by: Alexander Popiak <alexander.popiak@parity.io>

* rename functions @apopiak

* missed some renaming

* enable doc tests

* Update docs

Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com>
Co-authored-by: Alexander Popiak <alexander.popiak@parity.io>
This commit is contained in:
Shawn Tabrizi
2020-08-19 18:15:50 +02:00
committed by GitHub
parent 8b5ced7fa7
commit 481ad884d6
26 changed files with 421 additions and 205 deletions
+185 -108
View File
@@ -32,6 +32,7 @@ pub use sp_io::storage::root as storage_root;
pub use sp_runtime::traits::Zero;
pub use frame_support;
pub use paste;
pub use sp_storage::TrackedStorageKey;
/// Construct pallet benchmarks for weighing dispatchables.
///
@@ -418,156 +419,220 @@ macro_rules! benchmarks_iter {
#[doc(hidden)]
macro_rules! benchmark_backend {
// parsing arms
( { $( $instance:ident )? } $name:ident {
$( $where_clause:tt )*
} {
$( $common:tt )*
} {
$( PRE { $( $pre_parsed:tt )* } )*
} { $eval:block } {
(
{ $( $instance:ident )? }
$name:ident
{ $( $where_clause:tt )* }
{ $( $common:tt )* }
{ $( PRE { $( $pre_parsed:tt )* } )* }
{ $eval:block }
{
let $pre_id:tt : $pre_ty:ty = $pre_ex:expr;
$( $rest:tt )*
} $postcode:block) => {
}
$postcode:block
) => {
$crate::benchmark_backend! {
{ $( $instance)? } $name { $( $where_clause )* } { $( $common )* } {
{ $( $instance)? }
$name
{ $( $where_clause )* }
{ $( $common )* }
{
$( PRE { $( $pre_parsed )* } )*
PRE { $pre_id , $pre_ty , $pre_ex }
} { $eval } { $( $rest )* } $postcode
}
{ $eval }
{ $( $rest )* }
$postcode
}
};
( { $( $instance:ident )? } $name:ident {
$( $where_clause:tt )*
} {
$( $common:tt )*
} {
$( $parsed:tt )*
} { $eval:block } {
let $param:ident in ( $param_from:expr ) .. $param_to:expr => $param_instancer:expr;
$( $rest:tt )*
} $postcode:block) => {
(
{ $( $instance:ident )? }
$name:ident
{ $( $where_clause:tt )* }
{ $( $common:tt )* }
{ $( $parsed:tt )* }
{ $eval:block }
{
let $param:ident in ( $param_from:expr ) .. $param_to:expr => $param_instancer:expr;
$( $rest:tt )*
}
$postcode:block
) => {
$crate::benchmark_backend! {
{ $( $instance)? } $name { $( $where_clause )* } { $( $common )* } {
{ $( $instance)? }
$name
{ $( $where_clause )* }
{ $( $common )* }
{
$( $parsed )*
PARAM { $param , $param_from , $param_to , $param_instancer }
} { $eval } { $( $rest )* } $postcode
}
{ $eval }
{ $( $rest )* }
$postcode
}
};
// mutation arm to look after defaulting to a common param
( { $( $instance:ident )? } $name:ident {
$( $where_clause:tt )*
} {
$( { $common:ident , $common_from:tt , $common_to:expr , $common_instancer:expr } )*
} {
$( $parsed:tt )*
} { $eval:block } {
let $param:ident in ...;
$( $rest:tt )*
} $postcode:block) => {
(
{ $( $instance:ident )? }
$name:ident
{ $( $where_clause:tt )* }
{ $( { $common:ident , $common_from:tt , $common_to:expr , $common_instancer:expr } )* }
{ $( $parsed:tt )* }
{ $eval:block }
{
let $param:ident in ...;
$( $rest:tt )*
}
$postcode:block
) => {
$crate::benchmark_backend! {
{ $( $instance)? } $name { $( $where_clause )* } {
$( { $common , $common_from , $common_to , $common_instancer } )*
} {
$( $parsed )*
} { $eval } {
{ $( $instance)? }
$name
{ $( $where_clause )* }
{ $( { $common , $common_from , $common_to , $common_instancer } )* }
{ $( $parsed )* }
{ $eval }
{
let $param
in ({ $( let $common = $common_from; )* $param })
.. ({ $( let $common = $common_to; )* $param })
=> ({ $( let $common = || -> Result<(), &'static str> { $common_instancer ; Ok(()) }; )* $param()? });
$( $rest )*
} $postcode
}
$postcode
}
};
// mutation arm to look after defaulting only the range to common param
( { $( $instance:ident )? } $name:ident {
$( $where_clause:tt )*
} {
$( { $common:ident , $common_from:tt , $common_to:expr , $common_instancer:expr } )*
} {
$( $parsed:tt )*
} { $eval:block } {
let $param:ident in _ .. _ => $param_instancer:expr ;
$( $rest:tt )*
} $postcode:block) => {
(
{ $( $instance:ident )? }
$name:ident
{ $( $where_clause:tt )* }
{ $( { $common:ident , $common_from:tt , $common_to:expr , $common_instancer:expr } )* }
{ $( $parsed:tt )* }
{ $eval:block }
{
let $param:ident in _ .. _ => $param_instancer:expr ;
$( $rest:tt )*
}
$postcode:block
) => {
$crate::benchmark_backend! {
{ $( $instance)? } $name { $( $where_clause )* } {
$( { $common , $common_from , $common_to , $common_instancer } )*
} {
$( $parsed )*
} { $eval } {
{ $( $instance)? }
$name
{ $( $where_clause )* }
{ $( { $common , $common_from , $common_to , $common_instancer } )* }
{ $( $parsed )* }
{ $eval }
{
let $param
in ({ $( let $common = $common_from; )* $param })
.. ({ $( let $common = $common_to; )* $param })
=> $param_instancer ;
$( $rest )*
} $postcode
}
$postcode
}
};
// mutation arm to look after a single tt for param_from.
( { $( $instance:ident )? } $name:ident {
$( $where_clause:tt )*
} {
$( $common:tt )*
} {
$( $parsed:tt )*
} { $eval:block } {
let $param:ident in $param_from:tt .. $param_to:expr => $param_instancer:expr ;
$( $rest:tt )*
} $postcode:block) => {
(
{ $( $instance:ident )? }
$name:ident
{ $( $where_clause:tt )* }
{ $( $common:tt )* }
{ $( $parsed:tt )* }
{ $eval:block }
{
let $param:ident in $param_from:tt .. $param_to:expr => $param_instancer:expr ;
$( $rest:tt )*
}
$postcode:block
) => {
$crate::benchmark_backend! {
{ $( $instance)? }
$name { $( $where_clause )* } { $( $common )* } { $( $parsed )* } { $eval } {
$name
{ $( $where_clause )* }
{ $( $common )* }
{ $( $parsed )* }
{ $eval }
{
let $param in ( $param_from ) .. $param_to => $param_instancer;
$( $rest )*
} $postcode
}
$postcode
}
};
// mutation arm to look after the default tail of `=> ()`
( { $( $instance:ident )? } $name:ident {
$( $where_clause:tt )*
} {
$( $common:tt )*
} {
$( $parsed:tt )*
} { $eval:block } {
let $param:ident in $param_from:tt .. $param_to:expr;
$( $rest:tt )*
} $postcode:block) => {
(
{ $( $instance:ident )? }
$name:ident
{ $( $where_clause:tt )* }
{ $( $common:tt )* }
{ $( $parsed:tt )* }
{ $eval:block }
{
let $param:ident in $param_from:tt .. $param_to:expr;
$( $rest:tt )*
}
$postcode:block
) => {
$crate::benchmark_backend! {
{ $( $instance)? }
$name { $( $where_clause )* } { $( $common )* } { $( $parsed )* } { $eval } {
$name
{ $( $where_clause )* }
{ $( $common )* }
{ $( $parsed )* }
{ $eval }
{
let $param in $param_from .. $param_to => ();
$( $rest )*
} $postcode
}
$postcode
}
};
// mutation arm to look after `let _ =`
( { $( $instance:ident )? } $name:ident {
$( $where_clause:tt )*
} {
$( $common:tt )*
} {
$( $parsed:tt )*
} { $eval:block } {
let $pre_id:tt = $pre_ex:expr;
$( $rest:tt )*
} $postcode:block) => {
(
{ $( $instance:ident )? }
$name:ident
{ $( $where_clause:tt )* }
{ $( $common:tt )* }
{ $( $parsed:tt )* }
{ $eval:block }
{
let $pre_id:tt = $pre_ex:expr;
$( $rest:tt )*
}
$postcode:block
) => {
$crate::benchmark_backend! {
{ $( $instance)? }
$name { $( $where_clause )* } { $( $common )* } { $( $parsed )* } { $eval } {
$name
{ $( $where_clause )* }
{ $( $common )* }
{ $( $parsed )* }
{ $eval }
{
let $pre_id : _ = $pre_ex;
$( $rest )*
} $postcode
}
$postcode
}
};
// actioning arm
( { $( $instance:ident )? } $name:ident {
$( $where_clause:tt )*
} {
$( { $common:ident , $common_from:tt , $common_to:expr , $common_instancer:expr } )*
} {
$( 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 )* } $postcode:block) => {
(
{ $( $instance:ident )? }
$name:ident
{ $( $where_clause:tt )* }
{ $( { $common:ident , $common_from:tt , $common_to:expr , $common_instancer:expr } )* }
{
$( 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 )* }
$postcode:block
) => {
#[allow(non_camel_case_types)]
struct $name;
#[allow(unused_variables)]
@@ -726,7 +791,7 @@ macro_rules! impl_benchmark {
highest_range_values: &[u32],
steps: &[u32],
repeat: u32,
whitelist: &[Vec<u8>]
whitelist: &[$crate::TrackedStorageKey]
) -> Result<Vec<$crate::BenchmarkResults>, &'static str> {
// Map the input to the selected benchmark.
let extrinsic = sp_std::str::from_utf8(extrinsic)
@@ -736,8 +801,14 @@ macro_rules! impl_benchmark {
_ => return Err("Could not find extrinsic."),
};
// Add whitelist to DB
$crate::benchmarking::set_whitelist(whitelist.to_vec());
// Add whitelist to DB including whitelisted caller
let mut whitelist = whitelist.to_vec();
let whitelisted_caller_key =
<frame_system::Account::<T> as frame_support::storage::StorageMap<_,_>>::hashed_key_for(
$crate::whitelisted_caller::<T::AccountId>()
);
whitelist.push(whitelisted_caller_key.into());
$crate::benchmarking::set_whitelist(whitelist);
// Warm up the DB
$crate::benchmarking::commit_db();
@@ -947,19 +1018,25 @@ macro_rules! impl_benchmark_test {
/// let params = (&pallet, &benchmark, &lowest_range_values, &highest_range_values, &steps, repeat, &whitelist);
/// ```
///
/// The `whitelist` is a `Vec<Vec<u8>>` of storage keys that you would like to skip for DB tracking. For example:
/// The `whitelist` is a parameter you pass to control the DB read/write tracking.
/// We use a vector of [TrackedStorageKey](./struct.TrackedStorageKey.html), which is a simple struct used to set
/// if a key has been read or written to.
///
/// ```ignore
/// let whitelist: Vec<Vec<u8>> = vec![
/// For values that should be skipped entirely, we can just pass `key.into()`. For example:
///
/// ```
/// use frame_benchmarking::TrackedStorageKey;
/// let whitelist: Vec<TrackedStorageKey> = vec![
/// // Block Number
/// hex_literal::hex!("26aa394eea5630e07c48ae0c9558cef702a5c1b19ab7a04f536c519aca4983ac").to_vec(),
/// hex_literal::hex!("26aa394eea5630e07c48ae0c9558cef702a5c1b19ab7a04f536c519aca4983ac").to_vec().into(),
/// // Total Issuance
/// hex_literal::hex!("c2261276cc9d1f8598ea4b6a74b15c2f57c875e4cff74148e4628f264b974c80").to_vec(),
/// hex_literal::hex!("c2261276cc9d1f8598ea4b6a74b15c2f57c875e4cff74148e4628f264b974c80").to_vec().into(),
/// // Execution Phase
/// hex_literal::hex!("26aa394eea5630e07c48ae0c9558cef7ff553b5a9862a516939d82b3d3d8661a").to_vec(),
/// hex_literal::hex!("26aa394eea5630e07c48ae0c9558cef7ff553b5a9862a516939d82b3d3d8661a").to_vec().into(),
/// // Event Count
/// hex_literal::hex!("26aa394eea5630e07c48ae0c9558cef70a98fdbe9ce6c55837576c60c7af3850").to_vec(),
/// hex_literal::hex!("26aa394eea5630e07c48ae0c9558cef70a98fdbe9ce6c55837576c60c7af3850").to_vec().into(),
/// ];
/// ```
///
/// Then define a mutable local variable to hold your `BenchmarkBatch` object:
///
+3 -9
View File
@@ -20,7 +20,6 @@
#![cfg(test)]
use super::*;
use codec::Decode;
use sp_std::prelude::*;
use sp_runtime::{traits::{BlakeTwo256, IdentityLookup}, testing::{H256, Header}};
use frame_support::{
@@ -64,12 +63,10 @@ pub trait OtherTrait {
type OtherEvent;
}
pub trait Trait: OtherTrait where Self::OtherEvent: Into<Self::Event> {
pub trait Trait: frame_system::Trait + OtherTrait
where Self::OtherEvent: Into<<Self as Trait>::Event>
{
type Event;
type BlockNumber;
type AccountId: 'static + Default + Decode;
type Origin: From<frame_system::RawOrigin<Self::AccountId>> +
Into<Result<RawOrigin<Self::AccountId>, Self::Origin>>;
}
#[derive(Clone, Eq, PartialEq)]
@@ -105,9 +102,6 @@ impl frame_system::Trait for Test {
impl Trait for Test {
type Event = ();
type BlockNumber = u32;
type Origin = Origin;
type AccountId = u64;
}
impl OtherTrait for Test {
+43 -4
View File
@@ -21,6 +21,7 @@ use codec::{Encode, Decode};
use sp_std::{vec::Vec, prelude::Box};
use sp_io::hashing::blake2_256;
use sp_runtime::RuntimeString;
use sp_storage::TrackedStorageKey;
/// An alphabet of possible parameters to use for benchmarking.
#[derive(Encode, Decode, Clone, Copy, PartialEq, Debug)]
@@ -101,19 +102,52 @@ pub trait Benchmarking {
self.commit()
}
/// Get the read/write count
/// Get the read/write count.
fn read_write_count(&self) -> (u32, u32, u32, u32) {
self.read_write_count()
}
/// Reset the read/write count
/// Reset the read/write count.
fn reset_read_write_count(&mut self) {
self.reset_read_write_count()
}
fn set_whitelist(&mut self, new: Vec<Vec<u8>>) {
/// Get the DB whitelist.
fn get_whitelist(&self) -> Vec<TrackedStorageKey> {
self.get_whitelist()
}
/// Set the DB whitelist.
fn set_whitelist(&mut self, new: Vec<TrackedStorageKey>) {
self.set_whitelist(new)
}
// Add a new item to the DB whitelist.
fn add_to_whitelist(&mut self, add: TrackedStorageKey) {
let mut whitelist = self.get_whitelist();
match whitelist.iter_mut().find(|x| x.key == add.key) {
// If we already have this key in the whitelist, update to be the most constrained value.
Some(item) => {
*item = TrackedStorageKey {
key: add.key,
has_been_read: item.has_been_read || add.has_been_read,
has_been_written: item.has_been_written || add.has_been_written,
}
},
// If the key does not exist, add it.
None => {
whitelist.push(add);
}
}
self.set_whitelist(whitelist);
}
// Remove an item from the DB whitelist.
fn remove_from_whitelist(&mut self, remove: Vec<u8>) {
let mut whitelist = self.get_whitelist();
whitelist.retain(|x| x.key != remove);
self.set_whitelist(whitelist);
}
}
/// The pallet benchmarking trait.
@@ -141,7 +175,7 @@ pub trait Benchmarking<T> {
highest_range_values: &[u32],
steps: &[u32],
repeat: u32,
whitelist: &[Vec<u8>]
whitelist: &[TrackedStorageKey]
) -> Result<Vec<T>, &'static str>;
}
@@ -165,3 +199,8 @@ pub fn account<AccountId: Decode + Default>(name: &'static str, index: u32, seed
let entropy = (name, index, seed).using_encoded(blake2_256);
AccountId::decode(&mut &entropy[..]).unwrap_or_default()
}
/// This caller account is automatically whitelisted for DB reads/writes by the benchmarking macro.
pub fn whitelisted_caller<AccountId: Decode + Default>() -> AccountId {
account::<AccountId>("whitelisted_caller", 0, 0)
}