Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[benchmarks] Module loading benches (simple) #15255

Closed
wants to merge 25 commits into from
Closed
Changes from 1 commit
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
97188b4
[refactoring] Move explicit sync wrapper to aptos-types
georgemitenkov Nov 5, 2024
d958da3
[refactoring] Move & rename immutabke module cache to aptos-types
georgemitenkov Nov 5, 2024
9f0f976
[global cache] Draft e2e implementation
georgemitenkov Nov 5, 2024
32ebdd3
[testing] Added different test cases for global caches
georgemitenkov Nov 6, 2024
2a65957
[perf] Framework prefetch
georgemitenkov Nov 6, 2024
0fda569
[exp] Avoid repeated arcswap load
georgemitenkov Nov 6, 2024
60ceafb
[cleanup] Add errors, documentation, reset state on panics
georgemitenkov Nov 6, 2024
583ccfa
[refactoring] Remove versions from module code
georgemitenkov Nov 7, 2024
8bee4e8
[comments] Addressing Rati's comments + adding size in bytes to modul…
georgemitenkov Nov 7, 2024
6c40603
[draft] Better states and encapsulation
georgemitenkov Nov 8, 2024
7c2dbf7
prior to addressing comments:
georgemitenkov Nov 9, 2024
03a1ea2
[comments] Address Rati's and Zekun's comments
georgemitenkov Nov 11, 2024
28116bc
[fix] Add done --> ready transition for unit tests
georgemitenkov Nov 11, 2024
d45df1f
Move ready checks to cache manager, better encapsulation by passing h…
georgemitenkov Nov 12, 2024
f317dcf
[cleanup] Set execute_no_limit to always use Nones for block IDs
georgemitenkov Nov 12, 2024
03bb4df
Feature gating
georgemitenkov Nov 12, 2024
715f3a9
[comments] Addressing Igor's comments
georgemitenkov Nov 13, 2024
81e44f8
[congigs] Use 1 GB max size
georgemitenkov Nov 13, 2024
2f3708e
format fix
georgemitenkov Nov 13, 2024
003ecea
[comments] Added a comment about num_modules > 0
georgemitenkov Nov 13, 2024
12292a2
[comments] Use rw lock for module cache, but aquire before block exec…
georgemitenkov Nov 13, 2024
878792e
[benchmarks] Module loading benches (simple)
georgemitenkov Nov 12, 2024
fb126de
[fixes] Fix raw module data & add env vars
georgemitenkov Nov 12, 2024
e36de2d
[fix] Fix & improve script benchmark
georgemitenkov Nov 12, 2024
9fad83d
[hack] Cross-block debugger
georgemitenkov Nov 12, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
[comments] Address Rati's and Zekun's comments
georgemitenkov committed Nov 12, 2024
commit 03a1ea20bd42dd3bdb9c414cb144d52a2a81c1c5
Original file line number Diff line number Diff line change
@@ -5,7 +5,7 @@ use crate::module_and_script_storage::module_storage::AptosModuleStorage;
use ambassador::Delegate;
use aptos_types::{
error::PanicError,
state_store::{state_key::StateKey, state_value::StateValueMetadata, StateView},
state_store::{state_key::StateKey, state_value::StateValueMetadata, StateView, TStateView},
vm::modules::AptosModuleExtension,
};
use bytes::Bytes;
@@ -28,7 +28,7 @@ use move_vm_types::{
code::{ModuleBytesStorage, ModuleCode},
module_storage_error,
};
use std::sync::Arc;
use std::{ops::Deref, sync::Arc};

/// Avoids orphan rule to implement [ModuleBytesStorage] for [StateView].
struct StateViewAdapter<'s, S> {
@@ -48,6 +48,14 @@ impl<'s, S: StateView> ModuleBytesStorage for StateViewAdapter<'s, S> {
}
}

impl<'s, S: StateView> Deref for StateViewAdapter<'s, S> {
type Target = S;

fn deref(&self) -> &Self::Target {
&self.state_view
}
}

/// A (not thread-safe) implementation of code storage on top of a state view. It is never built
/// directly by clients - only via [AsAptosCodeStorage] trait. Can be used to resolve both modules
/// and cached scripts.
@@ -84,7 +92,7 @@ impl<'s, S: StateView, E: WithRuntimeEnvironment> AptosCodeStorageAdapter<'s, S,
}

/// Drains cached verified modules from the code storage, transforming them into format used by
/// global caches. Should only be called when the code storage borrows [StateView].
/// global caches.
pub fn into_verified_module_code_iter(
self,
) -> Result<
@@ -96,47 +104,42 @@ impl<'s, S: StateView, E: WithRuntimeEnvironment> AptosCodeStorageAdapter<'s, S,
>,
PanicError,
> {
let state_view = match self.storage.module_storage().byte_storage().state_view {
BorrowedOrOwned::Borrowed(state_view) => state_view,
BorrowedOrOwned::Owned(_) => {
return Err(PanicError::CodeInvariantError(
"Verified modules should only be extracted from borrowed state".to_string(),
))
},
};

let mut modules_to_add = vec![];
for (key, verified_code) in self
let (state_view, verified_modules_iter) = self
.storage
.into_module_storage()
.into_verified_modules_iter()
{
// We have cached the module previously, so we must be able to find it in storage.
let extension = state_view
.get_state_value(&StateKey::module_id(&key))
.map_err(|err| {
let msg = format!(
"Failed to retrieve module {}::{} from storage {:?}",
key.address(),
key.name(),
err
);
PanicError::CodeInvariantError(msg)
})?
.map(AptosModuleExtension::new)
.ok_or_else(|| {
let msg = format!(
"Module {}::{} should exist, but it does not anymore",
key.address(),
key.name()
);
PanicError::CodeInvariantError(msg)
})?;

let module = ModuleCode::from_verified_ref(verified_code, Arc::new(extension));
modules_to_add.push((key, Arc::new(module)))
}
Ok(modules_to_add.into_iter())
.unpack_into_verified_modules_iter();

Ok(verified_modules_iter
.map(|(key, verified_code)| {
// We have cached the module previously, so we must be able to find it in storage.
let extension = state_view
.get_state_value(&StateKey::module_id(&key))
.map_err(|err| {
let msg = format!(
"Failed to retrieve module {}::{} from storage {:?}",
key.address(),
key.name(),
err
);
PanicError::CodeInvariantError(msg)
})?
.map_or_else(
|| {
let msg = format!(
"Module {}::{} should exist, but it does not anymore",
key.address(),
key.name()
);
Err(PanicError::CodeInvariantError(msg))
},
|state_value| Ok(AptosModuleExtension::new(state_value)),
)?;

let module = ModuleCode::from_arced_verified(verified_code, Arc::new(extension));
Ok((key, Arc::new(module)))
})
.collect::<Result<Vec<_>, PanicError>>()?
.into_iter())
}
}

51 changes: 24 additions & 27 deletions aptos-move/aptos-vm/src/block_executor/mod.rs
Original file line number Diff line number Diff line change
@@ -39,8 +39,11 @@ use aptos_vm_logging::{
alert, flush_speculative_logs, init_speculative_logs, prelude::CRITICAL_ERRORS,
};
use aptos_vm_types::{
abstract_write_op::AbstractResourceWriteOp, module_and_script_storage::AsAptosCodeStorage,
module_write_set::ModuleWrite, output::VMOutput, resolver::ResourceGroupSize,
abstract_write_op::AbstractResourceWriteOp,
module_and_script_storage::{AptosCodeStorageAdapter, AsAptosCodeStorage},
module_write_set::ModuleWrite,
output::VMOutput,
resolver::ResourceGroupSize,
};
use move_binary_format::{
errors::{Location, VMError},
@@ -454,26 +457,23 @@ impl BlockAptosVM {
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_size {
module_cache.flush_unchecked();
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_is_greater_than(module_cache_config.max_module_cache_size_in_bytes)
{
module_cache.flush_unchecked();
if module_cache.size_in_bytes() > module_cache_config.max_module_cache_size_in_bytes {
module_cache.flush_unsync();
}

// 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 {
prefetch_aptos_framework(environment.clone(), state_view, &module_cache).map_err(
|err| {
alert!("Failed to load Aptos framework to module cache: {:?}", err);
VMError::from(err).into_vm_status()
},
)?;
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);
VMError::from(err).into_vm_status()
})?;
}

let executor = BlockExecutor::<
@@ -491,8 +491,6 @@ impl BlockAptosVM {

let ret = executor.execute_block(environment, signature_verified_block, state_view);
if !module_cache_manager.mark_done() {
// Something is wrong as we were not able to mark execution as done. Return an
// error.
return Err(VMStatus::error(
StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR,
Some("Unable to mark block execution as done".to_string()),
@@ -560,13 +558,10 @@ impl BlockAptosVM {
/// If Aptos framework exists, loads "transaction_validation.move" and all its transitive
/// dependencies from storage into provided module cache. If loading fails for any reason, a panic
/// error is returned.
fn prefetch_aptos_framework(
environment: AptosEnvironment,
state_view: &impl StateView,
fn prefetch_aptos_framework<S: StateView>(
code_storage: AptosCodeStorageAdapter<S, AptosEnvironment>,
module_cache: &GlobalModuleCache<ModuleId, CompiledModule, Module, AptosModuleExtension>,
) -> Result<(), PanicError> {
let code_storage = state_view.as_aptos_code_storage(environment);

// If framework code exists in storage, the transitive closure will be verified and cached.
let maybe_loaded = code_storage
.fetch_verified_module(&AccountAddress::ONE, ident_str!("transaction_validation"))
@@ -576,13 +571,11 @@ fn prefetch_aptos_framework(
PanicError::CodeInvariantError(format!("Unable to fetch Aptos framework: {:?}", err))
})?;

if let Some(module) = maybe_loaded {
drop(module);

if maybe_loaded.is_some() {
// Framework must have been loaded. Drain verified modules from local cache into
// global cache.
let verified_module_code_iter = code_storage.into_verified_module_code_iter()?;
module_cache.insert_verified_unchecked(verified_module_code_iter)?;
module_cache.insert_verified_unsync(verified_module_code_iter)?;
}
Ok(())
}
@@ -598,10 +591,12 @@ mod test {
let state_view = executor.get_state_view();

let environment = AptosEnvironment::new_with_delayed_field_optimization_enabled(state_view);
let code_storage = state_view.as_aptos_code_storage(environment);

let module_cache = GlobalModuleCache::empty();
assert_eq!(module_cache.num_modules(), 0);

let result = prefetch_aptos_framework(environment, state_view, &module_cache);
let result = prefetch_aptos_framework(code_storage, &module_cache);
assert!(result.is_ok());
assert!(module_cache.num_modules() > 0);
}
@@ -612,10 +607,12 @@ mod test {

let environment =
AptosEnvironment::new_with_delayed_field_optimization_enabled(&state_view);
let code_storage = state_view.as_aptos_code_storage(environment);

let module_cache = GlobalModuleCache::empty();
assert_eq!(module_cache.num_modules(), 0);

let result = prefetch_aptos_framework(environment, &state_view, &module_cache);
let result = prefetch_aptos_framework(code_storage, &module_cache);
assert!(result.is_ok());
assert_eq!(module_cache.num_modules(), 0);
}
18 changes: 9 additions & 9 deletions aptos-move/aptos-vm/tests/sharded_block_executor.rs
Original file line number Diff line number Diff line change
@@ -308,11 +308,11 @@ mod test_utils {
.into_iter()
.map(|t| t.into_txn())
.collect();
let executor = AptosVMBlockExecutor::new();
if let Some(module_cache_manager) = executor.module_cache_manager() {
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 = executor
let unsharded_txn_output = block_executor
.execute_block_no_limit(&ordered_txns, executor.data_store())
.unwrap();
compare_txn_outputs(unsharded_txn_output, sharded_txn_output);
@@ -362,11 +362,11 @@ mod test_utils {
)
.unwrap();

let executor = AptosVMBlockExecutor::new();
if let Some(module_cache_manager) = executor.module_cache_manager() {
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 = executor
let unsharded_txn_output = block_executor
.execute_block_no_limit(&execution_ordered_txns, executor.data_store())
.unwrap();
compare_txn_outputs(unsharded_txn_output, sharded_txn_output);
@@ -420,11 +420,11 @@ mod test_utils {
)
.unwrap();

let executor = AptosVMBlockExecutor::new();
if let Some(module_cache_manager) = executor.module_cache_manager() {
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 = executor
let unsharded_txn_output = block_executor
.execute_block_no_limit(&execution_ordered_txns, executor.data_store())
.unwrap();
compare_txn_outputs(unsharded_txn_output, sharded_txn_output);
10 changes: 5 additions & 5 deletions aptos-move/block-executor/src/captured_reads.rs
Original file line number Diff line number Diff line change
@@ -295,11 +295,11 @@ impl DelayedFieldRead {
/// Represents a module read, either from immutable cross-block cache, or from code [SyncCodeCache]
/// used by block executor (per-block cache). This way, when transaction needs to read a module
/// from [SyncCodeCache] it can first check the read-set here.
enum ModuleRead<DC, VC, S, V> {
enum ModuleRead<DC, VC, S> {
/// Read from the cross-block module cache.
GlobalCache,
/// Read from per-block cache ([SyncCodeCache]) used by parallel execution.
PerBlockCache(Option<(Arc<ModuleCode<DC, VC, S>>, V)>),
PerBlockCache(Option<(Arc<ModuleCode<DC, VC, S>>, Option<TxnIndex>)>),
}

/// Represents a result of a read from [CapturedReads] when they are used as the transaction-level
@@ -326,7 +326,7 @@ pub(crate) struct CapturedReads<T: Transaction, K, DC, VC, S> {

#[deprecated]
pub(crate) deprecated_module_reads: Vec<T::Key>,
module_reads: hashbrown::HashMap<K, ModuleRead<DC, VC, S, Option<TxnIndex>>>,
module_reads: hashbrown::HashMap<K, ModuleRead<DC, VC, S>>,

/// If there is a speculative failure (e.g. delta application failure, or an observed
/// inconsistency), the transaction output is irrelevant (must be discarded and transaction
@@ -1578,12 +1578,12 @@ mod test {
assert!(!valid);

// Without invalid module (and if it is not captured), validation should pass.
global_module_cache.remove(&1);
assert!(global_module_cache.remove(&1));
captured_reads.module_reads.remove(&1);
assert!(captured_reads.validate_module_reads(&global_module_cache, &per_block_module_cache));

// Validation fails if we captured a cross-block module which does not exist anymore.
global_module_cache.remove(&0);
assert!(global_module_cache.remove(&0));
let valid =
captured_reads.validate_module_reads(&global_module_cache, &per_block_module_cache);
assert!(!valid);
Loading