Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

DiskAddress is already a more efficient Option<> #341

Closed
wants to merge 3 commits into from
Closed

DiskAddress is already a more efficient Option<> #341

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8ede0c1
DiskAddress is already a more efficient Option<>
rkuris Nov 3, 2023
4df2859
Use NBRANCH when possible
rkuris Nov 3, 2023
c433572
Bugfix: Flatten would remove the null disk addresses
rkuris Nov 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 34 additions & 31 deletions firewood/src/merkle.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ impl<S: ShaleStore<Node> + Send + Sync> Merkle<S> {
self.store
.put_item(
Node::branch(BranchNode {
children: [None; NBRANCH],
children: Default::default(),
value: None,
children_encoded: Default::default(),
}),
Expand Down Expand Up @@ -117,7 +117,7 @@ impl<S: ShaleStore<Node> + Send + Sync> Merkle<S> {
.as_branch()
.ok_or(MerkleError::NotBranchNode)?
.children[0];
Ok(if let Some(root) = root {
Ok(if !root.is_null() {
let mut node = self.get_node(root)?;
let res = node.get_root_hash::<S>(self.store.as_ref()).clone();
if node.lazy_dirty.load(Relaxed) {
Expand All @@ -143,8 +143,10 @@ impl<S: ShaleStore<Node> + Send + Sync> Merkle<S> {
match &u_ref.inner {
NodeType::Branch(n) => {
writeln!(w, "{n:?}")?;
for c in n.children.iter().flatten() {
self.dump_(*c, w)?
for c in n.children {
if !c.is_null() {
self.dump_(c, w)?
}
}
}
NodeType::Leaf(n) => writeln!(w, "{n:?}").unwrap(),
Expand Down Expand Up @@ -199,10 +201,10 @@ impl<S: ShaleStore<Node> + Send + Sync> Merkle<S> {
let new_node = Node::leaf(PartialPath(new_node_path.to_vec()), Data(val));
let leaf_address = self.put_node(new_node)?.as_ptr();

let mut chd = [None; NBRANCH];
let mut chd: [DiskAddress; NBRANCH] = Default::default();

let last_matching_nibble = matching_path[idx];
chd[last_matching_nibble as usize] = Some(leaf_address);
chd[last_matching_nibble as usize] = leaf_address;

let address = match &node_to_split.inner {
NodeType::Extension(u) if u.path.len() == 0 => {
Expand All @@ -212,7 +214,7 @@ impl<S: ShaleStore<Node> + Send + Sync> Merkle<S> {
_ => node_to_split_address,
};

chd[n_path[idx] as usize] = Some(address);
chd[n_path[idx] as usize] = address;

let new_branch = Node::branch(BranchNode {
children: chd,
Expand Down Expand Up @@ -338,9 +340,9 @@ impl<S: ShaleStore<Node> + Send + Sync> Merkle<S> {
};

// [parent] (-> [ExtNode]) -> [branch with v] -> [Leaf]
let mut children = [None; NBRANCH];
let mut children: [DiskAddress; NBRANCH] = Default::default();

children[idx] = leaf_address.into();
children[idx] = leaf_address;

let branch_address = self
.put_node(Node::branch(BranchNode {
Expand Down Expand Up @@ -426,8 +428,8 @@ impl<S: ShaleStore<Node> + Send + Sync> Merkle<S> {
// to another node, we walk down that. Otherwise, we can store our
// value as a leaf and we're done
NodeType::Branch(n) => match n.children[current_nibble as usize] {
Some(c) => (node, c),
None => {
c if !c.is_null() => (node, c),
_ => {
// insert the leaf to the empty slot
// create a new leaf
let leaf_ptr = self
Expand All @@ -436,7 +438,7 @@ impl<S: ShaleStore<Node> + Send + Sync> Merkle<S> {
// set the current child to point to this leaf
node.write(|u| {
let uu = u.inner.as_branch_mut().unwrap();
uu.children[current_nibble as usize] = Some(leaf_ptr);
uu.children[current_nibble as usize] = leaf_ptr;
u.rehash();
})
.unwrap();
Expand Down Expand Up @@ -554,7 +556,7 @@ impl<S: ShaleStore<Node> + Send + Sync> Merkle<S> {
};

if let Some((idx, more, ext, val)) = info {
let mut chd = [None; NBRANCH];
let mut chd: [DiskAddress; NBRANCH] = Default::default();

let c_ptr = if more {
u_ptr
Expand All @@ -563,7 +565,7 @@ impl<S: ShaleStore<Node> + Send + Sync> Merkle<S> {
ext.unwrap()
};

chd[idx as usize] = Some(c_ptr);
chd[idx as usize] = c_ptr;

let branch = self
.put_node(Node::branch(BranchNode {
Expand All @@ -590,7 +592,7 @@ impl<S: ShaleStore<Node> + Send + Sync> Merkle<S> {
// the immediate parent of a leaf must be a branch
b_ref
.write(|b| {
b.inner.as_branch_mut().unwrap().children[b_idx as usize] = None;
b.inner.as_branch_mut().unwrap().children[b_idx as usize] = DiskAddress::null();
b.rehash()
})
.unwrap();
Expand All @@ -614,7 +616,7 @@ impl<S: ShaleStore<Node> + Send + Sync> Merkle<S> {
.as_ptr();
p_ref
.write(|p| {
p.inner.as_branch_mut().unwrap().children[p_idx as usize] = Some(leaf);
p.inner.as_branch_mut().unwrap().children[p_idx as usize] = leaf;
p.rehash()
})
.unwrap();
Expand Down Expand Up @@ -705,7 +707,7 @@ impl<S: ShaleStore<Node> + Send + Sync> Merkle<S> {
p_ref
.write(|p| {
p.inner.as_branch_mut().unwrap().children[p_idx as usize] =
Some(c_ptr);
c_ptr;
p.rehash()
})
.unwrap();
Expand Down Expand Up @@ -772,14 +774,13 @@ impl<S: ShaleStore<Node> + Send + Sync> Merkle<S> {
NodeType::Branch(n) => {
// from: [Branch] -> [Branch]x -> [Branch]
// to: [Branch] -> [Ext] -> [Branch]
n.children[b_idx as usize] = Some(
self.put_node(Node::from(NodeType::Extension(ExtNode {
n.children[b_idx as usize] = self
.put_node(Node::from(NodeType::Extension(ExtNode {
path: PartialPath(vec![idx]),
child: c_ptr,
child_encoded: None,
})))?
.as_ptr(),
);
.as_ptr();
}
NodeType::Extension(n) => {
// from: [Ext] -> [Branch]x -> [Branch]
Expand Down Expand Up @@ -825,7 +826,7 @@ impl<S: ShaleStore<Node> + Send + Sync> Merkle<S> {
drop(c_ref);
b_ref
.write(|b| {
b.inner.as_branch_mut().unwrap().children[b_idx as usize] = Some(c_ptr);
b.inner.as_branch_mut().unwrap().children[b_idx as usize] = c_ptr;
b.rehash()
})
.unwrap();
Expand Down Expand Up @@ -929,8 +930,10 @@ impl<S: ShaleStore<Node> + Send + Sync> Merkle<S> {
let u_ref = self.get_node(u)?;
match &u_ref.inner {
NodeType::Branch(n) => {
for c in n.children.iter().flatten() {
self.remove_tree_(*c, deleted)?
for c in n.children {
if !c.is_none() {
self.remove_tree_(c, deleted)?
}
}
}
NodeType::Leaf(_) => (),
Expand Down Expand Up @@ -1001,8 +1004,8 @@ impl<S: ShaleStore<Node> + Send + Sync> Merkle<S> {

let next_ptr = match &node_ref.inner {
NodeType::Branch(n) => match n.children[nib as usize] {
Some(c) => c,
None => return Ok(None),
c if !c.is_null() => c,
_ => return Ok(None),
},
NodeType::Leaf(n) => {
let node_ref = if once(nib).chain(key_nibbles).eq(n.0.iter().copied()) {
Expand Down Expand Up @@ -1092,8 +1095,8 @@ impl<S: ShaleStore<Node> + Send + Sync> Merkle<S> {
.ok_or(MerkleError::NotBranchNode)?
.children[0];
let mut u_ref = match root {
Some(root) => self.get_node(root)?,
None => return Ok(Proof(proofs)),
root if !root.is_null() => self.get_node(root)?,
_ => return Ok(Proof(proofs)),
};

let mut nskip = 0;
Expand All @@ -1108,8 +1111,8 @@ impl<S: ShaleStore<Node> + Send + Sync> Merkle<S> {
nodes.push(u_ref.as_ptr());
let next_ptr: DiskAddress = match &u_ref.inner {
NodeType::Branch(n) => match n.children[nib as usize] {
Some(c) => c,
None => break,
c if !c.is_null() => c,
_ => break,
},
NodeType::Leaf(_) => break,
NodeType::Extension(n) => {
Expand Down Expand Up @@ -1182,7 +1185,7 @@ fn set_parent(new_chd: DiskAddress, parents: &mut [(ObjRef, u8)]) {
p_ref
.write(|p| {
match &mut p.inner {
NodeType::Branch(pp) => pp.children[*idx as usize] = Some(new_chd),
NodeType::Branch(pp) => pp.children[*idx as usize] = new_chd,
NodeType::Extension(pp) => *pp.chd_mut() = new_chd,
_ => unreachable!(),
}
Expand Down
36 changes: 18 additions & 18 deletions firewood/src/merkle/node.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ impl<T: DeserializeOwned + AsRef<[u8]>> Encoded<T> {

#[derive(PartialEq, Eq, Clone)]
pub struct BranchNode {
pub(super) children: [Option<DiskAddress>; NBRANCH],
pub(super) children: [DiskAddress; NBRANCH],
pub(super) value: Option<Data>,
pub(super) children_encoded: [Option<Vec<u8>>; NBRANCH],
}
Expand All @@ -68,7 +68,7 @@ impl Debug for BranchNode {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> {
write!(f, "[Branch")?;
for (i, c) in self.children.iter().enumerate() {
if let Some(c) = c {
if !c.is_null() {
write!(f, " ({i:x} {c:?})")?;
}
}
Expand All @@ -91,15 +91,15 @@ impl Debug for BranchNode {
impl BranchNode {
pub(super) fn single_child(&self) -> (Option<(DiskAddress, u8)>, bool) {
let mut has_chd = false;
let mut only_chd = None;
let mut only_chd: Option<(DiskAddress, u8)> = None;
for (i, c) in self.children.iter().enumerate() {
if c.is_some() {
has_chd = true;
if only_chd.is_some() {
only_chd = None;
break;
}
only_chd = (*c).map(|e| (e, i as u8))
only_chd = c.map(|e| (DiskAddress::new(e), i as u8))
}
}
(only_chd, has_chd)
Expand All @@ -122,15 +122,15 @@ impl BranchNode {
chd_encoded[i] = Some(data).filter(|data| !data.is_empty());
}

Ok(BranchNode::new([None; NBRANCH], value, chd_encoded))
Ok(BranchNode::new(Default::default(), value, chd_encoded))
}

fn encode<S: ShaleStore<Node>>(&self, store: &S) -> Vec<u8> {
let mut list = <[Encoded<Vec<u8>>; NBRANCH + 1]>::default();

for (i, c) in self.children.iter().enumerate() {
match c {
Some(c) => {
c if !c.is_none() => {
let mut c_ref = store.get_item(*c).unwrap();

if c_ref.is_encoded_longer_than_hash_len::<S>(store) {
Expand All @@ -150,7 +150,7 @@ impl BranchNode {
list[i] = Encoded::Raw(child_encoded.to_vec());
}
}
None => {
_ => {
// Check if there is already a calculated encoded value for the child, which
// can happen when manually constructing a trie from proof.
if let Some(v) = &self.children_encoded[i] {
Expand All @@ -175,7 +175,7 @@ impl BranchNode {
}

pub fn new(
chd: [Option<DiskAddress>; NBRANCH],
chd: [DiskAddress; NBRANCH],
value: Option<Vec<u8>>,
chd_encoded: [Option<Vec<u8>>; NBRANCH],
) -> Self {
Expand All @@ -190,11 +190,11 @@ impl BranchNode {
&self.value
}

pub fn chd(&self) -> &[Option<DiskAddress>; NBRANCH] {
pub fn chd(&self) -> &[DiskAddress; NBRANCH] {
&self.children
}

pub fn chd_mut(&mut self) -> &mut [Option<DiskAddress>; NBRANCH] {
pub fn chd_mut(&mut self) -> &mut [DiskAddress; NBRANCH] {
&mut self.children
}

Expand Down Expand Up @@ -448,7 +448,7 @@ impl Node {
is_encoded_longer_than_hash_len: OnceLock::new(),
encoded: OnceLock::new(),
inner: NodeType::Branch(BranchNode {
children: [Some(DiskAddress::null()); NBRANCH],
children: Default::default(),
value: Some(Data(Vec::new())),
children_encoded: Default::default(),
}),
Expand Down Expand Up @@ -558,13 +558,13 @@ impl Storable for Node {
},
)?;
let mut cur = Cursor::new(node_raw.as_deref());
let mut chd = [None; NBRANCH];
let mut chd: [DiskAddress; 16] = Default::default();
let mut buff = [0; 8];
for chd in chd.iter_mut() {
cur.read_exact(&mut buff)?;
let addr = usize::from_le_bytes(buff);
if addr != 0 {
*chd = Some(DiskAddress::from(addr))
*chd = DiskAddress::from(addr)
}
}
cur.read_exact(&mut buff[..4])?;
Expand Down Expand Up @@ -795,8 +795,8 @@ impl Storable for Node {
cur.write_all(&[Self::BRANCH_NODE]).unwrap();
for c in n.children.iter() {
cur.write_all(&match c {
Some(p) => p.to_le_bytes(),
None => 0u64.to_le_bytes(),
p if !p.is_null() => p.to_le_bytes(),
_ => 0u64.to_le_bytes(),
})?;
}
match &n.value {
Expand Down Expand Up @@ -862,11 +862,11 @@ pub(super) mod tests {
value: Option<Vec<u8>>,
repeated_encoded_child: Option<Vec<u8>>,
) -> Node {
let children: [Option<DiskAddress>; NBRANCH] = from_fn(|i| {
let children: [DiskAddress; NBRANCH] = from_fn(|i| {
if i < NBRANCH / 2 {
DiskAddress::from(repeated_disk_address).into()
DiskAddress::from(repeated_disk_address)
} else {
None
DiskAddress::null()
}
});

Expand Down
Loading