From 9447f1f62664e829baff629da1c565ddceb7ea97 Mon Sep 17 00:00:00 2001 From: zzstoatzz Date: Mon, 1 Jun 2026 01:35:09 -0500 Subject: [PATCH] perf(repo): match Atmos MST lookup --- CHANGELOG.md | 2 +- devlog/012-mst-bench-notes.md | 27 +++++-- src/internal/repo/mst.zig | 136 ++++++++++++++++++---------------- 3 files changed, 94 insertions(+), 71 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 839d5d5..e9d04d9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ## 0.3.5 (planned) -- **perf**: align MST hot paths with Atmos while preserving AT Protocol root parity. The tree now uses nullable child pointers with CID-only lazy stubs, direct DAG-CBOR node serialization, cached clean-node CIDs, chunked key comparison, borrowed-key insertion for benchmark/import paths, and small-node linear lookup. The atproto-bench apples-to-apples MST run now has Zat ahead of Atmos on both insert+root and lookup while producing the same root bytes. +- **perf**: align MST hot paths with Atmos while preserving AT Protocol root parity. The tree now uses nullable child pointers with CID-only lazy stubs, direct DAG-CBOR node serialization, cached clean-node CIDs, chunked key comparison, borrowed-key insertion for benchmark/import paths, and Atmos-style ordered-tree lookup that loads only selected lazy child nodes. The atproto-bench apples-to-apples MST run now has Zat ahead of Atmos on both insert+root and lookup while producing the same root bytes. - **feat**: `Mst.collectBlocks` and `Mst.walk` add the missing middle layer for consumers that need commit-CAR MST blocks or ordered repo traversal without reaching through `Node` internals. This is additive public API; existing callers should not break. - **test**: ZDS main now consumes the new MST middle layer in record writes and repo import, pinned to Zat commit `485f1d485a9b8e7b703e8627a6b6a8c3e3c36a0e`, and is deployed as `atcr.io/zat.dev/zds:latest` from merged ZDS commit `8d1a8182dace`. - **docs**: devlog 012 captures the Atmos comparison, the MST representation changes, the lookup diagnosis, and the ZDS adoption proof. Ran `zig zen` during release prep; the relevant guidance here is "Communicate intent precisely", "Edge cases matter", "Avoid local maximums", and "Together we serve the users." diff --git a/devlog/012-mst-bench-notes.md b/devlog/012-mst-bench-notes.md index 9935547..98e9599 100644 --- a/devlog/012-mst-bench-notes.md +++ b/devlog/012-mst-bench-notes.md @@ -36,7 +36,8 @@ We tried that. `ChildRef` is gone from the hot structure: `Node.left` and `Entry - borrowed-key insertion for benchmark and caller-owned-key use cases - hot entry layout matching Atmos's `key/right/value` ordering - chunked MST key comparison instead of generic scalar byte ordering -- `getWithHeight` for callers that already know the key height +- `getWithHeight` kept as API compatibility for callers that already know the + key height, but lookup no longer needs the height - nullable child pointers instead of `ChildRef` union tags ## what moved @@ -55,20 +56,24 @@ The benchmark now reports one Zat path, not a menu of experiments: - cached node CIDs - direct MST node serialization - chunked key comparison -- precomputed lookup heights -- linear lookup scan through tiny MST nodes +- Atmos-style ordered-tree lookup through tiny MST nodes Latest cleaned run: | implementation | insert + root | lookup | |---|---:|---:| -| Zat | 5,499,189 records/sec | 6,782,217 lookups/sec | -| Atmos | 3,998,720 records/sec | 5,115,000 lookups/sec | +| Zat | 5,728,470 records/sec | 6,540,333 lookups/sec | +| Atmos | 4,147,900 records/sec | 5,512,120 lookups/sec | Both implementations produced the same root bytes: `01711220d59a82ffb8968ab6ff46354b382a18072f382240bc447c70e8cbc579221c8c2e` +Those rows are the median of three official `just bench-mst` runs after the +lookup path was changed to match Atmos semantically. Each run still uses one +warmup pass and five measured passes over the same deterministic 50k-record +corpus, with 500k lookups/pass. + ## why zat now wins insert Insert exercises ordered search plus mutation plus root computation. Zat now has three advantages there: @@ -99,7 +104,11 @@ Atmos lookup has: Zat had been using binary lower-bound for lookup because entries are sorted. That is asymptotically attractive and practically wrong here. With at most 32 entries per node, binary search pays for unpredictable midpoint branches and worse locality. Linear scan is more predictable, walks memory contiguously, and matches the practical Atmos shape. -After switching lookup to linear scan, Zat's selected lookup path moved ahead of Atmos in the apples-to-apples bench while keeping the same root bytes. +After switching lookup to an Atmos-style ordered walk, Zat's selected lookup +path moved ahead of Atmos in the apples-to-apples bench while keeping the same +root bytes. That walk does not use the key height to decide when to stop. It +compares the target key against entries in order, chooses `left` or the +previous entry's `right` child, and loads that selected child on the lazy path. Zat lookup now keeps: @@ -136,6 +145,12 @@ The likely direction is not "make Zig act like Go." It is to keep giving the hot One empirical note from the final cleanup: adding a per-entry key ownership bit made borrowed keys safer in the abstract but fattened the hot `Entry` layout and immediately showed up in lookup. The better match for this MST is arena/lifetime ownership: copied keys live with the tree, borrowed keys must outlive the tree, and delete removes logical entries without trying to reclaim per-key storage. +The last lookup cleanup removed the remaining height/layer branch from lookup +itself. That leaves height where the MST semantics need it, namely insertion, +deletion, splitting, and normalization. Plain lookup now matches Atmos: walk +the ordered tree by key and load child nodes only when that child is the chosen +path. + ## missing middle The first downstream adoption pass found the expected gap: ZDS was still doing diff --git a/src/internal/repo/mst.zig b/src/internal/repo/mst.zig index a85d71d..8f71f0a 100644 --- a/src/internal/repo/mst.zig +++ b/src/internal/repo/mst.zig @@ -244,18 +244,19 @@ pub const Mst = struct { /// look up a key, returning its CID value if present pub fn get(self: *const Mst, key: []const u8) ?cbor.Cid { - return findKey(self.root, self.root_layer orelse return null, key, keyHeight(key)); + return findKey(self.root, key); } /// look up a key when its MST height has already been computed. pub fn getWithHeight(self: *const Mst, key: []const u8, height: u32) ?cbor.Cid { - return findKey(self.root, self.root_layer orelse return null, key, height); + _ = height; + return findKey(self.root, key); } /// look up a key, resolving lazy stubs on demand. pub fn getLazy(self: *Mst, key: []const u8) !?cbor.Cid { try self.ensureRootLoaded(); - return try self.findKeyLazy(self.root, self.root_layer orelse return null, key, keyHeight(key)); + return try self.findKeyLazy(self.root, key); } fn entryLowerBound(entries: []const Node.Entry, key: []const u8) usize { @@ -276,80 +277,37 @@ pub const Mst = struct { return if (idx == 0) &node.left else &node.entries.items[idx - 1].right; } - fn childAtIndexConst(node: *const Node, idx: usize) ?*Node { - return if (idx == 0) node.left else node.entries.items[idx - 1].right; - } - - fn findKey(maybe_node: ?*Node, layer: u32, key: []const u8, height: u32) ?cbor.Cid { - const node = maybe_node orelse return null; - - if (height >= layer) { + fn findKey(root: ?*Node, key: []const u8) ?cbor.Cid { + var maybe_node = root; + while (maybe_node) |node| { + var next = node.left; for (node.entries.items) |entry| { switch (keyOrder(key, entry.key)) { - .lt => return null, + .lt => break, .eq => return entry.value, - .gt => {}, + .gt => next = entry.right, } } - return null; + maybe_node = next; } - - if (layer == 0) return null; - for (node.entries.items, 0..) |entry, i| { - switch (keyOrder(key, entry.key)) { - .lt => { - const child = if (i == 0) node.left else node.entries.items[i - 1].right; - return findKey(child, layer - 1, key, height); - }, - .eq => return entry.value, - .gt => {}, - } - } - - const child = if (node.entries.items.len > 0) - node.entries.items[node.entries.items.len - 1].right - else - node.left; - return findKey(child, layer - 1, key, height); + return null; } - fn findKeyLazy(self: *Mst, maybe_node: ?*Node, layer: u32, key: []const u8, height: u32) !?cbor.Cid { - const node = maybe_node orelse return null; - - if (height >= layer) { - for (node.entries.items) |entry| { + fn findKeyLazy(self: *Mst, root: ?*Node, key: []const u8) !?cbor.Cid { + var maybe_node = root; + while (maybe_node) |node| { + const loaded = try self.ensureNodeLoaded(node); + var child_ref: *?*Node = &loaded.left; + for (loaded.entries.items) |*entry| { switch (keyOrder(key, entry.key)) { - .lt => return null, + .lt => break, .eq => return entry.value, - .gt => {}, + .gt => child_ref = &entry.right, } } - return null; - } - - if (layer == 0) return null; - for (node.entries.items, 0..) |entry, i| { - switch (keyOrder(key, entry.key)) { - .lt => { - const child_ref = if (i == 0) &node.left else &node.entries.items[i - 1].right; - if (try self.ensureChildNode(child_ref)) |child| - return try self.findKeyLazy(child, layer - 1, key, height) - else - return null; - }, - .eq => return entry.value, - .gt => {}, - } + maybe_node = try self.ensureChildNode(child_ref); } - - const child_ref = if (node.entries.items.len > 0) - &node.entries.items[node.entries.items.len - 1].right - else - &node.left; - if (try self.ensureChildNode(child_ref)) |child| - return try self.findKeyLazy(child, layer - 1, key, height) - else - return null; + return null; } /// delete a key from the tree @@ -1245,6 +1203,56 @@ test "put and get" { try std.testing.expect(tree.get("nonexistent") == null); } +test "lookup walks ordered tree like lazy lookup" { + const alloc = std.testing.allocator; + var arena = std.heap.ArenaAllocator.init(alloc); + defer arena.deinit(); + const a = arena.allocator(); + + var tree = Mst.init(a); + const keys = [_][]const u8{ + "A0/374913", + "B1/986427", + "C0/451630", + "D2/269196", + "E0/670489", + "F1/085263", + "G0/765327", + "app.bsky.feed.post/9adeb165882c", + }; + + for (keys) |key| { + const value = try cbor.Cid.forDagCbor(a, key); + try tree.put(key, value); + } + try std.testing.expect((tree.root_layer orelse 0) > 0); + + var store = TestBlockStore{}; + const root_cid = try store.putNode(a, &tree, tree.root.?); + var lazy = try Mst.loadLazy(a, root_cid.raw, store.reader()); + + for (keys) |key| { + const expected = tree.get(key) orelse return error.NotFound; + const with_height = tree.getWithHeight(key, std.math.maxInt(u32)) orelse return error.NotFound; + const lazy_got = try lazy.getLazy(key) orelse return error.NotFound; + try std.testing.expectEqualSlices(u8, expected.raw, with_height.raw); + try std.testing.expectEqualSlices(u8, expected.raw, lazy_got.raw); + } + + const misses = [_][]const u8{ + "A0/000000", + "B1/999999", + "D2/269197", + "app.bsky.feed.post/000000000000", + "z", + }; + for (misses) |key| { + try std.testing.expect(tree.get(key) == null); + try std.testing.expect(tree.getWithHeight(key, 0) == null); + try std.testing.expect(try lazy.getLazy(key) == null); + } +} + test "put and delete" { const alloc = std.testing.allocator; var arena = std.heap.ArenaAllocator.init(alloc); -- 2.51.2