From d45df1f74f8c5d226f297dfb922f5dc90eb04019 Mon Sep 17 00:00:00 2001 From: George Mitenkov Date: Tue, 12 Nov 2024 07:32:25 +0000 Subject: [PATCH] Move ready checks to cache manager, better encapsulation by passing hash values throughout --- Cargo.lock | 1 + .../aptos-debugger/src/aptos_debugger.rs | 2 + .../src/data_collection.rs | 2 +- .../src/transaction_bench_state.rs | 4 + .../src/aptos_test_harness.rs | 14 +- .../src/bins/run_aptos_p2p.rs | 3 +- aptos-move/aptos-vm/src/aptos_vm.rs | 12 +- aptos-move/aptos-vm/src/block_executor/mod.rs | 62 +++--- aptos-move/aptos-vm/src/lib.rs | 21 +- .../sharded_executor_service.rs | 11 +- .../aptos-vm/tests/sharded_block_executor.rs | 24 +-- aptos-move/block-executor/Cargo.toml | 1 + .../src/code_cache_global_manager.rs | 202 +++++++++++++++--- aptos-move/e2e-tests/src/executor.rs | 10 +- .../executor-benchmark/src/native_executor.rs | 3 + execution/executor-service/src/test_utils.rs | 16 +- execution/executor/src/block_executor/mod.rs | 127 +++++------ execution/executor/src/chunk_executor/mod.rs | 9 +- .../src/chunk_executor/transaction_chunk.rs | 9 +- execution/executor/src/db_bootstrapper/mod.rs | 8 +- execution/executor/src/fuzzing.rs | 2 + .../src/tests/mock_vm/mock_vm_test.rs | 6 + execution/executor/src/tests/mock_vm/mod.rs | 4 +- execution/executor/src/tests/mod.rs | 1 + .../src/workflow/do_get_execution_output.rs | 45 +++- .../execution/ptx-executor/Cargo.toml | 1 + .../execution/ptx-executor/src/lib.rs | 3 + storage/db-tool/src/replay_on_archive.rs | 2 + .../runtime/src/storage/environment.rs | 14 +- 29 files changed, 375 insertions(+), 244 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a61a62bab4508..fb51fb9c3bf92 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1646,6 +1646,7 @@ dependencies = [ name = "aptos-experimental-ptx-executor" version = "0.1.0" dependencies = [ + "aptos-crypto", "aptos-experimental-runtimes", "aptos-infallible", "aptos-logger", diff --git a/aptos-move/aptos-debugger/src/aptos_debugger.rs b/aptos-move/aptos-debugger/src/aptos_debugger.rs index 6a57d69b33154..9e9f93c62adf1 100644 --- a/aptos-move/aptos-debugger/src/aptos_debugger.rs +++ b/aptos-move/aptos-debugger/src/aptos_debugger.rs @@ -439,6 +439,8 @@ fn execute_block_no_limit( onchain: BlockExecutorConfigFromOnchain::new_no_block_limit(), }, None, + None, + None, ) .map(BlockOutput::into_transaction_outputs_forced) } diff --git a/aptos-move/aptos-e2e-comparison-testing/src/data_collection.rs b/aptos-move/aptos-e2e-comparison-testing/src/data_collection.rs index c5c3e3b153ab0..6276683e8fd18 100644 --- a/aptos-move/aptos-e2e-comparison-testing/src/data_collection.rs +++ b/aptos-move/aptos-e2e-comparison-testing/src/data_collection.rs @@ -93,7 +93,7 @@ impl DataCollection { let val = debugger_state_view.get_state_value(TOTAL_SUPPLY_STATE_KEY.deref()); assert!(val.is_ok() && val.unwrap().is_some()); AptosVMBlockExecutor::new() - .execute_block_no_limit(&sig_verified_txns, debugger_state_view) + .execute_block_no_limit(&sig_verified_txns, debugger_state_view, None, None) .map_err(|err| format_err!("Unexpected VM Error: {:?}", err)) } diff --git a/aptos-move/aptos-transaction-benchmarks/src/transaction_bench_state.rs b/aptos-move/aptos-transaction-benchmarks/src/transaction_bench_state.rs index 9a18b17c35c96..03355bab525f6 100644 --- a/aptos-move/aptos-transaction-benchmarks/src/transaction_bench_state.rs +++ b/aptos-move/aptos-transaction-benchmarks/src/transaction_bench_state.rs @@ -223,6 +223,8 @@ where &ModuleCacheManager::new(), BlockExecutorConfig::new_maybe_block_limit(1, maybe_block_gas_limit), None, + None, + None, ) .expect("VM should not fail to start") .into_transaction_outputs_forced(); @@ -275,6 +277,8 @@ where maybe_block_gas_limit, ), None, + None, + None, ) .expect("VM should not fail to start") .into_transaction_outputs_forced(); diff --git a/aptos-move/aptos-transactional-test-harness/src/aptos_test_harness.rs b/aptos-move/aptos-transactional-test-harness/src/aptos_test_harness.rs index aff52ca04c4e9..811a000248bc5 100644 --- a/aptos-move/aptos-transactional-test-harness/src/aptos_test_harness.rs +++ b/aptos-move/aptos-transactional-test-harness/src/aptos_test_harness.rs @@ -515,14 +515,12 @@ impl<'a> AptosTestAdapter<'a> { fn run_transaction(&mut self, txn: Transaction) -> Result { let txn_block = vec![txn]; let sig_verified_block = into_signature_verified_block(txn_block); - - let executor = AptosVMBlockExecutor::new(); - if let Some(module_cache_manager) = executor.module_cache_manager() { - module_cache_manager.mark_ready(None, None); - } - - let mut outputs = - executor.execute_block_no_limit(&sig_verified_block, &self.storage.clone())?; + let mut outputs = AptosVMBlockExecutor::new().execute_block_no_limit( + &sig_verified_block, + &self.storage.clone(), + None, + None, + )?; assert_eq!(outputs.len(), 1); diff --git a/aptos-move/aptos-vm-profiling/src/bins/run_aptos_p2p.rs b/aptos-move/aptos-vm-profiling/src/bins/run_aptos_p2p.rs index 02a5ed3c37a8c..282156b481881 100644 --- a/aptos-move/aptos-vm-profiling/src/bins/run_aptos_p2p.rs +++ b/aptos-move/aptos-vm-profiling/src/bins/run_aptos_p2p.rs @@ -48,7 +48,8 @@ fn main() -> Result<()> { }) .collect(); - let outputs = AptosVMBlockExecutor::new().execute_block_no_limit(&txns, &state_store)?; + let outputs = + AptosVMBlockExecutor::new().execute_block_no_limit(&txns, &state_store, None, None)?; for i in 0..NUM_TXNS { assert!(outputs[i as usize].status().status().unwrap().is_success()); } diff --git a/aptos-move/aptos-vm/src/aptos_vm.rs b/aptos-move/aptos-vm/src/aptos_vm.rs index 30305923add99..d50b5c5371047 100644 --- a/aptos-move/aptos-vm/src/aptos_vm.rs +++ b/aptos-move/aptos-vm/src/aptos_vm.rs @@ -2793,19 +2793,13 @@ impl VMBlockExecutor for AptosVMBlockExecutor { } } - fn module_cache_manager( - &self, - ) -> Option< - &ModuleCacheManager, - > { - Some(&self.module_cache_manager) - } - fn execute_block( &self, transactions: &[SignatureVerifiedTransaction], state_view: &(impl StateView + Sync), onchain_config: BlockExecutorConfigFromOnchain, + parent_block: Option<&HashValue>, + current_block: Option, ) -> Result, VMStatus> { fail_point!("move_adapter::execute_block", |_| { Err(VMStatus::error( @@ -2837,6 +2831,8 @@ impl VMBlockExecutor for AptosVMBlockExecutor { }, onchain: onchain_config, }, + parent_block, + current_block, None, ); if ret.is_ok() { diff --git a/aptos-move/aptos-vm/src/block_executor/mod.rs b/aptos-move/aptos-vm/src/block_executor/mod.rs index 68877952e80ee..b19981e5aa937 100644 --- a/aptos-move/aptos-vm/src/block_executor/mod.rs +++ b/aptos-move/aptos-vm/src/block_executor/mod.rs @@ -45,10 +45,7 @@ use aptos_vm_types::{ output::VMOutput, resolver::ResourceGroupSize, }; -use move_binary_format::{ - errors::{Location, VMError}, - CompiledModule, -}; +use move_binary_format::{errors::VMError, CompiledModule}; use move_core_types::{ account_address::AccountAddress, ident_str, @@ -56,7 +53,7 @@ use move_core_types::{ value::MoveTypeLayout, vm_status::{StatusCode, VMStatus}, }; -use move_vm_runtime::{Module, ModuleStorage, WithRuntimeEnvironment}; +use move_vm_runtime::{Module, ModuleStorage}; use move_vm_types::delayed_values::delayed_field_id::DelayedFieldID; use once_cell::sync::{Lazy, OnceCell}; use std::{ @@ -420,6 +417,8 @@ impl BlockAptosVM { AptosModuleExtension, >, config: BlockExecutorConfig, + parent_block: Option<&HashValue>, + current_block: Option, transaction_commit_listener: Option, ) -> Result, VMStatus> { let _timer = BLOCK_EXECUTOR_EXECUTE_BLOCK_SECONDS.start_timer(); @@ -433,42 +432,19 @@ impl BlockAptosVM { BLOCK_EXECUTOR_CONCURRENCY.set(config.local.concurrency_level as i64); - let (environment, module_cache) = if module_cache_manager.mark_executing() { - let environment = module_cache_manager.get_or_initialize_environment(state_view); - let module_cache = module_cache_manager.module_cache(); - (environment, module_cache) - } else { - // Either we do not have global caches , in which case we can create new ones, or - // something went wrong, and we were not able to mark the state as executing. In - // this case, fallback to empty caches. Note that the alert should have been raised - // during marking. - let environment = - AptosEnvironment::new_with_delayed_field_optimization_enabled(state_view); - let module_cache = Arc::new(GlobalModuleCache::empty()); - (environment, module_cache) - }; - - // We should be checking different module cache configurations here. - let module_cache_config = &config.local.module_cache_config; - - // Check 1: struct re-indexing map is not too large. If it is, we flush the cache. Also, we - // need to flush modules because they store indices into re-indexing map. - let runtime_environment = environment.runtime_environment(); - let struct_name_index_map_size = runtime_environment - .struct_name_index_map_size() - .map_err(|err| err.finish(Location::Undefined).into_vm_status())?; - if struct_name_index_map_size > module_cache_config.max_struct_name_index_map_num_entries { - module_cache.flush_unsync(); - runtime_environment.flush_struct_name_and_info_caches(); - } - - // Check 2: If the module cache is too big, flush it. - if module_cache.size_in_bytes() > module_cache_config.max_module_cache_size_in_bytes { - module_cache.flush_unsync(); + if !module_cache_manager.mark_ready(parent_block, current_block) { + return Err(VMStatus::error( + StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR, + Some("Unable to mark module caches for block execution as ready".to_string()), + )); } + let (environment, module_cache) = module_cache_manager + .check_ready_and_get_caches(state_view, &config.local.module_cache_config)?; // Finally, to avoid cold starts, fetch the framework code prior to block execution. - if module_cache.num_modules() == 0 && module_cache_config.prefetch_framework_code { + if module_cache.num_modules() == 0 + && config.local.module_cache_config.prefetch_framework_code + { let code_storage = state_view.as_aptos_code_storage(environment.clone()); prefetch_aptos_framework(code_storage, &module_cache).map_err(|err| { alert!("Failed to load Aptos framework to module cache: {:?}", err); @@ -489,6 +465,12 @@ impl BlockAptosVM { transaction_commit_listener, ); + if !module_cache_manager.mark_executing() { + return Err(VMStatus::error( + StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR, + Some("Unable to mark block execution start".to_string()), + )); + } let ret = executor.execute_block(environment, signature_verified_block, state_view); if !module_cache_manager.mark_done() { return Err(VMStatus::error( @@ -542,6 +524,8 @@ impl BlockAptosVM { AptosModuleExtension, >, config: BlockExecutorConfig, + parent_block: Option<&HashValue>, + current_block: Option, transaction_commit_listener: Option, ) -> Result, VMStatus> { Self::execute_block_on_thread_pool::( @@ -550,6 +534,8 @@ impl BlockAptosVM { state_view, module_cache_manager, config, + parent_block, + current_block, transaction_commit_listener, ) } diff --git a/aptos-move/aptos-vm/src/lib.rs b/aptos-move/aptos-vm/src/lib.rs index 977287304b10c..34b1c8085d1bf 100644 --- a/aptos-move/aptos-vm/src/lib.rs +++ b/aptos-move/aptos-vm/src/lib.rs @@ -126,7 +126,6 @@ pub mod verifier; pub use crate::aptos_vm::{AptosSimulationVM, AptosVM}; use crate::sharded_block_executor::{executor_client::ExecutorClient, ShardedBlockExecutor}; -use aptos_block_executor::code_cache_global_manager::ModuleCacheManager; use aptos_crypto::HashValue; use aptos_types::{ block_executor::{ @@ -137,13 +136,9 @@ use aptos_types::{ signature_verified_transaction::SignatureVerifiedTransaction, BlockOutput, SignedTransaction, TransactionOutput, VMValidatorResult, }, - vm::modules::AptosModuleExtension, vm_status::VMStatus, }; use aptos_vm_types::module_and_script_storage::code_storage::AptosCodeStorage; -use move_binary_format::CompiledModule; -use move_core_types::language_storage::ModuleId; -use move_vm_runtime::Module; use std::{marker::Sync, sync::Arc}; pub use verifier::view_function::determine_is_view; @@ -166,22 +161,14 @@ pub trait VMBlockExecutor: Send + Sync { /// an old one. fn new() -> Self; - /// Returns the cache manager responsible for keeping module caches in sync. By default, is - /// [None]. - fn module_cache_manager( - &self, - ) -> Option< - &ModuleCacheManager, - > { - None - } - /// Executes a block of transactions and returns output for each one of them. fn execute_block( &self, transactions: &[SignatureVerifiedTransaction], state_view: &(impl StateView + Sync), onchain_config: BlockExecutorConfigFromOnchain, + parent_block: Option<&HashValue>, + current_block: Option, ) -> Result, VMStatus>; /// Executes a block of transactions and returns output for each one of them, without applying @@ -190,11 +177,15 @@ pub trait VMBlockExecutor: Send + Sync { &self, transactions: &[SignatureVerifiedTransaction], state_view: &(impl StateView + Sync), + parent_block: Option<&HashValue>, + current_block: Option, ) -> Result, VMStatus> { self.execute_block( transactions, state_view, BlockExecutorConfigFromOnchain::new_no_block_limit(), + parent_block, + current_block, ) .map(BlockOutput::into_transaction_outputs_forced) } diff --git a/aptos-move/aptos-vm/src/sharded_block_executor/sharded_executor_service.rs b/aptos-move/aptos-vm/src/sharded_block_executor/sharded_executor_service.rs index d6b09906a50a2..4c52ba947339f 100644 --- a/aptos-move/aptos-vm/src/sharded_block_executor/sharded_executor_service.rs +++ b/aptos-move/aptos-vm/src/sharded_block_executor/sharded_executor_service.rs @@ -136,17 +136,16 @@ impl ShardedExecutorService { ); }); s.spawn(move |_| { - // Since we execute blocks in parallel, we cannot share module caches, so each - // thread has its own caches. - let module_cache_manager = ModuleCacheManager::new(); - module_cache_manager.mark_ready(None, None); - let ret = BlockAptosVM::execute_block_on_thread_pool( executor_thread_pool, &signature_verified_transactions, aggr_overridden_state_view.as_ref(), - &module_cache_manager, + // Since we execute blocks in parallel, we cannot share module caches, so each + // thread has its own caches. + &ModuleCacheManager::new(), config, + None, + None, cross_shard_commit_sender, ) .map(BlockOutput::into_transaction_outputs_forced); diff --git a/aptos-move/aptos-vm/tests/sharded_block_executor.rs b/aptos-move/aptos-vm/tests/sharded_block_executor.rs index 1dcfbea2dd0f9..bbe6862f99163 100644 --- a/aptos-move/aptos-vm/tests/sharded_block_executor.rs +++ b/aptos-move/aptos-vm/tests/sharded_block_executor.rs @@ -308,12 +308,8 @@ mod test_utils { .into_iter() .map(|t| t.into_txn()) .collect(); - let block_executor = AptosVMBlockExecutor::new(); - if let Some(module_cache_manager) = block_executor.module_cache_manager() { - module_cache_manager.mark_ready(None, None); - } - let unsharded_txn_output = block_executor - .execute_block_no_limit(&ordered_txns, executor.data_store()) + let unsharded_txn_output = AptosVMBlockExecutor::new() + .execute_block_no_limit(&ordered_txns, executor.data_store(), None, None) .unwrap(); compare_txn_outputs(unsharded_txn_output, sharded_txn_output); } @@ -362,12 +358,8 @@ mod test_utils { ) .unwrap(); - let block_executor = AptosVMBlockExecutor::new(); - if let Some(module_cache_manager) = block_executor.module_cache_manager() { - module_cache_manager.mark_ready(None, None); - } - let unsharded_txn_output = block_executor - .execute_block_no_limit(&execution_ordered_txns, executor.data_store()) + let unsharded_txn_output = AptosVMBlockExecutor::new() + .execute_block_no_limit(&execution_ordered_txns, executor.data_store(), None, None) .unwrap(); compare_txn_outputs(unsharded_txn_output, sharded_txn_output); } @@ -420,12 +412,8 @@ mod test_utils { ) .unwrap(); - let block_executor = AptosVMBlockExecutor::new(); - if let Some(module_cache_manager) = block_executor.module_cache_manager() { - module_cache_manager.mark_ready(None, None); - } - let unsharded_txn_output = block_executor - .execute_block_no_limit(&execution_ordered_txns, executor.data_store()) + let unsharded_txn_output = AptosVMBlockExecutor::new() + .execute_block_no_limit(&execution_ordered_txns, executor.data_store(), None, None) .unwrap(); compare_txn_outputs(unsharded_txn_output, sharded_txn_output); } diff --git a/aptos-move/block-executor/Cargo.toml b/aptos-move/block-executor/Cargo.toml index 958c09a5787c2..a1aefe7441e45 100644 --- a/aptos-move/block-executor/Cargo.toml +++ b/aptos-move/block-executor/Cargo.toml @@ -55,6 +55,7 @@ aptos-types = { workspace = true, features = ["testing"] } criterion = { workspace = true } fail = { workspace = true, features = ["failpoints"] } itertools = { workspace = true } +move-vm-runtime = { workspace = true, features = ["testing"] } move-vm-types = { workspace = true, features = ["testing"] } proptest = { workspace = true } proptest-derive = { workspace = true } diff --git a/aptos-move/block-executor/src/code_cache_global_manager.rs b/aptos-move/block-executor/src/code_cache_global_manager.rs index 5793feab4c8b0..380462a482035 100644 --- a/aptos-move/block-executor/src/code_cache_global_manager.rs +++ b/aptos-move/block-executor/src/code_cache_global_manager.rs @@ -2,8 +2,12 @@ // SPDX-License-Identifier: Apache-2.0 use crate::{code_cache_global::GlobalModuleCache, explicit_sync_wrapper::ExplicitSyncWrapper}; -use aptos_types::state_store::StateView; +use aptos_types::{ + block_executor::config::BlockExecutorModuleCacheLocalConfig, state_store::StateView, +}; use aptos_vm_environment::environment::AptosEnvironment; +use move_binary_format::errors::Location; +use move_core_types::vm_status::{StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR, VMStatus}; use move_vm_runtime::WithRuntimeEnvironment; use move_vm_types::code::WithSize; use parking_lot::Mutex; @@ -97,10 +101,8 @@ where environment .runtime_environment() .flush_struct_name_and_info_caches(); - self.module_cache.flush_unsync(); - } else { - debug_assert!(self.module_cache.num_modules() == 0); } + self.module_cache.flush_unsync(); } *state = State::Ready(current); @@ -117,6 +119,52 @@ where } } + /// When in [State::Ready], runs different checks on cached modules and environment: + /// 1. If the environment is not initialized, or is different from the one in storage, it is + /// re-initialized, and module caches are flushed. + /// 2. If too many struct names have been cached in re-indexing map in runtime environment, + /// struct type caches and module caches are flushed. + /// 3. If module cache size is too large (in bytes), it is flushed. + /// The final environment and module caches are returned. + pub fn check_ready_and_get_caches( + &self, + state_view: &impl StateView, + config: &BlockExecutorModuleCacheLocalConfig, + ) -> Result<(AptosEnvironment, Arc>), VMStatus> { + let state = self.state.lock(); + if !matches!(state.deref(), State::Ready(_)) { + let msg = format!( + "Expected ready state to check caches, got {:?}", + state.deref() + ); + return Err(VMStatus::error( + UNKNOWN_INVARIANT_VIOLATION_ERROR, + Some(msg), + )); + } + + let environment = self.get_or_initialize_environment(state_view); + let module_cache = self.module_cache.clone(); + + // Check 1: struct re-indexing map is not too large. If it is, we flush the cache. Also, we + // need to flush modules because they store indices into re-indexing map. + let runtime_environment = environment.runtime_environment(); + let struct_name_index_map_size = runtime_environment + .struct_name_index_map_size() + .map_err(|err| err.finish(Location::Undefined).into_vm_status())?; + if struct_name_index_map_size > config.max_struct_name_index_map_num_entries { + module_cache.flush_unsync(); + runtime_environment.flush_struct_name_and_info_caches(); + } + + // Check 2: If the module cache is too big, flush it. + if module_cache.size_in_bytes() > config.max_module_cache_size_in_bytes { + module_cache.flush_unsync(); + } + + Ok((environment, module_cache)) + } + /// If state is [State::Ready], changes it to [State::Executing] with the same value, returning /// true. Otherwise, returns false indicating that state transition failed, also raising an /// alert. @@ -147,10 +195,8 @@ where /// Returns the cached global environment if it already exists, and matches the one in storage. /// If it does not exist, or does not match, the new environment is initialized from the given - /// state, cached, and returned. - pub fn get_or_initialize_environment(&self, state_view: &impl StateView) -> AptosEnvironment { - let _lock = self.state.lock(); - + /// state, cached, and returned. Should be called when in [State::Ready] state, under lock. + fn get_or_initialize_environment(&self, state_view: &impl StateView) -> AptosEnvironment { let new_environment = AptosEnvironment::new_with_delayed_field_optimization_enabled(state_view); @@ -159,7 +205,7 @@ where let environment_requires_update = existing_environment .as_ref() - .map_or(true, |environment| environment == &new_environment); + .map_or(true, |environment| environment != &new_environment); if environment_requires_update { *existing_environment = Some(new_environment); @@ -172,11 +218,6 @@ where .clone() .expect("Environment must be set") } - - /// Returns the global module cache. - pub fn module_cache(&self) -> Arc> { - self.module_cache.clone() - } } #[cfg(test)] @@ -186,8 +227,13 @@ mod test { on_chain_config::{FeatureFlag, Features, OnChainConfig}, state_store::{state_key::StateKey, state_value::StateValue, MockStateView}, }; - use move_vm_types::code::{ - mock_verified_code, MockDeserializedCode, MockExtension, MockVerifiedCode, + use claims::assert_ok; + use move_core_types::{ + account_address::AccountAddress, identifier::Identifier, language_storage::ModuleId, + }; + use move_vm_types::{ + code::{mock_verified_code, MockDeserializedCode, MockExtension, MockVerifiedCode}, + loaded_data::runtime_types::StructIdentifier, }; use std::{collections::HashMap, thread, thread::JoinHandle}; use test_case::test_case; @@ -209,7 +255,6 @@ mod test { assert!(!module_cache_manager.mark_executing()); assert!(!module_cache_manager.mark_done()); - assert!(module_cache_manager.mark_ready(previous.as_ref(), Some(77))); // Only in matching case the module cache is not flushed. @@ -223,11 +268,105 @@ mod test { assert_eq!(state, State::Ready(Some(77))) } + #[test] + fn test_check_ready() { + let state_view = MockStateView::empty(); + let config = BlockExecutorModuleCacheLocalConfig { + prefetch_framework_code: false, + max_module_cache_size_in_bytes: 8, + max_struct_name_index_map_num_entries: 2, + }; + + let module_cache_manager = ModuleCacheManager::< + i32, + i32, + MockDeserializedCode, + MockVerifiedCode, + MockExtension, + >::new(); + + // Set up the state and the environment. + *module_cache_manager.state.lock() = State::Ready(None); + let environment = module_cache_manager.get_or_initialize_environment(&state_view); + + module_cache_manager + .module_cache + .insert(0, mock_verified_code(0, MockExtension::new(16))); + assert_eq!(module_cache_manager.module_cache.num_modules(), 1); + + let runtime_environment = environment.runtime_environment(); + let dummy_struct_name = StructIdentifier { + module: ModuleId::new(AccountAddress::ONE, Identifier::new("foo").unwrap()), + name: Identifier::new("Bar").unwrap(), + }; + assert!(runtime_environment + .struct_name_to_idx_for_test(dummy_struct_name) + .is_ok()); + assert_eq!( + assert_ok!(runtime_environment.struct_name_index_map_size()), + 1 + ); + + // Module cache size in bytes is too large, should be flushed (but not struct types). + assert!(module_cache_manager + .check_ready_and_get_caches(&state_view, &config) + .is_ok()); + assert_eq!(module_cache_manager.module_cache.num_modules(), 0); + assert_eq!( + assert_ok!(runtime_environment.struct_name_index_map_size()), + 1 + ); + + module_cache_manager + .module_cache + .insert(0, mock_verified_code(0, MockExtension::new(4))); + + // This time size is less than the one specified in config. No flushing. + assert!(module_cache_manager + .check_ready_and_get_caches(&state_view, &config) + .is_ok()); + assert_eq!(module_cache_manager.module_cache.num_modules(), 1); + assert_eq!( + assert_ok!(runtime_environment.struct_name_index_map_size()), + 1 + ); + + let dummy_struct_names = [ + StructIdentifier { + module: ModuleId::new(AccountAddress::ONE, Identifier::new("foo").unwrap()), + name: Identifier::new("Foo").unwrap(), + }, + StructIdentifier { + module: ModuleId::new(AccountAddress::ONE, Identifier::new("foo").unwrap()), + name: Identifier::new("Baz").unwrap(), + }, + ]; + for dummy_struct_name in dummy_struct_names { + assert!(runtime_environment + .struct_name_to_idx_for_test(dummy_struct_name) + .is_ok()); + } + assert_eq!( + assert_ok!(runtime_environment.struct_name_index_map_size()), + 3 + ); + + // Too many struct names cached. + assert!(module_cache_manager + .check_ready_and_get_caches(&state_view, &config) + .is_ok()); + assert_eq!(module_cache_manager.module_cache.num_modules(), 0); + assert_eq!( + assert_ok!(runtime_environment.struct_name_index_map_size()), + 0 + ); + } + #[test] fn test_mark_executing() { let module_cache_manager = ModuleCacheManager::< - _, - u32, + i32, + i32, MockDeserializedCode, MockVerifiedCode, MockExtension, @@ -246,8 +385,8 @@ mod test { #[test] fn test_mark_done() { let module_cache_manager = ModuleCacheManager::< - _, - u32, + i32, + i32, MockDeserializedCode, MockVerifiedCode, MockExtension, @@ -283,8 +422,8 @@ mod test { #[test] fn test_mark_ready_concurrent() { let global_cache_manager = Arc::new(ModuleCacheManager::< - _, - u32, + i32, + i32, MockDeserializedCode, MockVerifiedCode, MockExtension, @@ -304,8 +443,8 @@ mod test { #[test] fn test_mark_executing_concurrent() { let global_cache_manager = Arc::new(ModuleCacheManager::< - _, - u32, + i32, + i32, MockDeserializedCode, MockVerifiedCode, MockExtension, @@ -326,8 +465,8 @@ mod test { #[test] fn test_mark_done_concurrent() { let global_cache_manager = Arc::new(ModuleCacheManager::< - _, - u32, + i32, + i32, MockDeserializedCode, MockVerifiedCode, MockExtension, @@ -368,7 +507,14 @@ mod test { #[test] fn test_get_or_initialize_environment() { - let module_cache_manager = ModuleCacheManager::::new(); + let module_cache_manager = ModuleCacheManager::< + i32, + i32, + MockDeserializedCode, + MockVerifiedCode, + MockExtension, + >::new(); + *module_cache_manager.state.lock() = State::Ready(None); module_cache_manager .module_cache diff --git a/aptos-move/e2e-tests/src/executor.rs b/aptos-move/e2e-tests/src/executor.rs index cff14879c0c52..da4598abf98c5 100644 --- a/aptos-move/e2e-tests/src/executor.rs +++ b/aptos-move/e2e-tests/src/executor.rs @@ -640,11 +640,6 @@ impl FakeExecutor { }, onchain: onchain_config, }; - - // Do not use shared module caches in tests. - let module_cache_manager = ModuleCacheManager::new(); - module_cache_manager.mark_ready(None, None); - BlockAptosVM::execute_block_on_thread_pool::< _, NoOpTransactionCommitHook, @@ -652,9 +647,12 @@ impl FakeExecutor { self.executor_thread_pool.clone(), txn_block, &state_view, - &module_cache_manager, + // Do not use shared module caches in tests. + &ModuleCacheManager::new(), config, None, + None, + None, ) .map(BlockOutput::into_transaction_outputs_forced) } diff --git a/execution/executor-benchmark/src/native_executor.rs b/execution/executor-benchmark/src/native_executor.rs index f90eb1498f79e..38e1b08b4c393 100644 --- a/execution/executor-benchmark/src/native_executor.rs +++ b/execution/executor-benchmark/src/native_executor.rs @@ -6,6 +6,7 @@ use crate::{ metrics::TIMER, }; use anyhow::Result; +use aptos_crypto::HashValue; use aptos_types::{ account_address::AccountAddress, account_config::{DepositEvent, WithdrawEvent}, @@ -359,6 +360,8 @@ impl VMBlockExecutor for NativeExecutor { transactions: &[SignatureVerifiedTransaction], state_view: &(impl StateView + Sync), _onchain_config: BlockExecutorConfigFromOnchain, + _parent_block: Option<&HashValue>, + _current_block: Option, ) -> Result, VMStatus> { let transaction_outputs = NATIVE_EXECUTOR_POOL .install(|| { diff --git a/execution/executor-service/src/test_utils.rs b/execution/executor-service/src/test_utils.rs index bfd2c36cd1776..15e7e3ea10761 100644 --- a/execution/executor-service/src/test_utils.rs +++ b/execution/executor-service/src/test_utils.rs @@ -137,12 +137,8 @@ pub fn test_sharded_block_executor_no_conflict> .into_iter() .map(|t| t.into_txn()) .collect(); - let block_executor = AptosVMBlockExecutor::new(); - if let Some(module_cache_manager) = block_executor.module_cache_manager() { - module_cache_manager.mark_ready(None, None); - } - let unsharded_txn_output = block_executor - .execute_block_no_limit(&txns, executor.data_store()) + let unsharded_txn_output = AptosVMBlockExecutor::new() + .execute_block_no_limit(&txns, executor.data_store(), None, None) .unwrap(); compare_txn_outputs(unsharded_txn_output, sharded_txn_output); sharded_block_executor.shutdown(); @@ -196,12 +192,8 @@ pub fn sharded_block_executor_with_conflict>( ) .unwrap(); - let block_executor = AptosVMBlockExecutor::new(); - if let Some(module_cache_manager) = block_executor.module_cache_manager() { - module_cache_manager.mark_ready(None, None); - } - let unsharded_txn_output = block_executor - .execute_block_no_limit(&execution_ordered_txns, executor.data_store()) + let unsharded_txn_output = AptosVMBlockExecutor::new() + .execute_block_no_limit(&execution_ordered_txns, executor.data_store(), None, None) .unwrap(); compare_txn_outputs(unsharded_txn_output, sharded_txn_output); sharded_block_executor.shutdown(); diff --git a/execution/executor/src/block_executor/mod.rs b/execution/executor/src/block_executor/mod.rs index 1595de3eeb4ba..45738dd6ca6a5 100644 --- a/execution/executor/src/block_executor/mod.rs +++ b/execution/executor/src/block_executor/mod.rs @@ -198,77 +198,64 @@ where "execute_block" ); let committed_block_id = self.committed_block_id(); - let (execution_output, state_checkpoint_output) = if parent_block_id != committed_block_id - && parent_output.has_reconfiguration() - { - // ignore reconfiguration suffix, even if the block is non-empty - info!( - LogSchema::new(LogEntry::BlockExecutor).block_id(block_id), - "reconfig_descendant_block_received" - ); - ( - parent_output.execution_output.reconfig_suffix(), - parent_output - .expect_state_checkpoint_output() - .reconfig_suffix(), - ) - } else { - let state_view = { - let _timer = OTHER_TIMERS.timer_with(&["verified_state_view"]); - - CachedStateView::new( - StateViewId::BlockExecution { block_id }, - Arc::clone(&self.db.reader), - parent_output.execution_output.next_version(), - parent_output.expect_result_state().current.clone(), - Arc::new(AsyncProofFetcher::new(self.db.reader.clone())), - )? - }; - - let execution_output = { - let _timer = GET_BLOCK_EXECUTION_OUTPUT_BY_EXECUTING.start_timer(); - fail_point!("executor::block_executor_execute_block", |_| { - Err(ExecutorError::from(anyhow::anyhow!( - "Injected error in block_executor_execute_block" - ))) - }); - - // In case block executor has a cache manager, we need to mark it as ready for - // execution. If for some reason this fails, return an error. - if let Some(module_cache_manager) = self.block_executor.module_cache_manager() { - // TODO(loader_v2): - // Refactor to be able to move this into AptosVM block executor. This will - // also allow us to remove all ready markings in other places. - if !module_cache_manager.mark_ready(Some(&parent_block_id), Some(block_id)) { - return Err(ExecutorError::internal_err( - "Unable to mark module cache manager as ready", - )); - } - } - - DoGetExecutionOutput::by_transaction_execution( - &self.block_executor, - transactions, - state_view, - onchain_config.clone(), - Some(block_id), - )? - }; - - let _timer = OTHER_TIMERS.timer_with(&["state_checkpoint"]); - - let state_checkpoint_output = THREAD_MANAGER.get_exe_cpu_pool().install(|| { - fail_point!("executor::block_state_checkpoint", |_| { - Err(anyhow::anyhow!("Injected error in block state checkpoint.")) - }); - DoStateCheckpoint::run( - &execution_output, - parent_output.expect_result_state(), - Option::>::None, + let (execution_output, state_checkpoint_output) = + if parent_block_id != committed_block_id && parent_output.has_reconfiguration() { + // ignore reconfiguration suffix, even if the block is non-empty + info!( + LogSchema::new(LogEntry::BlockExecutor).block_id(block_id), + "reconfig_descendant_block_received" + ); + ( + parent_output.execution_output.reconfig_suffix(), + parent_output + .expect_state_checkpoint_output() + .reconfig_suffix(), ) - })?; - (execution_output, state_checkpoint_output) - }; + } else { + let state_view = { + let _timer = OTHER_TIMERS.timer_with(&["verified_state_view"]); + + CachedStateView::new( + StateViewId::BlockExecution { block_id }, + Arc::clone(&self.db.reader), + parent_output.execution_output.next_version(), + parent_output.expect_result_state().current.clone(), + Arc::new(AsyncProofFetcher::new(self.db.reader.clone())), + )? + }; + + let execution_output = { + let _timer = GET_BLOCK_EXECUTION_OUTPUT_BY_EXECUTING.start_timer(); + fail_point!("executor::block_executor_execute_block", |_| { + Err(ExecutorError::from(anyhow::anyhow!( + "Injected error in block_executor_execute_block" + ))) + }); + + DoGetExecutionOutput::by_transaction_execution( + &self.block_executor, + transactions, + state_view, + onchain_config.clone(), + Some(&parent_block_id), + Some(block_id), + )? + }; + + let _timer = OTHER_TIMERS.timer_with(&["state_checkpoint"]); + + let state_checkpoint_output = THREAD_MANAGER.get_exe_cpu_pool().install(|| { + fail_point!("executor::block_state_checkpoint", |_| { + Err(anyhow::anyhow!("Injected error in block state checkpoint.")) + }); + DoStateCheckpoint::run( + &execution_output, + parent_output.expect_result_state(), + Option::>::None, + ) + })?; + (execution_output, state_checkpoint_output) + }; let output = PartialStateComputeResult::new(execution_output); output.set_state_checkpoint_output(state_checkpoint_output); diff --git a/execution/executor/src/chunk_executor/mod.rs b/execution/executor/src/chunk_executor/mod.rs index c97398fae868e..55c8e005ede28 100644 --- a/execution/executor/src/chunk_executor/mod.rs +++ b/execution/executor/src/chunk_executor/mod.rs @@ -597,19 +597,14 @@ impl ChunkExecutorInner { .map(|t| t.into()) .collect::>(); - // For now, we create executor for each chunk. - let executor = V::new(); - if let Some(module_cache_manager) = executor.module_cache_manager() { - module_cache_manager.mark_ready(None, None); - } - // State sync executor shouldn't have block gas limit. let execution_output = DoGetExecutionOutput::by_transaction_execution::( - &executor, + &V::new(), txns.into(), state_view, BlockExecutorConfigFromOnchain::new_no_block_limit(), None, + None, )?; // not `zip_eq`, deliberately for (version, txn_out, txn_info, write_set, events) in multizip(( diff --git a/execution/executor/src/chunk_executor/transaction_chunk.rs b/execution/executor/src/chunk_executor/transaction_chunk.rs index a51ec0f5c9bf8..46d84d4fc1ae1 100644 --- a/execution/executor/src/chunk_executor/transaction_chunk.rs +++ b/execution/executor/src/chunk_executor/transaction_chunk.rs @@ -83,18 +83,13 @@ impl TransactionChunk for ChunkToExecute { }; let _timer = VM_EXECUTE_CHUNK.start_timer(); - - let executor = V::new(); - if let Some(module_cache_manager) = executor.module_cache_manager() { - module_cache_manager.mark_ready(None, None); - } - DoGetExecutionOutput::by_transaction_execution::( - &executor, + &V::new(), sig_verified_txns.into(), state_view, BlockExecutorConfigFromOnchain::new_no_block_limit(), None, + None, ) } } diff --git a/execution/executor/src/db_bootstrapper/mod.rs b/execution/executor/src/db_bootstrapper/mod.rs index d9dcd513fe82d..ccb6bc6195014 100644 --- a/execution/executor/src/db_bootstrapper/mod.rs +++ b/execution/executor/src/db_bootstrapper/mod.rs @@ -131,17 +131,13 @@ pub fn calculate_genesis( get_state_epoch(&base_state_view)? }; - let executor = V::new(); - if let Some(module_cache_manager) = executor.module_cache_manager() { - module_cache_manager.mark_ready(None, None); - } - let execution_output = DoGetExecutionOutput::by_transaction_execution::( - &executor, + &V::new(), vec![genesis_txn.clone().into()].into(), base_state_view, BlockExecutorConfigFromOnchain::new_no_block_limit(), None, + None, )?; ensure!( execution_output.num_transactions_to_commit() != 0, diff --git a/execution/executor/src/fuzzing.rs b/execution/executor/src/fuzzing.rs index 67639b5831107..d73e272557dab 100644 --- a/execution/executor/src/fuzzing.rs +++ b/execution/executor/src/fuzzing.rs @@ -79,6 +79,8 @@ impl VMBlockExecutor for FakeVM { _transactions: &[SignatureVerifiedTransaction], _state_view: &impl StateView, _onchain_config: BlockExecutorConfigFromOnchain, + _parent_block: Option<&HashValue>, + _current_block: Option, ) -> Result, VMStatus> { Ok(BlockOutput::new(vec![], None)) } diff --git a/execution/executor/src/tests/mock_vm/mock_vm_test.rs b/execution/executor/src/tests/mock_vm/mock_vm_test.rs index 4df0ef06d0665..427897e3c9e52 100644 --- a/execution/executor/src/tests/mock_vm/mock_vm_test.rs +++ b/execution/executor/src/tests/mock_vm/mock_vm_test.rs @@ -29,6 +29,8 @@ fn test_mock_vm_different_senders() { .execute_block_no_limit( &into_signature_verified_block(txns.clone()), &MockStateView::empty(), + None, + None, ) .expect("MockVM should not fail to start"); @@ -69,6 +71,8 @@ fn test_mock_vm_same_sender() { .execute_block_no_limit( &into_signature_verified_block(txns), &MockStateView::empty(), + None, + None, ) .expect("MockVM should not fail to start"); @@ -107,6 +111,8 @@ fn test_mock_vm_payment() { .execute_block_no_limit( &into_signature_verified_block(txns), &MockStateView::empty(), + None, + None, ) .expect("MockVM should not fail to start"); diff --git a/execution/executor/src/tests/mock_vm/mod.rs b/execution/executor/src/tests/mock_vm/mod.rs index bb9ea70a99393..701c2e1d332fc 100644 --- a/execution/executor/src/tests/mock_vm/mod.rs +++ b/execution/executor/src/tests/mock_vm/mod.rs @@ -6,7 +6,7 @@ mod mock_vm_test; use anyhow::Result; -use aptos_crypto::{ed25519::Ed25519PrivateKey, PrivateKey, Uniform}; +use aptos_crypto::{ed25519::Ed25519PrivateKey, HashValue, PrivateKey, Uniform}; use aptos_types::{ account_address::AccountAddress, account_config::NEW_EPOCH_EVENT_V2_MOVE_TYPE_TAG, @@ -67,6 +67,8 @@ impl VMBlockExecutor for MockVM { transactions: &[SignatureVerifiedTransaction], state_view: &impl StateView, _onchain_config: BlockExecutorConfigFromOnchain, + _parent_block: Option<&HashValue>, + _current_block: Option, ) -> Result, VMStatus> { // output_cache is used to store the output of transactions so they are visible to later // transactions. diff --git a/execution/executor/src/tests/mod.rs b/execution/executor/src/tests/mod.rs index 8ca28af1a83ff..366c891ff1760 100644 --- a/execution/executor/src/tests/mod.rs +++ b/execution/executor/src/tests/mod.rs @@ -690,6 +690,7 @@ fn run_transactions_naive( .unwrap(), block_executor_onchain_config.clone(), None, + None, ) .unwrap(); let output = ApplyExecutionOutput::run(out, &ledger_view).unwrap(); diff --git a/execution/executor/src/workflow/do_get_execution_output.rs b/execution/executor/src/workflow/do_get_execution_output.rs index 4fad63d2e8261..5e7987b95098f 100644 --- a/execution/executor/src/workflow/do_get_execution_output.rs +++ b/execution/executor/src/workflow/do_get_execution_output.rs @@ -49,7 +49,8 @@ impl DoGetExecutionOutput { transactions: ExecutableTransactions, state_view: CachedStateView, onchain_config: BlockExecutorConfigFromOnchain, - append_state_checkpoint_to_block: Option, + parent_block: Option<&HashValue>, + current_block: Option, ) -> Result { let out = match transactions { ExecutableTransactions::Unsharded(txns) => { @@ -58,14 +59,15 @@ impl DoGetExecutionOutput { txns, state_view, onchain_config, - append_state_checkpoint_to_block, + parent_block, + current_block, )? }, ExecutableTransactions::Sharded(txns) => Self::by_transaction_execution_sharded::( txns, state_view, onchain_config, - append_state_checkpoint_to_block, + current_block, )?, }; @@ -89,10 +91,17 @@ impl DoGetExecutionOutput { transactions: Vec, state_view: CachedStateView, onchain_config: BlockExecutorConfigFromOnchain, - append_state_checkpoint_to_block: Option, + parent_block: Option<&HashValue>, + current_block: Option, ) -> Result { - let block_output = - Self::execute_block::(executor, &transactions, &state_view, onchain_config)?; + let block_output = Self::execute_block::( + executor, + &transactions, + &state_view, + onchain_config, + parent_block, + current_block, + )?; let (transaction_outputs, block_end_info) = block_output.into_inner(); Parser::parse( @@ -101,7 +110,7 @@ impl DoGetExecutionOutput { transaction_outputs, state_view.into_state_cache(), block_end_info, - append_state_checkpoint_to_block, + current_block, ) } @@ -202,9 +211,17 @@ impl DoGetExecutionOutput { transactions: &[SignatureVerifiedTransaction], state_view: &CachedStateView, onchain_config: BlockExecutorConfigFromOnchain, + parent_block: Option<&HashValue>, + current_block: Option, ) -> Result> { let _timer = OTHER_TIMERS.timer_with(&["vm_execute_block"]); - Ok(executor.execute_block(transactions, state_view, onchain_config)?) + Ok(executor.execute_block( + transactions, + state_view, + onchain_config, + parent_block, + current_block, + )?) } /// In consensus-only mode, executes the block of [Transaction]s using the @@ -217,6 +234,8 @@ impl DoGetExecutionOutput { transactions: &[SignatureVerifiedTransaction], state_view: &CachedStateView, onchain_config: BlockExecutorConfigFromOnchain, + parent_block: Option<&HashValue>, + current_block: Option, ) -> Result> { use aptos_types::{ state_store::{StateViewId, TStateView}, @@ -226,9 +245,13 @@ impl DoGetExecutionOutput { let transaction_outputs = match state_view.id() { // this state view ID implies a genesis block in non-test cases. - StateViewId::Miscellaneous => { - executor.execute_block(transactions, state_view, onchain_config)? - }, + StateViewId::Miscellaneous => executor.execute_block( + transactions, + state_view, + onchain_config, + parent_block, + current_block, + )?, _ => BlockOutput::new( transactions .iter() diff --git a/experimental/execution/ptx-executor/Cargo.toml b/experimental/execution/ptx-executor/Cargo.toml index 0da896d31500a..4ed757a8bba81 100644 --- a/experimental/execution/ptx-executor/Cargo.toml +++ b/experimental/execution/ptx-executor/Cargo.toml @@ -13,6 +13,7 @@ repository = { workspace = true } rust-version = { workspace = true } [dependencies] +aptos-crypto = { workspace = true } aptos-experimental-runtimes = { workspace = true } aptos-infallible = { workspace = true } aptos-logger = { workspace = true } diff --git a/experimental/execution/ptx-executor/src/lib.rs b/experimental/execution/ptx-executor/src/lib.rs index 9ed83b7d7a3f6..649c0651af63a 100644 --- a/experimental/execution/ptx-executor/src/lib.rs +++ b/experimental/execution/ptx-executor/src/lib.rs @@ -21,6 +21,7 @@ use crate::{ analyzer::PtxAnalyzer, finalizer::PtxFinalizer, metrics::TIMER, runner::PtxRunner, scheduler::PtxScheduler, sorter::PtxSorter, state_reader::PtxStateReader, }; +use aptos_crypto::HashValue; use aptos_experimental_runtimes::thread_manager::THREAD_MANAGER; use aptos_infallible::Mutex; use aptos_metrics_core::TimerHelper; @@ -53,6 +54,8 @@ impl VMBlockExecutor for PtxBlockExecutor { transactions: &[SignatureVerifiedTransaction], state_view: &(impl StateView + Sync), _onchain_config: BlockExecutorConfigFromOnchain, + _parent_block: Option<&HashValue>, + _current_block: Option, ) -> Result, VMStatus> { let _timer = TIMER.timer_with(&["block_total"]); diff --git a/storage/db-tool/src/replay_on_archive.rs b/storage/db-tool/src/replay_on_archive.rs index 5bc13308bee98..a5df5787a20f2 100644 --- a/storage/db-tool/src/replay_on_archive.rs +++ b/storage/db-tool/src/replay_on_archive.rs @@ -300,6 +300,8 @@ impl Verifier { &self .arc_db .state_view_at_version(start_version.checked_sub(1))?, + None, + None, )?; let mut failed_txns = Vec::new(); 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 62d885c36a96a..bc152dd1cc06b 100644 --- a/third_party/move/move-vm/runtime/src/storage/environment.rs +++ b/third_party/move/move-vm/runtime/src/storage/environment.rs @@ -25,7 +25,10 @@ use move_core_types::{ identifier::{IdentStr, Identifier}, vm_status::{sub_status::unknown_invariant_violation::EPARANOID_FAILURE, StatusCode}, }; -use move_vm_types::sha3_256; +use move_vm_types::{ + loaded_data::runtime_types::{StructIdentifier, StructNameIndex}, + sha3_256, +}; use std::sync::Arc; /// [MoveVM] runtime environment encapsulating different configurations. Shared between the VM and @@ -284,6 +287,15 @@ impl RuntimeEnvironment { self.flush_struct_info_cache(); self.struct_name_index_map.flush(); } + + /// Test-only function to be able to populate [StructNameIndexMap] outside of this crate. + #[cfg(any(test, feature = "testing"))] + pub fn struct_name_to_idx_for_test( + &self, + struct_name: StructIdentifier, + ) -> PartialVMResult { + self.struct_name_index_map.struct_name_to_idx(struct_name) + } } impl Clone for RuntimeEnvironment {