diff --git a/substrate/.maintain/frame-weight-template.hbs b/substrate/.maintain/frame-weight-template.hbs index 04453d2bfe..64d8f75b00 100644 --- a/substrate/.maintain/frame-weight-template.hbs +++ b/substrate/.maintain/frame-weight-template.hbs @@ -47,6 +47,9 @@ pub trait WeightInfo { pub struct SubstrateWeight(PhantomData); impl WeightInfo for SubstrateWeight { {{~#each benchmarks as |benchmark|}} + {{~#each benchmark.comments as |comment|}} + // {{comment}} + {{~/each}} fn {{benchmark.name~}} ( {{~#each benchmark.components as |c| ~}} @@ -76,6 +79,9 @@ impl WeightInfo for SubstrateWeight { // For backwards compatibility and tests impl WeightInfo for () { {{~#each benchmarks as |benchmark|}} + {{~#each benchmark.comments as |comment|}} + // {{comment}} + {{~/each}} fn {{benchmark.name~}} ( {{~#each benchmark.components as |c| ~}} diff --git a/substrate/Cargo.lock b/substrate/Cargo.lock index be1c28c854..fd6318ef84 100644 --- a/substrate/Cargo.lock +++ b/substrate/Cargo.lock @@ -1785,6 +1785,7 @@ dependencies = [ "Inflector", "chrono", "frame-benchmarking", + "frame-support", "handlebars", "parity-scale-codec", "sc-cli", diff --git a/substrate/bin/node-template/runtime/src/lib.rs b/substrate/bin/node-template/runtime/src/lib.rs index 940eb2379b..0d33662240 100644 --- a/substrate/bin/node-template/runtime/src/lib.rs +++ b/substrate/bin/node-template/runtime/src/lib.rs @@ -31,7 +31,7 @@ pub use pallet_balances::Call as BalancesCall; pub use sp_runtime::{Permill, Perbill}; pub use frame_support::{ construct_runtime, parameter_types, StorageValue, - traits::{KeyOwnerProofSystem, Randomness}, + traits::{KeyOwnerProofSystem, Randomness, StorageInfo}, weights::{ Weight, IdentityFee, constants::{BlockExecutionWeight, ExtrinsicBaseWeight, RocksDbWeight, WEIGHT_PER_SECOND}, @@ -450,8 +450,12 @@ impl_runtime_apis! { impl frame_benchmarking::Benchmark for Runtime { fn dispatch_benchmark( config: frame_benchmarking::BenchmarkConfig - ) -> Result, sp_runtime::RuntimeString> { + ) -> Result< + (Vec, Vec), + sp_runtime::RuntimeString, + > { use frame_benchmarking::{Benchmarking, BenchmarkBatch, add_benchmark, TrackedStorageKey}; + use frame_support::traits::StorageInfoTrait; use frame_system_benchmarking::Pallet as SystemBench; impl frame_system_benchmarking::Config for Runtime {} @@ -469,6 +473,8 @@ impl_runtime_apis! { hex_literal::hex!("26aa394eea5630e07c48ae0c9558cef780d41e5e16056765bc8461851072c9d7").to_vec().into(), ]; + let storage_info = AllPalletsWithSystem::storage_info(); + let mut batches = Vec::::new(); let params = (&config, &whitelist); @@ -478,7 +484,7 @@ impl_runtime_apis! { add_benchmark!(params, batches, pallet_template, TemplateModule); if batches.is_empty() { return Err("Benchmark not found for this pallet.".into()) } - Ok(batches) + Ok((batches, storage_info)) } } } diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index 8291e4b644..c29a3ebc17 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -1509,8 +1509,13 @@ impl_runtime_apis! { impl frame_benchmarking::Benchmark for Runtime { fn dispatch_benchmark( config: frame_benchmarking::BenchmarkConfig - ) -> Result, sp_runtime::RuntimeString> { + ) -> Result< + (Vec, Vec), + sp_runtime::RuntimeString, + > { use frame_benchmarking::{Benchmarking, BenchmarkBatch, add_benchmark, TrackedStorageKey}; + use frame_support::traits::StorageInfoTrait; + // Trying to add benchmarks directly to the Session Pallet caused cyclic dependency // issues. To get around that, we separated the Session benchmarks into its own crate, // which is why we need these two lines below. @@ -1537,6 +1542,8 @@ impl_runtime_apis! { hex_literal::hex!("26aa394eea5630e07c48ae0c9558cef7b99d880ec681799c0cf30e8886371da95ecffd7b6c0f78751baa9d281e0bfa3a6d6f646c70792f74727372790000000000000000000000000000000000000000").to_vec().into(), ]; + let storage_info = AllPalletsWithSystem::storage_info(); + let mut batches = Vec::::new(); let params = (&config, &whitelist); @@ -1574,7 +1581,7 @@ impl_runtime_apis! { add_benchmark!(params, batches, pallet_election_provider_multi_phase, ElectionProviderMultiPhase); if batches.is_empty() { return Err("Benchmark not found for this pallet.".into()) } - Ok(batches) + Ok((batches, storage_info)) } } } diff --git a/substrate/client/db/src/bench.rs b/substrate/client/db/src/bench.rs index 470448df76..4b34182a1c 100644 --- a/substrate/client/db/src/bench.rs +++ b/substrate/client/db/src/bench.rs @@ -66,40 +66,6 @@ impl sp_state_machine::Storage> for StorageDb { root: Cell, @@ -110,11 +76,14 @@ pub struct BenchmarkingState { record: Cell>>, shared_cache: SharedCache, // shared cache is always empty /// Key tracker for keys in the main trie. - main_key_tracker: RefCell, KeyTracker>>, + /// We track the total number of reads and writes to these keys, + /// not de-duplicated for repeats. + main_key_tracker: RefCell, TrackedStorageKey>>, /// Key tracker for keys in a child trie. /// Child trie are identified by their storage key (i.e. `ChildInfo::storage_key()`) - child_key_tracker: RefCell, HashMap, KeyTracker>>>, - read_write_tracker: RefCell, + /// We track the total number of reads and writes to these keys, + /// not de-duplicated for repeats. + child_key_tracker: RefCell, HashMap, TrackedStorageKey>>>, whitelist: RefCell>, proof_recorder: Option>, proof_recorder_root: Cell, @@ -137,7 +106,6 @@ impl BenchmarkingState { shared_cache: new_shared_cache(0, (1, 10)), main_key_tracker: Default::default(), child_key_tracker: Default::default(), - read_write_tracker: Default::default(), whitelist: Default::default(), proof_recorder: record_proof.then(Default::default), proof_recorder_root: Cell::new(root.clone()), @@ -191,10 +159,8 @@ impl BenchmarkingState { let whitelist = self.whitelist.borrow(); whitelist.iter().for_each(|key| { - let whitelisted = KeyTracker { - has_been_read: key.has_been_read, - has_been_written: key.has_been_written, - }; + let mut whitelisted = TrackedStorageKey::new(key.key.clone()); + whitelisted.whitelist(); main_key_tracker.insert(key.key.clone(), whitelisted); }); } @@ -203,12 +169,10 @@ impl BenchmarkingState { *self.main_key_tracker.borrow_mut() = HashMap::new(); *self.child_key_tracker.borrow_mut() = HashMap::new(); self.add_whitelist_to_tracker(); - *self.read_write_tracker.borrow_mut() = Default::default(); } // Childtrie is identified by its storage key (i.e. `ChildInfo::storage_key`) fn add_read_key(&self, childtrie: Option<&[u8]>, key: &[u8]) { - let mut read_write_tracker = self.read_write_tracker.borrow_mut(); let mut child_key_tracker = self.child_key_tracker.borrow_mut(); let mut main_key_tracker = self.main_key_tracker.borrow_mut(); @@ -218,33 +182,21 @@ impl BenchmarkingState { &mut main_key_tracker }; - let read = match key_tracker.get(key) { + let should_log = match key_tracker.get_mut(key) { None => { - let has_been_read = KeyTracker { - has_been_read: true, - has_been_written: false, - }; + let mut has_been_read = TrackedStorageKey::new(key.to_vec()); + has_been_read.add_read(); key_tracker.insert(key.to_vec(), has_been_read); - read_write_tracker.add_read(); true }, Some(tracker) => { - if !tracker.has_been_read { - let has_been_read = KeyTracker { - has_been_read: true, - has_been_written: tracker.has_been_written, - }; - key_tracker.insert(key.to_vec(), has_been_read); - read_write_tracker.add_read(); - true - } else { - read_write_tracker.add_repeat_read(); - false - } + let should_log = !tracker.has_been_read(); + tracker.add_read(); + should_log } }; - if read { + if should_log { if let Some(childtrie) = childtrie { log::trace!( target: "benchmark", @@ -258,7 +210,6 @@ impl BenchmarkingState { // Childtrie is identified by its storage key (i.e. `ChildInfo::storage_key`) fn add_write_key(&self, childtrie: Option<&[u8]>, key: &[u8]) { - let mut read_write_tracker = self.read_write_tracker.borrow_mut(); let mut child_key_tracker = self.child_key_tracker.borrow_mut(); let mut main_key_tracker = self.main_key_tracker.borrow_mut(); @@ -269,30 +220,21 @@ impl BenchmarkingState { }; // If we have written to the key, we also consider that we have read from it. - let has_been_written = KeyTracker { - has_been_read: true, - has_been_written: true, - }; - - let write = match key_tracker.get(key) { + let should_log = match key_tracker.get_mut(key) { None => { + let mut has_been_written = TrackedStorageKey::new(key.to_vec()); + has_been_written.add_write(); key_tracker.insert(key.to_vec(), has_been_written); - read_write_tracker.add_write(); true }, Some(tracker) => { - if !tracker.has_been_written { - key_tracker.insert(key.to_vec(), has_been_written); - read_write_tracker.add_write(); - true - } else { - read_write_tracker.add_repeat_write(); - false - } + let should_log = !tracker.has_been_written(); + tracker.add_write(); + should_log } }; - if write { + if should_log { if let Some(childtrie) = childtrie { log::trace!( target: "benchmark", @@ -303,6 +245,23 @@ impl BenchmarkingState { } } } + + // Return all the tracked storage keys among main and child trie. + fn all_trackers(&self) -> Vec { + let mut all_trackers = Vec::new(); + + self.main_key_tracker.borrow().iter().for_each(|(_, tracker)| { + all_trackers.push(tracker.clone()); + }); + + self.child_key_tracker.borrow().iter().for_each(|(_, child_tracker)| { + child_tracker.iter().for_each(|(_, tracker)| { + all_trackers.push(tracker.clone()); + }); + }); + + all_trackers + } } fn state_err() -> String { @@ -507,9 +466,30 @@ impl StateBackend> for BenchmarkingState { } /// Get the key tracking information for the state db. + /// 1. `reads` - Total number of DB reads. + /// 2. `repeat_reads` - Total number of in-memory reads. + /// 3. `writes` - Total number of DB writes. + /// 4. `repeat_writes` - Total number of in-memory writes. fn read_write_count(&self) -> (u32, u32, u32, u32) { - let count = *self.read_write_tracker.borrow_mut(); - (count.reads, count.repeat_reads, count.writes, count.repeat_writes) + let mut reads = 0; + let mut repeat_reads = 0; + let mut writes = 0; + let mut repeat_writes = 0; + + self.all_trackers().iter().for_each(|tracker| { + if !tracker.whitelisted { + if tracker.reads > 0 { + reads += 1; + repeat_reads += tracker.reads - 1; + } + + if tracker.writes > 0 { + writes += 1; + repeat_writes += tracker.reads - 1; + } + } + }); + (reads, repeat_reads, writes, repeat_writes) } /// Reset the key tracking information for the state db. @@ -525,6 +505,40 @@ impl StateBackend> for BenchmarkingState { *self.whitelist.borrow_mut() = new; } + fn get_read_and_written_keys(&self) -> Vec<(Vec, u32, u32, bool)> { + // We only track at the level of a key-prefix and not whitelisted for now for memory size. + // TODO: Refactor to enable full storage key transparency, where we can remove the + // `prefix_key_tracker`. + let mut prefix_key_tracker = HashMap::, (u32, u32, bool)>::new(); + self.all_trackers().iter().for_each(|tracker| { + if !tracker.whitelisted { + let prefix_length = tracker.key.len().min(32); + let prefix = tracker.key[0..prefix_length].to_vec(); + // each read / write of a specific key is counted at most one time, since + // additional reads / writes happen in the memory overlay. + let reads = tracker.reads.min(1); + let writes = tracker.writes.min(1); + if let Some(prefix_tracker) = prefix_key_tracker.get_mut(&prefix) { + prefix_tracker.0 += reads; + prefix_tracker.1 += writes; + } else { + prefix_key_tracker.insert( + prefix, + ( + reads, + writes, + tracker.whitelisted, + ), + ); + } + } + }); + + prefix_key_tracker.iter().map(|(key, tracker)| -> (Vec, u32, u32, bool) { + (key.to_vec(), tracker.0, tracker.1, tracker.2) + }).collect::>() + } + fn register_overlay_stats(&self, stats: &sp_state_machine::StateMachineStats) { self.state.borrow_mut().as_mut().map(|s| s.register_overlay_stats(stats)); } @@ -597,11 +611,11 @@ mod test { ] ).unwrap(); - let rw_tracker = bench_state.read_write_tracker.borrow(); - assert_eq!(rw_tracker.reads, 6); - assert_eq!(rw_tracker.repeat_reads, 0); - assert_eq!(rw_tracker.writes, 2); - assert_eq!(rw_tracker.repeat_writes, 0); + let rw_tracker = bench_state.read_write_count(); + assert_eq!(rw_tracker.0, 6); + assert_eq!(rw_tracker.1, 0); + assert_eq!(rw_tracker.2, 2); + assert_eq!(rw_tracker.3, 0); drop(rw_tracker); bench_state.wipe().unwrap(); } diff --git a/substrate/frame/benchmarking/src/analysis.rs b/substrate/frame/benchmarking/src/analysis.rs index 7b6d8838fd..f37ffba51f 100644 --- a/substrate/frame/benchmarking/src/analysis.rs +++ b/substrate/frame/benchmarking/src/analysis.rs @@ -375,6 +375,7 @@ mod tests { writes, repeat_writes: 0, proof_size: 0, + keys: vec![], } } diff --git a/substrate/frame/benchmarking/src/lib.rs b/substrate/frame/benchmarking/src/lib.rs index 8160bd5d1d..fb4fd0801a 100644 --- a/substrate/frame/benchmarking/src/lib.rs +++ b/substrate/frame/benchmarking/src/lib.rs @@ -824,6 +824,10 @@ macro_rules! impl_benchmark { let finish_storage_root = $crate::benchmarking::current_time(); let elapsed_storage_root = finish_storage_root - start_storage_root; + // TODO: Fix memory allocation issue then re-enable + // let read_and_written_keys = $crate::benchmarking::get_read_and_written_keys(); + let read_and_written_keys = Default::default(); + results.push($crate::BenchmarkResults { components: c.to_vec(), extrinsic_time: elapsed_extrinsic, @@ -833,6 +837,7 @@ macro_rules! impl_benchmark { writes: read_write_count.2, repeat_writes: read_write_count.3, proof_size: diff_pov, + keys: read_and_written_keys, }); } diff --git a/substrate/frame/benchmarking/src/utils.rs b/substrate/frame/benchmarking/src/utils.rs index 2db7b2e95d..c40434fb1a 100644 --- a/substrate/frame/benchmarking/src/utils.rs +++ b/substrate/frame/benchmarking/src/utils.rs @@ -21,6 +21,7 @@ use codec::{Encode, Decode}; use sp_std::{vec::Vec, prelude::Box}; use sp_io::hashing::blake2_256; use sp_storage::TrackedStorageKey; +use frame_support::traits::StorageInfo; /// An alphabet of possible parameters to use for benchmarking. #[derive(Encode, Decode, Clone, Copy, PartialEq, Debug)] @@ -63,6 +64,7 @@ pub struct BenchmarkResults { pub writes: u32, pub repeat_writes: u32, pub proof_size: u32, + pub keys: Vec<(Vec, u32, u32, bool)>, } /// Configuration used to setup and run runtime benchmarks. @@ -90,7 +92,8 @@ sp_api::decl_runtime_apis! { /// Runtime api for benchmarking a FRAME runtime. pub trait Benchmark { /// Dispatch the given benchmark. - fn dispatch_benchmark(config: BenchmarkConfig) -> Result, sp_runtime::RuntimeString>; + fn dispatch_benchmark(config: BenchmarkConfig) + -> Result<(Vec, Vec), sp_runtime::RuntimeString>; } } @@ -143,11 +146,9 @@ pub trait Benchmarking { 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, - } + item.reads += add.reads; + item.writes += add.writes; + item.whitelisted = item.whitelisted || add.whitelisted; }, // If the key does not exist, add it. None => { @@ -164,6 +165,10 @@ pub trait Benchmarking { self.set_whitelist(whitelist); } + fn get_read_and_written_keys(&self) -> Vec<(Vec, u32, u32, bool)> { + self.get_read_and_written_keys() + } + /// Get current estimated proof size. fn proof_size(&self) -> Option { self.proof_size() diff --git a/substrate/frame/support/procedural/src/storage/storage_struct.rs b/substrate/frame/support/procedural/src/storage/storage_struct.rs index c990bad85e..3b182983cd 100644 --- a/substrate/frame/support/procedural/src/storage/storage_struct.rs +++ b/substrate/frame/support/procedural/src/storage/storage_struct.rs @@ -273,9 +273,15 @@ pub fn decl_and_impl(def: &DeclStorageDefExt) -> TokenStream { #scrate::sp_std::vec![ #scrate::traits::StorageInfo { + pallet_name: < + #storage_struct as #scrate::#storage_generator_trait + >::module_prefix().to_vec(), + storage_name: < + #storage_struct as #scrate::#storage_generator_trait + >::storage_prefix().to_vec(), prefix: < #storage_struct as #scrate::#storage_generator_trait - >::storage_value_final_key(), + >::storage_value_final_key().to_vec(), max_values: Some(1), max_size: Some(max_size), } @@ -308,10 +314,18 @@ pub fn decl_and_impl(def: &DeclStorageDefExt) -> TokenStream { #scrate::sp_std::vec![ #scrate::traits::StorageInfo { + pallet_name: < + #storage_struct + as #scrate::storage::StoragePrefixedMap<#value_type> + >::module_prefix().to_vec(), + storage_name: < + #storage_struct + as #scrate::storage::StoragePrefixedMap<#value_type> + >::storage_prefix().to_vec(), prefix: < #storage_struct as #scrate::storage::StoragePrefixedMap<#value_type> - >::final_prefix(), + >::final_prefix().to_vec(), max_values: #max_values, max_size: Some(max_size), } @@ -350,10 +364,18 @@ pub fn decl_and_impl(def: &DeclStorageDefExt) -> TokenStream { #scrate::sp_std::vec![ #scrate::traits::StorageInfo { + pallet_name: < + #storage_struct + as #scrate::storage::StoragePrefixedMap<#value_type> + >::module_prefix().to_vec(), + storage_name: < + #storage_struct + as #scrate::storage::StoragePrefixedMap<#value_type> + >::storage_prefix().to_vec(), prefix: < #storage_struct as #scrate::storage::StoragePrefixedMap<#value_type> - >::final_prefix(), + >::final_prefix().to_vec(), max_values: #max_values, max_size: Some(max_size), } @@ -385,10 +407,18 @@ pub fn decl_and_impl(def: &DeclStorageDefExt) -> TokenStream { #scrate::sp_std::vec![ #scrate::traits::StorageInfo { + pallet_name: < + #storage_struct + as #scrate::storage::StoragePrefixedMap<#value_type> + >::module_prefix().to_vec(), + storage_name: < + #storage_struct + as #scrate::storage::StoragePrefixedMap<#value_type> + >::storage_prefix().to_vec(), prefix: < #storage_struct as #scrate::storage::StoragePrefixedMap<#value_type> - >::final_prefix(), + >::final_prefix().to_vec(), max_values: #max_values, max_size: Some(max_size), } @@ -413,9 +443,15 @@ pub fn decl_and_impl(def: &DeclStorageDefExt) -> TokenStream { { #scrate::sp_std::vec![ #scrate::traits::StorageInfo { + pallet_name: < + #storage_struct as #scrate::#storage_generator_trait + >::module_prefix().to_vec(), + storage_name: < + #storage_struct as #scrate::#storage_generator_trait + >::storage_prefix().to_vec(), prefix: < #storage_struct as #scrate::#storage_generator_trait - >::storage_value_final_key(), + >::storage_value_final_key().to_vec(), max_values: Some(1), max_size: None, } @@ -435,10 +471,18 @@ pub fn decl_and_impl(def: &DeclStorageDefExt) -> TokenStream { { #scrate::sp_std::vec![ #scrate::traits::StorageInfo { + pallet_name: < + #storage_struct + as #scrate::storage::StoragePrefixedMap<#value_type> + >::module_prefix().to_vec(), + storage_name: < + #storage_struct + as #scrate::storage::StoragePrefixedMap<#value_type> + >::storage_prefix().to_vec(), prefix: < #storage_struct as #scrate::storage::StoragePrefixedMap<#value_type> - >::final_prefix(), + >::final_prefix().to_vec(), max_values: #max_values, max_size: None, } @@ -458,10 +502,18 @@ pub fn decl_and_impl(def: &DeclStorageDefExt) -> TokenStream { { #scrate::sp_std::vec![ #scrate::traits::StorageInfo { + pallet_name: < + #storage_struct + as #scrate::storage::StoragePrefixedMap<#value_type> + >::module_prefix().to_vec(), + storage_name: < + #storage_struct + as #scrate::storage::StoragePrefixedMap<#value_type> + >::storage_prefix().to_vec(), prefix: < #storage_struct as #scrate::storage::StoragePrefixedMap<#value_type> - >::final_prefix(), + >::final_prefix().to_vec(), max_values: #max_values, max_size: None, } @@ -481,10 +533,18 @@ pub fn decl_and_impl(def: &DeclStorageDefExt) -> TokenStream { { #scrate::sp_std::vec![ #scrate::traits::StorageInfo { + pallet_name: < + #storage_struct + as #scrate::storage::StoragePrefixedMap<#value_type> + >::module_prefix().to_vec(), + storage_name: < + #storage_struct + as #scrate::storage::StoragePrefixedMap<#value_type> + >::storage_prefix().to_vec(), prefix: < #storage_struct as #scrate::storage::StoragePrefixedMap<#value_type> - >::final_prefix(), + >::final_prefix().to_vec(), max_values: #max_values, max_size: None, } diff --git a/substrate/frame/support/src/storage/types/double_map.rs b/substrate/frame/support/src/storage/types/double_map.rs index a8ab4329ce..5143967d8c 100644 --- a/substrate/frame/support/src/storage/types/double_map.rs +++ b/substrate/frame/support/src/storage/types/double_map.rs @@ -486,7 +486,9 @@ where fn storage_info() -> Vec { vec![ StorageInfo { - prefix: Self::final_prefix(), + pallet_name: Self::module_prefix().to_vec(), + storage_name: Self::storage_prefix().to_vec(), + prefix: Self::final_prefix().to_vec(), max_values: MaxValues::get(), max_size: Some( Hasher1::max_len::() @@ -517,7 +519,9 @@ where fn partial_storage_info() -> Vec { vec![ StorageInfo { - prefix: Self::final_prefix(), + pallet_name: Self::module_prefix().to_vec(), + storage_name: Self::storage_prefix().to_vec(), + prefix: Self::final_prefix().to_vec(), max_values: MaxValues::get(), max_size: None } diff --git a/substrate/frame/support/src/storage/types/map.rs b/substrate/frame/support/src/storage/types/map.rs index 800cd1153a..168d5236cc 100644 --- a/substrate/frame/support/src/storage/types/map.rs +++ b/substrate/frame/support/src/storage/types/map.rs @@ -363,7 +363,9 @@ where fn storage_info() -> Vec { vec![ StorageInfo { - prefix: Self::final_prefix(), + pallet_name: Self::module_prefix().to_vec(), + storage_name: Self::storage_prefix().to_vec(), + prefix: Self::final_prefix().to_vec(), max_values: MaxValues::get(), max_size: Some( Hasher::max_len::() @@ -391,7 +393,9 @@ where fn partial_storage_info() -> Vec { vec![ StorageInfo { - prefix: Self::final_prefix(), + pallet_name: Self::module_prefix().to_vec(), + storage_name: Self::storage_prefix().to_vec(), + prefix: Self::final_prefix().to_vec(), max_values: MaxValues::get(), max_size: None, } diff --git a/substrate/frame/support/src/storage/types/nmap.rs b/substrate/frame/support/src/storage/types/nmap.rs index b75542cbf9..63c27729d2 100755 --- a/substrate/frame/support/src/storage/types/nmap.rs +++ b/substrate/frame/support/src/storage/types/nmap.rs @@ -418,7 +418,9 @@ where fn storage_info() -> Vec { vec![ StorageInfo { - prefix: Self::final_prefix(), + pallet_name: Self::module_prefix().to_vec(), + storage_name: Self::storage_prefix().to_vec(), + prefix: Self::final_prefix().to_vec(), max_values: MaxValues::get(), max_size: Some( Key::key_max_encoded_len() @@ -445,7 +447,9 @@ where fn partial_storage_info() -> Vec { vec![ StorageInfo { - prefix: Self::final_prefix(), + pallet_name: Self::module_prefix().to_vec(), + storage_name: Self::storage_prefix().to_vec(), + prefix: Self::final_prefix().to_vec(), max_values: MaxValues::get(), max_size: None, } diff --git a/substrate/frame/support/src/storage/types/value.rs b/substrate/frame/support/src/storage/types/value.rs index 0bd171f10e..3fe7d43640 100644 --- a/substrate/frame/support/src/storage/types/value.rs +++ b/substrate/frame/support/src/storage/types/value.rs @@ -22,6 +22,7 @@ use crate::{ storage::{ StorageAppend, StorageTryAppend, StorageDecodeLength, types::{OptionQuery, QueryKindTrait, OnEmptyGetter}, + generator::{StorageValue as StorageValueT}, }, traits::{GetDefault, StorageInstance, StorageInfo}, }; @@ -217,7 +218,9 @@ where fn storage_info() -> Vec { vec![ StorageInfo { - prefix: Self::hashed_key(), + pallet_name: Self::module_prefix().to_vec(), + storage_name: Self::storage_prefix().to_vec(), + prefix: Self::hashed_key().to_vec(), max_values: Some(1), max_size: Some( Value::max_encoded_len() @@ -241,7 +244,9 @@ where fn partial_storage_info() -> Vec { vec![ StorageInfo { - prefix: Self::hashed_key(), + pallet_name: Self::module_prefix().to_vec(), + storage_name: Self::storage_prefix().to_vec(), + prefix: Self::hashed_key().to_vec(), max_values: Some(1), max_size: None, } diff --git a/substrate/frame/support/src/traits/storage.rs b/substrate/frame/support/src/traits/storage.rs index c1f97694df..c0cbfb3a90 100644 --- a/substrate/frame/support/src/traits/storage.rs +++ b/substrate/frame/support/src/traits/storage.rs @@ -48,11 +48,15 @@ pub trait StorageInstance { const STORAGE_PREFIX: &'static str; } -/// Some info about an individual storage in a pallet. +/// Metadata about storage from the runtime. #[derive(codec::Encode, codec::Decode, crate::RuntimeDebug, Eq, PartialEq, Clone)] pub struct StorageInfo { - /// The prefix of the storage. All keys after the prefix are considered part of the storage - pub prefix: [u8; 32], + /// Encoded string of pallet name. + pub pallet_name: Vec, + /// Encoded string of storage name. + pub storage_name: Vec, + /// The prefix of the storage. All keys after the prefix are considered part of this storage. + pub prefix: Vec, /// The maximum number of values in the storage, or none if no maximum specified. pub max_values: Option, /// The maximum size of key/values in the storage, or none if no maximum specified. diff --git a/substrate/frame/support/test/tests/decl_storage.rs b/substrate/frame/support/test/tests/decl_storage.rs index 56ea217bbf..85c3d8f675 100644 --- a/substrate/frame/support/test/tests/decl_storage.rs +++ b/substrate/frame/support/test/tests/decl_storage.rs @@ -438,147 +438,205 @@ mod tests { >::storage_info(), vec![ StorageInfo { - prefix: prefix(b"TestStorage", b"U32"), + pallet_name: b"TestStorage".to_vec(), + storage_name: b"U32".to_vec(), + prefix: prefix(b"TestStorage", b"U32").to_vec(), max_values: Some(1), max_size: Some(4), }, StorageInfo { - prefix: prefix(b"TestStorage", b"PUBU32"), + pallet_name: b"TestStorage".to_vec(), + storage_name: b"PUBU32".to_vec(), + prefix: prefix(b"TestStorage", b"PUBU32").to_vec(), max_values: Some(1), max_size: Some(4), }, StorageInfo { - prefix: prefix(b"TestStorage", b"U32MYDEF"), + pallet_name: b"TestStorage".to_vec(), + storage_name: b"U32MYDEF".to_vec(), + prefix: prefix(b"TestStorage", b"U32MYDEF").to_vec(), max_values: Some(1), max_size: Some(4), }, StorageInfo { - prefix: prefix(b"TestStorage", b"PUBU32MYDEF"), + pallet_name: b"TestStorage".to_vec(), + storage_name: b"PUBU32MYDEF".to_vec(), + prefix: prefix(b"TestStorage", b"PUBU32MYDEF").to_vec(), max_values: Some(1), max_size: Some(4), }, StorageInfo { - prefix: prefix(b"TestStorage", b"GETU32"), + pallet_name: b"TestStorage".to_vec(), + storage_name: b"GETU32".to_vec(), + prefix: prefix(b"TestStorage", b"GETU32").to_vec(), max_values: Some(1), max_size: Some(4), }, StorageInfo { - prefix: prefix(b"TestStorage", b"PUBGETU32"), + pallet_name: b"TestStorage".to_vec(), + storage_name: b"PUBGETU32".to_vec(), + prefix: prefix(b"TestStorage", b"PUBGETU32").to_vec(), max_values: Some(1), max_size: Some(4), }, StorageInfo { - prefix: prefix(b"TestStorage", b"GETU32WITHCONFIG"), + pallet_name: b"TestStorage".to_vec(), + storage_name: b"GETU32WITHCONFIG".to_vec(), + prefix: prefix(b"TestStorage", b"GETU32WITHCONFIG").to_vec(), max_values: Some(1), max_size: Some(4), }, StorageInfo { - prefix: prefix(b"TestStorage", b"PUBGETU32WITHCONFIG"), + pallet_name: b"TestStorage".to_vec(), + storage_name: b"PUBGETU32WITHCONFIG".to_vec(), + prefix: prefix(b"TestStorage", b"PUBGETU32WITHCONFIG").to_vec(), max_values: Some(1), max_size: Some(4), }, StorageInfo { - prefix: prefix(b"TestStorage", b"GETU32MYDEF"), + pallet_name: b"TestStorage".to_vec(), + storage_name: b"GETU32MYDEF".to_vec(), + prefix: prefix(b"TestStorage", b"GETU32MYDEF").to_vec(), max_values: Some(1), max_size: Some(4), }, StorageInfo { - prefix: prefix(b"TestStorage", b"PUBGETU32MYDEF"), + pallet_name: b"TestStorage".to_vec(), + storage_name: b"PUBGETU32MYDEF".to_vec(), + prefix: prefix(b"TestStorage", b"PUBGETU32MYDEF").to_vec(), max_values: Some(1), max_size: Some(4), }, StorageInfo { - prefix: prefix(b"TestStorage", b"GETU32WITHCONFIGMYDEF"), + pallet_name: b"TestStorage".to_vec(), + storage_name: b"GETU32WITHCONFIGMYDEF".to_vec(), + prefix: prefix(b"TestStorage", b"GETU32WITHCONFIGMYDEF").to_vec(), max_values: Some(1), max_size: Some(4), }, StorageInfo { - prefix: prefix(b"TestStorage", b"PUBGETU32WITHCONFIGMYDEF"), + pallet_name: b"TestStorage".to_vec(), + storage_name: b"PUBGETU32WITHCONFIGMYDEF".to_vec(), + prefix: prefix(b"TestStorage", b"PUBGETU32WITHCONFIGMYDEF").to_vec(), max_values: Some(1), max_size: Some(4), }, StorageInfo { - prefix: prefix(b"TestStorage", b"PUBGETU32WITHCONFIGMYDEFOPT"), + pallet_name: b"TestStorage".to_vec(), + storage_name: b"PUBGETU32WITHCONFIGMYDEFOPT".to_vec(), + prefix: prefix(b"TestStorage", b"PUBGETU32WITHCONFIGMYDEFOPT").to_vec(), max_values: Some(1), max_size: Some(4), }, StorageInfo { - prefix: prefix(b"TestStorage", b"GetU32WithBuilder"), + pallet_name: b"TestStorage".to_vec(), + storage_name: b"GetU32WithBuilder".to_vec(), + prefix: prefix(b"TestStorage", b"GetU32WithBuilder").to_vec(), max_values: Some(1), max_size: Some(4), }, StorageInfo { - prefix: prefix(b"TestStorage", b"GetOptU32WithBuilderSome"), + pallet_name: b"TestStorage".to_vec(), + storage_name: b"GetOptU32WithBuilderSome".to_vec(), + prefix: prefix(b"TestStorage", b"GetOptU32WithBuilderSome").to_vec(), max_values: Some(1), max_size: Some(4), }, StorageInfo { - prefix: prefix(b"TestStorage", b"GetOptU32WithBuilderNone"), + pallet_name: b"TestStorage".to_vec(), + storage_name: b"GetOptU32WithBuilderNone".to_vec(), + prefix: prefix(b"TestStorage", b"GetOptU32WithBuilderNone").to_vec(), max_values: Some(1), max_size: Some(4), }, StorageInfo { - prefix: prefix(b"TestStorage", b"MAPU32"), + pallet_name: b"TestStorage".to_vec(), + storage_name: b"MAPU32".to_vec(), + prefix: prefix(b"TestStorage", b"MAPU32").to_vec(), max_values: Some(3), max_size: Some(8 + 16), }, StorageInfo { - prefix: prefix(b"TestStorage", b"PUBMAPU32"), + pallet_name: b"TestStorage".to_vec(), + storage_name: b"PUBMAPU32".to_vec(), + prefix: prefix(b"TestStorage", b"PUBMAPU32").to_vec(), max_values: None, max_size: Some(8 + 16), }, StorageInfo { - prefix: prefix(b"TestStorage", b"GETMAPU32"), + pallet_name: b"TestStorage".to_vec(), + storage_name: b"GETMAPU32".to_vec(), + prefix: prefix(b"TestStorage", b"GETMAPU32").to_vec(), max_values: None, max_size: Some(8 + 16), }, StorageInfo { - prefix: prefix(b"TestStorage", b"PUBGETMAPU32"), + pallet_name: b"TestStorage".to_vec(), + storage_name: b"PUBGETMAPU32".to_vec(), + prefix: prefix(b"TestStorage", b"PUBGETMAPU32").to_vec(), max_values: None, max_size: Some(8 + 16), }, StorageInfo { - prefix: prefix(b"TestStorage", b"GETMAPU32MYDEF"), + pallet_name: b"TestStorage".to_vec(), + storage_name: b"GETMAPU32MYDEF".to_vec(), + prefix: prefix(b"TestStorage", b"GETMAPU32MYDEF").to_vec(), max_values: None, max_size: Some(8 + 16), }, StorageInfo { - prefix: prefix(b"TestStorage", b"PUBGETMAPU32MYDEF"), + pallet_name: b"TestStorage".to_vec(), + storage_name: b"PUBGETMAPU32MYDEF".to_vec(), + prefix: prefix(b"TestStorage", b"PUBGETMAPU32MYDEF").to_vec(), max_values: None, max_size: Some(8 + 16), }, StorageInfo { - prefix: prefix(b"TestStorage", b"DOUBLEMAP"), + pallet_name: b"TestStorage".to_vec(), + storage_name: b"DOUBLEMAP".to_vec(), + prefix: prefix(b"TestStorage", b"DOUBLEMAP").to_vec(), max_values: Some(3), max_size: Some(12 + 16 + 16), }, StorageInfo { - prefix: prefix(b"TestStorage", b"DOUBLEMAP2"), + pallet_name: b"TestStorage".to_vec(), + storage_name: b"DOUBLEMAP2".to_vec(), + prefix: prefix(b"TestStorage", b"DOUBLEMAP2").to_vec(), max_values: None, max_size: Some(12 + 16 + 16), }, StorageInfo { - prefix: prefix(b"TestStorage", b"COMPLEXTYPE1"), + pallet_name: b"TestStorage".to_vec(), + storage_name: b"COMPLEXTYPE1".to_vec(), + prefix: prefix(b"TestStorage", b"COMPLEXTYPE1").to_vec(), max_values: Some(1), max_size: Some(5), }, StorageInfo { - prefix: prefix(b"TestStorage", b"COMPLEXTYPE2"), + pallet_name: b"TestStorage".to_vec(), + storage_name: b"COMPLEXTYPE2".to_vec(), + prefix: prefix(b"TestStorage", b"COMPLEXTYPE2").to_vec(), max_values: Some(1), max_size: Some(1156), }, StorageInfo { - prefix: prefix(b"TestStorage", b"COMPLEXTYPE3"), + pallet_name: b"TestStorage".to_vec(), + storage_name: b"COMPLEXTYPE3".to_vec(), + prefix: prefix(b"TestStorage", b"COMPLEXTYPE3").to_vec(), max_values: Some(1), max_size: Some(100), }, StorageInfo { - prefix: prefix(b"TestStorage", b"NMAP"), + pallet_name: b"TestStorage".to_vec(), + storage_name: b"NMAP".to_vec(), + prefix: prefix(b"TestStorage", b"NMAP").to_vec(), max_values: None, max_size: Some(16 + 4 + 8 + 2 + 1), }, StorageInfo { - prefix: prefix(b"TestStorage", b"NMAP2"), + pallet_name: b"TestStorage".to_vec(), + storage_name: b"NMAP2".to_vec(), + prefix: prefix(b"TestStorage", b"NMAP2").to_vec(), max_values: None, max_size: Some(16 + 4 + 1), }, @@ -669,22 +727,30 @@ mod test2 { >::storage_info(), vec![ StorageInfo { - prefix: prefix(b"TestStorage", b"SingleDef"), + pallet_name: b"TestStorage".to_vec(), + storage_name: b"SingleDef".to_vec(), + prefix: prefix(b"TestStorage", b"SingleDef").to_vec(), max_values: Some(1), max_size: None, }, StorageInfo { - prefix: prefix(b"TestStorage", b"PairDef"), + pallet_name: b"TestStorage".to_vec(), + storage_name: b"PairDef".to_vec(), + prefix: prefix(b"TestStorage", b"PairDef").to_vec(), max_values: Some(1), max_size: None, }, StorageInfo { - prefix: prefix(b"TestStorage", b"Single"), + pallet_name: b"TestStorage".to_vec(), + storage_name: b"Single".to_vec(), + prefix: prefix(b"TestStorage", b"Single").to_vec(), max_values: Some(1), max_size: None, }, StorageInfo { - prefix: prefix(b"TestStorage", b"Pair"), + pallet_name: b"TestStorage".to_vec(), + storage_name: b"Pair".to_vec(), + prefix: prefix(b"TestStorage", b"Pair").to_vec(), max_values: Some(1), max_size: None, }, diff --git a/substrate/frame/support/test/tests/pallet.rs b/substrate/frame/support/test/tests/pallet.rs index f204de69b8..7438cee2bc 100644 --- a/substrate/frame/support/test/tests/pallet.rs +++ b/substrate/frame/support/test/tests/pallet.rs @@ -1171,54 +1171,74 @@ fn test_storage_info() { Example::storage_info(), vec![ StorageInfo { - prefix: prefix(b"Example", b"ValueWhereClause"), + pallet_name: b"Example".to_vec(), + storage_name: b"ValueWhereClause".to_vec(), + prefix: prefix(b"Example", b"ValueWhereClause").to_vec(), max_values: Some(1), max_size: Some(8), }, StorageInfo { - prefix: prefix(b"Example", b"Value"), + pallet_name: b"Example".to_vec(), + storage_name: b"Value".to_vec(), + prefix: prefix(b"Example", b"Value").to_vec(), max_values: Some(1), max_size: Some(4), }, StorageInfo { - prefix: prefix(b"Example", b"Value2"), + pallet_name: b"Example".to_vec(), + storage_name: b"Value2".to_vec(), + prefix: prefix(b"Example", b"Value2").to_vec(), max_values: Some(1), max_size: Some(8), }, StorageInfo { - prefix: prefix(b"Example", b"Map"), + pallet_name: b"Example".to_vec(), + storage_name: b"Map".to_vec(), + prefix: prefix(b"Example", b"Map").to_vec(), max_values: None, max_size: Some(3 + 16), }, StorageInfo { - prefix: prefix(b"Example", b"Map2"), + pallet_name: b"Example".to_vec(), + storage_name: b"Map2".to_vec(), + prefix: prefix(b"Example", b"Map2").to_vec(), max_values: Some(3), max_size: Some(6 + 8), }, StorageInfo { - prefix: prefix(b"Example", b"DoubleMap"), + pallet_name: b"Example".to_vec(), + storage_name: b"DoubleMap".to_vec(), + prefix: prefix(b"Example", b"DoubleMap").to_vec(), max_values: None, max_size: Some(7 + 16 + 8), }, StorageInfo { - prefix: prefix(b"Example", b"DoubleMap2"), + pallet_name: b"Example".to_vec(), + storage_name: b"DoubleMap2".to_vec(), + prefix: prefix(b"Example", b"DoubleMap2").to_vec(), max_values: Some(5), max_size: Some(14 + 8 + 16), }, StorageInfo { - prefix: prefix(b"Example", b"NMap"), + pallet_name: b"Example".to_vec(), + storage_name: b"NMap".to_vec(), + prefix: prefix(b"Example", b"NMap").to_vec(), max_values: None, max_size: Some(5 + 16), }, StorageInfo { - prefix: prefix(b"Example", b"NMap2"), + pallet_name: b"Example".to_vec(), + storage_name: b"NMap2".to_vec(), + prefix: prefix(b"Example", b"NMap2").to_vec(), max_values: Some(11), max_size: Some(14 + 8 + 16), }, #[cfg(feature = "conditional-storage")] { StorageInfo { - prefix: prefix(b"Example", b"ConditionalValue"), + pallet_name: b"Example".to_vec(), + storage_name: b"ConditionalValue".to_vec(), + prefix: prefix(b"Example", b"ConditionalValue").to_vec(), max_values: Some(1), max_size: Some(4), } @@ -1226,7 +1246,9 @@ fn test_storage_info() { #[cfg(feature = "conditional-storage")] { StorageInfo { - prefix: prefix(b"Example", b"ConditionalMap"), + pallet_name: b"Example".to_vec(), + storage_name: b"ConditionalMap".to_vec(), + prefix: prefix(b"Example", b"ConditionalMap").to_vec(), max_values: Some(12), max_size: Some(6 + 8), } @@ -1234,7 +1256,9 @@ fn test_storage_info() { #[cfg(feature = "conditional-storage")] { StorageInfo { - prefix: prefix(b"Example", b"ConditionalDoubleMap"), + pallet_name: b"Example".to_vec(), + storage_name: b"ConditionalDoubleMap".to_vec(), + prefix: prefix(b"Example", b"ConditionalDoubleMap").to_vec(), max_values: None, max_size: Some(7 + 16 + 8), } @@ -1242,7 +1266,9 @@ fn test_storage_info() { #[cfg(feature = "conditional-storage")] { StorageInfo { - prefix: prefix(b"Example", b"ConditionalNMap"), + pallet_name: b"Example".to_vec(), + storage_name: b"ConditionalNMap".to_vec(), + prefix: prefix(b"Example", b"ConditionalNMap").to_vec(), max_values: None, max_size: Some(7 + 16 + 8), } @@ -1254,7 +1280,9 @@ fn test_storage_info() { Example2::storage_info(), vec![ StorageInfo { - prefix: prefix(b"Example2", b"SomeValue"), + pallet_name: b"Example2".to_vec(), + storage_name: b"SomeValue".to_vec(), + prefix: prefix(b"Example2", b"SomeValue").to_vec(), max_values: Some(1), max_size: None, }, diff --git a/substrate/frame/timestamp/src/benchmarking.rs b/substrate/frame/timestamp/src/benchmarking.rs index d64fa8dc69..5d0178dc14 100644 --- a/substrate/frame/timestamp/src/benchmarking.rs +++ b/substrate/frame/timestamp/src/benchmarking.rs @@ -35,8 +35,9 @@ benchmarks! { let did_update_key = crate::DidUpdate::::hashed_key().to_vec(); frame_benchmarking::benchmarking::add_to_whitelist(TrackedStorageKey { key: did_update_key, - has_been_read: false, - has_been_written: true, + reads: 0, + writes: 1, + whitelisted: false, }); }: _(RawOrigin::None, t.into()) verify { diff --git a/substrate/primitives/externalities/src/lib.rs b/substrate/primitives/externalities/src/lib.rs index 7a8771bd62..80bb5b99f3 100644 --- a/substrate/primitives/externalities/src/lib.rs +++ b/substrate/primitives/externalities/src/lib.rs @@ -296,6 +296,13 @@ pub trait Externalities: ExtensionStore { fn proof_size(&self) -> Option { None } + + /// !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! + /// Benchmarking related functionality and shouldn't be used anywhere else! + /// !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! + /// + /// Get all the keys that have been read or written to during the benchmark. + fn get_read_and_written_keys(&self) -> Vec<(Vec, u32, u32, bool)>; } /// Extension for the [`Externalities`] trait. diff --git a/substrate/primitives/state-machine/src/backend.rs b/substrate/primitives/state-machine/src/backend.rs index 5b1f901a3e..0dc054ed50 100644 --- a/substrate/primitives/state-machine/src/backend.rs +++ b/substrate/primitives/state-machine/src/backend.rs @@ -267,6 +267,11 @@ pub trait Backend: sp_std::fmt::Debug { fn proof_size(&self) -> Option { unimplemented!() } + + /// Extend storage info for benchmarking db + fn get_read_and_written_keys(&self) -> Vec<(Vec, u32, u32, bool)> { + unimplemented!() + } } /// Trait that allows consolidate two transactions together. diff --git a/substrate/primitives/state-machine/src/basic.rs b/substrate/primitives/state-machine/src/basic.rs index 08849ebcc6..75b0c1c922 100644 --- a/substrate/primitives/state-machine/src/basic.rs +++ b/substrate/primitives/state-machine/src/basic.rs @@ -339,6 +339,10 @@ impl Externalities for BasicExternalities { fn set_whitelist(&mut self, _: Vec) { unimplemented!("set_whitelist is not supported in Basic") } + + fn get_read_and_written_keys(&self) -> Vec<(Vec, u32, u32, bool)> { + unimplemented!("get_read_and_written_keys is not supported in Basic") + } } impl sp_externalities::ExtensionStore for BasicExternalities { diff --git a/substrate/primitives/state-machine/src/ext.rs b/substrate/primitives/state-machine/src/ext.rs index d1d636cb65..d7d65b905f 100644 --- a/substrate/primitives/state-machine/src/ext.rs +++ b/substrate/primitives/state-machine/src/ext.rs @@ -749,6 +749,10 @@ where fn proof_size(&self) -> Option { self.backend.proof_size() } + + fn get_read_and_written_keys(&self) -> Vec<(Vec, u32, u32, bool)> { + self.backend.get_read_and_written_keys() + } } impl<'a, H, N, B> Ext<'a, H, N, B> diff --git a/substrate/primitives/state-machine/src/read_only.rs b/substrate/primitives/state-machine/src/read_only.rs index 7b67b61eea..01e1fb6b5b 100644 --- a/substrate/primitives/state-machine/src/read_only.rs +++ b/substrate/primitives/state-machine/src/read_only.rs @@ -203,6 +203,10 @@ impl<'a, H: Hasher, B: 'a + Backend> Externalities for ReadOnlyExternalities< fn set_whitelist(&mut self, _: Vec) { unimplemented!("set_whitelist is not supported in ReadOnlyExternalities") } + + fn get_read_and_written_keys(&self) -> Vec<(Vec, u32, u32, bool)> { + unimplemented!("get_read_and_written_keys is not supported in ReadOnlyExternalities") + } } impl<'a, H: Hasher, B: 'a + Backend> sp_externalities::ExtensionStore for ReadOnlyExternalities<'a, H, B> { diff --git a/substrate/primitives/storage/src/lib.rs b/substrate/primitives/storage/src/lib.rs index 76557d6475..87c10f770a 100644 --- a/substrate/primitives/storage/src/lib.rs +++ b/substrate/primitives/storage/src/lib.rs @@ -45,17 +45,56 @@ impl AsRef<[u8]> for StorageKey { #[cfg_attr(feature = "std", derive(Hash, PartialOrd, Ord))] pub struct TrackedStorageKey { pub key: Vec, - pub has_been_read: bool, - pub has_been_written: bool, + pub reads: u32, + pub writes: u32, + pub whitelisted: bool, } -// Easily convert a key to a `TrackedStorageKey` that has been read and written to. +impl TrackedStorageKey { + /// Create a default `TrackedStorageKey` + pub fn new(key: Vec) -> Self { + Self { + key, + reads: 0, + writes: 0, + whitelisted: false, + } + } + /// Check if this key has been "read", i.e. it exists in the memory overlay. + /// + /// Can be true if the key has been read, has been written to, or has been + /// whitelisted. + pub fn has_been_read(&self) -> bool { + self.whitelisted || self.reads > 0u32 || self.has_been_written() + } + /// Check if this key has been "written", i.e. a new value will be committed to the database. + /// + /// Can be true if the key has been written to, or has been whitelisted. + pub fn has_been_written(&self) -> bool { + self.whitelisted || self.writes > 0u32 + } + /// Add a storage read to this key. + pub fn add_read(&mut self) { + self.reads += 1; + } + /// Add a storage write to this key. + pub fn add_write(&mut self) { + self.writes += 1; + } + /// Whitelist this key. + pub fn whitelist(&mut self) { + self.whitelisted = true; + } +} + +// Easily convert a key to a `TrackedStorageKey` that has been whitelisted. impl From> for TrackedStorageKey { fn from(key: Vec) -> Self { Self { key: key, - has_been_read: true, - has_been_written: true, + reads: 0, + writes: 0, + whitelisted: true, } } } diff --git a/substrate/primitives/tasks/src/async_externalities.rs b/substrate/primitives/tasks/src/async_externalities.rs index b646149912..8402246cb4 100644 --- a/substrate/primitives/tasks/src/async_externalities.rs +++ b/substrate/primitives/tasks/src/async_externalities.rs @@ -190,6 +190,10 @@ impl Externalities for AsyncExternalities { fn set_whitelist(&mut self, _: Vec) { unimplemented!("set_whitelist is not supported in AsyncExternalities") } + + fn get_read_and_written_keys(&self) -> Vec<(Vec, u32, u32, bool)> { + unimplemented!("get_read_and_written_keys is not supported in AsyncExternalities") + } } impl sp_externalities::ExtensionStore for AsyncExternalities { diff --git a/substrate/utils/frame/benchmarking-cli/Cargo.toml b/substrate/utils/frame/benchmarking-cli/Cargo.toml index 51290e5f44..4a88a31b7f 100644 --- a/substrate/utils/frame/benchmarking-cli/Cargo.toml +++ b/substrate/utils/frame/benchmarking-cli/Cargo.toml @@ -14,6 +14,7 @@ targets = ["x86_64-unknown-linux-gnu"] [dependencies] frame-benchmarking = { version = "3.1.0", path = "../../../frame/benchmarking" } +frame-support = { version = "3.0.0", path = "../../../frame/support" } sp-core = { version = "3.0.0", path = "../../../primitives/core" } sc-service = { version = "0.9.0", default-features = false, path = "../../../client/service" } sc-cli = { version = "0.9.0", path = "../../../client/cli" } diff --git a/substrate/utils/frame/benchmarking-cli/src/command.rs b/substrate/utils/frame/benchmarking-cli/src/command.rs index 80d95d1c86..e1803a2c56 100644 --- a/substrate/utils/frame/benchmarking-cli/src/command.rs +++ b/substrate/utils/frame/benchmarking-cli/src/command.rs @@ -15,10 +15,10 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::sync::Arc; use crate::BenchmarkCmd; use codec::{Decode, Encode}; use frame_benchmarking::{Analysis, BenchmarkBatch, BenchmarkSelector}; +use frame_support::traits::StorageInfo; use sc_cli::{SharedParams, CliConfiguration, ExecutionStrategy, Result}; use sc_client_db::BenchmarkingState; use sc_executor::NativeExecutor; @@ -31,7 +31,7 @@ use sp_keystore::{ SyncCryptoStorePtr, KeystoreExt, testing::KeyStore, }; -use std::fmt::Debug; +use std::{fmt::Debug, sync::Arc}; impl BenchmarkCmd { /// Runs the command and benchmarks the chain. @@ -98,13 +98,16 @@ impl BenchmarkCmd { .execute(strategy.into()) .map_err(|e| format!("Error executing runtime benchmark: {:?}", e))?; - let results = , String> as Decode>::decode(&mut &result[..]) + let results = , Vec), + String, + > as Decode>::decode(&mut &result[..]) .map_err(|e| format!("Failed to decode benchmark results: {:?}", e))?; match results { - Ok(batches) => { + Ok((batches, storage_info)) => { if let Some(output_path) = &self.output { - crate::writer::write_results(&batches, output_path, self)?; + crate::writer::write_results(&batches, &storage_info, output_path, self)?; } for batch in batches.into_iter() { @@ -129,6 +132,7 @@ impl BenchmarkCmd { print!("extrinsic_time_ns,storage_root_time_ns,reads,repeat_reads,writes,repeat_writes,proof_size_bytes\n"); // Print the values batch.results.iter().for_each(|result| { + let parameters = &result.components; parameters.iter().for_each(|param| print!("{:?},", param.1)); // Print extrinsic time and storage root time diff --git a/substrate/utils/frame/benchmarking-cli/src/template.hbs b/substrate/utils/frame/benchmarking-cli/src/template.hbs index a6f0a5ddfc..2fcc50f823 100644 --- a/substrate/utils/frame/benchmarking-cli/src/template.hbs +++ b/substrate/utils/frame/benchmarking-cli/src/template.hbs @@ -20,6 +20,9 @@ use sp_std::marker::PhantomData; pub struct WeightInfo(PhantomData); impl {{pallet}}::WeightInfo for WeightInfo { {{~#each benchmarks as |benchmark|}} + {{~#each benchmark.comments as |comment|}} + // {{comment}} + {{~/each}} fn {{benchmark.name~}} ( {{~#each benchmark.components as |c| ~}} diff --git a/substrate/utils/frame/benchmarking-cli/src/writer.rs b/substrate/utils/frame/benchmarking-cli/src/writer.rs index 6fd6cc6eef..64a4ea62f0 100644 --- a/substrate/utils/frame/benchmarking-cli/src/writer.rs +++ b/substrate/utils/frame/benchmarking-cli/src/writer.rs @@ -17,7 +17,7 @@ // Outputs benchmark results to Rust files that can be ingested by the runtime. -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::fs; use std::path::PathBuf; use core::convert::TryInto; @@ -26,8 +26,12 @@ use serde::Serialize; use inflector::Inflector; use crate::BenchmarkCmd; -use frame_benchmarking::{BenchmarkBatch, BenchmarkSelector, Analysis, AnalysisChoice, RegressionModel}; +use frame_benchmarking::{ + BenchmarkBatch, BenchmarkSelector, Analysis, AnalysisChoice, RegressionModel, BenchmarkResults, +}; +use sp_core::hexdisplay::HexDisplay; use sp_runtime::traits::Zero; +use frame_support::traits::StorageInfo; const VERSION: &'static str = env!("CARGO_PKG_VERSION"); const TEMPLATE: &str = include_str!("./template.hbs"); @@ -59,6 +63,7 @@ struct BenchmarkData { component_weight: Vec, component_reads: Vec, component_writes: Vec, + comments: Vec, } // This forwards some specific metadata from the `BenchmarkCmd` @@ -108,6 +113,7 @@ fn io_error(s: &str) -> std::io::Error { // ``` fn map_results( batches: &[BenchmarkBatch], + storage_info: &[StorageInfo], analysis_choice: &AnalysisChoice, ) -> Result>, std::io::Error> { // Skip if batches is empty. @@ -123,7 +129,7 @@ fn map_results( let pallet_string = String::from_utf8(batch.pallet.clone()).unwrap(); let instance_string = String::from_utf8(batch.instance.clone()).unwrap(); - let benchmark_data = get_benchmark_data(batch, analysis_choice); + let benchmark_data = get_benchmark_data(batch, storage_info, analysis_choice); pallet_benchmarks.push(benchmark_data); // Check if this is the end of the iterator @@ -157,8 +163,12 @@ fn extract_errors(model: &Option) -> impl Iterator + // Analyze and return the relevant results for a given benchmark. fn get_benchmark_data( batch: &BenchmarkBatch, + storage_info: &[StorageInfo], analysis_choice: &AnalysisChoice, ) -> BenchmarkData { + // You can use this to put any additional comments with the benchmarking output. + let mut comments = Vec::::new(); + // Analyze benchmarks to get the linear regression. let analysis_function = match analysis_choice { AnalysisChoice::MinSquares => Analysis::min_squares_iqr, @@ -229,6 +239,9 @@ fn get_benchmark_data( }) .collect::>(); + // We add additional comments showing which storage items were touched. + add_storage_comments(&mut comments, &batch.results, storage_info); + BenchmarkData { name: String::from_utf8(batch.benchmark.clone()).unwrap(), components, @@ -238,12 +251,14 @@ fn get_benchmark_data( component_weight: used_extrinsic_time, component_reads: used_reads, component_writes: used_writes, + comments, } } // Create weight file from benchmark data and Handlebars template. pub fn write_results( batches: &[BenchmarkBatch], + storage_info: &[StorageInfo], path: &PathBuf, cmd: &BenchmarkCmd, ) -> Result<(), std::io::Error> { @@ -298,7 +313,7 @@ pub fn write_results( handlebars.register_escape_fn(|s| -> String { s.to_string() }); // Organize results by pallet into a JSON map - let all_results = map_results(batches, &analysis_choice)?; + let all_results = map_results(batches, storage_info, &analysis_choice)?; for ((pallet, instance), results) in all_results.iter() { let mut file_path = path.clone(); // If a user only specified a directory... @@ -332,6 +347,57 @@ pub fn write_results( Ok(()) } +// This function looks at the keys touched during the benchmark, and the storage info we collected +// from the pallets, and creates comments with information about the storage keys touched during +// each benchmark. +fn add_storage_comments( + comments: &mut Vec, + results: &[BenchmarkResults], + storage_info: &[StorageInfo], +) { + let storage_info_map = storage_info.iter().map(|info| (info.prefix.clone(), info)) + .collect::>(); + // This tracks the keys we already identified, so we only generate a single comment. + let mut identified = HashSet::>::new(); + + for result in results.clone() { + for (key, reads, writes, whitelisted) in &result.keys { + // skip keys which are whitelisted + if *whitelisted { continue; } + let prefix_length = key.len().min(32); + let prefix = key[0..prefix_length].to_vec(); + if identified.contains(&prefix) { + // skip adding comments for keys we already identified + continue; + } else { + // track newly identified keys + identified.insert(prefix.clone()); + } + match storage_info_map.get(&prefix) { + Some(key_info) => { + let comment = format!( + "Storage: {} {} (r:{} w:{})", + String::from_utf8(key_info.pallet_name.clone()).expect("encoded from string"), + String::from_utf8(key_info.storage_name.clone()).expect("encoded from string"), + reads, + writes, + ); + comments.push(comment) + }, + None => { + let comment = format!( + "Storage: unknown [0x{}] (r:{} w:{})", + HexDisplay::from(key), + reads, + writes, + ); + comments.push(comment) + } + } + } + } +} + // Add an underscore after every 3rd character, i.e. a separator for large numbers. fn underscore(i: Number) -> String where Number: std::string::ToString @@ -422,6 +488,7 @@ mod test { writes: (base + slope * i).into(), repeat_writes: 0, proof_size: 0, + keys: vec![], } ) } @@ -475,11 +542,15 @@ mod test { #[test] fn map_results_works() { - let mapped_results = map_results(&[ - test_data(b"first", b"first", BenchmarkParameter::a, 10, 3), - test_data(b"first", b"second", BenchmarkParameter::b, 9, 2), - test_data(b"second", b"first", BenchmarkParameter::c, 3, 4), - ], &AnalysisChoice::default()).unwrap(); + let mapped_results = map_results( + &[ + test_data(b"first", b"first", BenchmarkParameter::a, 10, 3), + test_data(b"first", b"second", BenchmarkParameter::b, 9, 2), + test_data(b"second", b"first", BenchmarkParameter::c, 3, 4), + ], + &[], + &AnalysisChoice::default(), + ).unwrap(); let first_benchmark = &mapped_results.get( &("first_pallet".to_string(), "instance".to_string())