From f5426c9d67d53f74a190f1d8b80cd4f3433f2cd3 Mon Sep 17 00:00:00 2001 From: finch Date: Wed, 29 Jul 2026 09:10:10 -0400 Subject: [PATCH 1/4] join: merge-walk the radix fans; imbl's diff mishandles clone-derived maps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two honest overlapping gossip sessions could silently delete a message nobody redacted: a trivially-resolving session hands the install the fork-time root itself, a concurrent session's install builds a copy-on-write sibling of that same map, and joining the two handed imbl's OrdMap::diff a clone-derived pair — which it walks wrong (https://github.com/jneem/imbl/issues/161: the cursor over-advances after a pointer-shared run, swallowing uncompared entries and desyncing onto innocent radixes, which deletion honoring then redacts). The new witness test sweeps redaction across every leaf and demands S2's install of its own causal past be a hash-identity; it fails under the diff-backed walk on imbl 7.0.0 and 7.0.1 both. The join now merge-walks the two ascending radix fans itself, pruning equal pairs by the same pointer-or-hash equality diff delegated to, so imbl's diff leaves the load-bearing path entirely (Children::diff_owned goes with it; Children::iter replaces it). Equal subtrees still prune before descent, so joins stay delta-proportional at the tree level; the per-divergent-node cost is the fan enumeration, bounded by 256. --- .../streaming/remote/proxy/tests/greeting.rs | 87 +++++++++++++++++++ src/tree/traverse/join.rs | 76 +++++++++++----- src/tree/typed/node.rs | 40 +++------ 3 files changed, 149 insertions(+), 54 deletions(-) diff --git a/src/tree/mirror/streaming/remote/proxy/tests/greeting.rs b/src/tree/mirror/streaming/remote/proxy/tests/greeting.rs index e919c724..8cc2c41a 100644 --- a/src/tree/mirror/streaming/remote/proxy/tests/greeting.rs +++ b/src/tree/mirror/streaming/remote/proxy/tests/greeting.rs @@ -199,3 +199,90 @@ fn mixed_empty_and_populated_converges() { assert_eq!(hash_of(&left), expected); assert_eq!(hash_of(&right), expected); } +/// WITNESS (reachability of the `imbl` `OrdMap::diff` defect, +/// , at the gossip install): +/// +/// Two honest, overlapping sessions at one peer silently delete a message +/// nobody redacted. +/// +/// The shape: session S2 (with a peer converged at T0) forks at T0; equal +/// handshake versions resolve without opening the descent, so S2's +/// reconciled root is the fork-time root handle itself — its children map +/// is the very `OrdMap` object M0. Meanwhile session S1 (with a peer that +/// redacted one message whose root radix `r_h` heads M0's second imbl +/// chunk) installs first: the install's `Tree::join` builds its merged map +/// as `ours.clone()` + `remove(r_h)` — a copy-on-write descendant M1 +/// sharing M0's first chunk. S2's install then joins M1 against M0. A +/// diff-backed walk over that clone-derived pair is exactly what imbl's +/// `OrdMap::diff` mishandles (issue #161: the cursor over-advances after a +/// pointer-shared run, swallowing the uncompared boundary pair and +/// desyncing onto *neighboring, unchanged* radixes, which version +/// dominance then redacts as phantom deletions). +/// +/// T0 is S2's causal past, so S2's install must be an identity on the +/// tree: the assertion is total (post-install hash equals pre-install +/// hash), swept over the redaction of each leaf in turn so the +/// chunk-boundary radix is hit whichever radix heads the second chunk. +/// `Tree::join` therefore merge-walks the radix fans itself and never +/// hands imbl's diff a clone-derived pair; this witness holds the join to +/// that, and fails if a diff-backed walk ever returns. +#[test] +fn overlapping_sessions_lose_innocent_leaf_after_honored_redaction() { + use crate::tree::Action; + + // T0: 25 unit messages. Hash-uniform keys give the root ~25 children, + // more than one imbl chunk (16), with every radix holding one leaf, so + // honoring one leaf's redaction empties its root radix. + let p = nth_party(0); + let mut t0 = Tree::new(); + t0.act(&p, (0..25).map(|_| Action::Insert(Message::new(())))); + let keys: Vec<_> = t0.iter().map(|(k, _, _)| k).collect(); + + // S2's wire session against a peer converged at T0, forked at T0: + // equal versions resolve to the fork-time root. + let (s2_reconciled, _) = wire_reconcile(t0.root.clone(), t0.root.clone()); + + let mut lost = Vec::new(); + for k in &keys { + // S1's counterparty: converged at T0, then redacted the leaf at + // `k` (a local act rebuilds its own maps afresh; the sharing that + // matters is created by our install below, not here). + let mut twin = Tree { + root: t0.root.clone(), + }; + twin.act(&nth_party(1), [Action::Forget(*k)]); + + // S1's session and install: reconcile T0 against the redacting + // twin over the wire, then join the result into the live tree. + // Deletion honoring drops radix `r_h`: the live tree's root map is + // now a copy-on-write descendant of M0 missing one radix. + let (s1_reconciled, _) = wire_reconcile(t0.root.clone(), twin.root.clone()); + let mut live = Tree { + root: t0.root.clone(), + }; + live.join(Tree { + root: s1_reconciled, + }); + let expected = live.hash(); + + // S2's install, after S1's: joining our own causal past must be an + // identity on the tree. + live.join(Tree { + root: s2_reconciled.clone(), + }); + + if live.hash() != expected { + let missing: Vec<_> = keys + .iter() + .filter(|k2| *k2 != k && live.get(k2).is_none()) + .copied() + .collect(); + lost.push((*k, missing)); + } + } + assert!( + lost.is_empty(), + "S2's install lost innocent leaves after S1 honored a redaction \ + (redacted key, innocent leaves missing): {lost:#?}", + ); +} diff --git a/src/tree/traverse/join.rs b/src/tree/traverse/join.rs index 10b4aca1..6b9363da 100644 --- a/src/tree/traverse/join.rs +++ b/src/tree/traverse/join.rs @@ -21,12 +21,18 @@ //! - **both have it, hashes equal**: the subtrees are identical (content //! addressing makes equal hash ⟹ equal content, versions included), so keep //! one verbatim. -//! - **both have it, hashes differ**: explode both one level and recurse only -//! into the radixes whose child subtrees differ (an [`OrdMap::diff`] that -//! prunes the shared ones by pointer), reassembling with [`Node::branch`] -//! (which re-compresses singletons and recomputes the joined branch version). +//! - **both have it, hashes differ**: explode both one level and merge-walk +//! the two ascending radix fans in lockstep, recursing only into the +//! radixes whose child subtrees differ — children equal by pointer or by +//! content hash carry over verbatim through the shared structure — and +//! reassembling with [`Node::branch`] (which re-compresses singletons and +//! recomputes the joined branch version). //! -//! [`OrdMap::diff`]: imbl::OrdMap::diff +//! The merge walk enumerates each *divergent* branch's full fan (≤ 256 +//! entries) rather than only its changed radixes; equal subtrees still +//! prune by pointer-or-hash before any descent, so a small delta against a +//! large shared tree costs work proportional to the delta at the tree +//! level, with a per-divergent-node constant bounded by the fan. use crate::Version; @@ -95,30 +101,52 @@ where return Some(ours); } - // Differing subtrees: descend one level, but only into the - // radixes that actually diverge. `OrdMap::diff` walks both - // persistent B-trees in lockstep and prunes whole spans that are - // pointer-equal — the shared backing a fork leaves behind — so it - // yields exactly the changed children, in ascending-radix order, - // without enumerating the full radix union or probing the - // unchanged children. A small delta against a large shared tree - // therefore costs work proportional to the delta, not to the - // fan-out. (`diff` classifies a child as unchanged via `Node`'s - // `PartialEq`, which is the same `ptr_eq`-or-hash short-circuit - // the node-level equality above uses: nothing is learned across - // an equal subtree, so it carries over verbatim.) + // Differing subtrees: descend one level, merge-walking the + // two ascending radix fans in lockstep and recursing only + // into the radixes that actually diverge. A child equal on + // both sides — by `Node`'s `ptr_eq`-or-hash equality, the + // same short-circuit the node-level check above uses — + // carries over verbatim: nothing is learned across an equal + // subtree. One-sided radixes recurse too: the asymmetric + // arms above filter them against the absent side's version, + // which is where deletion honoring drops what that side + // redacted. // - // Collect the divergent radixes first — cloning only those few - // children — so we don't hold `diff`'s borrow of `ours` / - // `theirs` across the recursive rewrite of the merged map. + // The walk reads both fans directly rather than diffing the + // two maps against each other: the merged map starts from + // *ours* and only divergent radixes are rewritten, so every + // shared child persists by structural sharing. let ours = ours.into_children(); let theirs = theirs.into_children(); - // Start the merged map from *ours* (moved — `diff`'s borrow has - // ended) and rewrite only the divergent radixes; every shared - // child carries over verbatim by structural sharing. let mut merged = ours.clone(); - for (radix, our_child, their_child) in ours.diff_owned(&theirs) { + let mut ours = ours.iter().peekable(); + let mut theirs = theirs.iter().peekable(); + loop { + let (radix, our_child, their_child) = match (ours.peek(), theirs.peek()) { + (None, None) => break, + (Some((radix, _)), None) => { + (*radix, ours.next().map(|(_, child)| child), None) + } + (None, Some((radix, _))) => { + (*radix, None, theirs.next().map(|(_, child)| child)) + } + (Some((ours_radix, _)), Some((theirs_radix, _))) => { + let radix = (*ours_radix).min(*theirs_radix); + ( + radix, + ours.next_if(|(r, _)| *r == radix).map(|(_, child)| child), + theirs.next_if(|(r, _)| *r == radix).map(|(_, child)| child), + ) + } + }; + + if let (Some(our_child), Some(their_child)) = (&our_child, &their_child) + && our_child == their_child + { + continue; + } + match Join::join(our_child, their_child, a_version, b_version) { Some(child) => { merged.insert(radix, child); diff --git a/src/tree/typed/node.rs b/src/tree/typed/node.rs index 1678f06a..db276156 100644 --- a/src/tree/typed/node.rs +++ b/src/tree/typed/node.rs @@ -1,7 +1,7 @@ use std::{fmt::Debug, iter::Map, marker::PhantomData}; use borsh::{BorshDeserialize, BorshSerialize}; -use imbl::{OrdMap, ordmap}; +use imbl::OrdMap; use crate::{Version, message::Message}; @@ -74,36 +74,16 @@ impl Children { self.inner.remove(radix).map(Node::from_untyped) } - /// Walk this map and `other` in lockstep, yielding only the radixes - /// whose children differ, as `(radix, ours, theirs)` with `None` for an - /// absent side. + /// The children in ascending radix order, as owned handles (each a + /// cheap reference bump into the shared structure). /// - /// Spans that are pointer-equal — the shared backing a - /// fork leaves behind — prune wholesale without being probed, so a - /// small delta against a large shared map costs work proportional to - /// the delta. The engine of [`Tree::join`](crate::tree::Tree::join)'s - /// recursion. - #[allow(clippy::type_complexity)] - pub fn diff_owned<'a>( - &'a self, - other: &'a Self, - ) -> impl Iterator>, Option>)> + 'a { - self.inner.diff(&other.inner).map(|item| match item { - ordmap::DiffItem::Add(&radix, theirs) => { - (radix, None, Some(Node::from_untyped(theirs.clone()))) - } - ordmap::DiffItem::Remove(&radix, ours) => { - (radix, Some(Node::from_untyped(ours.clone())), None) - } - ordmap::DiffItem::Update { - old: (&radix, ours), - new: (_, theirs), - } => ( - radix, - Some(Node::from_untyped(ours.clone())), - Some(Node::from_untyped(theirs.clone())), - ), - }) + /// The engine of [`Tree::join`](crate::tree::Tree::join)'s recursion: + /// the merge walk pairs two of these streams by radix and prunes equal + /// pairs by [`Node`]'s pointer-or-hash equality before descending. + pub fn iter(&self) -> impl Iterator)> + '_ { + self.inner + .iter() + .map(|(radix, child)| (*radix, Node::from_untyped(child.clone()))) } } From f70f9559ab385018a93d21f5aac4b3085ffa7d72 Mon Sep 17 00:00:00 2001 From: finch Date: Wed, 29 Jul 2026 10:43:21 -0400 Subject: [PATCH 2/4] the fan: a sorted inline vector for the radix children A branch's children map needs none of what a persistent ordered map charges for: every clone in the crate is immediately followed by an O(fan) walk (the join's lockstep merge, the explode-and-reassemble paths, the wire decoder), the values are one-word Arc handles, and every mutation site already holds the fan exclusively via make_mut or mem::take. Fan spends those facts: (radix, child) pairs strictly ascending in a SmallVec with two entries inline, so the transient singleton produced by exploding a path-compressed node - the hottest allocation shape in a profiled session - never touches the allocator, and the ascending order the hash preimage and wire encoding consume becomes a structural guarantee rather than caller discipline. Landed dark: nothing delegates to it yet (the swap follows), so the module carries a scoped dead_code allow that leaves with the wiring. The differential suite pins it against a BTreeMap oracle after every op - both iteration directions, all 256 point lookups, the successor probe against the oracle's range query, displaced-handle identity - plus strict-ascent, exact size_hint under two-ended consumption, the last-wins FromIterator contract, and a 40-byte size pin (smallvec's union layout) so inline-capacity growth stays a deliberate decision. Motivation: imbl's OrdMap::diff mishandles clone-derived maps (jneem/imbl#161, unpatched); the fan is the load-bearing replacement that lets the dependency leave entirely. --- Cargo.lock | 1 + Cargo.toml | 1 + src/tree/typed/untyped.rs | 2 + src/tree/typed/untyped/fan.rs | 264 ++++++++++++++++++++++++++ src/tree/typed/untyped/fan/tests.rs | 284 ++++++++++++++++++++++++++++ 5 files changed, 552 insertions(+) create mode 100644 src/tree/typed/untyped/fan.rs create mode 100644 src/tree/typed/untyped/fan/tests.rs diff --git a/Cargo.lock b/Cargo.lock index 7e51aacb..b5dc890d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4048,6 +4048,7 @@ dependencies = [ "ratatui", "rumors", "seq-macro", + "smallvec", "static_assertions", "thiserror 2.0.18", "tinyvec", diff --git a/Cargo.toml b/Cargo.toml index bd4b508f..4744a46a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -44,6 +44,7 @@ static_assertions = "1.1" itertools = "0.14" borsh = { version = "1.6", features = ["bytes", "derive", "de_strict_order"] } seq-macro = "0.3" +smallvec = { version = "1.15", features = ["union"] } tinyvec = { version = "1.11", features = ["alloc"] } thiserror = "2.0" hex = "0.4" diff --git a/src/tree/typed/untyped.rs b/src/tree/typed/untyped.rs index c3f20e62..ee5d96ae 100644 --- a/src/tree/typed/untyped.rs +++ b/src/tree/typed/untyped.rs @@ -8,6 +8,8 @@ use tinyvec::ArrayVec; use crate::{Version, message::Message, tree::typed::Hash}; +#[cfg_attr(not(test), allow(dead_code))] +mod fan; mod iter; pub use iter::{Iter, Leaf, Range, RangeOwned}; diff --git a/src/tree/typed/untyped/fan.rs b/src/tree/typed/untyped/fan.rs new file mode 100644 index 00000000..bb1c17e6 --- /dev/null +++ b/src/tree/typed/untyped/fan.rs @@ -0,0 +1,264 @@ +//! The radix fan: a branch's children as a flat, strictly-ascending +//! association from child index to child node. +//! +//! A fan holds at most 256 entries (its keys are the byte alphabet) and a +//! materialized branch holds at least 2 (path compression collapses +//! singletons), with the population concentrated near the small end: past +//! the first few levels of a uniform-hash tree almost every branch carries +//! only a handful of children. At that size a sorted inline vector beats a +//! tree-shaped map on every operation the crate performs — one contiguous +//! (often inline) allocation instead of a heap node per entry, and +//! cache-friendly iteration in the ascending radix order the hash preimage, +//! the wire encoding, and the merge walks all consume directly. +//! +//! [`Fan`] is deliberately *not* a persistent map. Structural sharing lives +//! one level up, on the node handles the fan stores (each entry is one +//! `Arc` reference): cloning a fan is one refcount bump per child, and +//! every mutation site first takes exclusive ownership of the fan (via +//! `Arc::make_mut` or `mem::take` on the enclosing node), so copy-on-write +//! inside the container would buy nothing that the walk following every +//! clone does not already pay for. + +use std::fmt::Debug; +use std::mem; + +use smallvec::SmallVec; + +use super::Node; + +#[cfg(test)] +mod tests; + +/// Entries a [`Fan`] holds inline before spilling to the heap. +/// +/// An entry is 16 bytes (`(u8, Node)`, padded to the handle's +/// alignment), so the fan occupies `8 + max(16 × FAN_INLINE, 16)` bytes +/// inline — 40 at 2. Two entries cover the modal materialized branch (path +/// compression guarantees at least two children, and interior branches +/// rarely carry more) and the transient singleton produced by exploding a +/// path-compressed node, so the tree's hottest reassembly paths never +/// touch the allocator for the fan itself; anything wider spills once to a +/// size-classed heap block the branch then owns. A larger inline capacity +/// would tax every node allocation — the fan is embedded in the largest +/// variant of every node's children enum — for shapes the tree seldom +/// materializes. +const FAN_INLINE: usize = 2; + +/// The children of a branch: `(radix, child)` pairs kept strictly +/// ascending by radix, with no duplicate radixes. +/// +/// The ordering invariant is private to this module; every constructor and +/// mutator preserves it, so consumers read ascending radix order +/// structurally — the hash preimage and the wire encoding need no re-sort +/// and no caller discipline. +pub struct Fan { + /// Invariant: strictly ascending by radix, no duplicates. + entries: SmallVec<[(u8, Node); FAN_INLINE]>, +} + +impl Default for Fan { + fn default() -> Self { + Self { + entries: SmallVec::new(), + } + } +} + +impl Clone for Fan { + fn clone(&self) -> Self { + Self { + entries: self.entries.clone(), + } + } +} + +impl Debug for Fan { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_map().entries(self.iter()).finish() + } +} + +impl Fan { + /// The empty fan. + pub fn new() -> Self { + Self::default() + } + + /// The fan holding exactly `child` at `radix`. + /// + /// The single-entry shape is transient — it exists between exploding a + /// path-compressed node and the `branch` constructor collapsing it back + /// — and fits inline, so building it never allocates. + pub fn unit(radix: u8, child: Node) -> Self { + let mut entries = SmallVec::new(); + entries.push((radix, child)); + Self { entries } + } + + /// The number of children present (0..=256). + pub fn len(&self) -> usize { + self.entries.len() + } + + /// Whether no child is present. + pub fn is_empty(&self) -> bool { + self.entries.is_empty() + } + + /// The index of `radix`, or the insertion point that keeps the fan + /// ascending. + fn search(&self, radix: u8) -> Result { + self.entries + .binary_search_by_key(&radix, |(radix, _)| *radix) + } + + /// The child at `radix`, if any. + pub fn get(&self, radix: u8) -> Option<&Node> { + self.search(radix).ok().map(|at| &self.entries[at].1) + } + + /// Insert `child` at `radix`, returning any child it displaced. + pub fn insert(&mut self, radix: u8, child: Node) -> Option> { + match self.search(radix) { + Ok(at) => Some(mem::replace(&mut self.entries[at].1, child)), + Err(at) => { + self.entries.insert(at, (radix, child)); + None + } + } + } + + /// Remove and return the child at `radix`, if any. + pub fn remove(&mut self, radix: u8) -> Option> { + self.search(radix).ok().map(|at| self.entries.remove(at).1) + } + + /// The least entry at or above `radix`, if any. + /// + /// The resume point of a suspended ascending walk: one binary search, + /// so the entries between the cursor and the answer are never + /// enumerated and no sibling handle is materialized. + pub fn successor(&self, radix: u8) -> Option<(u8, &Node)> { + let at = self + .entries + .partition_point(|(present, _)| *present < radix); + self.entries.get(at).map(|(radix, child)| (*radix, child)) + } + + /// Append `(radix, child)`, which must be strictly greater than the + /// fan's current last radix. + /// + /// Sorted bulk builds produce every entry in ascending radix order, so + /// this O(1) append is their build path; an out-of-order push trips a + /// debug assertion (in release it would silently break the invariant + /// the binary searches rely on). + pub fn push(&mut self, radix: u8, child: Node) { + debug_assert!( + self.entries.last().is_none_or(|(last, _)| *last < radix), + "Fan::push given a radix not greater than the current last", + ); + self.entries.push((radix, child)); + } + + /// Iterate the fan in ascending radix order. + /// + /// Double-ended, and the length reported by `size_hint` is exact at + /// every step: preimage assembly sizes its buffer from it. + pub fn iter(&self) -> Iter<'_, T> { + Iter { + inner: self.entries.iter(), + } + } + + /// Iterate the children alone, in ascending radix order. + pub fn values(&self) -> impl DoubleEndedIterator> + ExactSizeIterator { + self.entries.iter().map(|(_, child)| child) + } +} + +/// Collect `(radix, child)` pairs into a fan. +/// +/// Later pairs displace earlier ones at the same radix, matching repeated +/// [`insert`](Fan::insert). Every reassembly in the crate feeds pairs +/// already strictly ascending and duplicate-free, which this recognizes in +/// one pass; anything else pays one stable sort. +impl FromIterator<(u8, Node)> for Fan { + fn from_iter)>>(iter: I) -> Self { + let mut entries: SmallVec<[(u8, Node); FAN_INLINE]> = iter.into_iter().collect(); + if !entries.windows(2).all(|pair| pair[0].0 < pair[1].0) { + entries.sort_by_key(|(radix, _)| *radix); + let mut deduped: SmallVec<[(u8, Node); FAN_INLINE]> = + SmallVec::with_capacity(entries.len()); + for (radix, child) in entries { + match deduped.last_mut() { + Some((last, slot)) if *last == radix => *slot = child, + _ => deduped.push((radix, child)), + } + } + entries = deduped; + } + Self { entries } + } +} + +/// The borrowing walk over a fan, ascending by radix; see [`Fan::iter`]. +pub struct Iter<'a, T> { + inner: std::slice::Iter<'a, (u8, Node)>, +} + +impl<'a, T> Iterator for Iter<'a, T> { + type Item = (u8, &'a Node); + + fn next(&mut self) -> Option { + self.inner.next().map(|(radix, child)| (*radix, child)) + } + + fn size_hint(&self) -> (usize, Option) { + self.inner.size_hint() + } +} + +impl DoubleEndedIterator for Iter<'_, T> { + fn next_back(&mut self) -> Option { + self.inner.next_back().map(|(radix, child)| (*radix, child)) + } +} + +impl ExactSizeIterator for Iter<'_, T> {} + +/// The consuming walk over a fan, ascending by radix. +pub struct IntoIter { + inner: smallvec::IntoIter<[(u8, Node); FAN_INLINE]>, +} + +impl Iterator for IntoIter { + type Item = (u8, Node); + + fn next(&mut self) -> Option { + self.inner.next() + } + + fn size_hint(&self) -> (usize, Option) { + self.inner.size_hint() + } +} + +impl DoubleEndedIterator for IntoIter { + fn next_back(&mut self) -> Option { + self.inner.next_back() + } +} + +impl ExactSizeIterator for IntoIter {} + +impl IntoIterator for Fan { + type Item = (u8, Node); + type IntoIter = IntoIter; + + /// Consume the fan in ascending radix order. + fn into_iter(self) -> Self::IntoIter { + IntoIter { + inner: self.entries.into_iter(), + } + } +} diff --git a/src/tree/typed/untyped/fan/tests.rs b/src/tree/typed/untyped/fan/tests.rs new file mode 100644 index 00000000..a826aadc --- /dev/null +++ b/src/tree/typed/untyped/fan/tests.rs @@ -0,0 +1,284 @@ +use std::collections::BTreeMap; + +use proptest::collection::vec; +use proptest::prelude::*; +use proptest::test_runner::TestCaseError; + +use crate::{Version, message::Message}; + +use super::super::Node; +use super::Fan; + +/// A fresh child handle with its own backing allocation. +/// +/// Each call yields a distinct `Arc`, so `Node::ptr_eq` distinguishes any +/// two children a test creates: the differential comparisons below check +/// *which* handle a fan holds, not merely that one is present. +fn child() -> Node<()> { + Node::leaf(Version::new(), Message::new(())) +} + +/// One step of the differential op sequence. +#[derive(Debug, Clone)] +enum Op { + Insert(u8), + Remove(u8), +} + +/// A short sequence of inserts and removals over arbitrary radixes. +fn arb_ops() -> impl Strategy> { + vec( + prop_oneof![ + any::().prop_map(Op::Insert), + any::().prop_map(Op::Remove), + ], + 1..64, + ) +} + +/// Check every observation of `fan` against the `oracle` holding clones of +/// the same handles. +/// +/// Compared: length, emptiness, both iteration directions (radix sequence +/// and per-child identity), the values-only walk both ways, point lookups +/// across the whole alphabet, and the successor probe against the oracle's +/// range query. +fn equivalent(fan: &Fan<()>, oracle: &BTreeMap>) -> Result<(), TestCaseError> { + prop_assert_eq!(fan.len(), oracle.len()); + prop_assert_eq!(fan.is_empty(), oracle.is_empty()); + + for ((radix, child), (expected_radix, expected)) in fan.iter().zip(oracle.iter()) { + prop_assert_eq!(radix, *expected_radix); + prop_assert!(child.ptr_eq(expected)); + } + prop_assert_eq!(fan.iter().count(), oracle.len()); + + for ((radix, child), (expected_radix, expected)) in fan.iter().rev().zip(oracle.iter().rev()) { + prop_assert_eq!(radix, *expected_radix); + prop_assert!(child.ptr_eq(expected)); + } + + prop_assert_eq!(fan.values().len(), oracle.len()); + for (child, expected) in fan.values().zip(oracle.values()) { + prop_assert!(child.ptr_eq(expected)); + } + for (child, expected) in fan.values().rev().zip(oracle.values().rev()) { + prop_assert!(child.ptr_eq(expected)); + } + + for radix in u8::MIN..=u8::MAX { + match (fan.get(radix), oracle.get(&radix)) { + (Some(child), Some(expected)) => prop_assert!(child.ptr_eq(expected)), + (None, None) => {} + (ours, theirs) => prop_assert!( + false, + "get({}) diverged: fan {:?}, oracle {:?}", + radix, + ours.is_some(), + theirs.is_some(), + ), + } + match (fan.successor(radix), oracle.range(radix..).next()) { + (Some((at, child)), Some((expected_at, expected))) => { + prop_assert_eq!(at, *expected_at); + prop_assert!(child.ptr_eq(expected)); + } + (None, None) => {} + (ours, theirs) => prop_assert!( + false, + "successor({}) diverged: fan {:?}, oracle {:?}", + radix, + ours.map(|(at, _)| at), + theirs.map(|(at, _)| *at), + ), + } + } + Ok(()) +} + +proptest! { + /// After every step of any insert/remove sequence, a fan observes + /// identically to a `BTreeMap` fed the same operations. + /// + /// Same length, same ascending and descending iteration (radixes and + /// handle identities), same point lookups, same successor answers, and + /// the same displaced/removed handles returned from the mutations. + #[test] + fn fan_matches_btreemap_oracle(ops in arb_ops()) { + let mut fan = Fan::new(); + let mut oracle = BTreeMap::new(); + for op in ops { + match op { + Op::Insert(radix) => { + let node = child(); + let displaced = fan.insert(radix, node.clone()); + let expected = oracle.insert(radix, node); + prop_assert_eq!(displaced.is_some(), expected.is_some()); + if let (Some(displaced), Some(expected)) = (displaced, expected) { + prop_assert!(displaced.ptr_eq(&expected)); + } + } + Op::Remove(radix) => { + let removed = fan.remove(radix); + let expected = oracle.remove(&radix); + prop_assert_eq!(removed.is_some(), expected.is_some()); + if let (Some(removed), Some(expected)) = (removed, expected) { + prop_assert!(removed.ptr_eq(&expected)); + } + } + } + equivalent(&fan, &oracle)?; + } + } + + /// Any insert/remove sequence leaves the fan strictly ascending by + /// radix with no duplicates: the structural invariant the hash + /// preimage, the wire encoding, and the merge walks read without + /// re-sorting. + #[test] + fn ops_preserve_strict_ascent(ops in arb_ops()) { + let mut fan = Fan::new(); + for op in ops { + match op { + Op::Insert(radix) => { + fan.insert(radix, child()); + } + Op::Remove(radix) => { + fan.remove(radix); + } + } + } + let radixes: Vec = fan.iter().map(|(radix, _)| radix).collect(); + prop_assert!( + radixes.windows(2).all(|pair| pair[0] < pair[1]), + "fan radixes not strictly ascending: {:?}", + radixes, + ); + } + + /// `size_hint` stays exact under partial consumption from both ends, + /// for the borrowing and the consuming walks alike. + /// + /// After taking `j` from the front and `k` from the back of an + /// `n`-entry fan, both report exactly `n - j - k` remaining. + #[test] + fn size_hint_is_exact_from_both_ends( + radixes in proptest::collection::btree_set(any::(), 0..16), + splits in (0usize..=16, 0usize..=16), + ) { + let fan: Fan<()> = radixes.iter().map(|radix| (*radix, child())).collect(); + let n = fan.len(); + let (j, k) = splits; + let (j, k) = (j.min(n), k.min(n - j.min(n))); + + let mut iter = fan.iter(); + for _ in 0..j { + iter.next(); + } + for _ in 0..k { + iter.next_back(); + } + prop_assert_eq!(iter.size_hint(), (n - j - k, Some(n - j - k))); + prop_assert_eq!(iter.len(), n - j - k); + + let mut into_iter = fan.into_iter(); + for _ in 0..j { + into_iter.next(); + } + for _ in 0..k { + into_iter.next_back(); + } + prop_assert_eq!(into_iter.size_hint(), (n - j - k, Some(n - j - k))); + prop_assert_eq!(into_iter.len(), n - j - k); + } + + /// Collecting an arbitrary pair list — duplicates included, any order — + /// builds the same fan as inserting the pairs one by one into an empty + /// fan: later pairs displace earlier ones at the same radix. + #[test] + fn collect_agrees_with_sequential_insert(radixes in vec(any::(), 0..32)) { + let pairs: Vec<(u8, Node<()>)> = + radixes.into_iter().map(|radix| (radix, child())).collect(); + + let collected: Fan<()> = pairs.iter().map(|(radix, node)| (*radix, node.clone())).collect(); + let mut sequential = Fan::new(); + for (radix, node) in pairs { + sequential.insert(radix, node); + } + + prop_assert_eq!(collected.len(), sequential.len()); + for ((radix, child), (expected_radix, expected)) in + collected.iter().zip(sequential.iter()) + { + prop_assert_eq!(radix, expected_radix); + prop_assert!(child.ptr_eq(expected)); + } + } + + /// Consuming a fan and collecting it back reproduces the fan exactly, + /// entry identities included: the consuming walk hands out the very + /// handles the fan held, in order. + #[test] + fn into_iter_collect_round_trips(radixes in proptest::collection::btree_set(any::(), 0..16)) { + let fan: Fan<()> = radixes.iter().map(|radix| (*radix, child())).collect(); + let round_tripped: Fan<()> = fan.clone().into_iter().collect(); + + prop_assert_eq!(round_tripped.len(), fan.len()); + for ((radix, child), (expected_radix, expected)) in + round_tripped.iter().zip(fan.iter()) + { + prop_assert_eq!(radix, expected_radix); + prop_assert!(child.ptr_eq(expected)); + } + } + + /// Building a fan by ascending `push` yields the same fan as collecting + /// the same pairs: the O(1) append path and the general constructor + /// agree wherever both apply. + #[test] + fn push_agrees_with_collect(radixes in proptest::collection::btree_set(any::(), 0..16)) { + let pairs: Vec<(u8, Node<()>)> = + radixes.iter().map(|radix| (*radix, child())).collect(); + + let mut pushed = Fan::new(); + for (radix, node) in &pairs { + pushed.push(*radix, node.clone()); + } + let collected: Fan<()> = pairs.into_iter().collect(); + + prop_assert_eq!(pushed.len(), collected.len()); + for ((radix, child), (expected_radix, expected)) in + pushed.iter().zip(collected.iter()) + { + prop_assert_eq!(radix, expected_radix); + prop_assert!(child.ptr_eq(expected)); + } + } +} + +/// `unit` builds the one-entry fan: the entry is retrievable at its radix, +/// the length is one, and nothing else is present. +#[test] +fn unit_holds_exactly_one_child() { + let node = child(); + let fan = Fan::unit(7, node.clone()); + assert_eq!(fan.len(), 1); + assert!(fan.get(7).expect("unit child present").ptr_eq(&node)); + assert!(fan.get(8).is_none()); + assert_eq!(fan.iter().count(), 1); +} + +/// The fan occupies 40 bytes on 64-bit targets. +/// +/// An entry is 16 bytes (`(u8, Node)` padded to the 8-byte handle +/// alignment), so the inline form is 8 bytes of capacity plus +/// `16 × FAN_INLINE` of storage — the packed overlay of inline and spilled +/// forms that `smallvec`'s `union` feature provides (without it a +/// discriminant adds 8 bytes). The pin keeps inline-capacity growth a +/// deliberate decision — every node allocation pays for these bytes, +/// leaves included. +#[cfg(target_pointer_width = "64")] +#[test] +fn fan_is_forty_bytes() { + assert_eq!(std::mem::size_of::>(), 40); +} From 8f87ddd01480d59ecb0eff82b535c690ecf21797 Mon Sep 17 00:00:00 2001 From: finch Date: Wed, 29 Jul 2026 11:02:38 -0400 Subject: [PATCH 3/4] the swap: every radix fan is a Fan, and imbl departs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Children — typed shell and untyped branch alike — now hold the sorted inline Fan instead of imbl's OrdMap, and imbl leaves the dependency graph entirely (cargo tree -i imbl finds nothing). The motivation is jneem/imbl#161: OrdMap::diff walks clone-derived pairs wrong, and while the join already merge-walks the fans itself, a dependency whose core operation miscompares shared structure has no business under a convergence protocol. Wire bytes and hashes cannot move: both are pure functions of ascending radix iteration, which the fan guarantees structurally, and the insta snapshots ride through untouched. The owned walk's resume probe becomes Fan::successor (same O(log fan), still materializing no sibling handles); the explode of a path-compressed node becomes Fan::unit, building its transient singleton inline instead of allocating a map; from_sorted_leaves lays its groups down with O(1) ascending appends; Children::remove takes its radix by value (it is Copy). A size-budget test pins the per-node price this trades in — measured on 64-bit: Fan 40 B, Children 136 B, NodeInner 184 B — so future growth of the node allocation is a reviewed decision. The PhantomData H> height marker stays, re-justified on its own terms: it keeps auto-trait checks from walking the 32-level peano chain, whatever the fan holds. --- Cargo.lock | 65 --------------------------------- Cargo.toml | 1 - src/tree/traverse/act.rs | 2 +- src/tree/traverse/join.rs | 2 +- src/tree/typed/levels.rs | 4 +- src/tree/typed/node.rs | 48 ++++++++++++------------ src/tree/typed/untyped.rs | 35 +++++++++--------- src/tree/typed/untyped/iter.rs | 13 +++---- src/tree/typed/untyped/tests.rs | 41 +++++++++++++++------ 9 files changed, 80 insertions(+), 131 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b5dc890d..bf52210a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -165,12 +165,6 @@ dependencies = [ "rustversion", ] -[[package]] -name = "archery" -version = "1.2.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "70e0a5f99dfebb87bb342d0f53bb92c81842e100bbb915223e38349580e5441d" - [[package]] name = "arrayref" version = "0.3.9" @@ -384,12 +378,6 @@ version = "2.12.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "84d7ced0ae9557296835c32bf1b1e02b44c746701f898460fb000d7eaa84f00a" -[[package]] -name = "bitmaps" -version = "3.2.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a1d084b0137aaa901caf9f1e8b21daa6aa24d41cd806e111335541eff9683bd6" - [[package]] name = "bitvec" version = "1.0.1" @@ -2073,30 +2061,6 @@ dependencies = [ "xmltree", ] -[[package]] -name = "imbl" -version = "7.0.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e525189e5f603908d0c6e0d402cb5de9c4b2c8866151fabc4ebd771ed2630a2e" -dependencies = [ - "archery", - "bitmaps", - "imbl-sized-chunks", - "rand_core 0.9.5", - "rand_xoshiro", - "version_check", - "wide", -] - -[[package]] -name = "imbl-sized-chunks" -version = "0.1.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8f4241005618a62f8d57b2febd02510fb96e0137304728543dfc5fd6f052c22d" -dependencies = [ - "bitmaps", -] - [[package]] name = "indexmap" version = "2.14.0" @@ -3791,15 +3755,6 @@ dependencies = [ "rand_core 0.9.5", ] -[[package]] -name = "rand_xoshiro" -version = "0.7.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f703f4665700daf5512dcca5f43afa6af89f09db47fb56be587f80636bda2d41" -dependencies = [ - "rand_core 0.9.5", -] - [[package]] name = "ratatui" version = "0.30.1" @@ -4039,7 +3994,6 @@ dependencies = [ "futures", "futures-util", "hex", - "imbl", "insta", "itertools 0.14.0", "pollster", @@ -4183,15 +4137,6 @@ version = "1.0.23" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9774ba4a74de5f7b1c1451ed6cd5285a32eddb5cccb8cc655a4e50009e06477f" -[[package]] -name = "safe_arch" -version = "0.7.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "96b02de82ddbe1b636e6170c21be622223aea188ef2e139be0a5b219ec215323" -dependencies = [ - "bytemuck", -] - [[package]] name = "same-file" version = "1.0.6" @@ -5583,16 +5528,6 @@ dependencies = [ "wezterm-dynamic", ] -[[package]] -name = "wide" -version = "0.7.33" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0ce5da8ecb62bcd8ec8b7ea19f69a51275e91299be594ea5cc6ef7819e16cd03" -dependencies = [ - "bytemuck", - "safe_arch", -] - [[package]] name = "widestring" version = "1.2.1" diff --git a/Cargo.toml b/Cargo.toml index 4744a46a..8444be4d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -37,7 +37,6 @@ protocol-v1 = [] [dependencies] before = { path = "crates/before", features = ["borsh"] } -imbl = "7" bytes = "1" blake3 = "1.8" static_assertions = "1.1" diff --git a/src/tree/traverse/act.rs b/src/tree/traverse/act.rs index 1d7eb4a1..9d961df0 100644 --- a/src/tree/traverse/act.rs +++ b/src/tree/traverse/act.rs @@ -103,7 +103,7 @@ where .collect(); // Mutably pull the existing child out of the parent: - let existing_child = existing_children.remove(&radix); + let existing_child = existing_children.remove(radix); // Short-circuit when solely trying to delete from a non-existent child: if existing_child.is_none() diff --git a/src/tree/traverse/join.rs b/src/tree/traverse/join.rs index 6b9363da..01d911aa 100644 --- a/src/tree/traverse/join.rs +++ b/src/tree/traverse/join.rs @@ -152,7 +152,7 @@ where merged.insert(radix, child); } None => { - merged.remove(&radix); + merged.remove(radix); } } } diff --git a/src/tree/typed/levels.rs b/src/tree/typed/levels.rs index 1548a081..35429c6c 100644 --- a/src/tree/typed/levels.rs +++ b/src/tree/typed/levels.rs @@ -29,8 +29,8 @@ pub trait Levels: Default + Clone + sealed::Sealed { /// inductive case can coerce to `Pin>`. That /// coercion discharges the inner state machine's auto-trait check at each /// recursion site, terminating what would otherwise be a height-deep walk - /// through the `imbl` btree internals; the captured node values (containing - /// messages) must therefore be `Send + Sync`. + /// through every level's captured state; the captured node values + /// (containing messages) must therefore be `Send + Sync`. type Message: Send + Sync; /// The height of the bottom-most level. diff --git a/src/tree/typed/node.rs b/src/tree/typed/node.rs index db276156..5a959e99 100644 --- a/src/tree/typed/node.rs +++ b/src/tree/typed/node.rs @@ -1,7 +1,6 @@ use std::{fmt::Debug, iter::Map, marker::PhantomData}; use borsh::{BorshDeserialize, BorshSerialize}; -use imbl::OrdMap; use crate::{Version, message::Message}; @@ -10,23 +9,24 @@ use super::height::{self, Height, S, Z}; #[cfg(any(test, feature = "protocol-v1"))] use super::levels::{Top, levels}; use super::untyped; +use untyped::fan::{self, Fan}; /// The typed node with a height of 32; the root of the tree. pub type Root = Node; /// The radix-indexed children of a branch one level above height `H`: a -/// typed shell over the untyped persistent map, so inserts and removals -/// stay height-correct at compile time. +/// typed shell over the untyped radix fan, so inserts and removals stay +/// height-correct at compile time. pub struct Children { height: PhantomData H>, - inner: OrdMap>, + inner: Fan, } impl Default for Children { fn default() -> Self { Self { height: PhantomData, - inner: OrdMap::new(), + inner: Fan::new(), } } } @@ -41,14 +41,14 @@ impl Clone for Children { } impl Children { - fn from_untyped_map(inner: OrdMap>) -> Self { + fn from_fan(inner: Fan) -> Self { Self { height: PhantomData, inner, } } - fn into_untyped_map(self) -> OrdMap> { + fn into_fan(self) -> Fan { self.inner } @@ -70,7 +70,7 @@ impl Children { } /// Remove and return the child at `radix`, if any. - pub fn remove(&mut self, radix: &u8) -> Option> { + pub fn remove(&mut self, radix: u8) -> Option> { self.inner.remove(radix).map(Node::from_untyped) } @@ -83,13 +83,13 @@ impl Children { pub fn iter(&self) -> impl Iterator)> + '_ { self.inner .iter() - .map(|(radix, child)| (*radix, Node::from_untyped(child.clone()))) + .map(|(radix, child)| (radix, Node::from_untyped(child.clone()))) } } impl FromIterator<(u8, Node)> for Children { fn from_iter)>>(iter: I) -> Self { - Self::from_untyped_map( + Self::from_fan( iter.into_iter() .map(|(radix, child)| (radix, child.into_untyped())) .collect(), @@ -103,10 +103,7 @@ fn typed_child((radix, inner): (u8, untyped::Node)) -> (u8, Nod impl IntoIterator for Children { type Item = (u8, Node); - type IntoIter = Map< - > as IntoIterator>::IntoIter, - fn((u8, untyped::Node)) -> (u8, Node), - >; + type IntoIter = Map, fn((u8, untyped::Node)) -> (u8, Node)>; fn into_iter(self) -> Self::IntoIter { self.inner @@ -120,12 +117,12 @@ impl IntoIterator for Children { /// /// The height marker is held as `PhantomData H>` rather than /// `PhantomData`. Function pointers are unconditionally `Send + Sync`, -/// so the auto-trait check on `Node` does not descend into the -/// `S...>>>` peano-style height chain. Without this, the -/// `SharedPointer<...>: Send` obligation imposed by `imbl` (which -/// requires its contents to be `Sync`) recursively walks 32 levels of -/// `S<…>: Sync`, even though the type variable `H` itself is purely phantom -/// and never constructs anything that could fail to be `Send`/`Sync`. +/// so any auto-trait obligation on `Node` discharges without descending +/// the `S...>>>` peano-style height chain: a bare +/// `PhantomData` would send the trait solver walking 32 levels of +/// `S<…>: Sync` on every `Send`/`Sync` check, even though the type +/// variable `H` is purely phantom and never constructs anything that +/// could fail to be `Send`/`Sync`. #[repr(transparent)] pub struct Node { height: PhantomData H>, @@ -300,7 +297,7 @@ where pub fn branch(children: Children) -> Option { Some(Node { height: PhantomData, - inner: untyped::Node::branch(children.into_untyped_map())?, + inner: untyped::Node::branch(children.into_fan())?, }) } @@ -312,7 +309,7 @@ where Err(_) => unreachable!("typed nonzero-height node cannot be an uncompressed leaf"), }; - Children::from_untyped_map(children) + Children::from_fan(children) } /// Wrap `child` (at height `H`) beneath slot `index` of a virtual branch @@ -437,9 +434,10 @@ impl PartialEq for Node { // `prefix_len > 0` and reconstruct through [`Node::beneath`]. // // The branch decoder builds a typed [`Children`] through its safe `insert` -// API rather than transmuting an `OrdMap>`: `Node` carries no -// unsafe code, so the wire decoder stays within the same safe boundary as -// [`Node::branch`]. +// API rather than transmuting an untyped fan: `Node` carries no unsafe +// code, so the wire decoder stays within the same safe boundary as +// [`Node::branch`]. The wire's ascending radix order makes each insert an +// appending binary-search miss, so the rebuild costs no shifting. impl BorshSerialize for Node where diff --git a/src/tree/typed/untyped.rs b/src/tree/typed/untyped.rs index ee5d96ae..708afbb4 100644 --- a/src/tree/typed/untyped.rs +++ b/src/tree/typed/untyped.rs @@ -3,14 +3,13 @@ use std::mem; use std::sync::{Arc, OnceLock}; use borsh::BorshSerialize; -use imbl::OrdMap; use tinyvec::ArrayVec; use crate::{Version, message::Message, tree::typed::Hash}; -#[cfg_attr(not(test), allow(dead_code))] -mod fan; +pub mod fan; mod iter; +use fan::Fan; pub use iter::{Iter, Leaf, Range, RangeOwned}; /// One storage node — a leaf or a branch behind a shared `Arc`, carrying @@ -157,7 +156,7 @@ enum Children { /// prefix does. version_bytes: OnceLock, /// The children of this branch. - children: OrdMap>, + children: Fan, }, } @@ -202,12 +201,12 @@ impl Node { /// Construct a new branch node from a list of children with distinct /// indices (inverse to [`Node::into_children`]). - pub fn branch(children: OrdMap>) -> Option { + pub fn branch(children: Fan) -> Option { match children.len() { 0 => None, 1 => { let Some((index, node)) = children.into_iter().next() else { - unreachable!("a map with 1 element cannot fail to iterate"); + unreachable!("a fan with 1 element cannot fail to iterate"); }; Some(node.beneath(index)) } @@ -225,11 +224,11 @@ impl Node { } } - /// Convert a node into a map from child index to child node (inverse to + /// Convert a node into its radix fan of children (inverse to /// [`Node::branch`]). /// /// If `self` is a leaf node, returns `Err(self)`. - pub fn into_children(mut self) -> Result>, Node> { + pub fn into_children(mut self) -> Result, Node> { if !self.inner.prefix.is_empty() { // Path-compressed: pop the top (shallowest) byte and rewrap self // under it. Popping shortens the prefix, so the observable hash @@ -239,12 +238,12 @@ impl Node { let inner = Arc::make_mut(&mut self.inner); let index = inner.prefix.pop().expect("non-empty prefix"); inner.hash = OnceLock::new(); - Ok(OrdMap::from_iter([(index, self)])) + Ok(Fan::unit(index, self)) } else { match &self.inner.children { Children::Leaf { .. } => Err(self), Children::Branch { .. } => { - // Extract the children map; self is dropped, so leaving + // Extract the children fan; self is dropped, so leaving // its precomputed metadata referencing the now-vacated // branch is harmless. let inner = Arc::make_mut(&mut self.inner); @@ -315,7 +314,7 @@ impl Node { .expect("distinct 32-byte paths diverge before the bottom"); let count = leaves.len(); - let mut children = OrdMap::new(); + let mut children = Fan::new(); let mut rest = leaves; while let Some(radix) = rest.first().map(|(path, _)| path[branch_at]) { let split = rest @@ -323,7 +322,9 @@ impl Node { .position(|(path, _)| path[branch_at] != radix) .unwrap_or(rest.len()); let (group, tail) = mem::take(&mut rest).split_at_mut(split); - children.insert(radix, Self::from_sorted_leaves(branch_at + 1, group)); + // Groups peel off in ascending radix order, so each child is an + // O(1) append. + children.push(radix, Self::from_sorted_leaves(branch_at + 1, group)); rest = tail; } debug_assert!(children.len() >= 2, "a branch point separates >= 2 runs"); @@ -384,7 +385,7 @@ impl Node { } Children::Branch { children, .. } => { let (radix, rest) = path.split_first()?; - node = children.get(radix)?; + node = children.get(*radix)?; path = rest; } } @@ -474,7 +475,7 @@ impl Node { Children::Leaf { .. } => Hash::leaf(&prefix), Children::Branch { children, .. } => Hash::branch( &prefix, - children.iter().map(|(radix, child)| (*radix, child.hash())), + children.iter().map(|(radix, child)| (radix, child.hash())), ), } }) @@ -615,8 +616,8 @@ impl Node { /// 3. the body, dispatched on `children`: /// - [`Children::Leaf`]: `version: Version`, then `message: Message`; /// - [`Children::Branch`]: `count_minus_two: u8`, then for each - /// child (in canonical `OrdMap` key order): `radix: u8`, - /// `serialize_to(child)`. + /// child (in ascending radix order, structural in the fan): + /// `radix: u8`, `serialize_to(child)`. /// /// Leaf-vs-branch is **not** tagged on the wire: at the receiver, the /// typed height and the running `prefix_len` together name the body's @@ -652,7 +653,7 @@ impl Node { ) })?; count_minus_two.serialize(writer)?; - for (radix, child) in children { + for (radix, child) in children.iter() { radix.serialize(writer)?; child.serialize_to(writer)?; } diff --git a/src/tree/typed/untyped/iter.rs b/src/tree/typed/untyped/iter.rs index 8434f7b4..0855d38f 100644 --- a/src/tree/typed/untyped/iter.rs +++ b/src/tree/typed/untyped/iter.rs @@ -234,7 +234,7 @@ impl<'a, T, R: RangeBounds> Walk<'a, T, R> { if back { for (radix, child) in children.iter() { let mut child_path = path; - child_path.push(*radix); + child_path.push(radix); self.frames.push_back(Frame { node: child, path: child_path, @@ -244,7 +244,7 @@ impl<'a, T, R: RangeBounds> Walk<'a, T, R> { } else { for (radix, child) in children.iter().rev() { let mut child_path = path; - child_path.push(*radix); + child_path.push(radix); self.frames.push_front(Frame { node: child, path: child_path, @@ -536,14 +536,13 @@ impl> RangeOwned { None => loop { let level = self.spine.last_mut()?; let next_child = match &level.node.inner.children { - // Probe for the smallest not-yet-visited radix: an - // O(log fan-out) map lookup, so unvisited siblings + // Probe for the smallest not-yet-visited radix: one + // O(log fan-out) binary search, so unvisited siblings // are never enumerated or held. Children::Branch { children, .. } if level.next <= u8::MAX as u16 => { children - .range(level.next as u8..) - .next() - .map(|(radix, child)| (*radix, child.clone())) + .successor(level.next as u8) + .map(|(radix, child)| (radix, child.clone())) } Children::Branch { .. } => None, Children::Leaf { .. } => { diff --git a/src/tree/typed/untyped/tests.rs b/src/tree/typed/untyped/tests.rs index 19ddcf01..61cf0424 100644 --- a/src/tree/typed/untyped/tests.rs +++ b/src/tree/typed/untyped/tests.rs @@ -1,6 +1,5 @@ use std::collections::BTreeSet; -use imbl::OrdMap; use proptest::collection::{btree_set, vec}; use proptest::prelude::*; use proptest::test_runner::TestCaseError; @@ -8,7 +7,7 @@ use proptest::test_runner::TestCaseError; use crate::tree::arb::arb_version; use crate::{Version, message::Message}; -use super::Node; +use super::{Node, fan::Fan}; /// Upper bound on the depth of trees generated in property tests. /// @@ -80,7 +79,7 @@ fn arb_tree(depth: usize, budget: usize) -> BoxedStrategy> { (Just(indices), subtrees) }) .prop_map(|(indices, subtrees)| { - let children: OrdMap> = indices.into_iter().zip(subtrees).collect(); + let children: Fan<()> = indices.into_iter().zip(subtrees).collect(); Node::branch(children).expect("branch input has >= 1 child") }) .boxed() @@ -139,7 +138,7 @@ where Node::leaf(version, f(leaf)) } Ok(children) => { - let rebuilt: OrdMap> = children + let rebuilt: Fan<()> = children .into_iter() .map(|(k, v)| (k, rebuild_with(v, f))) .collect(); @@ -156,7 +155,7 @@ where /// one-child case is handled by `beneath`-collapse instead. #[test] fn empty_branch_is_none() { - let empty: OrdMap> = OrdMap::new(); + let empty: Fan<()> = Fan::new(); assert!(Node::branch(empty).is_none()); } @@ -257,7 +256,7 @@ proptest! { let mut wrapped = child; for &index in &indices { - wrapped = Node::branch(OrdMap::from_iter([(index, wrapped)])) + wrapped = Node::branch(Fan::unit(index, wrapped)) .expect("one-child branch is non-empty"); } @@ -285,11 +284,11 @@ proptest! { // Build the wrapped node by nesting singleton branches. let mut wrapped = child.clone(); for &index in &indices { - wrapped = Node::branch(OrdMap::from_iter([(index, wrapped)])) + wrapped = Node::branch(Fan::unit(index, wrapped)) .expect("one-child branch is non-empty"); } - // Pop the topmost byte. The returned map has exactly one entry + // Pop the topmost byte. The returned fan has exactly one entry // because `wrapped` was a singleton-branch chain; the entry's // key is the popped byte and its value is the same node with a // one-shorter prefix. @@ -298,16 +297,16 @@ proptest! { let (popped_byte, popped) = popped_children .iter() .next() - .map(|(k, v)| (*k, v.clone())) + .map(|(k, v)| (k, v.clone())) .expect("singleton"); - popped_children.remove(&popped_byte); + popped_children.remove(popped_byte); prop_assert_eq!(popped_byte, *indices.last().expect("non-empty indices")); // Build a reference node with the same children but the shortened // prefix from scratch. let mut reference = child; for &index in &indices[..indices.len() - 1] { - reference = Node::branch(OrdMap::from_iter([(index, reference)])) + reference = Node::branch(Fan::unit(index, reference)) .expect("one-child branch is non-empty"); } @@ -328,7 +327,7 @@ proptest! { child in (0..=MAX_TEST_DEPTH).prop_flat_map(|d| arb_tree(d, TREE_LEAF_BUDGET)), ) { let child_hash = child.hash(); - let wrapped = Node::branch(OrdMap::from_iter([(index, child)])) + let wrapped = Node::branch(Fan::unit(index, child)) .expect("one-child branch is non-empty"); prop_assert_ne!(wrapped.hash(), child_hash); @@ -551,3 +550,21 @@ fn small_tree_hash_matches_byte_literal_preimage() { assert_eq!(tree.hash(), super::Hash::of(&preimage)); } + +/// Growing the per-node allocation price must be a deliberate, reviewed +/// decision, never a silent regression. +/// +/// Every node allocation pays `NodeInner`'s full size: the `Children` +/// enum takes its largest variant, so leaves (the most numerous nodes) +/// carry the branch variant's width, fan included. +/// +/// Measured on 64-bit: `Fan<()>` = 40 (8 capacity + 2 inline 16-byte +/// entries), `Children<()>` = 136 (three memo cells, the leaf count, and +/// the fan), `NodeInner<()>` = 184 (prefix `Vec` + hash memo + children). +#[test] +#[cfg(target_pointer_width = "64")] +fn node_inner_stays_within_budget() { + assert!(std::mem::size_of::>() <= 40); + assert!(std::mem::size_of::>() <= 136); + assert!(std::mem::size_of::>() <= 184); +} From 4bbd6c5b39b1433849965a39357a5cde20e0b13d Mon Sep 17 00:00:00 2001 From: finch Date: Wed, 29 Jul 2026 11:15:31 -0400 Subject: [PATCH 4/4] the sweep: prose re-denominated to the fan Every sentence that named the retired map now states what is: sharing lives on the per-node Arc handles, ascending radix order is structural in the fan, and the join prunes pointer-or-hash-equal children in its lockstep merge-walk. The witness test in greeting.rs keeps its body byte-for-byte and its assertions untouched; its framing now pins the behavioral invariant directly (installing one's own causal past is a hash identity, so the join must pair clone-derived fans radix by radix and never shortcut across pointer-shared runs). That witness's original provenance: it was constructed to reach imbl's OrdMap::diff defect (https://github.com/jneem/imbl/issues/161, the cursor over-advancing after a pointer-shared run) at the gossip install, and it remains the regression tripwire for any future walk with the same shortcut shape. The dated allocator-profile record in design/streaming-latency-serialization.md keeps its original vocabulary: it is a measurement of the code as it stood, and provenance lives in design-doc records. --- design/streaming-latency-serialization.md | 8 +-- results/mirror-complexity.tex | 6 +-- src/tree.rs | 8 +-- src/tree/mirror/alternating/message.rs | 2 +- .../streaming/remote/proxy/tests/greeting.rs | 52 +++++++++---------- src/tree/typed/hash.rs | 10 ++-- 6 files changed, 45 insertions(+), 41 deletions(-) diff --git a/design/streaming-latency-serialization.md b/design/streaming-latency-serialization.md index b98a3c5c..9125ed0f 100644 --- a/design/streaming-latency-serialization.md +++ b/design/streaming-latency-serialization.md @@ -291,7 +291,7 @@ boundary; completion time is approximately so `K ≥ expected disputed scopes` recovers full pipelining, and any `K > 1` divides the latency term proportionally. Memory: worst case ~K·fan node handles per recursive boundary (handles, not nodes — -`imbl` structural sharing keeps the bytes shared), which is exactly +the per-node `Arc` sharing keeps the bytes shared), which is exactly the trade the assembly fan queue already makes at K = fan. A default of one full fan (256) is the natural unit: it collapses the pathology by 24×–30× here, costs at most one fan of `Query` values @@ -698,9 +698,9 @@ session holds its pre-session root alive from handshake to output buffered `Query` or `Resolution` points into is resident *anyway*; K× more handles pin K× more refcounts, not K× more tree. And no path in the walk deep-copies: `Backend::children` iterates `Arc` clones -out of the persistent map, and `Backend::parent` builds fresh spine -nodes *from* shared children. The imbl `OrdMap` diff machinery the -tree rests on prunes pointer-equal spans wholesale for the same +out of the radix fan, and `Backend::parent` builds fresh spine +nodes *from* shared children. The join's lockstep merge-walk prunes +children equal by pointer-or-hash before descending for the same reason. ### 6.2 What K does not buy (paid identically at K = 1) diff --git a/results/mirror-complexity.tex b/results/mirror-complexity.tex index ec409355..76053982 100644 --- a/results/mirror-complexity.tex +++ b/results/mirror-complexity.tex @@ -181,9 +181,9 @@ \section{The system, in one page} vectors (\code{partition.rs:}\allowbreak\code{100-115, 197-203}; \code{src/tree/typed/}\allowbreak\code{levels/level.rs:1-10}), so per-round work is linear in the -frontier and message sizes --- no hidden log factors beyond the \code{imbl} -\code{OrdMap} child maps (an $O(\log 256)$ constant per child operation, -absorbed below). +frontier and message sizes --- no hidden log factors beyond the sorted +child fans (an $O(\log 256)$-bounded probe and $O(256)$-bounded rebuild +per child operation, absorbed below). \textbf{Termination is in-band}: a side is done when its outgoing \code{requested} and \code{uncertain} are both empty diff --git a/src/tree.rs b/src/tree.rs index 9a73a2b8..e64849b1 100644 --- a/src/tree.rs +++ b/src/tree.rs @@ -34,9 +34,11 @@ //! //! # Memos and sharing //! -//! Nodes are persistent (`imbl::OrdMap` children behind `Arc`), so cloning -//! a tree — every [`Snapshot`](crate::Snapshot), every gossip session's -//! working copy — is O(1) and shares structure; mutation is copy-on-write. +//! Every node lives behind an `Arc` (its children a sorted radix fan of +//! further handles), so cloning a tree — every +//! [`Snapshot`](crate::Snapshot), every gossip session's working copy — is +//! O(1) and shares structure; mutation is copy-on-write along the touched +//! spine. //! Each branch lazily memoizes three pure functions of its subtree: the //! Merkle **hash** (mirror pruning), and the **ceiling** and **floor** of //! its leaves' versions. The version bounds power both deletion honoring diff --git a/src/tree/mirror/alternating/message.rs b/src/tree/mirror/alternating/message.rs index 2c252b67..2859d75e 100644 --- a/src/tree/mirror/alternating/message.rs +++ b/src/tree/mirror/alternating/message.rs @@ -46,7 +46,7 @@ //! Multi-child branches always carry at least two children; singletons //! appear on the wire only as `prefix_len > 0` and reconstruct through //! [`Node::beneath`](crate::tree::typed::Node::beneath). Branch radices -//! are required to be strictly ascending (matching the backing `OrdMap`'s +//! are required to be strictly ascending (matching the backing fan's //! canonical iteration order). //! //! ## The three channels diff --git a/src/tree/mirror/streaming/remote/proxy/tests/greeting.rs b/src/tree/mirror/streaming/remote/proxy/tests/greeting.rs index 8cc2c41a..da6bae8e 100644 --- a/src/tree/mirror/streaming/remote/proxy/tests/greeting.rs +++ b/src/tree/mirror/streaming/remote/proxy/tests/greeting.rs @@ -199,40 +199,40 @@ fn mixed_empty_and_populated_converges() { assert_eq!(hash_of(&left), expected); assert_eq!(hash_of(&right), expected); } -/// WITNESS (reachability of the `imbl` `OrdMap::diff` defect, -/// , at the gossip install): +/// WITNESS (the gossip install must merge-walk clone-derived fans): /// -/// Two honest, overlapping sessions at one peer silently delete a message -/// nobody redacted. +/// Two honest, overlapping sessions at one peer must not silently delete a +/// message nobody redacted. /// /// The shape: session S2 (with a peer converged at T0) forks at T0; equal /// handshake versions resolve without opening the descent, so S2's -/// reconciled root is the fork-time root handle itself — its children map -/// is the very `OrdMap` object M0. Meanwhile session S1 (with a peer that -/// redacted one message whose root radix `r_h` heads M0's second imbl -/// chunk) installs first: the install's `Tree::join` builds its merged map -/// as `ours.clone()` + `remove(r_h)` — a copy-on-write descendant M1 -/// sharing M0's first chunk. S2's install then joins M1 against M0. A -/// diff-backed walk over that clone-derived pair is exactly what imbl's -/// `OrdMap::diff` mishandles (issue #161: the cursor over-advances after a -/// pointer-shared run, swallowing the uncompared boundary pair and -/// desyncing onto *neighboring, unchanged* radixes, which version -/// dominance then redacts as phantom deletions). +/// reconciled root is the fork-time root handle itself — its children fan +/// is the very object M0. Meanwhile session S1 (with a peer that redacted +/// one message at root radix `r_h`) installs first: the install's +/// `Tree::join` builds its merged fan as `ours.clone()` + `remove(r_h)` — +/// a clone-derived sibling M1 sharing every remaining child handle with +/// M0. S2's install then joins M1 against M0: a wide pair, identical +/// except at `r_h`, whose equal children form pointer-shared run after +/// pointer-shared run. A shortcut walk that elides entries after a shared +/// run — anything less than pairing the two fans radix by radix — desyncs +/// onto *neighboring, unchanged* radixes, which version dominance then +/// redacts as phantom deletions. /// /// T0 is S2's causal past, so S2's install must be an identity on the /// tree: the assertion is total (post-install hash equals pre-install -/// hash), swept over the redaction of each leaf in turn so the -/// chunk-boundary radix is hit whichever radix heads the second chunk. -/// `Tree::join` therefore merge-walks the radix fans itself and never -/// hands imbl's diff a clone-derived pair; this witness holds the join to -/// that, and fails if a diff-backed walk ever returns. +/// hash), swept over the redaction of each leaf in turn so every radix +/// position plays the divergence point in some round. `Tree::join` +/// merge-walks the two radix fans in lockstep, pruning equal pairs by +/// pointer-or-hash before descending; this witness holds the join to +/// that, and fails if a shortcut walk over shared runs ever returns. #[test] fn overlapping_sessions_lose_innocent_leaf_after_honored_redaction() { use crate::tree::Action; - // T0: 25 unit messages. Hash-uniform keys give the root ~25 children, - // more than one imbl chunk (16), with every radix holding one leaf, so - // honoring one leaf's redaction empties its root radix. + // T0: 25 unit messages. Hash-uniform keys give the root a wide fan + // (~25 children), every radix holding one leaf, so honoring one leaf's + // redaction empties its root radix — and sweeping the redacted leaf + // puts shared runs on both sides of every divergence point. let p = nth_party(0); let mut t0 = Tree::new(); t0.act(&p, (0..25).map(|_| Action::Insert(Message::new(())))); @@ -245,7 +245,7 @@ fn overlapping_sessions_lose_innocent_leaf_after_honored_redaction() { let mut lost = Vec::new(); for k in &keys { // S1's counterparty: converged at T0, then redacted the leaf at - // `k` (a local act rebuilds its own maps afresh; the sharing that + // `k` (a local act rebuilds its own fans afresh; the sharing that // matters is created by our install below, not here). let mut twin = Tree { root: t0.root.clone(), @@ -254,8 +254,8 @@ fn overlapping_sessions_lose_innocent_leaf_after_honored_redaction() { // S1's session and install: reconcile T0 against the redacting // twin over the wire, then join the result into the live tree. - // Deletion honoring drops radix `r_h`: the live tree's root map is - // now a copy-on-write descendant of M0 missing one radix. + // Deletion honoring drops radix `r_h`: the live tree's root fan is + // now a clone-derived sibling of M0 missing one radix. let (s1_reconciled, _) = wire_reconcile(t0.root.clone(), twin.root.clone()); let mut live = Tree { root: t0.root.clone(), diff --git a/src/tree/typed/hash.rs b/src/tree/typed/hash.rs index 013a0c6b..4e80308f 100644 --- a/src/tree/typed/hash.rs +++ b/src/tree/typed/hash.rs @@ -156,7 +156,7 @@ impl Hash { // branch, and never slower at the hot small nodes (short prefix, // small fan), whose whole preimage fits one 64-byte block and costs // a single compression. `size_hint` sizes the buffer exactly for - // the `OrdMap`/array/empty callers (all exact). + // the fan/array/empty callers (all exact). let prefix_len = u8::try_from(prefix.len()).expect("a compressed span fits in one length byte"); let children = children.into_iter(); @@ -175,9 +175,11 @@ impl Hash { count = count .checked_add(1) .expect("branch fan-out is bounded by the 256-way radix"); - // The convention requires ascending radix order, but only the - // `OrdMap` caller guarantees it structurally: trip at the - // violation site rather than as a cross-peer hash desync. + // The convention requires ascending radix order. The fan caller + // guarantees it structurally (the fan's sorted invariant is + // private, and every constructor preserves it); for direct and + // test callers, trip at the violation site rather than as a + // cross-peer hash desync. debug_assert!( previous.is_none_or(|previous| previous < radix), "branch children must arrive in strictly ascending radix order",