Skip to content

Commit

Permalink
[compiler-v2] Test case reduced from move-stdlib showing opportunity …
Browse files Browse the repository at this point in the history
…for optimization (#15338)
  • Loading branch information
vineethk authored and msmouse committed Nov 25, 2024
1 parent e2359fd commit 6137e8f
Show file tree
Hide file tree
Showing 83 changed files with 517 additions and 312 deletions.
2 changes: 0 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion api/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ use aptos_gas_schedule::{AptosGasParameters, FromOnChainGasSchedule};
use aptos_logger::{error, info, Schema};
use aptos_mempool::{MempoolClientRequest, MempoolClientSender, SubmissionStatus};
use aptos_storage_interface::{
state_view::{DbStateView, DbStateViewAtVersion, LatestDbStateCheckpointView},
state_store::state_view::db_state_view::{
DbStateView, DbStateViewAtVersion, LatestDbStateCheckpointView,
},
AptosDbError, DbReader, Order, MAX_REQUEST_LIMIT,
};
use aptos_types::{
Expand Down
4 changes: 3 additions & 1 deletion api/test-context/src/test_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@ use aptos_sdk::{
transaction::SignedTransaction, AccountKey, LocalAccount,
},
};
use aptos_storage_interface::{state_view::DbStateView, DbReaderWriter};
use aptos_storage_interface::{
state_store::state_view::db_state_view::DbStateView, DbReaderWriter,
};
use aptos_temppath::TempPath;
use aptos_types::{
account_address::{create_multisig_account_address, AccountAddress},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use aptos_language_e2e_tests::data_store::FakeDataStore;
use aptos_types::{
state_store::{
state_key::StateKey, state_storage_usage::StateStorageUsage, state_value::StateValue,
Result as StateViewResult, TStateView,
StateViewResult, TStateView,
},
transaction::Version,
};
Expand Down
9 changes: 2 additions & 7 deletions aptos-move/aptos-release-builder/src/simulate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,8 @@ use aptos_types::{
account_config::ChainIdResource,
on_chain_config::{ApprovedExecutionHashes, Features, GasScheduleV2, OnChainConfig},
state_store::{
in_memory_state_view::InMemoryStateView, state_key::StateKey,
state_storage_usage::StateStorageUsage, state_value::StateValue,
Result as StateStoreResult, StateView, TStateView,
state_key::StateKey, state_storage_usage::StateStorageUsage, state_value::StateValue,
StateView, StateViewResult as StateStoreResult, TStateView,
},
transaction::{ExecutionStatus, Script, TransactionArgument, TransactionStatus},
write_set::{TransactionWrite, WriteSet},
Expand Down Expand Up @@ -306,10 +305,6 @@ where
fn get_usage(&self) -> StateStoreResult<StateStorageUsage> {
Ok(StateStorageUsage::Untracked)
}

fn as_in_memory_state_view(&self) -> InMemoryStateView {
panic!("not supported")
}
}

/***************************************************************************************************
Expand Down
2 changes: 1 addition & 1 deletion aptos-move/aptos-validator-interface/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use aptos_types::{
account_address::AccountAddress,
state_store::{
state_key::StateKey, state_storage_usage::StateStorageUsage, state_value::StateValue,
Result as StateViewResult, StateViewId, TStateView,
StateViewId, StateViewResult, TStateView,
},
transaction::{Transaction, TransactionInfo, Version},
};
Expand Down
10 changes: 5 additions & 5 deletions aptos-move/aptos-vm-types/src/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use aptos_aggregator::resolver::{TAggregatorV1View, TDelayedFieldView};
use aptos_types::{
serde_helper::bcs_utils::size_u32_as_uleb128,
state_store::{
errors::StateviewError,
errors::StateViewError,
state_key::StateKey,
state_storage_usage::StateStorageUsage,
state_value::{StateValue, StateValueMetadata},
Expand Down Expand Up @@ -181,9 +181,9 @@ pub trait StateStorageView {
fn id(&self) -> StateViewId;

/// Reads the state value from the DB. Used to enforce read-before-write for module writes.
fn read_state_value(&self, state_key: &Self::Key) -> Result<(), StateviewError>;
fn read_state_value(&self, state_key: &Self::Key) -> Result<(), StateViewError>;

fn get_usage(&self) -> Result<StateStorageUsage, StateviewError>;
fn get_usage(&self) -> Result<StateStorageUsage, StateViewError>;
}

/// A fine-grained view of the state during execution.
Expand Down Expand Up @@ -289,12 +289,12 @@ where
self.id()
}

fn read_state_value(&self, state_key: &Self::Key) -> Result<(), StateviewError> {
fn read_state_value(&self, state_key: &Self::Key) -> Result<(), StateViewError> {
self.get_state_value(state_key)?;
Ok(())
}

fn get_usage(&self) -> Result<StateStorageUsage, StateviewError> {
fn get_usage(&self) -> Result<StateStorageUsage, StateViewError> {
self.get_usage().map_err(Into::into)
}
}
Expand Down
6 changes: 3 additions & 3 deletions aptos-move/aptos-vm-types/src/resource_group_adapter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,7 @@ mod tests {
use super::*;
use crate::tests::utils::{mock_tag_0, mock_tag_1, mock_tag_2};
use aptos_types::state_store::{
errors::StateviewError, state_storage_usage::StateStorageUsage, state_value::StateValue,
errors::StateViewError, state_storage_usage::StateStorageUsage, state_value::StateValue,
TStateView,
};
use claims::{assert_gt, assert_none, assert_ok_eq, assert_some, assert_some_eq};
Expand Down Expand Up @@ -443,14 +443,14 @@ mod tests {
fn get_state_value(
&self,
state_key: &Self::Key,
) -> Result<Option<StateValue>, StateviewError> {
) -> Result<Option<StateValue>, StateViewError> {
Ok(self
.group
.get(state_key)
.map(|entry| StateValue::new_legacy(entry.blob.clone().into())))
}

fn get_usage(&self) -> Result<StateStorageUsage, StateviewError> {
fn get_usage(&self) -> Result<StateStorageUsage, StateViewError> {
unimplemented!();
}
}
Expand Down
6 changes: 3 additions & 3 deletions aptos-move/aptos-vm/src/data_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use aptos_types::{
error::{PanicError, PanicOr},
on_chain_config::{ConfigStorage, Features, OnChainConfig},
state_store::{
errors::StateviewError,
errors::StateViewError,
state_key::StateKey,
state_storage_usage::StateStorageUsage,
state_value::{StateValue, StateValueMetadata},
Expand Down Expand Up @@ -318,11 +318,11 @@ impl<'e, E: ExecutorView> StateStorageView for StorageAdapter<'e, E> {
self.executor_view.id()
}

fn read_state_value(&self, state_key: &Self::Key) -> Result<(), StateviewError> {
fn read_state_value(&self, state_key: &Self::Key) -> Result<(), StateViewError> {
self.executor_view.read_state_value(state_key)
}

fn get_usage(&self) -> Result<StateStorageUsage, StateviewError> {
fn get_usage(&self) -> Result<StateStorageUsage, StateViewError> {
self.executor_view.get_usage()
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use aptos_aggregator::{
use aptos_types::{
error::{code_invariant_error, expect_ok, PanicError, PanicOr},
state_store::{
errors::StateviewError,
errors::StateViewError,
state_key::StateKey,
state_storage_usage::StateStorageUsage,
state_value::{StateValue, StateValueMetadata},
Expand Down Expand Up @@ -324,12 +324,12 @@ impl<'r> StateStorageView for ExecutorViewWithChangeSet<'r> {
self.base_executor_view.id()
}

fn read_state_value(&self, state_key: &Self::Key) -> Result<(), StateviewError> {
fn read_state_value(&self, state_key: &Self::Key) -> Result<(), StateViewError> {
self.base_executor_view.read_state_value(state_key)
}

fn get_usage(&self) -> Result<StateStorageUsage, StateviewError> {
Err(StateviewError::Other(
fn get_usage(&self) -> Result<StateStorageUsage, StateViewError> {
Err(StateViewError::Other(
"Unexpected access to get_usage()".to_string(),
))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@

use aptos_types::{
state_store::{
errors::StateviewError, state_key::StateKey, state_storage_usage::StateStorageUsage,
errors::StateViewError, state_key::StateKey, state_storage_usage::StateStorageUsage,
state_value::StateValue, StateView, TStateView,
},
write_set::TOTAL_SUPPLY_STATE_KEY,
};

type Result<T, E = StateviewError> = std::result::Result<T, E>;
type Result<T, E = StateViewError> = std::result::Result<T, E>;

pub const TOTAL_SUPPLY_AGGR_BASE_VAL: u128 = u128::MAX >> 1;
#[derive(Clone)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use aptos_logger::trace;
use aptos_types::{
block_executor::partitioner::TransactionWithDependencies,
state_store::{
errors::StateviewError, state_key::StateKey, state_storage_usage::StateStorageUsage,
errors::StateViewError, state_key::StateKey, state_storage_usage::StateStorageUsage,
state_value::StateValue, StateView, TStateView,
},
transaction::analyzed_transaction::AnalyzedTransaction,
Expand Down Expand Up @@ -74,14 +74,14 @@ impl<'a, S: StateView + Sync + Send> CrossShardStateView<'a, S> {
impl<'a, S: StateView + Sync + Send> TStateView for CrossShardStateView<'a, S> {
type Key = StateKey;

fn get_state_value(&self, state_key: &StateKey) -> Result<Option<StateValue>, StateviewError> {
fn get_state_value(&self, state_key: &StateKey) -> Result<Option<StateValue>, StateViewError> {
if let Some(value) = self.cross_shard_data.get(state_key) {
return Ok(value.get_value());
}
self.base_view.get_state_value(state_key)
}

fn get_usage(&self) -> Result<StateStorageUsage, StateviewError> {
fn get_usage(&self) -> Result<StateStorageUsage, StateViewError> {
Ok(StateStorageUsage::new_untracked())
}
}
Expand All @@ -90,19 +90,27 @@ impl<'a, S: StateView + Sync + Send> TStateView for CrossShardStateView<'a, S> {
mod tests {
use crate::sharded_block_executor::cross_shard_state_view::CrossShardStateView;
use aptos_types::state_store::{
in_memory_state_view::InMemoryStateView, state_key::StateKey, state_value::StateValue,
TStateView,
};
use once_cell::sync::Lazy;
use std::{
collections::{HashMap, HashSet},
sync::Arc,
thread,
time::Duration,
errors::StateViewError, state_key::StateKey, state_storage_usage::StateStorageUsage,
state_value::StateValue, TStateView,
};
use std::{collections::HashSet, sync::Arc, thread, time::Duration};

struct EmptyView;

pub static EMPTY_VIEW: Lazy<Arc<InMemoryStateView>> =
Lazy::new(|| Arc::new(InMemoryStateView::new(HashMap::new())));
impl TStateView for EmptyView {
type Key = StateKey;

fn get_state_value(
&self,
_state_key: &StateKey,
) -> Result<Option<StateValue>, StateViewError> {
Ok(None)
}

fn get_usage(&self) -> Result<StateStorageUsage, StateViewError> {
unreachable!()
}
}

#[test]
fn test_cross_shard_state_view_get_state_value() {
Expand All @@ -114,7 +122,7 @@ mod tests {
let mut state_keys = HashSet::new();
state_keys.insert(state_key.clone());

let cross_shard_state_view = Arc::new(CrossShardStateView::new(state_keys, &EMPTY_VIEW));
let cross_shard_state_view = Arc::new(CrossShardStateView::new(state_keys, &EmptyView));
let cross_shard_state_view_clone = cross_shard_state_view.clone();

let wait_thread = thread::spawn(move || {
Expand Down
10 changes: 5 additions & 5 deletions aptos-move/block-executor/src/proptest_types/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use aptos_types::{
fee_statement::FeeStatement,
on_chain_config::CurrentTimeMicroseconds,
state_store::{
errors::StateviewError,
errors::StateViewError,
state_storage_usage::StateStorageUsage,
state_value::{StateValue, StateValueMetadata},
StateViewId, TStateView,
Expand Down Expand Up @@ -73,7 +73,7 @@ where
type Key = K;

// Contains mock storage value with STORAGE_AGGREGATOR_VALUE.
fn get_state_value(&self, _: &K) -> Result<Option<StateValue>, StateviewError> {
fn get_state_value(&self, _: &K) -> Result<Option<StateValue>, StateViewError> {
Ok(Some(StateValue::new_legacy(
serialize(&STORAGE_AGGREGATOR_VALUE).into(),
)))
Expand All @@ -83,7 +83,7 @@ where
StateViewId::Miscellaneous
}

fn get_usage(&self) -> Result<StateStorageUsage, StateviewError> {
fn get_usage(&self) -> Result<StateStorageUsage, StateViewError> {
unreachable!("Not used in tests");
}
}
Expand All @@ -99,7 +99,7 @@ where
type Key = K;

// Contains mock storage value with a non-empty group (w. value at RESERVED_TAG).
fn get_state_value(&self, key: &K) -> Result<Option<StateValue>, StateviewError> {
fn get_state_value(&self, key: &K) -> Result<Option<StateValue>, StateViewError> {
if self.group_keys.contains(key) {
let group: BTreeMap<u32, Bytes> = BTreeMap::from([(RESERVED_TAG, vec![0].into())]);

Expand All @@ -117,7 +117,7 @@ where
StateViewId::Miscellaneous
}

fn get_usage(&self) -> Result<StateStorageUsage, StateviewError> {
fn get_usage(&self) -> Result<StateStorageUsage, StateViewError> {
unreachable!("Not used in tests");
}
}
Expand Down
6 changes: 3 additions & 3 deletions aptos-move/block-executor/src/view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ use aptos_types::{
error::{code_invariant_error, expect_ok, PanicError, PanicOr},
executable::{Executable, ModulePath},
state_store::{
errors::StateviewError,
errors::StateViewError,
state_storage_usage::StateStorageUsage,
state_value::{StateValue, StateValueMetadata},
StateViewId, TStateView,
Expand Down Expand Up @@ -1628,12 +1628,12 @@ impl<'a, T: Transaction, S: TStateView<Key = T::Key>, X: Executable> StateStorag
self.base_view.id()
}

fn read_state_value(&self, state_key: &Self::Key) -> Result<(), StateviewError> {
fn read_state_value(&self, state_key: &Self::Key) -> Result<(), StateViewError> {
self.base_view.get_state_value(state_key)?;
Ok(())
}

fn get_usage(&self) -> Result<StateStorageUsage, StateviewError> {
fn get_usage(&self) -> Result<StateStorageUsage, StateViewError> {
self.base_view.get_usage()
}
}
Expand Down
12 changes: 4 additions & 8 deletions aptos-move/e2e-tests/src/data_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ use aptos_types::{
chain_id::ChainId,
on_chain_config::{Features, OnChainConfig},
state_store::{
errors::StateviewError, in_memory_state_view::InMemoryStateView, state_key::StateKey,
state_storage_usage::StateStorageUsage, state_value::StateValue, TStateView,
errors::StateViewError, state_key::StateKey, state_storage_usage::StateStorageUsage,
state_value::StateValue, TStateView,
},
transaction::ChangeSet,
write_set::{TransactionWrite, WriteSet},
Expand Down Expand Up @@ -143,21 +143,17 @@ impl FakeDataStore {
impl TStateView for FakeDataStore {
type Key = StateKey;

fn get_state_value(&self, state_key: &StateKey) -> Result<Option<StateValue>, StateviewError> {
fn get_state_value(&self, state_key: &StateKey) -> Result<Option<StateValue>, StateViewError> {
Ok(self.state_data.get(state_key).cloned())
}

fn get_usage(&self) -> Result<StateStorageUsage, StateviewError> {
fn get_usage(&self) -> Result<StateStorageUsage, StateViewError> {
let mut usage = StateStorageUsage::new_untracked();
for (k, v) in self.state_data.iter() {
usage.add_item(k.size() + v.size())
}
Ok(usage)
}

fn as_in_memory_state_view(&self) -> InMemoryStateView {
InMemoryStateView::new(self.state_data.clone())
}
}

#[cfg(test)]
Expand Down
Loading

0 comments on commit 6137e8f

Please sign in to comment.