Repository navigation
fix(graph): a path search keeps one predecessor per node, so it stays linear - #3116
Merged
Merged
Conversation
… linear findPath's breadth-first search copied the whole path so far into every queue entry and dequeued with Array.shift(). A long path cost time and heap quadratic in its length, and a queued frontier held one copy per entry. The search now keeps one predecessor per discovered node in a map, walks a queue of ids with a head index, and builds the path once at the end. It keeps the enqueue-once set from PR #2016. Targets are fetched once each even when parallel edges point at the same node. Red, green and A/B: - Red on main: a 400-hop chain walks 81,001 array elements, against under 4,000 now, and a fan-in batch asks for the same node twice. No timing in either test. - Same paths: 400 seeded pairs each on excalidraw, gin and flask gave the same node and edge sequence on both arms (1,200 of 1,200). - A 20,000-hop chain: 1,007 ms and 47 MB of heap on main, 30 ms and 13 MB here. The test added in PR #2016 now counts node lookups per id instead of patching Array.prototype.push, because the queue no longer holds objects with a path. Its expectation is unchanged. Original PR #1583 by @danusha2345. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
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.
The shortest-path search stops copying the whole path into every queue entry, so a long path or a dense graph no longer costs quadratic time and heap. This is the path part of #1583, split out as its own commit.
Root cause
findPathkept a full copy of the path so far in each queue entry and dequeued withArray.shift(). A chain of N hops copied 1 + 2 + ... + N elements.Fix (from the PR)
Each discovered node keeps one predecessor in a map. The queue holds ids with a head index, and the path is built once at the end. The enqueue-once set from #2016 stays. Targets are fetched once each even when parallel edges point at the same node.
What we changed
The test from #2016 now counts node lookups per id instead of patching
Array.prototype.push, because the queue no longer holds objects with a path. Its expectation is unchanged. That change is the contributor's.Validation
findPathbacks the library'sfindPathonly, socodegraph_exploreand its budgets are not touched.CODEGRAPH_KERNEL_EXPECT=1, 4 workers): 840 files, 9,883 passed, 0 failed, 87 skipped. The rebase brought in nothing that shares a file with this commit besidesCHANGELOG.md;graph.test.tspasses 40 of 40 on this head.Lands #1583 (path part) by @danusha2345.
🤖 Generated with Claude Code