Skip to content

Commit

Permalink
counters
Browse files Browse the repository at this point in the history
  • Loading branch information
georgemitenkov committed Nov 19, 2024
1 parent c8baef0 commit 2b41a15
Show file tree
Hide file tree
Showing 8 changed files with 32 additions and 26 deletions.
4 changes: 2 additions & 2 deletions aptos-move/block-executor/src/captured_reads.rs
Original file line number Diff line number Diff line change
Expand Up @@ -661,7 +661,7 @@ where
}

self.module_reads.iter().all(|(key, read)| match read {
ModuleRead::GlobalCache(_) => global_module_cache.is_not_overridden(key),
ModuleRead::GlobalCache(_) => global_module_cache.contains_not_overridden(key),
ModuleRead::PerBlockCache(previous) => {
let current_version = per_block_module_cache.get_module_version(key);
let previous_version = previous.as_ref().map(|(_, version)| *version);
Expand Down Expand Up @@ -1713,6 +1713,6 @@ mod test {
// Assume we re-read the new correct version. Then validation should pass again.
captured_reads.capture_per_block_cache_read(0, Some((a, Some(10))));
assert!(captured_reads.validate_module_reads(&global_module_cache, &per_block_module_cache));
assert!(!global_module_cache.is_not_overridden(&0));
assert!(!global_module_cache.contains_not_overridden(&0));
}
}
8 changes: 3 additions & 5 deletions aptos-move/block-executor/src/code_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

use crate::{
captured_reads::CacheRead,
counters::GLOBAL_MODULE_CACHE_NUM_MISSES_PER_BLOCK,
counters::GLOBAL_MODULE_CACHE_MISS_SECONDS,
view::{LatestView, ViewState},
};
use ambassador::delegate_to_methods;
Expand Down Expand Up @@ -153,9 +153,8 @@ impl<'a, T: Transaction, S: TStateView<Key = T::Key>, X: Executable> ModuleCache
return Ok(Some((module, Self::Version::default())));
}

GLOBAL_MODULE_CACHE_NUM_MISSES_PER_BLOCK.inc();

// If not global cache, check per-block cache.
let _timer = GLOBAL_MODULE_CACHE_MISS_SECONDS.start_timer();
let read = state
.versioned_map
.module_cache()
Expand All @@ -172,8 +171,7 @@ impl<'a, T: Transaction, S: TStateView<Key = T::Key>, X: Executable> ModuleCache
return Ok(Some((module, Self::Version::default())));
}

GLOBAL_MODULE_CACHE_NUM_MISSES_PER_BLOCK.inc();

let _timer = GLOBAL_MODULE_CACHE_MISS_SECONDS.start_timer();
let read = state
.unsync_map
.module_cache()
Expand Down
8 changes: 4 additions & 4 deletions aptos-move/block-executor/src/code_cache_global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ where
}

/// Returns true if the key exists in cache and the corresponding module is not overridden.
pub fn is_not_overridden(&self, key: &K) -> bool {
pub fn contains_not_overridden(&self, key: &K) -> bool {
self.module_cache
.get(key)
.is_some_and(|entry| entry.is_not_overridden())
Expand Down Expand Up @@ -214,9 +214,9 @@ mod test {

assert_eq!(cache.num_modules(), 2);

assert!(cache.is_not_overridden(&0));
assert!(!cache.is_not_overridden(&1));
assert!(!cache.is_not_overridden(&3));
assert!(cache.contains_not_overridden(&0));
assert!(!cache.contains_not_overridden(&1));
assert!(!cache.contains_not_overridden(&3));

assert!(cache.get(&0).is_some());
assert!(cache.get(&1).is_none());
Expand Down
14 changes: 8 additions & 6 deletions aptos-move/block-executor/src/counters.rs
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ pub static EFFECTIVE_BLOCK_GAS: Lazy<HistogramVec> = Lazy::new(|| {
pub static APPROX_BLOCK_OUTPUT_SIZE: Lazy<HistogramVec> = Lazy::new(|| {
register_histogram_vec!(
"aptos_execution_approx_block_output_size",
"Historgram for different approx block output sizes - used for evaluting block ouptut limit.",
"Histogram for different approx block output sizes - used for evaluating block output limit.",
&["mode"],
output_buckets(),
)
Expand Down Expand Up @@ -350,11 +350,13 @@ pub static GLOBAL_MODULE_CACHE_NUM_MODULES: Lazy<IntGauge> = Lazy::new(|| {
.unwrap()
});

/// Count of global module cache misses per-block.
pub static GLOBAL_MODULE_CACHE_NUM_MISSES_PER_BLOCK: Lazy<IntCounter> = Lazy::new(|| {
register_int_counter!(
"global_module_cache_num_misses_per_block",
"Number of global module cache misses in parallel execution (per-block)"
pub static GLOBAL_MODULE_CACHE_MISS_SECONDS: Lazy<Histogram> = Lazy::new(|| {
register_histogram!(
// metric name
"global_module_cache_miss_modules",
// metric description
"The time spent in seconds after global module cache miss to access per-block module cache",
time_buckets(),
)
.unwrap()
});
Expand Down
7 changes: 3 additions & 4 deletions aptos-move/block-executor/src/executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ use crate::{
code_cache_global::GlobalModuleCache,
code_cache_global_manager::AptosModuleCacheManagerGuard,
counters::{
self, BLOCK_EXECUTOR_INNER_EXECUTE_BLOCK, GLOBAL_MODULE_CACHE_NUM_MISSES_PER_BLOCK,
PARALLEL_EXECUTION_SECONDS, RAYON_EXECUTION_SECONDS, TASK_EXECUTE_SECONDS,
TASK_VALIDATE_SECONDS, VM_INIT_SECONDS, WORK_WITH_TASK_SECONDS,
self, BLOCK_EXECUTOR_INNER_EXECUTE_BLOCK, PARALLEL_EXECUTION_SECONDS,
RAYON_EXECUTION_SECONDS, TASK_EXECUTE_SECONDS, TASK_VALIDATE_SECONDS, VM_INIT_SECONDS,
WORK_WITH_TASK_SECONDS,
},
errors::*,
executor_utilities::*,
Expand Down Expand Up @@ -1710,7 +1710,6 @@ where
base_view: &S,
module_cache_manager_guard: &mut AptosModuleCacheManagerGuard,
) -> BlockExecutionResult<BlockOutput<E::Output>, E::Error> {
GLOBAL_MODULE_CACHE_NUM_MISSES_PER_BLOCK.reset();
let _timer = BLOCK_EXECUTOR_INNER_EXECUTE_BLOCK.start_timer();

if self.config.local.concurrency_level > 1 {
Expand Down
9 changes: 4 additions & 5 deletions third_party/move/move-vm/runtime/src/loader/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,7 @@ impl Loader {
I: IntoIterator<Item = (&'a AccountAddress, &'a IdentStr)>,
I::IntoIter: DoubleEndedIterator,
{
let _timer = VM_TIMER.timer_with_label("Loader::check_dependencies_and_charge_gas");
match self {
Self::V1(loader) => loader.check_dependencies_and_charge_gas(
module_store,
Expand Down Expand Up @@ -526,7 +527,7 @@ impl LoaderV1 {
data_store: &mut TransactionDataCache,
module_store: &LegacyModuleStorageAdapter,
) -> VMResult<Arc<CompiledScript>> {
let _timer = VM_TIMER.timer_with_label("Loader::deserialize_and_verify_script");
let _timer = VM_TIMER.timer_with_label("LoaderV1::deserialize_and_verify_script");

let script = data_store.load_compiled_script_to_cache(script, hash_value)?;

Expand Down Expand Up @@ -847,8 +848,6 @@ impl LoaderV1 {
I: IntoIterator<Item = (&'a AccountAddress, &'a IdentStr)>,
I::IntoIter: DoubleEndedIterator,
{
let _timer = VM_TIMER.timer_with_label("Loader::check_dependencies_and_charge_gas");

// Initialize the work list (stack) and the map of visited modules.
//
// TODO: Determine the reserved capacity based on the max number of dependencies allowed.
Expand Down Expand Up @@ -918,7 +917,7 @@ impl LoaderV1 {
return Ok(cached);
}

let _timer = VM_TIMER.timer_with_label("Loader::load_module [cache miss]");
let _timer = VM_TIMER.timer_with_label("LoaderV1::load_module [cache miss]");

// otherwise, load the transitive closure of the target module
let module_ref = self.load_and_verify_module_and_dependencies_and_friends(
Expand Down Expand Up @@ -962,7 +961,7 @@ impl LoaderV1 {
// Verify the module if it hasn't been verified before.
if VERIFIED_MODULES.lock().get(&hash_value).is_none() {
let _timer = VM_TIMER
.timer_with_label("Loader::load_and_verify_module [verification cache miss]");
.timer_with_label("LoaderV1::load_and_verify_module [verification cache miss]");

move_bytecode_verifier::verify_module_with_config(
&self.vm_config.verifier_config,
Expand Down
5 changes: 5 additions & 0 deletions third_party/move/move-vm/runtime/src/storage/environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ use move_core_types::{
identifier::{IdentStr, Identifier},
vm_status::{sub_status::unknown_invariant_violation::EPARANOID_FAILURE, StatusCode},
};
use move_vm_metrics::{Timer, VM_TIMER};
#[cfg(any(test, feature = "testing"))]
use move_vm_types::loaded_data::runtime_types::{StructIdentifier, StructNameIndex};
use std::sync::Arc;
Expand Down Expand Up @@ -152,6 +153,10 @@ impl RuntimeEnvironment {
module_hash: &[u8; 32],
) -> VMResult<LocallyVerifiedModule> {
if !VERIFIED_MODULES_V2.contains(module_hash) {
let _timer = VM_TIMER.timer_with_label(
"LoaderV2::build_locally_verified_module [verification cache miss]",
);

// For regular execution, we cache already verified modules. Note that this even caches
// verification for the published modules. This should be ok because as long as the
// hash is the same, the deployed bytecode and any dependencies are the same, and so
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use move_core_types::{
account_address::AccountAddress, identifier::IdentStr, language_storage::ModuleId,
metadata::Metadata,
};
use move_vm_metrics::{Timer, VM_TIMER};
use move_vm_types::{
code::{ModuleCache, ModuleCode, ModuleCodeBuilder, WithBytes, WithHash, WithSize},
module_cyclic_dependency_error, module_linker_error,
Expand Down Expand Up @@ -191,6 +192,8 @@ where
return Ok(Some(module.code().verified().clone()));
}

let _timer = VM_TIMER.timer_with_label("ModuleStorage::fetch_verified_module [cache miss]");

let mut visited = HashSet::new();
visited.insert(id.clone());
Ok(Some(visit_dependencies_and_verify(
Expand Down

0 comments on commit 2b41a15

Please sign in to comment.