Skip to content

perf(storage): check node visibility once per node group in recursive joins - #1565

Draft
zahariash wants to merge 1 commit into
LadybugDB:mainfrom
zahariash:perf/node-visibility-cache
Draft

zahariash wants to merge 1 commit into
LadybugDB:mainfrom
zahariash:perf/node-visibility-cache

Conversation

@zahariash

@zahariash zahariash commented Nov 11, 2026 •

Copy link
Copy Markdown
Contributor

This work was produced with the help of language models.

Part of #1457.

Recursive joins on SF1 ran slower with 16 threads than with one. LDBC IC1, at its core MATCH (p:Person {ID: 1131}), (friend:Person {firstName: 'Jose'}) MATCH path = (p)-[:knows* SHORTEST 1..3]-(friend), took 54 to 60 ms at one thread and 62 to 70 ms at 16, and IC13 27 and 49 ms. Since #459 the neighbour scan calls NodeTable::isVisible for every neighbour, which takes the node group collection's mutex twice and the node group's chunked-group mutex once, and every knows neighbour in SF1 is in the same Person node group.

Fix. The scan keeps a NodeVisibilityCache. For each node group it asks once, under the group's lock, how many leading rows the transaction sees, counting whole vectors from the version info, and answers offsets inside that run without a lock. Offsets past the run and the transaction's own inserts still go through isVisible, and a group that another transaction's COPY appended and rolled back is looked up again and found missing. Counts for the first 16 node groups are kept inline, so scans of tables up to 2M nodes allocate nothing more.

Each neighbour scan, which the GDS algorithms and the vector index also use, makes its own cache, as do the source loops of RecursiveExtend and GroupedReachableCount. A count stays right while the transaction deletes no row of the table, which none of these callers does while its scan is open. Plans do not change. A row the transaction cannot see, such as a deleted node, whose marker survives checkpoints, or another transaction's uncommitted insert, ends the counted run, and the rest of that node group takes the locked path, as on main.

Medians of 9 runs in two rounds (four for IC1 at one thread, the noisiest case), wheel-like Release builds, with instructions per execution at one thread:

query threads main, ms this PR, ms instructions
IC1 1 60.3 / 53.6 / 57.1 / 58.5 51.1 / 56.4 / 53.1 / 55.1 −14%
IC1 16 70.1 / 62.0 41.7 / 45.4
IC13 1 27.5 / 26.9 18.2 / 18.9 −43%
IC13 16 48.5 / 49.4 8.1 / 8.6
IC14 1 218.0 / 222.4 221.2 / 205.1 −10%
IC14 16 142.9 / 127.6 93.4 / 84.8
complex14 q1 16 65.6 / 63.8 29.2 / 28.5 −34%

IC12 runs 5% fewer instructions and complex14 q13 6% fewer. Sixteen other SF1 queries (IC2, IC4, IC5, IC7, complex14 q3, q4 and q7, and nine of the 30-query suite) stay within 0.6% of main's instructions, or 0.5 million for those under 30 million. All of them make the same malloc calls as on main, and all 58 SF1 queries return the same rows, up to order among ties and inside collect().

On a generated graph of two components with 1M nodes and 5M edges each, a SHORTEST search between them, which reads all of one component, takes 319 / 327 ms at 16 threads against 1,302 / 1,258 on main, and 1,561 / 1,617 against 1,748 / 1,896 at one thread (medians of 5 in two rounds, one process per query and build). On a 1000×1000 grid, whose frontiers stay small, searches stay within 3%.

Tests. node_visibility.test deletes one of 3,000 checkpointed nodes and inserts two in an open transaction, where variable-length, SHORTEST and *0..1 queries skip the deleted node and reach the new ones while other connections see neither until commit. It also checks a read-only snapshot taken before a committed delete, and uncheckpointed rows grouped by an end-node property, which runs GroupedReachableCount. The file passes on main unchanged: it pins the visibility rules the cache must keep. Two VersionInfoScanTest cases check where the counted run ends, and NodeVisibilityCacheTest checks that offsets in node groups removed by a rolled-back COPY are not visible. Full ctest passes, 3,448 tests.

OnDiskGraphNbrScanState in graph/on_disk_graph.h changes layout. The fts and vector extensions include that header but use the state only through NbrScanState, so this targets 0.23.0.

… joins

Since LadybugDB#459 the neighbour scan of a recursive join calls NodeTable::isVisible
for every neighbour, and each call takes the node group collection's mutex
twice and the node group's chunked-group mutex once. LDBC IC13 on SF1 reads
about 361k adjacency entries that all point into one Person node group, so
16 threads contend on the same two mutexes and run slower than one.

The scan now keeps a NodeVisibilityCache. For each node group it asks once,
under the group's lock, how many leading rows the transaction sees, counted
in whole vectors from the version info, and answers offsets in that run
without a lock. Other offsets, and the transaction's own inserts, still go
through isVisible. A group that a rolled-back COPY removed after the cache
was made is looked up with a bounds check and treated as empty. Counts for
the first 16 node groups are stored inline, so most scans allocate nothing.

The counts stay right while the transaction deletes no row of the table.
Each neighbour scan makes its own cache, and so do the source loops of
RecursiveExtend and GroupedReachableCount. The neighbour scan also serves
the GDS algorithms and the vector index's search and insert, none of which
deletes while its scan is open.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@zahariash
zahariash marked this pull request as draft November 11, 2026 08:07

This branch has not been deployed

No deployments
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.

1 participant