From 6bdeb0215fac545c03d6fcc908df0daaadb588b1 Mon Sep 17 00:00:00 2001 From: George Mitenkov Date: Fri, 30 Aug 2024 10:09:30 +0100 Subject: [PATCH] [block-stm] Integrate script caches --- Cargo.lock | 1 + aptos-move/aptos-vm/src/aptos_vm.rs | 2 +- aptos-move/block-executor/src/executor.rs | 25 ++-- .../block-executor/src/modules_and_scripts.rs | 108 +++++++++++++++--- aptos-move/mvhashmap/Cargo.toml | 1 + aptos-move/mvhashmap/src/lib.rs | 14 ++- aptos-move/mvhashmap/src/scripts.rs | 32 ++++++ aptos-move/mvhashmap/src/unsync_map.rs | 46 +++++++- .../mvhashmap/src/versioned_code_storage.rs | 105 +++++++++++++++++ .../mvhashmap/src/versioned_module_storage.rs | 7 +- .../move/move-vm/runtime/src/data_cache.rs | 14 +-- third_party/move/move-vm/runtime/src/lib.rs | 2 +- .../runtime/src/storage/code_storage.rs | 19 ++- .../runtime/src/storage/environment.rs | 10 +- .../implementations/unsync_code_storage.rs | 21 +--- 15 files changed, 339 insertions(+), 68 deletions(-) create mode 100644 aptos-move/mvhashmap/src/scripts.rs create mode 100644 aptos-move/mvhashmap/src/versioned_code_storage.rs diff --git a/Cargo.lock b/Cargo.lock index 8115c32a89f390..321a7029ee5d99 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2823,6 +2823,7 @@ dependencies = [ "derivative", "move-binary-format", "move-core-types", + "move-vm-runtime", "move-vm-types", "proptest", "proptest-derive", diff --git a/aptos-move/aptos-vm/src/aptos_vm.rs b/aptos-move/aptos-vm/src/aptos_vm.rs index 312752c902e7d7..4b2edcf7237844 100644 --- a/aptos-move/aptos-vm/src/aptos_vm.rs +++ b/aptos-move/aptos-vm/src/aptos_vm.rs @@ -3122,7 +3122,7 @@ pub(crate) fn is_account_init_for_sponsored_transaction( None, ) .map_err(|e| e.finish(Location::Undefined))?; - return Ok(maybe_bytes.is_some()); + return Ok(maybe_bytes.is_none()); } Ok(false) } diff --git a/aptos-move/block-executor/src/executor.rs b/aptos-move/block-executor/src/executor.rs index db64496423fe1d..9e42a1d60f937c 100644 --- a/aptos-move/block-executor/src/executor.rs +++ b/aptos-move/block-executor/src/executor.rs @@ -195,6 +195,7 @@ where // TODO(loader_v2): We do not need to clone out all module writes in V2 design // since we do not store them anymore. versioned_cache + .code_storage() .module_storage() .write_pending(k, idx_to_execute) } else { @@ -300,7 +301,10 @@ where Resource => versioned_cache.data().remove(&k, idx_to_execute), Module => { if runtime_environment.vm_config().use_loader_v2 { - versioned_cache.module_storage().remove(&k, idx_to_execute); + versioned_cache + .code_storage() + .module_storage() + .remove(&k, idx_to_execute); } else { versioned_cache.modules().remove(&k, idx_to_execute); } @@ -371,7 +375,10 @@ where let is_valid = read_set.validate_data_reads(versioned_cache.data(), idx_to_validate) && read_set.validate_group_reads(versioned_cache.group_data(), idx_to_validate) - && read_set.validate_module_reads(versioned_cache.module_storage(), idx_to_validate); + && read_set.validate_module_reads( + versioned_cache.code_storage().module_storage(), + idx_to_validate, + ); Ok(is_valid) } @@ -561,13 +568,11 @@ where if runtime_environment.vm_config().use_loader_v2 { if let Some(module_write_set) = last_input_output.module_write_set(txn_idx) { - for (key, v) in module_write_set.into_iter() { - let entry = - ModuleStorageEntry::from_transaction_write(runtime_environment, v)?; - versioned_cache - .module_storage() - .write_published(&key, txn_idx, entry); - } + versioned_cache.code_storage().write_published_modules( + txn_idx, + runtime_environment, + module_write_set.into_iter(), + )?; scheduler.finish_module_publishing_after_commit(txn_idx, executed_at_commit); } } @@ -1065,7 +1070,7 @@ where if runtime_environment.vm_config().use_loader_v2 { let entry = ModuleStorageEntry::from_transaction_write(runtime_environment, write_op)?; - unsync_map.write_module_storage_entry(key, entry); + unsync_map.publish_module_storage_entry(key, entry); } else { unsync_map.write_module(key, write_op); } diff --git a/aptos-move/block-executor/src/modules_and_scripts.rs b/aptos-move/block-executor/src/modules_and_scripts.rs index 77ae4271cd03c9..725f8dfe30f7bb 100644 --- a/aptos-move/block-executor/src/modules_and_scripts.rs +++ b/aptos-move/block-executor/src/modules_and_scripts.rs @@ -20,7 +20,8 @@ use move_binary_format::{ }; use move_core_types::{account_address::AccountAddress, identifier::IdentStr, metadata::Metadata}; use move_vm_runtime::{ - module_cyclic_dependency_error, module_linker_error, CodeStorage, Module, ModuleStorage, Script, + deserialize_script, module_cyclic_dependency_error, module_linker_error, script_hash, + CodeStorage, Module, ModuleStorage, Script, }; use std::{collections::HashSet, sync::Arc}; @@ -63,15 +64,79 @@ impl<'a, T: Transaction, S: TStateView, X: Executable> CodeStorage { fn deserialize_and_cache_script( &self, - _serialized_script: &[u8], + serialized_script: &[u8], ) -> VMResult> { - // TODO(loader_v2): Support scripts. - todo!() + let hash = script_hash(serialized_script); + + let maybe_compiled_script = match &self.latest_view { + ViewState::Sync(state) => state + .versioned_map + .code_storage() + .get_deserialized_script(&hash), + ViewState::Unsync(state) => state.unsync_map.get_deserialized_script(&hash), + }; + + Ok(match maybe_compiled_script { + Some(compiled_script) => compiled_script, + None => { + let compiled_script = self.deserialize_script(serialized_script)?; + + match &self.latest_view { + ViewState::Sync(state) => state + .versioned_map + .code_storage() + .cache_deserialized_script(hash, compiled_script.clone()), + ViewState::Unsync(state) => state + .unsync_map + .cache_deserialized_script(hash, compiled_script.clone()), + } + compiled_script + }, + }) } - fn verify_and_cache_script(&self, _serialized_script: &[u8]) -> VMResult> { - // TODO(loader_v2): Support scripts. - todo!() + fn verify_and_cache_script(&self, serialized_script: &[u8]) -> VMResult> { + let hash = script_hash(serialized_script); + + let maybe_verified_script = match &self.latest_view { + ViewState::Sync(state) => state + .versioned_map + .code_storage() + .get_verified_script(&hash), + ViewState::Unsync(state) => state.unsync_map.get_verified_script(&hash), + }; + + let compiled_script = match maybe_verified_script { + Some(Ok(script)) => return Ok(script), + Some(Err(compiled_script)) => compiled_script, + None => self.deserialize_and_cache_script(serialized_script)?, + }; + + // Locally verify the script. + let partially_verified_script = self + .runtime_environment + .build_partially_verified_script(compiled_script)?; + + // Verify the script by also looking at its dependencies. + let immediate_dependencies = partially_verified_script + .immediate_dependencies_iter() + .map(|(addr, name)| self.fetch_verified_module(addr, name)) + .collect::>>()?; + let script = self + .runtime_environment + .build_verified_script(partially_verified_script, &immediate_dependencies)?; + let script = Arc::new(script); + + match &self.latest_view { + ViewState::Sync(state) => state + .versioned_map + .code_storage() + .cache_verified_script(hash, script.clone()), + ViewState::Unsync(state) => { + state.unsync_map.cache_verified_script(hash, script.clone()) + }, + } + Ok(script) } } @@ -166,11 +231,13 @@ impl<'a, T: Transaction, S: TStateView, X: Executable> LatestView< match &self.latest_view { ViewState::Sync(state) => { - let result = state.versioned_map.module_storage().get_or_else( - key, - ShiftedTxnIndex::new(self.txn_idx), - || self.get_base_module_storage_entry(key), - )?; + let result = state + .versioned_map + .code_storage() + .module_storage() + .get_or_else(key, ShiftedTxnIndex::new(self.txn_idx), || { + self.get_base_module_storage_entry(key) + })?; state .captured_reads .borrow_mut() @@ -283,11 +350,11 @@ impl<'a, T: Transaction, S: TStateView, X: Executable> LatestView< let key = T::Key::from_address_and_module_name(address, module_name); match &self.latest_view { ViewState::Sync(state) => { - state.versioned_map.module_storage().write_if_not_verified( - &key, - txn_idx, - verified_entry, - ); + state + .versioned_map + .code_storage() + .module_storage() + .write_if_not_verified(&key, txn_idx, verified_entry); }, ViewState::Unsync(state) => { state @@ -297,4 +364,11 @@ impl<'a, T: Transaction, S: TStateView, X: Executable> LatestView< } Ok(module) } + + /// Returns the deserialized script based on the current runtime environment. + fn deserialize_script(&self, serialized_script: &[u8]) -> VMResult> { + let deserializer_config = &self.runtime_environment.vm_config().deserializer_config; + let compiled_script = deserialize_script(serialized_script, deserializer_config)?; + Ok(Arc::new(compiled_script)) + } } diff --git a/aptos-move/mvhashmap/Cargo.toml b/aptos-move/mvhashmap/Cargo.toml index 0ac539ed1b8394..062356c93962b4 100644 --- a/aptos-move/mvhashmap/Cargo.toml +++ b/aptos-move/mvhashmap/Cargo.toml @@ -26,6 +26,7 @@ derivative = { workspace = true } move-binary-format = { workspace = true } move-core-types = { workspace = true } move-vm-types = { workspace = true } +move-vm-runtime = { workspace = true } serde = { workspace = true } [dev-dependencies] diff --git a/aptos-move/mvhashmap/src/lib.rs b/aptos-move/mvhashmap/src/lib.rs index caa0365aa0773e..986508ea5cdd4e 100644 --- a/aptos-move/mvhashmap/src/lib.rs +++ b/aptos-move/mvhashmap/src/lib.rs @@ -3,8 +3,8 @@ // SPDX-License-Identifier: Apache-2.0 use crate::{ - versioned_data::VersionedData, versioned_delayed_fields::VersionedDelayedFields, - versioned_group_data::VersionedGroupData, versioned_module_storage::VersionedModuleStorage, + versioned_code_storage::VersionedCodeStorage, versioned_data::VersionedData, + versioned_delayed_fields::VersionedDelayedFields, versioned_group_data::VersionedGroupData, versioned_modules::VersionedModules, }; use aptos_types::{ @@ -16,12 +16,14 @@ use std::{fmt::Debug, hash::Hash}; pub mod types; pub mod unsync_map; +pub mod versioned_code_storage; pub mod versioned_data; pub mod versioned_delayed_fields; pub mod versioned_group_data; pub mod versioned_module_storage; pub mod versioned_modules; +mod scripts; #[cfg(test)] mod unit_tests; @@ -39,7 +41,7 @@ pub struct MVHashMap { group_data: VersionedGroupData, delayed_fields: VersionedDelayedFields, modules: VersionedModules, - module_storage: VersionedModuleStorage, + code_storage: VersionedCodeStorage, } impl< @@ -60,7 +62,7 @@ impl< delayed_fields: VersionedDelayedFields::new(), modules: VersionedModules::new(), - module_storage: VersionedModuleStorage::empty(), + code_storage: VersionedCodeStorage::empty(), } } @@ -94,8 +96,8 @@ impl< &self.modules } - pub fn module_storage(&self) -> &VersionedModuleStorage { - &self.module_storage + pub fn code_storage(&self) -> &VersionedCodeStorage { + &self.code_storage } } diff --git a/aptos-move/mvhashmap/src/scripts.rs b/aptos-move/mvhashmap/src/scripts.rs new file mode 100644 index 00000000000000..7897e14567b503 --- /dev/null +++ b/aptos-move/mvhashmap/src/scripts.rs @@ -0,0 +1,32 @@ +// Copyright © Aptos Foundation +// SPDX-License-Identifier: Apache-2.0 + +use move_binary_format::file_format::CompiledScript; +use move_vm_runtime::Script; +use std::sync::Arc; + +/// An entry for the script cache, used by the Aptos code cache. Entries +/// can live in the cache in different states (deserialized / verified). +#[derive(Debug)] +pub(crate) enum ScriptCacheEntry { + Deserialized(Arc), + Verified(Arc