From 54658bce7ba051133f0bbd3d22f0dfa1987ac9c9 Mon Sep 17 00:00:00 2001 From: George Mitenkov Date: Thu, 29 Aug 2024 15:31:22 +0100 Subject: [PATCH] [aptos-vm] Handle runtime environment nicely --- Cargo.lock | 5 ----- aptos-move/aptos-vm-types/Cargo.toml | 1 - aptos-move/aptos-vm/src/aptos_vm.rs | 7 ++++++- .../aptos-vm/src/block_executor/vm_wrapper.rs | 7 +++++++ aptos-move/aptos-vm/src/move_vm_ext/vm.rs | 4 ++-- aptos-move/block-executor/Cargo.toml | 1 - aptos-move/block-executor/src/executor.rs | 17 ++++++++--------- .../block-executor/src/proptest_types/types.rs | 18 +++++++++++++++--- aptos-move/block-executor/src/task.rs | 3 ++- .../src/tests/code_publishing.rs | 2 +- aptos-move/mvhashmap/Cargo.toml | 2 -- aptos-move/vm-genesis/Cargo.toml | 1 - third_party/move/move-vm/runtime/src/lib.rs | 2 +- .../move-vm/runtime/src/storage/environment.rs | 2 +- .../implementations/unsync_code_storage.rs | 8 ++++---- .../implementations/unsync_module_storage.rs | 4 ++-- 16 files changed, 49 insertions(+), 35 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6fcebe200065f..170e1b6f7b8ba 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -669,7 +669,6 @@ dependencies = [ "anyhow", "aptos-aggregator", "aptos-drop-helper", - "aptos-framework", "aptos-infallible", "aptos-logger", "aptos-metrics-core", @@ -2828,9 +2827,7 @@ dependencies = [ "derivative", "move-binary-format", "move-core-types", - "move-vm-runtime", "move-vm-types", - "parking_lot 0.12.1", "proptest", "proptest-derive", "rayon", @@ -4399,7 +4396,6 @@ dependencies = [ "aptos-vm-types", "bcs 0.1.4", "bytes", - "claims", "move-core-types", "move-vm-runtime", "move-vm-types", @@ -4460,7 +4456,6 @@ dependencies = [ "aptos-gas-algebra", "aptos-gas-schedule", "aptos-language-e2e-tests", - "aptos-native-interface", "aptos-types", "aptos-vm", "bcs 0.1.4", diff --git a/aptos-move/aptos-vm-types/Cargo.toml b/aptos-move/aptos-vm-types/Cargo.toml index f4803fd3f9053..d2188639c5e3a 100644 --- a/aptos-move/aptos-vm-types/Cargo.toml +++ b/aptos-move/aptos-vm-types/Cargo.toml @@ -18,7 +18,6 @@ anyhow = { workspace = true } aptos-aggregator = { workspace = true } aptos-gas-algebra = { workspace = true } aptos-gas-schedule = { workspace = true } -aptos-native-interface = { workspace = true } aptos-types = { workspace = true } bcs = { workspace = true } bytes = { workspace = true } diff --git a/aptos-move/aptos-vm/src/aptos_vm.rs b/aptos-move/aptos-vm/src/aptos_vm.rs index b8a2a2b1108c6..312752c902e7d 100644 --- a/aptos-move/aptos-vm/src/aptos_vm.rs +++ b/aptos-move/aptos-vm/src/aptos_vm.rs @@ -112,7 +112,7 @@ use move_vm_runtime::{ logging::expect_no_verification_errors, module_linker_error, module_traversal::{TraversalContext, TraversalStorage}, - TemporaryModuleStorage, UnreachableCodeStorage, + RuntimeEnvironment, TemporaryModuleStorage, UnreachableCodeStorage, }; use move_vm_types::gas::{GasMeter, UnmeteredGasMeter}; use num_cpus; @@ -312,6 +312,11 @@ impl AptosVM { self.move_vm.env.chain_id() } + #[inline(always)] + pub fn runtime_environment(&self) -> &RuntimeEnvironment { + self.move_vm.runtime_environment() + } + /// Sets execution concurrency level when invoked the first time. pub fn set_concurrency_level_once(mut concurrency_level: usize) { concurrency_level = min(concurrency_level, num_cpus::get()); diff --git a/aptos-move/aptos-vm/src/block_executor/vm_wrapper.rs b/aptos-move/aptos-vm/src/block_executor/vm_wrapper.rs index 3b1c2b99249b1..ffed1c86079f1 100644 --- a/aptos-move/aptos-vm/src/block_executor/vm_wrapper.rs +++ b/aptos-move/aptos-vm/src/block_executor/vm_wrapper.rs @@ -20,6 +20,7 @@ use aptos_vm_types::{ }; use fail::fail_point; use move_core_types::vm_status::{StatusCode, VMStatus}; +use move_vm_runtime::{RuntimeEnvironment, WithRuntimeEnvironment}; use std::sync::Arc; pub(crate) struct AptosExecutorTask { @@ -27,6 +28,12 @@ pub(crate) struct AptosExecutorTask { id: StateViewId, } +impl WithRuntimeEnvironment for AptosExecutorTask { + fn runtime_environment(&self) -> &RuntimeEnvironment { + self.vm.runtime_environment() + } +} + impl ExecutorTask for AptosExecutorTask { type Environment = Arc; type Error = VMStatus; diff --git a/aptos-move/aptos-vm/src/move_vm_ext/vm.rs b/aptos-move/aptos-vm/src/move_vm_ext/vm.rs index 904238a818f38..a4b5e4dc51671 100644 --- a/aptos-move/aptos-vm/src/move_vm_ext/vm.rs +++ b/aptos-move/aptos-vm/src/move_vm_ext/vm.rs @@ -154,14 +154,14 @@ impl MoveVmExt { .features() .is_enabled(FeatureFlag::DISALLOW_USER_NATIVES); - // TODO(loader_v2): Instead, propagate environment down to Block-STM. + // TODO(loader_v2): Refresh environment when configs change. set_runtime_environment( vm_config.clone(), aptos_natives_with_builder(&mut builder, inject_create_signer_for_gov_sim), ); let vm = if env.features().is_loader_v2_enabled() { // TODO(loader_v2): For now re-create the VM every time. Later we can have a - // single VM created once. + // single VM created once, which also holds the environment. MoveVM::new_with_runtime_environment(fetch_runtime_environment()) } else { WarmVmCache::get_warm_vm( diff --git a/aptos-move/block-executor/Cargo.toml b/aptos-move/block-executor/Cargo.toml index fda7111ea6089..eba80e5c57b70 100644 --- a/aptos-move/block-executor/Cargo.toml +++ b/aptos-move/block-executor/Cargo.toml @@ -20,7 +20,6 @@ aptos-infallible = { workspace = true } aptos-logger = { workspace = true } aptos-metrics-core = { workspace = true } aptos-mvhashmap = { workspace = true } -aptos-framework = { workspace = true } aptos-types = { workspace = true } aptos-vm-logging = { workspace = true } aptos-vm-types = { workspace = true } diff --git a/aptos-move/block-executor/src/executor.rs b/aptos-move/block-executor/src/executor.rs index 5ab2f5aa69729..db64496423fe1 100644 --- a/aptos-move/block-executor/src/executor.rs +++ b/aptos-move/block-executor/src/executor.rs @@ -45,9 +45,7 @@ use aptos_types::{ write_set::{TransactionWrite, WriteOp}, }; use aptos_vm_logging::{alert, clear_speculative_txn_logs, init_speculative_logs, prelude::*}; -use aptos_vm_types::{ - change_set::randomly_check_layout_matches, environment::fetch_runtime_environment, -}; +use aptos_vm_types::change_set::randomly_check_layout_matches; use bytes::Bytes; use claims::assert_none; use core::panic; @@ -826,7 +824,6 @@ where scheduler: &Scheduler, // TODO: should not need to pass base view. base_view: &S, - runtime_environment: &RuntimeEnvironment, start_shared_counter: u32, shared_counter: &AtomicU32, shared_commit_state: &ExplicitSyncWrapper>, @@ -838,6 +835,9 @@ where let executor = E::init(env.clone(), base_view); drop(init_timer); + // Shared environment used by each executor. + let runtime_environment = executor.runtime_environment(); + let _timer = WORK_WITH_TASK_SECONDS.start_timer(); let mut scheduler_task = SchedulerTask::Retry; @@ -987,7 +987,6 @@ where let scheduler = Scheduler::new(num_txns); let timer = RAYON_EXECUTION_SECONDS.start_timer(); - let runtime_environment = fetch_runtime_environment(); self.executor_thread_pool.scope(|s| { for _ in 0..num_workers { s.spawn(|_| { @@ -998,7 +997,6 @@ where &versioned_cache, &scheduler, base_view, - &runtime_environment, start_shared_counter, &shared_counter, &shared_commit_state, @@ -1135,6 +1133,8 @@ where let executor = E::init(env, base_view); drop(init_timer); + let runtime_environment = executor.runtime_environment(); + let start_counter = gen_id_start_value(true); let counter = RefCell::new(start_counter); let unsync_map = UnsyncMap::new(); @@ -1147,11 +1147,10 @@ where let last_input_output: TxnLastInputOutput = TxnLastInputOutput::new(num_txns as TxnIndex); - let runtime_environment = fetch_runtime_environment(); for (idx, txn) in signature_verified_block.iter().enumerate() { let latest_view = LatestView::::new( base_view, - runtime_environment.as_ref(), + runtime_environment, ViewState::Unsync(SequentialState::new(&unsync_map, start_counter, &counter)), idx as TxnIndex, ); @@ -1321,7 +1320,7 @@ where // Apply the writes. let resource_write_set = output.resource_write_set(); Self::apply_output_sequential( - runtime_environment.as_ref(), + runtime_environment, &unsync_map, &output, resource_write_set.clone(), diff --git a/aptos-move/block-executor/src/proptest_types/types.rs b/aptos-move/block-executor/src/proptest_types/types.rs index 3300fcfe0ec74..55f55de44f619 100644 --- a/aptos-move/block-executor/src/proptest_types/types.rs +++ b/aptos-move/block-executor/src/proptest_types/types.rs @@ -33,6 +33,7 @@ use aptos_vm_types::{ use bytes::Bytes; use claims::{assert_ge, assert_le, assert_ok}; use move_core_types::{identifier::IdentStr, value::MoveTypeLayout}; +use move_vm_runtime::{RuntimeEnvironment, WithRuntimeEnvironment}; use move_vm_types::delayed_values::delayed_field_id::DelayedFieldID; use once_cell::sync::OnceCell; use proptest::{arbitrary::Arbitrary, collection::vec, prelude::*, proptest, sample::Index}; @@ -843,12 +844,23 @@ impl> + Arbitrary + Clone + Debug + Eq + Sync + Send> Transactio // Mock transaction executor implementation. /////////////////////////////////////////////////////////////////////////// -#[derive(Default)] -pub(crate) struct MockTask(PhantomData<(K, E)>); +pub(crate) struct MockTask { + runtime_environment: RuntimeEnvironment, + phantom_data: PhantomData<(K, E)>, +} impl MockTask { pub fn new() -> Self { - Self(PhantomData) + Self { + runtime_environment: RuntimeEnvironment::test(), + phantom_data: PhantomData, + } + } +} + +impl WithRuntimeEnvironment for MockTask { + fn runtime_environment(&self) -> &RuntimeEnvironment { + &self.runtime_environment } } diff --git a/aptos-move/block-executor/src/task.rs b/aptos-move/block-executor/src/task.rs index f548c4173ef52..760ec29220b5b 100644 --- a/aptos-move/block-executor/src/task.rs +++ b/aptos-move/block-executor/src/task.rs @@ -19,6 +19,7 @@ use aptos_vm_types::{ resolver::{TExecutorView, TResourceGroupView}, }; use move_core_types::{value::MoveTypeLayout, vm_status::StatusCode}; +use move_vm_runtime::WithRuntimeEnvironment; use std::{ collections::{BTreeMap, HashSet}, fmt::Debug, @@ -52,7 +53,7 @@ pub struct Accesses { /// Trait for single threaded transaction executor. // TODO: Sync should not be required. Sync is only introduced because this trait occurs as a phantom type of executor struct. -pub trait ExecutorTask: Sync { +pub trait ExecutorTask: Sync + WithRuntimeEnvironment { /// Type of transaction and its associated key and value. type Txn: Transaction; diff --git a/aptos-move/e2e-move-tests/src/tests/code_publishing.rs b/aptos-move/e2e-move-tests/src/tests/code_publishing.rs index 495d0437273ee..e0f4b80f08abe 100644 --- a/aptos-move/e2e-move-tests/src/tests/code_publishing.rs +++ b/aptos-move/e2e-move-tests/src/tests/code_publishing.rs @@ -11,7 +11,7 @@ use aptos_types::{ account_address::{create_resource_address, AccountAddress}, move_utils::MemberId, on_chain_config::{FeatureFlag, Features, OnChainConfig}, - transaction::{AbortInfo, ExecutionStatus, TransactionPayload}, + transaction::{ExecutionStatus, TransactionPayload}, }; use claims::assert_ok; use move_core_types::{ diff --git a/aptos-move/mvhashmap/Cargo.toml b/aptos-move/mvhashmap/Cargo.toml index f324cc171e41f..0ac539ed1b839 100644 --- a/aptos-move/mvhashmap/Cargo.toml +++ b/aptos-move/mvhashmap/Cargo.toml @@ -26,8 +26,6 @@ derivative = { workspace = true } move-binary-format = { workspace = true } move-core-types = { workspace = true } move-vm-types = { workspace = true } -move-vm-runtime = { workspace = true } -parking_lot = { workspace = true } serde = { workspace = true } [dev-dependencies] diff --git a/aptos-move/vm-genesis/Cargo.toml b/aptos-move/vm-genesis/Cargo.toml index 31b89a92a7565..b8d7e7efb3a11 100644 --- a/aptos-move/vm-genesis/Cargo.toml +++ b/aptos-move/vm-genesis/Cargo.toml @@ -22,7 +22,6 @@ aptos-vm = { workspace = true } aptos-vm-types = { workspace = true } bcs = { workspace = true } bytes = { workspace = true } -claims = { workspace = true } move-core-types = { workspace = true } move-vm-runtime = { workspace = true } move-vm-types = { workspace = true } diff --git a/third_party/move/move-vm/runtime/src/lib.rs b/third_party/move/move-vm/runtime/src/lib.rs index c7725138afc56..e652c36a26428 100644 --- a/third_party/move/move-vm/runtime/src/lib.rs +++ b/third_party/move/move-vm/runtime/src/lib.rs @@ -36,7 +36,7 @@ pub use loader::{LoadedFunction, Module, Script}; pub use storage::{ code_storage::{ambassador_impl_CodeStorage, script_hash, CodeStorage}, dummy::use_loader_v2_based_on_env, - environment::RuntimeEnvironment, + environment::{RuntimeEnvironment, WithRuntimeEnvironment}, implementations::{ unreachable_code_storage::UnreachableCodeStorage, unsync_code_storage::{IntoUnsyncCodeStorage, UnsyncCodeStorage}, diff --git a/third_party/move/move-vm/runtime/src/storage/environment.rs b/third_party/move/move-vm/runtime/src/storage/environment.rs index 07c94dac69cbd..37ff7287e9e08 100644 --- a/third_party/move/move-vm/runtime/src/storage/environment.rs +++ b/third_party/move/move-vm/runtime/src/storage/environment.rs @@ -180,6 +180,6 @@ impl RuntimeEnvironment { } /// Represents any type that contains a [RuntimeEnvironment]. -pub trait WithEnvironment { +pub trait WithRuntimeEnvironment { fn runtime_environment(&self) -> &RuntimeEnvironment; } diff --git a/third_party/move/move-vm/runtime/src/storage/implementations/unsync_code_storage.rs b/third_party/move/move-vm/runtime/src/storage/implementations/unsync_code_storage.rs index 6a442048fa995..ffc80d8fecec8 100644 --- a/third_party/move/move-vm/runtime/src/storage/implementations/unsync_code_storage.rs +++ b/third_party/move/move-vm/runtime/src/storage/implementations/unsync_code_storage.rs @@ -4,7 +4,7 @@ use crate::{ script_hash, storage::{ - environment::WithEnvironment, + environment::WithRuntimeEnvironment, implementations::unsync_module_storage::IntoUnsyncModuleStorage, module_storage::{ambassador_impl_ModuleStorage, ModuleBytesStorage}, }, @@ -61,7 +61,7 @@ impl<'e, B: ModuleBytesStorage> IntoUnsyncCodeStorage<'e, B> for B { } } -impl UnsyncCodeStorage { +impl UnsyncCodeStorage { /// Create a new storage with no scripts. There are no constrains on which /// modules exist in module storage. fn new(module_storage: M) -> Self { @@ -123,7 +123,7 @@ impl UnsyncCodeStorage { } } -impl CodeStorage for UnsyncCodeStorage { +impl CodeStorage for UnsyncCodeStorage { fn deserialize_and_cache_script( &self, serialized_script: &[u8], @@ -174,7 +174,7 @@ impl CodeStorage for UnsyncCodeStorage { } #[cfg(test)] -impl UnsyncCodeStorage { +impl UnsyncCodeStorage { fn matches bool>( &self, script_hashes: impl IntoIterator, diff --git a/third_party/move/move-vm/runtime/src/storage/implementations/unsync_module_storage.rs b/third_party/move/move-vm/runtime/src/storage/implementations/unsync_module_storage.rs index e1792ff188f6e..5d80600829ca8 100644 --- a/third_party/move/move-vm/runtime/src/storage/implementations/unsync_module_storage.rs +++ b/third_party/move/move-vm/runtime/src/storage/implementations/unsync_module_storage.rs @@ -4,7 +4,7 @@ use crate::{ module_cyclic_dependency_error, module_linker_error, storage::{ - environment::{RuntimeEnvironment, WithEnvironment}, + environment::{RuntimeEnvironment, WithRuntimeEnvironment}, module_storage::{ModuleBytesStorage, ModuleStorage}, }, Module, @@ -281,7 +281,7 @@ impl<'e, B: ModuleBytesStorage> UnsyncModuleStorage<'e, B> { } } -impl<'e, B: ModuleBytesStorage> WithEnvironment for UnsyncModuleStorage<'e, B> { +impl<'e, B: ModuleBytesStorage> WithRuntimeEnvironment for UnsyncModuleStorage<'e, B> { fn runtime_environment(&self) -> &RuntimeEnvironment { self.runtime_environment }