diff --git a/Cargo.toml b/Cargo.toml index d6247a3..dd2275f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -21,7 +21,6 @@ ethereum_hashing = "0.8" ethereum_ssz = "0.10" ethereum_ssz_derive = "0.10" itertools = "0.13.0" -parking_lot = "0.12.1" rayon = "1.5.1" serde = { version = "1.0.0", features = ["derive"] } tree_hash = "0.12" diff --git a/src/leaf.rs b/src/leaf.rs index b1daec7..b1a99a6 100644 --- a/src/leaf.rs +++ b/src/leaf.rs @@ -1,6 +1,6 @@ use crate::Arc; use educe::Educe; -use parking_lot::RwLock; +use std::sync::OnceLock; use tree_hash::Hash256; #[derive(Debug, Educe)] @@ -8,8 +8,8 @@ use tree_hash::Hash256; #[educe(PartialEq, Hash)] pub struct Leaf { #[educe(PartialEq(ignore), Hash(ignore))] - #[cfg_attr(feature = "arbitrary", arbitrary(with = crate::utils::arb_rwlock))] - pub hash: RwLock, + #[cfg_attr(feature = "arbitrary", arbitrary(with = crate::utils::arb_oncelock))] + pub hash: OnceLock, #[cfg_attr(feature = "arbitrary", arbitrary(with = crate::utils::arb_arc))] pub value: Arc, } @@ -20,7 +20,10 @@ where { fn clone(&self) -> Self { Self { - hash: RwLock::new(*self.hash.read()), + hash: match self.hash.get() { + Some(&h) => OnceLock::from(h), + None => OnceLock::new(), + }, value: self.value.clone(), } } @@ -28,12 +31,15 @@ where impl Leaf { pub fn new(value: T) -> Self { - Self::with_hash(value, Hash256::ZERO) + Self::with_hash(value, None) } - pub fn with_hash(value: T, hash: Hash256) -> Self { + pub fn with_hash(value: T, hash: Option) -> Self { Self { - hash: RwLock::new(hash), + hash: match hash { + Some(h) => OnceLock::from(h), + None => OnceLock::new(), + }, value: Arc::new(value), } } diff --git a/src/packed_leaf.rs b/src/packed_leaf.rs index ad34d46..abb9ba7 100644 --- a/src/packed_leaf.rs +++ b/src/packed_leaf.rs @@ -1,7 +1,7 @@ use crate::{Error, UpdateMap}; use educe::Educe; -use parking_lot::RwLock; use std::ops::ControlFlow; +use std::sync::OnceLock; use tree_hash::{BYTES_PER_CHUNK, Hash256, TreeHash}; #[derive(Debug, Educe)] @@ -9,8 +9,8 @@ use tree_hash::{BYTES_PER_CHUNK, Hash256, TreeHash}; #[educe(PartialEq, Hash)] pub struct PackedLeaf { #[educe(PartialEq(ignore), Hash(ignore))] - #[cfg_attr(feature = "arbitrary", arbitrary(with = crate::utils::arb_rwlock))] - pub hash: RwLock, + #[cfg_attr(feature = "arbitrary", arbitrary(with = crate::utils::arb_oncelock))] + pub hash: OnceLock, pub values: Vec, } @@ -20,7 +20,10 @@ where { fn clone(&self) -> Self { Self { - hash: RwLock::new(*self.hash.read()), + hash: match self.hash.get() { + Some(&h) => OnceLock::from(h), + None => OnceLock::new(), + }, values: self.values.clone(), } } @@ -28,29 +31,23 @@ where 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; + if let Some(&cached) = self.hash.get() { + return cached; } - + let mut hash = Hash256::ZERO; let hash_bytes = hash.as_mut_slice(); - 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()); } - - *self.hash.write() = hash; + let _ = self.hash.set(hash); hash } pub fn empty() -> Self { PackedLeaf { - hash: RwLock::new(Hash256::ZERO), + hash: OnceLock::new(), values: Vec::with_capacity(T::tree_hash_packing_factor()), } } @@ -60,7 +57,7 @@ impl PackedLeaf { values.push(value); PackedLeaf { - hash: RwLock::new(Hash256::ZERO), + hash: OnceLock::new(), values, } } @@ -68,14 +65,14 @@ impl PackedLeaf { pub fn repeat(value: T, n: usize) -> Self { assert!(n <= T::tree_hash_packing_factor()); PackedLeaf { - hash: RwLock::new(Hash256::ZERO), + hash: OnceLock::new(), values: vec![value; n], } } pub fn insert_at_index(&self, index: usize, value: T) -> Result { let mut updated = PackedLeaf { - hash: RwLock::new(Hash256::ZERO), + hash: OnceLock::new(), values: self.values.clone(), }; let sub_index = index % T::tree_hash_packing_factor(); @@ -86,11 +83,14 @@ impl PackedLeaf { pub fn update>( &self, prefix: usize, - hash: Hash256, + hash: Option, updates: &U, ) -> Result { let mut updated = PackedLeaf { - hash: RwLock::new(hash), + hash: match hash { + Some(h) => OnceLock::from(h), + None => OnceLock::new(), + }, values: self.values.clone(), }; @@ -104,8 +104,8 @@ impl PackedLeaf { } pub fn insert_mut(&mut self, sub_index: usize, value: T) -> Result<(), Error> { - // Ensure hash is 0. - *self.hash.get_mut() = Hash256::ZERO; + // Ensure hash is cleared. + self.hash = OnceLock::new(); if sub_index == self.values.len() { self.values.push(value); diff --git a/src/repeat.rs b/src/repeat.rs index ebaaa8f..a956e4f 100644 --- a/src/repeat.rs +++ b/src/repeat.rs @@ -1,7 +1,6 @@ use crate::utils::{Length, opt_packing_factor}; use crate::{Arc, Error, Leaf, List, PackedLeaf, Tree, UpdateMap, Value}; use smallvec::{SmallVec, smallvec}; -use tree_hash::Hash256; use typenum::Unsigned; /// Efficiently construct a list from `n` copies of `elem`. @@ -44,32 +43,26 @@ where for depth in 0..tree_depth { let new_layer = match &layer[..] { [(repeat_leaf, 1)] => { - smallvec![( - Tree::node(repeat_leaf.clone(), Tree::zero(depth), Hash256::ZERO), - 1, - )] + smallvec![(Tree::node(repeat_leaf.clone(), Tree::zero(depth), None), 1,)] } [(repeat_leaf, repeat_count)] if repeat_count.is_multiple_of(2) => { smallvec![( - Tree::node(repeat_leaf.clone(), repeat_leaf.clone(), Hash256::ZERO), + Tree::node(repeat_leaf.clone(), repeat_leaf.clone(), None), repeat_count / 2, )] } [(repeat_leaf, repeat_count)] => { smallvec![ ( - Tree::node(repeat_leaf.clone(), repeat_leaf.clone(), Hash256::ZERO), + Tree::node(repeat_leaf.clone(), repeat_leaf.clone(), None), repeat_count / 2, ), - ( - Tree::node(repeat_leaf.clone(), Tree::zero(depth), Hash256::ZERO), - 1, - ), + (Tree::node(repeat_leaf.clone(), Tree::zero(depth), None), 1,), ] } [(repeat_leaf, 1), (lonely_leaf, 1)] => { smallvec![( - Tree::node(repeat_leaf.clone(), lonely_leaf.clone(), Hash256::ZERO), + Tree::node(repeat_leaf.clone(), lonely_leaf.clone(), None), 1, )] } @@ -77,22 +70,19 @@ where if repeat_count.is_multiple_of(2) { smallvec![ ( - Tree::node(repeat_leaf.clone(), repeat_leaf.clone(), Hash256::ZERO), + Tree::node(repeat_leaf.clone(), repeat_leaf.clone(), None), repeat_count / 2, ), - ( - Tree::node(lonely_leaf.clone(), Tree::zero(depth), Hash256::ZERO), - 1, - ), + (Tree::node(lonely_leaf.clone(), Tree::zero(depth), None), 1,), ] } else { smallvec![ ( - Tree::node(repeat_leaf.clone(), repeat_leaf.clone(), Hash256::ZERO), + Tree::node(repeat_leaf.clone(), repeat_leaf.clone(), None), repeat_count / 2, ), ( - Tree::node(repeat_leaf.clone(), lonely_leaf.clone(), Hash256::ZERO), + Tree::node(repeat_leaf.clone(), lonely_leaf.clone(), None), 1, ), ] diff --git a/src/tests/size_of.rs b/src/tests/size_of.rs index 31b53f7..f28b0df 100644 --- a/src/tests/size_of.rs +++ b/src/tests/size_of.rs @@ -1,6 +1,6 @@ use crate::{Arc, Leaf, PackedLeaf, Tree}; -use parking_lot::RwLock; use std::mem::size_of; +use std::sync::OnceLock; use tree_hash::Hash256; /// It's important that the Tree nodes have a predictable size. @@ -10,8 +10,8 @@ fn size_of_hash256() { assert_eq!(size_of::>(), 48); assert_eq!(size_of::>(), 64); - let rw_lock_size = size_of::>(); - assert_eq!(rw_lock_size, 40); + let once_lock_size = size_of::>(); + assert_eq!(once_lock_size, 36); let arc_size = size_of::>>(); assert_eq!(arc_size, 8); @@ -27,11 +27,11 @@ fn size_of_u8() { assert_eq!(size_of::>(), 64); assert_eq!( size_of::>(), - size_of::>() + size_of::>() + size_of::>() + size_of::>() + 4 // 4 bytes alignment padding ); - let rw_lock_size = size_of::>(); - assert_eq!(rw_lock_size, 16); + let once_lock_size = size_of::>(); + assert_eq!(once_lock_size, 8); let arc_size = size_of::>>(); assert_eq!(arc_size, 8); diff --git a/src/tree.rs b/src/tree.rs index b40eae9..786641e 100644 --- a/src/tree.rs +++ b/src/tree.rs @@ -2,10 +2,10 @@ use crate::utils::{Length, opt_hash, opt_packing_depth, opt_packing_factor}; use crate::{Arc, Error, Leaf, PackedLeaf, UpdateMap, Value}; use educe::Educe; use ethereum_hashing::{ZERO_HASHES, hash32_concat}; -use parking_lot::RwLock; use std::collections::BTreeMap; use std::collections::HashMap; use std::ops::ControlFlow; +use std::sync::OnceLock; use tree_hash::Hash256; #[derive(Debug, Educe)] @@ -16,8 +16,8 @@ pub enum Tree { PackedLeaf(PackedLeaf), Node { #[educe(PartialEq(ignore), Hash(ignore))] - #[cfg_attr(feature = "arbitrary", arbitrary(with = crate::utils::arb_rwlock))] - hash: RwLock, + #[cfg_attr(feature = "arbitrary", arbitrary(with = crate::utils::arb_oncelock))] + hash: OnceLock, #[cfg_attr(feature = "arbitrary", arbitrary(with = crate::utils::arb_arc))] left: Arc, #[cfg_attr(feature = "arbitrary", arbitrary(with = crate::utils::arb_arc))] @@ -30,7 +30,10 @@ impl Clone for Tree { fn clone(&self) -> Self { match self { Self::Node { hash, left, right } => Self::Node { - hash: RwLock::new(*hash.read()), + hash: match hash.get() { + Some(&h) => OnceLock::from(h), + None => OnceLock::new(), + }, left: left.clone(), right: right.clone(), }, @@ -46,9 +49,12 @@ impl Tree { Self::zero(depth) } - pub fn node(left: Arc, right: Arc, hash: Hash256) -> Arc { + pub fn node(left: Arc, right: Arc, hash: Option) -> Arc { Arc::new(Self::Node { - hash: RwLock::new(hash), + hash: match hash { + Some(h) => OnceLock::from(h), + None => OnceLock::new(), + }, left, right, }) @@ -62,13 +68,13 @@ impl Tree { Arc::new(Self::Leaf(Leaf::new(value))) } - pub fn leaf_with_hash(value: T, hash: Hash256) -> Arc { + pub fn leaf_with_hash(value: T, hash: Option) -> Arc { Arc::new(Self::Leaf(Leaf::with_hash(value, hash))) } pub fn node_unboxed(left: Arc, right: Arc) -> Self { Self::Node { - hash: RwLock::new(Hash256::ZERO), + hash: OnceLock::new(), left, right, } @@ -125,14 +131,14 @@ impl Tree { Ok(Self::node( left.with_updated_leaf(index, new_value, new_depth)?, right.clone(), - Hash256::ZERO, + None, )) } else { // Index lies on the right, recurse right Ok(Self::node( left.clone(), right.with_updated_leaf(index, new_value, new_depth)?, - Hash256::ZERO, + None, )) } } @@ -147,7 +153,7 @@ impl Tree { // Split zero node into a node with left and right, and recurse into // the appropriate subtree let new_zero = Self::zero(depth - 1); - Self::node(new_zero.clone(), new_zero, Hash256::ZERO) + Self::node(new_zero.clone(), new_zero, None) .with_updated_leaf(index, new_value, depth) } } @@ -162,7 +168,7 @@ impl Tree { depth: usize, hashes: Option<&BTreeMap<(usize, usize), Hash256>>, ) -> Result, Error> { - let hash = opt_hash(hashes, depth, prefix).unwrap_or_default(); + let hash = opt_hash(hashes, depth, prefix); match self { Self::Leaf(_) if depth == 0 => { @@ -306,14 +312,14 @@ impl Tree { ) if full_depth > 0 => { use RebaseAction::*; - let orig_hash = *orig_hash_lock.read(); - let base_hash = *base_hash_lock.read(); + let orig_hash = orig_hash_lock.get().copied(); + let base_hash = base_hash_lock.get().copied(); // If hashes *and* lengths are equal then we can short-cut the recursion // and immediately replace `orig` by the `base` node. If `lengths` are `None` // then we know they are already equal (e.g. we're in a vector). - if !orig_hash.is_zero() - && orig_hash == base_hash + if let (Some(oh), Some(bh)) = (orig_hash, base_hash) + && oh == bh && lengths.is_none_or(|(orig_length, base_length)| orig_length == base_length) { return Ok(EqualReplace(base)); @@ -345,55 +351,29 @@ impl Tree { Ok(NotEqualNoop) } (EqualNoop, EqualNoop) => Ok(EqualNoop), - (NotEqualNoop | EqualNoop, NotEqualReplace(new_right)) => { - Ok(NotEqualReplace(Arc::new(Self::Node { - hash: RwLock::new(orig_hash), - left: l1.clone(), - right: new_right, - }))) - } - (NotEqualNoop | EqualNoop, EqualReplace(new_right)) => { - Ok(NotEqualReplace(Arc::new(Self::Node { - hash: RwLock::new(orig_hash), - left: l1.clone(), - right: new_right.clone(), - }))) - } + (NotEqualNoop | EqualNoop, NotEqualReplace(new_right)) => Ok(NotEqualReplace( + Self::node(l1.clone(), new_right, orig_hash), + )), + (NotEqualNoop | EqualNoop, EqualReplace(new_right)) => Ok(NotEqualReplace( + Self::node(l1.clone(), new_right.clone(), orig_hash), + )), (NotEqualReplace(new_left), NotEqualNoop | EqualNoop) => { - Ok(NotEqualReplace(Arc::new(Self::Node { - hash: RwLock::new(orig_hash), - left: new_left, - right: r1.clone(), - }))) + Ok(NotEqualReplace(Self::node(new_left, r1.clone(), orig_hash))) } (NotEqualReplace(new_left), NotEqualReplace(new_right)) => { - Ok(NotEqualReplace(Arc::new(Self::Node { - hash: RwLock::new(orig_hash), - left: new_left, - right: new_right, - }))) - } - (NotEqualReplace(new_left), EqualReplace(new_right)) => { - Ok(NotEqualReplace(Arc::new(Self::Node { - hash: RwLock::new(orig_hash), - left: new_left, - right: new_right.clone(), - }))) - } - (EqualReplace(new_left), NotEqualNoop) => { - Ok(NotEqualReplace(Arc::new(Self::Node { - hash: RwLock::new(orig_hash), - left: new_left.clone(), - right: r1.clone(), - }))) - } - (EqualReplace(new_left), NotEqualReplace(new_right)) => { - Ok(NotEqualReplace(Arc::new(Self::Node { - hash: RwLock::new(orig_hash), - left: new_left.clone(), - right: new_right, - }))) + Ok(NotEqualReplace(Self::node(new_left, new_right, orig_hash))) } + (NotEqualReplace(new_left), EqualReplace(new_right)) => Ok(NotEqualReplace( + Self::node(new_left, new_right.clone(), orig_hash), + )), + (EqualReplace(new_left), NotEqualNoop) => Ok(NotEqualReplace(Self::node( + new_left.clone(), + r1.clone(), + orig_hash, + ))), + (EqualReplace(new_left), NotEqualReplace(new_right)) => Ok(NotEqualReplace( + Self::node(new_left.clone(), new_right, orig_hash), + )), (EqualReplace(_), EqualReplace(_)) | (EqualReplace(_), EqualNoop) => { Ok(EqualReplace(base)) } @@ -438,12 +418,8 @@ impl Tree { match &**orig { Self::Leaf(_) | Self::PackedLeaf(_) | Self::Zero(_) => Ok(IntraRebaseAction::Noop), Self::Node { hash, left, right } if current_depth > 0 => { - let hash = *hash.read(); - // Tree must be fully hashed prior to intra-rebase. - if hash.is_zero() { - return Err(Error::IntraRebaseZeroHash); - } + let hash = hash.get().copied().ok_or(Error::IntraRebaseZeroHash)?; if let Some(known_subtree) = known_subtrees.get(&(current_depth, hash)) { // Node is already known from elsewhere in the tree. We can replace it without @@ -457,15 +433,15 @@ impl Tree { let action = match (left_action, right_action) { (IntraRebaseAction::Noop, IntraRebaseAction::Noop) => IntraRebaseAction::Noop, (IntraRebaseAction::Noop, IntraRebaseAction::Replace(new_right)) => { - IntraRebaseAction::Replace(Self::node(left.clone(), new_right, hash)) + IntraRebaseAction::Replace(Self::node(left.clone(), new_right, Some(hash))) } (IntraRebaseAction::Replace(new_left), IntraRebaseAction::Noop) => { - IntraRebaseAction::Replace(Self::node(new_left, right.clone(), hash)) + IntraRebaseAction::Replace(Self::node(new_left, right.clone(), Some(hash))) } ( IntraRebaseAction::Replace(new_left), IntraRebaseAction::Replace(new_right), - ) => IntraRebaseAction::Replace(Self::node(new_left, new_right, hash)), + ) => IntraRebaseAction::Replace(Self::node(new_left, new_right, Some(hash))), }; // Add the new version of this node to the known subtrees. @@ -495,44 +471,26 @@ impl Tree { pub fn tree_hash(&self) -> Hash256 { match self { Self::Leaf(Leaf { hash, value }) => { - // FIXME(sproul): upgradeable RwLock? - let read_lock = hash.read(); - let existing_hash = *read_lock; - drop(read_lock); - - // NOTE: We re-compute the hash whenever it is non-zero. Computed hashes may - // legitimately be zero, but this only occurs at the leaf level when the value is - // entirely zeroes (e.g. [0u64, 0, 0, 0]). In order to avoid storing an - // `Option` we choose to re-compute the hash in this case. In practice - // this is unlikely to provide any performance penalty except at very small list - // lengths (<= 32), because a node higher in the tree will cache a non-zero hash - // preventing its children from being visited more than once. - if !existing_hash.is_zero() { - existing_hash - } else { - let tree_hash = value.tree_hash_root(); - *hash.write() = tree_hash; - tree_hash + if let Some(&cached) = hash.get() { + return cached; } + let computed = value.tree_hash_root(); + let _ = hash.set(computed); + computed } Self::PackedLeaf(leaf) => leaf.tree_hash(), Self::Zero(depth) => Hash256::from(ZERO_HASHES[*depth]), Self::Node { hash, left, right } => { - let read_lock = hash.read(); - let existing_hash = *read_lock; - drop(read_lock); - - if !existing_hash.is_zero() { - existing_hash - } else { - // Parallelism goes brrrr. - let (left_hash, right_hash) = - rayon::join(|| left.tree_hash(), || right.tree_hash()); - let tree_hash = - Hash256::from(hash32_concat(left_hash.as_slice(), right_hash.as_slice())); - *hash.write() = tree_hash; - tree_hash + if let Some(&cached) = hash.get() { + return cached; } + // Parallelism goes brrrr. + let (left_hash, right_hash) = + rayon::join(|| left.tree_hash(), || right.tree_hash()); + let computed = + Hash256::from(hash32_concat(left_hash.as_slice(), right_hash.as_slice())); + let _ = hash.set(computed); + computed } } } diff --git a/src/utils.rs b/src/utils.rs index 5bcb7ac..f420364 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -106,10 +106,10 @@ pub fn arb_arc<'a, T: Arbitrary<'a>>( } #[cfg(feature = "arbitrary")] -pub fn arb_rwlock<'a, T: Arbitrary<'a>>( +pub fn arb_oncelock<'a, T: Arbitrary<'a>>( u: &mut arbitrary::Unstructured<'a>, -) -> arbitrary::Result> { - T::arbitrary(u).map(parking_lot::RwLock::new) +) -> arbitrary::Result> { + T::arbitrary(u).map(std::sync::OnceLock::from) } #[cfg(test)]