Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Add special tag to exclude runtime storage items from benchmarking #12205

Merged
merged 72 commits into from
Sep 16, 2022
Merged
Show file tree
Hide file tree
Changes from 49 commits
Commits
Show all changes
72 commits
Select commit Hold shift + click to select a range
f08d3ae
initial setup
sam0x17 Aug 30, 2022
c381aef
add WhitelistedStorageKeys trait
sam0x17 Aug 30, 2022
1275c42
add (A, B) tuple implementation for whitelisted_storage_keys()
sam0x17 Aug 31, 2022
84beb0f
fix formatting
sam0x17 Aug 31, 2022
3699c19
implement WhitelistedStorageKeys for all tuple combinations
sam0x17 Aug 31, 2022
3056275
impl_for_tuples up to 128 for WhitelistedStorageKeys
sam0x17 Aug 31, 2022
adc7585
refactor to #[benchmarking(cached)]
sam0x17 Aug 31, 2022
f645511
tweak error message and mark BlockNumber as cached
sam0x17 Aug 31, 2022
1b8af15
add benchmarking(cached) to the other default types
sam0x17 Sep 1, 2022
2a1ae01
add docs for benchmarking(cached)
sam0x17 Sep 2, 2022
586bafb
properly parse storage type declaration
sam0x17 Sep 6, 2022
cfcd073
make storage_alias structs public so we can use them in this macro
sam0x17 Sep 6, 2022
8534891
use BTreeMap since TrackedStorageKey missing Ord outside of std
sam0x17 Sep 6, 2022
e2550b4
make WhitelistedStorageKeys accessible
sam0x17 Sep 6, 2022
157fb29
basic detection of benchmarking(cached) :boom:
sam0x17 Sep 6, 2022
5318167
proper parsing of #[benchmarking(cached)] from pallet parse macro
sam0x17 Sep 7, 2022
8cde8fe
store presence of #[benchmarking(cached)] macro on StorageDef
sam0x17 Sep 7, 2022
4cccd8a
compiling blank impl for WhitelistedStorageKeys
sam0x17 Sep 8, 2022
f21315c
move impl to expand_pallet_struct
sam0x17 Sep 8, 2022
faa63a7
use frame_support::sp_std::vec::Vec properly
sam0x17 Sep 8, 2022
73b01f1
successfully compiling with storage info loaded into a variable :boom:
sam0x17 Sep 9, 2022
fc6d239
plausible implementation for whitelisted_storage_keys()
sam0x17 Sep 9, 2022
f920bb9
use Pallet::whitelisted_storage_keys() instead of hard-coded list
sam0x17 Sep 9, 2022
0040a45
AllPallets::whitelisted_storage_keys() properly working :boom:
sam0x17 Sep 9, 2022
4a0fe7f
collect storage names
sam0x17 Sep 9, 2022
8ab6ab3
whitelisted_storage_keys() impl working :boom:
sam0x17 Sep 9, 2022
8ca026e
clean up
sam0x17 Sep 9, 2022
58a6229
fix compiler error
sam0x17 Sep 10, 2022
854a102
just one compiler error
sam0x17 Sep 10, 2022
771202c
fix doc compiler error
sam0x17 Sep 12, 2022
fe856ac
use better import path
sam0x17 Sep 12, 2022
0936ca4
fix comment
sam0x17 Sep 12, 2022
a72a280
whoops
sam0x17 Sep 12, 2022
c4ecb9a
whoops again
sam0x17 Sep 12, 2022
c6fd281
fix macro import issue
sam0x17 Sep 12, 2022
f360944
cargo fmt
sam0x17 Sep 12, 2022
92d5f5e
mark example as ignore
sam0x17 Sep 12, 2022
da51def
use keyword tokens instead of string parsing
sam0x17 Sep 13, 2022
e00420f
fix keyword-based parsing of benchmarking(cached)
sam0x17 Sep 13, 2022
a63acdf
preliminary spec for check_whitelist()
sam0x17 Sep 13, 2022
1a0ba3f
add additional test for benchmarking whitelist
sam0x17 Sep 13, 2022
d0ee2c6
add TODO note
sam0x17 Sep 13, 2022
2d1d637
remove irrelevant line from example
sam0x17 Sep 13, 2022
9ce39d2
use filter_map instead of filter and map
sam0x17 Sep 13, 2022
33828ea
simplify syntax
sam0x17 Sep 13, 2022
49b13ac
clean up
sam0x17 Sep 13, 2022
a2e8185
fix test
sam0x17 Sep 13, 2022
557a0c2
fix tests
sam0x17 Sep 13, 2022
d20bfe9
use keyword parsing instead of string parsing
sam0x17 Sep 14, 2022
1d31abe
use collect() instead of a for loop
sam0x17 Sep 14, 2022
dcb6312
fix compiler error
sam0x17 Sep 15, 2022
6e66eb1
clean up benchmarking(cached) marking code
sam0x17 Sep 15, 2022
dcda735
use cloned()
sam0x17 Sep 15, 2022
3f7bbc5
refactor to not use panic! and remove need for pub types in storage_a…
sam0x17 Sep 15, 2022
975f371
remove unneeded use
sam0x17 Sep 15, 2022
5ff3623
remove unneeded visibility changes
sam0x17 Sep 15, 2022
7214d0f
don't manually hard code hash for treasury account as hex
sam0x17 Sep 16, 2022
6e49069
proper Ord, PartialOrd, and Hash impls for TrackedStorageKey
sam0x17 Sep 16, 2022
4a71bd4
use BTreeSet instead of BTreeMap
sam0x17 Sep 16, 2022
7752321
fix comments
sam0x17 Sep 16, 2022
72a8ac8
cargo fmt
sam0x17 Sep 16, 2022
20e1375
switch to pallet::whitelist and re-do it basti's way :D
sam0x17 Sep 16, 2022
eb46fbb
make PartialOrd for TrackedStorageKey consistent with Ord
sam0x17 Sep 16, 2022
fd23e9e
more correct implementation of hash-related traits for TrackedStorageKey
sam0x17 Sep 16, 2022
d483a7a
fix integration test
sam0x17 Sep 16, 2022
05db4e5
update TODO
sam0x17 Sep 16, 2022
cb4ed97
remove unused keyword
sam0x17 Sep 16, 2022
4faf150
remove more unused keywords
sam0x17 Sep 16, 2022
91f33f0
use into_iter()
sam0x17 Sep 16, 2022
42127fc
Update frame/support/procedural/src/pallet/parse/mod.rs
sam0x17 Sep 16, 2022
cbf5959
add docs for whitelisted
sam0x17 Sep 16, 2022
ae1bf10
fix comment
sam0x17 Sep 16, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 39 additions & 12 deletions bin/node-template/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -514,18 +514,8 @@ impl_runtime_apis! {
impl frame_system_benchmarking::Config for Runtime {}
impl baseline::Config for Runtime {}

let whitelist: Vec<TrackedStorageKey> = vec![
// Block Number
hex_literal::hex!("26aa394eea5630e07c48ae0c9558cef702a5c1b19ab7a04f536c519aca4983ac").to_vec().into(),
// Total Issuance
hex_literal::hex!("c2261276cc9d1f8598ea4b6a74b15c2f57c875e4cff74148e4628f264b974c80").to_vec().into(),
// Execution Phase
hex_literal::hex!("26aa394eea5630e07c48ae0c9558cef7ff553b5a9862a516939d82b3d3d8661a").to_vec().into(),
// Event Count
hex_literal::hex!("26aa394eea5630e07c48ae0c9558cef70a98fdbe9ce6c55837576c60c7af3850").to_vec().into(),
// System Events
hex_literal::hex!("26aa394eea5630e07c48ae0c9558cef780d41e5e16056765bc8461851072c9d7").to_vec().into(),
];
use frame_support::traits::WhitelistedStorageKeys;
let whitelist: Vec<TrackedStorageKey> = AllPalletsWithSystem::whitelisted_storage_keys();

let mut batches = Vec::<BenchmarkBatch>::new();
let params = (&config, &whitelist);
Expand Down Expand Up @@ -556,3 +546,40 @@ impl_runtime_apis! {
}
}
}

#[cfg(test)]
mod tests {
use super::*;
use frame_support::traits::WhitelistedStorageKeys;
use sp_core::hexdisplay::HexDisplay;
use std::collections::HashSet;

#[test]
fn check_whitelist() {
let whitelist: HashSet<String> = AllPalletsWithSystem::whitelisted_storage_keys()
.iter()
.map(|e| HexDisplay::from(&e.key).to_string())
.collect();

// Block Number
assert!(
whitelist.contains("26aa394eea5630e07c48ae0c9558cef702a5c1b19ab7a04f536c519aca4983ac")
);
// Total Issuance
assert!(
whitelist.contains("c2261276cc9d1f8598ea4b6a74b15c2f57c875e4cff74148e4628f264b974c80")
);
// Execution Phase
assert!(
whitelist.contains("26aa394eea5630e07c48ae0c9558cef7ff553b5a9862a516939d82b3d3d8661a")
);
// Event Count
assert!(
whitelist.contains("26aa394eea5630e07c48ae0c9558cef70a98fdbe9ce6c55837576c60c7af3850")
);
// System Events
assert!(
whitelist.contains("26aa394eea5630e07c48ae0c9558cef780d41e5e16056765bc8461851072c9d7")
);
}
}
61 changes: 45 additions & 16 deletions bin/node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2142,22 +2142,15 @@ impl_runtime_apis! {
impl baseline::Config for Runtime {}
impl pallet_nomination_pools_benchmarking::Config for Runtime {}

let whitelist: Vec<TrackedStorageKey> = vec![
// Block Number
hex_literal::hex!("26aa394eea5630e07c48ae0c9558cef702a5c1b19ab7a04f536c519aca4983ac").to_vec().into(),
// Total Issuance
hex_literal::hex!("c2261276cc9d1f8598ea4b6a74b15c2f57c875e4cff74148e4628f264b974c80").to_vec().into(),
// Execution Phase
hex_literal::hex!("26aa394eea5630e07c48ae0c9558cef7ff553b5a9862a516939d82b3d3d8661a").to_vec().into(),
// Event Count
hex_literal::hex!("26aa394eea5630e07c48ae0c9558cef70a98fdbe9ce6c55837576c60c7af3850").to_vec().into(),
// System Events
hex_literal::hex!("26aa394eea5630e07c48ae0c9558cef780d41e5e16056765bc8461851072c9d7").to_vec().into(),
// System BlockWeight
hex_literal::hex!("26aa394eea5630e07c48ae0c9558cef734abf5cb34d6244378cddbf18e849d96").to_vec().into(),
// Treasury Account
hex_literal::hex!("26aa394eea5630e07c48ae0c9558cef7b99d880ec681799c0cf30e8886371da95ecffd7b6c0f78751baa9d281e0bfa3a6d6f646c70792f74727372790000000000000000000000000000000000000000").to_vec().into(),
];
use frame_support::traits::WhitelistedStorageKeys;
let mut whitelist: Vec<TrackedStorageKey> = AllPalletsWithSystem::whitelisted_storage_keys();
sam0x17 marked this conversation as resolved.
Show resolved Hide resolved

// Treasury Account (manually whitelist this for now since no
// accessible storage definition we can attach the
// benchmarking(cached) attr macro to)
// TODO: figure out a workaround for this that doesn't involve
// manually hard-coding
whitelist.push(hex_literal::hex!("26aa394eea5630e07c48ae0c9558cef7b99d880ec681799c0cf30e8886371da95ecffd7b6c0f78751baa9d281e0bfa3a6d6f646c70792f74727372790000000000000000000000000000000000000000").to_vec().into());
sam0x17 marked this conversation as resolved.
Show resolved Hide resolved

let mut batches = Vec::<BenchmarkBatch>::new();
let params = (&config, &whitelist);
Expand All @@ -2171,8 +2164,44 @@ impl_runtime_apis! {
mod tests {
use super::*;
use frame_election_provider_support::NposSolution;
use frame_support::traits::WhitelistedStorageKeys;
use frame_system::offchain::CreateSignedTransaction;
use sp_core::hexdisplay::HexDisplay;
use sp_runtime::UpperOf;
use std::collections::HashSet;

#[test]
fn check_whitelist() {
let whitelist: HashSet<String> = AllPalletsWithSystem::whitelisted_storage_keys()
.iter()
.map(|e| HexDisplay::from(&e.key).to_string())
.collect();

// Block Number
assert!(
whitelist.contains("26aa394eea5630e07c48ae0c9558cef702a5c1b19ab7a04f536c519aca4983ac")
);
// Total Issuance
assert!(
whitelist.contains("c2261276cc9d1f8598ea4b6a74b15c2f57c875e4cff74148e4628f264b974c80")
);
// Execution Phase
assert!(
whitelist.contains("26aa394eea5630e07c48ae0c9558cef7ff553b5a9862a516939d82b3d3d8661a")
);
// Event Count
assert!(
whitelist.contains("26aa394eea5630e07c48ae0c9558cef70a98fdbe9ce6c55837576c60c7af3850")
);
// System Events
assert!(
whitelist.contains("26aa394eea5630e07c48ae0c9558cef780d41e5e16056765bc8461851072c9d7")
);
// System BlockWeight
assert!(
whitelist.contains("26aa394eea5630e07c48ae0c9558cef734abf5cb34d6244378cddbf18e849d96")
);
}

#[test]
fn validate_transaction_submitter_bounds() {
Expand Down
3 changes: 2 additions & 1 deletion frame/balances/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ use codec::{Codec, Decode, Encode, MaxEncodedLen};
#[cfg(feature = "std")]
use frame_support::traits::GenesisBuild;
use frame_support::{
ensure,
benchmarking, ensure,
pallet_prelude::DispatchResult,
traits::{
tokens::{fungible, BalanceStatus as Status, DepositConsequence, WithdrawConsequence},
Expand Down Expand Up @@ -492,6 +492,7 @@ pub mod pallet {
/// The total units issued in the system.
#[pallet::storage]
#[pallet::getter(fn total_issuance)]
#[benchmarking(cached)]
pub type TotalIssuance<T: Config<I>, I: 'static = ()> = StorageValue<_, T::Balance, ValueQuery>;

/// The Balances pallet example of storing the balance of an account.
Expand Down
20 changes: 20 additions & 0 deletions frame/support/procedural/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -583,3 +583,23 @@ pub fn storage_alias(_: TokenStream, input: TokenStream) -> TokenStream {
.unwrap_or_else(|r| r.into_compile_error())
.into()
}

mod kw {
syn::custom_keyword!(cached);
}

#[proc_macro_attribute]
pub fn benchmarking(attr: TokenStream, item: TokenStream) -> TokenStream {
sam0x17 marked this conversation as resolved.
Show resolved Hide resolved
sam0x17 marked this conversation as resolved.
Show resolved Hide resolved
match syn::parse::<kw::cached>(attr) {
Ok(_) => benchmarking_cached(item),
Err(_) => panic!("only benchmarking(cached) is supported at this time"),
sam0x17 marked this conversation as resolved.
Show resolved Hide resolved
}
}

fn benchmarking_cached(item: TokenStream) -> TokenStream {
// re-use storage_alias's Input parser since it is accessible
// and valid for all storage item declarations
syn::parse2::<crate::storage_alias::Input>(item.clone().into())
.expect("benchmarking(cached) can only be attached to a valid storage type declaration");
sam0x17 marked this conversation as resolved.
Show resolved Hide resolved
item
}
19 changes: 19 additions & 0 deletions frame/support/procedural/src/pallet/expand/pallet_struct.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,24 @@ pub fn expand_pallet_struct(def: &mut Def) -> proc_macro2::TokenStream {
quote::quote! { #frame_support::traits::StorageVersion::default() }
};

let whitelisted_storage_idents: Vec<syn::Ident> = def
.storages
.iter()
.filter_map(|s| s.benchmarking_cached.then_some(s.ident.clone()))
.collect();

let whitelisted_storage_keys_impl = quote::quote![
use #frame_support::traits::{StorageInfoTrait, TrackedStorageKey, WhitelistedStorageKeys};
impl<#type_impl_gen> WhitelistedStorageKeys for #pallet_ident<#type_use_gen> #storages_where_clauses {
fn whitelisted_storage_keys() -> #frame_support::sp_std::vec::Vec<TrackedStorageKey> {
use #frame_support::sp_std::{vec, vec::Vec};
sam0x17 marked this conversation as resolved.
Show resolved Hide resolved
vec![#(
TrackedStorageKey::new(#whitelisted_storage_idents::<#type_use_gen>::hashed_key().to_vec())
),*]
}
}
];

quote::quote_spanned!(def.pallet_struct.attr_span =>
#pallet_error_metadata

Expand Down Expand Up @@ -253,5 +271,6 @@ pub fn expand_pallet_struct(def: &mut Def) -> proc_macro2::TokenStream {
}

#storage_info
#whitelisted_storage_keys_impl
)
}
33 changes: 30 additions & 3 deletions frame/support/procedural/src/pallet/parse/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@ pub mod type_value;
pub mod validate_unsigned;

use frame_support_procedural_tools::generate_crate_access_2018;
use syn::spanned::Spanned;
use proc_macro2::Group;
use quote::ToTokens;
use syn::{spanned::Spanned, AttrStyle};

/// Parsed definition of a pallet.
pub struct Def {
Expand Down Expand Up @@ -123,8 +125,31 @@ impl Def {
origin = Some(origin::OriginDef::try_from(index, item)?),
Some(PalletAttr::Inherent(_)) if inherent.is_none() =>
inherent = Some(inherent::InherentDef::try_from(index, item)?),
Some(PalletAttr::Storage(span)) =>
storages.push(storage::StorageDef::try_from(span, index, item)?),
Some(PalletAttr::Storage(span)) => {
let mut storage_def = storage::StorageDef::try_from(span, index, item)?;
// check for #[benchmarking(cached)] calls
if let syn::Item::Type(typ) = item {
sam0x17 marked this conversation as resolved.
Show resolved Hide resolved
for attr in typ.attrs.as_slice() {
if attr.style == AttrStyle::Outer {
if let Some(seg) = attr.path.segments.last() {
if let Ok(_) =
syn::parse2::<keyword::benchmarking>(seg.to_token_stream())
{
if let Ok(group) = syn::parse2::<Group>(attr.tokens.clone())
{
if let Ok(_) =
syn::parse2::<keyword::cached>(group.stream())
{
storage_def.benchmarking_cached = true;
sam0x17 marked this conversation as resolved.
Show resolved Hide resolved
sam0x17 marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
}
}
}
}
storages.push(storage_def);
},
Some(PalletAttr::ValidateUnsigned(_)) if validate_unsigned.is_none() => {
let v = validate_unsigned::ValidateUnsignedDef::try_from(index, item)?;
validate_unsigned = Some(v);
Expand Down Expand Up @@ -383,6 +408,8 @@ mod keyword {
syn::custom_keyword!(generate_store);
syn::custom_keyword!(Store);
syn::custom_keyword!(extra_constants);
syn::custom_keyword!(cached);
syn::custom_keyword!(benchmarking);
}

/// Parse attributes for item in pallet module
Expand Down
2 changes: 2 additions & 0 deletions frame/support/procedural/src/pallet/parse/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ pub struct StorageDef {
pub named_generics: Option<StorageGenerics>,
/// If the value stored in this storage is unbounded.
pub unbounded: bool,
pub benchmarking_cached: bool,
}

/// The parsed generic from the
Expand Down Expand Up @@ -814,6 +815,7 @@ impl StorageDef {
cfg_attrs,
named_generics,
unbounded,
benchmarking_cached: false,
})
}
}
1 change: 1 addition & 0 deletions frame/support/procedural/src/storage/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ pub(crate) use instance_trait::INHERENT_INSTANCE_NAME;
use frame_support_procedural_tools::{
generate_crate_access, generate_hidden_includes, syn_ext as ext,
};

use quote::quote;

/// All information contained in input of decl_storage
Expand Down
24 changes: 12 additions & 12 deletions frame/support/procedural/src/storage_alias.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ use syn::{
};

/// Represents a path that only consists of [`Ident`] separated by `::`.
struct SimplePath {
pub struct SimplePath {
leading_colon: Option<Token![::]>,
segments: Punctuated<Ident, Token![::]>,
}
Expand Down Expand Up @@ -65,7 +65,7 @@ impl ToTokens for SimplePath {

/// Represents generics which only support [`Ident`] separated by commas as you would pass it to a
/// type.
struct TypeGenerics {
pub struct TypeGenerics {
lt_token: Token![<],
params: Punctuated<Ident, token::Comma>,
gt_token: Token![>],
Expand Down Expand Up @@ -97,9 +97,9 @@ impl ToTokens for TypeGenerics {
}

/// Represents generics which only support [`TypeParam`] separated by commas.
struct SimpleGenerics {
pub struct SimpleGenerics {
lt_token: Token![<],
params: Punctuated<TypeParam, token::Comma>,
pub params: Punctuated<TypeParam, token::Comma>,
gt_token: Token![>],
}

Expand Down Expand Up @@ -141,7 +141,7 @@ mod storage_types {
}

/// The supported storage types
enum StorageType {
pub enum StorageType {
Value {
_kw: storage_types::StorageValue,
_lt_token: Token![<],
Expand Down Expand Up @@ -406,15 +406,15 @@ impl Parse for StorageType {
}

/// The input expected by this macro.
struct Input {
attributes: Vec<Attribute>,
visibility: Visibility,
pub struct Input {
pub attributes: Vec<Attribute>,
pub visibility: Visibility,
_type: Token![type],
storage_name: Ident,
storage_generics: Option<SimpleGenerics>,
where_clause: Option<WhereClause>,
pub storage_name: Ident,
pub storage_generics: Option<SimpleGenerics>,
pub where_clause: Option<WhereClause>,
_equal: Token![=],
storage_type: StorageType,
pub storage_type: StorageType,
sam0x17 marked this conversation as resolved.
Show resolved Hide resolved
_semicolon: Token![;],
}

Expand Down
21 changes: 21 additions & 0 deletions frame/support/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,27 @@ impl TypeId for PalletId {
/// ```
pub use frame_support_procedural::storage_alias;

/// A series of attribute macros related to benchmarking. Right now just
/// [`benchmarking(cached)`](frame_support_procedural::benchmarking(cached)) is provided.
///
/// # benchmarking(cached)
///
/// This is an attribute macro that can be attached to a [`StorageValue`]. Doing
/// so will exclude reads of that value's storage key from counting towards
/// weight calculations during benchmarking.
///
/// This attribute should be attached to [`StorageValue`]s that are known to be
/// read/used in every block. This will result in a more efficient read pattern
/// for the value and a more accurate benchmarking weight
///
/// ## Example
/// ```ignore
/// #[pallet::storage]
/// #[benchmarking(cached)]
/// pub(super) type Number<T: Config> = StorageValue<_, T::BlockNumber, ValueQuery>;
/// ```
pub use frame_support_procedural::benchmarking;

/// Create new implementations of the [`Get`](crate::traits::Get) trait.
///
/// The so-called parameter type can be created in four different ways:
Expand Down
1 change: 1 addition & 0 deletions frame/support/src/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ pub mod schedule;
mod storage;
pub use storage::{
Instance, PartialStorageInfoTrait, StorageInfo, StorageInfoTrait, StorageInstance,
TrackedStorageKey, WhitelistedStorageKeys,
};

mod dispatch;
Expand Down
Loading