From d20a8690a5944c03b9b0e0659d29585c36e10ff5 Mon Sep 17 00:00:00 2001 From: aldenhu Date: Tue, 5 Nov 2024 23:30:39 +0000 Subject: [PATCH 1/8] remove Hash impl for WriteSet family --- types/src/transaction/change_set.rs | 2 +- types/src/transaction/mod.rs | 2 +- types/src/write_set.rs | 10 ++++------ 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/types/src/transaction/change_set.rs b/types/src/transaction/change_set.rs index 1b269cedde151..184fc2cf09811 100644 --- a/types/src/transaction/change_set.rs +++ b/types/src/transaction/change_set.rs @@ -5,7 +5,7 @@ use crate::{contract_event::ContractEvent, write_set::WriteSet}; use serde::{Deserialize, Serialize}; -#[derive(Clone, Debug, Eq, Hash, PartialEq, Serialize, Deserialize)] +#[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)] pub struct ChangeSet { write_set: WriteSet, events: Vec, diff --git a/types/src/transaction/mod.rs b/types/src/transaction/mod.rs index f0601df007400..b557dfc00e877 100644 --- a/types/src/transaction/mod.rs +++ b/types/src/transaction/mod.rs @@ -433,7 +433,7 @@ impl TransactionPayload { } /// Two different kinds of WriteSet transactions. -#[derive(Clone, Debug, Hash, Eq, PartialEq, Serialize, Deserialize)] +#[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)] pub enum WriteSetPayload { /// Directly passing in the WriteSet. Direct(ChangeSet), diff --git a/types/src/write_set.rs b/types/src/write_set.rs index 159b33e7ba63a..d579f5cc951f7 100644 --- a/types/src/write_set.rs +++ b/types/src/write_set.rs @@ -94,7 +94,7 @@ impl PersistedWriteOp { } } -#[derive(Clone, Eq, Hash, PartialEq)] +#[derive(Clone, Eq, PartialEq)] pub enum WriteOp { Creation { data: Bytes, @@ -442,9 +442,7 @@ impl std::fmt::Debug for WriteOp { } } -#[derive( - BCSCryptoHash, Clone, CryptoHasher, Debug, Eq, Hash, PartialEq, Serialize, Deserialize, -)] +#[derive(BCSCryptoHash, Clone, CryptoHasher, Debug, Eq, PartialEq, Serialize, Deserialize)] pub enum WriteSet { V0(WriteSetV0), } @@ -485,7 +483,7 @@ impl DerefMut for WriteSet { /// where `Value(val)` means that serialized representation should be updated to `val`, and /// `Deletion` means that we are going to delete this access path. #[derive( - BCSCryptoHash, Clone, CryptoHasher, Debug, Default, Eq, Hash, PartialEq, Serialize, Deserialize, + BCSCryptoHash, Clone, CryptoHasher, Debug, Default, Eq, PartialEq, Serialize, Deserialize, )] pub struct WriteSetV0(WriteSetMut); @@ -532,7 +530,7 @@ impl WriteSetV0 { /// A mutable version of `WriteSet`. /// /// This is separate because it goes through validation before becoming an immutable `WriteSet`. -#[derive(Clone, Debug, Default, Eq, Hash, PartialEq, Serialize, Deserialize)] +#[derive(Clone, Debug, Default, Eq, PartialEq, Serialize, Deserialize)] pub struct WriteSetMut { // TODO: Change to HashMap with a stable iterator for serialization. write_set: BTreeMap, From b980060c6c5bd444e351fe63353eb58807a4436f Mon Sep 17 00:00:00 2001 From: aldenhu Date: Tue, 5 Nov 2024 23:08:35 +0000 Subject: [PATCH 2/8] WriteOp carries StateValue --- .../aptos-vm-types/src/abstract_write_op.rs | 4 +- aptos-move/aptos-vm-types/src/change_set.rs | 12 +- .../aptos-vm-types/src/module_write_set.rs | 2 +- .../src/tests/test_change_set.rs | 38 +--- .../src/move_vm_ext/write_op_converter.rs | 22 +- .../src/tests/storage_refund.rs | 2 +- crates/aptos-rosetta/src/test/mod.rs | 8 +- .../src/stream_coordinator.rs | 2 +- testsuite/generate-format/src/api.rs | 4 +- testsuite/generate-format/src/aptos.rs | 9 +- testsuite/generate-format/src/consensus.rs | 9 +- types/src/state_store/state_value.rs | 17 ++ types/src/write_set.rs | 198 ++++++++---------- 13 files changed, 134 insertions(+), 193 deletions(-) diff --git a/aptos-move/aptos-vm-types/src/abstract_write_op.rs b/aptos-move/aptos-vm-types/src/abstract_write_op.rs index c73dc7af2cacd..381228f272940 100644 --- a/aptos-move/aptos-vm-types/src/abstract_write_op.rs +++ b/aptos-move/aptos-vm-types/src/abstract_write_op.rs @@ -111,7 +111,7 @@ impl AbstractResourceWriteOp { /// Deposit amount is inserted into metadata at a different time than the WriteOp is created. /// So this method is needed to be able to update metadata generically across different variants. - pub fn get_metadata_mut(&mut self) -> &mut StateValueMetadata { + pub fn metadata_mut(&mut self) -> &mut StateValueMetadata { use AbstractResourceWriteOp::*; match self { Write(write_op) @@ -119,7 +119,7 @@ impl AbstractResourceWriteOp { | WriteResourceGroup(GroupWrite { metadata_op: write_op, .. - }) => write_op.get_metadata_mut(), + }) => write_op.metadata_mut(), InPlaceDelayedFieldChange(InPlaceDelayedFieldChangeOp { metadata, .. }) | ResourceGroupInPlaceDelayedFieldChange(ResourceGroupInPlaceDelayedFieldChangeOp { metadata, diff --git a/aptos-move/aptos-vm-types/src/change_set.rs b/aptos-move/aptos-vm-types/src/change_set.rs index 5f6029dd2f984..74e609df51abf 100644 --- a/aptos-move/aptos-vm-types/src/change_set.rs +++ b/aptos-move/aptos-vm-types/src/change_set.rs @@ -411,18 +411,18 @@ impl VMChangeSet { if let Some(write_op) = aggregator_v1_write_set.get_mut(&state_key) { // In this case, delta follows a write op. match write_op { - Creation { data, .. } | Modification { data, .. } => { + Creation(v) | Modification(v) => { // Apply delta on top of creation or modification. // TODO[agg_v1](cleanup): This will not be needed anymore once aggregator // change sets carry non-serialized information. - let base: u128 = bcs::from_bytes(data) + let base: u128 = bcs::from_bytes(v.bytes()) .expect("Deserializing into an aggregator value always succeeds"); let value = additional_delta_op .apply_to(base) .map_err(PartialVMError::from)?; - *data = serialize(&value).into(); + v.set_bytes(serialize(&value).into()) }, - Deletion { .. } => { + Deletion(..) => { // This case (applying a delta to deleted item) should // never happen. Let's still return an error instead of // panicking. @@ -883,7 +883,7 @@ impl ChangeSetInterface for VMChangeSet { key, op_size: op.materialized_size(), prev_size: op.prev_materialized_size(key, executor_view)?, - metadata_mut: op.get_metadata_mut(), + metadata_mut: op.metadata_mut(), }) }); let v1_aggregators = self.aggregator_v1_write_set.iter_mut().map(|(key, op)| { @@ -893,7 +893,7 @@ impl ChangeSetInterface for VMChangeSet { prev_size: executor_view .get_aggregator_v1_state_value_size(key)? .unwrap_or(0), - metadata_mut: op.get_metadata_mut(), + metadata_mut: op.metadata_mut(), }) }); diff --git a/aptos-move/aptos-vm-types/src/module_write_set.rs b/aptos-move/aptos-vm-types/src/module_write_set.rs index be236a46a674d..c6d3d634e7b5c 100644 --- a/aptos-move/aptos-vm-types/src/module_write_set.rs +++ b/aptos-move/aptos-vm-types/src/module_write_set.rs @@ -123,7 +123,7 @@ impl ModuleWriteSet { key, op_size: write.write_op().write_op_size(), prev_size, - metadata_mut: write.write_op_mut().get_metadata_mut(), + metadata_mut: write.write_op_mut().metadata_mut(), }) }) } diff --git a/aptos-move/aptos-vm-types/src/tests/test_change_set.rs b/aptos-move/aptos-vm-types/src/tests/test_change_set.rs index 2d887e12e5c80..ff5f0c16512c8 100644 --- a/aptos-move/aptos-vm-types/src/tests/test_change_set.rs +++ b/aptos-move/aptos-vm-types/src/tests/test_change_set.rs @@ -436,10 +436,7 @@ fn test_aggregator_v2_snapshots_and_derived() { #[test] fn test_resource_groups_squashing() { - let modification_metadata = WriteOp::Modification { - data: Bytes::new(), - metadata: raw_metadata(2000), - }; + let modification_metadata = WriteOp::modification(Bytes::new(), raw_metadata(2000)); macro_rules! as_create_op { ($val:expr) => { @@ -593,10 +590,7 @@ fn test_write_and_read_discrepancy_caught() { )]) .try_build()); - let metadata_op = WriteOp::Modification { - data: Bytes::new(), - metadata: raw_metadata(1000), - }; + let metadata_op = WriteOp::modification(Bytes::new(), raw_metadata(1000)); let group_size = ResourceGroupSize::Combined { num_tagged_resources: 1, all_tagged_resources_size: 14, @@ -637,17 +631,9 @@ mod tests { pub(crate) fn write_op_with_metadata(type_idx: u8, v: u128) -> WriteOp { match type_idx { - CREATION => WriteOp::Creation { - data: vec![].into(), - metadata: raw_metadata(v as u64), - }, - MODIFICATION => WriteOp::Modification { - data: vec![].into(), - metadata: raw_metadata(v as u64), - }, - DELETION => WriteOp::Deletion { - metadata: raw_metadata(v as u64), - }, + CREATION => WriteOp::creation(vec![].into(), raw_metadata(v as u64)), + MODIFICATION => WriteOp::modification(vec![].into(), raw_metadata(v as u64)), + DELETION => WriteOp::deletion(raw_metadata(v as u64)), _ => unreachable!("Wrong type index for test"), } } @@ -694,9 +680,7 @@ mod tests { None ); assert_group_write_size!( - WriteOp::Deletion { - metadata: raw_metadata(10) - }, + WriteOp::deletion(raw_metadata(10)), ResourceGroupSize::zero_combined(), None ); @@ -729,10 +713,7 @@ mod tests { Some(sizes[0]) ); assert_group_write_size!( - WriteOp::Creation { - data: Bytes::new(), - metadata: raw_metadata(20) - }, + WriteOp::creation(Bytes::new(), raw_metadata(20)), sizes[1], Some(sizes[1]) ); @@ -742,10 +723,7 @@ mod tests { Some(sizes[2]) ); assert_group_write_size!( - WriteOp::Modification { - data: Bytes::new(), - metadata: raw_metadata(30) - }, + WriteOp::modification(Bytes::new(), raw_metadata(30)), sizes[3], Some(sizes[3]) ); diff --git a/aptos-move/aptos-vm/src/move_vm_ext/write_op_converter.rs b/aptos-move/aptos-vm/src/move_vm_ext/write_op_converter.rs index 353a4dd202b15..1d54be49fe16f 100644 --- a/aptos-move/aptos-vm/src/move_vm_ext/write_op_converter.rs +++ b/aptos-move/aptos-vm/src/move_vm_ext/write_op_converter.rs @@ -210,7 +210,6 @@ impl<'r> WriteOpConverter<'r> { legacy_creation_as_modification: bool, ) -> PartialVMResult { use MoveStorageOp::*; - use WriteOp::*; let write_op = match (state_value_metadata, move_storage_op) { (None, Modify(_) | Delete) => { // Possible under speculative execution, returning speculative error waiting for re-execution. @@ -238,18 +237,12 @@ impl<'r> WriteOpConverter<'r> { WriteOp::legacy_creation(data) } }, - Some(metadata) => Creation { - data, - metadata: metadata.clone(), - }, - }, - (Some(metadata), Modify(data)) => { - // Inherit metadata even if the feature flags is turned off, for compatibility. - Modification { data, metadata } + Some(metadata) => WriteOp::creation(data, metadata.clone()), }, + (Some(metadata), Modify(data)) => WriteOp::modification(data, metadata), (Some(metadata), Delete) => { // Inherit metadata even if the feature flags is turned off, for compatibility. - Deletion { metadata } + WriteOp::deletion(metadata) }, }; Ok(write_op) @@ -270,13 +263,10 @@ impl<'r> WriteOpConverter<'r> { match &self.new_slot_metadata { // n.b. Aggregator writes historically did not distinguish Create vs Modify. None => WriteOp::legacy_modification(data), - Some(metadata) => WriteOp::Creation { - data, - metadata: metadata.clone(), - }, + Some(metadata) => WriteOp::creation(data, metadata.clone()), } }, - Some(metadata) => WriteOp::Modification { data, metadata }, + Some(metadata) => WriteOp::modification(data, metadata), }; Ok(op) @@ -623,7 +613,7 @@ mod tests { // Deletion should still contain the metadata - for storage refunds. assert_eq!(group_write.metadata_op().metadata(), &metadata); - assert_eq!(group_write.metadata_op(), &WriteOp::Deletion { metadata }); + assert_eq!(group_write.metadata_op(), &WriteOp::deletion(metadata)); assert_none!(group_write.metadata_op().bytes()); } } diff --git a/aptos-move/e2e-move-tests/src/tests/storage_refund.rs b/aptos-move/e2e-move-tests/src/tests/storage_refund.rs index ff510308a8b6b..15c1e536eed11 100644 --- a/aptos-move/e2e-move-tests/src/tests/storage_refund.rs +++ b/aptos-move/e2e-move-tests/src/tests/storage_refund.rs @@ -159,7 +159,7 @@ fn assert_result( for (_state_key, write_op) in txn_out.write_set() { match write_op { WriteOp::Creation { .. } => creates += 1, - WriteOp::Deletion { metadata } => { + WriteOp::Deletion(metadata) => { if metadata.is_none() { panic!("This test expects all deletions to have metadata") } diff --git a/crates/aptos-rosetta/src/test/mod.rs b/crates/aptos-rosetta/src/test/mod.rs index 173b55935e574..4dc6873870005 100644 --- a/crates/aptos-rosetta/src/test/mod.rs +++ b/crates/aptos-rosetta/src/test/mod.rs @@ -98,10 +98,10 @@ fn resource_group_modification_write_op( ) -> (StateKey, WriteOp) { let encoded = bcs::to_bytes(input).unwrap(); let state_key = StateKey::resource_group(address, resource); - let write_op = WriteOp::Modification { - data: encoded.into(), - metadata: StateValueMetadata::new(0, 0, &CurrentTimeMicroseconds { microseconds: 0 }), - }; + let write_op = WriteOp::modification( + encoded.into(), + StateValueMetadata::new(0, 0, &CurrentTimeMicroseconds { microseconds: 0 }), + ); (state_key, write_op) } diff --git a/ecosystem/indexer-grpc/indexer-grpc-fullnode/src/stream_coordinator.rs b/ecosystem/indexer-grpc/indexer-grpc-fullnode/src/stream_coordinator.rs index ce190dc2d2e4b..fed09104aa89f 100644 --- a/ecosystem/indexer-grpc/indexer-grpc-fullnode/src/stream_coordinator.rs +++ b/ecosystem/indexer-grpc/indexer-grpc-fullnode/src/stream_coordinator.rs @@ -475,7 +475,7 @@ impl IndexerStreamCoordinator { .iter() .map(|(state_key, write_op)| WriteOpSizeInfo { key_bytes: Self::ser_size_u32(state_key), - value_bytes: write_op.size() as u32, + value_bytes: write_op.bytes_size() as u32, }) .collect(), } diff --git a/testsuite/generate-format/src/api.rs b/testsuite/generate-format/src/api.rs index e9290bb7fdd5e..4be4dfcb71592 100644 --- a/testsuite/generate-format/src/api.rs +++ b/testsuite/generate-format/src/api.rs @@ -95,9 +95,7 @@ pub fn get_registry() -> Result { // 1. Record samples for types with custom deserializers. trace_crypto_values(&mut tracer, &mut samples)?; tracer.trace_value(&mut samples, &event::EventKey::random())?; - tracer.trace_value(&mut samples, &write_set::WriteOp::Deletion { - metadata: StateValueMetadata::none(), - })?; + tracer.trace_value(&mut samples, &write_set::WriteOp::legacy_deletion())?; // 2. Trace the main entry point(s) + every enum separately. // stdlib types diff --git a/testsuite/generate-format/src/aptos.rs b/testsuite/generate-format/src/aptos.rs index 0fa55466b8cdc..0c4e5fc70da05 100644 --- a/testsuite/generate-format/src/aptos.rs +++ b/testsuite/generate-format/src/aptos.rs @@ -14,10 +14,7 @@ use aptos_crypto_derive::{BCSCryptoHash, CryptoHasher}; use aptos_types::{ block_metadata_ext::BlockMetadataExt, contract_event, event, - state_store::{ - state_key::StateKey, - state_value::{PersistedStateValueMetadata, StateValueMetadata}, - }, + state_store::{state_key::StateKey, state_value::PersistedStateValueMetadata}, transaction, transaction::block_epilogue::BlockEpiloguePayload, validator_txn::ValidatorTransaction, @@ -90,9 +87,7 @@ pub fn get_registry() -> Result { // 1. Record samples for types with custom deserializers. trace_crypto_values(&mut tracer, &mut samples)?; tracer.trace_value(&mut samples, &event::EventKey::random())?; - tracer.trace_value(&mut samples, &write_set::WriteOp::Deletion { - metadata: StateValueMetadata::none(), - })?; + tracer.trace_value(&mut samples, &write_set::WriteOp::legacy_deletion())?; // 2. Trace the main entry point(s) + every enum separately. tracer.trace_type::(&samples)?; diff --git a/testsuite/generate-format/src/consensus.rs b/testsuite/generate-format/src/consensus.rs index 5ef14eb37c69b..17c079dfcbbfc 100644 --- a/testsuite/generate-format/src/consensus.rs +++ b/testsuite/generate-format/src/consensus.rs @@ -14,10 +14,7 @@ use aptos_crypto_derive::{BCSCryptoHash, CryptoHasher}; use aptos_types::{ block_metadata_ext::BlockMetadataExt, contract_event, event, - state_store::{ - state_key::StateKey, - state_value::{PersistedStateValueMetadata, StateValueMetadata}, - }, + state_store::{state_key::StateKey, state_value::PersistedStateValueMetadata}, transaction, transaction::block_epilogue::BlockEpiloguePayload, validator_txn::ValidatorTransaction, @@ -89,9 +86,7 @@ pub fn get_registry() -> Result { &aptos_consensus_types::block::Block::make_genesis_block(), )?; tracer.trace_value(&mut samples, &event::EventKey::random())?; - tracer.trace_value(&mut samples, &write_set::WriteOp::Deletion { - metadata: StateValueMetadata::none(), - })?; + tracer.trace_value(&mut samples, &write_set::WriteOp::legacy_deletion())?; // 2. Trace the main entry point(s) + every enum separately. tracer.trace_type::(&samples)?; diff --git a/types/src/state_store/state_value.rs b/types/src/state_store/state_value.rs index d69b8b3fc8ec5..5db5a22b088b2 100644 --- a/types/src/state_store/state_value.rs +++ b/types/src/state_store/state_value.rs @@ -280,6 +280,23 @@ impl StateValue { )) } + pub fn into_bytes(self) -> Bytes { + self.inner.data + } + + pub fn set_bytes(&mut self, data: Bytes) { + self.inner.data = data; + self.hash = OnceCell::new(); + } + + pub fn metadata(&self) -> &StateValueMetadata { + &self.inner.metadata + } + + pub fn metadata_mut(&mut self) -> &mut StateValueMetadata { + &mut self.inner.metadata + } + pub fn into_metadata(self) -> StateValueMetadata { self.inner.metadata } diff --git a/types/src/write_set.rs b/types/src/write_set.rs index d579f5cc951f7..41b5435badeb0 100644 --- a/types/src/write_set.rs +++ b/types/src/write_set.rs @@ -5,12 +5,9 @@ //! For each transaction the VM executes, the VM will output a `WriteSet` that contains each access //! path it updates. For each access path, the VM can either give its new value or delete it. -use crate::{ - state_store::{ - state_key::StateKey, - state_value::{PersistedStateValueMetadata, StateValue, StateValueMetadata}, - }, - write_set::WriteOp::{Creation, Deletion, Modification}, +use crate::state_store::{ + state_key::StateKey, + state_value::{PersistedStateValueMetadata, StateValue, StateValueMetadata}, }; use anyhow::{bail, ensure, Result}; use aptos_crypto_derive::{BCSCryptoHash, CryptoHasher}; @@ -68,45 +65,25 @@ impl PersistedWriteOp { use PersistedWriteOp::*; match self { - Creation(data) => WriteOp::Creation { - data, - metadata: StateValueMetadata::none(), - }, - Modification(data) => WriteOp::Modification { - data, - metadata: StateValueMetadata::none(), - }, - Deletion => WriteOp::Deletion { - metadata: StateValueMetadata::none(), - }, - CreationWithMetadata { data, metadata } => WriteOp::Creation { - data, - metadata: metadata.into_in_mem_form(), - }, - ModificationWithMetadata { data, metadata } => WriteOp::Modification { - data, - metadata: metadata.into_in_mem_form(), + Creation(data) => WriteOp::legacy_creation(data), + Modification(data) => WriteOp::legacy_modification(data), + Deletion => WriteOp::legacy_deletion(), + CreationWithMetadata { data, metadata } => { + WriteOp::creation(data, metadata.into_in_mem_form()) }, - DeletionWithMetadata { metadata } => WriteOp::Deletion { - metadata: metadata.into_in_mem_form(), + ModificationWithMetadata { data, metadata } => { + WriteOp::modification(data, metadata.into_in_mem_form()) }, + DeletionWithMetadata { metadata } => WriteOp::deletion(metadata.into_in_mem_form()), } } } #[derive(Clone, Eq, PartialEq)] pub enum WriteOp { - Creation { - data: Bytes, - metadata: StateValueMetadata, - }, - Modification { - data: Bytes, - metadata: StateValueMetadata, - }, - Deletion { - metadata: StateValueMetadata, - }, + Creation(StateValue), + Modification(StateValue), + Deletion(StateValueMetadata), } impl WriteOp { @@ -116,17 +93,17 @@ impl WriteOp { let metadata = self.metadata().clone().into_persistable(); match metadata { None => match self { - WriteOp::Creation { data, .. } => Creation(data.clone()), - WriteOp::Modification { data, .. } => Modification(data.clone()), + WriteOp::Creation(v) => Creation(v.bytes().clone()), + WriteOp::Modification(v) => Modification(v.bytes().clone()), WriteOp::Deletion { .. } => Deletion, }, Some(metadata) => match self { - WriteOp::Creation { data, .. } => CreationWithMetadata { - data: data.clone(), + WriteOp::Creation(v) => CreationWithMetadata { + data: v.bytes().clone(), metadata, }, - WriteOp::Modification { data, .. } => ModificationWithMetadata { - data: data.clone(), + WriteOp::Modification(v) => ModificationWithMetadata { + data: v.bytes().clone(), metadata, }, WriteOp::Deletion { .. } => DeletionWithMetadata { metadata }, @@ -147,43 +124,32 @@ impl WriteOp { => { bail!("The given change sets cannot be squashed") }, - (Creation {metadata: old_meta, .. } , Modification {data, metadata}) => { - Self::ensure_metadata_compatible(old_meta, &metadata)?; + (Creation(c) , Modification(m)) => { + Self::ensure_metadata_compatible(c.metadata(), m.metadata())?; - *op = Creation { - data, - metadata, - }; + *op = Creation(m) }, - (Modification{metadata: old_meta, .. } , Modification {data, metadata}) => { - Self::ensure_metadata_compatible(old_meta, &metadata)?; + (Modification(c) , Modification(m)) => { + Self::ensure_metadata_compatible(c.metadata(), m.metadata())?; - *op = Modification { - data, - metadata, - }; + *op = Modification(m); }, - (Modification {metadata: old_meta, ..}, Deletion {metadata}) => { - Self::ensure_metadata_compatible(old_meta, &metadata)?; + (Modification(m), Deletion(d_meta)) => { + Self::ensure_metadata_compatible(m.metadata(), &d_meta)?; - *op = Deletion { - metadata, - } + *op = Deletion(d_meta) }, - (Deletion {metadata}, Creation {data, ..}) => { + (Deletion(d_meta), Creation(c)) => { // n.b. With write sets from multiple sessions being squashed together, it's possible // to see two ops carrying different metadata (or one with it the other without) // due to deleting in one session and recreating in another. The original metadata // shouldn't change due to the squash. // And because the deposit or refund happens after all squashing is finished, it's // not a concern of fairness. - *op = Modification { - data, - metadata: metadata.clone(), - } + *op = Modification(StateValue::new_with_metadata(c.into_bytes(), d_meta.clone())) }, - (Creation { metadata: old_meta, .. }, Deletion { metadata }) => { - Self::ensure_metadata_compatible(old_meta, &metadata)?; + (Creation(c), Deletion(d_meta)) => { + Self::ensure_metadata_compatible(c.metadata(), &d_meta)?; return Ok(false) }, @@ -204,70 +170,73 @@ impl WriteOp { Ok(()) } - pub fn bytes(&self) -> Option<&Bytes> { + pub fn state_value_ref(&self) -> Option<&StateValue> { use WriteOp::*; match self { - Creation { data, .. } | Modification { data, .. } => Some(data), - Deletion { .. } => None, + Creation(v) | Modification(v) => Some(v), + Deletion(..) => None, } } - pub fn size(&self) -> usize { - use WriteOp::*; + pub fn bytes(&self) -> Option<&Bytes> { + self.state_value_ref().map(StateValue::bytes) + } - match self { - Creation { data, .. } | Modification { data, .. } => data.len(), - Deletion { .. } => 0, - } + /// Size not counting metadata. + pub fn bytes_size(&self) -> usize { + self.bytes().map_or(0, Bytes::len) } pub fn metadata(&self) -> &StateValueMetadata { use WriteOp::*; match self { - Creation { metadata, .. } | Modification { metadata, .. } | Deletion { metadata } => { - metadata - }, + Creation(v) | Modification(v) => v.metadata(), + Deletion(meta) => meta, } } - pub fn get_metadata_mut(&mut self) -> &mut StateValueMetadata { + pub fn metadata_mut(&mut self) -> &mut StateValueMetadata { use WriteOp::*; match self { - Creation { metadata, .. } | Modification { metadata, .. } | Deletion { metadata } => { - metadata - }, + Creation(v) | Modification(v) => v.metadata_mut(), + Deletion(meta) => meta, } } pub fn into_metadata(self) -> StateValueMetadata { + use WriteOp::*; + match self { - Creation { metadata, .. } | Modification { metadata, .. } | Deletion { metadata } => { - metadata - }, + Creation(v) | Modification(v) => v.into_metadata(), + Deletion(meta) => meta, } } + pub fn creation(data: Bytes, metadata: StateValueMetadata) -> Self { + Self::Creation(StateValue::new_with_metadata(data, metadata)) + } + + pub fn modification(data: Bytes, metadata: StateValueMetadata) -> Self { + Self::Modification(StateValue::new_with_metadata(data, metadata)) + } + + pub fn deletion(metadata: StateValueMetadata) -> Self { + Self::Deletion(metadata) + } + pub fn legacy_creation(data: Bytes) -> Self { - Self::Creation { - data, - metadata: StateValueMetadata::none(), - } + Self::Creation(StateValue::new_legacy(data)) } pub fn legacy_modification(data: Bytes) -> Self { - Self::Modification { - data, - metadata: StateValueMetadata::none(), - } + Self::Modification(StateValue::new_legacy(data)) } pub fn legacy_deletion() -> Self { - Self::Deletion { - metadata: StateValueMetadata::none(), - } + Self::Deletion(StateValueMetadata::none()) } } @@ -316,8 +285,7 @@ pub trait TransactionWrite: Debug { // Provided as a separate method to avoid the clone in as_state_value method // (although default implementation below does just that). fn as_state_value_metadata(&self) -> Option { - self.as_state_value() - .map(|state_value| state_value.into_metadata()) + self.as_state_value().map(StateValue::into_metadata) } // Often, the contents of W:TransactionWrite are converted to Option, e.g. @@ -376,23 +344,19 @@ impl TransactionWrite for WriteOp { } fn as_state_value(&self) -> Option { - self.bytes() - .map(|bytes| StateValue::new_with_metadata(bytes.clone(), self.metadata().clone())) + self.state_value_ref().cloned() } // Note that even if WriteOp is DeletionWithMetadata, the method returns None, as a later // read would not read the metadata of the deletion op. fn as_state_value_metadata(&self) -> Option { - self.bytes().map(|_| self.metadata().clone()) + self.state_value_ref().map(StateValue::metadata).cloned() } fn from_state_value(maybe_state_value: Option) -> Self { match maybe_state_value { None => Self::legacy_deletion(), - Some(state_value) => { - let (metadata, data) = state_value.unpack(); - Self::Modification { data, metadata } - }, + Some(state_value) => Self::Modification(state_value), } } @@ -409,33 +373,37 @@ impl TransactionWrite for WriteOp { use WriteOp::*; match self { - Creation { data, .. } | Modification { data, .. } => *data = bytes, + Creation(v) | Modification(v) => v.set_bytes(bytes), Deletion { .. } => (), } } } #[allow(clippy::format_collect)] -impl std::fmt::Debug for WriteOp { +impl Debug for WriteOp { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + use WriteOp::*; + match self { - Creation { data, metadata } => write!( + Creation(v) => write!( f, "Creation({}, metadata:{:?})", - data.iter() + v.bytes() + .iter() .map(|byte| format!("{:02x}", byte)) .collect::(), - metadata, + v.metadata(), ), - Modification { data, metadata } => write!( + Modification(v) => write!( f, "Modification({}, metadata:{:?})", - data.iter() + v.bytes() + .iter() .map(|byte| format!("{:02x}", byte)) .collect::(), - metadata, + v.metadata(), ), - Deletion { metadata } => { + Deletion(metadata) => { write!(f, "Deletion(metadata:{:?})", metadata,) }, } From a4c0e86e92c512dba93b872b4e3deec8f63a4c6c Mon Sep 17 00:00:00 2001 From: aldenhu Date: Wed, 6 Nov 2024 02:17:22 +0000 Subject: [PATCH 3/8] do not cache hash --- testsuite/generate-format/src/api.rs | 5 +- types/src/state_store/state_value.rs | 78 +++++++--------------------- 2 files changed, 20 insertions(+), 63 deletions(-) diff --git a/testsuite/generate-format/src/api.rs b/testsuite/generate-format/src/api.rs index 4be4dfcb71592..59e60f4ad3fe1 100644 --- a/testsuite/generate-format/src/api.rs +++ b/testsuite/generate-format/src/api.rs @@ -15,10 +15,7 @@ use aptos_types::{ account_config::{CoinStoreResource, DepositEvent, WithdrawEvent}, block_metadata_ext::BlockMetadataExt, contract_event, event, - state_store::{ - state_key::StateKey, - state_value::{PersistedStateValueMetadata, StateValueMetadata}, - }, + state_store::{state_key::StateKey, state_value::PersistedStateValueMetadata}, transaction::{ self, authenticator::{AccountAuthenticator, TransactionAuthenticator}, diff --git a/types/src/state_store/state_value.rs b/types/src/state_store/state_value.rs index 5db5a22b088b2..464cca5804988 100644 --- a/types/src/state_store/state_value.rs +++ b/types/src/state_store/state_value.rs @@ -5,13 +5,9 @@ use crate::{ on_chain_config::CurrentTimeMicroseconds, proof::SparseMerkleRangeProof, state_store::state_key::StateKey, transaction::Version, }; -use aptos_crypto::{ - hash::{CryptoHash, SPARSE_MERKLE_PLACEHOLDER_HASH}, - HashValue, -}; +use aptos_crypto::{hash::SPARSE_MERKLE_PLACEHOLDER_HASH, HashValue}; use aptos_crypto_derive::{BCSCryptoHash, CryptoHasher}; use bytes::Bytes; -use once_cell::sync::OnceCell; #[cfg(any(test, feature = "fuzzing"))] use proptest::{arbitrary::Arbitrary, prelude::*}; use serde::{Deserialize, Deserializer, Serialize, Serializer}; @@ -161,20 +157,6 @@ impl StateValueMetadata { } } -#[derive(Clone, Debug, CryptoHasher)] -pub struct StateValue { - inner: StateValueInner, - hash: OnceCell, -} - -impl PartialEq for StateValue { - fn eq(&self, other: &Self) -> bool { - self.inner == other.inner - } -} - -impl Eq for StateValue {} - #[derive(BCSCryptoHash, CryptoHasher, Deserialize, Serialize)] #[serde(rename = "StateValue")] enum PersistedStateValue { @@ -186,27 +168,23 @@ enum PersistedStateValue { } impl PersistedStateValue { - fn into_in_mem_form(self) -> StateValueInner { + fn into_in_mem_form(self) -> StateValue { match self { - PersistedStateValue::V0(data) => StateValueInner { - data, - metadata: StateValueMetadata::none(), - }, - PersistedStateValue::WithMetadata { data, metadata } => StateValueInner { - data, - metadata: metadata.into_in_mem_form(), + PersistedStateValue::V0(data) => StateValue::new_legacy(data), + PersistedStateValue::WithMetadata { data, metadata } => { + StateValue::new_with_metadata(data, metadata.into_in_mem_form()) }, } } } -#[derive(Clone, Debug, Eq, PartialEq)] -struct StateValueInner { +#[derive(BCSCryptoHash, Clone, CryptoHasher, Debug, Eq, PartialEq)] +pub struct StateValue { data: Bytes, metadata: StateValueMetadata, } -impl StateValueInner { +impl StateValue { fn to_persistable_form(&self) -> PersistedStateValue { let Self { data, metadata } = self.clone(); let metadata = metadata.into_persistable(); @@ -234,9 +212,7 @@ impl<'de> Deserialize<'de> for StateValue { where D: Deserializer<'de>, { - let inner = PersistedStateValue::deserialize(deserializer)?.into_in_mem_form(); - let hash = OnceCell::new(); - Ok(Self { inner, hash }) + Ok(PersistedStateValue::deserialize(deserializer)?.into_in_mem_form()) } } @@ -245,7 +221,7 @@ impl Serialize for StateValue { where S: Serializer, { - self.inner.to_persistable_form().serialize(serializer) + self.to_persistable_form().serialize(serializer) } } @@ -255,9 +231,7 @@ impl StateValue { } pub fn new_with_metadata(data: Bytes, metadata: StateValueMetadata) -> Self { - let inner = StateValueInner { data, metadata }; - let hash = OnceCell::new(); - Self { inner, hash } + Self { data, metadata } } pub fn size(&self) -> usize { @@ -265,7 +239,7 @@ impl StateValue { } pub fn bytes(&self) -> &Bytes { - &self.inner.data + &self.data } /// Applies a bytes-to-bytes transformation on the state value contents, @@ -274,35 +248,31 @@ impl StateValue { self, f: F, ) -> anyhow::Result { - Ok(Self::new_with_metadata( - f(self.inner.data)?, - self.inner.metadata, - )) + Ok(Self::new_with_metadata(f(self.data)?, self.metadata)) } pub fn into_bytes(self) -> Bytes { - self.inner.data + self.data } pub fn set_bytes(&mut self, data: Bytes) { - self.inner.data = data; - self.hash = OnceCell::new(); + self.data = data; } pub fn metadata(&self) -> &StateValueMetadata { - &self.inner.metadata + &self.metadata } pub fn metadata_mut(&mut self) -> &mut StateValueMetadata { - &mut self.inner.metadata + &mut self.metadata } pub fn into_metadata(self) -> StateValueMetadata { - self.inner.metadata + self.metadata } pub fn unpack(self) -> (StateValueMetadata, Bytes) { - let StateValueInner { data, metadata } = self.inner; + let Self { data, metadata } = self; (metadata, data) } } @@ -321,16 +291,6 @@ impl From for StateValue { } } -impl CryptoHash for StateValue { - type Hasher = StateValueHasher; - - fn hash(&self) -> HashValue { - *self - .hash - .get_or_init(|| CryptoHash::hash(&self.inner.to_persistable_form())) - } -} - /// TODO(joshlind): add a proof implementation (e.g., verify()) and unit tests /// for these once we start supporting them. /// From f7ae2cba748dd71e4367dd1a5d2638dda02bbd94 Mon Sep 17 00:00:00 2001 From: aldenhu Date: Fri, 1 Nov 2024 00:19:16 +0000 Subject: [PATCH 4/8] avoid allocating 16 hashmaps per transaction --- Cargo.lock | 1 - execution/executor-benchmark/src/lib.rs | 1 + .../src/state_checkpoint_output.rs | 15 +- .../src/state_compute_result.rs | 1 - execution/executor/Cargo.toml | 1 - .../types/in_memory_state_calculator_v2.rs | 162 ++++------- storage/aptosdb/src/db/fake_aptosdb.rs | 7 +- .../src/db/include/aptosdb_testonly.rs | 31 +- .../aptosdb/src/db/include/aptosdb_writer.rs | 2 +- storage/aptosdb/src/db/test_helper.rs | 62 ++-- .../src/pruner/state_merkle_pruner/test.rs | 12 +- .../aptosdb/src/state_store/buffered_state.rs | 21 +- storage/aptosdb/src/state_store/mod.rs | 273 +++++++++--------- .../state_store/state_snapshot_committer.rs | 34 ++- .../src/state_store/state_store_test.rs | 11 +- storage/scratchpad/src/sparse_merkle/mod.rs | 4 +- .../storage-interface/src/chunk_to_commit.rs | 6 +- storage/storage-interface/src/state_delta.rs | 23 +- types/src/proptest_types.rs | 26 +- types/src/transaction/mod.rs | 28 +- types/src/write_set.rs | 21 ++ 21 files changed, 338 insertions(+), 404 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a38bd0ab3a12f..e41ac4f746144 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1472,7 +1472,6 @@ dependencies = [ "aptos-types", "aptos-vm", "aptos-vm-genesis", - "arr_macro", "bcs 0.1.4", "bytes", "dashmap", diff --git a/execution/executor-benchmark/src/lib.rs b/execution/executor-benchmark/src/lib.rs index 09f2a5b91558b..8332c7f0c2974 100644 --- a/execution/executor-benchmark/src/lib.rs +++ b/execution/executor-benchmark/src/lib.rs @@ -508,6 +508,7 @@ impl GasMeasurement { } } +/// FIXME(aldenhu): update static OTHER_LABELS: &[(&str, bool, &str)] = &[ ("1.", true, "verified_state_view"), ("2.", true, "state_checkpoint"), diff --git a/execution/executor-types/src/state_checkpoint_output.rs b/execution/executor-types/src/state_checkpoint_output.rs index c611d6c2c90a0..c5faf719a1396 100644 --- a/execution/executor-types/src/state_checkpoint_output.rs +++ b/execution/executor-types/src/state_checkpoint_output.rs @@ -7,9 +7,12 @@ use crate::transactions_with_output::TransactionsWithOutput; use aptos_crypto::HashValue; use aptos_drop_helper::DropHelper; use aptos_storage_interface::state_delta::StateDelta; -use aptos_types::{state_store::ShardedStateUpdates, transaction::TransactionStatus}; +use aptos_types::{ + state_store::{state_key::StateKey, state_value::StateValue}, + transaction::TransactionStatus, +}; use derive_more::Deref; -use std::sync::Arc; +use std::{collections::HashMap, sync::Arc}; #[derive(Default)] pub struct TransactionsByStatus { @@ -69,15 +72,13 @@ impl StateCheckpointOutput { pub fn new( parent_state: Arc, result_state: Arc, - state_updates_before_last_checkpoint: Option, - per_version_state_updates: Vec, + state_updates_before_last_checkpoint: Option>>, state_checkpoint_hashes: Vec>, ) -> Self { Self::new_impl(Inner { parent_state, result_state, state_updates_before_last_checkpoint, - per_version_state_updates, state_checkpoint_hashes, }) } @@ -87,7 +88,6 @@ impl StateCheckpointOutput { parent_state: state.clone(), result_state: state, state_updates_before_last_checkpoint: None, - per_version_state_updates: vec![], state_checkpoint_hashes: vec![], }) } @@ -111,8 +111,7 @@ impl StateCheckpointOutput { pub struct Inner { pub parent_state: Arc, pub result_state: Arc, - pub state_updates_before_last_checkpoint: Option, - pub per_version_state_updates: Vec, + pub state_updates_before_last_checkpoint: Option>>, pub state_checkpoint_hashes: Vec>, } diff --git a/execution/executor-types/src/state_compute_result.rs b/execution/executor-types/src/state_compute_result.rs index c47ceeb97c420..aaa0b864afe63 100644 --- a/execution/executor-types/src/state_compute_result.rs +++ b/execution/executor-types/src/state_compute_result.rs @@ -157,7 +157,6 @@ impl StateComputeResult { transactions: self.execution_output.to_commit.txns(), transaction_outputs: self.execution_output.to_commit.transaction_outputs(), transaction_infos: &self.ledger_update_output.transaction_infos, - per_version_state_updates: &self.state_checkpoint_output.per_version_state_updates, base_state_version: self.state_checkpoint_output.parent_state.base_version, latest_in_memory_state: &self.state_checkpoint_output.result_state, state_updates_until_last_checkpoint: self diff --git a/execution/executor/Cargo.toml b/execution/executor/Cargo.toml index fcc3213c594bb..69d56c2f39bde 100644 --- a/execution/executor/Cargo.toml +++ b/execution/executor/Cargo.toml @@ -29,7 +29,6 @@ aptos-sdk = { workspace = true } aptos-storage-interface = { workspace = true } aptos-types = { workspace = true } aptos-vm = { workspace = true } -arr_macro = { workspace = true } bcs = { workspace = true } bytes = { workspace = true } dashmap = { workspace = true } diff --git a/execution/executor/src/types/in_memory_state_calculator_v2.rs b/execution/executor/src/types/in_memory_state_calculator_v2.rs index 21987a51c39b3..7488277555f0e 100644 --- a/execution/executor/src/types/in_memory_state_calculator_v2.rs +++ b/execution/executor/src/types/in_memory_state_calculator_v2.rs @@ -17,15 +17,13 @@ use aptos_storage_interface::{ }; use aptos_types::{ state_store::{ - create_empty_sharded_state_updates, state_key::StateKey, - state_storage_usage::StateStorageUsage, state_value::StateValue, ShardedStateUpdates, + state_key::StateKey, state_storage_usage::StateStorageUsage, state_value::StateValue, }, - transaction::Version, + transaction::{TransactionOutput, Version}, write_set::{TransactionWrite, WriteSet}, }; -use arr_macro::arr; use dashmap::DashMap; -use itertools::zip_eq; +use itertools::Itertools; use rayon::prelude::*; use std::{collections::HashMap, ops::Deref, sync::Arc}; @@ -42,11 +40,6 @@ impl InMemoryStateCalculatorV2 { Self::validate_input_for_block(parent_state, &execution_output.to_commit)?; } - let state_updates_vec = Self::get_sharded_state_updates( - execution_output.to_commit.transaction_outputs(), - |txn_output| txn_output.write_set(), - ); - // If there are multiple checkpoints in the chunk, we only calculate the SMT (and its root // hash) for the last one. let last_checkpoint_index = execution_output.to_commit.get_last_checkpoint_index(); @@ -54,7 +47,11 @@ impl InMemoryStateCalculatorV2 { Self::calculate_impl( parent_state, &execution_output.state_cache, - state_updates_vec, + execution_output + .to_commit + .transaction_outputs + .iter() + .map(TransactionOutput::write_set), last_checkpoint_index, execution_output.is_block, known_state_checkpoints, @@ -67,22 +64,20 @@ impl InMemoryStateCalculatorV2 { last_checkpoint_index: Option, write_sets: &[WriteSet], ) -> Result { - let state_updates_vec = Self::get_sharded_state_updates(write_sets, |write_set| write_set); - Self::calculate_impl( parent_state, state_cache, - state_updates_vec, + write_sets, last_checkpoint_index, false, Option::>::None, ) } - fn calculate_impl( + fn calculate_impl<'a>( parent_state: &Arc, state_cache: &StateCache, - state_updates_vec: Vec, + write_sets: impl IntoIterator, last_checkpoint_index: Option, is_block: bool, known_state_checkpoints: Option>>, @@ -96,25 +91,24 @@ impl InMemoryStateCalculatorV2 { } = state_cache; assert!(frozen_base.smt.is_the_same(&parent_state.current)); + let write_sets = write_sets.into_iter().collect_vec(); + let num_txns = write_sets.len(); let (updates_before_last_checkpoint, updates_after_last_checkpoint) = if let Some(index) = last_checkpoint_index { ( - Self::calculate_updates(&state_updates_vec[..=index]), - Self::calculate_updates(&state_updates_vec[index + 1..]), + Self::calculate_updates(&write_sets[..=index]), + Self::calculate_updates(&write_sets[index + 1..]), ) } else { - ( - create_empty_sharded_state_updates(), - Self::calculate_updates(&state_updates_vec), - ) + (HashMap::new(), Self::calculate_updates(&write_sets)) }; + let all_updates = Self::calculate_updates(&write_sets); - let num_txns = state_updates_vec.len(); - - let usage = Self::calculate_usage(parent_state.current.usage(), sharded_state_cache, &[ - &updates_before_last_checkpoint, - &updates_after_last_checkpoint, - ]); + let usage = Self::calculate_usage( + parent_state.current.usage(), + sharded_state_cache, + &all_updates, + ); let first_version = parent_state.current_version.map_or(0, |v| v + 1); let proof_reader = ProofReader::new(proofs); @@ -181,11 +175,7 @@ impl InMemoryStateCalculatorV2 { } else { let mut updates_since_latest_checkpoint = parent_state.updates_since_base.deref().deref().clone(); - zip_eq( - updates_since_latest_checkpoint.iter_mut(), - updates_after_last_checkpoint, - ) - .for_each(|(base, delta)| base.extend(delta)); + updates_since_latest_checkpoint.extend(updates_after_last_checkpoint); updates_since_latest_checkpoint }; @@ -209,54 +199,25 @@ impl InMemoryStateCalculatorV2 { parent_state.clone(), Arc::new(result_state), last_checkpoint_index.map(|_| updates_before_last_checkpoint), - state_updates_vec, state_checkpoint_hashes, )) } - fn get_sharded_state_updates<'a, T, F>( - outputs: &'a [T], - write_set_fn: F, - ) -> Vec - where - T: Sync + 'a, - F: Fn(&'a T) -> &'a WriteSet + Sync, - { - let _timer = OTHER_TIMERS.timer_with(&["get_sharded_state_updates"]); + fn calculate_updates<'a>( + write_sets: &'a [&'a WriteSet], + ) -> HashMap> { + let _timer = OTHER_TIMERS.timer_with(&["calculate_updates"]); - outputs - .par_iter() - .map(|output| { - let mut updates = arr![HashMap::new(); 16]; - write_set_fn(output) + write_sets + .iter() + .flat_map(|write_set| { + write_set .iter() - .for_each(|(state_key, write_op)| { - updates[state_key.get_shard_id() as usize] - .insert(state_key.clone(), write_op.as_state_value()); - }); - updates + .map(|(key, op)| (key.clone(), op.as_state_value())) }) .collect() } - fn calculate_updates(state_updates_vec: &[ShardedStateUpdates]) -> ShardedStateUpdates { - let _timer = OTHER_TIMERS.timer_with(&["calculate_updates"]); - let mut updates: ShardedStateUpdates = create_empty_sharded_state_updates(); - updates - .par_iter_mut() - .enumerate() - .for_each(|(i, per_shard_update)| { - per_shard_update.extend( - state_updates_vec - .iter() - .flat_map(|hms| &hms[i]) - .map(|(k, v)| (k.clone(), v.clone())) - .collect::>(), - ) - }); - updates - } - fn add_to_delta( k: &StateKey, v: &Option, @@ -280,7 +241,7 @@ impl InMemoryStateCalculatorV2 { fn calculate_usage( old_usage: StateStorageUsage, sharded_state_cache: &ShardedStateCache, - updates: &[&ShardedStateUpdates; 2], + updates: &HashMap>, ) -> StateStorageUsage { let _timer = OTHER_TIMERS .with_label_values(&["calculate_usage"]) @@ -288,38 +249,21 @@ impl InMemoryStateCalculatorV2 { if old_usage.is_untracked() { return StateStorageUsage::new_untracked(); } - let (items_delta, bytes_delta) = updates[0] + + let (items_delta, bytes_delta) = updates .par_iter() - .zip_eq(updates[1].par_iter()) - .enumerate() - .map( - |(i, (shard_updates_before_checkpoint, shard_updates_after_checkpoint))| { - let mut items_delta = 0i64; - let mut bytes_delta = 0i64; - let num_updates_before_checkpoint = shard_updates_before_checkpoint.len(); - for (index, (k, v)) in shard_updates_before_checkpoint - .iter() - .chain(shard_updates_after_checkpoint.iter()) - .enumerate() - { - // Ignore updates before the checkpoint if there is an update for the same - // key after the checkpoint. - if index < num_updates_before_checkpoint - && shard_updates_after_checkpoint.contains_key(k) - { - continue; - } - Self::add_to_delta( - k, - v, - sharded_state_cache.shard(i as u8), - &mut items_delta, - &mut bytes_delta, - ); - } - (items_delta, bytes_delta) - }, - ) + .map(|(key, value)| { + let mut items_delta = 0i64; + let mut bytes_delta = 0i64; + Self::add_to_delta( + key, + value, + sharded_state_cache.shard(key.get_shard_id()), + &mut items_delta, + &mut bytes_delta, + ); + (items_delta, bytes_delta) + }) .reduce( || (0i64, 0i64), |(items_now, bytes_now), (items_delta, bytes_delta)| { @@ -334,7 +278,7 @@ impl InMemoryStateCalculatorV2 { fn make_checkpoint( latest_checkpoint: FrozenSparseMerkleTree, - updates: &ShardedStateUpdates, + updates: &HashMap>, usage: StateStorageUsage, proof_reader: &ProofReader, ) -> Result> { @@ -342,12 +286,10 @@ impl InMemoryStateCalculatorV2 { // Update SMT. // - // TODO(grao): Consider use the sharded updates directly instead of flatten. - let smt_updates: Vec<_> = updates + // TODO(aldenhu): avoid collecting into vec + let smt_updates = updates .iter() - .flatten() - .map(|(key, value)| (key.hash(), value.as_ref())) - .collect(); + .map(|(key, value)| (key.hash(), value.as_ref())); let new_checkpoint = latest_checkpoint.batch_update(smt_updates, usage, proof_reader)?; Ok(new_checkpoint) } @@ -365,7 +307,7 @@ impl InMemoryStateCalculatorV2 { base.current_version, ); ensure!( - base.updates_since_base.iter().all(|shard| shard.is_empty()), + base.updates_since_base.is_empty(), "Base state is corrupted, updates_since_base is not empty at a checkpoint." ); diff --git a/storage/aptosdb/src/db/fake_aptosdb.rs b/storage/aptosdb/src/db/fake_aptosdb.rs index 948d7e9504280..254cad17e3593 100644 --- a/storage/aptosdb/src/db/fake_aptosdb.rs +++ b/storage/aptosdb/src/db/fake_aptosdb.rs @@ -118,10 +118,9 @@ impl FakeBufferedState { new_state_after_checkpoint.base_version > self.state_after_checkpoint.base_version, "Diff between base and latest checkpoints provided, while they are the same.", ); - combine_sharded_state_updates( - &mut self.state_after_checkpoint.updates_since_base, - updates_until_next_checkpoint_since_current, - ); + self.state_after_checkpoint + .updates_since_base + .extend(updates_until_next_checkpoint_since_current); self.state_after_checkpoint.current = new_state_after_checkpoint.base.clone(); self.state_after_checkpoint.current_version = new_state_after_checkpoint.base_version; let state_after_checkpoint = self diff --git a/storage/aptosdb/src/db/include/aptosdb_testonly.rs b/storage/aptosdb/src/db/include/aptosdb_testonly.rs index 58a629b61c720..5f9c803f3170d 100644 --- a/storage/aptosdb/src/db/include/aptosdb_testonly.rs +++ b/storage/aptosdb/src/db/include/aptosdb_testonly.rs @@ -1,10 +1,10 @@ // Copyright © Aptos Foundation // SPDX-License-Identifier: Apache-2.0 +use std::collections::HashMap; use crate::state_store::buffered_state::BufferedState; use aptos_config::config::{ BUFFERED_STATE_TARGET_ITEMS_FOR_TEST, DEFAULT_MAX_NUM_NODES_PER_LRU_CACHE_SHARD}; use aptos_infallible::Mutex; -use aptos_types::state_store::{create_empty_sharded_state_updates, ShardedStateUpdates}; use std::default::Default; use aptos_storage_interface::cached_state_view::ShardedStateCache; use aptos_storage_interface::state_delta::StateDelta; @@ -133,8 +133,7 @@ pub struct ChunkToCommitOwned { transaction_infos: Vec, base_state_version: Option, latest_in_memory_state: Arc, - per_version_state_updates: Vec, - state_updates_until_last_checkpoint: Option, + state_updates_until_last_checkpoint: Option>>, sharded_state_cache: Option, is_reconfig: bool, } @@ -146,13 +145,13 @@ impl ChunkToCommitOwned { base_state_version: Option, latest_in_memory_state: &StateDelta, ) -> Self { - let (transactions, transaction_outputs, transaction_infos, per_version_state_updates) = Self::disassemble_txns_to_commit(txns_to_commit); + let (transactions, transaction_outputs, transaction_infos) = Self::disassemble_txns_to_commit(txns_to_commit); let state_updates_until_last_checkpoint = Self::gather_state_updates_until_last_checkpoint( first_version, latest_in_memory_state, - &per_version_state_updates, &transaction_infos, + &transaction_outputs, ); Self { @@ -162,7 +161,6 @@ impl ChunkToCommitOwned { transaction_infos, base_state_version, latest_in_memory_state: Arc::new(latest_in_memory_state.clone()), - per_version_state_updates, state_updates_until_last_checkpoint, sharded_state_cache: None, is_reconfig: false, @@ -177,7 +175,6 @@ impl ChunkToCommitOwned { transaction_infos: &self.transaction_infos, base_state_version: self.base_state_version, latest_in_memory_state: &self.latest_in_memory_state, - per_version_state_updates: &self.per_version_state_updates, state_updates_until_last_checkpoint: self.state_updates_until_last_checkpoint.as_ref(), sharded_state_cache: self.sharded_state_cache.as_ref(), is_reconfig: self.is_reconfig, @@ -185,11 +182,11 @@ impl ChunkToCommitOwned { } fn disassemble_txns_to_commit(txns_to_commit: &[TransactionToCommit]) -> ( - Vec, Vec, Vec, Vec, + Vec, Vec, Vec ) { txns_to_commit.iter().map(|txn_to_commit| { let TransactionToCommit { - transaction, transaction_info, state_updates, write_set, events, is_reconfig: _, transaction_auxiliary_data + transaction, transaction_info, write_set, events, is_reconfig: _, transaction_auxiliary_data } = txn_to_commit; let transaction_output = TransactionOutput::new( @@ -200,16 +197,16 @@ impl ChunkToCommitOwned { transaction_auxiliary_data.clone(), ); - (transaction.clone(), transaction_output, transaction_info.clone(), state_updates.clone()) + (transaction.clone(), transaction_output, transaction_info.clone()) }).multiunzip() } pub fn gather_state_updates_until_last_checkpoint( first_version: Version, latest_in_memory_state: &StateDelta, - per_version_state_updates: &[ShardedStateUpdates], transaction_infos: &[TransactionInfo], - ) -> Option { + transaction_outputs: &[TransactionOutput], + ) -> Option>> { if let Some(latest_checkpoint_version) = latest_in_memory_state.base_version { if latest_checkpoint_version >= first_version { let idx = (latest_checkpoint_version - first_version) as usize; @@ -219,15 +216,7 @@ impl ChunkToCommitOwned { latest_checkpoint_version, first_version + idx as u64 ); - let mut sharded_state_updates = create_empty_sharded_state_updates(); - sharded_state_updates.par_iter_mut().enumerate().for_each( - |(shard_id, state_updates_shard)| { - per_version_state_updates[..=idx].iter().for_each(|updates| { - state_updates_shard.extend(updates[shard_id].clone()); - }) - }, - ); - return Some(sharded_state_updates); + return Some(transaction_outputs[..=idx].iter().map(TransactionOutput::write_set).flat_map(WriteSet::state_updates).collect()) } } diff --git a/storage/aptosdb/src/db/include/aptosdb_writer.rs b/storage/aptosdb/src/db/include/aptosdb_writer.rs index e2e9416933bcc..7d29cde563a1b 100644 --- a/storage/aptosdb/src/db/include/aptosdb_writer.rs +++ b/storage/aptosdb/src/db/include/aptosdb_writer.rs @@ -343,7 +343,7 @@ impl AptosDB { // TODO(grao): Make state_store take sharded state updates. self.state_store.put_value_sets( - chunk.per_version_state_updates, + chunk.transaction_outputs.iter().map(TransactionOutput::write_set), chunk.first_version, chunk.latest_in_memory_state.current.usage(), chunk.sharded_state_cache, diff --git a/storage/aptosdb/src/db/test_helper.rs b/storage/aptosdb/src/db/test_helper.rs index 9128226c2c4e3..1e498e6d30413 100644 --- a/storage/aptosdb/src/db/test_helper.rs +++ b/storage/aptosdb/src/db/test_helper.rs @@ -33,9 +33,8 @@ use aptos_types::{ proptest_types::{AccountInfoUniverse, BlockGen}, state_store::{state_key::StateKey, state_value::StateValue}, transaction::{Transaction, TransactionInfo, TransactionToCommit, Version}, + write_set::TransactionWrite, }; -#[cfg(test)] -use arr_macro::arr; use proptest::{collection::vec, prelude::*, sample::Index}; use std::{collections::HashMap, fmt::Debug}; @@ -69,11 +68,11 @@ pub(crate) fn update_store( enable_sharding: bool, ) -> HashValue { use aptos_storage_interface::{jmt_update_refs, jmt_updates}; + use aptos_types::write_set::WriteSet; + let mut root_hash = *aptos_crypto::hash::SPARSE_MERKLE_PLACEHOLDER_HASH; for (i, (key, value)) in input.enumerate() { let value_state_set = vec![(&key, value.as_ref())].into_iter().collect(); - let mut sharded_value_state_set = arr![HashMap::new(); 16]; - sharded_value_state_set[key.get_shard_id() as usize].insert(key.clone(), value.clone()); let jmt_updates = jmt_updates(&value_state_set); let version = first_version + i as Version; root_hash = store @@ -88,7 +87,7 @@ pub(crate) fn update_store( let schema_batch = SchemaBatch::new(); store .put_value_sets( - &[sharded_value_state_set], + &[WriteSet::new_for_test([(key.clone(), value.clone())])], version, StateStorageUsage::new_untracked(), None, @@ -114,14 +113,11 @@ pub(crate) fn update_store( pub fn update_in_memory_state(state: &mut StateDelta, txns_to_commit: &[TransactionToCommit]) { let mut next_version = state.current_version.map_or(0, |v| v + 1); for txn_to_commit in txns_to_commit { - txn_to_commit - .state_updates() - .iter() - .flatten() - .for_each(|(key, value)| { - state.updates_since_base[key.get_shard_id() as usize] - .insert(key.clone(), value.clone()); - }); + txn_to_commit.write_set.iter().for_each(|(key, op)| { + state + .updates_since_base + .insert(key.clone(), op.as_state_value()); + }); next_version += 1; if txn_to_commit.has_state_checkpoint_hash() { state.current = state @@ -130,7 +126,6 @@ pub fn update_in_memory_state(state: &mut StateDelta, txns_to_commit: &[Transact state .updates_since_base .iter() - .flatten() .map(|(k, v)| (k.hash(), v.as_ref())) .collect(), &ProofReader::new_empty(), @@ -139,9 +134,7 @@ pub fn update_in_memory_state(state: &mut StateDelta, txns_to_commit: &[Transact state.current_version = next_version.checked_sub(1); state.base = state.current.clone(); state.base_version = state.current_version; - state.updates_since_base.iter_mut().for_each(|shard| { - shard.clear(); - }); + state.updates_since_base.clear(); } } @@ -152,7 +145,6 @@ pub fn update_in_memory_state(state: &mut StateDelta, txns_to_commit: &[Transact state .updates_since_base .iter() - .flatten() .map(|(k, v)| (k.hash(), v.as_ref())) .collect(), &ProofReader::new_empty(), @@ -326,9 +318,7 @@ fn gen_snapshot_version( updates.extend( txns_to_commit[0..=idx] .iter() - .flat_map(|x| x.state_updates().clone()) - .flatten() - .collect::>(), + .flat_map(|x| x.write_set().state_updates()), ); if updates.len() >= threshold { snapshot_version = Some(cur_ver + idx as u64); @@ -337,17 +327,13 @@ fn gen_snapshot_version( updates.extend( txns_to_commit[idx + 1..] .iter() - .flat_map(|x| x.state_updates().clone()) - .flatten() - .collect::>(), + .flat_map(|x| x.write_set().state_updates()), ); } else { updates.extend( txns_to_commit .iter() - .flat_map(|x| x.state_updates().clone()) - .flatten() - .collect::>(), + .flat_map(|x| x.write_set().state_updates()), ); } snapshot_version @@ -457,7 +443,7 @@ fn verify_snapshots( txns_to_commit: Vec<&TransactionToCommit>, ) { let mut cur_version = start_version; - let mut updates: HashMap<&StateKey, Option<&StateValue>> = HashMap::new(); + let mut updates: HashMap> = HashMap::new(); for snapshot_version in snapshot_versions { let start = (cur_version - start_version) as usize; let end = (snapshot_version - start_version) as usize; @@ -472,19 +458,14 @@ fn verify_snapshots( updates.extend( txns_to_commit[start..=end] .iter() - .flat_map(|x| { - x.state_updates() - .iter() - .flatten() - .map(|(k, v_opt)| (k, v_opt.as_ref())) - }) - .collect::>>(), + .flat_map(|x| x.write_set().iter()) + .map(|(k, op)| (k.clone(), op.as_state_value())), ); for (state_key, state_value) in &updates { let (state_value_in_db, proof) = db .get_state_value_with_proof_by_version(state_key, snapshot_version) .unwrap(); - assert_eq!(state_value_in_db.as_ref(), *state_value); + assert_eq!(state_value_in_db.as_ref(), state_value.as_ref()); proof .verify( expected_root_hash, @@ -818,10 +799,11 @@ pub fn verify_committed_transactions( ); // Fetch and verify account states. - for (state_key, state_value) in txn_to_commit.state_updates().iter().flatten() { - updates.insert(state_key, state_value); + for (state_key, write_op) in txn_to_commit.write_set().iter() { + let state_value = write_op.as_state_value(); let state_value_in_db = db.get_state_value_by_version(state_key, cur_ver).unwrap(); - assert_eq!(state_value_in_db, *state_value); + assert_eq!(state_value_in_db, state_value); + updates.insert(state_key, state_value); } if !txn_to_commit.has_state_checkpoint_hash() { @@ -952,7 +934,7 @@ pub fn put_as_state_root(db: &AptosDB, version: Version, key: StateKey, value: S .clone(); in_memory_state.current = smt; in_memory_state.current_version = Some(version); - in_memory_state.updates_since_base[key.get_shard_id() as usize].insert(key, Some(value)); + in_memory_state.updates_since_base.insert(key, Some(value)); db.state_store .buffered_state() .lock() diff --git a/storage/aptosdb/src/pruner/state_merkle_pruner/test.rs b/storage/aptosdb/src/pruner/state_merkle_pruner/test.rs index cc15fc12a7f13..9a15d2fe6d82d 100644 --- a/storage/aptosdb/src/pruner/state_merkle_pruner/test.rs +++ b/storage/aptosdb/src/pruner/state_merkle_pruner/test.rs @@ -29,8 +29,8 @@ use aptos_types::{ state_value::{StaleStateValueByKeyHashIndex, StaleStateValueIndex, StateValue}, }, transaction::Version, + write_set::WriteSet, }; -use arr_macro::arr; use proptest::{prelude::*, proptest}; use std::{collections::HashMap, sync::Arc}; @@ -39,13 +39,11 @@ fn put_value_set( value_set: Vec<(StateKey, StateValue)>, version: Version, ) -> HashValue { - let mut sharded_value_set = arr![HashMap::new(); 16]; + let write_set = + WriteSet::new_for_test(value_set.iter().map(|(k, v)| (k.clone(), Some(v.clone())))); let value_set: HashMap<_, _> = value_set .iter() - .map(|(key, value)| { - sharded_value_set[key.get_shard_id() as usize].insert(key.clone(), Some(value.clone())); - (key, Some(value)) - }) + .map(|(key, value)| (key, Some(value))) .collect(); let jmt_updates = jmt_updates(&value_set); @@ -63,7 +61,7 @@ fn put_value_set( let enable_sharding = state_store.state_kv_db.enabled_sharding(); state_store .put_value_sets( - &[sharded_value_set], + &[write_set], version, StateStorageUsage::new_untracked(), None, diff --git a/storage/aptosdb/src/state_store/buffered_state.rs b/storage/aptosdb/src/state_store/buffered_state.rs index 2050af1e1de3a..2b9236527f4c7 100644 --- a/storage/aptosdb/src/state_store/buffered_state.rs +++ b/storage/aptosdb/src/state_store/buffered_state.rs @@ -11,10 +11,11 @@ use aptos_logger::info; use aptos_scratchpad::SmtAncestors; use aptos_storage_interface::{db_ensure as ensure, state_delta::StateDelta, AptosDbError, Result}; use aptos_types::{ - state_store::{combine_sharded_state_updates, state_value::StateValue, ShardedStateUpdates}, + state_store::{state_key::StateKey, state_value::StateValue}, transaction::Version, }; use std::{ + collections::HashMap, sync::{ mpsc, mpsc::{Sender, SyncSender}, @@ -111,12 +112,7 @@ impl BufferedState { let take_out_to_commit = { let state_until_checkpoint = self.state_until_checkpoint.as_ref().expect("Must exist"); - state_until_checkpoint - .updates_since_base - .iter() - .map(|shard| shard.len()) - .sum::() - >= self.target_items + state_until_checkpoint.updates_since_base.len() >= self.target_items || state_until_checkpoint.current_version.map_or(0, |v| v + 1) - state_until_checkpoint.base_version.map_or(0, |v| v + 1) >= TARGET_SNAPSHOT_INTERVAL_IN_VERSION @@ -154,7 +150,9 @@ impl BufferedState { /// This method updates the buffered state with new data. pub fn update( &mut self, - updates_until_next_checkpoint_since_current_option: Option<&ShardedStateUpdates>, + updates_until_next_checkpoint_since_current_option: Option< + &HashMap>, + >, new_state_after_checkpoint: &StateDelta, sync_commit: bool, ) -> Result<()> { @@ -172,9 +170,10 @@ impl BufferedState { new_state_after_checkpoint.base_version > self.state_after_checkpoint.base_version, "Diff between base and latest checkpoints provided, while they are the same.", ); - combine_sharded_state_updates( - &mut self.state_after_checkpoint.updates_since_base, - updates_until_next_checkpoint_since_current, + self.state_after_checkpoint.updates_since_base.extend( + updates_until_next_checkpoint_since_current + .iter() + .map(|(k, v)| (k.clone(), v.clone())), ); self.state_after_checkpoint.current = new_state_after_checkpoint.base.clone(); self.state_after_checkpoint.current_version = new_state_after_checkpoint.base_version; diff --git a/storage/aptosdb/src/state_store/mod.rs b/storage/aptosdb/src/state_store/mod.rs index 38ab16199c188..066a0f05bd9a8 100644 --- a/storage/aptosdb/src/state_store/mod.rs +++ b/storage/aptosdb/src/state_store/mod.rs @@ -48,6 +48,7 @@ use aptos_experimental_runtimes::thread_manager::THREAD_MANAGER; use aptos_infallible::Mutex; use aptos_jellyfish_merkle::iterator::JellyfishMerkleIterator; use aptos_logger::info; +use aptos_metrics_core::TimerHelper; use aptos_schemadb::SchemaBatch; use aptos_scratchpad::{SmtAncestors, SparseMerkleTree}; use aptos_storage_interface::{ @@ -60,19 +61,20 @@ use aptos_storage_interface::{ use aptos_types::{ proof::{definition::LeafCount, SparseMerkleProofExt, SparseMerkleRangeProof}, state_store::{ - create_empty_sharded_state_updates, state_key::{prefix::StateKeyPrefix, StateKey}, state_storage_usage::StateStorageUsage, state_value::{ StaleStateValueByKeyHashIndex, StaleStateValueIndex, StateValue, StateValueChunkWithProof, }, - ShardedStateUpdates, StateViewId, + StateViewId, }, transaction::Version, write_set::{TransactionWrite, WriteSet}, }; +use arr_macro::arr; use claims::{assert_ge, assert_le}; +use itertools::Itertools; use rayon::prelude::*; use std::{collections::HashSet, ops::Deref, sync::Arc}; @@ -92,6 +94,12 @@ const MAX_WRITE_SETS_AFTER_SNAPSHOT: LeafCount = buffered_state::TARGET_SNAPSHOT pub const MAX_COMMIT_PROGRESS_DIFFERENCE: u64 = 1_000_000; +type ShardedKvUpdates = [Vec<((StateKey, Version), Option)>; NUM_STATE_SHARDS]; + +fn empty_kv_updates() -> ShardedKvUpdates { + arr![vec![]; 16] +} + pub(crate) struct StateDb { pub ledger_db: Arc, pub state_merkle_db: Arc, @@ -648,47 +656,22 @@ impl StateStore { sharded_state_kv_batches: &ShardedStateKvSchemaBatch, enable_sharding: bool, ) -> Result<()> { - let _timer = OTHER_TIMERS_SECONDS - .with_label_values(&["put_writesets"]) - .start_timer(); - - let value_state_sets: Vec = write_sets - .iter() - .map(|ws| { - let mut sharded_state_updates = create_empty_sharded_state_updates(); - ws.iter().for_each(|(key, value)| { - sharded_state_updates[key.get_shard_id() as usize] - .insert(key.clone(), value.as_state_value()); - }); - sharded_state_updates - }) - .collect::>(); - - self.put_stats_and_indices( - &value_state_sets, + self.put_value_sets( + &write_sets, first_version, StateStorageUsage::new_untracked(), - None, + None, // state cache batch, sharded_state_kv_batches, - None, - enable_sharding, - )?; - - self.put_state_values( - &value_state_sets, - first_version, - sharded_state_kv_batches, enable_sharding, - )?; - - Ok(()) + None, // last_checkpoint_index + ) } /// Put the `value_state_sets` into its own CF. - pub fn put_value_sets( + pub fn put_value_sets<'a>( &self, - value_state_sets: &[ShardedStateUpdates], + write_sets: impl IntoIterator, first_version: Version, expected_usage: StateStorageUsage, sharded_state_cache: Option<&ShardedStateCache>, @@ -697,13 +680,15 @@ impl StateStore { enable_sharding: bool, last_checkpoint_index: Option, ) -> Result<()> { - let _timer = OTHER_TIMERS_SECONDS - .with_label_values(&["put_value_sets"]) - .start_timer(); + let _timer = OTHER_TIMERS_SECONDS.timer_with(&["put_value_sets"]); + + let (kv_updates_per_shard, num_versions) = + Self::get_sharded_kv_updates(first_version, write_sets); self.put_stats_and_indices( - value_state_sets, + &kv_updates_per_shard, first_version, + num_versions, expected_usage, sharded_state_cache, ledger_batch, @@ -712,49 +697,56 @@ impl StateStore { enable_sharding, )?; - let _timer = OTHER_TIMERS_SECONDS - .with_label_values(&["add_state_kv_batch"]) - .start_timer(); - self.put_state_values( - value_state_sets, - first_version, + &kv_updates_per_shard, sharded_state_kv_batches, enable_sharding, ) } + fn get_sharded_kv_updates<'a>( + first_version: Version, + write_sets: impl IntoIterator, + ) -> (ShardedKvUpdates, usize) { + let _timer = OTHER_TIMERS_SECONDS.timer_with(&["get_sharded_kv_updates"]); + + let mut updates_by_shard = empty_kv_updates(); + let num_versions = write_sets + .into_iter() + .enumerate() + .map(|(idx, write_set)| { + let version = first_version + idx as Version; + write_set.iter().for_each(|(state_key, write_op)| { + updates_by_shard[state_key.get_shard_id() as usize] + .push(((state_key.clone(), version), write_op.as_state_value())); + }); + }) + .count(); + + (updates_by_shard, num_versions) + } + pub fn put_state_values( &self, - value_state_sets: &[ShardedStateUpdates], - first_version: Version, + kv_updates_per_shard: &ShardedKvUpdates, sharded_state_kv_batches: &ShardedStateKvSchemaBatch, enable_sharding: bool, ) -> Result<()> { + let _timer = OTHER_TIMERS_SECONDS.timer_with(&["add_state_kv_batch"]); + sharded_state_kv_batches .par_iter() - .enumerate() - .try_for_each(|(shard_id, batch)| { - value_state_sets - .par_iter() - .enumerate() - .flat_map_iter(|(i, shards)| { - let version = first_version + i as Version; - let kvs = &shards[shard_id]; - kvs.iter().map(move |(k, v)| { - if enable_sharding { - batch.put::( - &(k.clone().hash(), version), - v, - ) - } else { - batch.put::(&(k.clone(), version), v) - } - }) - }) - .collect::>() - })?; - Ok(()) + .zip_eq(kv_updates_per_shard.par_iter()) + .try_for_each(|(batch, updates)| { + updates.iter().try_for_each(|(key_and_ver, val)| { + if enable_sharding { + let (key, ver) = key_and_ver; + batch.put::(&(CryptoHash::hash(key), *ver), val) + } else { + batch.put::(key_and_ver, val) + } + }) + }) } pub fn get_usage(&self, version: Option) -> Result { @@ -775,8 +767,9 @@ impl StateStore { /// extra stale index as 1 cover the latter case. pub fn put_stats_and_indices( &self, - value_state_sets: &[ShardedStateUpdates], + per_shard_kv_updates: &ShardedKvUpdates, first_version: Version, + num_versions: usize, expected_usage: StateStorageUsage, // If not None, it must contains all keys in the value_state_sets. // TODO(grao): Restructure this function. @@ -786,11 +779,7 @@ impl StateStore { last_checkpoint_index: Option, enable_sharding: bool, ) -> Result<()> { - let _timer = OTHER_TIMERS_SECONDS - .with_label_values(&["put_stats_and_indices"]) - .start_timer(); - - let num_versions = value_state_sets.len(); + let _timer = OTHER_TIMERS_SECONDS.timer_with(&["put_stats_and_indices"]); let base_version = first_version.checked_sub(1); let mut usage = self.get_usage(base_version)?; @@ -798,27 +787,29 @@ impl StateStore { let mut state_cache_with_version = &ShardedStateCache::default(); if let Some(base_version) = base_version { - let _timer = OTHER_TIMERS_SECONDS - .with_label_values(&["put_stats_and_indices__total_get"]) - .start_timer(); + let _timer = OTHER_TIMERS_SECONDS.timer_with(&["put_stats_and_indices__total_get"]); if let Some(sharded_state_cache) = sharded_state_cache { // For some entries the base value version is None, here is to fiil those in. // See `ShardedStateCache`. self.prepare_version_in_cache(base_version, sharded_state_cache)?; state_cache_with_version = sharded_state_cache; } else { - let key_set = value_state_sets - .iter() - .flat_map(|sharded_states| sharded_states.iter().flatten()) - .map(|(key, _)| key) - .collect::>(); + // TODO(aldenhu): get all updates from StateDelta directly + let key_set = { + let _timer = OTHER_TIMERS_SECONDS + .timer_with(&["put_stats_and_indices__get_all_updates"]); + per_shard_kv_updates + .iter() + .flatten() + .map(|((key, _ver), _val)| key) + .collect::>() + }; THREAD_MANAGER.get_high_pri_io_pool().scope(|s| { for key in key_set { let cache = state_cache_with_version.shard(key.get_shard_id()); s.spawn(move |_| { let _timer = OTHER_TIMERS_SECONDS - .with_label_values(&["put_stats_and_indices__get_state_value"]) - .start_timer(); + .timer_with(&["put_stats_and_indices__get_state_value"]); let version_and_value = self .state_db .get_state_value_with_version_by_version(key, base_version) @@ -834,26 +825,80 @@ impl StateStore { } } - let _timer = OTHER_TIMERS_SECONDS - .with_label_values(&["put_stats_and_indices__calculate_total_size"]) - .start_timer(); + let usage_deltas = Self::put_stale_state_value_index( + first_version, + num_versions, + per_shard_kv_updates, + sharded_state_kv_batches, + enable_sharding, + &mut state_cache_with_version, + ); + + for i in 0..num_versions { + let mut items_delta = 0; + let mut bytes_delta = 0; + for usage_delta in usage_deltas.iter() { + items_delta += usage_delta[i].0; + bytes_delta += usage_delta[i].1; + } + usage = StateStorageUsage::new( + (usage.items() as i64 + items_delta) as usize, + (usage.bytes() as i64 + bytes_delta) as usize, + ); + if (i == num_versions - 1) || Some(i) == last_checkpoint_index { + let version = first_version + i as u64; + info!("Write usage at version {version}, {usage:?}."); + batch.put::(&version, &usage.into())? + } + } + + if !expected_usage.is_untracked() { + ensure!( + expected_usage == usage, + "Calculated state db usage at version {} not expected. expected: {:?}, calculated: {:?}, base version: {:?}, base version usage: {:?}", + first_version + num_versions as u64 - 1, + expected_usage, + usage, + base_version, + base_version_usage, + ); + } + + STATE_ITEMS.set(usage.items() as i64); + TOTAL_STATE_BYTES.set(usage.bytes() as i64); + + Ok(()) + } + + fn put_stale_state_value_index( + first_version: Version, + num_versions: usize, + per_shard_kv_updates: &ShardedKvUpdates, + sharded_state_kv_batches: &ShardedStateKvSchemaBatch, + enable_sharding: bool, + state_cache_with_version: &mut &ShardedStateCache, + ) -> Vec> { + let _timer = OTHER_TIMERS_SECONDS.timer_with(&["put_stale_kv_index"]); // calculate total state size in bytes let usage_deltas: Vec> = state_cache_with_version .par_iter() + .zip_eq(per_shard_kv_updates.par_iter()) .enumerate() - .map(|(shard_id, cache)| { - let _timer = OTHER_TIMERS_SECONDS - .with_label_values(&[&format!( - "put_stats_and_indices__calculate_total_size__shard_{shard_id}" - )]) - .start_timer(); + .map(|(shard_id, (cache, kv_updates))| { + let _timer = + OTHER_TIMERS_SECONDS.timer_with(&[&format!("put_stale_kv_index__{shard_id}")]); + let mut usage_delta = Vec::with_capacity(num_versions); - for (idx, kvs) in value_state_sets.iter().enumerate() { - let version = first_version + idx as Version; + let mut iter = kv_updates.iter(); + + // TODO(aldenhu): no need to iter by version after we calcualte the usage elsewhere + for version in first_version..first_version + num_versions as Version { + let ver_iter = iter.take_while_ref(|((_, ver), _)| *ver == version); + let mut items_delta = 0; let mut bytes_delta = 0; - for (key, value) in kvs[shard_id].iter() { + for ((key, _ver), value) in ver_iter { if let Some(value) = value { items_delta += 1; bytes_delta += (key.size() + value.size()) as i64; @@ -931,41 +976,7 @@ impl StateStore { usage_delta }) .collect(); - - for i in 0..num_versions { - let mut items_delta = 0; - let mut bytes_delta = 0; - for usage_delta in usage_deltas.iter() { - items_delta += usage_delta[i].0; - bytes_delta += usage_delta[i].1; - } - usage = StateStorageUsage::new( - (usage.items() as i64 + items_delta) as usize, - (usage.bytes() as i64 + bytes_delta) as usize, - ); - if (i == num_versions - 1) || Some(i) == last_checkpoint_index { - let version = first_version + i as u64; - info!("Write usage at version {version}, {usage:?}."); - batch.put::(&version, &usage.into())? - } - } - - if !expected_usage.is_untracked() { - ensure!( - expected_usage == usage, - "Calculated state db usage at version {} not expected. expected: {:?}, calculated: {:?}, base version: {:?}, base version usage: {:?}", - first_version + value_state_sets.len() as u64 - 1, - expected_usage, - usage, - base_version, - base_version_usage, - ); - } - - STATE_ITEMS.set(usage.items() as i64); - TOTAL_STATE_BYTES.set(usage.bytes() as i64); - - Ok(()) + usage_deltas } pub(crate) fn shard_state_value_batch( diff --git a/storage/aptosdb/src/state_store/state_snapshot_committer.rs b/storage/aptosdb/src/state_store/state_snapshot_committer.rs index fa608ff32c2ea..59f9cabdb0ba3 100644 --- a/storage/aptosdb/src/state_store/state_snapshot_committer.rs +++ b/storage/aptosdb/src/state_store/state_snapshot_committer.rs @@ -4,6 +4,7 @@ //! This file defines the state snapshot committer running in background thread within StateStore. use crate::{ + common::NUM_STATE_SHARDS, metrics::OTHER_TIMERS_SECONDS, state_store::{ buffered_state::CommitMessage, @@ -12,14 +13,19 @@ use crate::{ }, versioned_node_cache::VersionedNodeCache, }; +use aptos_crypto::hash::CryptoHash; use aptos_experimental_runtimes::thread_manager::THREAD_MANAGER; use aptos_logger::trace; +use aptos_metrics_core::TimerHelper; use aptos_scratchpad::SmtAncestors; -use aptos_storage_interface::{jmt_update_refs, jmt_updates, state_delta::StateDelta, Result}; +use aptos_storage_interface::{jmt_update_refs, state_delta::StateDelta, Result}; use aptos_types::state_store::state_value::StateValue; +use arr_macro::arr; +use itertools::Itertools; use rayon::prelude::*; use static_assertions::const_assert; use std::{ + collections::HashMap, sync::{ mpsc, mpsc::{Receiver, SyncSender}, @@ -95,8 +101,25 @@ impl StateSnapshotCommitter { .get_shard_persisted_versions(base_version) .unwrap(); + // TODO(aldenhu): avoid collecting to per shard updates before hand + let jmt_updates_by_shard = { + let _timer = + OTHER_TIMERS_SECONDS.timer_with(&["get_sharded_jmt_updates"]); + let mut ret = arr![HashMap::new(); 16]; + delta_to_commit + .updates_since_base + .iter() + .for_each(|(key, val_opt)| { + ret[key.get_shard_id() as usize].insert( + key.hash(), + val_opt.as_ref().map(|v| (v.hash(), key.clone())), + ); + }); + ret.map(|hash_map| hash_map.into_iter().collect_vec()) + }; + THREAD_MANAGER.get_non_exe_cpu_pool().install(|| { - (0..16) + (0..NUM_STATE_SHARDS as u8) .into_par_iter() .map(|shard_id| { let node_hashes = delta_to_commit @@ -104,12 +127,7 @@ impl StateSnapshotCommitter { .new_node_hashes_since(&delta_to_commit.base, shard_id); self.state_db.state_merkle_db.merklize_value_set_for_shard( shard_id, - jmt_update_refs(&jmt_updates( - &delta_to_commit.updates_since_base[shard_id as usize] - .iter() - .map(|(k, v)| (k, v.as_ref())) - .collect(), - )), + jmt_update_refs(&jmt_updates_by_shard[shard_id as usize]), Some(&node_hashes), version, base_version, diff --git a/storage/aptosdb/src/state_store/state_store_test.rs b/storage/aptosdb/src/state_store/state_store_test.rs index 5abad36344546..058f058a405ec 100644 --- a/storage/aptosdb/src/state_store/state_store_test.rs +++ b/storage/aptosdb/src/state_store/state_store_test.rs @@ -25,7 +25,6 @@ use aptos_types::{ state_store::state_key::inner::StateKeyTag, AptosCoinType, }; -use arr_macro::arr; use proptest::{collection::hash_map, prelude::*}; use std::collections::HashMap; @@ -35,13 +34,11 @@ fn put_value_set( version: Version, base_version: Option, ) -> HashValue { - let mut sharded_value_set = arr![HashMap::new(); 16]; + let write_set = + WriteSet::new_for_test(value_set.iter().map(|(k, v)| (k.clone(), Some(v.clone())))); let value_set: HashMap<_, _> = value_set .iter() - .map(|(key, value)| { - sharded_value_set[key.get_shard_id() as usize].insert(key.clone(), Some(value.clone())); - (key, Some(value)) - }) + .map(|(key, value)| (key, Some(value))) .collect(); let jmt_updates = jmt_updates(&value_set); @@ -53,7 +50,7 @@ fn put_value_set( let state_kv_metadata_batch = SchemaBatch::new(); state_store .put_value_sets( - &[sharded_value_set], + &[write_set], version, StateStorageUsage::new_untracked(), None, diff --git a/storage/scratchpad/src/sparse_merkle/mod.rs b/storage/scratchpad/src/sparse_merkle/mod.rs index 6b9f35e145439..afff0bd188caf 100644 --- a/storage/scratchpad/src/sparse_merkle/mod.rs +++ b/storage/scratchpad/src/sparse_merkle/mod.rs @@ -492,9 +492,9 @@ where /// all at once. /// Since the tree is immutable, existing tree remains the same and may share parts with the /// new, returned tree. - pub fn batch_update( + pub fn batch_update<'a>( &self, - updates: Vec<(HashValue, Option<&V>)>, + updates: impl IntoIterator)>, usage: StateStorageUsage, proof_reader: &impl ProofRead, ) -> Result { diff --git a/storage/storage-interface/src/chunk_to_commit.rs b/storage/storage-interface/src/chunk_to_commit.rs index 77a013f26524f..9923be11e3fb7 100644 --- a/storage/storage-interface/src/chunk_to_commit.rs +++ b/storage/storage-interface/src/chunk_to_commit.rs @@ -3,9 +3,10 @@ use crate::{cached_state_view::ShardedStateCache, state_delta::StateDelta}; use aptos_types::{ - state_store::ShardedStateUpdates, + state_store::{state_key::StateKey, state_value::StateValue}, transaction::{Transaction, TransactionInfo, TransactionOutput, Version}, }; +use std::collections::HashMap; #[derive(Clone)] pub struct ChunkToCommit<'a> { @@ -16,8 +17,7 @@ pub struct ChunkToCommit<'a> { pub transaction_infos: &'a [TransactionInfo], pub base_state_version: Option, pub latest_in_memory_state: &'a StateDelta, - pub per_version_state_updates: &'a [ShardedStateUpdates], - pub state_updates_until_last_checkpoint: Option<&'a ShardedStateUpdates>, + pub state_updates_until_last_checkpoint: Option<&'a HashMap>>, pub sharded_state_cache: Option<&'a ShardedStateCache>, pub is_reconfig: bool, } diff --git a/storage/storage-interface/src/state_delta.rs b/storage/storage-interface/src/state_delta.rs index ea93d8afa910d..8d2ee191a3da4 100644 --- a/storage/storage-interface/src/state_delta.rs +++ b/storage/storage-interface/src/state_delta.rs @@ -6,11 +6,11 @@ use aptos_drop_helper::DropHelper; use aptos_scratchpad::SparseMerkleTree; use aptos_types::{ state_store::{ - combine_sharded_state_updates, create_empty_sharded_state_updates, - state_storage_usage::StateStorageUsage, state_value::StateValue, ShardedStateUpdates, + state_key::StateKey, state_storage_usage::StateStorageUsage, state_value::StateValue, }, transaction::Version, }; +use std::{collections::HashMap, ops::DerefMut}; /// This represents two state sparse merkle trees at their versions in memory with the updates /// reflecting the difference of `current` on top of `base`. @@ -25,7 +25,7 @@ pub struct StateDelta { pub base_version: Option, pub current: SparseMerkleTree, pub current_version: Option, - pub updates_since_base: DropHelper, + pub updates_since_base: DropHelper>>, } impl StateDelta { @@ -34,7 +34,7 @@ impl StateDelta { base_version: Option, current: SparseMerkleTree, current_version: Option, - updates_since_base: ShardedStateUpdates, + updates_since_base: HashMap>, ) -> Self { assert!(base.is_family(¤t)); assert!(base_version.map_or(0, |v| v + 1) <= current_version.map_or(0, |v| v + 1)); @@ -49,13 +49,7 @@ impl StateDelta { pub fn new_empty() -> Self { let smt = SparseMerkleTree::new_empty(); - Self::new( - smt.clone(), - None, - smt, - None, - create_empty_sharded_state_updates(), - ) + Self::new(smt.clone(), None, smt, None, HashMap::new()) } pub fn new_at_checkpoint( @@ -69,13 +63,14 @@ impl StateDelta { checkpoint_version, smt, checkpoint_version, - create_empty_sharded_state_updates(), + HashMap::new(), ) } - pub fn merge(&mut self, other: StateDelta) { + pub fn merge(&mut self, mut other: StateDelta) { assert!(other.follow(self)); - combine_sharded_state_updates(&mut self.updates_since_base, &other.updates_since_base); + self.updates_since_base + .extend(other.updates_since_base.deref_mut().drain()); self.current = other.current; self.current_version = other.current_version; diff --git a/types/src/proptest_types.rs b/types/src/proptest_types.rs index fb26fdf7ce988..4338137b5ffac 100644 --- a/types/src/proptest_types.rs +++ b/types/src/proptest_types.rs @@ -19,7 +19,7 @@ use crate::{ ledger_info::{generate_ledger_info_with_sig, LedgerInfo, LedgerInfoWithSignatures}, on_chain_config::{Features, ValidatorSet}, proof::TransactionInfoListWithProof, - state_store::{state_key::StateKey, state_value::StateValue}, + state_store::state_key::StateKey, transaction::{ block_epilogue::BlockEndInfo, ChangeSet, ExecutionStatus, Module, RawTransaction, Script, SignatureCheckedTransaction, SignedTransaction, Transaction, TransactionArgument, @@ -41,8 +41,6 @@ use aptos_crypto::{ traits::*, HashValue, }; -use arr_macro::arr; -use bytes::Bytes; use move_core_types::language_storage::TypeTag; use proptest::{ collection::{vec, SizeRange}, @@ -53,7 +51,7 @@ use proptest::{ use proptest_derive::Arbitrary; use serde_json::Value; use std::{ - collections::{BTreeMap, BTreeSet, HashMap}, + collections::{BTreeMap, BTreeSet}, iter::Iterator, sync::Arc, }; @@ -793,33 +791,22 @@ impl TransactionToCommitGen { .map(|(index, event_gen)| event_gen.materialize(index, universe)) .collect(); - let (state_updates, write_set): (HashMap<_, _>, BTreeMap<_, _>) = self + let write_set: BTreeMap<_, _> = self .account_state_gens .into_iter() .flat_map(|(index, account_gen)| { account_gen.materialize(index, universe).into_iter().map( move |(state_key, value)| { - ( - ( - state_key.clone(), - Some(StateValue::new_legacy(Bytes::copy_from_slice(&value))), - ), - (state_key, WriteOp::legacy_modification(value.into())), - ) + (state_key, WriteOp::legacy_modification(value.into())) }, ) }) - .unzip(); - let mut sharded_state_updates = arr![HashMap::new(); 16]; - state_updates.into_iter().for_each(|(k, v)| { - sharded_state_updates[k.get_shard_id() as usize].insert(k, v); - }); + .collect(); TransactionToCommit::new( Transaction::UserTransaction(transaction), TransactionInfo::new_placeholder(self.gas_used, None, self.status), - sharded_state_updates, - WriteSetMut::new(write_set).freeze().expect("Cannot fail"), + WriteSet::new(write_set).unwrap(), events, false, /* event_gen never generates reconfig events */ TransactionAuxiliaryData::default(), @@ -1177,7 +1164,6 @@ impl BlockGen { Some(HashValue::random()), ExecutionStatus::Success, ), - arr![HashMap::new(); 16], WriteSet::default(), Vec::new(), false, diff --git a/types/src/transaction/mod.rs b/types/src/transaction/mod.rs index b557dfc00e877..a89e9d7137c0f 100644 --- a/types/src/transaction/mod.rs +++ b/types/src/transaction/mod.rs @@ -13,7 +13,6 @@ use crate::{ ledger_info::LedgerInfo, on_chain_config::{FeatureFlag, Features}, proof::{TransactionInfoListWithProof, TransactionInfoWithProof}, - state_store::ShardedStateUpdates, transaction::authenticator::{ AccountAuthenticator, AnyPublicKey, AnySignature, SingleKeyAuthenticator, TransactionAuthenticator, @@ -54,12 +53,15 @@ pub mod user_transaction_context; pub mod webauthn; pub use self::block_epilogue::{BlockEndInfo, BlockEpiloguePayload}; -#[cfg(any(test, feature = "fuzzing"))] -use crate::state_store::create_empty_sharded_state_updates; use crate::{ - block_metadata_ext::BlockMetadataExt, contract_event::TransactionEvent, executable::ModulePath, - fee_statement::FeeStatement, keyless::FederatedKeylessPublicKey, - proof::accumulator::InMemoryEventAccumulator, validator_txn::ValidatorTransaction, + block_metadata_ext::BlockMetadataExt, + contract_event::TransactionEvent, + executable::ModulePath, + fee_statement::FeeStatement, + keyless::FederatedKeylessPublicKey, + proof::accumulator::InMemoryEventAccumulator, + state_store::{state_key::StateKey, state_value::StateValue}, + validator_txn::ValidatorTransaction, write_set::TransactionWrite, }; pub use block_output::BlockOutput; @@ -1515,7 +1517,6 @@ impl Display for TransactionInfo { pub struct TransactionToCommit { pub transaction: Transaction, pub transaction_info: TransactionInfo, - pub state_updates: ShardedStateUpdates, pub write_set: WriteSet, pub events: Vec, pub is_reconfig: bool, @@ -1526,7 +1527,6 @@ impl TransactionToCommit { pub fn new( transaction: Transaction, transaction_info: TransactionInfo, - state_updates: ShardedStateUpdates, write_set: WriteSet, events: Vec, is_reconfig: bool, @@ -1535,7 +1535,6 @@ impl TransactionToCommit { TransactionToCommit { transaction, transaction_info, - state_updates, write_set, events, is_reconfig, @@ -1548,7 +1547,6 @@ impl TransactionToCommit { Self { transaction: Transaction::StateCheckpoint(HashValue::zero()), transaction_info: TransactionInfo::dummy(), - state_updates: create_empty_sharded_state_updates(), write_set: Default::default(), events: vec![], is_reconfig: false, @@ -1608,14 +1606,16 @@ impl TransactionToCommit { self.transaction_info = txn_info } - pub fn state_updates(&self) -> &ShardedStateUpdates { - &self.state_updates - } - pub fn write_set(&self) -> &WriteSet { &self.write_set } + pub fn state_updates(&self) -> impl IntoIterator)> + '_ { + self.write_set + .iter() + .map(|(key, op)| (key.clone(), op.as_state_value())) + } + pub fn events(&self) -> &[ContractEvent] { &self.events } diff --git a/types/src/write_set.rs b/types/src/write_set.rs index 41b5435badeb0..091dca68cd4dc 100644 --- a/types/src/write_set.rs +++ b/types/src/write_set.rs @@ -427,6 +427,27 @@ impl WriteSet { Self::V0(write_set) => write_set.0, } } + + pub fn new(write_ops: impl IntoIterator) -> Result { + WriteSetMut::new(write_ops).freeze() + } + + pub fn new_for_test(kvs: impl IntoIterator)>) -> Self { + Self::new(kvs.into_iter().map(|(k, v_opt)| { + ( + k, + v_opt.map_or_else(WriteOp::legacy_deletion, |v| { + WriteOp::legacy_modification(v.bytes().clone()) + }), + ) + })) + .expect("Must succeed") + } + + pub fn state_updates(&self) -> impl Iterator)> + '_ { + self.iter() + .map(|(key, op)| (key.clone(), op.as_state_value())) + } } impl Deref for WriteSet { From 866bb89a888f95015b1f2e0b73938b0ab936588a Mon Sep 17 00:00:00 2001 From: aldenhu Date: Tue, 29 Oct 2024 00:00:46 +0000 Subject: [PATCH 5/8] trivial: remove unused code --- .../src/state_checkpoint_output.rs | 50 ------------------- storage/storage-interface/src/state_delta.rs | 4 -- 2 files changed, 54 deletions(-) diff --git a/execution/executor-types/src/state_checkpoint_output.rs b/execution/executor-types/src/state_checkpoint_output.rs index c5faf719a1396..11911883288ea 100644 --- a/execution/executor-types/src/state_checkpoint_output.rs +++ b/execution/executor-types/src/state_checkpoint_output.rs @@ -3,65 +3,15 @@ #![forbid(unsafe_code)] -use crate::transactions_with_output::TransactionsWithOutput; use aptos_crypto::HashValue; use aptos_drop_helper::DropHelper; use aptos_storage_interface::state_delta::StateDelta; use aptos_types::{ state_store::{state_key::StateKey, state_value::StateValue}, - transaction::TransactionStatus, }; use derive_more::Deref; use std::{collections::HashMap, sync::Arc}; -#[derive(Default)] -pub struct TransactionsByStatus { - // Statuses of the input transactions, in the same order as the input transactions. - // Contains BlockMetadata/Validator transactions, - // but doesn't contain StateCheckpoint/BlockEpilogue, as those get added during execution - statuses_for_input_txns: Vec, - // List of all transactions to be committed, including StateCheckpoint/BlockEpilogue if needed. - to_commit: TransactionsWithOutput, - to_discard: TransactionsWithOutput, - to_retry: TransactionsWithOutput, -} - -impl TransactionsByStatus { - pub fn new( - statuses_for_input_txns: Vec, - to_commit: TransactionsWithOutput, - to_discard: TransactionsWithOutput, - to_retry: TransactionsWithOutput, - ) -> Self { - Self { - statuses_for_input_txns, - to_commit, - to_discard, - to_retry, - } - } - - pub fn input_txns_len(&self) -> usize { - self.statuses_for_input_txns.len() - } - - pub fn into_inner( - self, - ) -> ( - Vec, - TransactionsWithOutput, - TransactionsWithOutput, - TransactionsWithOutput, - ) { - ( - self.statuses_for_input_txns, - self.to_commit, - self.to_discard, - self.to_retry, - ) - } -} - #[derive(Clone, Debug, Default, Deref)] pub struct StateCheckpointOutput { #[deref] diff --git a/storage/storage-interface/src/state_delta.rs b/storage/storage-interface/src/state_delta.rs index 8d2ee191a3da4..3f1fc0d6b0ec4 100644 --- a/storage/storage-interface/src/state_delta.rs +++ b/storage/storage-interface/src/state_delta.rs @@ -85,10 +85,6 @@ impl StateDelta { && self.current.has_same_root_hash(&other.current) } - pub fn base_root_hash(&self) -> HashValue { - self.base.root_hash() - } - pub fn root_hash(&self) -> HashValue { self.current.root_hash() } From f713f1152c93653371008952f7f8ff27fc77f8c8 Mon Sep 17 00:00:00 2001 From: aldenhu Date: Mon, 28 Oct 2024 22:44:32 +0000 Subject: [PATCH 6/8] temp: CachedStateView / StateDelta etc --- Cargo.lock | 4 + .../executor-types/src/execution_output.rs | 20 +- .../src/transactions_with_output.rs | 1 + execution/executor/Cargo.toml | 1 + .../src/workflow/do_get_execution_output.rs | 38 ++- experimental/storage/layered-map/src/layer.rs | 26 +- .../storage/layered-map/src/map/mod.rs | 24 +- .../layered-map/src/map/new_layer_impl.rs | 8 +- storage/aptosdb/src/state_store/mod.rs | 2 +- storage/storage-interface/Cargo.toml | 3 + .../src/cached_state_view.rs | 286 ++++++------------ .../storage-interface/src/executed_trees.rs | 18 +- storage/storage-interface/src/lib.rs | 1 - storage/storage-interface/src/state_delta.rs | 139 ++++++--- 14 files changed, 286 insertions(+), 285 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e41ac4f746144..d6b0380cac29c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1459,6 +1459,7 @@ dependencies = [ "aptos-executor-service", "aptos-executor-test-helpers", "aptos-executor-types", + "aptos-experimental-layered-map", "aptos-experimental-runtimes", "aptos-genesis", "aptos-indexer-grpc-table-info", @@ -3894,6 +3895,7 @@ dependencies = [ "anyhow", "aptos-crypto", "aptos-drop-helper", + "aptos-experimental-layered-map", "aptos-experimental-runtimes", "aptos-logger", "aptos-metrics-core", @@ -3905,6 +3907,8 @@ dependencies = [ "bcs 0.1.4", "crossbeam-channel", "dashmap", + "derive_more", + "itertools 0.13.0", "once_cell", "parking_lot 0.12.1", "proptest", diff --git a/execution/executor-types/src/execution_output.rs b/execution/executor-types/src/execution_output.rs index b9e2ac4806144..b4979a9c39b97 100644 --- a/execution/executor-types/src/execution_output.rs +++ b/execution/executor-types/src/execution_output.rs @@ -11,11 +11,12 @@ use aptos_types::{ contract_event::ContractEvent, epoch_state::EpochState, transaction::{ - block_epilogue::BlockEndInfo, ExecutionStatus, Transaction, TransactionStatus, Version, + block_epilogue::BlockEndInfo, Transaction, TransactionStatus, Version, }, }; use derive_more::Deref; use std::sync::Arc; +use aptos_storage_interface::state_delta::InMemState; #[derive(Clone, Debug, Deref)] pub struct ExecutionOutput { @@ -34,6 +35,8 @@ impl ExecutionOutput { state_cache: StateCache, block_end_info: Option, next_epoch_state: Option, + last_checkpoint_state: Option, + result_state: InMemState, subscribable_events: Planned>, ) -> Self { if is_block { @@ -58,6 +61,8 @@ impl ExecutionOutput { state_cache, block_end_info, next_epoch_state, + last_checkpoint_state, + result_state, subscribable_events, }) } @@ -73,11 +78,14 @@ impl ExecutionOutput { state_cache: StateCache::new_empty(state.current.clone()), block_end_info: None, next_epoch_state: None, + last_checkpoint_state: None, + result_state: InMemState::new_empty(), subscribable_events: Planned::ready(vec![]), }) } pub fn new_dummy_with_input_txns(txns: Vec) -> Self { + /* let num_txns = txns.len(); let success_status = TransactionStatus::Keep(ExecutionStatus::Success); Self::new_impl(Inner { @@ -92,6 +100,8 @@ impl ExecutionOutput { next_epoch_state: None, subscribable_events: Planned::ready(vec![]), }) + */ + todo!() // FIXME(aldenhu) } pub fn new_dummy() -> Self { @@ -99,6 +109,8 @@ impl ExecutionOutput { } pub fn reconfig_suffix(&self) -> Self { + todo!() // FIXME(aldenhu) + /* Self::new_impl(Inner { is_block: false, first_version: self.next_version(), @@ -111,6 +123,8 @@ impl ExecutionOutput { next_epoch_state: self.next_epoch_state.clone(), subscribable_events: Planned::ready(vec![]), }) + + */ } fn new_impl(inner: Inner) -> Self { @@ -156,6 +170,10 @@ pub struct Inner { /// state cache. pub next_epoch_state: Option, pub subscribable_events: Planned>, + /// n.b. For state sync chunks, it's possible that there's 0 to multiple state checkpoints in a + /// chunk, while for consensus the last transaction should always be a state checkpoint. + pub last_checkpoint_state: Option, + pub result_state: InMemState, } impl Inner { diff --git a/execution/executor-types/src/transactions_with_output.rs b/execution/executor-types/src/transactions_with_output.rs index 54204f6608d96..de88d8a6105f5 100644 --- a/execution/executor-types/src/transactions_with_output.rs +++ b/execution/executor-types/src/transactions_with_output.rs @@ -63,6 +63,7 @@ impl TransactionsWithOutput { &self.transaction_outputs } + // TODO(aldenhu): move to call site pub fn get_last_checkpoint_index(&self) -> Option { (0..self.len()) .rev() diff --git a/execution/executor/Cargo.toml b/execution/executor/Cargo.toml index 69d56c2f39bde..d1660a4efdcee 100644 --- a/execution/executor/Cargo.toml +++ b/execution/executor/Cargo.toml @@ -20,6 +20,7 @@ aptos-drop-helper = { workspace = true } aptos-executor-service = { workspace = true } aptos-executor-types = { workspace = true } aptos-experimental-runtimes = { workspace = true } +aptos-experimental-layered-map = { workspace = true } aptos-indexer-grpc-table-info = { workspace = true } aptos-infallible = { workspace = true } aptos-logger = { workspace = true } diff --git a/execution/executor/src/workflow/do_get_execution_output.rs b/execution/executor/src/workflow/do_get_execution_output.rs index c2c34a0d4e0f7..6bfd0ba695e40 100644 --- a/execution/executor/src/workflow/do_get_execution_output.rs +++ b/execution/executor/src/workflow/do_get_execution_output.rs @@ -5,6 +5,7 @@ use crate::{ metrics, metrics::{EXECUTOR_ERRORS, OTHER_TIMERS}, }; +use aptos_experimental_layered_map::LayeredMap; use anyhow::{anyhow, Result}; use aptos_crypto::HashValue; use aptos_executor_service::{ @@ -40,6 +41,7 @@ use aptos_types::{ use aptos_vm::VMExecutor; use itertools::Itertools; use std::{iter, sync::Arc}; +use aptos_storage_interface::state_delta::{InMemState, StateDelta, StateUpdate}; pub struct DoGetExecutionOutput; @@ -95,7 +97,7 @@ impl DoGetExecutionOutput { state_view.next_version(), transactions.into_iter().map(|t| t.into_inner()).collect(), transaction_outputs, - state_view.into_state_cache(), + state_view.seal(), block_end_info, append_state_checkpoint_to_block, ) @@ -126,7 +128,7 @@ impl DoGetExecutionOutput { .map(|t| t.into_txn().into_inner()) .collect(), transaction_outputs, - state_view.into_state_cache(), + state_view.seal(), None, // block end info append_state_checkpoint_to_block, ) @@ -150,7 +152,7 @@ impl DoGetExecutionOutput { state_view.next_version(), transactions, transaction_outputs, - state_view.into_state_cache(), + state_view.seal(), None, // block end info None, // append state checkpoint to block )?; @@ -312,6 +314,7 @@ impl Parser { .then(|| Self::ensure_next_epoch_state(&to_commit)) .transpose()? }; + let (last_checkpoint_state, result_state) = Self::update_state(&to_commit, &state_cache); let out = ExecutionOutput::new( is_block, @@ -323,6 +326,8 @@ impl Parser { state_cache, block_end_info, next_epoch_state, + last_checkpoint_state, + result_state, Planned::place_holder(), ); let ret = out.clone(); @@ -481,6 +486,33 @@ impl Parser { (&validator_set).into(), )) } + + fn update_state( + to_commit: &TransactionsWithOutput, + state_cache: &StateCache, + ) -> (Option, InMemState) { + let mut write_sets = to_commit + .transaction_outputs() + .iter() + .map(TransactionOutput::write_set); + if let Some(idx) = to_commit.get_last_checkpoint_index() { + let last_checkpoint_state = state_cache.speculative_state.update( + write_sets.by_ref().take(idx + 1) + ); + let result_state = if idx + 1 == to_commit.len() { + last_checkpoint_state.clone() + } else { + StateDelta::new( + state_cache.speculative_state.base.clone(), + last_checkpoint_state.clone(), + ).update(write_sets) + }; + (Some(last_checkpoint_state), result_state) + } else { + let result_state = state_cache.speculative_state.update(write_sets); + (None, result_state) + } + } } struct WriteSetStateView<'a> { diff --git a/experimental/storage/layered-map/src/layer.rs b/experimental/storage/layered-map/src/layer.rs index 8806ef10bec1c..d52af1a75582f 100644 --- a/experimental/storage/layered-map/src/layer.rs +++ b/experimental/storage/layered-map/src/layer.rs @@ -19,10 +19,10 @@ struct LayerInner { children: Mutex>>>, use_case: &'static str, family: HashValue, - layer: u64, - // Base layer when self is created -- `self` won't even weak-link to a node created in - // the base or an older layer. - base_layer: u64, + layer_num: u64, + /// Base layer when self is created -- `self` won't even weak-link to a node created in + /// the base or an older layer. + base_layer_num: u64, } impl Drop for LayerInner { @@ -53,8 +53,8 @@ impl LayerInner { children: Mutex::new(Vec::new()), use_case, family, - layer: 0, - base_layer: 0, + layer_num: 0, + base_layer_num: 0, }) } @@ -64,8 +64,8 @@ impl LayerInner { children: Mutex::new(Vec::new()), use_case: self.use_case, family: self.family, - layer: self.layer + 1, - base_layer, + layer_num: self.layer_num + 1, + base_layer_num: base_layer, }); self.children.lock().push(child.clone()); child.log_layer("spawn"); @@ -78,7 +78,7 @@ impl LayerInner { } fn log_layer(&self, event: &'static str) { - LAYER.set_with(&[self.use_case, event], self.layer as i64); + LAYER.set_with(&[self.use_case, event], self.layer_num as i64); } } @@ -110,8 +110,8 @@ impl MapLayer { pub fn into_layers_view_after(self, base_layer: MapLayer) -> LayeredMap { assert!(base_layer.is_family(&self)); - assert!(base_layer.inner.layer >= self.inner.base_layer); - assert!(base_layer.inner.layer <= self.inner.layer); + assert!(base_layer.inner.layer_num >= self.inner.base_layer_num); + assert!(base_layer.inner.layer_num <= self.inner.layer_num); self.log_layer("view"); base_layer.log_layer("as_view_base"); @@ -135,8 +135,8 @@ impl MapLayer { Arc::ptr_eq(&self.inner, &other.inner) } - pub(crate) fn layer(&self) -> u64 { - self.inner.layer + pub(crate) fn layer_num(&self) -> u64 { + self.inner.layer_num } pub(crate) fn use_case(&self) -> &'static str { diff --git a/experimental/storage/layered-map/src/map/mod.rs b/experimental/storage/layered-map/src/map/mod.rs index 62605dfed47ab..234fce694edb4 100644 --- a/experimental/storage/layered-map/src/map/mod.rs +++ b/experimental/storage/layered-map/src/map/mod.rs @@ -33,18 +33,22 @@ where } } - pub fn unpack(self) -> (MapLayer, MapLayer) { + pub fn into_top_layer(self) -> MapLayer { let Self { - base_layer, + base_layer: _, top_layer, _hash_builder, } = self; - (base_layer, top_layer) + top_layer + } + + pub fn base_layer(&self) -> &MapLayer { + &self.base_layer } - pub(crate) fn base_layer(&self) -> u64 { - self.base_layer.layer() + pub(crate) fn base_layer_num(&self) -> u64 { + self.base_layer.layer_num() } } @@ -66,7 +70,7 @@ where foot = foot << 1 | bits.next().expect("bits exhausted") as usize; } - self.get_under_node(peak.expect_foot(foot, self.base_layer()), key, &mut bits) + self.get_under_node(peak.expect_foot(foot, self.base_layer_num()), key, &mut bits) } fn get_under_node( @@ -82,7 +86,7 @@ where match cur_node { NodeStrongRef::Empty => return None, NodeStrongRef::Leaf(leaf) => { - return leaf.get_value(key, self.base_layer()).cloned() + return leaf.get_value(key, self.base_layer_num()).cloned() }, NodeStrongRef::Internal(internal) => match bits.next() { None => { @@ -90,9 +94,9 @@ where }, Some(bit) => { if bit { - cur_node = internal.right.get_strong(self.base_layer()); + cur_node = internal.right.get_strong(self.base_layer_num()); } else { - cur_node = internal.left.get_strong(self.base_layer()); + cur_node = internal.left.get_strong(self.base_layer_num()); } }, }, @@ -111,6 +115,6 @@ where self.top_layer .peak() .into_feet_iter() - .flat_map(|node| DescendantIterator::new(node, self.base_layer())) + .flat_map(|node| DescendantIterator::new(node, self.base_layer_num())) } } diff --git a/experimental/storage/layered-map/src/map/new_layer_impl.rs b/experimental/storage/layered-map/src/map/new_layer_impl.rs index 84e5cd3105d8a..2329d36bdc409 100644 --- a/experimental/storage/layered-map/src/map/new_layer_impl.rs +++ b/experimental/storage/layered-map/src/map/new_layer_impl.rs @@ -40,16 +40,16 @@ where let height = Self::new_peak_height(self.top_layer.peak().num_leaves(), items.len()); let mut new_peak = FlattenPerfectTree::new_with_empty_nodes(height); let builder = SubTreeBuilder { - layer: self.top_layer.layer() + 1, - base_layer: self.base_layer(), + layer: self.top_layer.layer_num() + 1, + base_layer: self.base_layer_num(), depth: 0, - position_info: PositionInfo::new(self.top_layer.peak(), self.base_layer()), + position_info: PositionInfo::new(self.top_layer.peak(), self.base_layer_num()), output_position_info: OutputPositionInfo::new(new_peak.get_mut()), items: &items, }; builder.build().finalize(); - self.top_layer.spawn(new_peak, self.base_layer()) + self.top_layer.spawn(new_peak, self.base_layer_num()) } fn new_peak_height(previous_peak_feet: usize, items_in_new_layer: usize) -> usize { diff --git a/storage/aptosdb/src/state_store/mod.rs b/storage/aptosdb/src/state_store/mod.rs index 066a0f05bd9a8..0194b9d165834 100644 --- a/storage/aptosdb/src/state_store/mod.rs +++ b/storage/aptosdb/src/state_store/mod.rs @@ -578,7 +578,7 @@ impl StateStore { InMemoryStateCalculatorV2::calculate_for_write_sets_after_snapshot( // TODO(aldenhu): avoid cloning the HashMap inside. &Arc::new(buffered_state.current_state().clone()), - &latest_snapshot_state_view.into_state_cache(), + &latest_snapshot_state_view.seal(), last_checkpoint_index, &write_sets, )?; diff --git a/storage/storage-interface/Cargo.toml b/storage/storage-interface/Cargo.toml index d3bdb9fcbcae1..c0e1f26553218 100644 --- a/storage/storage-interface/Cargo.toml +++ b/storage/storage-interface/Cargo.toml @@ -16,6 +16,7 @@ rust-version = { workspace = true } anyhow = { workspace = true } aptos-crypto = { workspace = true } aptos-drop-helper = { workspace = true } +aptos-experimental-layered-map = { workspace = true } aptos-experimental-runtimes = { workspace = true } aptos-logger = { workspace = true } aptos-metrics-core = { workspace = true } @@ -26,6 +27,8 @@ aptos-vm = { workspace = true } bcs = { workspace = true } crossbeam-channel = { workspace = true } dashmap = { workspace = true } +derive_more = { workspace = true } +itertools = { workspace = true } once_cell = { workspace = true } parking_lot = { workspace = true } proptest = { workspace = true } diff --git a/storage/storage-interface/src/cached_state_view.rs b/storage/storage-interface/src/cached_state_view.rs index cf9571cf10c44..3ccdd9070cfa4 100644 --- a/storage/storage-interface/src/cached_state_view.rs +++ b/storage/storage-interface/src/cached_state_view.rs @@ -2,41 +2,67 @@ // SPDX-License-Identifier: Apache-2.0 use crate::{ - async_proof_fetcher::AsyncProofFetcher, metrics::TIMER, state_view::DbStateView, DbReader, + metrics::TIMER, + state_view::{DbStateView, }, + DbReader, }; -use aptos_crypto::{hash::CryptoHash, HashValue}; -use aptos_experimental_runtimes::thread_manager::THREAD_MANAGER; -use aptos_scratchpad::{FrozenSparseMerkleTree, SparseMerkleTree, StateStoreStatus}; use aptos_types::{ - proof::SparseMerkleProofExt, state_store::{ errors::StateviewError, state_key::StateKey, state_storage_usage::StateStorageUsage, state_value::StateValue, StateViewId, TStateView, }, transaction::Version, - write_set::WriteSet, }; use core::fmt; use dashmap::DashMap; -use once_cell::sync::Lazy; use parking_lot::RwLock; use rayon::iter::{IndexedParallelIterator, IntoParallelRefIterator}; use std::{ - collections::{HashMap, HashSet}, + collections::{HashMap, }, fmt::{Debug, Formatter}, sync::Arc, }; - -static IO_POOL: Lazy = Lazy::new(|| { - rayon::ThreadPoolBuilder::new() - .num_threads(32) - .thread_name(|index| format!("kv_reader_{}", index)) - .build() - .unwrap() -}); +use aptos_types::write_set::{TransactionWrite, WriteOp}; +use crate::state_delta::{InMemState, StateDelta, StateUpdate}; type Result = std::result::Result; -type StateCacheShard = DashMap, Option)>; +type StateCacheShard = DashMap; + +#[derive(Clone, Debug)] +pub enum CachedStateValue { + NonExistent, + Update(StateUpdate), +} + +impl CachedStateValue { + pub fn from_write_op(version: Version, write_op: &WriteOp) -> Self { + Self::new_update( + version, + write_op.as_state_value(), + ) + } + + pub fn from_db_result(db_result: Option<(Version, StateValue)>) -> Self { + match db_result { + Some((version, value)) => Self::new_update(version, Some(value)), + None => Self::NonExistent, + } + } + + pub fn new_update(version: Version, value: Option) -> Self { + Self::Update(StateUpdate { + version, + value, + }) + } + + pub fn value_opt(&self) -> Option { + match self { + Self::NonExistent => None, + Self::Update( StateUpdate {value, .. } ) => value.clone(), + } + } +} // Sharded by StateKey.get_shard_id(). The version in the value indicates there is an entry on that // version for the given StateKey, and the version is the maximum one which <= the base version. It @@ -48,33 +74,10 @@ pub struct ShardedStateCache { } impl ShardedStateCache { - pub fn combine(&mut self, rhs: Self) { - use rayon::prelude::*; - THREAD_MANAGER.get_exe_cpu_pool().install(|| { - self.shards - .par_iter_mut() - .zip_eq(rhs.shards.into_par_iter()) - .for_each(|(l, r)| { - for (k, (ver, val)) in r.into_iter() { - l.entry(k).or_insert((ver, val)); - } - }) - }); - } - pub fn shard(&self, shard_id: u8) -> &StateCacheShard { &self.shards[shard_id as usize] } - pub fn flatten(self) -> DashMap> { - // TODO(grao): Rethink the strategy for state sync, and optimize this. - self.shards - .into_iter() - .flatten() - .map(|(key, (_ver_opt, val_opt))| (key, val_opt)) - .collect() - } - pub fn par_iter(&self) -> impl IndexedParallelIterator { self.shards.par_iter() } @@ -86,57 +89,14 @@ pub struct CachedStateView { /// For logging and debugging purpose, identifies what this view is for. id: StateViewId, - next_version: Version, + /// The in-memory state on top of the snapshot. + speculative_state: StateDelta, - /// A readable snapshot in the persistent storage. - snapshot: Option<(Version, HashValue)>, + /// The persisted state, readable from the persistent storage. + reader: Arc, - /// The in-memory state on top of the snapshot. - speculative_state: FrozenSparseMerkleTree, - - /// The cache of verified account states from `reader` and `speculative_state_view`, - /// represented by a hashmap with an account address as key and a pair of an ordered - /// account state map and an an optional account state proof as value. When the VM queries an - /// `access_path`, this cache will first check whether `reader_cache` is hit. If hit, it - /// will return the corresponding value of that `access_path`; otherwise, the account state - /// will be loaded into the cache from scratchpad or persistent storage in order as a - /// deserialized ordered map and then be returned. If the VM queries this account again, - /// the cached data can be read directly without bothering storage layer. The proofs in - /// cache are needed by ScratchPad after VM execution to construct an in-memory sparse Merkle - /// tree. - /// ```text - /// +----------------------------+ - /// | In-memory SparseMerkleTree <------+ - /// +-------------^--------------+ | - /// | | - /// write sets | - /// | cached account state map - /// +-------+-------+ proof - /// | V M | | - /// +-------^-------+ | - /// | | - /// value of `account_address/path` | - /// | | - /// +---------------------------+---------------------+-------+ - /// | +-------------------------+---------------------+-----+ | - /// | | state_cache, state_key_to_proof_cache | | - /// | +---------------^---------------------------^---------+ | - /// | | | | - /// | state store values only state blob proof | - /// | | | | - /// | | | | - /// | +---------------+--------------+ +----------+---------+ | - /// | | speculative_state | | reader | | - /// | +------------------------------+ +--------------------+ | - /// +---------------------------------------------------------+ - /// ``` - /// Cache of state key to state value, which is used in case of fine grained storage object. - /// Eventually this should replace the `account_to_state_cache` as we deprecate account state blob - /// completely and migrate to fine grained storage. A value of None in this cache reflects that - /// the corresponding key has been deleted. This is a temporary hack until we support deletion - /// in JMT node. + /// State values (with update versions) read across the lifetime of the state view. sharded_state_cache: ShardedStateCache, - proof_fetcher: Arc, } impl Debug for CachedStateView { @@ -146,153 +106,83 @@ impl Debug for CachedStateView { } impl CachedStateView { - /// Constructs a [`CachedStateView`] with persistent state view in the DB and the in-memory - /// speculative state represented by `speculative_state`. The persistent state view is the - /// latest one preceding `next_version` + /// FIXME(aldenhu): consolidate pub fn new( id: StateViewId, + speculative_state: StateDelta, reader: Arc, - next_version: Version, - speculative_state: SparseMerkleTree, - proof_fetcher: Arc, ) -> Result { - // n.b. Freeze the state before getting the state snapshot, otherwise it's possible that - // after we got the snapshot, in-mem trees newer than it gets dropped before being frozen, - // due to a commit happening from another thread. - let base_smt = reader.get_buffered_state_base()?; - let speculative_state = speculative_state.freeze(&base_smt); - let snapshot = reader - .get_state_snapshot_before(next_version) - .map_err(Into::::into)?; - Ok(Self::new_impl( id, - next_version, - snapshot, speculative_state, - proof_fetcher, + reader, )) } pub fn new_impl( id: StateViewId, - next_version: Version, - snapshot: Option<(Version, HashValue)>, - speculative_state: FrozenSparseMerkleTree, - proof_fetcher: Arc, + speculative_state: StateDelta, + reader: Arc, ) -> Self { Self { id, - next_version, - snapshot, speculative_state, + reader, sharded_state_cache: ShardedStateCache::default(), - proof_fetcher, } } - pub fn prime_cache_by_write_set<'a, T: IntoIterator + Send>( - &self, - write_sets: T, - ) -> Result<()> { - IO_POOL.scope(|s| { - write_sets - .into_iter() - .flat_map(|write_set| write_set.iter()) - .map(|(key, _)| key) - .collect::>() - .into_iter() - .for_each(|key| { - s.spawn(move |_| { - self.get_state_value_bytes(key).expect("Must succeed."); - }) - }); - }); - Ok(()) - } - - pub fn into_state_cache(self) -> StateCache { + pub fn seal(self) -> StateCache { StateCache { - frozen_base: self.speculative_state, + speculative_state: self.speculative_state, sharded_state_cache: self.sharded_state_cache, - proofs: self.proof_fetcher.get_proof_cache(), - } - } - - fn get_version_and_state_value_internal( - &self, - state_key: &StateKey, - ) -> Result<(Option, Option)> { - // Do most of the work outside the write lock. - let key_hash = state_key.hash(); - match self.speculative_state.get(key_hash) { - StateStoreStatus::ExistsInScratchPad(value) => Ok((None, Some(value))), - StateStoreStatus::DoesNotExist => Ok((None, None)), - // Part of the tree is unknown, need to request proof for later usage (updating the tree) - StateStoreStatus::UnknownSubtreeRoot { - hash: subtree_root_hash, - depth: subtree_root_depth, - } => self.fetch_value_and_maybe_proof_in_snapshot( - state_key, - Some((subtree_root_hash, subtree_root_depth)), - ), - // Tree is known, but we only know the hash of the value, need to request the actual - // StateValue. - StateStoreStatus::UnknownValue => { - self.fetch_value_and_maybe_proof_in_snapshot(state_key, None) - }, } } - fn fetch_value_and_maybe_proof_in_snapshot( - &self, - state_key: &StateKey, - fetch_proof: Option<(HashValue, usize)>, - ) -> Result<(Option, Option)> { - let version_and_value_opt = match self.snapshot { - None => None, - Some((version, _root_hash)) => match fetch_proof { - None => self.proof_fetcher.fetch_state_value(state_key, version)?, - Some((subtree_root_hash, subtree_depth)) => self - .proof_fetcher - .fetch_state_value_with_version_and_schedule_proof_read( - state_key, - version, - subtree_depth, - Some(subtree_root_hash), - )?, - }, + fn get_uncached(&self, state_key: &StateKey) -> Result { + let ret = if let Some(state_update) = self.speculative_state.get(state_key) { + CachedStateValue::Update(state_update) + } else if let Some(base_version) = self.speculative_state.base_version() { + CachedStateValue::from_db_result( + self.reader + .get_state_value_with_version_by_version(state_key, base_version)?, + ) + } else { + CachedStateValue::NonExistent }; - Ok(match version_and_value_opt { - None => (None, None), - Some((version, value)) => (Some(version), Some(value)), - }) + Ok(ret) } pub fn next_version(&self) -> Version { - self.next_version + self.speculative_state.next_version() } } +/// FIXME(alden): remove, since parent_state is not useful? #[derive(Debug)] pub struct StateCache { - pub frozen_base: FrozenSparseMerkleTree, + /// The state being read from the `CachedStateView`. + pub speculative_state: StateDelta, + /// KVs got read at the latest_state version during the lifetime of the CachedStateView. pub sharded_state_cache: ShardedStateCache, - pub proofs: HashMap, } impl StateCache { - pub fn new_empty(smt: SparseMerkleTree) -> Self { + pub fn new_empty(_state: InMemState) -> Self { + /* FIXME(aldenhu) let frozen_base = smt.freeze(&smt); Self { frozen_base, sharded_state_cache: ShardedStateCache::default(), proofs: HashMap::new(), } + + */ + todo!() } pub fn new_dummy() -> Self { - Self::new_empty(SparseMerkleTree::new_empty()) + Self::new_empty(InMemState::new_empty()) } } @@ -305,30 +195,28 @@ impl TStateView for CachedStateView { fn get_state_value(&self, state_key: &StateKey) -> Result> { let _timer = TIMER.with_label_values(&["get_state_value"]).start_timer(); + // First check if the cache has the state value. - if let Some(version_and_value_opt) = self + if let Some(state_update) = self .sharded_state_cache .shard(state_key.get_shard_id()) .get(state_key) { - // This can return None, which means the value has been deleted from the DB. - let value_opt = &version_and_value_opt.1; - return Ok(value_opt.clone()); + return Ok(state_update.value_opt()); } - let version_and_state_value_option = - self.get_version_and_state_value_internal(state_key)?; + let state_update = self.get_uncached(state_key)?; // Update the cache if still empty - let new_version_and_value = self + let state_update = self .sharded_state_cache .shard(state_key.get_shard_id()) .entry(state_key.clone()) - .or_insert(version_and_state_value_option); - let value_opt = &new_version_and_value.1; - Ok(value_opt.clone()) + .or_insert(state_update); + Ok(state_update.value_opt()) } fn get_usage(&self) -> Result { - Ok(self.speculative_state.usage()) + // FIXME(aldenhu): Add `.usage()` to speculative state + todo!() } } diff --git a/storage/storage-interface/src/executed_trees.rs b/storage/storage-interface/src/executed_trees.rs index 58c6194fa8668..0a1aff53d981d 100644 --- a/storage/storage-interface/src/executed_trees.rs +++ b/storage/storage-interface/src/executed_trees.rs @@ -2,7 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 use crate::{ - async_proof_fetcher::AsyncProofFetcher, cached_state_view::CachedStateView, + cached_state_view::CachedStateView, state_delta::StateDelta, DbReader, }; use aptos_crypto::HashValue; @@ -53,7 +53,7 @@ impl ExecutedTrees { transaction_accumulator: Arc, ) -> Self { assert_eq!( - state.current_version.map_or(0, |v| v + 1), + state.next_version(), transaction_accumulator.num_leaves() ); Self { @@ -95,17 +95,11 @@ impl ExecutedTrees { pub fn verified_state_view( &self, - id: StateViewId, - reader: Arc, - proof_fetcher: Arc, + _id: StateViewId, + _reader: Arc, ) -> Result { - CachedStateView::new( - id, - reader, - self.transaction_accumulator.num_leaves(), - self.state.current.clone(), - proof_fetcher, - ) + // FIXME(aldenhu) + todo!() } } diff --git a/storage/storage-interface/src/lib.rs b/storage/storage-interface/src/lib.rs index d553fbed04080..a777bd53d258b 100644 --- a/storage/storage-interface/src/lib.rs +++ b/storage/storage-interface/src/lib.rs @@ -45,7 +45,6 @@ mod metrics; pub mod mock; pub mod state_delta; pub mod state_view; - use crate::chunk_to_commit::ChunkToCommit; use aptos_scratchpad::SparseMerkleTree; pub use aptos_types::block_info::BlockHeight; diff --git a/storage/storage-interface/src/state_delta.rs b/storage/storage-interface/src/state_delta.rs index 3f1fc0d6b0ec4..7d99a4f6ffa05 100644 --- a/storage/storage-interface/src/state_delta.rs +++ b/storage/storage-interface/src/state_delta.rs @@ -1,62 +1,82 @@ // Copyright © Aptos Foundation // SPDX-License-Identifier: Apache-2.0 +use derive_more::Deref; use aptos_crypto::HashValue; -use aptos_drop_helper::DropHelper; -use aptos_scratchpad::SparseMerkleTree; +use aptos_experimental_layered_map::{LayeredMap, MapLayer}; use aptos_types::{ state_store::{ - state_key::StateKey, state_storage_usage::StateStorageUsage, state_value::StateValue, + state_storage_usage::StateStorageUsage, state_value::StateValue, }, transaction::Version, }; -use std::{collections::HashMap, ops::DerefMut}; - -/// This represents two state sparse merkle trees at their versions in memory with the updates -/// reflecting the difference of `current` on top of `base`. -/// -/// The `base` is the state SMT that current is based on. -/// The `current` is the state SMT that results from applying updates_since_base on top of `base`. -/// `updates_since_base` tracks all those key-value pairs that's changed since `base`, useful -/// when the next checkpoint is calculated. +use aptos_types::state_store::state_key::StateKey; +use aptos_types::write_set::{TransactionWrite, WriteSet}; +use itertools::Itertools; + +#[derive(Clone, Debug)] +pub struct StateUpdate { + pub version: Version, + pub value: Option, +} + +impl StateUpdate { + pub fn new(version: Version, value: Option) -> Self { + Self { version, value } + } +} + #[derive(Clone, Debug)] +pub struct InMemState { + pub next_version: Version, + pub updates: MapLayer, + pub usage: StateStorageUsage, +} + +impl InMemState { + pub fn new_empty() -> Self { + // FIXME(aldenh): check call site and implement + todo!() + } +} + +/// Represents all state updates in the (base, current] range +#[derive(Clone, Debug, Deref)] pub struct StateDelta { - pub base: SparseMerkleTree, - pub base_version: Option, - pub current: SparseMerkleTree, - pub current_version: Option, - pub updates_since_base: DropHelper>>, + // exclusive + pub base: InMemState, + pub current: InMemState, + #[deref] + pub updates: LayeredMap, } impl StateDelta { pub fn new( - base: SparseMerkleTree, - base_version: Option, - current: SparseMerkleTree, - current_version: Option, - updates_since_base: HashMap>, + base: InMemState, + current: InMemState, ) -> Self { - assert!(base.is_family(¤t)); - assert!(base_version.map_or(0, |v| v + 1) <= current_version.map_or(0, |v| v + 1)); + let updates = current.updates.view_layers_after(&base.updates); Self { base, - base_version, current, - current_version, - updates_since_base: DropHelper::new(updates_since_base), + updates, } } pub fn new_empty() -> Self { + /* FIXME(aldenhu): let smt = SparseMerkleTree::new_empty(); Self::new(smt.clone(), None, smt, None, HashMap::new()) + */ + todo!() } pub fn new_at_checkpoint( - root_hash: HashValue, - usage: StateStorageUsage, - checkpoint_version: Option, + _root_hash: HashValue, + _usage: StateStorageUsage, + _checkpoint_version: Option, ) -> Self { + /* FIXME(aldenhu): let smt = SparseMerkleTree::new(root_hash, usage); Self::new( smt.clone(), @@ -65,38 +85,75 @@ impl StateDelta { checkpoint_version, HashMap::new(), ) + + */ + todo!() } - pub fn merge(&mut self, mut other: StateDelta) { + pub fn merge(&mut self, _other: StateDelta) { + /* FIXME(aldenhu): assert!(other.follow(self)); self.updates_since_base .extend(other.updates_since_base.deref_mut().drain()); self.current = other.current; self.current_version = other.current_version; + */ + todo!() } - pub fn follow(&self, other: &StateDelta) -> bool { - self.base_version == other.current_version && other.current.has_same_root_hash(&self.base) - } - - pub fn has_same_current_state(&self, other: &StateDelta) -> bool { + pub fn has_same_current_state(&self, _other: &StateDelta) -> bool { + /* FIXME(aldenhu): self.current_version == other.current_version && self.current.has_same_root_hash(&other.current) + */ + todo!() } - pub fn root_hash(&self) -> HashValue { - self.current.root_hash() + pub fn next_version(&self) -> Version { + self.current.next_version } - pub fn next_version(&self) -> Version { - self.current_version.map_or(0, |v| v + 1) + pub fn base_version(&self) -> Option { + self.base.next_version.checked_sub(1) } - pub fn replace_with(&mut self, mut rhs: Self) -> Self { + pub fn replace_with(&mut self, mut _rhs: Self) -> Self { + /* FIXME(aldenhu): std::mem::swap(self, &mut rhs); rhs + */ + todo!() } + + pub fn update<'a>( + &self, + write_sets: impl IntoIterator, + ) -> InMemState { + let mut next_version = self.next_version(); + let kvs = write_sets.into_iter().flat_map(|write_set| { + write_set.iter().map(move |(state_key, write_op)| { + let version = next_version; + next_version += 1; + (state_key.clone(), StateUpdate::new(version, write_op.as_state_value())) + }) + }).collect_vec(); + let updates = self.updates.new_layer(&kvs); + let usage = Self::caculate_usage(self.current.usage, &kvs); + + InMemState { + next_version, + updates, + usage, + } + } + + + fn caculate_usage(_base_usage: StateStorageUsage, _updates: &[(StateKey, StateUpdate)]) -> StateStorageUsage { + // FIXME(aldenhu) + todo!() + } + } impl Default for StateDelta { From 56064e1758141ea7770d77c037849299414dd772 Mon Sep 17 00:00:00 2001 From: aldenhu Date: Wed, 30 Oct 2024 18:15:58 +0000 Subject: [PATCH 7/8] temp --- .../executor-types/src/execution_output.rs | 58 ++++++++++------ .../src/state_checkpoint_output.rs | 34 ++++----- .../src/state_compute_result.rs | 2 +- execution/executor/src/block_executor/mod.rs | 68 +++++++++++------- .../types/in_memory_state_calculator_v2.rs | 41 +++-------- .../src/types/partial_state_compute_result.rs | 2 +- .../src/workflow/do_get_execution_output.rs | 17 +++-- .../src/workflow/do_state_checkpoint.rs | 69 ++++++++++++++++--- .../storage/layered-map/src/map/mod.rs | 6 +- storage/aptosdb/src/state_store/mod.rs | 2 +- .../src/cached_state_view.rs | 44 ++++-------- .../storage-interface/src/executed_trees.rs | 10 +-- storage/storage-interface/src/lib.rs | 13 +++- .../src/state_authenticator.rs | 29 ++++++++ storage/storage-interface/src/state_delta.rs | 55 +++++++++------ 15 files changed, 270 insertions(+), 180 deletions(-) create mode 100644 storage/storage-interface/src/state_authenticator.rs diff --git a/execution/executor-types/src/execution_output.rs b/execution/executor-types/src/execution_output.rs index b4979a9c39b97..f1274f64ead21 100644 --- a/execution/executor-types/src/execution_output.rs +++ b/execution/executor-types/src/execution_output.rs @@ -6,17 +6,17 @@ use crate::{planned::Planned, transactions_with_output::TransactionsWithOutput}; use aptos_drop_helper::DropHelper; -use aptos_storage_interface::{cached_state_view::StateCache, state_delta::StateDelta}; +use aptos_storage_interface::{ + cached_state_view::StateCache, + state_delta::{InMemState, StateDelta}, +}; use aptos_types::{ contract_event::ContractEvent, epoch_state::EpochState, - transaction::{ - block_epilogue::BlockEndInfo, Transaction, TransactionStatus, Version, - }, + transaction::{block_epilogue::BlockEndInfo, Transaction, TransactionStatus, Version}, }; use derive_more::Deref; use std::sync::Arc; -use aptos_storage_interface::state_delta::InMemState; #[derive(Clone, Debug, Deref)] pub struct ExecutionOutput { @@ -35,10 +35,17 @@ impl ExecutionOutput { state_cache: StateCache, block_end_info: Option, next_epoch_state: Option, + parent_state: InMemState, last_checkpoint_state: Option, result_state: InMemState, subscribable_events: Planned>, ) -> Self { + assert_eq!( + first_version, parent_state.next_version(), + ); + assert_eq!( + first_version + to_commit.len(), result_state.next_version(), + ); if is_block { // If it's a block, ensure it ends with state checkpoint. assert!( @@ -46,6 +53,9 @@ impl ExecutionOutput { || to_commit.is_empty() // reconfig suffix || to_commit.transactions.last().unwrap().is_non_reconfig_block_ending() ); + assert!( + last_checkpoint_state.is_none() + ); } else { // If it's not, there shouldn't be any transaction to be discarded or retried. assert!(to_discard.is_empty() && to_retry.is_empty()); @@ -61,13 +71,14 @@ impl ExecutionOutput { state_cache, block_end_info, next_epoch_state, + parent_state, last_checkpoint_state, result_state, subscribable_events, }) } - pub fn new_empty(state: Arc) -> Self { + pub fn new_empty(state: InMemState) -> Self { Self::new_impl(Inner { is_block: false, first_version: state.next_version(), @@ -78,8 +89,9 @@ impl ExecutionOutput { state_cache: StateCache::new_empty(state.current.clone()), block_end_info: None, next_epoch_state: None, + parent_state: state.clone(), last_checkpoint_state: None, - result_state: InMemState::new_empty(), + result_state: state, subscribable_events: Planned::ready(vec![]), }) } @@ -110,21 +122,21 @@ impl ExecutionOutput { pub fn reconfig_suffix(&self) -> Self { todo!() // FIXME(aldenhu) - /* - Self::new_impl(Inner { - is_block: false, - first_version: self.next_version(), - statuses_for_input_txns: vec![], - to_commit: TransactionsWithOutput::new_empty(), - to_discard: TransactionsWithOutput::new_empty(), - to_retry: TransactionsWithOutput::new_empty(), - state_cache: StateCache::new_dummy(), - block_end_info: None, - next_epoch_state: self.next_epoch_state.clone(), - subscribable_events: Planned::ready(vec![]), - }) + /* + Self::new_impl(Inner { + is_block: false, + first_version: self.next_version(), + statuses_for_input_txns: vec![], + to_commit: TransactionsWithOutput::new_empty(), + to_discard: TransactionsWithOutput::new_empty(), + to_retry: TransactionsWithOutput::new_empty(), + state_cache: StateCache::new_dummy(), + block_end_info: None, + next_epoch_state: self.next_epoch_state.clone(), + subscribable_events: Planned::ready(vec![]), + }) - */ + */ } fn new_impl(inner: Inner) -> Self { @@ -149,6 +161,7 @@ impl ExecutionOutput { #[derive(Debug)] pub struct Inner { pub is_block: bool, + // FIXME(aldenhu): redundant pub first_version: Version, // Statuses of the input transactions, in the same order as the input transactions. // Contains BlockMetadata/Validator transactions, @@ -159,6 +172,7 @@ pub struct Inner { pub to_discard: TransactionsWithOutput, pub to_retry: TransactionsWithOutput, + /// FIXME(aldenhu): is it useful now that we have the InMemState calculated already? /// Carries the frozen base state view, so all in-mem nodes involved won't drop before the /// execution result is processed; as well as all the accounts touched during execution, together /// with their proofs. @@ -170,6 +184,8 @@ pub struct Inner { /// state cache. pub next_epoch_state: Option, pub subscribable_events: Planned>, + + pub parent_state: InMemState, /// n.b. For state sync chunks, it's possible that there's 0 to multiple state checkpoints in a /// chunk, while for consensus the last transaction should always be a state checkpoint. pub last_checkpoint_state: Option, diff --git a/execution/executor-types/src/state_checkpoint_output.rs b/execution/executor-types/src/state_checkpoint_output.rs index 11911883288ea..419eef8bffdc6 100644 --- a/execution/executor-types/src/state_checkpoint_output.rs +++ b/execution/executor-types/src/state_checkpoint_output.rs @@ -5,10 +5,7 @@ use aptos_crypto::HashValue; use aptos_drop_helper::DropHelper; -use aptos_storage_interface::state_delta::StateDelta; -use aptos_types::{ - state_store::{state_key::StateKey, state_value::StateValue}, -}; +use aptos_storage_interface::{state_authenticator::StateAuthenticator, state_delta::StateDelta}; use derive_more::Deref; use std::{collections::HashMap, sync::Arc}; @@ -20,26 +17,30 @@ pub struct StateCheckpointOutput { impl StateCheckpointOutput { pub fn new( - parent_state: Arc, - result_state: Arc, - state_updates_before_last_checkpoint: Option>>, + parent_auth: StateAuthenticator, + last_checkpoint_auth: Option, + state_auth: StateAuthenticator, state_checkpoint_hashes: Vec>, ) -> Self { Self::new_impl(Inner { - parent_state, - result_state, - state_updates_before_last_checkpoint, + parent_auth, + last_checkpoint_auth, + state_auth, state_checkpoint_hashes, }) } pub fn new_empty(state: Arc) -> Self { + /* Self::new_impl(Inner { parent_state: state.clone(), - result_state: state, + state_authenticator: state, state_updates_before_last_checkpoint: None, state_checkpoint_hashes: vec![], }) + + */ + todo!() // FIXME(aldenhu) } pub fn new_dummy() -> Self { @@ -53,16 +54,15 @@ impl StateCheckpointOutput { } pub fn reconfig_suffix(&self) -> Self { - Self::new_empty(self.result_state.clone()) + Self::new_empty(self.state_auth.clone()) } } #[derive(Debug, Default)] pub struct Inner { - pub parent_state: Arc, - pub result_state: Arc, - pub state_updates_before_last_checkpoint: Option>>, + /// FIXME(aldenhu): see if it's useful + pub parent_auth: StateAuthenticator, + pub last_checkpoint_auth: Option, + pub state_auth: StateAuthenticator, pub state_checkpoint_hashes: Vec>, } - -impl Inner {} diff --git a/execution/executor-types/src/state_compute_result.rs b/execution/executor-types/src/state_compute_result.rs index aaa0b864afe63..f88f81471b6cf 100644 --- a/execution/executor-types/src/state_compute_result.rs +++ b/execution/executor-types/src/state_compute_result.rs @@ -158,7 +158,7 @@ impl StateComputeResult { transaction_outputs: self.execution_output.to_commit.transaction_outputs(), transaction_infos: &self.ledger_update_output.transaction_infos, base_state_version: self.state_checkpoint_output.parent_state.base_version, - latest_in_memory_state: &self.state_checkpoint_output.result_state, + latest_in_memory_state: &self.state_checkpoint_output.state_auth, state_updates_until_last_checkpoint: self .state_checkpoint_output .state_updates_before_last_checkpoint diff --git a/execution/executor/src/block_executor/mod.rs b/execution/executor/src/block_executor/mod.rs index 2ef3b126ce34f..96cef31ab5795 100644 --- a/execution/executor/src/block_executor/mod.rs +++ b/execution/executor/src/block_executor/mod.rs @@ -19,8 +19,8 @@ use crate::{ use anyhow::Result; use aptos_crypto::HashValue; use aptos_executor_types::{ - execution_output::ExecutionOutput, state_compute_result::StateComputeResult, - BlockExecutorTrait, ExecutorError, ExecutorResult, + execution_output::ExecutionOutput, state_checkpoint_output, + state_compute_result::StateComputeResult, BlockExecutorTrait, ExecutorError, ExecutorResult, }; use aptos_experimental_runtimes::thread_manager::THREAD_MANAGER; use aptos_infallible::RwLock; @@ -39,6 +39,7 @@ use aptos_types::{ }; use aptos_vm::AptosVM; use block_tree::BlockTree; +use bytes::Buf; use fail::fail_point; use std::{marker::PhantomData, sync::Arc}; @@ -128,7 +129,7 @@ where .read() .as_ref() .expect("BlockExecutor is not reset") - .execute_and_state_checkpoint(block, parent_block_id, onchain_config) + .execute_block(block, parent_block_id, onchain_config) } fn ledger_update( @@ -201,7 +202,7 @@ where self.block_tree.root_block().id } - fn execute_and_state_checkpoint( + fn execute_block( &self, block: ExecutableBlock, parent_block_id: HashValue, @@ -226,7 +227,7 @@ where "execute_block" ); let committed_block_id = self.committed_block_id(); - let (execution_output, state_checkpoint_output) = + let execution_output = if parent_block_id != committed_block_id && parent_output.has_reconfiguration() { // ignore reconfiguration suffix, even if the block is non-empty info!( @@ -245,10 +246,8 @@ where 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())), + parent_output.execution_output.result_state.clone(), + self.db.clone().reader().clone(), )? }; @@ -282,7 +281,6 @@ where (execution_output, state_checkpoint_output) }; let output = PartialStateComputeResult::new(execution_output); - output.set_state_checkpoint_output(state_checkpoint_output); let _ = self .block_tree @@ -311,8 +309,7 @@ where // At this point of time two things must happen // 1. The block tree must also have the current block id with or without the ledger update output. // 2. We must have the ledger update output of the parent block. - let parent_output = parent_block.output.expect_ledger_update_output(); - let parent_accumulator = parent_output.txn_accumulator(); + let parent_output = &parent_block.output; let block = block_vec.pop().expect("Must exist").unwrap(); let output = &block.output; parent_block.ensure_has_child(block_id)?; @@ -320,24 +317,43 @@ where return Ok(complete_result); } - let output = - if parent_block_id != committed_block_id && parent_block.output.has_reconfiguration() { - info!( - LogSchema::new(LogEntry::BlockExecutor).block_id(block_id), - "reconfig_descendant_block_received" - ); - parent_output.reconfig_suffix() - } else { - THREAD_MANAGER.get_non_exe_cpu_pool().install(|| { + if parent_block_id != committed_block_id && parent_output.has_reconfiguration() { + info!( + LogSchema::new(LogEntry::BlockExecutor).block_id(block_id), + "reconfig_descendant_block_received" + ); + output.set_state_checkpoint_output( + parent_output + .expect_state_checkpoint_output() + .reconfig_suffix(), + ); + output.set_ledger_update_output( + parent_output + .expect_ledger_update_output() + .reconfig_suffix(), + ); + } else { + output.set_state_checkpoint_output(DoStateCheckpoint::run( + &parent_output.execution_output, + &parent_output + .expect_state_checkpoint_output() + .state_auth, + None, // known_state_checkpoints + )?); + + output.set_ledger_update_output(THREAD_MANAGER.get_non_exe_cpu_pool().install( + || { DoLedgerUpdate::run( &output.execution_output, output.expect_state_checkpoint_output(), - parent_accumulator.clone(), + parent_output + .expect_ledger_update_output() + .transaction_accumulator + .clone(), ) - })? - }; - - block.output.set_ledger_update_output(output); + }, + )?); + }; Ok(block.output.expect_complete_result()) } diff --git a/execution/executor/src/types/in_memory_state_calculator_v2.rs b/execution/executor/src/types/in_memory_state_calculator_v2.rs index 7488277555f0e..2a610bb5aaa15 100644 --- a/execution/executor/src/types/in_memory_state_calculator_v2.rs +++ b/execution/executor/src/types/in_memory_state_calculator_v2.rs @@ -26,19 +26,22 @@ use dashmap::DashMap; use itertools::Itertools; use rayon::prelude::*; use std::{collections::HashMap, ops::Deref, sync::Arc}; +use aptos_storage_interface::state_authenticator::StateAuthenticator; +/// FIXME(aldenhu): rename /// Helper class for calculating state changes after a block of transactions are executed. pub struct InMemoryStateCalculatorV2 {} impl InMemoryStateCalculatorV2 { pub fn calculate_for_transactions( execution_output: &ExecutionOutput, - parent_state: &Arc, + parent_auth: &StateAuthenticator, known_state_checkpoints: Option>>, ) -> Result { - if execution_output.is_block { - Self::validate_input_for_block(parent_state, &execution_output.to_commit)?; - } + let state_updates_vec = Self::get_sharded_state_updates( + execution_output.to_commit.transaction_outputs(), + |txn_output| txn_output.write_set(), + ); // If there are multiple checkpoints in the chunk, we only calculate the SMT (and its root // hash) for the last one. @@ -129,6 +132,7 @@ impl InMemoryStateCalculatorV2 { parent_state.base.freeze(&frozen_base.base_smt) }; + /* let mut latest_checkpoint_version = parent_state.base_version; let mut state_checkpoint_hashes = known_state_checkpoints .map_or_else(|| vec![None; num_txns], |v| v.into_iter().collect()); @@ -147,6 +151,7 @@ impl InMemoryStateCalculatorV2 { } latest_checkpoint_version = Some(first_version + index as u64); } + */ let current_version = first_version + num_txns as u64 - 1; // We need to calculate the SMT at the end of the chunk, if it is not already calculated. @@ -293,32 +298,4 @@ impl InMemoryStateCalculatorV2 { let new_checkpoint = latest_checkpoint.batch_update(smt_updates, usage, proof_reader)?; Ok(new_checkpoint) } - - fn validate_input_for_block( - base: &StateDelta, - to_commit: &TransactionsWithOutput, - ) -> Result<()> { - let num_txns = to_commit.len(); - ensure!(num_txns != 0, "Empty block is not allowed."); - ensure!( - base.base_version == base.current_version, - "Block base state is not a checkpoint. base_version {:?}, current_version {:?}", - base.base_version, - base.current_version, - ); - ensure!( - base.updates_since_base.is_empty(), - "Base state is corrupted, updates_since_base is not empty at a checkpoint." - ); - - for (i, (txn, _txn_out, is_reconfig)) in to_commit.iter().enumerate() { - ensure!( - TransactionsWithOutput::need_checkpoint(txn, is_reconfig) ^ (i != num_txns - 1), - "Checkpoint is allowed iff it's the last txn in the block. index: {i}, num_txns: {num_txns}, is_last: {}, txn: {txn:?}, is_reconfig: {}", - i == num_txns - 1, - is_reconfig, - ); - } - Ok(()) - } } diff --git a/execution/executor/src/types/partial_state_compute_result.rs b/execution/executor/src/types/partial_state_compute_result.rs index a3167de3f65c8..553ad2f175ea2 100644 --- a/execution/executor/src/types/partial_state_compute_result.rs +++ b/execution/executor/src/types/partial_state_compute_result.rs @@ -64,7 +64,7 @@ impl PartialStateComputeResult { } pub fn expect_result_state(&self) -> &Arc { - &self.expect_state_checkpoint_output().result_state + &self.expect_state_checkpoint_output().state_auth } pub fn set_state_checkpoint_output(&self, state_checkpoint_output: StateCheckpointOutput) { diff --git a/execution/executor/src/workflow/do_get_execution_output.rs b/execution/executor/src/workflow/do_get_execution_output.rs index 6bfd0ba695e40..ebd7d0eddbed9 100644 --- a/execution/executor/src/workflow/do_get_execution_output.rs +++ b/execution/executor/src/workflow/do_get_execution_output.rs @@ -5,7 +5,6 @@ use crate::{ metrics, metrics::{EXECUTOR_ERRORS, OTHER_TIMERS}, }; -use aptos_experimental_layered_map::LayeredMap; use anyhow::{anyhow, Result}; use aptos_crypto::HashValue; use aptos_executor_service::{ @@ -16,10 +15,14 @@ use aptos_executor_types::{ execution_output::ExecutionOutput, planned::Planned, should_forward_to_subscription_service, transactions_with_output::TransactionsWithOutput, }; +use aptos_experimental_layered_map::LayeredMap; use aptos_experimental_runtimes::thread_manager::THREAD_MANAGER; use aptos_logger::prelude::*; use aptos_metrics_core::TimerHelper; -use aptos_storage_interface::cached_state_view::{CachedStateView, StateCache}; +use aptos_storage_interface::{ + cached_state_view::{CachedStateView, StateCache}, + state_delta::{InMemState, StateDelta, StateUpdate}, +}; use aptos_types::{ block_executor::{ config::BlockExecutorConfigFromOnchain, @@ -41,7 +44,6 @@ use aptos_types::{ use aptos_vm::VMExecutor; use itertools::Itertools; use std::{iter, sync::Arc}; -use aptos_storage_interface::state_delta::{InMemState, StateDelta, StateUpdate}; pub struct DoGetExecutionOutput; @@ -496,16 +498,17 @@ impl Parser { .iter() .map(TransactionOutput::write_set); if let Some(idx) = to_commit.get_last_checkpoint_index() { - let last_checkpoint_state = state_cache.speculative_state.update( - write_sets.by_ref().take(idx + 1) - ); + let last_checkpoint_state = state_cache + .speculative_state + .update(write_sets.by_ref().take(idx + 1)); let result_state = if idx + 1 == to_commit.len() { last_checkpoint_state.clone() } else { StateDelta::new( state_cache.speculative_state.base.clone(), last_checkpoint_state.clone(), - ).update(write_sets) + ) + .update(write_sets) }; (Some(last_checkpoint_state), result_state) } else { diff --git a/execution/executor/src/workflow/do_state_checkpoint.rs b/execution/executor/src/workflow/do_state_checkpoint.rs index a971ca2130a3a..70710e5c60da4 100644 --- a/execution/executor/src/workflow/do_state_checkpoint.rs +++ b/execution/executor/src/workflow/do_state_checkpoint.rs @@ -1,28 +1,77 @@ // Copyright (c) Aptos Foundation // SPDX-License-Identifier: Apache-2.0 -use crate::types::in_memory_state_calculator_v2::InMemoryStateCalculatorV2; -use anyhow::Result; +use std::sync::Arc; +use anyhow::{ensure, Result}; use aptos_crypto::HashValue; use aptos_executor_types::{ execution_output::ExecutionOutput, state_checkpoint_output::StateCheckpointOutput, }; -use aptos_storage_interface::state_delta::StateDelta; -use std::sync::Arc; +use aptos_storage_interface::state_authenticator::StateAuthenticator; +use crate::types::in_memory_state_calculator_v2::InMemoryStateCalculatorV2; pub struct DoStateCheckpoint; impl DoStateCheckpoint { pub fn run( execution_output: &ExecutionOutput, - parent_state: &Arc, + parent_auth: &StateAuthenticator, + persisted_auth: &StateAuthenticator, known_state_checkpoints: Option>>, ) -> Result { - // Apply the write set, get the latest state. - InMemoryStateCalculatorV2::calculate_for_transactions( - execution_output, - parent_state, - known_state_checkpoints, + let last_checkpoint_auth: Option; + let state_auth: StateAuthenticator; + + if let Some(last_checkpoint_state) = execution_output.last_checkpoint_state.as_ref() { + let before_checkpoint = last_checkpoint_state.clone().into_delta( + execution_output.parent_state.clone(), + ); + let after_checkpoint = execution_output.result_state.clone().into_delta( + last_checkpoint_state.clone(), + ); + + last_checkpoint_auth = Some(parent_auth.update( + persisted_auth, + &before_checkpoint, + )); + state_auth = last_checkpoint_auth.as_ref().unwrap().update( + persisted_auth, + &after_checkpoint, + ); + } else { + last_checkpoint_auth = None; + let updates = execution_output.result_state.clone().into_delta( + execution_output.parent_state.clone(), + ); + state_auth = parent_auth.update( + persisted_auth, + &updates, + ); + }; + + let mut state_checkpoint_hashes = known_state_checkpoints + .map_or_else(|| vec![None; num_txns], |v| v.into_iter().collect()); + ensure!( + state_checkpoint_hashes.len() == execution_output.to_commit.len(), + "Bad number of known hashes." + ); + if let Some(last_checkpoint_state) = &execution_output.last_checkpoint_state { + if let Some(h) = state_checkpoint_hashes[index] { + ensure!( + h == latest_checkpoint.root_hash(), + "Last checkpoint not expected." + ); + } else { + state_checkpoint_hashes[index] = Some(latest_checkpoint.root_hash()); + } + latest_checkpoint_version = Some(first_version + index as u64); + } + + StateCheckpointOutput::new( + parent_auth.clone(), + last_checkpoint_auth, + state_auth, + state_checkpoint_hashes, ) } } diff --git a/experimental/storage/layered-map/src/map/mod.rs b/experimental/storage/layered-map/src/map/mod.rs index 234fce694edb4..d56d17e0169a8 100644 --- a/experimental/storage/layered-map/src/map/mod.rs +++ b/experimental/storage/layered-map/src/map/mod.rs @@ -70,7 +70,11 @@ where foot = foot << 1 | bits.next().expect("bits exhausted") as usize; } - self.get_under_node(peak.expect_foot(foot, self.base_layer_num()), key, &mut bits) + self.get_under_node( + peak.expect_foot(foot, self.base_layer_num()), + key, + &mut bits, + ) } fn get_under_node( diff --git a/storage/aptosdb/src/state_store/mod.rs b/storage/aptosdb/src/state_store/mod.rs index 0194b9d165834..43dd87323385a 100644 --- a/storage/aptosdb/src/state_store/mod.rs +++ b/storage/aptosdb/src/state_store/mod.rs @@ -588,7 +588,7 @@ impl StateStore { state_checkpoint_output .state_updates_before_last_checkpoint .as_ref(), - &state_checkpoint_output.result_state, + &state_checkpoint_output.state_auth, true, /* sync_commit */ )?; } diff --git a/storage/storage-interface/src/cached_state_view.rs b/storage/storage-interface/src/cached_state_view.rs index 3ccdd9070cfa4..ce08da55a606d 100644 --- a/storage/storage-interface/src/cached_state_view.rs +++ b/storage/storage-interface/src/cached_state_view.rs @@ -3,7 +3,8 @@ use crate::{ metrics::TIMER, - state_view::{DbStateView, }, + state_delta::{InMemState, StateDelta, StateUpdate}, + state_view::DbStateView, DbReader, }; use aptos_types::{ @@ -12,18 +13,17 @@ use aptos_types::{ state_value::StateValue, StateViewId, TStateView, }, transaction::Version, + write_set::{TransactionWrite, WriteOp}, }; use core::fmt; use dashmap::DashMap; use parking_lot::RwLock; use rayon::iter::{IndexedParallelIterator, IntoParallelRefIterator}; use std::{ - collections::{HashMap, }, + collections::HashMap, fmt::{Debug, Formatter}, sync::Arc, }; -use aptos_types::write_set::{TransactionWrite, WriteOp}; -use crate::state_delta::{InMemState, StateDelta, StateUpdate}; type Result = std::result::Result; type StateCacheShard = DashMap; @@ -36,10 +36,7 @@ pub enum CachedStateValue { impl CachedStateValue { pub fn from_write_op(version: Version, write_op: &WriteOp) -> Self { - Self::new_update( - version, - write_op.as_state_value(), - ) + Self::new_update(version, write_op.as_state_value()) } pub fn from_db_result(db_result: Option<(Version, StateValue)>) -> Self { @@ -50,16 +47,13 @@ impl CachedStateValue { } pub fn new_update(version: Version, value: Option) -> Self { - Self::Update(StateUpdate { - version, - value, - }) + Self::Update(StateUpdate { version, value }) } pub fn value_opt(&self) -> Option { match self { Self::NonExistent => None, - Self::Update( StateUpdate {value, .. } ) => value.clone(), + Self::Update(StateUpdate { value, .. }) => value.clone(), } } } @@ -106,30 +100,16 @@ impl Debug for CachedStateView { } impl CachedStateView { - /// FIXME(aldenhu): consolidate - pub fn new( - id: StateViewId, - speculative_state: StateDelta, - reader: Arc, - ) -> Result { - Ok(Self::new_impl( - id, - speculative_state, - reader, - )) - } + pub fn new(id: StateViewId, state: InMemState, reader: Arc) -> Result { + let persisted_state = reader.get_persisted_state_before(&state)?; + let speculative_state = state.into_delta(persisted_state); - pub fn new_impl( - id: StateViewId, - speculative_state: StateDelta, - reader: Arc, - ) -> Self { - Self { + Ok(Self { id, speculative_state, reader, sharded_state_cache: ShardedStateCache::default(), - } + }) } pub fn seal(self) -> StateCache { diff --git a/storage/storage-interface/src/executed_trees.rs b/storage/storage-interface/src/executed_trees.rs index 0a1aff53d981d..69ebc71b681b9 100644 --- a/storage/storage-interface/src/executed_trees.rs +++ b/storage/storage-interface/src/executed_trees.rs @@ -1,10 +1,7 @@ // Copyright © Aptos Foundation // SPDX-License-Identifier: Apache-2.0 -use crate::{ - cached_state_view::CachedStateView, - state_delta::StateDelta, DbReader, -}; +use crate::{cached_state_view::CachedStateView, state_delta::StateDelta, DbReader}; use aptos_crypto::HashValue; use aptos_types::{ proof::accumulator::{InMemoryAccumulator, InMemoryTransactionAccumulator}, @@ -52,10 +49,7 @@ impl ExecutedTrees { state: Arc, transaction_accumulator: Arc, ) -> Self { - assert_eq!( - state.next_version(), - transaction_accumulator.num_leaves() - ); + assert_eq!(state.next_version(), transaction_accumulator.num_leaves()); Self { state, transaction_accumulator, diff --git a/storage/storage-interface/src/lib.rs b/storage/storage-interface/src/lib.rs index a777bd53d258b..6d3409af61130 100644 --- a/storage/storage-interface/src/lib.rs +++ b/storage/storage-interface/src/lib.rs @@ -43,14 +43,17 @@ mod executed_trees; mod metrics; #[cfg(any(test, feature = "fuzzing"))] pub mod mock; +pub mod state_authenticator; pub mod state_delta; pub mod state_view; -use crate::chunk_to_commit::ChunkToCommit; + +use crate::{chunk_to_commit::ChunkToCommit, state_delta::InMemState}; use aptos_scratchpad::SparseMerkleTree; pub use aptos_types::block_info::BlockHeight; use aptos_types::state_store::state_key::prefix::StateKeyPrefix; pub use errors::AptosDbError; pub use executed_trees::ExecutedTrees; +use crate::state_authenticator::StateAuthenticator; pub type Result = std::result::Result; // This is last line of defense against large queries slipping through external facing interfaces, @@ -383,6 +386,14 @@ pub trait DbReader: Send + Sync { /// Get the oldest in memory state tree. fn get_buffered_state_base(&self) -> Result>; + /// Get state at a version at or older than the parameter. + /// State older than the returned state can be queried from the DB. + fn get_persisted_state_before(&self, state: &InMemState) -> Result; + + /// Get state authenticator at a version at or older than the parameter. + /// State proofs older than the returned state can be queried from the DB. + fn get_persisted_state_authen_before(&self, state: &StateAuthenticator) -> Result; + /// Get the ledger info of the epoch that `known_version` belongs to. fn get_epoch_ending_ledger_info( &self, diff --git a/storage/storage-interface/src/state_authenticator.rs b/storage/storage-interface/src/state_authenticator.rs new file mode 100644 index 0000000000000..8d692c77f1557 --- /dev/null +++ b/storage/storage-interface/src/state_authenticator.rs @@ -0,0 +1,29 @@ +// Copyright (c) Aptos Foundation +// SPDX-License-Identifier: Apache-2.0 + +use aptos_scratchpad::SparseMerkleTree; +use aptos_types::state_store::state_key::StateKey; +use aptos_types::state_store::state_value::StateValue; +use crate::state_delta::{StateDelta, StateUpdate}; + +/// note: only a single field for now, more to be introduced later. +#[derive(Clone, Debug)] +pub struct StateAuthenticator { + pub global_state: SparseMerkleTree, +} + +impl StateAuthenticator { + pub fn new(global_state: SparseMerkleTree) -> Self { + Self { global_state } + } + + pub fn update( + &self, + _persisted_auth: &StateAuthenticator, + _state_delta: &StateDelta, + ) -> Self { + /// FIXME(aldenhu) + todo!() + } + +} diff --git a/storage/storage-interface/src/state_delta.rs b/storage/storage-interface/src/state_delta.rs index 7d99a4f6ffa05..8c20a0489fcf4 100644 --- a/storage/storage-interface/src/state_delta.rs +++ b/storage/storage-interface/src/state_delta.rs @@ -1,17 +1,16 @@ // Copyright © Aptos Foundation // SPDX-License-Identifier: Apache-2.0 -use derive_more::Deref; use aptos_crypto::HashValue; use aptos_experimental_layered_map::{LayeredMap, MapLayer}; use aptos_types::{ state_store::{ - state_storage_usage::StateStorageUsage, state_value::StateValue, + state_key::StateKey, state_storage_usage::StateStorageUsage, state_value::StateValue, }, transaction::Version, + write_set::{TransactionWrite, WriteSet}, }; -use aptos_types::state_store::state_key::StateKey; -use aptos_types::write_set::{TransactionWrite, WriteSet}; +use derive_more::Deref; use itertools::Itertools; #[derive(Clone, Debug)] @@ -38,6 +37,17 @@ impl InMemState { // FIXME(aldenh): check call site and implement todo!() } + + pub fn next_version(&self) -> Version { + self.next_version + } + + pub fn into_delta(self, base: InMemState) -> StateDelta { + StateDelta::new( + base, + self, + ) + } } /// Represents all state updates in the (base, current] range @@ -51,10 +61,7 @@ pub struct StateDelta { } impl StateDelta { - pub fn new( - base: InMemState, - current: InMemState, - ) -> Self { + pub fn new(base: InMemState, current: InMemState) -> Self { let updates = current.updates.view_layers_after(&base.updates); Self { base, @@ -111,7 +118,7 @@ impl StateDelta { } pub fn next_version(&self) -> Version { - self.current.next_version + self.current.next_version() } pub fn base_version(&self) -> Option { @@ -126,18 +133,21 @@ impl StateDelta { todo!() } - pub fn update<'a>( - &self, - write_sets: impl IntoIterator, - ) -> InMemState { + pub fn update<'a>(&self, write_sets: impl IntoIterator) -> InMemState { let mut next_version = self.next_version(); - let kvs = write_sets.into_iter().flat_map(|write_set| { - write_set.iter().map(move |(state_key, write_op)| { - let version = next_version; - next_version += 1; - (state_key.clone(), StateUpdate::new(version, write_op.as_state_value())) + let kvs = write_sets + .into_iter() + .flat_map(|write_set| { + write_set.iter().map(move |(state_key, write_op)| { + let version = next_version; + next_version += 1; + ( + state_key.clone(), + StateUpdate::new(version, write_op.as_state_value()), + ) + }) }) - }).collect_vec(); + .collect_vec(); let updates = self.updates.new_layer(&kvs); let usage = Self::caculate_usage(self.current.usage, &kvs); @@ -148,12 +158,13 @@ impl StateDelta { } } - - fn caculate_usage(_base_usage: StateStorageUsage, _updates: &[(StateKey, StateUpdate)]) -> StateStorageUsage { + fn caculate_usage( + _base_usage: StateStorageUsage, + _updates: &[(StateKey, StateUpdate)], + ) -> StateStorageUsage { // FIXME(aldenhu) todo!() } - } impl Default for StateDelta { From 58cde75157d3ba247305e0565cf623cdcbf0e153 Mon Sep 17 00:00:00 2001 From: aldenhu Date: Thu, 31 Oct 2024 17:07:22 +0000 Subject: [PATCH 8/8] temp --- .../executor-types/src/execution_output.rs | 16 ++--- execution/executor/src/block_executor/mod.rs | 4 +- .../types/in_memory_state_calculator_v2.rs | 2 +- .../src/workflow/do_state_checkpoint.rs | 66 ++++++++----------- .../aptosdb/src/db/include/aptosdb_writer.rs | 37 ++--------- .../aptosdb/src/state_store/buffered_state.rs | 15 +++-- .../src/cached_state_view.rs | 3 +- .../storage-interface/src/chunk_to_commit.rs | 13 ++-- storage/storage-interface/src/lib.rs | 11 +++- .../src/state_authenticator.rs | 34 ++++++---- storage/storage-interface/src/state_delta.rs | 21 +++--- 11 files changed, 109 insertions(+), 113 deletions(-) diff --git a/execution/executor-types/src/execution_output.rs b/execution/executor-types/src/execution_output.rs index f1274f64ead21..da00c67948f3e 100644 --- a/execution/executor-types/src/execution_output.rs +++ b/execution/executor-types/src/execution_output.rs @@ -8,7 +8,7 @@ use crate::{planned::Planned, transactions_with_output::TransactionsWithOutput}; use aptos_drop_helper::DropHelper; use aptos_storage_interface::{ cached_state_view::StateCache, - state_delta::{InMemState, StateDelta}, + state_delta::{InMemState}, }; use aptos_types::{ contract_event::ContractEvent, @@ -40,12 +40,10 @@ impl ExecutionOutput { result_state: InMemState, subscribable_events: Planned>, ) -> Self { - assert_eq!( - first_version, parent_state.next_version(), - ); - assert_eq!( - first_version + to_commit.len(), result_state.next_version(), - ); + assert_eq!(first_version, parent_state.next_version()); + let next_version = first_version + to_commit.len() as Version; + assert_eq!(next_version, result_state.next_version()); + if is_block { // If it's a block, ensure it ends with state checkpoint. assert!( @@ -53,9 +51,7 @@ impl ExecutionOutput { || to_commit.is_empty() // reconfig suffix || to_commit.transactions.last().unwrap().is_non_reconfig_block_ending() ); - assert!( - last_checkpoint_state.is_none() - ); + assert!(last_checkpoint_state.is_none()); } else { // If it's not, there shouldn't be any transaction to be discarded or retried. assert!(to_discard.is_empty() && to_retry.is_empty()); diff --git a/execution/executor/src/block_executor/mod.rs b/execution/executor/src/block_executor/mod.rs index 96cef31ab5795..9972e71d35813 100644 --- a/execution/executor/src/block_executor/mod.rs +++ b/execution/executor/src/block_executor/mod.rs @@ -335,9 +335,7 @@ where } else { output.set_state_checkpoint_output(DoStateCheckpoint::run( &parent_output.execution_output, - &parent_output - .expect_state_checkpoint_output() - .state_auth, + &parent_output.expect_state_checkpoint_output().state_auth, None, // known_state_checkpoints )?); diff --git a/execution/executor/src/types/in_memory_state_calculator_v2.rs b/execution/executor/src/types/in_memory_state_calculator_v2.rs index 2a610bb5aaa15..d8dbc478c9bc6 100644 --- a/execution/executor/src/types/in_memory_state_calculator_v2.rs +++ b/execution/executor/src/types/in_memory_state_calculator_v2.rs @@ -13,6 +13,7 @@ use aptos_metrics_core::TimerHelper; use aptos_scratchpad::FrozenSparseMerkleTree; use aptos_storage_interface::{ cached_state_view::{ShardedStateCache, StateCache}, + state_authenticator::StateAuthenticator, state_delta::StateDelta, }; use aptos_types::{ @@ -26,7 +27,6 @@ use dashmap::DashMap; use itertools::Itertools; use rayon::prelude::*; use std::{collections::HashMap, ops::Deref, sync::Arc}; -use aptos_storage_interface::state_authenticator::StateAuthenticator; /// FIXME(aldenhu): rename /// Helper class for calculating state changes after a block of transactions are executed. diff --git a/execution/executor/src/workflow/do_state_checkpoint.rs b/execution/executor/src/workflow/do_state_checkpoint.rs index 70710e5c60da4..830088700498d 100644 --- a/execution/executor/src/workflow/do_state_checkpoint.rs +++ b/execution/executor/src/workflow/do_state_checkpoint.rs @@ -1,14 +1,13 @@ // Copyright (c) Aptos Foundation // SPDX-License-Identifier: Apache-2.0 -use std::sync::Arc; use anyhow::{ensure, Result}; use aptos_crypto::HashValue; use aptos_executor_types::{ execution_output::ExecutionOutput, state_checkpoint_output::StateCheckpointOutput, }; use aptos_storage_interface::state_authenticator::StateAuthenticator; -use crate::types::in_memory_state_calculator_v2::InMemoryStateCalculatorV2; +use aptos_storage_interface::state_delta::InMemState; pub struct DoStateCheckpoint; @@ -19,59 +18,52 @@ impl DoStateCheckpoint { persisted_auth: &StateAuthenticator, known_state_checkpoints: Option>>, ) -> Result { - let last_checkpoint_auth: Option; - let state_auth: StateAuthenticator; + let state = &execution_output.result_state; + let parent_state = &execution_output.parent_state; + let last_checkpoint_state = execution_output.last_checkpoint_state.as_ref(); - if let Some(last_checkpoint_state) = execution_output.last_checkpoint_state.as_ref() { - let before_checkpoint = last_checkpoint_state.clone().into_delta( - execution_output.parent_state.clone(), - ); - let after_checkpoint = execution_output.result_state.clone().into_delta( - last_checkpoint_state.clone(), - ); + let last_checkpoint_auth: Option<_> = last_checkpoint_state.map(|state| { + let updates = state.clone().into_delta(parent_state.clone()); + parent_auth.update(persisted_auth, &updates) + }); - last_checkpoint_auth = Some(parent_auth.update( - persisted_auth, - &before_checkpoint, - )); - state_auth = last_checkpoint_auth.as_ref().unwrap().update( - persisted_auth, - &after_checkpoint, - ); + let state_auth = if Some(execution_output.next_version()) + == last_checkpoint_state.map(|s| s.next_version()) + { + last_checkpoint_auth.as_ref().unwrap().clone() } else { - last_checkpoint_auth = None; - let updates = execution_output.result_state.clone().into_delta( - execution_output.parent_state.clone(), - ); - state_auth = parent_auth.update( - persisted_auth, - &updates, - ); + let base = last_checkpoint_state.unwrap_or(&parent_state); + let updates = state.clone().into_delta(base.clone()); + parent_auth.update(persisted_auth, &updates) }; + let num_txns = execution_output.num_transactions_to_commit(); let mut state_checkpoint_hashes = known_state_checkpoints .map_or_else(|| vec![None; num_txns], |v| v.into_iter().collect()); ensure!( - state_checkpoint_hashes.len() == execution_output.to_commit.len(), + state_checkpoint_hashes.len() == num_txns, "Bad number of known hashes." ); - if let Some(last_checkpoint_state) = &execution_output.last_checkpoint_state { + if let Some(auth) = &last_checkpoint_auth { + let index = auth.next_version() - parent_auth.next_version() - 1; if let Some(h) = state_checkpoint_hashes[index] { - ensure!( - h == latest_checkpoint.root_hash(), - "Last checkpoint not expected." - ); + ensure!(h == auth.root_hash(), "Last checkpoint not expected."); } else { - state_checkpoint_hashes[index] = Some(latest_checkpoint.root_hash()); + state_checkpoint_hashes[index] = Some(auth.root_hash()); } - latest_checkpoint_version = Some(first_version + index as u64); } - StateCheckpointOutput::new( + assert_eq!( + last_checkpoint_state.as_ref().map(InMemState::next_version), + last_checkpoint_auth.as_ref().map(StateAuthenticator::next_version) + ); + assert_eq!(state.next_version(), state_auth.next_version()); + + Ok(StateCheckpointOutput::new( parent_auth.clone(), last_checkpoint_auth, state_auth, state_checkpoint_hashes, - ) + )) } } diff --git a/storage/aptosdb/src/db/include/aptosdb_writer.rs b/storage/aptosdb/src/db/include/aptosdb_writer.rs index 7d29cde563a1b..1ce765ae1828c 100644 --- a/storage/aptosdb/src/db/include/aptosdb_writer.rs +++ b/storage/aptosdb/src/db/include/aptosdb_writer.rs @@ -21,7 +21,7 @@ impl DbWriter for AptosDB { .expect("Concurrent committing detected."); let _timer = OTHER_TIMERS_SECONDS.timer_with(&["pre_commit_ledger"]); - chunk.latest_in_memory_state.current.log_generation("db_save"); + chunk.state_auth.global_state.log_generation("db_save"); self.pre_commit_validation(&chunk)?; let _new_root_hash = self.calculate_and_commit_ledger_and_state_kv( @@ -229,38 +229,16 @@ impl AptosDB { !chunk.is_empty(), "chunk is empty, nothing to save.", ); - ensure!( - Some(chunk.expect_last_version()) == chunk.latest_in_memory_state.current_version, - "the last_version {:?} to commit doesn't match the current_version {:?} in latest_in_memory_state", - chunk.expect_last_version(), - chunk.latest_in_memory_state.current_version.expect("Must exist"), - ); - let num_transactions_in_db = self.get_pre_committed_version()?.map_or(0, |v| v + 1); { - let buffered_state = self.state_store.buffered_state().lock(); + // FIXME(aldenhu): track outside of BufferedState + let locked = self.state_store.buffered_state().lock(); ensure!( - chunk.base_state_version == buffered_state.current_state().base_version, - "base_state_version {:?} does not equal to the base_version {:?} in buffered state with current version {:?}", - chunk.base_state_version, - buffered_state.current_state().base_version, - buffered_state.current_state().current_version, - ); - - // Ensure the incoming committing requests are always consecutive and the version in - // buffered state is consistent with that in db. - let next_version_in_buffered_state = buffered_state - .current_state() - .current_version - .map(|version| version + 1) - .unwrap_or(0); - ensure!(num_transactions_in_db == chunk.first_version && num_transactions_in_db == next_version_in_buffered_state, - "The first version passed in ({}), the next version in buffered state ({}) and the next version in db ({}) are inconsistent.", - chunk.first_version, - next_version_in_buffered_state, - num_transactions_in_db, + locked.current_state().is_the_same(chunk.parent_state), + "Not committing consecutively." ); } + // FIXME(aldenhu): check parent accumulator, parent state, and parent auth Ok(()) } @@ -341,11 +319,10 @@ impl AptosDB { let sharded_state_kv_batches = new_sharded_kv_schema_batch(); let state_kv_metadata_batch = SchemaBatch::new(); - // TODO(grao): Make state_store take sharded state updates. self.state_store.put_value_sets( chunk.transaction_outputs.iter().map(TransactionOutput::write_set), chunk.first_version, - chunk.latest_in_memory_state.current.usage(), + chunk.state.usage(), chunk.sharded_state_cache, &ledger_metadata_batch, &sharded_state_kv_batches, diff --git a/storage/aptosdb/src/state_store/buffered_state.rs b/storage/aptosdb/src/state_store/buffered_state.rs index 2b9236527f4c7..483ed1cd51ac1 100644 --- a/storage/aptosdb/src/state_store/buffered_state.rs +++ b/storage/aptosdb/src/state_store/buffered_state.rs @@ -23,6 +23,8 @@ use std::{ }, thread::JoinHandle, }; +use aptos_storage_interface::state_authenticator::StateAuthenticator; +use aptos_storage_interface::state_delta::InMemState; pub(crate) const ASYNC_COMMIT_CHANNEL_BUFFER_SIZE: u64 = 1; pub(crate) const TARGET_SNAPSHOT_INTERVAL_IN_VERSION: u64 = 100_000; @@ -35,10 +37,13 @@ pub(crate) const TARGET_SNAPSHOT_INTERVAL_IN_VERSION: u64 = 100_000; /// state_until_checkpoint.current = state_after_checkpoint.base, same for their versions. #[derive(Debug)] pub struct BufferedState { - // state until the latest checkpoint. - state_until_checkpoint: Option>, - // state after the latest checkpoint. - state_after_checkpoint: StateDelta, + committed_state: InMemState, + committed_auth: StateAuthenticator, + last_checkpoint_state: InMemState, + last_checkpoint_auth: StateAuthenticator, + latest_state: InMemState, + latest_auth: StateAuthenticator, + state_commit_sender: SyncSender>>, target_items: usize, join_handle: Option>, @@ -85,7 +90,7 @@ impl BufferedState { (myself, smt_ancestors) } - pub fn current_state(&self) -> &StateDelta { + pub fn current_state(&self) -> &StateAuthenticator { &self.state_after_checkpoint } diff --git a/storage/storage-interface/src/cached_state_view.rs b/storage/storage-interface/src/cached_state_view.rs index ce08da55a606d..899a3b5b97014 100644 --- a/storage/storage-interface/src/cached_state_view.rs +++ b/storage/storage-interface/src/cached_state_view.rs @@ -143,7 +143,8 @@ impl CachedStateView { pub struct StateCache { /// The state being read from the `CachedStateView`. pub speculative_state: StateDelta, - /// KVs got read at the latest_state version during the lifetime of the CachedStateView. + /// KVs got read from the DB at the base of `speculative_state` during the lifetime of the + /// CachedStateView pub sharded_state_cache: ShardedStateCache, } diff --git a/storage/storage-interface/src/chunk_to_commit.rs b/storage/storage-interface/src/chunk_to_commit.rs index 9923be11e3fb7..21d148f4e201a 100644 --- a/storage/storage-interface/src/chunk_to_commit.rs +++ b/storage/storage-interface/src/chunk_to_commit.rs @@ -6,18 +6,23 @@ use aptos_types::{ state_store::{state_key::StateKey, state_value::StateValue}, transaction::{Transaction, TransactionInfo, TransactionOutput, Version}, }; -use std::collections::HashMap; +use crate::state_authenticator::StateAuthenticator; +use crate::state_delta::InMemState; +/// FIXME(aldenhu): clean up unused fields #[derive(Clone)] pub struct ChunkToCommit<'a> { pub first_version: Version, pub transactions: &'a [Transaction], - // TODO(aldenhu): make it a ref pub transaction_outputs: &'a [TransactionOutput], pub transaction_infos: &'a [TransactionInfo], pub base_state_version: Option, - pub latest_in_memory_state: &'a StateDelta, - pub state_updates_until_last_checkpoint: Option<&'a HashMap>>, + pub parent_state: &'a InMemState, + pub state: &'a InMemState, + pub parent_auth: &'a StateAuthenticator, + pub state_auth: &'a StateAuthenticator, + pub last_checkpoint_state: Option<&'a InMemState>, + pub last_checkpoint_auth: Option<&'a StateAuthenticator>, pub sharded_state_cache: Option<&'a ShardedStateCache>, pub is_reconfig: bool, } diff --git a/storage/storage-interface/src/lib.rs b/storage/storage-interface/src/lib.rs index 6d3409af61130..bfbc976512c91 100644 --- a/storage/storage-interface/src/lib.rs +++ b/storage/storage-interface/src/lib.rs @@ -47,13 +47,15 @@ pub mod state_authenticator; pub mod state_delta; pub mod state_view; -use crate::{chunk_to_commit::ChunkToCommit, state_delta::InMemState}; +use crate::{ + chunk_to_commit::ChunkToCommit, state_authenticator::StateAuthenticator, + state_delta::InMemState, +}; use aptos_scratchpad::SparseMerkleTree; pub use aptos_types::block_info::BlockHeight; use aptos_types::state_store::state_key::prefix::StateKeyPrefix; pub use errors::AptosDbError; pub use executed_trees::ExecutedTrees; -use crate::state_authenticator::StateAuthenticator; pub type Result = std::result::Result; // This is last line of defense against large queries slipping through external facing interfaces, @@ -392,7 +394,10 @@ pub trait DbReader: Send + Sync { /// Get state authenticator at a version at or older than the parameter. /// State proofs older than the returned state can be queried from the DB. - fn get_persisted_state_authen_before(&self, state: &StateAuthenticator) -> Result; + fn get_persisted_state_authen_before( + &self, + state: &StateAuthenticator, + ) -> Result; /// Get the ledger info of the epoch that `known_version` belongs to. fn get_epoch_ending_ledger_info( diff --git a/storage/storage-interface/src/state_authenticator.rs b/storage/storage-interface/src/state_authenticator.rs index 8d692c77f1557..391070f604c28 100644 --- a/storage/storage-interface/src/state_authenticator.rs +++ b/storage/storage-interface/src/state_authenticator.rs @@ -1,29 +1,41 @@ // Copyright (c) Aptos Foundation // SPDX-License-Identifier: Apache-2.0 +use crate::state_delta::StateDelta; +use aptos_crypto::HashValue; use aptos_scratchpad::SparseMerkleTree; -use aptos_types::state_store::state_key::StateKey; -use aptos_types::state_store::state_value::StateValue; -use crate::state_delta::{StateDelta, StateUpdate}; +use aptos_types::{state_store::state_value::StateValue, transaction::Version}; /// note: only a single field for now, more to be introduced later. #[derive(Clone, Debug)] pub struct StateAuthenticator { + next_version: Version, pub global_state: SparseMerkleTree, } impl StateAuthenticator { - pub fn new(global_state: SparseMerkleTree) -> Self { - Self { global_state } + pub fn new(next_version: Version, global_state: SparseMerkleTree) -> Self { + Self { + next_version, + global_state, + } } - pub fn update( - &self, - _persisted_auth: &StateAuthenticator, - _state_delta: &StateDelta, - ) -> Self { - /// FIXME(aldenhu) + pub fn update(&self, _persisted_auth: &StateAuthenticator, _state_delta: &StateDelta) -> Self { + // FIXME(aldenhu) todo!() } + pub fn root_hash(&self) -> HashValue { + self.global_state.root_hash() + } + + pub fn next_version(&self) -> Version { + self.next_version + } + + pub fn is_the_same(&self, rhs: &Self) -> bool { + // FIXME(aldenhu) + todo!() + } } diff --git a/storage/storage-interface/src/state_delta.rs b/storage/storage-interface/src/state_delta.rs index 8c20a0489fcf4..4f3ecd892ee8b 100644 --- a/storage/storage-interface/src/state_delta.rs +++ b/storage/storage-interface/src/state_delta.rs @@ -27,9 +27,9 @@ impl StateUpdate { #[derive(Clone, Debug)] pub struct InMemState { - pub next_version: Version, - pub updates: MapLayer, - pub usage: StateStorageUsage, + next_version: Version, + updates: MapLayer, + usage: StateStorageUsage, } impl InMemState { @@ -42,11 +42,16 @@ impl InMemState { self.next_version } + pub fn updates(&self) -> &MapLayer { + &self.updates + } + + pub fn usage(&self) -> StateStorageUsage { + self.usage + } + pub fn into_delta(self, base: InMemState) -> StateDelta { - StateDelta::new( - base, - self, - ) + StateDelta::new(base, self) } } @@ -62,7 +67,7 @@ pub struct StateDelta { impl StateDelta { pub fn new(base: InMemState, current: InMemState) -> Self { - let updates = current.updates.view_layers_after(&base.updates); + let updates = current.updates().view_layers_after(&base.updates()); Self { base, current,