From 01675b80b081dba2ee59dc8c9a6814c0976c2d2d Mon Sep 17 00:00:00 2001 From: Mac L Date: Thu, 13 Jul 2023 13:35:43 +1000 Subject: [PATCH 01/10] Compute values on demand --- src/builder.rs | 7 +- src/cow.rs | 9 +- src/interface.rs | 30 +++---- src/interface_iter.rs | 18 ++-- src/iter.rs | 22 ++--- src/lib.rs | 7 ++ src/list.rs | 67 +++++++------- src/packed_leaf.rs | 144 +++++++++++++++++++------------ src/repeat.rs | 6 +- src/serde.rs | 5 +- src/tests/builder.rs | 16 +++- src/tests/diff.rs | 6 +- src/tests/iterator.rs | 25 +++++- src/tests/packed.rs | 18 ++-- src/tests/proptest/operations.rs | 17 ++-- src/tests/repeat.rs | 4 +- src/tests/size_of.rs | 14 +-- src/tree.rs | 47 +++++----- src/update_map.rs | 25 +++--- src/utils.rs | 2 +- src/vector.rs | 73 ++++++++-------- 21 files changed, 319 insertions(+), 243 deletions(-) diff --git a/src/builder.rs b/src/builder.rs index 5ac0983..f16d376 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -1,8 +1,7 @@ use crate::utils::{opt_packing_depth, opt_packing_factor, Length}; -use crate::{Arc, Error, PackedLeaf, Tree}; -use tree_hash::TreeHash; +use crate::{Arc, Error, PackedLeaf, Tree, Value}; -pub struct Builder { +pub struct Builder { stack: Vec>, depth: usize, length: Length, @@ -12,7 +11,7 @@ pub struct Builder { packing_depth: usize, } -impl Builder { +impl Builder { pub fn new(depth: usize) -> Self { Self { stack: Vec::with_capacity(depth), diff --git a/src/cow.rs b/src/cow.rs index ccdd53b..284955c 100644 --- a/src/cow.rs +++ b/src/cow.rs @@ -1,3 +1,4 @@ +use std::borrow::Cow as StdCow; use std::collections::btree_map::VacantEntry; use std::ops::Deref; @@ -33,7 +34,7 @@ pub trait CowTrait<'a, T: Clone>: Deref { pub enum BTreeCow<'a, T: Clone> { Immutable { - value: &'a T, + value: StdCow<'a, T>, entry: VacantEntry<'a, usize, T>, }, Mutable { @@ -44,7 +45,7 @@ pub enum BTreeCow<'a, T: Clone> { impl<'a, T: Clone> CowTrait<'a, T> for BTreeCow<'a, T> { fn to_mut(self) -> &'a mut T { match self { - Self::Immutable { value, entry } => entry.insert(value.clone()), + Self::Immutable { value, entry } => entry.insert(value.into_owned()), Self::Mutable { value } => value, } } @@ -63,7 +64,7 @@ impl<'a, T: Clone> Deref for BTreeCow<'a, T> { pub enum VecCow<'a, T: Clone> { Immutable { - value: &'a T, + value: StdCow<'a, T>, entry: vec_map::VacantEntry<'a, T>, }, Mutable { @@ -74,7 +75,7 @@ pub enum VecCow<'a, T: Clone> { impl<'a, T: Clone> CowTrait<'a, T> for VecCow<'a, T> { fn to_mut(self) -> &'a mut T { match self { - Self::Immutable { value, entry } => entry.insert(value.clone()), + Self::Immutable { value, entry } => entry.insert(value.into_owned()), Self::Mutable { value } => value, } } diff --git a/src/interface.rs b/src/interface.rs index 4ecb7f3..edae456 100644 --- a/src/interface.rs +++ b/src/interface.rs @@ -3,18 +3,16 @@ use crate::utils::{updated_length, Length}; use crate::{ interface_iter::{InterfaceIter, InterfaceIterCow}, iter::Iter, - Cow, Error, + Cow, Error, Value, }; use arbitrary::Arbitrary; +use std::borrow::Cow as StdCow; use std::collections::BTreeMap; use std::marker::PhantomData; -use tree_hash::{Hash256, TreeHash}; +use tree_hash::Hash256; -pub trait ImmList -where - T: TreeHash + Clone, -{ - fn get(&self, idx: usize) -> Option<&T>; +pub trait ImmList { + fn get(&self, idx: usize) -> Option>; fn len(&self) -> Length; @@ -25,10 +23,7 @@ where fn iter_from(&self, index: usize) -> Iter; } -pub trait MutList: ImmList -where - T: TreeHash + Clone, -{ +pub trait MutList: ImmList { fn validate_push(current_len: usize) -> Result<(), Error>; fn replace(&mut self, index: usize, value: T) -> Result<(), Error>; fn update>( @@ -41,7 +36,7 @@ where #[derive(Debug, PartialEq, Clone, Arbitrary)] pub struct Interface where - T: TreeHash + Clone, + T: Value, B: MutList, U: UpdateMap, { @@ -52,7 +47,7 @@ where impl Interface where - T: TreeHash + Clone, + T: Value, B: MutList, U: UpdateMap, { @@ -64,13 +59,13 @@ where } } - pub fn get(&self, idx: usize) -> Option<&T> { + pub fn get(&self, idx: usize) -> Option> { self.updates.get(idx).or_else(|| self.backing.get(idx)) } pub fn get_mut(&mut self, idx: usize) -> Option<&mut T> { self.updates - .get_mut_with(idx, |idx| self.backing.get(idx).cloned()) + .get_mut_with(idx, |idx| self.backing.get(idx).map(|res| res.into_owned())) } pub fn get_cow(&mut self, index: usize) -> Option> { @@ -180,7 +175,10 @@ mod test { *c2.to_mut() = 11; assert_eq!(*list.get(0).unwrap(), 11); - assert_eq!(list.iter().cloned().collect::>(), vec![11, 2, 3]); + assert_eq!( + list.iter().map(|res| res.into_owned()).collect::>(), + vec![11, 2, 3] + ); } #[test] diff --git a/src/interface_iter.rs b/src/interface_iter.rs index ee90f6f..e659d42 100644 --- a/src/interface_iter.rs +++ b/src/interface_iter.rs @@ -1,19 +1,19 @@ use crate::iter::Iter; -use crate::{Cow, UpdateMap}; -use tree_hash::TreeHash; +use crate::{Cow, UpdateMap, Value}; +use std::borrow::Cow as StdCow; #[derive(Debug)] -pub struct InterfaceIter<'a, T: TreeHash + Clone, U: UpdateMap> { +pub struct InterfaceIter<'a, T: Value, U: UpdateMap> { pub(crate) tree_iter: Iter<'a, T>, pub(crate) updates: &'a U, pub(crate) index: usize, pub(crate) length: usize, } -impl<'a, T: TreeHash + Clone, U: UpdateMap> Iterator for InterfaceIter<'a, T, U> { - type Item = &'a T; +impl<'a, T: Value, U: UpdateMap> Iterator for InterfaceIter<'a, T, U> { + type Item = StdCow<'a, T>; - fn next(&mut self) -> Option<&'a T> { + fn next(&mut self) -> Option> { let index = self.index; self.index += 1; @@ -30,16 +30,16 @@ impl<'a, T: TreeHash + Clone, U: UpdateMap> Iterator for InterfaceIter<'a, T, } } -impl<'a, T: TreeHash + Clone, U: UpdateMap> ExactSizeIterator for InterfaceIter<'a, T, U> {} +impl<'a, T: Value, U: UpdateMap> ExactSizeIterator for InterfaceIter<'a, T, U> {} #[derive(Debug)] -pub struct InterfaceIterCow<'a, T: TreeHash + Clone, U: UpdateMap> { +pub struct InterfaceIterCow<'a, T: Value, U: UpdateMap> { pub(crate) tree_iter: Iter<'a, T>, pub(crate) updates: &'a mut U, pub(crate) index: usize, } -impl<'a, T: TreeHash + Clone, U: UpdateMap> InterfaceIterCow<'a, T, U> { +impl<'a, T: Value, U: UpdateMap> InterfaceIterCow<'a, T, U> { pub fn next_cow(&mut self) -> Option<(usize, Cow)> { let index = self.index; self.index += 1; diff --git a/src/iter.rs b/src/iter.rs index 1d10fa2..25489b2 100644 --- a/src/iter.rs +++ b/src/iter.rs @@ -1,11 +1,11 @@ use crate::{ utils::{opt_packing_depth, opt_packing_factor, Length}, - Leaf, PackedLeaf, Tree, + Leaf, Tree, Value, }; -use tree_hash::TreeHash; +use std::borrow::Cow as StdCow; #[derive(Debug)] -pub struct Iter<'a, T: TreeHash + Clone> { +pub struct Iter<'a, T: Value> { /// Stack of tree nodes corresponding to the current position. stack: Vec<&'a Tree>, /// The list index corresponding to the current position (next element to be yielded). @@ -22,7 +22,7 @@ pub struct Iter<'a, T: TreeHash + Clone> { length: Length, } -impl<'a, T: TreeHash + Clone> Iter<'a, T> { +impl<'a, T: Value> Iter<'a, T> { pub fn from_index(index: usize, root: &'a Tree, depth: usize, length: Length) -> Self { let mut stack = Vec::with_capacity(depth); stack.push(root); @@ -38,8 +38,8 @@ impl<'a, T: TreeHash + Clone> Iter<'a, T> { } } -impl<'a, T: TreeHash + Clone> Iterator for Iter<'a, T> { - type Item = &'a T; +impl<'a, T: Value> Iterator for Iter<'a, T> { + type Item = StdCow<'a, T>; fn next(&mut self) -> Option { if self.index >= self.length.as_usize() { @@ -58,12 +58,12 @@ impl<'a, T: TreeHash + Clone> Iterator for Iter<'a, T> { self.stack.pop(); } - result + result.map(|res| StdCow::Borrowed(res)) } - Some(Tree::PackedLeaf(PackedLeaf { values, .. })) => { + Some(Tree::PackedLeaf(packed_leaf)) => { let sub_index = self.index % self.packing_factor; - let result = values.get(sub_index); + let result = packed_leaf.get(sub_index); self.index += 1; @@ -80,7 +80,7 @@ impl<'a, T: TreeHash + Clone> Iterator for Iter<'a, T> { } } - result + result.map(|res| StdCow::Owned(res)) } Some(Tree::Node { left, right, .. }) => { let depth = self.full_depth - self.stack.len(); @@ -105,4 +105,4 @@ impl<'a, T: TreeHash + Clone> Iterator for Iter<'a, T> { } } -impl<'a, T: TreeHash + Clone> ExactSizeIterator for Iter<'a, T> {} +impl<'a, T: Value> ExactSizeIterator for Iter<'a, T> {} diff --git a/src/lib.rs b/src/lib.rs index eeb52a4..d9c5b73 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -30,3 +30,10 @@ pub use tree::Tree; pub use triomphe::Arc; pub use update_map::UpdateMap; pub use vector::Vector; + +use ssz::{Decode, Encode}; +use tree_hash::TreeHash; + +pub trait Value: Encode + Decode + TreeHash + PartialEq + Clone {} + +impl Value for T where T: Encode + Decode + TreeHash + PartialEq + Clone {} diff --git a/src/list.rs b/src/list.rs index 0504546..35c80ff 100644 --- a/src/list.rs +++ b/src/list.rs @@ -6,12 +6,13 @@ use crate::iter::Iter; use crate::serde::ListVisitor; use crate::update_map::MaxMap; use crate::utils::{arb_arc, int_log, opt_packing_depth, updated_length, Length}; -use crate::{Arc, Cow, Error, Tree, UpdateMap}; +use crate::{Arc, Cow, Error, Tree, UpdateMap, Value}; use arbitrary::Arbitrary; use derivative::Derivative; use itertools::process_results; use serde::{ser::SerializeSeq, Deserialize, Deserializer, Serialize, Serializer}; use ssz::{Decode, Encode, SszEncoder, TryFromIter, BYTES_PER_LENGTH_OFFSET}; +use std::borrow::Cow as StdCow; use std::collections::BTreeMap; use std::marker::PhantomData; use tree_hash::{Hash256, PackedEncoding, TreeHash}; @@ -19,19 +20,17 @@ use typenum::Unsigned; use vec_map::VecMap; #[derive(Debug, Clone, Derivative, Arbitrary)] -#[derivative(PartialEq( - bound = "T: TreeHash + PartialEq + Clone, N: Unsigned, U: UpdateMap + PartialEq" -))] -#[arbitrary(bound = "T: Arbitrary<'arbitrary> + TreeHash + PartialEq + Clone")] +#[derivative(PartialEq(bound = "T: Value, N: Unsigned, U: UpdateMap + PartialEq"))] +#[arbitrary(bound = "T: Arbitrary<'arbitrary> + Value")] #[arbitrary(bound = "N: Unsigned, U: Arbitrary<'arbitrary> + UpdateMap + PartialEq")] -pub struct List = MaxMap>> { +pub struct List = MaxMap>> { pub(crate) interface: Interface, U>, } #[derive(Debug, Clone, Derivative, Arbitrary)] -#[derivative(PartialEq(bound = "T: TreeHash + PartialEq + Clone, N: Unsigned"))] -#[arbitrary(bound = "T: Arbitrary<'arbitrary> + TreeHash + PartialEq + Clone, N: Unsigned")] -pub struct ListInner { +#[derivative(PartialEq(bound = "T: Value, N: Unsigned"))] +#[arbitrary(bound = "T: Arbitrary<'arbitrary> + Value, N: Unsigned")] +pub struct ListInner { #[arbitrary(with = arb_arc)] pub(crate) tree: Arc>, pub(crate) length: Length, @@ -41,7 +40,7 @@ pub struct ListInner { _phantom: PhantomData, } -impl> List { +impl> List { pub fn new(vec: Vec) -> Result { Self::try_from_iter(vec) } @@ -105,7 +104,7 @@ impl> List { } pub fn to_vec(&self) -> Vec { - self.iter().cloned().collect() + self.iter().map(|res| res.into_owned()).collect() } pub fn iter(&self) -> InterfaceIter { @@ -128,7 +127,7 @@ impl> List { } // Wrap trait methods so we present a Vec-like interface without having to import anything. - pub fn get(&self, index: usize) -> Option<&T> { + pub fn get(&self, index: usize) -> Option> { self.interface.get(index) } @@ -173,8 +172,8 @@ impl> List { } } -impl ImmList for ListInner { - fn get(&self, index: usize) -> Option<&T> { +impl ImmList for ListInner { + fn get(&self, index: usize) -> Option> { if index < self.len().as_usize() { self.tree .get_recursive(index, self.depth, self.packing_depth) @@ -194,7 +193,7 @@ impl ImmList for ListInner { impl MutList for ListInner where - T: TreeHash + Clone, + T: Value, N: Unsigned, { fn validate_push(current_len: usize) -> Result<(), Error> { @@ -241,9 +240,7 @@ where } } -impl> - List -{ +impl> List { pub fn rebase(&self, base: &Self) -> Result { // Diff self from base. let diff = ListDiff::compute_diff(base, self)?; @@ -262,13 +259,13 @@ impl Default for List { +impl Default for List { fn default() -> Self { Self::empty() } } -impl TreeHash for List { +impl TreeHash for List { fn tree_hash_type() -> tree_hash::TreeHashType { tree_hash::TreeHashType::List } @@ -290,8 +287,8 @@ impl TreeHash for List { } } -impl<'a, T: TreeHash + Clone, N: Unsigned, U: UpdateMap> IntoIterator for &'a List { - type Item = &'a T; +impl<'a, T: Value, N: Unsigned, U: UpdateMap> IntoIterator for &'a List { + type Item = StdCow<'a, T>; type IntoIter = InterfaceIter<'a, T, U>; fn into_iter(self) -> Self::IntoIter { @@ -299,7 +296,7 @@ impl<'a, T: TreeHash + Clone, N: Unsigned, U: UpdateMap> IntoIterator for &'a } } -impl> Serialize for List +impl> Serialize for List where T: Serialize, { @@ -309,7 +306,7 @@ where { let mut seq = serializer.serialize_seq(Some(self.len()))?; for e in self { - seq.serialize_element(e)?; + seq.serialize_element(&e)?; } seq.end() } @@ -317,7 +314,7 @@ where impl<'de, T, N, U> Deserialize<'de> for List where - T: Deserialize<'de> + TreeHash + Clone, + T: Deserialize<'de> + Value, N: Unsigned, U: UpdateMap, { @@ -330,7 +327,7 @@ where } // FIXME: duplicated from `ssz::encode::impl_for_vec` -impl Encode for List { +impl Encode for List { fn is_ssz_fixed_len() -> bool { false } @@ -346,8 +343,8 @@ impl Encode for List { } fn ssz_append(&self, buf: &mut Vec) { - if T::is_ssz_fixed_len() { - buf.reserve(T::ssz_fixed_len() * self.len()); + if ::is_ssz_fixed_len() { + buf.reserve(::ssz_fixed_len() * self.len()); for item in self { item.ssz_append(buf); @@ -356,7 +353,7 @@ impl Encode for List { let mut encoder = SszEncoder::container(buf, self.len() * BYTES_PER_LENGTH_OFFSET); for item in self { - encoder.append(item); + encoder.append(&item.into_owned()); } encoder.finalize(); @@ -366,7 +363,7 @@ impl Encode for List { impl TryFromIter for List where - T: TreeHash + Clone, + T: Value, N: Unsigned, { type Error = Error; @@ -381,7 +378,7 @@ where impl Decode for List where - T: Decode + TreeHash + Clone, + T: Value, N: Unsigned, { fn is_ssz_fixed_len() -> bool { @@ -393,10 +390,10 @@ where if bytes.is_empty() { Ok(List::empty()) - } else if T::is_ssz_fixed_len() { + } else if ::is_ssz_fixed_len() { let num_items = bytes .len() - .checked_div(T::ssz_fixed_len()) + .checked_div(::ssz_fixed_len()) .ok_or(ssz::DecodeError::ZeroLengthItem)?; if num_items > max_len { @@ -407,7 +404,9 @@ where } process_results( - bytes.chunks(T::ssz_fixed_len()).map(T::from_ssz_bytes), + bytes + .chunks(::ssz_fixed_len()) + .map(T::from_ssz_bytes), |iter| { List::try_from_iter(iter).map_err(|e| { ssz::DecodeError::BytesInvalid(format!("Error building ssz List: {:?}", e)) diff --git a/src/packed_leaf.rs b/src/packed_leaf.rs index b0f3770..511141b 100644 --- a/src/packed_leaf.rs +++ b/src/packed_leaf.rs @@ -1,132 +1,166 @@ -use crate::{utils::arb_rwlock, Error, UpdateMap}; +use crate::{utils::arb_rwlock, Error, UpdateMap, Value}; use arbitrary::Arbitrary; +use core::marker::PhantomData; use derivative::Derivative; use parking_lot::RwLock; use std::ops::ControlFlow; -use tree_hash::{Hash256, TreeHash, BYTES_PER_CHUNK}; +use tree_hash::{Hash256, BYTES_PER_CHUNK}; #[derive(Debug, Derivative, Arbitrary)] #[derivative(PartialEq, Hash)] -pub struct PackedLeaf { +pub struct PackedLeaf { #[derivative(PartialEq = "ignore", Hash = "ignore")] #[arbitrary(with = arb_rwlock)] pub hash: RwLock, - pub(crate) values: Vec, + pub length: u8, + _phantom: PhantomData, } impl Clone for PackedLeaf where - T: TreeHash + Clone, + T: Value, { fn clone(&self) -> Self { Self { hash: RwLock::new(*self.hash.read()), - values: self.values.clone(), + length: self.length, + _phantom: PhantomData, } } } -impl PackedLeaf { - pub fn tree_hash(&self) -> Hash256 { - let read_lock = self.hash.read(); - let mut hash = *read_lock; - drop(read_lock); +impl PackedLeaf { + fn length(&self) -> usize { + self.length as usize + } - if !hash.is_zero() { - return hash; - } + fn value_len(_value: &T) -> usize { + BYTES_PER_CHUNK / T::tree_hash_packing_factor() + } - let hash_bytes = hash.as_bytes_mut(); + pub fn values(&self) -> Vec { + self.hash + .read() + .as_bytes() + .chunks_exact(BYTES_PER_CHUNK / T::tree_hash_packing_factor()) + .take(self.length()) + .map(|bytes| T::from_ssz_bytes(bytes).expect("Should always deserialize")) + .collect::>() + } - let value_len = BYTES_PER_CHUNK / T::tree_hash_packing_factor(); - for (i, value) in self.values.iter().enumerate() { - hash_bytes[i * value_len..(i + 1) * value_len] - .copy_from_slice(&value.tree_hash_packed_encoding()); - } + pub fn get(&self, index: usize) -> Option { + self.values().get(index).cloned() + } - *self.hash.write() = hash; - hash + pub fn tree_hash(&self) -> Hash256 { + *self.hash.read() } pub fn empty() -> Self { PackedLeaf { hash: RwLock::new(Hash256::zero()), - values: Vec::with_capacity(T::tree_hash_packing_factor()), + length: 0, + _phantom: PhantomData, } } pub fn single(value: T) -> Self { - let mut values = Vec::with_capacity(T::tree_hash_packing_factor()); - values.push(value); + let mut hash = Hash256::zero(); + let hash_bytes = hash.as_bytes_mut(); + + let value_len = Self::value_len(&value); + hash_bytes[0..value_len].copy_from_slice(&value.as_ssz_bytes()); PackedLeaf { - hash: RwLock::new(Hash256::zero()), - values, + hash: RwLock::new(hash), + length: 1, + _phantom: PhantomData, } } pub fn repeat(value: T, n: usize) -> Self { assert!(n <= T::tree_hash_packing_factor()); + + let mut hash = Hash256::zero(); + let hash_bytes = hash.as_bytes_mut(); + + let value_len = Self::value_len(&value); + + for (i, value) in vec![value; n].iter().enumerate() { + hash_bytes[i * value_len..(i + 1) * value_len].copy_from_slice(&value.as_ssz_bytes()); + } + PackedLeaf { - hash: RwLock::new(Hash256::zero()), - values: vec![value; n], + hash: RwLock::new(hash), + length: n as u8, + _phantom: PhantomData, } } pub fn insert_at_index(&self, index: usize, value: T) -> Result { - let mut updated = PackedLeaf { - hash: RwLock::new(Hash256::zero()), - values: self.values.clone(), - }; - let sub_index = index % T::tree_hash_packing_factor(); - updated.insert_mut(sub_index, value)?; + let mut updated = self.clone(); + + updated.insert_mut(index, value)?; + Ok(updated) } pub fn update>( &self, prefix: usize, - hash: Hash256, + _hash: Hash256, updates: &U, ) -> Result { - let mut updated = PackedLeaf { - hash: RwLock::new(hash), - values: self.values.clone(), - }; - let packing_factor = T::tree_hash_packing_factor(); let start = prefix; let end = prefix + packing_factor; + + let mut updated = self.clone(); + updates.for_each_range(start, end, |index, value| { ControlFlow::Continue(updated.insert_mut(index % packing_factor, value.clone())) })?; + Ok(updated) } - pub fn insert_mut(&mut self, sub_index: usize, value: T) -> Result<(), Error> { - // Ensure hash is 0. - *self.hash.get_mut() = Hash256::zero(); + pub fn insert_mut(&mut self, index: usize, value: T) -> Result<(), Error> { + // Convert the index to the index of the underlying bytes. + let sub_index = index * Self::value_len(&value); - if sub_index == self.values.len() { - self.values.push(value); - } else if sub_index < self.values.len() { - self.values[sub_index] = value; - } else { + if sub_index >= BYTES_PER_CHUNK { return Err(Error::PackedLeafOutOfBounds { sub_index, - len: self.values.len(), + len: self.length(), }); } + + let value_len = Self::value_len(&value); + + let mut hash = *self.hash.read(); + let hash_bytes = hash.as_bytes_mut(); + + hash_bytes[sub_index..sub_index + value_len].copy_from_slice(&value.as_ssz_bytes()); + + *self.hash.write() = hash; + + if index == self.length() { + self.length += 1; + } else if index > self.length() { + panic!("This is bad"); + } + Ok(()) } pub fn push(&mut self, value: T) -> Result<(), Error> { - if self.values.len() == T::tree_hash_packing_factor() { - return Err(Error::PackedLeafFull { - len: self.values.len(), - }); + // Ensure a new T will not overflow the leaf. + if self.length() >= T::tree_hash_packing_factor() { + return Err(Error::PackedLeafFull { len: self.length() }); } - self.values.push(value); + + self.insert_mut(self.length(), value.clone())?; + Ok(()) } } diff --git a/src/repeat.rs b/src/repeat.rs index c5cca99..e6a70f3 100644 --- a/src/repeat.rs +++ b/src/repeat.rs @@ -1,13 +1,13 @@ use crate::utils::{opt_packing_factor, Length}; -use crate::{Arc, Error, Leaf, List, PackedLeaf, Tree, UpdateMap}; +use crate::{Arc, Error, Leaf, List, PackedLeaf, Tree, UpdateMap, Value}; use smallvec::{smallvec, SmallVec}; -use tree_hash::{Hash256, TreeHash}; +use tree_hash::Hash256; use typenum::Unsigned; /// Efficiently construct a list from `n` copies of `elem`. pub fn repeat_list(elem: T, n: usize) -> Result, Error> where - T: TreeHash + Clone, + T: Value, N: Unsigned, U: UpdateMap, { diff --git a/src/serde.rs b/src/serde.rs index 406feb2..282896f 100644 --- a/src/serde.rs +++ b/src/serde.rs @@ -1,8 +1,7 @@ -use crate::{List, UpdateMap}; +use crate::{List, UpdateMap, Value}; use itertools::process_results; use serde::Deserialize; use std::marker::PhantomData; -use tree_hash::TreeHash; use typenum::Unsigned; pub struct ListVisitor { @@ -19,7 +18,7 @@ impl Default for ListVisitor { impl<'a, T, N, U> serde::de::Visitor<'a> for ListVisitor where - T: Deserialize<'a> + TreeHash + Clone, + T: Deserialize<'a> + Value, N: Unsigned, U: UpdateMap, { diff --git a/src/tests/builder.rs b/src/tests/builder.rs index f516b6e..caa5892 100644 --- a/src/tests/builder.rs +++ b/src/tests/builder.rs @@ -17,7 +17,13 @@ fn build_partial_hash256_list() { let slow_list = List::::try_from_iter_slow(sub_vec.clone()).unwrap(); assert_eq!(fast_list, slow_list); - assert_eq!(fast_list.iter().cloned().collect::>(), sub_vec); + assert_eq!( + fast_list + .iter() + .map(|item| item.into_owned()) + .collect::>(), + sub_vec + ); } } @@ -34,6 +40,12 @@ fn build_packed_u64_list() { let slow_list = List::::try_from_iter(sub_vec.clone()).unwrap(); assert_eq!(fast_list, slow_list); - assert_eq!(fast_list.iter().cloned().collect::>(), sub_vec); + assert_eq!( + fast_list + .iter() + .map(|item| item.into_owned()) + .collect::>(), + sub_vec + ); } } diff --git a/src/tests/diff.rs b/src/tests/diff.rs index 7193887..4d4bc68 100644 --- a/src/tests/diff.rs +++ b/src/tests/diff.rs @@ -1,4 +1,4 @@ -use crate::{Diff, Error, List, ListDiff}; +use crate::{Diff, Error, List, ListDiff, Value}; use std::fmt::Debug; use tree_hash::TreeHash; use typenum::{Unsigned, U16}; @@ -45,7 +45,7 @@ where fn with_updated_index(list: &List, index: usize, value: T) -> List where - T: TreeHash + Send + Sync + Clone, + T: Value + Send + Sync, N: Unsigned, { let mut updated = list.clone(); @@ -58,7 +58,7 @@ where fn extended(list: &List, values: Vec) -> List where - T: TreeHash + Send + Sync + Clone, + T: Value + Send + Sync, N: Unsigned, { let mut updated = list.clone(); diff --git a/src/tests/iterator.rs b/src/tests/iterator.rs index 3afaaf2..5be4719 100644 --- a/src/tests/iterator.rs +++ b/src/tests/iterator.rs @@ -9,7 +9,13 @@ fn hash256_vec_iter() { let vec = (0..n).map(Hash256::from_low_u64_be).collect::>(); let vector = Vector::::new(vec.clone()).unwrap(); - assert_eq!(vector.iter().cloned().collect::>(), vec); + assert_eq!( + vector + .iter() + .map(|item| item.into_owned()) + .collect::>(), + vec + ); } #[test] @@ -19,7 +25,12 @@ fn hash256_list_iter() { let vec = (0..n).map(Hash256::from_low_u64_be).collect::>(); let list = List::::new(vec.clone()).unwrap(); - assert_eq!(list.iter().cloned().collect::>(), vec); + assert_eq!( + list.iter() + .map(|item| item.into_owned()) + .collect::>(), + vec + ); } #[test] @@ -33,7 +44,10 @@ fn hash256_list_iter_from() { for i in 0..=n { assert_eq!( - list.iter_from(i).unwrap().cloned().collect::>(), + list.iter_from(i) + .unwrap() + .map(|item| item.into_owned()) + .collect::>(), &vec[i..] ); } @@ -58,7 +72,10 @@ fn hash256_vector_iter_from() { for i in 0..=n { assert_eq!( - vect.iter_from(i).unwrap().cloned().collect::>(), + vect.iter_from(i) + .unwrap() + .map(|item| item.into_owned()) + .collect::>(), &vec[i..] ); } diff --git a/src/tests/packed.rs b/src/tests/packed.rs index 5351b38..2ef994d 100644 --- a/src/tests/packed.rs +++ b/src/tests/packed.rs @@ -9,11 +9,11 @@ fn u64_packed_list_build_and_iter() { let vec = (0..len).map(|i| 2 * i).collect::>(); let list = List::::new(vec.clone()).unwrap(); - let from_iter = list.iter().copied().collect::>(); + let from_iter = list.iter().map(|res| res.into_owned()).collect::>(); assert_eq!(vec, from_iter); for i in 0..len as usize { - assert_eq!(list.get(i), vec.get(i)); + assert_eq!(list.get(i).map(|res| res.into_owned()).as_ref(), vec.get(i)); } } } @@ -36,11 +36,17 @@ fn u64_packed_vector_build_and_iter() { let vec = (0..len).map(|i| 2 * i).collect::>(); let vector = Vector::::new(vec.clone()).unwrap(); - let from_iter = vector.iter().copied().collect::>(); + let from_iter = vector + .iter() + .map(|res| res.into_owned()) + .collect::>(); assert_eq!(vec, from_iter); for i in 0..len as usize { - assert_eq!(vector.get(i), vec.get(i)); + assert_eq!( + vector.get(i).map(|res| res.into_owned()).as_ref(), + vec.get(i) + ); } } @@ -74,11 +80,11 @@ fn out_of_order_mutations() { for (i, v) in mutations { *list.get_mut(i).unwrap() = v; vec[i] = v; - assert_eq!(list.get(i), Some(&v)); + assert_eq!(list.get(i).map(|res| res.into_owned()), Some(v)); list.apply_updates().unwrap(); - assert_eq!(list.get(i), Some(&v)); + assert_eq!(list.get(i).map(|res| res.into_owned()), Some(v)); } assert_eq!(list.to_vec(), vec); diff --git a/src/tests/proptest/operations.rs b/src/tests/proptest/operations.rs index fba0b97..141cf5d 100644 --- a/src/tests/proptest/operations.rs +++ b/src/tests/proptest/operations.rs @@ -2,6 +2,7 @@ use super::{arb_hash256, arb_index, arb_large, arb_list, arb_vect, Large}; use crate::{Diff, Error, List, ListDiff, Vector, VectorDiff}; use proptest::prelude::*; use ssz::{Decode, Encode}; +use std::borrow::Cow as StdCow; use std::fmt::Debug; use std::marker::PhantomData; use tree_hash::{Hash256, TreeHash}; @@ -17,7 +18,7 @@ pub struct Spec { _phantom: PhantomData, } -impl Spec { +impl Spec { pub fn list(values: Vec) -> Self { assert!(values.len() <= N::to_usize()); Self { @@ -40,13 +41,15 @@ impl Spec { self.values.len() } - pub fn iter(&self) -> impl Iterator { - self.values.iter() + pub fn iter(&self) -> impl Iterator> { + self.values.iter().map(|item| StdCow::Borrowed(item)) } - pub fn iter_from(&self, index: usize) -> Result, Error> { + pub fn iter_from(&self, index: usize) -> Result>, Error> { if index <= self.len() { - Ok(self.values[index..].iter()) + Ok(self.values[index..] + .iter() + .map(|item| StdCow::Borrowed(item))) } else { Err(Error::OutOfBoundsIterFrom { index, @@ -55,8 +58,8 @@ impl Spec { } } - pub fn get(&self, index: usize) -> Option<&T> { - self.values.get(index) + pub fn get(&self, index: usize) -> Option> { + self.values.get(index).map(|val| StdCow::Borrowed(val)) } pub fn set(&mut self, index: usize, value: T) -> Option<()> { diff --git a/src/tests/repeat.rs b/src/tests/repeat.rs index 7014bd0..552041f 100644 --- a/src/tests/repeat.rs +++ b/src/tests/repeat.rs @@ -1,9 +1,9 @@ -use crate::List; +use crate::{List, Value}; use std::fmt::Debug; use tree_hash::TreeHash; use typenum::{Unsigned, U1024, U64, U8}; -fn list_test(val: T) { +fn list_test(val: T) { for n in 96..=N::to_usize() { let fast = List::::repeat(val.clone(), n).unwrap(); let slow = List::::repeat_slow(val.clone(), n).unwrap(); diff --git a/src/tests/size_of.rs b/src/tests/size_of.rs index 00e0b9d..a7694ed 100644 --- a/src/tests/size_of.rs +++ b/src/tests/size_of.rs @@ -6,9 +6,9 @@ use tree_hash::Hash256; /// It's important that the Tree nodes have a predictable size. #[test] fn size_of_hash256() { - assert_eq!(size_of::>(), 72); + assert_eq!(size_of::>(), 64); assert_eq!(size_of::>(), 48); - assert_eq!(size_of::>(), 64); + assert_eq!(size_of::>(), 48); let rw_lock_size = size_of::>(); assert_eq!(rw_lock_size, 40); @@ -18,19 +18,19 @@ fn size_of_hash256() { assert_eq!( size_of::>(), - size_of::>() + 8 + size_of::>() + 16 ); } /// It's important that the Tree nodes have a predictable size. #[test] fn size_of_u8() { - assert_eq!(size_of::>(), 72); + assert_eq!(size_of::>(), 64); assert_eq!(size_of::>(), 48); - assert_eq!(size_of::>(), 64); + assert_eq!(size_of::>(), 48); assert_eq!( size_of::>(), - size_of::>() + size_of::>() + size_of::>() + size_of::() ); let rw_lock_size = size_of::>(); @@ -39,5 +39,5 @@ fn size_of_u8() { let arc_size = size_of::>>(); assert_eq!(arc_size, 8); - assert_eq!(size_of::>(), size_of::>() + 8); + assert_eq!(size_of::>(), size_of::>() + 16); } diff --git a/src/tree.rs b/src/tree.rs index e900289..e45b1f8 100644 --- a/src/tree.rs +++ b/src/tree.rs @@ -1,19 +1,19 @@ use crate::utils::{arb_arc, arb_rwlock, opt_hash, opt_packing_depth, opt_packing_factor}; -use crate::{Arc, Error, Leaf, PackedLeaf, UpdateMap}; +use crate::{Arc, Error, Leaf, PackedLeaf, UpdateMap, Value}; use arbitrary::Arbitrary; use derivative::Derivative; use ethereum_hashing::{hash32_concat, ZERO_HASHES}; use parking_lot::RwLock; use serde::{Deserialize, Serialize}; -use ssz::{Decode, Encode}; use ssz_derive::{Decode, Encode}; +use std::borrow::Cow as StdCow; use std::collections::BTreeMap; use std::ops::ControlFlow; -use tree_hash::{Hash256, TreeHash}; +use tree_hash::Hash256; #[derive(Debug, Derivative, Arbitrary)] #[derivative(PartialEq, Hash)] -pub enum Tree { +pub enum Tree { Leaf(Leaf), PackedLeaf(PackedLeaf), Node { @@ -28,7 +28,7 @@ pub enum Tree { Zero(usize), } -impl Clone for Tree { +impl Clone for Tree { fn clone(&self) -> Self { match self { Self::Node { hash, left, right } => Self::Node { @@ -43,7 +43,7 @@ impl Clone for Tree { } } -impl Tree { +impl Tree { pub fn empty(depth: usize) -> Arc { Self::zero(depth) } @@ -84,12 +84,17 @@ impl Tree { Self::Leaf(Leaf::new(value)) } - pub fn get_recursive(&self, index: usize, depth: usize, packing_depth: usize) -> Option<&T> { + pub fn get_recursive( + &self, + index: usize, + depth: usize, + packing_depth: usize, + ) -> Option> { match self { - Self::Leaf(Leaf { value, .. }) if depth == 0 => Some(value), - Self::PackedLeaf(PackedLeaf { values, .. }) if depth == 0 => { - values.get(index % T::tree_hash_packing_factor()) - } + Self::Leaf(Leaf { value, .. }) if depth == 0 => Some(StdCow::Borrowed(value)), + Self::PackedLeaf(packed_leaf) if depth == 0 => packed_leaf + .get(index % T::tree_hash_packing_factor()) + .map(|res| StdCow::Owned(res)), Self::Node { left, right, .. } if depth > 0 => { let new_depth = depth - 1; // Left @@ -171,7 +176,7 @@ impl Tree { let index = prefix; let value = updates .get(index) - .cloned() + .map(|res| res.into_owned()) .ok_or(Error::LeafUpdateMissing { index })?; Ok(Self::leaf_with_hash(value, hash)) } @@ -223,7 +228,7 @@ impl Tree { let index = prefix; let value = updates .get(index) - .cloned() + .map(|res| res.into_owned()) .ok_or(Error::LeafUpdateMissing { index })?; Ok(Self::leaf_with_hash(value, hash)) } @@ -239,7 +244,7 @@ impl Tree { } } -impl Tree { +impl Tree { pub fn diff( &self, other: &Self, @@ -258,9 +263,9 @@ impl Tree { } (Self::PackedLeaf(l1), Self::PackedLeaf(l2)) if depth == 0 => { let mut equal = true; - for i in 0..l2.values.len() { - let v2 = &l2.values[i]; - match l1.values.get(i) { + for i in 0..l2.values().len() { + let v2 = &l2.values()[i]; + match l1.values().get(i) { Some(v1) if v1 == v2 => continue, _ => { equal = false; @@ -344,7 +349,7 @@ impl Tree { Self::PackedLeaf(packed_leaf) if depth == 0 => { diff.hashes .insert((depth, prefix), *packed_leaf.hash.read()); - for (i, value) in packed_leaf.values.iter().enumerate() { + for (i, value) in packed_leaf.values().iter().enumerate() { diff.leaves.insert(prefix | i, value.clone()); } Ok(()) @@ -368,14 +373,14 @@ impl Tree { } #[derive(Debug, PartialEq, Encode, Decode, Deserialize, Serialize, Derivative)] -#[derivative(Default(bound = "T: TreeHash + Clone"))] -pub struct TreeDiff { +#[derivative(Default(bound = "T: Value"))] +pub struct TreeDiff { pub leaves: BTreeMap, /// Map from `(depth, prefix)` to node hash. pub hashes: BTreeMap<(usize, usize), Hash256>, } -impl Tree { +impl Tree { pub fn tree_hash(&self) -> Hash256 { match self { Self::Leaf(Leaf { hash, value }) => { diff --git a/src/update_map.rs b/src/update_map.rs index eb7ea4a..fde00c1 100644 --- a/src/update_map.rs +++ b/src/update_map.rs @@ -1,14 +1,15 @@ use crate::cow::{BTreeCow, Cow, VecCow}; use crate::utils::max_btree_index; use arbitrary::Arbitrary; +use std::borrow::Cow as StdCow; use std::collections::{btree_map::Entry, BTreeMap}; use std::ops::ControlFlow; use vec_map::VecMap; /// Trait for map types which can be used to store intermediate updates before application /// to the tree. -pub trait UpdateMap: Default + Clone { - fn get(&self, k: usize) -> Option<&T>; +pub trait UpdateMap: Default + Clone { + fn get(&self, k: usize) -> Option>; fn get_mut_with(&mut self, k: usize, f: F) -> Option<&mut T> where @@ -16,7 +17,7 @@ pub trait UpdateMap: Default + Clone { fn get_cow_with<'a, F>(&'a mut self, k: usize, f: F) -> Option> where - F: FnOnce(usize) -> Option<&'a T>, + F: FnOnce(usize) -> Option>, T: Clone + 'a; fn insert(&mut self, k: usize, value: T) -> Option; @@ -36,8 +37,8 @@ pub trait UpdateMap: Default + Clone { } impl UpdateMap for BTreeMap { - fn get(&self, k: usize) -> Option<&T> { - BTreeMap::get(self, &k) + fn get(&self, k: usize) -> Option> { + BTreeMap::get(self, &k).map(|res| StdCow::Borrowed(res)) } fn get_mut_with(&mut self, idx: usize, f: F) -> Option<&mut T> @@ -56,7 +57,7 @@ impl UpdateMap for BTreeMap { fn get_cow_with<'a, F>(&'a mut self, idx: usize, f: F) -> Option> where - F: FnOnce(usize) -> Option<&'a T>, + F: FnOnce(usize) -> Option>, { let cow = match self.entry(idx) { Entry::Vacant(entry) => { @@ -97,8 +98,8 @@ impl UpdateMap for BTreeMap { } impl UpdateMap for VecMap { - fn get(&self, k: usize) -> Option<&T> { - VecMap::get(self, k) + fn get(&self, k: usize) -> Option> { + VecMap::get(self, k).map(|res| StdCow::Borrowed(res)) } fn get_mut_with(&mut self, idx: usize, f: F) -> Option<&mut T> @@ -117,7 +118,7 @@ impl UpdateMap for VecMap { fn get_cow_with<'a, F>(&'a mut self, idx: usize, f: F) -> Option> where - F: FnOnce(usize) -> Option<&'a T>, + F: FnOnce(usize) -> Option>, { let cow = match self.entry(idx) { vec_map::Entry::Vacant(entry) => { @@ -171,11 +172,11 @@ pub struct MaxMap { max_key: usize, } -impl UpdateMap for MaxMap +impl UpdateMap for MaxMap where M: UpdateMap, { - fn get(&self, k: usize) -> Option<&T> { + fn get(&self, k: usize) -> Option> { self.inner.get(k) } @@ -188,7 +189,7 @@ where fn get_cow_with<'a, F>(&'a mut self, k: usize, f: F) -> Option> where - F: FnOnce(usize) -> Option<&'a T>, + F: FnOnce(usize) -> Option>, T: Clone + 'a, { self.inner.get_cow_with(k, f) diff --git a/src/utils.rs b/src/utils.rs index e0aa78a..01e84ce 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -49,7 +49,7 @@ pub fn max_btree_index(map: &BTreeMap) -> Option { } /// Compute the length a data structure will have after applying `updates`. -pub fn updated_length, T>(prev_len: Length, updates: &U) -> Length { +pub fn updated_length, T: Clone>(prev_len: Length, updates: &U) -> Length { updates.max_index().map_or(prev_len, |max_idx| { Length(std::cmp::max(max_idx + 1, prev_len.as_usize())) }) diff --git a/src/vector.rs b/src/vector.rs index c57189e..89b7ab6 100644 --- a/src/vector.rs +++ b/src/vector.rs @@ -4,38 +4,35 @@ use crate::interface_iter::InterfaceIter; use crate::iter::Iter; use crate::update_map::MaxMap; use crate::utils::{arb_arc, Length}; -use crate::{Arc, Cow, Error, List, Tree, UpdateMap}; +use crate::{Arc, Cow, Error, List, Tree, UpdateMap, Value}; use arbitrary::Arbitrary; use derivative::Derivative; use serde::{Deserialize, Serialize}; use ssz::{Decode, Encode, SszEncoder, TryFromIter, BYTES_PER_LENGTH_OFFSET}; +use std::borrow::Cow as StdCow; use std::collections::BTreeMap; use std::convert::TryFrom; use std::marker::PhantomData; -use tree_hash::{Hash256, PackedEncoding, TreeHash}; +use tree_hash::{Hash256, PackedEncoding}; use typenum::Unsigned; use vec_map::VecMap; #[derive(Debug, Derivative, Clone, Serialize, Deserialize, Arbitrary)] -#[derivative(PartialEq( - bound = "T: TreeHash + Clone + PartialEq, N: Unsigned, U: UpdateMap + PartialEq" -))] +#[derivative(PartialEq(bound = "T: Value, N: Unsigned, U: UpdateMap + PartialEq"))] #[serde(try_from = "List")] #[serde(into = "List")] -#[serde(bound(serialize = "T: TreeHash + Clone + Serialize, N: Unsigned, U: UpdateMap"))] -#[serde(bound( - deserialize = "T: TreeHash + Clone + Deserialize<'de>, N: Unsigned, U: UpdateMap" -))] -#[arbitrary(bound = "T: Arbitrary<'arbitrary> + TreeHash + Clone")] +#[serde(bound(serialize = "T: Value + Serialize, N: Unsigned, U: UpdateMap"))] +#[serde(bound(deserialize = "T: Value + Deserialize<'de>, N: Unsigned, U: UpdateMap"))] +#[arbitrary(bound = "T: Arbitrary<'arbitrary> + Value")] #[arbitrary(bound = "N: Unsigned, U: Arbitrary<'arbitrary> + UpdateMap")] -pub struct Vector = MaxMap>> { +pub struct Vector = MaxMap>> { pub(crate) interface: Interface, U>, } #[derive(Debug, Derivative, Clone, Arbitrary)] -#[derivative(PartialEq(bound = "T: TreeHash + Clone + PartialEq, N: Unsigned"))] -#[arbitrary(bound = "T: Arbitrary<'arbitrary> + TreeHash + Clone, N: Unsigned")] -pub struct VectorInner { +#[derivative(PartialEq(bound = "T: Value, N: Unsigned"))] +#[arbitrary(bound = "T: Arbitrary<'arbitrary> + Value, N: Unsigned")] +pub struct VectorInner { #[arbitrary(with = arb_arc)] pub(crate) tree: Arc>, pub(crate) depth: usize, @@ -44,7 +41,7 @@ pub struct VectorInner { _phantom: PhantomData, } -impl> Vector { +impl> Vector { pub fn new(vec: Vec) -> Result { if vec.len() == N::to_usize() { Self::try_from(List::new(vec)?) @@ -65,7 +62,7 @@ impl> Vector { } pub fn to_vec(&self) -> Vec { - self.iter().cloned().collect() + self.iter().map(|res| res.into_owned()).collect() } pub fn iter(&self) -> InterfaceIter { @@ -83,7 +80,7 @@ impl> Vector { } // Wrap trait methods so we present a Vec-like interface without having to import anything. - pub fn get(&self, index: usize) -> Option<&T> { + pub fn get(&self, index: usize) -> Option> { self.interface.get(index) } @@ -112,7 +109,7 @@ impl> Vector { } } -impl> TryFrom> for Vector { +impl> TryFrom> for Vector { type Error = Error; fn try_from(list: List) -> Result { @@ -140,9 +137,7 @@ impl> TryFrom> f } } -impl> - Vector -{ +impl> Vector { pub fn rebase(&self, base: &Self) -> Result { // Diff self from base. let diff = VectorDiff::compute_diff(base, self)?; @@ -161,7 +156,7 @@ impl> From> for List { +impl> From> for List { fn from(vector: Vector) -> Self { List::from_parts( vector.interface.backing.tree, @@ -171,8 +166,8 @@ impl> From> fo } } -impl ImmList for VectorInner { - fn get(&self, index: usize) -> Option<&T> { +impl ImmList for VectorInner { + fn get(&self, index: usize) -> Option> { if index < self.len().as_usize() { self.tree .get_recursive(index, self.depth, self.packing_depth) @@ -192,7 +187,7 @@ impl ImmList for VectorInner { impl MutList for VectorInner where - T: TreeHash + Clone, + T: Value, N: Unsigned, { fn validate_push(_current_len: usize) -> Result<(), Error> { @@ -230,7 +225,7 @@ where } } -impl Default for Vector { +impl Default for Vector { fn default() -> Self { Self::from_elem(T::default()).unwrap_or_else(|e| { panic!( @@ -242,7 +237,7 @@ impl Default for Vector { } } -impl tree_hash::TreeHash for Vector { +impl tree_hash::TreeHash for Vector { fn tree_hash_type() -> tree_hash::TreeHashType { tree_hash::TreeHashType::Vector } @@ -264,7 +259,7 @@ impl tree_hash::TreeHash for Vec impl TryFromIter for Vector where - T: TreeHash + Clone, + T: Value, N: Unsigned, { type Error = Error; @@ -277,8 +272,8 @@ where } } -impl<'a, T: TreeHash + Clone, N: Unsigned, U: UpdateMap> IntoIterator for &'a Vector { - type Item = &'a T; +impl<'a, T: Value, N: Unsigned, U: UpdateMap> IntoIterator for &'a Vector { + type Item = StdCow<'a, T>; type IntoIter = InterfaceIter<'a, T, U>; fn into_iter(self) -> Self::IntoIter { @@ -287,14 +282,14 @@ impl<'a, T: TreeHash + Clone, N: Unsigned, U: UpdateMap> IntoIterator for &'a } // FIXME: duplicated from `ssz::encode::impl_for_vec` -impl Encode for Vector { +impl Encode for Vector { fn is_ssz_fixed_len() -> bool { - T::is_ssz_fixed_len() + ::is_ssz_fixed_len() } fn ssz_fixed_len() -> usize { if ::is_ssz_fixed_len() { - T::ssz_fixed_len() * N::to_usize() + ::ssz_fixed_len() * N::to_usize() } else { BYTES_PER_LENGTH_OFFSET } @@ -311,8 +306,8 @@ impl Encode for Vector { } fn ssz_append(&self, buf: &mut Vec) { - if T::is_ssz_fixed_len() { - buf.reserve(T::ssz_fixed_len() * self.len()); + if ::is_ssz_fixed_len() { + buf.reserve(::ssz_fixed_len() * self.len()); for item in self.iter() { item.ssz_append(buf); @@ -321,7 +316,7 @@ impl Encode for Vector { let mut encoder = SszEncoder::container(buf, self.len() * ssz::BYTES_PER_LENGTH_OFFSET); for item in self.iter() { - encoder.append(item); + encoder.append(&item.into_owned()); } encoder.finalize(); @@ -329,14 +324,14 @@ impl Encode for Vector { } } -impl Decode for Vector { +impl Decode for Vector { fn is_ssz_fixed_len() -> bool { - T::is_ssz_fixed_len() + ::is_ssz_fixed_len() } fn ssz_fixed_len() -> usize { if ::is_ssz_fixed_len() { - T::ssz_fixed_len() * N::to_usize() + ::ssz_fixed_len() * N::to_usize() } else { ssz::BYTES_PER_LENGTH_OFFSET } From 193d8c095b66c111b01e7547b67358091b6ae670 Mon Sep 17 00:00:00 2001 From: Mac L Date: Thu, 13 Jul 2023 14:27:51 +1000 Subject: [PATCH 02/10] Fix clippy --- src/list.rs | 2 +- src/packed_leaf.rs | 2 +- src/vector.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/list.rs b/src/list.rs index 35c80ff..79de59d 100644 --- a/src/list.rs +++ b/src/list.rs @@ -353,7 +353,7 @@ impl Encode for List { let mut encoder = SszEncoder::container(buf, self.len() * BYTES_PER_LENGTH_OFFSET); for item in self { - encoder.append(&item.into_owned()); + encoder.append(item.as_ref()); } encoder.finalize(); diff --git a/src/packed_leaf.rs b/src/packed_leaf.rs index 511141b..30b65ca 100644 --- a/src/packed_leaf.rs +++ b/src/packed_leaf.rs @@ -159,7 +159,7 @@ impl PackedLeaf { return Err(Error::PackedLeafFull { len: self.length() }); } - self.insert_mut(self.length(), value.clone())?; + self.insert_mut(self.length(), value)?; Ok(()) } diff --git a/src/vector.rs b/src/vector.rs index 89b7ab6..373af76 100644 --- a/src/vector.rs +++ b/src/vector.rs @@ -316,7 +316,7 @@ impl Encode for Vector { let mut encoder = SszEncoder::container(buf, self.len() * ssz::BYTES_PER_LENGTH_OFFSET); for item in self.iter() { - encoder.append(&item.into_owned()); + encoder.append(item.as_ref()); } encoder.finalize(); From bce1c83f26a09277b49625ae496dad1b9884185e Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Fri, 14 Jul 2023 15:07:52 +1000 Subject: [PATCH 03/10] Use unsafe to great effect Co-authored-by: Mac Ladson --- src/cow.rs | 9 +++--- src/interface.rs | 12 +++---- src/interface_iter.rs | 5 ++- src/iter.rs | 7 ++--- src/list.rs | 11 +++---- src/packed_leaf.rs | 54 ++++++++++++++------------------ src/tests/builder.rs | 16 ++-------- src/tests/iterator.rs | 25 +++------------ src/tests/packed.rs | 18 ++++------- src/tests/proptest/operations.rs | 15 ++++----- src/tests/size_of.rs | 13 +++----- src/tree.rs | 34 +++++++++----------- src/update_map.rs | 21 ++++++------- src/vector.rs | 11 +++---- 14 files changed, 94 insertions(+), 157 deletions(-) diff --git a/src/cow.rs b/src/cow.rs index 284955c..ccdd53b 100644 --- a/src/cow.rs +++ b/src/cow.rs @@ -1,4 +1,3 @@ -use std::borrow::Cow as StdCow; use std::collections::btree_map::VacantEntry; use std::ops::Deref; @@ -34,7 +33,7 @@ pub trait CowTrait<'a, T: Clone>: Deref { pub enum BTreeCow<'a, T: Clone> { Immutable { - value: StdCow<'a, T>, + value: &'a T, entry: VacantEntry<'a, usize, T>, }, Mutable { @@ -45,7 +44,7 @@ pub enum BTreeCow<'a, T: Clone> { impl<'a, T: Clone> CowTrait<'a, T> for BTreeCow<'a, T> { fn to_mut(self) -> &'a mut T { match self { - Self::Immutable { value, entry } => entry.insert(value.into_owned()), + Self::Immutable { value, entry } => entry.insert(value.clone()), Self::Mutable { value } => value, } } @@ -64,7 +63,7 @@ impl<'a, T: Clone> Deref for BTreeCow<'a, T> { pub enum VecCow<'a, T: Clone> { Immutable { - value: StdCow<'a, T>, + value: &'a T, entry: vec_map::VacantEntry<'a, T>, }, Mutable { @@ -75,7 +74,7 @@ pub enum VecCow<'a, T: Clone> { impl<'a, T: Clone> CowTrait<'a, T> for VecCow<'a, T> { fn to_mut(self) -> &'a mut T { match self { - Self::Immutable { value, entry } => entry.insert(value.into_owned()), + Self::Immutable { value, entry } => entry.insert(value.clone()), Self::Mutable { value } => value, } } diff --git a/src/interface.rs b/src/interface.rs index edae456..05c34fd 100644 --- a/src/interface.rs +++ b/src/interface.rs @@ -6,13 +6,12 @@ use crate::{ Cow, Error, Value, }; use arbitrary::Arbitrary; -use std::borrow::Cow as StdCow; use std::collections::BTreeMap; use std::marker::PhantomData; use tree_hash::Hash256; pub trait ImmList { - fn get(&self, idx: usize) -> Option>; + fn get(&self, idx: usize) -> Option<&T>; fn len(&self) -> Length; @@ -59,13 +58,13 @@ where } } - pub fn get(&self, idx: usize) -> Option> { + pub fn get(&self, idx: usize) -> Option<&T> { self.updates.get(idx).or_else(|| self.backing.get(idx)) } pub fn get_mut(&mut self, idx: usize) -> Option<&mut T> { self.updates - .get_mut_with(idx, |idx| self.backing.get(idx).map(|res| res.into_owned())) + .get_mut_with(idx, |idx| self.backing.get(idx).cloned()) } pub fn get_cow(&mut self, index: usize) -> Option> { @@ -175,10 +174,7 @@ mod test { *c2.to_mut() = 11; assert_eq!(*list.get(0).unwrap(), 11); - assert_eq!( - list.iter().map(|res| res.into_owned()).collect::>(), - vec![11, 2, 3] - ); + assert_eq!(list.iter().copied().collect::>(), vec![11, 2, 3]); } #[test] diff --git a/src/interface_iter.rs b/src/interface_iter.rs index e659d42..aff6a6b 100644 --- a/src/interface_iter.rs +++ b/src/interface_iter.rs @@ -1,6 +1,5 @@ use crate::iter::Iter; use crate::{Cow, UpdateMap, Value}; -use std::borrow::Cow as StdCow; #[derive(Debug)] pub struct InterfaceIter<'a, T: Value, U: UpdateMap> { @@ -11,9 +10,9 @@ pub struct InterfaceIter<'a, T: Value, U: UpdateMap> { } impl<'a, T: Value, U: UpdateMap> Iterator for InterfaceIter<'a, T, U> { - type Item = StdCow<'a, T>; + type Item = &'a T; - fn next(&mut self) -> Option> { + fn next(&mut self) -> Option<&'a T> { let index = self.index; self.index += 1; diff --git a/src/iter.rs b/src/iter.rs index 25489b2..99500bb 100644 --- a/src/iter.rs +++ b/src/iter.rs @@ -2,7 +2,6 @@ use crate::{ utils::{opt_packing_depth, opt_packing_factor, Length}, Leaf, Tree, Value, }; -use std::borrow::Cow as StdCow; #[derive(Debug)] pub struct Iter<'a, T: Value> { @@ -39,7 +38,7 @@ impl<'a, T: Value> Iter<'a, T> { } impl<'a, T: Value> Iterator for Iter<'a, T> { - type Item = StdCow<'a, T>; + type Item = &'a T; fn next(&mut self) -> Option { if self.index >= self.length.as_usize() { @@ -58,7 +57,7 @@ impl<'a, T: Value> Iterator for Iter<'a, T> { self.stack.pop(); } - result.map(|res| StdCow::Borrowed(res)) + result } Some(Tree::PackedLeaf(packed_leaf)) => { let sub_index = self.index % self.packing_factor; @@ -80,7 +79,7 @@ impl<'a, T: Value> Iterator for Iter<'a, T> { } } - result.map(|res| StdCow::Owned(res)) + result } Some(Tree::Node { left, right, .. }) => { let depth = self.full_depth - self.stack.len(); diff --git a/src/list.rs b/src/list.rs index 79de59d..42c8ef8 100644 --- a/src/list.rs +++ b/src/list.rs @@ -12,7 +12,6 @@ use derivative::Derivative; use itertools::process_results; use serde::{ser::SerializeSeq, Deserialize, Deserializer, Serialize, Serializer}; use ssz::{Decode, Encode, SszEncoder, TryFromIter, BYTES_PER_LENGTH_OFFSET}; -use std::borrow::Cow as StdCow; use std::collections::BTreeMap; use std::marker::PhantomData; use tree_hash::{Hash256, PackedEncoding, TreeHash}; @@ -104,7 +103,7 @@ impl> List { } pub fn to_vec(&self) -> Vec { - self.iter().map(|res| res.into_owned()).collect() + self.iter().cloned().collect() } pub fn iter(&self) -> InterfaceIter { @@ -127,7 +126,7 @@ impl> List { } // Wrap trait methods so we present a Vec-like interface without having to import anything. - pub fn get(&self, index: usize) -> Option> { + pub fn get(&self, index: usize) -> Option<&T> { self.interface.get(index) } @@ -173,7 +172,7 @@ impl> List { } impl ImmList for ListInner { - fn get(&self, index: usize) -> Option> { + fn get(&self, index: usize) -> Option<&T> { if index < self.len().as_usize() { self.tree .get_recursive(index, self.depth, self.packing_depth) @@ -288,7 +287,7 @@ impl TreeHash for List { } impl<'a, T: Value, N: Unsigned, U: UpdateMap> IntoIterator for &'a List { - type Item = StdCow<'a, T>; + type Item = &'a T; type IntoIter = InterfaceIter<'a, T, U>; fn into_iter(self) -> Self::IntoIter { @@ -353,7 +352,7 @@ impl Encode for List { let mut encoder = SszEncoder::container(buf, self.len() * BYTES_PER_LENGTH_OFFSET); for item in self { - encoder.append(item.as_ref()); + encoder.append(item); } encoder.finalize(); diff --git a/src/packed_leaf.rs b/src/packed_leaf.rs index 30b65ca..4aa916a 100644 --- a/src/packed_leaf.rs +++ b/src/packed_leaf.rs @@ -1,17 +1,14 @@ -use crate::{utils::arb_rwlock, Error, UpdateMap, Value}; +use crate::{Error, UpdateMap, Value}; use arbitrary::Arbitrary; use core::marker::PhantomData; use derivative::Derivative; -use parking_lot::RwLock; use std::ops::ControlFlow; use tree_hash::{Hash256, BYTES_PER_CHUNK}; #[derive(Debug, Derivative, Arbitrary)] #[derivative(PartialEq, Hash)] pub struct PackedLeaf { - #[derivative(PartialEq = "ignore", Hash = "ignore")] - #[arbitrary(with = arb_rwlock)] - pub hash: RwLock, + pub hash: Hash256, pub length: u8, _phantom: PhantomData, } @@ -22,7 +19,7 @@ where { fn clone(&self) -> Self { Self { - hash: RwLock::new(*self.hash.read()), + hash: self.hash, length: self.length, _phantom: PhantomData, } @@ -30,35 +27,31 @@ where } impl PackedLeaf { - fn length(&self) -> usize { + pub fn length(&self) -> usize { self.length as usize } - fn value_len(_value: &T) -> usize { + fn value_len() -> usize { BYTES_PER_CHUNK / T::tree_hash_packing_factor() } - pub fn values(&self) -> Vec { - self.hash - .read() - .as_bytes() - .chunks_exact(BYTES_PER_CHUNK / T::tree_hash_packing_factor()) - .take(self.length()) - .map(|bytes| T::from_ssz_bytes(bytes).expect("Should always deserialize")) - .collect::>() - } - - pub fn get(&self, index: usize) -> Option { - self.values().get(index).cloned() + pub fn get(&self, index: usize) -> Option<&T> { + if index >= self.length() { + return None; + } + let hash_base_ptr: *const Hash256 = &self.hash; + let base_ptr: *const T = hash_base_ptr as *const T; + let elem_ptr: *const T = unsafe { base_ptr.offset(index as isize) }; + Some(unsafe { &*elem_ptr }) } pub fn tree_hash(&self) -> Hash256 { - *self.hash.read() + self.hash } pub fn empty() -> Self { PackedLeaf { - hash: RwLock::new(Hash256::zero()), + hash: Hash256::zero(), length: 0, _phantom: PhantomData, } @@ -68,11 +61,11 @@ impl PackedLeaf { let mut hash = Hash256::zero(); let hash_bytes = hash.as_bytes_mut(); - let value_len = Self::value_len(&value); + let value_len = Self::value_len(); hash_bytes[0..value_len].copy_from_slice(&value.as_ssz_bytes()); PackedLeaf { - hash: RwLock::new(hash), + hash, length: 1, _phantom: PhantomData, } @@ -84,14 +77,14 @@ impl PackedLeaf { let mut hash = Hash256::zero(); let hash_bytes = hash.as_bytes_mut(); - let value_len = Self::value_len(&value); + let value_len = Self::value_len(); for (i, value) in vec![value; n].iter().enumerate() { hash_bytes[i * value_len..(i + 1) * value_len].copy_from_slice(&value.as_ssz_bytes()); } PackedLeaf { - hash: RwLock::new(hash), + hash, length: n as u8, _phantom: PhantomData, } @@ -105,6 +98,7 @@ impl PackedLeaf { Ok(updated) } + // FIXME: remove _hash/work out what's going on pub fn update>( &self, prefix: usize, @@ -126,7 +120,7 @@ impl PackedLeaf { pub fn insert_mut(&mut self, index: usize, value: T) -> Result<(), Error> { // Convert the index to the index of the underlying bytes. - let sub_index = index * Self::value_len(&value); + let sub_index = index * Self::value_len(); if sub_index >= BYTES_PER_CHUNK { return Err(Error::PackedLeafOutOfBounds { @@ -135,14 +129,14 @@ impl PackedLeaf { }); } - let value_len = Self::value_len(&value); + let value_len = Self::value_len(); - let mut hash = *self.hash.read(); + let mut hash = self.hash; let hash_bytes = hash.as_bytes_mut(); hash_bytes[sub_index..sub_index + value_len].copy_from_slice(&value.as_ssz_bytes()); - *self.hash.write() = hash; + self.hash = hash; if index == self.length() { self.length += 1; diff --git a/src/tests/builder.rs b/src/tests/builder.rs index caa5892..f516b6e 100644 --- a/src/tests/builder.rs +++ b/src/tests/builder.rs @@ -17,13 +17,7 @@ fn build_partial_hash256_list() { let slow_list = List::::try_from_iter_slow(sub_vec.clone()).unwrap(); assert_eq!(fast_list, slow_list); - assert_eq!( - fast_list - .iter() - .map(|item| item.into_owned()) - .collect::>(), - sub_vec - ); + assert_eq!(fast_list.iter().cloned().collect::>(), sub_vec); } } @@ -40,12 +34,6 @@ fn build_packed_u64_list() { let slow_list = List::::try_from_iter(sub_vec.clone()).unwrap(); assert_eq!(fast_list, slow_list); - assert_eq!( - fast_list - .iter() - .map(|item| item.into_owned()) - .collect::>(), - sub_vec - ); + assert_eq!(fast_list.iter().cloned().collect::>(), sub_vec); } } diff --git a/src/tests/iterator.rs b/src/tests/iterator.rs index 5be4719..3afaaf2 100644 --- a/src/tests/iterator.rs +++ b/src/tests/iterator.rs @@ -9,13 +9,7 @@ fn hash256_vec_iter() { let vec = (0..n).map(Hash256::from_low_u64_be).collect::>(); let vector = Vector::::new(vec.clone()).unwrap(); - assert_eq!( - vector - .iter() - .map(|item| item.into_owned()) - .collect::>(), - vec - ); + assert_eq!(vector.iter().cloned().collect::>(), vec); } #[test] @@ -25,12 +19,7 @@ fn hash256_list_iter() { let vec = (0..n).map(Hash256::from_low_u64_be).collect::>(); let list = List::::new(vec.clone()).unwrap(); - assert_eq!( - list.iter() - .map(|item| item.into_owned()) - .collect::>(), - vec - ); + assert_eq!(list.iter().cloned().collect::>(), vec); } #[test] @@ -44,10 +33,7 @@ fn hash256_list_iter_from() { for i in 0..=n { assert_eq!( - list.iter_from(i) - .unwrap() - .map(|item| item.into_owned()) - .collect::>(), + list.iter_from(i).unwrap().cloned().collect::>(), &vec[i..] ); } @@ -72,10 +58,7 @@ fn hash256_vector_iter_from() { for i in 0..=n { assert_eq!( - vect.iter_from(i) - .unwrap() - .map(|item| item.into_owned()) - .collect::>(), + vect.iter_from(i).unwrap().cloned().collect::>(), &vec[i..] ); } diff --git a/src/tests/packed.rs b/src/tests/packed.rs index 2ef994d..0c5c2d8 100644 --- a/src/tests/packed.rs +++ b/src/tests/packed.rs @@ -9,11 +9,11 @@ fn u64_packed_list_build_and_iter() { let vec = (0..len).map(|i| 2 * i).collect::>(); let list = List::::new(vec.clone()).unwrap(); - let from_iter = list.iter().map(|res| res.into_owned()).collect::>(); + let from_iter = list.iter().cloned().collect::>(); assert_eq!(vec, from_iter); for i in 0..len as usize { - assert_eq!(list.get(i).map(|res| res.into_owned()).as_ref(), vec.get(i)); + assert_eq!(list.get(i).cloned().as_ref(), vec.get(i)); } } } @@ -36,17 +36,11 @@ fn u64_packed_vector_build_and_iter() { let vec = (0..len).map(|i| 2 * i).collect::>(); let vector = Vector::::new(vec.clone()).unwrap(); - let from_iter = vector - .iter() - .map(|res| res.into_owned()) - .collect::>(); + let from_iter = vector.iter().cloned().collect::>(); assert_eq!(vec, from_iter); for i in 0..len as usize { - assert_eq!( - vector.get(i).map(|res| res.into_owned()).as_ref(), - vec.get(i) - ); + assert_eq!(vector.get(i).cloned().as_ref(), vec.get(i)); } } @@ -80,11 +74,11 @@ fn out_of_order_mutations() { for (i, v) in mutations { *list.get_mut(i).unwrap() = v; vec[i] = v; - assert_eq!(list.get(i).map(|res| res.into_owned()), Some(v)); + assert_eq!(list.get(i).cloned(), Some(v)); list.apply_updates().unwrap(); - assert_eq!(list.get(i).map(|res| res.into_owned()), Some(v)); + assert_eq!(list.get(i).cloned(), Some(v)); } assert_eq!(list.to_vec(), vec); diff --git a/src/tests/proptest/operations.rs b/src/tests/proptest/operations.rs index 141cf5d..9f9631d 100644 --- a/src/tests/proptest/operations.rs +++ b/src/tests/proptest/operations.rs @@ -2,7 +2,6 @@ use super::{arb_hash256, arb_index, arb_large, arb_list, arb_vect, Large}; use crate::{Diff, Error, List, ListDiff, Vector, VectorDiff}; use proptest::prelude::*; use ssz::{Decode, Encode}; -use std::borrow::Cow as StdCow; use std::fmt::Debug; use std::marker::PhantomData; use tree_hash::{Hash256, TreeHash}; @@ -41,15 +40,13 @@ impl Spec { self.values.len() } - pub fn iter(&self) -> impl Iterator> { - self.values.iter().map(|item| StdCow::Borrowed(item)) + pub fn iter(&self) -> impl Iterator { + self.values.iter() } - pub fn iter_from(&self, index: usize) -> Result>, Error> { + pub fn iter_from(&self, index: usize) -> Result, Error> { if index <= self.len() { - Ok(self.values[index..] - .iter() - .map(|item| StdCow::Borrowed(item))) + Ok(self.values[index..].iter()) } else { Err(Error::OutOfBoundsIterFrom { index, @@ -58,8 +55,8 @@ impl Spec { } } - pub fn get(&self, index: usize) -> Option> { - self.values.get(index).map(|val| StdCow::Borrowed(val)) + pub fn get(&self, index: usize) -> Option<&T> { + self.values.get(index) } pub fn set(&mut self, index: usize, value: T) -> Option<()> { diff --git a/src/tests/size_of.rs b/src/tests/size_of.rs index a7694ed..87fdc45 100644 --- a/src/tests/size_of.rs +++ b/src/tests/size_of.rs @@ -8,7 +8,7 @@ use tree_hash::Hash256; fn size_of_hash256() { assert_eq!(size_of::>(), 64); assert_eq!(size_of::>(), 48); - assert_eq!(size_of::>(), 48); + assert_eq!(size_of::>(), 33); let rw_lock_size = size_of::>(); assert_eq!(rw_lock_size, 40); @@ -16,10 +16,7 @@ fn size_of_hash256() { let arc_size = size_of::>>(); assert_eq!(arc_size, 8); - assert_eq!( - size_of::>(), - size_of::>() + 16 - ); + assert_eq!(size_of::>(), size_of::>() + 16); } /// It's important that the Tree nodes have a predictable size. @@ -27,10 +24,10 @@ fn size_of_hash256() { fn size_of_u8() { assert_eq!(size_of::>(), 64); assert_eq!(size_of::>(), 48); - assert_eq!(size_of::>(), 48); + assert_eq!(size_of::>(), 33); assert_eq!( size_of::>(), - size_of::>() + size_of::() + size_of::() + size_of::() ); let rw_lock_size = size_of::>(); @@ -39,5 +36,5 @@ fn size_of_u8() { let arc_size = size_of::>>(); assert_eq!(arc_size, 8); - assert_eq!(size_of::>(), size_of::>() + 16); + assert_eq!(size_of::>(), size_of::>() + 16); } diff --git a/src/tree.rs b/src/tree.rs index e45b1f8..24f3faa 100644 --- a/src/tree.rs +++ b/src/tree.rs @@ -6,7 +6,6 @@ use ethereum_hashing::{hash32_concat, ZERO_HASHES}; use parking_lot::RwLock; use serde::{Deserialize, Serialize}; use ssz_derive::{Decode, Encode}; -use std::borrow::Cow as StdCow; use std::collections::BTreeMap; use std::ops::ControlFlow; use tree_hash::Hash256; @@ -84,17 +83,12 @@ impl Tree { Self::Leaf(Leaf::new(value)) } - pub fn get_recursive( - &self, - index: usize, - depth: usize, - packing_depth: usize, - ) -> Option> { + pub fn get_recursive(&self, index: usize, depth: usize, packing_depth: usize) -> Option<&T> { match self { - Self::Leaf(Leaf { value, .. }) if depth == 0 => Some(StdCow::Borrowed(value)), - Self::PackedLeaf(packed_leaf) if depth == 0 => packed_leaf - .get(index % T::tree_hash_packing_factor()) - .map(|res| StdCow::Owned(res)), + Self::Leaf(Leaf { value, .. }) if depth == 0 => Some(value), + Self::PackedLeaf(packed_leaf) if depth == 0 => { + packed_leaf.get(index % T::tree_hash_packing_factor()) + } Self::Node { left, right, .. } if depth > 0 => { let new_depth = depth - 1; // Left @@ -176,7 +170,7 @@ impl Tree { let index = prefix; let value = updates .get(index) - .map(|res| res.into_owned()) + .cloned() .ok_or(Error::LeafUpdateMissing { index })?; Ok(Self::leaf_with_hash(value, hash)) } @@ -228,7 +222,7 @@ impl Tree { let index = prefix; let value = updates .get(index) - .map(|res| res.into_owned()) + .cloned() .ok_or(Error::LeafUpdateMissing { index })?; Ok(Self::leaf_with_hash(value, hash)) } @@ -263,9 +257,9 @@ impl Tree { } (Self::PackedLeaf(l1), Self::PackedLeaf(l2)) if depth == 0 => { let mut equal = true; - for i in 0..l2.values().len() { - let v2 = &l2.values()[i]; - match l1.values().get(i) { + for i in 0..l2.length() { + let v2 = l2.get(i).unwrap(); // FIXME + match l1.get(i) { Some(v1) if v1 == v2 => continue, _ => { equal = false; @@ -275,7 +269,7 @@ impl Tree { } } if !equal { - let hash = *l2.hash.read(); + let hash = l2.hash; diff.hashes.insert((depth, prefix), hash); } Ok(()) @@ -347,9 +341,9 @@ impl Tree { Ok(()) } Self::PackedLeaf(packed_leaf) if depth == 0 => { - diff.hashes - .insert((depth, prefix), *packed_leaf.hash.read()); - for (i, value) in packed_leaf.values().iter().enumerate() { + diff.hashes.insert((depth, prefix), packed_leaf.hash); + for i in 0..packed_leaf.length() { + let value = packed_leaf.get(i).unwrap(); diff.leaves.insert(prefix | i, value.clone()); } Ok(()) diff --git a/src/update_map.rs b/src/update_map.rs index fde00c1..81651e0 100644 --- a/src/update_map.rs +++ b/src/update_map.rs @@ -1,7 +1,6 @@ use crate::cow::{BTreeCow, Cow, VecCow}; use crate::utils::max_btree_index; use arbitrary::Arbitrary; -use std::borrow::Cow as StdCow; use std::collections::{btree_map::Entry, BTreeMap}; use std::ops::ControlFlow; use vec_map::VecMap; @@ -9,7 +8,7 @@ use vec_map::VecMap; /// Trait for map types which can be used to store intermediate updates before application /// to the tree. pub trait UpdateMap: Default + Clone { - fn get(&self, k: usize) -> Option>; + fn get(&self, k: usize) -> Option<&T>; fn get_mut_with(&mut self, k: usize, f: F) -> Option<&mut T> where @@ -17,7 +16,7 @@ pub trait UpdateMap: Default + Clone { fn get_cow_with<'a, F>(&'a mut self, k: usize, f: F) -> Option> where - F: FnOnce(usize) -> Option>, + F: FnOnce(usize) -> Option<&'a T>, T: Clone + 'a; fn insert(&mut self, k: usize, value: T) -> Option; @@ -37,8 +36,8 @@ pub trait UpdateMap: Default + Clone { } impl UpdateMap for BTreeMap { - fn get(&self, k: usize) -> Option> { - BTreeMap::get(self, &k).map(|res| StdCow::Borrowed(res)) + fn get(&self, k: usize) -> Option<&T> { + BTreeMap::get(self, &k) } fn get_mut_with(&mut self, idx: usize, f: F) -> Option<&mut T> @@ -57,7 +56,7 @@ impl UpdateMap for BTreeMap { fn get_cow_with<'a, F>(&'a mut self, idx: usize, f: F) -> Option> where - F: FnOnce(usize) -> Option>, + F: FnOnce(usize) -> Option<&'a T>, { let cow = match self.entry(idx) { Entry::Vacant(entry) => { @@ -98,8 +97,8 @@ impl UpdateMap for BTreeMap { } impl UpdateMap for VecMap { - fn get(&self, k: usize) -> Option> { - VecMap::get(self, k).map(|res| StdCow::Borrowed(res)) + fn get(&self, k: usize) -> Option<&T> { + VecMap::get(self, k) } fn get_mut_with(&mut self, idx: usize, f: F) -> Option<&mut T> @@ -118,7 +117,7 @@ impl UpdateMap for VecMap { fn get_cow_with<'a, F>(&'a mut self, idx: usize, f: F) -> Option> where - F: FnOnce(usize) -> Option>, + F: FnOnce(usize) -> Option<&'a T>, { let cow = match self.entry(idx) { vec_map::Entry::Vacant(entry) => { @@ -176,7 +175,7 @@ impl UpdateMap for MaxMap where M: UpdateMap, { - fn get(&self, k: usize) -> Option> { + fn get(&self, k: usize) -> Option<&T> { self.inner.get(k) } @@ -189,7 +188,7 @@ where fn get_cow_with<'a, F>(&'a mut self, k: usize, f: F) -> Option> where - F: FnOnce(usize) -> Option>, + F: FnOnce(usize) -> Option<&'a T>, T: Clone + 'a, { self.inner.get_cow_with(k, f) diff --git a/src/vector.rs b/src/vector.rs index 373af76..bc1591a 100644 --- a/src/vector.rs +++ b/src/vector.rs @@ -9,7 +9,6 @@ use arbitrary::Arbitrary; use derivative::Derivative; use serde::{Deserialize, Serialize}; use ssz::{Decode, Encode, SszEncoder, TryFromIter, BYTES_PER_LENGTH_OFFSET}; -use std::borrow::Cow as StdCow; use std::collections::BTreeMap; use std::convert::TryFrom; use std::marker::PhantomData; @@ -62,7 +61,7 @@ impl> Vector { } pub fn to_vec(&self) -> Vec { - self.iter().map(|res| res.into_owned()).collect() + self.iter().cloned().collect() } pub fn iter(&self) -> InterfaceIter { @@ -80,7 +79,7 @@ impl> Vector { } // Wrap trait methods so we present a Vec-like interface without having to import anything. - pub fn get(&self, index: usize) -> Option> { + pub fn get(&self, index: usize) -> Option<&T> { self.interface.get(index) } @@ -167,7 +166,7 @@ impl> From> for List ImmList for VectorInner { - fn get(&self, index: usize) -> Option> { + fn get(&self, index: usize) -> Option<&T> { if index < self.len().as_usize() { self.tree .get_recursive(index, self.depth, self.packing_depth) @@ -273,7 +272,7 @@ where } impl<'a, T: Value, N: Unsigned, U: UpdateMap> IntoIterator for &'a Vector { - type Item = StdCow<'a, T>; + type Item = &'a T; type IntoIter = InterfaceIter<'a, T, U>; fn into_iter(self) -> Self::IntoIter { @@ -316,7 +315,7 @@ impl Encode for Vector { let mut encoder = SszEncoder::container(buf, self.len() * ssz::BYTES_PER_LENGTH_OFFSET); for item in self.iter() { - encoder.append(item.as_ref()); + encoder.append(item); } encoder.finalize(); From 33ebf5c7e74112f48240b64377958675cdbef8bd Mon Sep 17 00:00:00 2001 From: Mac L Date: Fri, 14 Jul 2023 15:16:35 +1000 Subject: [PATCH 04/10] Clippy --- src/packed_leaf.rs | 2 +- src/tree.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/packed_leaf.rs b/src/packed_leaf.rs index 4aa916a..73d1af8 100644 --- a/src/packed_leaf.rs +++ b/src/packed_leaf.rs @@ -41,7 +41,7 @@ impl PackedLeaf { } let hash_base_ptr: *const Hash256 = &self.hash; let base_ptr: *const T = hash_base_ptr as *const T; - let elem_ptr: *const T = unsafe { base_ptr.offset(index as isize) }; + let elem_ptr: *const T = unsafe { base_ptr.add(index) }; Some(unsafe { &*elem_ptr }) } diff --git a/src/tree.rs b/src/tree.rs index 24f3faa..4b31a1b 100644 --- a/src/tree.rs +++ b/src/tree.rs @@ -258,7 +258,7 @@ impl Tree { (Self::PackedLeaf(l1), Self::PackedLeaf(l2)) if depth == 0 => { let mut equal = true; for i in 0..l2.length() { - let v2 = l2.get(i).unwrap(); // FIXME + let v2 = l2.get(i).expect("Is always in-bounds"); // FIXME match l1.get(i) { Some(v1) if v1 == v2 => continue, _ => { @@ -343,7 +343,7 @@ impl Tree { Self::PackedLeaf(packed_leaf) if depth == 0 => { diff.hashes.insert((depth, prefix), packed_leaf.hash); for i in 0..packed_leaf.length() { - let value = packed_leaf.get(i).unwrap(); + let value = packed_leaf.get(i).expect("Is always in-bounds"); // FIXME diff.leaves.insert(prefix | i, value.clone()); } Ok(()) From bfe252d7714b880e91744ec3789d02badc9457f3 Mon Sep 17 00:00:00 2001 From: Mac L Date: Mon, 7 Aug 2023 12:36:34 +1000 Subject: [PATCH 05/10] Cleanup --- src/packed_leaf.rs | 13 +++++-------- src/tree.rs | 4 ++-- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/src/packed_leaf.rs b/src/packed_leaf.rs index 73d1af8..720f434 100644 --- a/src/packed_leaf.rs +++ b/src/packed_leaf.rs @@ -98,13 +98,7 @@ impl PackedLeaf { Ok(updated) } - // FIXME: remove _hash/work out what's going on - pub fn update>( - &self, - prefix: usize, - _hash: Hash256, - updates: &U, - ) -> Result { + pub fn update>(&self, prefix: usize, updates: &U) -> Result { let packing_factor = T::tree_hash_packing_factor(); let start = prefix; let end = prefix + packing_factor; @@ -141,7 +135,10 @@ impl PackedLeaf { if index == self.length() { self.length += 1; } else if index > self.length() { - panic!("This is bad"); + return Err(Error::PackedLeafOutOfBounds { + sub_index, + len: self.length(), + }); } Ok(()) diff --git a/src/tree.rs b/src/tree.rs index 4b31a1b..ebf9939 100644 --- a/src/tree.rs +++ b/src/tree.rs @@ -175,7 +175,7 @@ impl Tree { Ok(Self::leaf_with_hash(value, hash)) } Self::PackedLeaf(packed_leaf) if depth == 0 => Ok(Arc::new(Self::PackedLeaf( - packed_leaf.update(prefix, hash, updates)?, + packed_leaf.update(prefix, updates)?, ))), Self::Node { left, right, .. } if depth > 0 => { let packing_depth = opt_packing_depth::().unwrap_or(0); @@ -216,7 +216,7 @@ impl Tree { Self::Zero(zero_depth) if *zero_depth == depth => { if depth == 0 { if opt_packing_factor::().is_some() { - let packed_leaf = PackedLeaf::empty().update(prefix, hash, updates)?; + let packed_leaf = PackedLeaf::empty().update(prefix, updates)?; Ok(Arc::new(Self::PackedLeaf(packed_leaf))) } else { let index = prefix; From b252dcf0f6f149e20aeaa8ad33c14a6c4549cb5b Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Mon, 7 Aug 2023 14:22:17 +1000 Subject: [PATCH 06/10] Fix alignment issues --- src/packed_leaf.rs | 25 ++++++++++++++++++------- src/tests/size_of.rs | 6 +++--- src/tree.rs | 7 ------- 3 files changed, 21 insertions(+), 17 deletions(-) diff --git a/src/packed_leaf.rs b/src/packed_leaf.rs index 720f434..71a884e 100644 --- a/src/packed_leaf.rs +++ b/src/packed_leaf.rs @@ -5,10 +5,21 @@ use derivative::Derivative; use std::ops::ControlFlow; use tree_hash::{Hash256, BYTES_PER_CHUNK}; +/// `Hash256` type which is aligned to a 16-byte boundary. +/// +/// This allows pointers to types with alignment 1, 2, 4, 8, 16 to be constructed pointing +/// *into* an `AlignedHash256`. +/// +/// In future this could be aligned to a 32-byte boundary, although that would blow out the size +/// of `PackedLeaf` and `Tree`. +#[derive(Clone, Copy, Debug, PartialEq, Hash, Arbitrary)] +#[repr(align(16))] +pub struct AlignedHash256(Hash256); + #[derive(Debug, Derivative, Arbitrary)] #[derivative(PartialEq, Hash)] pub struct PackedLeaf { - pub hash: Hash256, + pub hash: AlignedHash256, pub length: u8, _phantom: PhantomData, } @@ -39,19 +50,19 @@ impl PackedLeaf { if index >= self.length() { return None; } - let hash_base_ptr: *const Hash256 = &self.hash; + let hash_base_ptr: *const AlignedHash256 = &self.hash; let base_ptr: *const T = hash_base_ptr as *const T; let elem_ptr: *const T = unsafe { base_ptr.add(index) }; Some(unsafe { &*elem_ptr }) } pub fn tree_hash(&self) -> Hash256 { - self.hash + self.hash.0 } pub fn empty() -> Self { PackedLeaf { - hash: Hash256::zero(), + hash: AlignedHash256(Hash256::zero()), length: 0, _phantom: PhantomData, } @@ -65,7 +76,7 @@ impl PackedLeaf { hash_bytes[0..value_len].copy_from_slice(&value.as_ssz_bytes()); PackedLeaf { - hash, + hash: AlignedHash256(hash), length: 1, _phantom: PhantomData, } @@ -84,7 +95,7 @@ impl PackedLeaf { } PackedLeaf { - hash, + hash: AlignedHash256(hash), length: n as u8, _phantom: PhantomData, } @@ -126,7 +137,7 @@ impl PackedLeaf { let value_len = Self::value_len(); let mut hash = self.hash; - let hash_bytes = hash.as_bytes_mut(); + let hash_bytes = hash.0.as_bytes_mut(); hash_bytes[sub_index..sub_index + value_len].copy_from_slice(&value.as_ssz_bytes()); diff --git a/src/tests/size_of.rs b/src/tests/size_of.rs index 87fdc45..e93626c 100644 --- a/src/tests/size_of.rs +++ b/src/tests/size_of.rs @@ -8,7 +8,7 @@ use tree_hash::Hash256; fn size_of_hash256() { assert_eq!(size_of::>(), 64); assert_eq!(size_of::>(), 48); - assert_eq!(size_of::>(), 33); + assert_eq!(size_of::>(), 48); let rw_lock_size = size_of::>(); assert_eq!(rw_lock_size, 40); @@ -24,10 +24,10 @@ fn size_of_hash256() { fn size_of_u8() { assert_eq!(size_of::>(), 64); assert_eq!(size_of::>(), 48); - assert_eq!(size_of::>(), 33); + assert_eq!(size_of::>(), 48); assert_eq!( size_of::>(), - size_of::() + size_of::() + size_of::() + size_of::() ); let rw_lock_size = size_of::>(); diff --git a/src/tree.rs b/src/tree.rs index ebf9939..374d5a5 100644 --- a/src/tree.rs +++ b/src/tree.rs @@ -256,22 +256,16 @@ impl Tree { Ok(()) } (Self::PackedLeaf(l1), Self::PackedLeaf(l2)) if depth == 0 => { - let mut equal = true; for i in 0..l2.length() { let v2 = l2.get(i).expect("Is always in-bounds"); // FIXME match l1.get(i) { Some(v1) if v1 == v2 => continue, _ => { - equal = false; let index = prefix | i; diff.leaves.insert(index, v2.clone()); } } } - if !equal { - let hash = l2.hash; - diff.hashes.insert((depth, prefix), hash); - } Ok(()) } (Self::Zero(z1), Self::Zero(z2)) if z1 == z2 && *z1 == depth => Ok(()), @@ -341,7 +335,6 @@ impl Tree { Ok(()) } Self::PackedLeaf(packed_leaf) if depth == 0 => { - diff.hashes.insert((depth, prefix), packed_leaf.hash); for i in 0..packed_leaf.length() { let value = packed_leaf.get(i).expect("Is always in-bounds"); // FIXME diff.leaves.insert(prefix | i, value.clone()); From 74da78c958aa248ab1d1c4e0fa00c9eb0eeeae70 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Mon, 7 Aug 2023 14:24:41 +1000 Subject: [PATCH 07/10] Add test for aligned hash256 --- src/packed_leaf.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/packed_leaf.rs b/src/packed_leaf.rs index 71a884e..f072418 100644 --- a/src/packed_leaf.rs +++ b/src/packed_leaf.rs @@ -166,3 +166,13 @@ impl PackedLeaf { Ok(()) } } + +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn align_of_aligned_hash256() { + assert_eq!(std::mem::align_of::(), 16); + } +} From 8aa528d42ba53512e58ac97dbfed8ba1fa805126 Mon Sep 17 00:00:00 2001 From: Mac L Date: Mon, 7 Aug 2023 14:27:24 +1000 Subject: [PATCH 08/10] Remove expects --- src/tree.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/tree.rs b/src/tree.rs index 374d5a5..5dccd04 100644 --- a/src/tree.rs +++ b/src/tree.rs @@ -257,7 +257,10 @@ impl Tree { } (Self::PackedLeaf(l1), Self::PackedLeaf(l2)) if depth == 0 => { for i in 0..l2.length() { - let v2 = l2.get(i).expect("Is always in-bounds"); // FIXME + let v2 = l2.get(i).ok_or(Error::PackedLeafOutOfBounds { + sub_index: i, + len: l2.length(), + })?; match l1.get(i) { Some(v1) if v1 == v2 => continue, _ => { @@ -336,7 +339,10 @@ impl Tree { } Self::PackedLeaf(packed_leaf) if depth == 0 => { for i in 0..packed_leaf.length() { - let value = packed_leaf.get(i).expect("Is always in-bounds"); // FIXME + let value = packed_leaf.get(i).ok_or(Error::PackedLeafOutOfBounds { + sub_index: i, + len: packed_leaf.length(), + })?; diff.leaves.insert(prefix | i, value.clone()); } Ok(()) From 158c4690b31f5e30389adef92e033cff281936c4 Mon Sep 17 00:00:00 2001 From: Mac L Date: Mon, 7 Aug 2023 14:30:29 +1000 Subject: [PATCH 09/10] Avoid allocation in repeat --- src/packed_leaf.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/packed_leaf.rs b/src/packed_leaf.rs index f072418..a9f94b0 100644 --- a/src/packed_leaf.rs +++ b/src/packed_leaf.rs @@ -90,7 +90,7 @@ impl PackedLeaf { let value_len = Self::value_len(); - for (i, value) in vec![value; n].iter().enumerate() { + for (i, value) in std::iter::repeat(value).take(n).enumerate() { hash_bytes[i * value_len..(i + 1) * value_len].copy_from_slice(&value.as_ssz_bytes()); } From de30b36b4b343ff4f90dd84494965e52fd0350b2 Mon Sep 17 00:00:00 2001 From: Mac L Date: Wed, 30 Aug 2023 18:17:51 +1000 Subject: [PATCH 10/10] Fix conflicts --- src/iter.rs | 2 +- src/tree.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/iter.rs b/src/iter.rs index 0ff0430..99500bb 100644 --- a/src/iter.rs +++ b/src/iter.rs @@ -1,6 +1,6 @@ use crate::{ utils::{opt_packing_depth, opt_packing_factor, Length}, - Leaf, PackedLeaf, Tree, Value, + Leaf, Tree, Value, }; #[derive(Debug)] diff --git a/src/tree.rs b/src/tree.rs index 02d3056..6ead86b 100644 --- a/src/tree.rs +++ b/src/tree.rs @@ -393,7 +393,7 @@ impl Tree { } } (Self::PackedLeaf(l1), Self::PackedLeaf(l2)) => { - if l1.values == l2.values { + if l1 == l2 { Ok(RebaseAction::EqualReplace(base)) } else { Ok(RebaseAction::NotEqualNoop)