Retire imbl: the radix fan becomes a hand-rolled sorted Fan - #7
Merged
Conversation
… maps 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 (jneem/imbl#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.
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<T> 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.
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<fn() -> 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.
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 (jneem/imbl#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.
Collaborator
Author
Bench A/B:
|
| bench | fan estimate | change (3-run mid) | spread |
|---|---|---|---|
| batch_insert/100 | 910.81 µs | −25.9% | [−25.9%, −24.7%] |
| batch_insert/10000 | 94.618 ms | −25.0% | [−25.9%, −24.8%] |
| batch_insert/1000000 | 11.213 s | −21.7% | [−22.4%, −21.6%] |
| iter/100 | 4.3744 µs | −17.9% | [−18.6%, −17.9%] |
| iter/10000 | 554.35 µs | −19.5% | [−19.7%, −19.5%] |
| iter/1000000 | 180.70 ms | −4.9% | [−8.4%, +4.7%] |
| redact/100 | 810.73 µs | −19.4% | [−21.0%, −18.3%] |
| redact/10000 | 124.98 ms | −25.3% | [−25.7%, −24.7%] |
| redact/1000000 | 14.510 s | −25.7% | [−26.0%, −25.5%] |
| range_delta/n=100,delta=1 | 10.103 µs | −9.3% | [−9.6%, −9.1%] |
| range_delta/n=100,delta=100 | 4.3570 µs | −22.2% | [−22.3%, −21.7%] |
| range_delta/n=10000,delta=1 | 36.201 µs | −8.4% | [−8.6%, −2.9%] |
| range_delta/n=10000,delta=100 | 436.85 µs | −10.0% | [−10.6%, −8.1%] |
| range_delta/n=10000,delta=10000 | 497.21 µs | −24.0% | [−25.0%, −23.3%] |
| range_delta/n=1000000,delta=1 | 61.340 µs | −18.4% | [−18.6%, −17.2%] |
| range_delta/n=1000000,delta=100 | 4.1098 ms | −46.7% | [−48.3%, −44.2%] |
| range_delta/n=1000000,delta=10000 | 83.872 ms | −7.6% | [−8.6%, +3.7%] |
| observer_replay/100 | 11.319 µs | −25.6% | [−33.5%, −25.5%] |
| observer_replay/10000 | 1.3263 ms | −22.1% | [−29.5%, −22.0%] |
| observer_replay/1000000 | 325.43 ms | −1.4% | [−5.5%, −0.8%] |
| observer_delta/n=100,delta=1 | 12.155 µs | −26.6% | [−26.6%, −26.3%] |
| observer_delta/n=100,delta=100 | 13.587 µs | −27.0% | [−27.1%, −17.2%] |
| observer_delta/n=10000,delta=1 | 42.347 µs | −24.2% | [−26.3%, −24.2%] |
| observer_delta/n=10000,delta=100 | 503.56 µs | −23.7% | [−25.0%, −23.7%] |
| observer_delta/n=10000,delta=10000 | 1.5566 ms | −23.6% | [−24.1%, −15.9%] |
| observer_delta/n=1000000,delta=1 | 69.490 µs | −34.9% | [−35.1%, −34.7%] |
| observer_delta/n=1000000,delta=100 | 5.3033 ms | −14.1% | [−16.2%, −13.9%] |
| observer_delta/n=1000000,delta=10000 | 79.714 ms | −17.6% | [−18.3%, −17.4%] |
| causal_replay/100 | 32.316 µs | −17.6% | [−20.2%, −13.1%] |
| causal_replay/10000 | 4.5316 ms | −15.0% | [−16.5%, −14.8%] |
| causal_replay/1000000 | 1.4567 s | +4.5% | [−4.0%, +7.2%] |
| causal_delta/n=100,delta=1 | 12.798 µs | −25.9% | [−26.1%, −25.4%] |
| causal_delta/n=100,delta=100 | 31.769 µs | −17.9% | [−21.5%, −17.8%] |
| causal_delta/n=10000,delta=1 | 44.350 µs | −25.4% | [−25.6%, −23.5%] |
| causal_delta/n=10000,delta=100 | 539.96 µs | −24.6% | [−24.6%, −23.1%] |
| causal_delta/n=10000,delta=10000 | 4.6310 ms | −17.4% | [−19.1%, −16.9%] |
| causal_delta/n=1000000,delta=1 | 69.870 µs | −35.2% | [−35.6%, −34.7%] |
| causal_delta/n=1000000,delta=100 | 4.5774 ms | −22.9% | [−31.0%, −19.0%] |
| causal_delta/n=1000000,delta=10000 | 78.482 ms | −12.7% | [−14.4%, −2.0%] |
| get/100 | 97.999 ns | +2.9% | [+2.4%, +4.0%] |
| get/10000 | 121.73 ns | −3.5% | [−3.5%, −3.4%] |
| get/1000000 | 140.90 ns | −6.2% | [−6.3%, −6.1%] |
The other three (1 run each)
- gossip_fixed (full V2 sessions, 100 series): improved essentially everywhere — steady-state sessions −4…−8% across bidir/unilateral × insertions/redactions at every corpus size, and the empty-session overhead points −33…−38% (the allocator-bound path where the per-virtual-level singleton allocation used to live). No series regressed.
- window_wallclock: 256KiB-1k +0.1% (no change), 256KiB-10k −0.3%, 16MiB-10k −6.1% (improved).
- branch_hash (sanity — its input was already a plain
Vec, no fan involved): all fan-outs ≥16 within ±1.2%; the 2/4-entry sub-µs cases wobbled +0.7…+6.9% with criterion verdicts "no change"/mixed — noise at the ~100–300 ns scale, as expected for an untouched code path.
Watch items (reported, neither trips the >5% consistent-regression threshold)
- get/100 +2.9% [+2.4%, +4.0%] — consistent across runs, ≈3 ns absolute at ~98 ns: binary search over a ~100-entry root fan pays a couple more comparisons than the B-tree's chunked walk at exactly this fan width; at 10k/1M it flips to −3.5%/−6.2% (deeper trees, smaller fans on the descent, cache-friendlier probes).
- causal_replay/1000000 [−4.0%, +7.2%] — the spread brackets zero; the two heavier-delta siblings (
causal_delta/n=1000000) improved −13…−35%, so this reads as run-to-run noise on a 1.5 s bench, not a shape problem.
Verdict. Exceeds the plan's "neutral-to-better" expectation: the operation mix that dominates production (batch insert, redact, session walks, full gossip sessions) improved 5–35% with zero significant regressions; the two watch items are nanosecond-scale or noise-bounded. No K/shape revisit is warranted.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
imbl 7.x carries an unpatched
OrdMap::diffcorrectness bug (jneem/imbl#161) that could silently corrupt synchronization; main'sf5426c9dalready evicteddifffrom the load-bearing path. This PR finishes the job: the radix fan does not need a persistent map at all — every fan clone in the crate is immediately followed by an O(fan) walk (the join's lockstep merge, the explode/reassemble paths, the wire decoder), so imbl's O(1) clone bought a ≤2× constant on already-O(fan) paths while charging a heavy dep stack (archery, bitmaps, wide, rand_xoshiro), the measured per-virtual-level singleton-map allocator hotspot (~45% of allocator self-time per profiled session), and theSharedPointer: Syncauto-trait contortions.The commits
f70f9559the fan —Fan<T>: a newtype over strictly-ascendingSmallVec<[(u8, Node<T>); 2]>(union layout, 40 B, pinned). K=2 makes the transient singleton explode and the modal 2-child interior fan allocation-free. Differential proptest suite against aBTreeMaporacle (op sequences; ptr_eq per child; successor vsrange(k..).next(); exactsize_hintunder two-ended partial consumption; last-winsFromIterator).8f87ddd0the swap —Children(typed and untyped) holds aFan; imbl leavesCargo.toml. Wire bytes and hashes are pure functions of ascending radix iteration, so byte-identity holds by construction: zero insta-snapshot churn, verified through the full gate.NodeInnermeasured at 184 B and budget-pinned (≤184/≤136/≤40).4bbd6c5bthe sweep — prose re-denominated to the fan (hash.rs's "only the OrdMap caller guarantees it structurally" caveat inverts into a type-level guarantee; the #161 witness test re-framed to the behavioral invariant it pins, body byte-identical; stale design-doc claim about diff machinery corrected). One deliberate residue: the dated allocator-profile measurement record indesign/streaming-latency-serialization.md, which names what it measured.