Skip to content

Commit

Permalink
revert state value validation (perf experiment)
Browse files Browse the repository at this point in the history
  • Loading branch information
georgemitenkov committed Oct 14, 2024
1 parent 18df366 commit d0e08f9
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 37 deletions.
11 changes: 5 additions & 6 deletions aptos-move/block-executor/src/captured_reads.rs
Original file line number Diff line number Diff line change
Expand Up @@ -621,13 +621,12 @@ impl<T: Transaction> CapturedReads<T> {
return false;
}

// TODO(loader_v2):
// Should we also check state value metadata? If so, using transaction indices to decide
// the result of validations might be easier.
self.module_cache_reads.iter().all(|(id, previous_read)| {
let previous_hash_and_state_value_metadata = previous_read
.as_ref()
.map(|e| (e.hash(), e.state_value_metadata()));
code_cache
.module_cache()
.check_hash_and_state_value_metadata(id, previous_hash_and_state_value_metadata)
let previous_hash = previous_read.as_ref().map(|e| e.hash());
code_cache.module_cache().check_hash(id, previous_hash)
})
}

Expand Down
35 changes: 4 additions & 31 deletions aptos-move/mvhashmap/src/code_cache/sync.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
// Copyright © Aptos Foundation
// SPDX-License-Identifier: Apache-2.0

use aptos_types::{
state_store::state_value::StateValueMetadata,
vm::{modules::ModuleCacheEntry, scripts::ScriptCacheEntry},
};
use aptos_types::vm::{modules::ModuleCacheEntry, scripts::ScriptCacheEntry};
use crossbeam::utils::CachePadded;
use dashmap::{mapref::entry::Entry, DashMap};
use hashbrown::HashMap;
Expand Down Expand Up @@ -79,33 +76,9 @@ impl ModuleCache {
/// same state value metadata as previously, or does not contain the cached entry in case it
/// did not contain it before. Used to validate module storage reads when there are modules
/// published.
/// Note that [StateValueMetadata] comparison is required because it is currently possible to
/// publish modules with the same code (e.g., changing a single module as part of a package,
/// but publishing the whole package), and hence the same cache. For those, the state value
/// metadata changes, at least because we no longer have a creation but a modification op.
pub fn check_hash_and_state_value_metadata(
&self,
module_id: &ModuleId,
previous_hash_and_state_value_metadata: Option<(&[u8; 32], &StateValueMetadata)>,
) -> bool {
let current_hash_and_state_value_metadata = self
.cache
.get(module_id)
.map(|e| (*e.hash(), e.state_value_metadata().clone()));
match (
&current_hash_and_state_value_metadata,
previous_hash_and_state_value_metadata,
) {
(
Some((current_hash, current_state_value_metadata)),
Some((previous_hash, previous_state_value_metadata)),
) => {
current_hash == previous_hash
&& current_state_value_metadata == previous_state_value_metadata
},
(None, None) => true,
(Some(..), None) | (None, Some(..)) => false,
}
pub fn check_hash(&self, module_id: &ModuleId, previous_hash: Option<&[u8; 32]>) -> bool {
let current_hash = self.cache.get(module_id).map(|e| *e.hash());
current_hash.as_ref() == previous_hash
}

/// Return the cached module from the module cache. If it is not cached, use the passed
Expand Down

0 comments on commit d0e08f9

Please sign in to comment.