Skip to content

perf(docmodel): O(1) block lookups in isAncestor to cut hydrate CPU#859

Open
juligasa wants to merge 1 commit into
mainfrom
fix/hydrate-miss-cost
Open

perf(docmodel): O(1) block lookups in isAncestor to cut hydrate CPU#859
juligasa wants to merge 1 commit into
mainfrom
fix/hydrate-miss-cost

Conversation

@juligasa

Copy link
Copy Markdown
Collaborator

Problem

Document.Hydrate rebuilds block-tree state via treeOpSet.State() -> isAncestor, doing a comparator-heavy B-tree lookup per ancestor step. A production CPU profile of the hyper.media daemon (~330% CPU) showed ~57% of all CPU in that B-tree machinery (Less/hintsearch/nodeAscend). The #858 version cache reduced how often we hydrate but not the per-hydrate cost, so cache misses — unavoidable for "latest" reads of documents the daemon just synced — still saturate CPU.

Change

blockTreeState.blocks is point-lookup only (no ordered iteration), so swap it from *btree.Map[string, blockState] to a plain map[string]blockState. treeOpSets ordered maps (log, sublists) stay B-trees. No CRDT semantics change; ~12 call sites updated.

Results (RUN_HYDRATE_BENCH=1)

bench before after
HydrateDeep2000 320.6 ms 51.1 ms (6.3x)
HydrateFlat2000 2.22 ms 1.47 ms

Test plan

go test -race ./backend/api/documents/v3alpha/docmodel/ — pass.

Document.Hydrate rebuilds tree state via treeOpSet.State() -> isAncestor, which did a comparator-heavy B-tree lookup per ancestor step. In a production CPU profile of the hyper.media daemon ~57% of all CPU was in that B-tree machinery (Less/hintsearch/nodeAscend). The #858 version cache cut how often we hydrate but not the per-hydrate cost, so cache misses (latest reads of just-synced docs) still saturate CPU.

blockTreeState.blocks is point-lookup only (no ordered iteration), so switch it from *btree.Map[string,blockState] to a plain map[string]blockState. treeOpSet's ordered maps stay B-trees; no CRDT semantics change.

Deep-doc hydrate 320.6ms -> 51.1ms (6.3x); flat 2.22ms -> 1.47ms. go test (incl -race) passes.
@ericvicenti

ericvicenti commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

Claude Fable:

Heads-up: I've merged this into #860 and combined it with a correctness fix, so #860 is now a superset of this PR (your map/O(1) optimization plus a bound on the isAncestor walk).

The reason for combining: the production CPU fallover was actually an infinite loop — three documents contain a block that is its own parent (X.Parent == X), and isAncestor's unbounded for {} never terminates on them. The btree→map change here makes the loop faster but doesn't stop it, so on its own it wouldn't fix those docs (they'd keep pinning cores). The profile that shows btree ops hot in isAncestor is hot because of that infinite loop, which is what threw us both off.

Your optimization is great and kept in #860 as-is — this is purely additive. Suggest closing this in favor of #860 (details there: #860). Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants