diff --git a/Cargo.lock b/Cargo.lock index 96bc03fd1c305..f999d987754b1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6475,6 +6475,7 @@ dependencies = [ "cumulus-primitives-proof-size-hostfunction", "cumulus-test-runtime", "frame-benchmarking", + "frame-storage-access-test-runtime", "frame-support", "frame-system", "gethostname", @@ -6493,6 +6494,8 @@ dependencies = [ "sc-client-api", "sc-client-db", "sc-executor 0.32.0", + "sc-executor-common 0.29.0", + "sc-executor-wasmtime 0.29.0", "sc-runtime-utilities", "sc-service", "sc-sysinfo", @@ -6744,6 +6747,19 @@ dependencies = [ "tokio-retry", ] +[[package]] +name = "frame-storage-access-test-runtime" +version = "0.1.0" +dependencies = [ + "cumulus-pallet-parachain-system", + "parity-scale-codec", + "sp-core 28.0.0", + "sp-runtime 31.0.1", + "sp-state-machine 0.35.0", + "sp-trie 29.0.0", + "substrate-wasm-builder", +] + [[package]] name = "frame-support" version = "28.0.0" diff --git a/Cargo.toml b/Cargo.toml index 4e2defc9b5879..bbb0ae4b5a00f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -554,6 +554,7 @@ members = [ "substrate/utils/frame/rpc/state-trie-migration-rpc", "substrate/utils/frame/rpc/support", "substrate/utils/frame/rpc/system", + "substrate/utils/frame/storage-access-test-runtime", "substrate/utils/prometheus", "substrate/utils/substrate-bip39", "substrate/utils/wasm-builder", @@ -806,6 +807,7 @@ frame-election-provider-support = { path = "substrate/frame/election-provider-su frame-executive = { path = "substrate/frame/executive", default-features = false } frame-metadata = { version = "23.0.0", default-features = false } frame-metadata-hash-extension = { path = "substrate/frame/metadata-hash-extension", default-features = false } +frame-storage-access-test-runtime = { path = "substrate/utils/frame/storage-access-test-runtime", default-features = false } frame-support = { path = "substrate/frame/support", default-features = false } frame-support-procedural = { path = "substrate/frame/support/procedural", default-features = false } frame-support-procedural-tools = { path = "substrate/frame/support/procedural/tools", default-features = false } diff --git a/cumulus/pallets/parachain-system/src/validate_block/mod.rs b/cumulus/pallets/parachain-system/src/validate_block/mod.rs index 8b10d7ca4e50c..28b744f7eb6b5 100644 --- a/cumulus/pallets/parachain-system/src/validate_block/mod.rs +++ b/cumulus/pallets/parachain-system/src/validate_block/mod.rs @@ -24,11 +24,11 @@ mod tests; #[cfg(not(feature = "std"))] #[doc(hidden)] -mod trie_cache; +pub mod trie_cache; #[cfg(any(test, not(feature = "std")))] #[doc(hidden)] -mod trie_recorder; +pub mod trie_recorder; #[cfg(not(feature = "std"))] #[doc(hidden)] diff --git a/cumulus/pallets/parachain-system/src/validate_block/trie_cache.rs b/cumulus/pallets/parachain-system/src/validate_block/trie_cache.rs index 9590af993e9f9..fde3bb1ddc294 100644 --- a/cumulus/pallets/parachain-system/src/validate_block/trie_cache.rs +++ b/cumulus/pallets/parachain-system/src/validate_block/trie_cache.rs @@ -27,7 +27,7 @@ use trie_db::{node::NodeOwned, Hasher}; /// Special purpose trie cache implementation that is able to cache an unlimited number /// of values. To be used in `validate_block` to serve values and nodes that /// have already been loaded and decoded from the storage proof. -pub(crate) struct TrieCache<'a, H: Hasher> { +pub struct TrieCache<'a, H: Hasher> { node_cache: RefMut<'a, BTreeMap>>, value_cache: Option, trie_db::CachedValue>>>, } @@ -65,7 +65,7 @@ impl<'a, H: Hasher> trie_db::TrieCache> for TrieCache<'a, H> { } /// Provider of [`TrieCache`] instances. -pub(crate) struct CacheProvider { +pub struct CacheProvider { node_cache: RefCell>>, /// Cache: `storage_root` => `storage_key` => `value`. /// diff --git a/cumulus/pallets/parachain-system/src/validate_block/trie_recorder.rs b/cumulus/pallets/parachain-system/src/validate_block/trie_recorder.rs index c164cebd351f1..e8e400cee230e 100644 --- a/cumulus/pallets/parachain-system/src/validate_block/trie_recorder.rs +++ b/cumulus/pallets/parachain-system/src/validate_block/trie_recorder.rs @@ -34,7 +34,7 @@ use trie_db::{Hasher, RecordedForKey, TrieAccess}; /// /// The internal size counting logic should align /// with ['sp_trie::recorder::Recorder']. -pub(crate) struct SizeOnlyRecorder<'a, H: Hasher> { +pub struct SizeOnlyRecorder<'a, H: Hasher> { seen_nodes: RefMut<'a, BTreeSet>, encoded_size: RefMut<'a, usize>, recorded_keys: RefMut<'a, BTreeMap, RecordedForKey>>, @@ -90,7 +90,7 @@ impl<'a, H: trie_db::Hasher> trie_db::TrieRecorder for SizeOnlyRecorder< } #[derive(Clone)] -pub(crate) struct SizeOnlyRecorderProvider { +pub struct SizeOnlyRecorderProvider { seen_nodes: Rc>>, encoded_size: Rc>, recorded_keys: Rc, RecordedForKey>>>, diff --git a/prdoc/pr_8069.prdoc b/prdoc/pr_8069.prdoc new file mode 100644 index 0000000000000..45f9c6d3d4348 --- /dev/null +++ b/prdoc/pr_8069.prdoc @@ -0,0 +1,17 @@ +# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0 +# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json + +title: Benchmark storage access on block validation + +doc: + - audience: [Runtime Dev, Node Dev] + description: | + Adds checking storage weights on block validation for both read and write benchmarks. + +crates: + - name: cumulus-pallet-parachain-system + bump: minor + - name: frame-benchmarking-cli + bump: minor + - name: frame-storage-access-test-runtime + bump: major diff --git a/substrate/utils/frame/benchmarking-cli/Cargo.toml b/substrate/utils/frame/benchmarking-cli/Cargo.toml index 540a08a98953d..b903175a38149 100644 --- a/substrate/utils/frame/benchmarking-cli/Cargo.toml +++ b/substrate/utils/frame/benchmarking-cli/Cargo.toml @@ -25,6 +25,7 @@ comfy-table = { workspace = true } cumulus-client-parachain-inherent = { workspace = true, default-features = true } cumulus-primitives-proof-size-hostfunction = { workspace = true, default-features = true } frame-benchmarking = { workspace = true, default-features = true } +frame-storage-access-test-runtime = { workspace = true, default-features = true } frame-support = { workspace = true, default-features = true } frame-system = { workspace = true, default-features = true } gethostname = { workspace = true } @@ -42,6 +43,8 @@ sc-cli = { workspace = true, default-features = false } sc-client-api = { workspace = true, default-features = true } sc-client-db = { workspace = true, default-features = false } sc-executor = { workspace = true, default-features = true } +sc-executor-common = { workspace = true } +sc-executor-wasmtime = { workspace = true } sc-runtime-utilities = { workspace = true, default-features = true } sc-service = { workspace = true, default-features = false } sc-sysinfo = { workspace = true, default-features = true } @@ -79,6 +82,7 @@ westend-runtime = { workspace = true, default-features = true } default = [] runtime-benchmarks = [ "frame-benchmarking/runtime-benchmarks", + "frame-storage-access-test-runtime/runtime-benchmarks", "frame-support/runtime-benchmarks", "frame-system/runtime-benchmarks", "polkadot-parachain-primitives/runtime-benchmarks", diff --git a/substrate/utils/frame/benchmarking-cli/src/storage/cmd.rs b/substrate/utils/frame/benchmarking-cli/src/storage/cmd.rs index da791c25d228f..68a81fb51524a 100644 --- a/substrate/utils/frame/benchmarking-cli/src/storage/cmd.rs +++ b/substrate/utils/frame/benchmarking-cli/src/storage/cmd.rs @@ -26,7 +26,7 @@ use sp_runtime::traits::{Block as BlockT, HashingFor}; use sp_state_machine::Storage; use sp_storage::{ChildInfo, ChildType, PrefixedStorageKey, StateVersion}; -use clap::{Args, Parser}; +use clap::{Args, Parser, ValueEnum}; use log::info; use rand::prelude::*; use serde::Serialize; @@ -36,6 +36,16 @@ use std::{fmt::Debug, path::PathBuf, sync::Arc}; use super::template::TemplateData; use crate::shared::{new_rng, HostInfoParams, WeightParams}; +/// The mode in which to run the storage benchmark. +#[derive(Default, Debug, Clone, Copy, PartialEq, Eq, Serialize, ValueEnum)] +pub enum StorageBenchmarkMode { + /// Run the benchmark for block import. + #[default] + ImportBlock, + /// Run the benchmark for block validation. + ValidateBlock, +} + /// Benchmark the storage speed of a chain snapshot. #[derive(Debug, Parser)] pub struct StorageCmd { @@ -129,13 +139,36 @@ pub struct StorageParams { #[arg(long, default_value = "false")] pub disable_pov_recorder: bool, - /// The batch size for the write benchmark. + /// The batch size for the read/write benchmark. /// /// Since the write size needs to also include the cost of computing the storage root, which is /// done once at the end of the block, the batch size is used to simulate multiple writes in a /// block. #[arg(long, default_value_t = 100_000)] pub batch_size: usize, + + /// The mode in which to run the storage benchmark. + /// + /// PoV recorder must be activated to provide a storage proof for block validation at runtime. + #[arg(long, value_enum, default_value_t = StorageBenchmarkMode::ImportBlock)] + pub mode: StorageBenchmarkMode, + + /// Number of rounds to execute block validation during the benchmark. + /// + /// We need to run the benchmark several times to avoid fluctuations during runtime setup. + /// This is only used when `mode` is `validate-block`. + #[arg(long, default_value_t = 20)] + pub validate_block_rounds: u32, +} + +impl StorageParams { + pub fn is_import_block_mode(&self) -> bool { + matches!(self.mode, StorageBenchmarkMode::ImportBlock) + } + + pub fn is_validate_block_mode(&self) -> bool { + matches!(self.mode, StorageBenchmarkMode::ValidateBlock) + } } impl StorageCmd { diff --git a/substrate/utils/frame/benchmarking-cli/src/storage/mod.rs b/substrate/utils/frame/benchmarking-cli/src/storage/mod.rs index 188cc5e3d4e41..ef04fde9a8d51 100644 --- a/substrate/utils/frame/benchmarking-cli/src/storage/mod.rs +++ b/substrate/utils/frame/benchmarking-cli/src/storage/mod.rs @@ -21,3 +21,37 @@ pub mod template; pub mod write; pub use cmd::StorageCmd; + +/// Empirically, the maximum batch size for block validation should be no more than 10,000. +/// Bigger sizes may cause problems with runtime memory allocation. +pub(crate) const MAX_BATCH_SIZE_FOR_BLOCK_VALIDATION: usize = 10_000; + +pub(crate) fn get_wasm_module() -> Box { + let blob = sc_executor_common::runtime_blob::RuntimeBlob::uncompress_if_needed( + frame_storage_access_test_runtime::WASM_BINARY + .expect("You need to build the WASM binaries to run the benchmark!"), + ) + .expect("Failed to create runtime blob"); + let config = sc_executor_wasmtime::Config { + allow_missing_func_imports: true, + cache_path: None, + semantics: sc_executor_wasmtime::Semantics { + heap_alloc_strategy: sc_executor_common::wasm_runtime::HeapAllocStrategy::Dynamic { + maximum_pages: Some(4096), + }, + instantiation_strategy: sc_executor::WasmtimeInstantiationStrategy::PoolingCopyOnWrite, + deterministic_stack_limit: None, + canonicalize_nans: false, + parallel_compilation: false, + wasm_multi_value: false, + wasm_bulk_memory: false, + wasm_reference_types: false, + wasm_simd: false, + }, + }; + + Box::new( + sc_executor_wasmtime::create_runtime::(blob, config) + .expect("Unable to create wasm module."), + ) +} diff --git a/substrate/utils/frame/benchmarking-cli/src/storage/read.rs b/substrate/utils/frame/benchmarking-cli/src/storage/read.rs index 126eb815f75b5..7a222d7167a70 100644 --- a/substrate/utils/frame/benchmarking-cli/src/storage/read.rs +++ b/substrate/utils/frame/benchmarking-cli/src/storage/read.rs @@ -15,16 +15,20 @@ // See the License for the specific language governing permissions and // limitations under the License. -use log::info; +use codec::Encode; +use frame_storage_access_test_runtime::StorageAccessParams; +use log::{debug, info}; use rand::prelude::*; use sc_cli::{Error, Result}; use sc_client_api::{Backend as ClientBackend, StorageProvider, UsageProvider}; use sp_api::CallApiAt; use sp_runtime::traits::{Block as BlockT, HashingFor, Header as HeaderT}; use sp_state_machine::{backend::AsTrieBackend, Backend}; +use sp_storage::ChildInfo; +use sp_trie::StorageProof; use std::{fmt::Debug, sync::Arc, time::Instant}; -use super::cmd::StorageCmd; +use super::{cmd::StorageCmd, get_wasm_module, MAX_BATCH_SIZE_FOR_BLOCK_VALIDATION}; use crate::shared::{new_rng, BenchRecord}; impl StorageCmd { @@ -41,6 +45,15 @@ impl StorageCmd { BA: ClientBackend, <::Header as HeaderT>::Number: From, { + if self.params.is_validate_block_mode() && self.params.disable_pov_recorder { + return Err("PoV recorder must be activated to provide a storage proof for block validation at runtime. Remove `--disable-pov-recorder` from the command line.".into()) + } + if self.params.is_validate_block_mode() && + self.params.batch_size > MAX_BATCH_SIZE_FOR_BLOCK_VALIDATION + { + return Err(format!("Batch size is too large. This may cause problems with runtime memory allocation. Better set `--batch-size {}` or less.", MAX_BATCH_SIZE_FOR_BLOCK_VALIDATION).into()) + } + let mut record = BenchRecord::default(); let best_hash = client.usage_info().chain.best_hash; @@ -49,6 +62,9 @@ impl StorageCmd { let mut keys: Vec<_> = client.storage_keys(best_hash, None, None)?.collect(); let (mut rng, _) = new_rng(None); keys.shuffle(&mut rng); + if keys.is_empty() { + return Err("Can't process benchmarking with empty storage".into()) + } let mut child_nodes = Vec::new(); // Interesting part here: @@ -57,85 +73,191 @@ impl StorageCmd { // Read using the same TrieBackend and recorder for up to `batch_size` keys. // This would allow us to measure the amortized cost of reading a key. - let recorder = (!self.params.disable_pov_recorder).then(|| Default::default()); - let mut state = client + let state = client .state_at(best_hash) .map_err(|_err| Error::Input("State not found".into()))?; - let mut as_trie_backend = state.as_trie_backend(); - let mut backend = sp_state_machine::TrieBackendBuilder::wrap(&as_trie_backend) - .with_optional_recorder(recorder) - .build(); + // We reassign the backend and recorder for every batch size. + // Using a new recorder for every read vs using the same for the entire batch + // produces significant different results. Since in the real use case we use a + // single recorder per block, simulate the same behavior by creating a new + // recorder every batch size, so that the amortized cost of reading a key is + // measured in conditions closer to the real world. + let (mut backend, mut recorder) = self.create_backend::(&state); + let mut read_in_batch = 0; + let mut on_validation_batch = vec![]; + let mut on_validation_size = 0; + let last_key = keys.last().expect("Checked above to be non-empty"); for key in keys.as_slice() { match (self.params.include_child_trees, self.is_child_key(key.clone().0)) { (true, Some(info)) => { // child tree key for ck in client.child_storage_keys(best_hash, info.clone(), None, None)? { - child_nodes.push((ck.clone(), info.clone())); + child_nodes.push((ck, info.clone())); } }, _ => { // regular key + on_validation_batch.push((key.0.clone(), None)); let start = Instant::now(); - let v = backend .storage(key.0.as_ref()) .expect("Checked above to exist") .ok_or("Value unexpectedly empty")?; - record.append(v.len(), start.elapsed())?; + on_validation_size += v.len(); + if self.params.is_import_block_mode() { + record.append(v.len(), start.elapsed())?; + } }, } read_in_batch += 1; - if read_in_batch >= self.params.batch_size { - // Using a new recorder for every read vs using the same for the entire batch - // produces significant different results. Since in the real use case we use a - // single recorder per block, simulate the same behavior by creating a new - // recorder every batch size, so that the amortized cost of reading a key is - // measured in conditions closer to the real world. - let recorder = (!self.params.disable_pov_recorder).then(|| Default::default()); - state = client - .state_at(best_hash) - .map_err(|_err| Error::Input("State not found".to_string()))?; - as_trie_backend = state.as_trie_backend(); - backend = sp_state_machine::TrieBackendBuilder::wrap(&as_trie_backend) - .with_optional_recorder(recorder) - .build(); + let is_batch_full = read_in_batch >= self.params.batch_size || key == last_key; + + // Read keys on block validation + if is_batch_full && self.params.is_validate_block_mode() { + let root = backend.root(); + let storage_proof = recorder + .clone() + .map(|r| r.drain_storage_proof()) + .expect("Storage proof must exist for block validation"); + let elapsed = measure_block_validation::( + *root, + storage_proof, + on_validation_batch.clone(), + self.params.validate_block_rounds, + ); + record.append(on_validation_size / on_validation_batch.len(), elapsed)?; + + on_validation_batch = vec![]; + on_validation_size = 0; + } + + // Reload recorder + if is_batch_full { + (backend, recorder) = self.create_backend::(&state); read_in_batch = 0; } } - if self.params.include_child_trees { + if self.params.include_child_trees && !child_nodes.is_empty() { child_nodes.shuffle(&mut rng); info!("Reading {} child keys", child_nodes.len()); + let (last_child_key, last_child_info) = + child_nodes.last().expect("Checked above to be non-empty"); for (key, info) in child_nodes.as_slice() { + on_validation_batch.push((key.0.clone(), Some(info.clone()))); let start = Instant::now(); let v = backend .child_storage(info, key.0.as_ref()) .expect("Checked above to exist") .ok_or("Value unexpectedly empty")?; - record.append(v.len(), start.elapsed())?; - + on_validation_size += v.len(); + if self.params.is_import_block_mode() { + record.append(v.len(), start.elapsed())?; + } read_in_batch += 1; - if read_in_batch >= self.params.batch_size { - // Using a new recorder for every read vs using the same for the entire batch - // produces significant different results. Since in the real use case we use a - // single recorder per block, simulate the same behavior by creating a new - // recorder every batch size, so that the amortized cost of reading a key is - // measured in conditions closer to the real world. - let recorder = (!self.params.disable_pov_recorder).then(|| Default::default()); - state = client - .state_at(best_hash) - .map_err(|_err| Error::Input("State not found".to_string()))?; - as_trie_backend = state.as_trie_backend(); - backend = sp_state_machine::TrieBackendBuilder::wrap(&as_trie_backend) - .with_optional_recorder(recorder) - .build(); + let is_batch_full = read_in_batch >= self.params.batch_size || + (last_child_key == key && last_child_info == info); + + // Read child keys on block validation + if is_batch_full && self.params.is_validate_block_mode() { + let root = backend.root(); + let storage_proof = recorder + .clone() + .map(|r| r.drain_storage_proof()) + .expect("Storage proof must exist for block validation"); + let elapsed = measure_block_validation::( + *root, + storage_proof, + on_validation_batch.clone(), + self.params.validate_block_rounds, + ); + record.append(on_validation_size / on_validation_batch.len(), elapsed)?; + + on_validation_batch = vec![]; + on_validation_size = 0; + } + + // Reload recorder + if is_batch_full { + (backend, recorder) = self.create_backend::(&state); read_in_batch = 0; } } } + Ok(record) } + + fn create_backend<'a, B, C>( + &self, + state: &'a C::StateBackend, + ) -> ( + sp_state_machine::TrieBackend< + &'a >>::TrieBackendStorage, + HashingFor, + &'a sp_trie::cache::LocalTrieCache>, + >, + Option>>, + ) + where + C: CallApiAt, + B: BlockT + Debug, + { + let recorder = (!self.params.disable_pov_recorder).then(|| Default::default()); + let backend = sp_state_machine::TrieBackendBuilder::wrap(state.as_trie_backend()) + .with_optional_recorder(recorder.clone()) + .build(); + + (backend, recorder) + } +} + +fn measure_block_validation( + root: B::Hash, + storage_proof: StorageProof, + on_validation_batch: Vec<(Vec, Option)>, + rounds: u32, +) -> std::time::Duration { + debug!( + "POV: len {:?} {:?}", + storage_proof.len(), + storage_proof.clone().encoded_compact_size::>(root) + ); + let batch_size = on_validation_batch.len(); + let wasm_module = get_wasm_module(); + let mut instance = wasm_module.new_instance().expect("Failed to create wasm instance"); + let params = StorageAccessParams::::new_read(root, storage_proof, on_validation_batch); + let dry_run_encoded = params.as_dry_run().encode(); + let encoded = params.encode(); + + let mut durations_in_nanos = Vec::new(); + + for i in 1..=rounds { + info!("validate_block with {} keys, round {}/{}", batch_size, i, rounds); + + // Dry run to get the time it takes without storage access + let dry_run_start = Instant::now(); + instance + .call_export("validate_block", &dry_run_encoded) + .expect("Failed to call validate_block"); + let dry_run_elapsed = dry_run_start.elapsed(); + debug!("validate_block dry-run time {:?}", dry_run_elapsed); + + let start = Instant::now(); + instance + .call_export("validate_block", &encoded) + .expect("Failed to call validate_block"); + let elapsed = start.elapsed(); + debug!("validate_block time {:?}", elapsed); + + durations_in_nanos + .push(elapsed.saturating_sub(dry_run_elapsed).as_nanos() as u64 / batch_size as u64); + } + + std::time::Duration::from_nanos( + durations_in_nanos.iter().sum::() / durations_in_nanos.len() as u64, + ) } diff --git a/substrate/utils/frame/benchmarking-cli/src/storage/template.rs b/substrate/utils/frame/benchmarking-cli/src/storage/template.rs index 43aea75b47711..03802dedc38ac 100644 --- a/substrate/utils/frame/benchmarking-cli/src/storage/template.rs +++ b/substrate/utils/frame/benchmarking-cli/src/storage/template.rs @@ -74,7 +74,11 @@ impl TemplateData { .unwrap_or_default(); Ok(TemplateData { - db_name: format!("{}", cfg.database), + db_name: if params.is_validate_block_mode() { + String::from("InMemoryDb") + } else { + format!("{}", cfg.database) + }, runtime_name: cfg.chain_spec.name().into(), version: VERSION.into(), date: chrono::Utc::now().format("%Y-%m-%d (Y/M/D)").to_string(), diff --git a/substrate/utils/frame/benchmarking-cli/src/storage/weights.hbs b/substrate/utils/frame/benchmarking-cli/src/storage/weights.hbs index 135b18b193746..33e3eea2e7e21 100644 --- a/substrate/utils/frame/benchmarking-cli/src/storage/weights.hbs +++ b/substrate/utils/frame/benchmarking-cli/src/storage/weights.hbs @@ -22,7 +22,11 @@ pub mod constants { use sp_weights::RuntimeDbWeight; parameter_types! { - {{#if (eq db_name "ParityDb")}} + {{#if (eq db_name "InMemoryDb")}} + /// `InMemoryDb` weights are measured in the context of the validation functions. + /// To avoid submitting overweight blocks to the relay chain this is the configuration + /// parachains should use. + {{else if (eq db_name "ParityDb")}} /// `ParityDB` can be enabled with a feature flag, but is still experimental. These weights /// are available for brave runtime engineers who may want to try this out as default. {{else}} diff --git a/substrate/utils/frame/benchmarking-cli/src/storage/write.rs b/substrate/utils/frame/benchmarking-cli/src/storage/write.rs index 4a56ee3f9d8a1..fa818b847f124 100644 --- a/substrate/utils/frame/benchmarking-cli/src/storage/write.rs +++ b/substrate/utils/frame/benchmarking-cli/src/storage/write.rs @@ -15,6 +15,10 @@ // See the License for the specific language governing permissions and // limitations under the License. +use codec::Encode; +use frame_storage_access_test_runtime::StorageAccessParams; +use log::{debug, info, trace, warn}; +use rand::prelude::*; use sc_cli::Result; use sc_client_api::{Backend as ClientBackend, StorageProvider, UsageProvider}; use sc_client_db::{DbHash, DbState, DbStateBuilder}; @@ -22,23 +26,25 @@ use sp_blockchain::HeaderBackend; use sp_database::{ColumnId, Transaction}; use sp_runtime::traits::{Block as BlockT, HashingFor, Header as HeaderT}; use sp_state_machine::Backend as StateBackend; -use sp_trie::PrefixedMemoryDB; - -use log::{info, trace}; -use rand::prelude::*; use sp_storage::{ChildInfo, StateVersion}; +use sp_trie::{recorder::Recorder, PrefixedMemoryDB}; use std::{ fmt::Debug, sync::Arc, time::{Duration, Instant}, }; -use super::cmd::StorageCmd; +use super::{cmd::StorageCmd, get_wasm_module, MAX_BATCH_SIZE_FOR_BLOCK_VALIDATION}; use crate::shared::{new_rng, BenchRecord}; impl StorageCmd { /// Benchmarks the time it takes to write a single Storage item. + /// /// Uses the latest state that is available for the given client. + /// + /// Unlike reading benchmark, where we read every single key, here we write a batch of keys in + /// one time. So writing a remaining keys with the size much smaller than batch size can + /// dramatically distort the results. To avoid this, we skip the remaining keys. pub(crate) fn bench_write( &self, client: Arc, @@ -52,6 +58,15 @@ impl StorageCmd { BA: ClientBackend, C: UsageProvider + HeaderBackend + StorageProvider, { + if self.params.is_validate_block_mode() && self.params.disable_pov_recorder { + return Err("PoV recorder must be activated to provide a storage proof for block validation at runtime. Remove `--disable-pov-recorder`.".into()) + } + if self.params.is_validate_block_mode() && + self.params.batch_size > MAX_BATCH_SIZE_FOR_BLOCK_VALIDATION + { + return Err(format!("Batch size is too large. This may cause problems with runtime memory allocation. Better set `--batch-size {}` or less.", MAX_BATCH_SIZE_FOR_BLOCK_VALIDATION).into()) + } + // Store the time that it took to write each value. let mut record = BenchRecord::default(); @@ -59,28 +74,26 @@ impl StorageCmd { let header = client.header(best_hash)?.ok_or("Header not found")?; let original_root = *header.state_root(); - info!("Preparing keys from block {}", best_hash); - let build_trie_backend = |storage: Arc< - dyn sp_state_machine::Storage>, - >, - original_root, - enable_pov_recorder: bool| { - let pov_recorder = enable_pov_recorder.then(|| Default::default()); - - DbStateBuilder::>::new(storage.clone(), original_root) - .with_optional_cache(shared_trie_cache.as_ref().map(|c| c.local_cache_trusted())) - .with_optional_recorder(pov_recorder) - .build() - }; - - let trie = - build_trie_backend(storage.clone(), original_root, !self.params.disable_pov_recorder); + let (trie, _) = self.create_trie_backend::( + original_root, + &storage, + shared_trie_cache.as_ref(), + ); + info!("Preparing keys from block {}", best_hash); // Load all KV pairs and randomly shuffle them. let mut kvs: Vec<_> = trie.pairs(Default::default())?.collect(); let (mut rng, _) = new_rng(None); kvs.shuffle(&mut rng); - info!("Writing {} keys", kvs.len()); + if kvs.is_empty() { + return Err("Can't process benchmarking with empty storage".into()) + } + + info!("Writing {} keys in batches of {}", kvs.len(), self.params.batch_size); + let remainder = kvs.len() % self.params.batch_size; + if self.params.is_validate_block_mode() && remainder != 0 { + info!("Remaining `{remainder}` keys will be skipped"); + } let mut child_nodes = Vec::new(); let mut batched_keys = Vec::new(); @@ -91,11 +104,10 @@ impl StorageCmd { let (k, original_v) = key_value?; match (self.params.include_child_trees, self.is_child_key(k.to_vec())) { (true, Some(info)) => { - let child_keys = - client.child_storage_keys(best_hash, info.clone(), None, None)?; - for ck in child_keys { - child_nodes.push((ck.clone(), info.clone())); - } + let child_keys = client + .child_storage_keys(best_hash, info.clone(), None, None)? + .collect::>(); + child_nodes.push((child_keys, info.clone())); }, _ => { // regular key @@ -124,81 +136,248 @@ impl StorageCmd { continue } - // For every batched write use a different trie instance and recorder, so we - // don't benefit from past runs. - let trie = build_trie_backend( - storage.clone(), - original_root, - !self.params.disable_pov_recorder, - ); // Write each value in one commit. - let (size, duration) = measure_per_key_amortised_write_cost::( - db.clone(), - &trie, - batched_keys.clone(), - self.state_version(), - state_col, - None, - )?; + let (size, duration) = if self.params.is_validate_block_mode() { + self.measure_per_key_amortised_validate_block_write_cost::( + original_root, + &storage, + shared_trie_cache.as_ref(), + batched_keys.clone(), + None, + )? + } else { + self.measure_per_key_amortised_import_block_write_cost::( + original_root, + &storage, + shared_trie_cache.as_ref(), + db.clone(), + batched_keys.clone(), + self.state_version(), + state_col, + None, + )? + }; record.append(size, duration)?; batched_keys.clear(); }, } } - if self.params.include_child_trees { - child_nodes.shuffle(&mut rng); - info!("Writing {} child keys", child_nodes.len()); + if self.params.include_child_trees && !child_nodes.is_empty() { + info!("Writing {} child keys", child_nodes.iter().map(|(c, _)| c.len()).sum::()); + for (mut child_keys, info) in child_nodes { + if child_keys.len() < self.params.batch_size { + warn!( + "{} child keys will be skipped because it's less than batch size", + child_keys.len() + ); + continue; + } - for (key, info) in child_nodes { - if let Some(original_v) = client - .child_storage(best_hash, &info.clone(), &key) - .expect("Checked above to exist") - { - let mut new_v = vec![0; original_v.0.len()]; + child_keys.shuffle(&mut rng); - loop { - rng.fill_bytes(&mut new_v[..]); - if check_new_value::( - db.clone(), - &trie, - &key.0, - &new_v, - self.state_version(), - state_col, - Some(&info), - ) { - break + for key in child_keys { + if let Some(original_v) = client + .child_storage(best_hash, &info, &key) + .expect("Checked above to exist") + { + let mut new_v = vec![0; original_v.0.len()]; + + loop { + rng.fill_bytes(&mut new_v[..]); + if check_new_value::( + db.clone(), + &trie, + &key.0, + &new_v, + self.state_version(), + state_col, + Some(&info), + ) { + break + } + } + batched_keys.push((key.0, new_v.to_vec())); + if batched_keys.len() < self.params.batch_size { + continue } - } - batched_keys.push((key.0, new_v.to_vec())); - if batched_keys.len() < self.params.batch_size { - continue + let (size, duration) = if self.params.is_validate_block_mode() { + self.measure_per_key_amortised_validate_block_write_cost::( + original_root, + &storage, + shared_trie_cache.as_ref(), + batched_keys.clone(), + None, + )? + } else { + self.measure_per_key_amortised_import_block_write_cost::( + original_root, + &storage, + shared_trie_cache.as_ref(), + db.clone(), + batched_keys.clone(), + self.state_version(), + state_col, + Some(&info), + )? + }; + record.append(size, duration)?; + batched_keys.clear(); } - - let trie = build_trie_backend( - storage.clone(), - original_root, - !self.params.disable_pov_recorder, - ); - - let (size, duration) = measure_per_key_amortised_write_cost::( - db.clone(), - &trie, - batched_keys.clone(), - self.state_version(), - state_col, - Some(&info), - )?; - record.append(size, duration)?; - batched_keys.clear(); } } } Ok(record) } + + fn create_trie_backend( + &self, + original_root: Block::Hash, + storage: &Arc>>, + shared_trie_cache: Option<&sp_trie::cache::SharedTrieCache>>, + ) -> (DbState>, Option>>) + where + Block: BlockT
+ Debug, + H: HeaderT, + { + let recorder = (!self.params.disable_pov_recorder).then(|| Default::default()); + let trie = DbStateBuilder::>::new(storage.clone(), original_root) + .with_optional_cache(shared_trie_cache.map(|c| c.local_cache_trusted())) + .with_optional_recorder(recorder.clone()) + .build(); + + (trie, recorder) + } + + /// Measures write benchmark + /// if `child_info` exist then it means this is a child tree key + fn measure_per_key_amortised_import_block_write_cost( + &self, + original_root: Block::Hash, + storage: &Arc>>, + shared_trie_cache: Option<&sp_trie::cache::SharedTrieCache>>, + db: Arc>, + changes: Vec<(Vec, Vec)>, + version: StateVersion, + col: ColumnId, + child_info: Option<&ChildInfo>, + ) -> Result<(usize, Duration)> + where + Block: BlockT
+ Debug, + H: HeaderT, + { + let batch_size = changes.len(); + let average_len = changes.iter().map(|(_, v)| v.len()).sum::() / batch_size; + // For every batched write use a different trie instance and recorder, so we + // don't benefit from past runs. + let (trie, _recorder) = + self.create_trie_backend::(original_root, storage, shared_trie_cache); + + let start = Instant::now(); + // Create a TX that will modify the Trie in the DB and + // calculate the root hash of the Trie after the modification. + let replace = changes + .iter() + .map(|(key, new_v)| (key.as_ref(), Some(new_v.as_ref()))) + .collect::>(); + let stx = match child_info { + Some(info) => trie.child_storage_root(info, replace.iter().cloned(), version).2, + None => trie.storage_root(replace.iter().cloned(), version).1, + }; + // Only the keep the insertions, since we do not want to benchmark pruning. + let tx = convert_tx::(db.clone(), stx.clone(), false, col); + db.commit(tx).map_err(|e| format!("Writing to the Database: {}", e))?; + let result = (average_len, start.elapsed() / batch_size as u32); + + // Now undo the changes by removing what was added. + let tx = convert_tx::(db.clone(), stx.clone(), true, col); + db.commit(tx).map_err(|e| format!("Writing to the Database: {}", e))?; + + Ok(result) + } + + /// Measures write benchmark on block validation + /// if `child_info` exist then it means this is a child tree key + fn measure_per_key_amortised_validate_block_write_cost( + &self, + original_root: Block::Hash, + storage: &Arc>>, + shared_trie_cache: Option<&sp_trie::cache::SharedTrieCache>>, + changes: Vec<(Vec, Vec)>, + maybe_child_info: Option<&ChildInfo>, + ) -> Result<(usize, Duration)> + where + Block: BlockT
+ Debug, + H: HeaderT, + { + let batch_size = changes.len(); + let average_len = changes.iter().map(|(_, v)| v.len()).sum::() / batch_size; + let (trie, recorder) = + self.create_trie_backend::(original_root, storage, shared_trie_cache); + for (key, _) in changes.iter() { + let _v = trie + .storage(key) + .expect("Checked above to exist") + .ok_or("Value unexpectedly empty")?; + } + let storage_proof = recorder + .map(|r| r.drain_storage_proof()) + .expect("Storage proof must exist for block validation"); + let root = trie.root(); + debug!( + "POV: len {:?} {:?}", + storage_proof.len(), + storage_proof.clone().encoded_compact_size::>(*root) + ); + let params = StorageAccessParams::::new_write( + *root, + storage_proof, + (changes, maybe_child_info.cloned()), + ); + + let mut durations_in_nanos = Vec::new(); + let wasm_module = get_wasm_module(); + let mut instance = wasm_module.new_instance().expect("Failed to create wasm instance"); + let dry_run_encoded = params.as_dry_run().encode(); + let encoded = params.encode(); + + for i in 1..=self.params.validate_block_rounds { + info!( + "validate_block with {} keys, round {}/{}", + batch_size, i, self.params.validate_block_rounds + ); + + // Dry run to get the time it takes without storage access + let dry_run_start = Instant::now(); + instance + .call_export("validate_block", &dry_run_encoded) + .expect("Failed to call validate_block"); + let dry_run_elapsed = dry_run_start.elapsed(); + debug!("validate_block dry-run time {:?}", dry_run_elapsed); + + let start = Instant::now(); + instance + .call_export("validate_block", &encoded) + .expect("Failed to call validate_block"); + let elapsed = start.elapsed(); + debug!("validate_block time {:?}", elapsed); + + durations_in_nanos.push( + elapsed.saturating_sub(dry_run_elapsed).as_nanos() as u64 / batch_size as u64, + ); + } + + let result = ( + average_len, + std::time::Duration::from_nanos( + durations_in_nanos.iter().sum::() / durations_in_nanos.len() as u64, + ), + ); + + Ok(result) + } } /// Converts a Trie transaction into a DB transaction. @@ -227,39 +406,6 @@ fn convert_tx( ret } -/// Measures write benchmark -/// if `child_info` exist then it means this is a child tree key -fn measure_per_key_amortised_write_cost( - db: Arc>, - trie: &DbState>, - changes: Vec<(Vec, Vec)>, - version: StateVersion, - col: ColumnId, - child_info: Option<&ChildInfo>, -) -> Result<(usize, Duration)> { - let start = Instant::now(); - // Create a TX that will modify the Trie in the DB and - // calculate the root hash of the Trie after the modification. - let average_len = changes.iter().map(|(_, v)| v.len()).sum::() / changes.len(); - let replace = changes - .iter() - .map(|(key, new_v)| (key.as_ref(), Some(new_v.as_ref()))) - .collect::>(); - let stx = match child_info { - Some(info) => trie.child_storage_root(info, replace.iter().cloned(), version).2, - None => trie.storage_root(replace.iter().cloned(), version).1, - }; - // Only the keep the insertions, since we do not want to benchmark pruning. - let tx = convert_tx::(db.clone(), stx.clone(), false, col); - db.commit(tx).map_err(|e| format!("Writing to the Database: {}", e))?; - let result = (average_len, start.elapsed() / changes.len() as u32); - - // Now undo the changes by removing what was added. - let tx = convert_tx::(db.clone(), stx.clone(), true, col); - db.commit(tx).map_err(|e| format!("Writing to the Database: {}", e))?; - Ok(result) -} - /// Checks if a new value causes any collision in tree updates /// returns true if there is no collision /// if `child_info` exist then it means this is a child tree key diff --git a/substrate/utils/frame/storage-access-test-runtime/Cargo.toml b/substrate/utils/frame/storage-access-test-runtime/Cargo.toml new file mode 100644 index 0000000000000..5093653d2dc83 --- /dev/null +++ b/substrate/utils/frame/storage-access-test-runtime/Cargo.toml @@ -0,0 +1,42 @@ +[package] +name = "frame-storage-access-test-runtime" +description = "A runtime for testing storage access on block validation" +version = "0.1.0" +authors.workspace = true +edition.workspace = true +build = "build.rs" +license = "Apache-2.0" +homepage.workspace = true +repository.workspace = true +publish = true + +[lints] +workspace = true + +[dependencies] +codec = { features = ["derive"], workspace = true } +cumulus-pallet-parachain-system = { workspace = true, optional = true } +sp-core = { workspace = true } +sp-runtime = { workspace = true } +sp-state-machine = { workspace = true } +sp-trie = { workspace = true } + +[build-dependencies] +substrate-wasm-builder = { optional = true, workspace = true, default-features = true } + +[features] +default = ["std"] +no_std = [] +std = [ + "codec/std", + "cumulus-pallet-parachain-system/std", + "sp-core/std", + "sp-runtime/std", + "sp-state-machine/std", + "sp-trie/std", + "substrate-wasm-builder", +] +runtime-benchmarks = [ + "cumulus-pallet-parachain-system/runtime-benchmarks", + "sp-runtime/runtime-benchmarks", +] diff --git a/substrate/utils/frame/storage-access-test-runtime/build.rs b/substrate/utils/frame/storage-access-test-runtime/build.rs new file mode 100644 index 0000000000000..651f57388e0d0 --- /dev/null +++ b/substrate/utils/frame/storage-access-test-runtime/build.rs @@ -0,0 +1,28 @@ +// This file is part of Substrate. + +// Copyright (C) Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +fn main() { + #[cfg(feature = "std")] + { + substrate_wasm_builder::WasmBuilder::new() + .with_current_project() + .export_heap_base() + .import_memory() + .disable_runtime_version_section_check() + .build(); + } +} diff --git a/substrate/utils/frame/storage-access-test-runtime/src/lib.rs b/substrate/utils/frame/storage-access-test-runtime/src/lib.rs new file mode 100644 index 0000000000000..4b1ab69b14268 --- /dev/null +++ b/substrate/utils/frame/storage-access-test-runtime/src/lib.rs @@ -0,0 +1,181 @@ +// This file is part of Substrate. + +// Copyright (C) Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Test runtime to benchmark storage access on block validation + +#![cfg_attr(not(feature = "std"), no_std)] + +extern crate alloc; + +use alloc::vec::Vec; +use codec::{Decode, Encode}; +use sp_core::storage::ChildInfo; +use sp_runtime::traits; +use sp_trie::StorageProof; + +#[cfg(all(not(feature = "std"), feature = "runtime-benchmarks"))] +use { + cumulus_pallet_parachain_system::validate_block::{ + trie_cache::CacheProvider, trie_recorder::SizeOnlyRecorderProvider, + }, + sp_core::storage::StateVersion, + sp_runtime::{generic, OpaqueExtrinsic}, + sp_state_machine::{Backend, TrieBackendBuilder}, +}; + +// Include the WASM binary +#[cfg(feature = "std")] +include!(concat!(env!("OUT_DIR"), "/wasm_binary.rs")); + +/// Parameters for benchmarking storage access on block validation. +/// +/// On dry-run, the storage access is not performed to measure the cost of the runtime call. +#[derive(Decode, Clone)] +#[cfg_attr(feature = "std", derive(Encode))] +pub struct StorageAccessParams { + pub state_root: B::Hash, + pub storage_proof: StorageProof, + pub payload: StorageAccessPayload, + /// On dry-run, we don't read/write to the storage. + pub is_dry_run: bool, +} + +/// Payload for benchmarking read and write operations on block validation. +#[derive(Debug, Clone, Decode, Encode)] +pub enum StorageAccessPayload { + // Storage keys with optional child info. + Read(Vec<(Vec, Option)>), + // Storage key-value pairs with optional child info. + Write((Vec<(Vec, Vec)>, Option)), +} + +impl StorageAccessParams { + /// Create a new params for reading from the storage. + pub fn new_read( + state_root: B::Hash, + storage_proof: StorageProof, + payload: Vec<(Vec, Option)>, + ) -> Self { + Self { + state_root, + storage_proof, + payload: StorageAccessPayload::Read(payload), + is_dry_run: false, + } + } + + /// Create a new params for writing to the storage. + pub fn new_write( + state_root: B::Hash, + storage_proof: StorageProof, + payload: (Vec<(Vec, Vec)>, Option), + ) -> Self { + Self { + state_root, + storage_proof, + payload: StorageAccessPayload::Write(payload), + is_dry_run: false, + } + } + + /// Create a dry-run version of the params. + pub fn as_dry_run(&self) -> Self { + Self { + state_root: self.state_root, + storage_proof: self.storage_proof.clone(), + payload: self.payload.clone(), + is_dry_run: true, + } + } +} + +/// Imitates `cumulus_pallet_parachain_system::validate_block::implementation::validate_block` +/// +/// Only performs the storage access, this is used to benchmark the storage access cost. +#[doc(hidden)] +#[cfg(all(not(feature = "std"), feature = "runtime-benchmarks"))] +pub fn proceed_storage_access(mut params: &[u8]) { + let StorageAccessParams { state_root, storage_proof, payload, is_dry_run } = + StorageAccessParams::::decode(&mut params) + .expect("Invalid arguments to `validate_block`."); + + let db = storage_proof.into_memory_db(); + let recorder = SizeOnlyRecorderProvider::>::default(); + let cache_provider = CacheProvider::new(); + let backend = TrieBackendBuilder::new_with_cache(db, state_root, cache_provider) + .with_recorder(recorder) + .build(); + + if is_dry_run { + return; + } + + match payload { + StorageAccessPayload::Read(keys) => + for (key, maybe_child_info) in keys { + match maybe_child_info { + Some(child_info) => { + let _ = backend + .child_storage(&child_info, key.as_ref()) + .expect("Key not found") + .ok_or("Value unexpectedly empty"); + }, + None => { + let _ = backend + .storage(key.as_ref()) + .expect("Key not found") + .ok_or("Value unexpectedly empty"); + }, + } + }, + StorageAccessPayload::Write((changes, maybe_child_info)) => { + let delta = changes.iter().map(|(key, value)| (key.as_ref(), Some(value.as_ref()))); + match maybe_child_info { + Some(child_info) => { + backend.child_storage_root(&child_info, delta, StateVersion::V1); + }, + None => { + backend.storage_root(delta, StateVersion::V1); + }, + } + }, + } +} + +/// Wasm binary unwrapped. If built with `SKIP_WASM_BUILD`, the function panics. +#[cfg(feature = "std")] +pub fn wasm_binary_unwrap() -> &'static [u8] { + WASM_BINARY.expect( + "Development wasm binary is not available. Unset SKIP_WASM_BUILD and compile the runtime again.", + ) +} + +#[cfg(enable_alloc_error_handler)] +#[alloc_error_handler] +#[no_mangle] +pub fn oom(_: core::alloc::Layout) -> ! { + core::intrinsics::abort(); +} + +#[cfg(all(not(feature = "std"), feature = "runtime-benchmarks"))] +#[no_mangle] +pub extern "C" fn validate_block(params: *const u8, len: usize) -> u64 { + type Block = generic::Block, OpaqueExtrinsic>; + let params = unsafe { alloc::slice::from_raw_parts(params, len) }; + proceed_storage_access::(params); + 1 +}