diff --git a/src/interface.rs b/src/interface.rs index ba3c5a2..919a7db 100644 --- a/src/interface.rs +++ b/src/interface.rs @@ -174,7 +174,7 @@ mod test { *c2.into_mut().unwrap() = 11; assert_eq!(*list.get(0).unwrap(), 11); - assert_eq!(list.iter().cloned().collect::>(), vec![11, 2, 3]); + assert_eq!(list.iter().copied().collect::>(), vec![11, 2, 3]); } #[test] diff --git a/src/iter.rs b/src/iter.rs index 0400fce..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)] @@ -59,10 +59,10 @@ impl<'a, T: Value> Iterator for Iter<'a, T> { result } - 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; diff --git a/src/list.rs b/src/list.rs index d331f7b..20c394e 100644 --- a/src/list.rs +++ b/src/list.rs @@ -324,7 +324,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() } diff --git a/src/packed_leaf.rs b/src/packed_leaf.rs index b0f3770..a9f94b0 100644 --- a/src/packed_leaf.rs +++ b/src/packed_leaf.rs @@ -1,132 +1,178 @@ -use crate::{utils::arb_rwlock, Error, UpdateMap}; +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, TreeHash, BYTES_PER_CHUNK}; +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 { - #[derivative(PartialEq = "ignore", Hash = "ignore")] - #[arbitrary(with = arb_rwlock)] - pub hash: RwLock, - pub(crate) values: Vec, +pub struct PackedLeaf { + pub hash: AlignedHash256, + 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(), + hash: self.hash, + 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); - - if !hash.is_zero() { - return hash; - } +impl PackedLeaf { + pub fn length(&self) -> usize { + self.length as usize + } - let hash_bytes = hash.as_bytes_mut(); + fn value_len() -> usize { + BYTES_PER_CHUNK / T::tree_hash_packing_factor() + } - 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<&T> { + if index >= self.length() { + return None; } + 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 }) + } - *self.hash.write() = hash; - hash + pub fn tree_hash(&self) -> Hash256 { + self.hash.0 } pub fn empty() -> Self { PackedLeaf { - hash: RwLock::new(Hash256::zero()), - values: Vec::with_capacity(T::tree_hash_packing_factor()), + hash: AlignedHash256(Hash256::zero()), + 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(); + hash_bytes[0..value_len].copy_from_slice(&value.as_ssz_bytes()); PackedLeaf { - hash: RwLock::new(Hash256::zero()), - values, + hash: AlignedHash256(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(); + + 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()); + } + PackedLeaf { - hash: RwLock::new(Hash256::zero()), - values: vec![value; n], + hash: AlignedHash256(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, - updates: &U, - ) -> Result { - let mut updated = PackedLeaf { - hash: RwLock::new(hash), - values: self.values.clone(), - }; - + 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; + + 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(); - 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(); + + let mut hash = self.hash; + let hash_bytes = hash.0.as_bytes_mut(); + + hash_bytes[sub_index..sub_index + value_len].copy_from_slice(&value.as_ssz_bytes()); + + self.hash = hash; + + if index == self.length() { + self.length += 1; + } else if index > self.length() { + return Err(Error::PackedLeafOutOfBounds { + sub_index, + len: self.length(), + }); + } + 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)?; + Ok(()) } } + +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn align_of_aligned_hash256() { + assert_eq!(std::mem::align_of::(), 16); + } +} diff --git a/src/tests/packed.rs b/src/tests/packed.rs index 5351b38..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().copied().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), vec.get(i)); + assert_eq!(list.get(i).cloned().as_ref(), vec.get(i)); } } } @@ -36,11 +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().copied().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), vec.get(i)); + assert_eq!(vector.get(i).cloned().as_ref(), vec.get(i)); } } @@ -74,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), Some(&v)); + assert_eq!(list.get(i).cloned(), Some(v)); list.apply_updates().unwrap(); - assert_eq!(list.get(i), 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 5e5c103..93293ff 100644 --- a/src/tests/proptest/operations.rs +++ b/src/tests/proptest/operations.rs @@ -17,7 +17,7 @@ pub struct Spec { _phantom: PhantomData, } -impl Spec { +impl Spec { pub fn list(values: Vec) -> Self { assert!(values.len() <= N::to_usize()); Self { diff --git a/src/tests/size_of.rs b/src/tests/size_of.rs index 00e0b9d..e93626c 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); @@ -16,21 +16,18 @@ fn size_of_hash256() { let arc_size = size_of::>>(); assert_eq!(arc_size, 8); - assert_eq!( - size_of::>(), - size_of::>() + 8 - ); + assert_eq!(size_of::>(), 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 +36,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 d816e6e..6ead86b 100644 --- a/src/tree.rs +++ b/src/tree.rs @@ -86,8 +86,8 @@ impl Tree { pub fn get_recursive(&self, index: usize, depth: usize, packing_depth: usize) -> Option<&T> { 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::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; @@ -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; @@ -267,22 +267,19 @@ impl Tree { Ok(()) } (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).ok_or(Error::PackedLeafOutOfBounds { + sub_index: i, + len: l2.length(), + })?; + 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.read(); - diff.hashes.insert((depth, prefix), hash); - } Ok(()) } (Self::Zero(z1), Self::Zero(z2)) if z1 == z2 && *z1 == depth => Ok(()), @@ -352,9 +349,11 @@ 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() { + for i in 0..packed_leaf.length() { + 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(()) @@ -394,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) diff --git a/src/update_map.rs b/src/update_map.rs index 27945a8..a9984d5 100644 --- a/src/update_map.rs +++ b/src/update_map.rs @@ -7,7 +7,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 { +pub trait UpdateMap: Default + Clone { fn get(&self, k: usize) -> Option<&T>; fn get_mut_with(&mut self, k: usize, f: F) -> Option<&mut T> @@ -177,7 +177,7 @@ pub struct MaxMap { max_key: usize, } -impl UpdateMap for MaxMap +impl UpdateMap for MaxMap where M: UpdateMap, { 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())) })