Compute packed leaf values on demand (Option 1)

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<_>>(), vec![11, 2, 3]);
+        assert_eq!(list.iter().copied().collect::<Vec<_>>(), vec![11, 2, 3]);
     }
 
     #[test]

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;
 

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()
     }

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<T: TreeHash + Clone> {
-    #[derivative(PartialEq = "ignore", Hash = "ignore")]
-    #[arbitrary(with = arb_rwlock)]
-    pub hash: RwLock<Hash256>,
-    pub(crate) values: Vec<T>,
+pub struct PackedLeaf<T: Value> {
+    pub hash: AlignedHash256,
+    pub length: u8,
+    _phantom: PhantomData<T>,
 }
 
 impl<T> Clone for PackedLeaf<T>
 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<T: TreeHash + Clone> PackedLeaf<T> {
-    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<T: Value> PackedLeaf<T> {
+    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<Self, Error> {
-        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<U: UpdateMap<T>>(
-        &self,
-        prefix: usize,
-        hash: Hash256,
-        updates: &U,
-    ) -> Result<Self, Error> {
-        let mut updated = PackedLeaf {
-            hash: RwLock::new(hash),
-            values: self.values.clone(),
-        };
-
+    pub fn update<U: UpdateMap<T>>(&self, prefix: usize, updates: &U) -> Result<Self, Error> {
         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::<AlignedHash256>(), 16);
+    }
+}

src/tests/packed.rs

@@ -9,11 +9,11 @@ fn u64_packed_list_build_and_iter() {
         let vec = (0..len).map(|i| 2 * i).collect::<Vec<u64>>();
         let list = List::<u64, U16>::new(vec.clone()).unwrap();
 
-        let from_iter = list.iter().copied().collect::<Vec<_>>();
+        let from_iter = list.iter().cloned().collect::<Vec<_>>();
         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::<Vec<u64>>();
     let vector = Vector::<u64, U16>::new(vec.clone()).unwrap();
 
-    let from_iter = vector.iter().copied().collect::<Vec<_>>();
+    let from_iter = vector.iter().cloned().collect::<Vec<_>>();
     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);

src/tests/proptest/operations.rs

@@ -17,7 +17,7 @@ pub struct Spec<T, N: Unsigned> {
     _phantom: PhantomData<N>,
 }
 
-impl<T, N: Unsigned> Spec<T, N> {
+impl<T: Clone, N: Unsigned> Spec<T, N> {
     pub fn list(values: Vec<T>) -> Self {
         assert!(values.len() <= N::to_usize());
         Self {

src/tests/size_of.rs

@@ -6,31 +6,28 @@ use tree_hash::Hash256;
 /// It's important that the Tree nodes have a predictable size.
 #[test]
 fn size_of_hash256() {
-    assert_eq!(size_of::<Tree<Hash256>>(), 72);
+    assert_eq!(size_of::<Tree<Hash256>>(), 64);
     assert_eq!(size_of::<Leaf<Hash256>>(), 48);
-    assert_eq!(size_of::<PackedLeaf<Hash256>>(), 64);
+    assert_eq!(size_of::<PackedLeaf<Hash256>>(), 48);
 
     let rw_lock_size = size_of::<RwLock<Hash256>>();
     assert_eq!(rw_lock_size, 40);
 
     let arc_size = size_of::<Arc<Tree<Hash256>>>();
     assert_eq!(arc_size, 8);
 
-    assert_eq!(
-        size_of::<Tree<Hash256>>(),
-        size_of::<PackedLeaf<Hash256>>() + 8
-    );
+    assert_eq!(size_of::<Tree<Hash256>>(), size_of::<Leaf<Hash256>>() + 16);
 }
 
 /// It's important that the Tree nodes have a predictable size.
 #[test]
 fn size_of_u8() {
-    assert_eq!(size_of::<Tree<u8>>(), 72);
+    assert_eq!(size_of::<Tree<u8>>(), 64);
     assert_eq!(size_of::<Leaf<u8>>(), 48);
-    assert_eq!(size_of::<PackedLeaf<u8>>(), 64);
+    assert_eq!(size_of::<PackedLeaf<u8>>(), 48);
     assert_eq!(
         size_of::<PackedLeaf<u8>>(),
-        size_of::<RwLock<Hash256>>() + size_of::<Vec<u8>>()
+        size_of::<Hash256>() + size_of::<u128>()
     );
 
     let rw_lock_size = size_of::<RwLock<u8>>();
@@ -39,5 +36,5 @@ fn size_of_u8() {
     let arc_size = size_of::<Arc<Tree<u8>>>();
     assert_eq!(arc_size, 8);
 
-    assert_eq!(size_of::<Tree<u8>>(), size_of::<PackedLeaf<u8>>() + 8);
+    assert_eq!(size_of::<Tree<u8>>(), size_of::<Leaf<u8>>() + 16);
 }