Skip to content

Make meet and join value bias independent of node layout #181

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

Closed
imlvts wants to merge 3 commits into from
Closed

Make meet and join value bias independent of node layout #181

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
219ec96
Make meet and join value bias independent of node layout
imlvts Nov 3, 2026
445bd54
Test that a join keeps a value across clone-shared structure
imlvts Oct 27, 2026
b8ee700
Drop an emptied child beside a value in a list-node meet
imlvts Oct 27, 2026
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
21 changes: 6 additions & 15 deletions src/dense_byte_node.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -606,17 +606,6 @@ impl<V: Clone + Send + Sync, A: Allocator, Cf: CoFree<V=V, A=A>> ByteNode<Cf, A>
}

impl<V: Clone + Send + Sync, A: Allocator> CellByteNode<V, A> {

/// Clones this node into its ordinary representation without cloning its pinned CoFrees.
pub(crate) fn clone_as_dense(&self) -> DenseByteNode<V, A> {
let mut dense_node = DenseByteNode::with_capacity_in(self.values.len(), self.alloc.clone());
dense_node.mask = self.mask;
dense_node.values.extend(self.values.iter().map(|cf| {
OrdinaryCoFree::new(cf.rec().cloned(), cf.val().cloned())
}));
dense_node
}

/// Ensures that a CoFree exists for the specified key, and returns a reference to the node and
/// value option
///
Expand Down Expand Up @@ -1454,8 +1443,10 @@ impl<V: Clone + Send + Sync, A: Allocator, Cf: CoFree<V=V, A=A>> TrieNode<V, A>
self.pmeet(other_dense_node).map(|new_node| TrieNodeODRc::new_in(new_node, self.alloc.clone()))
},
LINE_LIST_NODE_TAG => {
//The list node enumerates its payloads, but `self` is the left operand of every
// collision, hence `swapped`
let other_list_node = unsafe { other.as_list_unchecked() };
other_list_node.pmeet_dyn(self.as_tagged()).invert_identity()
other_list_node.pmeet_dyn_oriented(self.as_tagged(), true).invert_identity()
},
#[cfg(feature = "bridge_nodes")]
TaggedNodeRef::BridgeNode(other_bridge_node) => {
Expand All @@ -1465,9 +1456,9 @@ impl<V: Clone + Send + Sync, A: Allocator, Cf: CoFree<V=V, A=A>> TrieNode<V, A>
let other_byte_node = unsafe { other.as_cell_unchecked() };
self.pmeet(other_byte_node).map(|new_node| TrieNodeODRc::new_in(new_node, self.alloc.clone()))
},
TINY_REF_NODE_TAG => {
let tiny_node = unsafe { other.as_tiny_unchecked() };
tiny_node.pmeet_dyn(self.as_tagged()).invert_identity()
TINY_REF_NODE_TAG => match unsafe { other.as_tiny_unchecked() }.into_full() {
Some(full_node) => self.pmeet_dyn(full_node.as_tagged()),
None => AlgebraicResult::None,
},
EMPTY_NODE_TAG => AlgebraicResult::None,
_ => unsafe{ unreachable_unchecked() }
Expand Down
118 changes: 58 additions & 60 deletions src/line_list_node.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1386,10 +1386,11 @@ fn merge_guts<'a, V: Clone + Lattice + Send + Sync, A: Allocator, const ASLOT: u
unsafe{ intermediate_node.set_payload_owned::<0>(&a_key[overlap..], a_payload); }
debug_assert!(validate_node(&intermediate_node));
let intermediate_node = TrieNodeODRc::new_in(intermediate_node, a.alloc.clone());
return match b_child.pjoin(&intermediate_node) {
//`a` is the left operand, so its payload goes on the left of the join
return match intermediate_node.pjoin(b_child) {
AlgebraicResult::Element(joined) => AlgebraicResult::Element((&a_key[0..overlap], ValOrChild::Child(joined))),
//`b`'s child already held `a`'s payload, so `b`'s slot is the result
AlgebraicResult::Identity(mask) if mask & SELF_IDENT > 0 => AlgebraicResult::Identity(COUNTER_IDENT),
AlgebraicResult::Identity(mask) if mask & COUNTER_IDENT > 0 => AlgebraicResult::Identity(COUNTER_IDENT),
AlgebraicResult::Identity(_) if a.other_slot_has_value_at::<ASLOT>(b_key) && b_child.is_empty() => AlgebraicResult::None,
AlgebraicResult::Identity(_) => AlgebraicResult::Element((&a_key[0..overlap], ValOrChild::Child(intermediate_node))),
AlgebraicResult::None => unreachable!(), //`intermediate_node` is never empty
Expand Down Expand Up @@ -2723,36 +2724,13 @@ impl<V: Clone + Send + Sync, A: Allocator> TrieNode<V, A> for LineListNode<V, A>
Err(joined_dense_node) => joined_dense_node.map(|node| TrieNodeODRc::new_in(node, self.alloc.clone())),
}
},
DENSE_BYTE_NODE_TAG => {
let other_dense_node = unsafe{ other.as_dense_unchecked() };
let mut new_node = other_dense_node.clone();
match new_node.merge_from_list_node(self) {
//Both nodes were empty so the join is empty too
AlgebraicStatus::None => {
debug_assert!(self.node_is_empty() && other_dense_node.node_is_empty());
AlgebraicResult::None
},
AlgebraicStatus::Identity => AlgebraicResult::Identity(COUNTER_IDENT),
AlgebraicStatus::Element => AlgebraicResult::Element(TrieNodeODRc::new_in(new_node, self.alloc.clone()))
}
},
//Joined through a byte-node copy of `self`, so `self` stays the left operand and keeps its
// values on a collision. Merging `self` into a clone of `other` would keep `other`'s
DENSE_BYTE_NODE_TAG | CELL_BYTE_NODE_TAG => self.to_dense().as_tagged().pjoin_dyn(other),
#[cfg(feature = "bridge_nodes")]
TaggedNodeRef::BridgeNode(_other_bridge_node) => {
unimplemented!()
},
CELL_BYTE_NODE_TAG => {
let other_cell_node = unsafe{ other.as_cell_unchecked() };
let mut new_node = other_cell_node.clone_as_dense();
match new_node.merge_from_list_node(self) {
//See the DENSE_BYTE_NODE_TAG arm: two empty nodes join to an empty result
AlgebraicStatus::None => {
debug_assert!(self.node_is_empty() && other_cell_node.node_is_empty());
AlgebraicResult::None
},
AlgebraicStatus::Identity => AlgebraicResult::Identity(COUNTER_IDENT),
AlgebraicStatus::Element => AlgebraicResult::Element(TrieNodeODRc::new_in(new_node, self.alloc.clone()))
}
},
TINY_REF_NODE_TAG => {
//Expand the tiny node and keep `self` on the left (see DenseByteNode::pjoin_dyn)
let tiny_node = unsafe{ other.as_tiny_unchecked() };
Expand All @@ -2776,23 +2754,19 @@ impl<V: Clone + Send + Sync, A: Allocator> TrieNode<V, A> for LineListNode<V, A>
let other_list_node = unsafe{ other_node.as_list_unchecked() };
merge_into_list_nodes(self, other_list_node)
},
DENSE_BYTE_NODE_TAG => {
let other_dense_node = unsafe{ other_node.as_dense_unchecked() };
let mut new_node = other_dense_node.clone();
let status = new_node.merge_from_list_node(self);
debug_assert!(!status.is_none());
(AlgebraicStatus::Element, Err(TrieNodeODRc::new_in(new_node, self.alloc.clone())))
//As in `pjoin_dyn`: through a byte-node copy of `self`, which then replaces `self`
DENSE_BYTE_NODE_TAG | CELL_BYTE_NODE_TAG => {
let mut new_node = self.to_dense();
let (_, result) = new_node.make_mut().join_into_dyn(other);
if let Err(replacement) = result {
new_node = replacement;
}
(AlgebraicStatus::Element, Err(new_node))
},
#[cfg(feature = "bridge_nodes")]
TaggedNodeRef::BridgeNode(_other_bridge_node) => {
unimplemented!()
},
CELL_BYTE_NODE_TAG => {
let mut new_node = unsafe{ other_node.as_cell_unchecked() }.clone_as_dense();
let status = new_node.merge_from_list_node(self);
debug_assert!(!status.is_none());
(AlgebraicStatus::Element, Err(TrieNodeODRc::new_in(new_node, self.alloc.clone())))
},
EMPTY_NODE_TAG => (AlgebraicStatus::Identity, Ok(())),
_ => unsafe{ unreachable_unchecked() }
}
Expand Down Expand Up @@ -2988,6 +2962,29 @@ impl<V: Clone + Send + Sync, A: Allocator> TrieNode<V, A> for LineListNode<V, A>
}

fn pmeet_dyn(&self, other: TaggedNodeRef<V, A>) -> AlgebraicResult<TrieNodeODRc<V, A>> where V: Lattice {
self.pmeet_dyn_oriented(other, false)
}
fn psubtract_dyn(&self, other: TaggedNodeRef<V, A>) -> AlgebraicResult<TrieNodeODRc<V, A>> where V: DistributiveLattice {
debug_assert!(validate_node(self));
let slot0_result = self.subtract_from_slot_contents::<0>(other);
let slot1_result = self.subtract_from_slot_contents::<1>(other);
self.combine_slot_results_into_node_result(slot0_result, slot1_result)
}
fn prestrict_dyn(&self, other: TaggedNodeRef<V, A>) -> AlgebraicResult<TrieNodeODRc<V, A>> {
debug_assert!(validate_node(self));
let slot0_result = self.restrict_slot_contents::<0>(other);
let slot1_result = self.restrict_slot_contents::<1>(other);
self.combine_slot_results_into_node_result(slot0_result, slot1_result)
}
fn clone_self(&self) -> TrieNodeODRc<V, A> {
TrieNodeODRc::new_in(self.clone(), self.alloc.clone())
}
}

impl<V: Clone + Send + Sync, A: Allocator> LineListNode<V, A> {
/// [TrieNode::pmeet_dyn], with `swapped` meaning `self` is really the *right* operand and
/// `other` the left one (see [pmeet_generic]). A byte node meets a list node this way
pub(crate) fn pmeet_dyn_oriented(&self, other: TaggedNodeRef<V, A>, swapped: bool) -> AlgebraicResult<TrieNodeODRc<V, A>> where V: Lattice {
debug_assert!(validate_node(self));

let mut self_payloads_buf: [(&[u8], PayloadRef<V, A>); 2] = [(&[], PayloadRef::None); 2];
Expand All @@ -3012,32 +3009,33 @@ impl<V: Clone + Send + Sync, A: Allocator> TrieNode<V, A> for LineListNode<V, A>
_ => unsafe{ unreachable_unchecked() }
};

pmeet_generic::<2, V, A, _>(self_payloads, other, |payloads| {
pmeet_generic::<2, V, A, _>(self_payloads, other, swapped, |payloads| {
debug_assert_eq!(payloads.len(), self_payloads.len());
let slot0_payload = payloads.get_mut(0).and_then(|p| core::mem::take(p)).map(|p| p.into());
let slot1_payload = payloads.get_mut(1).and_then(|p| core::mem::take(p)).map(|p| p.into());
let new_node = self.clone_with_updated_payloads(slot0_payload, slot1_payload).unwrap();
let mut slot0_payload = payloads.get_mut(0).and_then(|p| core::mem::take(p));
let mut slot1_payload = payloads.get_mut(1).and_then(|p| core::mem::take(p));
//A child that met down to an empty node is a dangling sentinel. Beside a value under the
// same key it is redundant, since the value keeps the path, and a valid node may not hold it
if self_slot_count == 2 && self_payloads[0].0 == self_payloads[1].0 {
let is_empty_child = |p: &Option<ValOrChild<V, A>>| matches!(p, Some(ValOrChild::Child(child)) if child.as_tagged().node_is_empty());
let is_val = |p: &Option<ValOrChild<V, A>>| matches!(p, Some(ValOrChild::Val(_)));
if is_empty_child(&slot0_payload) && is_val(&slot1_payload) {
slot0_payload = None;
}
if is_empty_child(&slot1_payload) && is_val(&slot0_payload) {
slot1_payload = None;
}
}
let new_node = self.clone_with_updated_payloads(slot0_payload.map(|p| p.into()), slot1_payload.map(|p| p.into())).unwrap();
TrieNodeODRc::new_in(new_node, self.alloc.clone())
})
}
fn psubtract_dyn(&self, other: TaggedNodeRef<V, A>) -> AlgebraicResult<TrieNodeODRc<V, A>> where V: DistributiveLattice {
debug_assert!(validate_node(self));
let slot0_result = self.subtract_from_slot_contents::<0>(other);
let slot1_result = self.subtract_from_slot_contents::<1>(other);
self.combine_slot_results_into_node_result(slot0_result, slot1_result)
}
fn prestrict_dyn(&self, other: TaggedNodeRef<V, A>) -> AlgebraicResult<TrieNodeODRc<V, A>> {
debug_assert!(validate_node(self));
let slot0_result = self.restrict_slot_contents::<0>(other);
let slot1_result = self.restrict_slot_contents::<1>(other);
self.combine_slot_results_into_node_result(slot0_result, slot1_result)
}
fn clone_self(&self) -> TrieNodeODRc<V, A> {
TrieNodeODRc::new_in(self.clone(), self.alloc.clone())

/// A byte node holding the same contents, for an operation against a byte node that has to
/// keep `self` on the left
pub(crate) fn to_dense(&self) -> TrieNodeODRc<V, A> {
self.clone().convert_to_dense::<OrdinaryCoFree<V, A>>(2)
}
}

impl<V: Clone + Send + Sync, A: Allocator> LineListNode<V, A> {
#[inline]
fn preserve_prune_limit(&mut self, key: &[u8], prune_limit: usize) {
if prune_limit > 0 && prune_limit < key.len() {
Expand Down
35 changes: 35 additions & 0 deletions src/trie_map.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1592,6 +1592,41 @@ mod tests {
assert_eq!(map.val_at(b"foo/bar"), Some(&300));
assert_eq!(map.val_at(b"missing"), None);
}
/// `b` is a clone of `a` plus one value, so the two share nodes. A join may not lose that
/// value in either operand order. Found by the algebraic fuzzer's `join-commutative` law.
#[test]
fn map_join_keeps_value_across_shared_structure() {
let mut a = PathMap::<u64>::new();
for p in [&[0u8, 0, 0, 0, 0][..], &[1, 0, 1], &[1, 0, 3]] {
a.create_path(p);
}
let mut b = a.clone();
b.set_val_at([1u8, 0, 0, 0], 1);

assert_eq!(a.join(&b).val_at([1u8, 0, 0, 0]), Some(&1));
assert_eq!(b.join(&a).val_at([1u8, 0, 0, 0]), Some(&1));
}

/// A meet where a list node holds a value and a child under one key, and the child meets an
/// empty node: the result must not keep an empty child beside the value
#[test]
fn map_meet_value_beside_emptied_child() {
let mut x = PathMap::<u64>::new();
x.create_path([2u8]);
let mut big = PathMap::<u64>::new();
big.set_val_at([1u8], 2);
big.create_path([1u8, 2, 1, 2, 1]);
big.set_val_at([2u8], 1);
let mut c = PathMap::<u64>::new();
c.set_val_at([2u8], 1);
c.set_val_at([2u8, 0], 2);
c.set_val_at([2u8, 2], 2);

let met = x.join(&big).meet(&x.join(&c));
assert_eq!(met.val_at([2u8]), Some(&1));
assert_eq!(met.val_count(), 1);
}

}

//GOAT, Consider refactor of zipper traits. `WriteZipper` -> `PathWriter`. Zipper is split into the zipper
Expand Down
Loading