Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[vm] Introducing a delta write #2057

Merged
merged 5 commits into from
Jul 21, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions api/types/src/convert.rs
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,8 @@ impl<'a, R: MoveResolverExt + ?Sized> MoveConverter<'a, R> {
data: self.try_into_resource(&typ, &val)?,
},
},
// Deltas never use access paths.
WriteOp::Delta(..) => unreachable!("unexpected conversion"),
};
Ok(ret)
}
Expand All @@ -281,6 +283,8 @@ impl<'a, R: MoveResolverExt + ?Sized> MoveConverter<'a, R> {
key,
value: value.into(),
},
// Deltas are materialized into WriteOP::Value(..) in executor.
WriteOp::Delta(..) => unreachable!("unexpected conversion"),
};
Ok(ret)
}
Expand Down
3 changes: 3 additions & 0 deletions aptos-move/aptos-vm/src/data_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,9 @@ impl<'a, S: StateView> StateViewCache<'a, S> {
self.data_map.remove(ap);
self.data_map.insert(ap.clone(), None);
}
WriteOp::Delta(..) => {
unimplemented!("sequential execution is not supported for deltas")
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ impl<'a, S: StateView> StateView for VersionedView<'a, S> {
Some(v) => Ok(match v.as_ref() {
WriteOp::Value(w) => Some(w.clone()),
WriteOp::Deletion => None,
WriteOp::Delta(..) => {
unimplemented!("parallel execution is not supported for deltas")
}
}),
None => self.base_view.get_state_value(state_key),
}
Expand Down
1 change: 1 addition & 0 deletions aptos-move/e2e-tests/src/data_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ impl FakeDataStore {
WriteOp::Deletion => {
self.remove(state_key);
}
WriteOp::Delta(..) => unreachable!("deltas are only used in executor"),
}
}
}
Expand Down
3 changes: 3 additions & 0 deletions aptos-move/genesis-viewer/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ fn print_modules(ws: &WriteSet) {
AccessPath::try_from(k.clone()).expect("State key can't be converted to access path");
match v {
WriteOp::Deletion => panic!("found WriteOp::Deletion in WriteSet"),
WriteOp::Delta(..) => panic!("found WriteOp::Delta in WriteSet"),
WriteOp::Value(blob) => {
let tag = ap.path.get(0).expect("empty blob in WriteSet");
if *tag == 0 {
Expand All @@ -213,6 +214,7 @@ fn print_resources(storage: &impl MoveResolverExt, ws: &WriteSet) {
AccessPath::try_from(k.clone()).expect("State key can't be converted to access path");
match v {
WriteOp::Deletion => panic!("found WriteOp::Deletion in WriteSet"),
WriteOp::Delta(..) => panic!("found WriteOp::Delta in WriteSet"),
WriteOp::Value(blob) => {
let tag = ap.path.get(0).expect("empty blob in WriteSet");
if *tag == 1 {
Expand Down Expand Up @@ -243,6 +245,7 @@ fn print_account_states(storage: &impl MoveResolverExt, ws: &WriteSet) {
AccessPath::try_from(k.clone()).expect("State key can't be converted to access path");
match v {
WriteOp::Deletion => panic!("found WriteOp::Deletion in WriteSet"),
WriteOp::Delta(..) => panic!("found WriteOp::Delta in WriteSet"),
WriteOp::Value(blob) => {
let tag = ap.path.get(0).expect("empty blob in WriteSet");
if *tag == 1 {
Expand Down
2 changes: 2 additions & 0 deletions aptos-move/transaction-replay/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,10 +177,12 @@ impl AptosDebugger {
access_path::Path::Resource(tag) => match op {
WriteOp::Deletion => state_view.delete_resource(addr, tag)?,
WriteOp::Value(bytes) => state_view.save_resource(addr, tag, bytes)?,
WriteOp::Delta(..) => unreachable!("deltas are only used in executor"),
},
access_path::Path::Code(module_id) => match op {
WriteOp::Deletion => state_view.delete_module(&module_id)?,
WriteOp::Value(bytes) => state_view.save_module(&module_id, bytes)?,
WriteOp::Delta(..) => unreachable!("deltas are only used in executor"),
},
}
}
Expand Down
16 changes: 15 additions & 1 deletion documentation/specifications/move_adapter/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -531,13 +531,27 @@ pub struct TransactionOutput {
/// `WriteSet` contains all access paths that one transaction modifies.
/// Each of them is a `WriteOp` where `Value(val)` means that serialized
/// representation should be updated to `val`, and `Deletion` means that
/// we are going to delete this access path.
/// we are going to delete this access path. A special `WriteOp` used by
/// aggregator - `Delta(op, limit, value)`, means that `value` should be
/// applied to deserialized representation with `op` operation and a
/// postcondition `result <= limit` must be ensured.
pub struct WriteSet {
write_set: Vec<(AccessPath, WriteOp)>,
}

/// `DeltaOperation` specifies the function which is used to apply delta.
pub enum DeltaOperation {
Addition,
Subtraction,
}

/// Value when delta application overflows, i.e. the postcondition of delta
/// application.
pub struct DeltaLimit(pub u128);

pub enum WriteOp {
Deletion,
Delta(DeltaOperation, DeltaLimit, u128),
Value(Vec<u8>),
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,7 @@ fn process_state_key_write_op(
let state_value = match write_op {
WriteOp::Value(new_value) => StateValue::from(new_value),
WriteOp::Deletion => StateValue::empty(),
WriteOp::Delta(..) => unreachable!("deltas are only used in executor"),
};
match state_cache.entry(state_key.clone()) {
hash_map::Entry::Occupied(mut entry) => {
Expand Down
34 changes: 33 additions & 1 deletion types/src/write_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,44 @@
// SPDX-License-Identifier: Apache-2.0

//! 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.
//! path it updates. For each access path, the VM can either give its new value or delete it. If
//! aggregator is used, a delta update is used.

use crate::state_store::state_key::StateKey;
use anyhow::Result;
use aptos_crypto_derive::{BCSCryptoHash, CryptoHasher};
use serde::{Deserialize, Serialize};

/// Specifies operation such as +, - to use with `WriteOp::Delta`.
#[derive(Copy, Clone, Hash, PartialOrd, Ord, PartialEq, Eq, Serialize, Deserialize)]
pub enum DeltaOperation {
Addition,
Subtraction,
}

impl std::fmt::Debug for DeltaOperation {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
DeltaOperation::Addition => write!(f, "+"),
DeltaOperation::Subtraction => write!(f, "-"),
}
}
}

#[derive(Copy, Clone, Hash, PartialOrd, Ord, PartialEq, Eq, Serialize, Deserialize)]
pub struct DeltaLimit(pub u128);

impl std::fmt::Debug for DeltaLimit {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "result <= {}", self.0)
}
}

#[derive(Clone, Eq, Hash, PartialEq, Serialize, Deserialize)]
pub enum WriteOp {
Deletion,
#[serde(skip)]
Delta(DeltaOperation, DeltaLimit, u128),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a reason why you don't carry u128 in the DeltaOperation? like
enum DeltaOperation {
Addition(u128),
Subtraction(u128),
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initially, the idea was to have delta(op, bytes) so that any update in the future cam simply specify op and serialize value into bytes. So no particular reason of keeping u128 in Delta. If we move it to operation as you suggested, I think we should move the limit as well since it is a postcondition of DeltaOperation

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should merge the u128, I don't have strong opinion on DeltaLimit since it at least has a name 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, let's merge u128 only then for now :)

Value(#[serde(with = "serde_bytes")] Vec<u8>),
}

Expand All @@ -20,6 +48,7 @@ impl WriteOp {
pub fn is_deletion(&self) -> bool {
match self {
WriteOp::Deletion => true,
WriteOp::Delta(..) => false,
WriteOp::Value(_) => false,
}
}
Expand All @@ -36,6 +65,9 @@ impl std::fmt::Debug for WriteOp {
.map(|byte| format!("{:02x}", byte))
.collect::<String>()
),
WriteOp::Delta(op, limit, value) => {
write!(f, "Delta({:?}{} ensures {:?})", op, value, limit)
}
WriteOp::Deletion => write!(f, "Deletion"),
}
}
Expand Down