perf+robustness: eight verified hot-path fixes for large graphs (tour-gen O(n²)→O(n), single-parse, louvain crash, …)#490
Open
DashBot-0001 wants to merge 5 commits into
Conversation
All four preserve behavior byte-for-byte and are [executed]-verified. 1. merge-batch-graphs.py — precompile the project-prefix regex once at import instead of rebuilding+re-escaping the 24-alternative pattern string on every node. The regex cache keyed on the final string, so the compile was cached but the join+re.escape work was not. Verified: 69/69 unittest pass; 19.5x faster, 0 parity mismatches over 50k ids. 2. tour-generator.ts — replace O(n^2) `topoOrder.includes()` (per code node) with a Set, and the O(n) `queue.shift()` Kahn dequeue with a head cursor. Verified: tour-generator.test.ts green before+after; 81x faster at 20k nodes with byte-identical output (matters for the 15k-file repo, Egonex-AI#226). 3. layer-detector.ts — collapse two full passes over graph.nodes into one, deferring path-less file nodes to Core to preserve original ordering and Map key-insertion order. Verified: layer-detector.test.ts green before+after. 4. embedding-search.ts — hoist the query vector's magnitude out of the per-node cosine loop (it is invariant across a search). Same arithmetic order → bit-identical scores. Verified: embedding-search.test.ts green before+after. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
detectCommunities reassigns -1 community sentinels to ids past the current max via `Math.max(...Array.from(map.values()).filter(v => v >= 0), -1)`. Spreading every community id as call arguments throws `RangeError: Maximum call stack size exceeded` once the node count crosses the engine's argument limit — reachable on the ~3k+ node graphs this dashboard targets. Replace with a reducing loop: same result (verified byte-for-byte over 5000 fuzz cases + edge cases), no spread, and it also drops the throwaway filtered array allocation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…rsers `extract-structure.mjs` calls `analyzeFile` then `extractCallGraph` on every code file (lines 100 + 109) — two full tree-sitter parses of identical content, on the indexing hot path that runs on every `/understand`. - Add `TreeSitterPlugin.analyzeFileFull()` (and an optional `AnalyzerPlugin` interface method + `PluginRegistry` delegation) that parses once and runs both extractors on the same rootNode. Both extractors are pure functions of the tree, so output is byte-identical to the two separate calls. - `extract-structure.mjs` uses it when present and falls back to the original two-call path (preserving their independent error degradation) when the registry/plugin lacks it or it throws. - `getParser` now caches one reusable parser per language instead of allocating `new Parser()` + `setLanguage` and `delete()`-ing it on every call. Trees are still deleted per parse. Verified [executed]: - Project's own suites green before+after: tree-sitter-plugin, plugin-registry, parsers, tour-generator, layer-detector, embedding-search — 132/132. - Against the real tree-sitter TS grammar: analyzeFileFull.structure ≡ analyzeFile and analyzeFileFull.callGraph ≡ extractCallGraph (0 mismatches over many files and repeated reuse of the cached parser); single-parse is ~39% less parse work per code file. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`buildNonCodeBatches` re-materialized `[...byPath.keys()]` seven times and, for Group A (Dockerfile clusters) and Group D (SQL migrations), re-filtered the full path list once per directory — O(dirs·N). On a many-service monorepo (one Dockerfile per service) that was the dominant cost. Hoist the path list once and build a dir→paths index once; Group A/D lookups become O(1). Output is byte-for-byte identical (the path ordering, sorts and group boundaries are all preserved). Verified [executed]: byte-identical to the previous implementation on the `scan-result-non-code.json` fixture and over 300 fuzzed non-code repos (0 mismatches); 17–31× faster on 300–600-service synthetic monorepos (e.g. 4250 files: 105ms → 3.4ms). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`getEdgeCategory` linear-scanned every category's type array (and re-ran `Object.entries(EDGE_CATEGORY_MAP)`) for every edge, inside `filterEdges`. Build a reverse `edgeType → category` index once at module load; lookup is now O(1). First-category-wins order preserved. Verified [executed]: byte-identical to the scan over all known edge types plus unknowns (0 mismatches). Co-Authored-By: Claude Opus 4.8 (1M context) <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.
What
Eight small, behavior-preserving fixes on hot paths, motivated by large-graph scaling (the 15k-file repo in #226, many-service monorepos, and the ~3k-node dashboard target). Each is verified against the project's existing tests (green before and after) and the perf/robustness-sensitive ones are benchmarked with byte-identical output.
skills/understand/merge-batch-graphs.pyre.escape-ing the pattern string on every node.test_merge_batch_graphs.py69/69 pass; 19.5× faster, 0 parity mismatches over 50k node ids.packages/core/src/analyzer/tour-generator.tsO(n²)topoOrder.includes()(per code node) with aSet, and theO(n)queue.shift()Kahn dequeue with a head cursor.tour-generator.test.tsgreen; 81× faster at 20k nodes, byte-identical topo order.packages/core/src/analyzer/layer-detector.tsgraph.nodesinto one; path-less file nodes deferred toCoreto preserve ordering + Map key-insertion order.layer-detector.test.tsgreen.packages/core/src/embedding-search.tsembedding-search.test.tsgreen.packages/dashboard/src/utils/louvain.tsMath.max(...Array.from(map.values())…)with a reducing loop — the spread throwsRangeError: Maximum call stack size exceededat the ~3k+ node target.core/.../tree-sitter-plugin.ts,registry.ts,types.ts,skills/understand/extract-structure.mjsextract-structure.mjsparsed every code file twice (analyzeFilethenextractCallGraph). Add an optionalanalyzeFileFull()that parses once and runs both extractors on the same tree (caller falls back when absent). Also cache one reusable tree-sitter parser per language.tree-sitter-plugin/plugin-registry/parserssuites green before+after (132/132); against the real TS grammaranalyzeFileFull≡analyzeFile+extractCallGraph(0 mismatches) at ~39% less parse work/code file.skills/understand/compute-batches.mjsbuildNonCodeBatchesre-filtered the full path list once per Dockerfile/migration dir —O(dirs·N). Hoist the path list once + index paths by dir; Group A/D lookups becomeO(1).scan-result-non-code.jsonfixture + 300 fuzzed repos (0 mismatches); 17–31× faster on 300–600-service monorepos (4250 files: 105ms → 3.4ms).packages/dashboard/src/utils/filters.tsgetEdgeCategorylinear-scanned every category's type array per edge infilterEdges. Build a reverseedgeType → categoryindex once at module load; lookup is O(1).Why
tour-generator'sincludes()is the clearest asymptotic trap on a 15k-file repo. #6 halves tree-sitter parse work for code files on the indexing hot path; #7 is the same shape for non-code files on many-service monorepos (the microservices-demo case). #5 removes a real crash vector at the dashboard's stated scale. The rest are free — same outputs, less work, all on paths that run on every/understand, every search, and every layer/tour build.Safety
All eight preserve behavior exactly. #2/#4/#5/#6/#7 produce byte-identical output (verified by diffing old vs new over large/fuzzed inputs and against the real tree-sitter grammar); #1/#3 are covered by the existing suites. #6 is additive (
analyzeFileFullis optional, caller falls back). No breaking API changes.Related
dashboard/utils/layout.worker.tsis dead code. That's a bigger, behavior-changing fix (worker-ization + deterministic seeding) so it's tracked separately rather than bundled here.skills/understand-domain/extract-domain-context.py— the gitignore glob→regex is unanchored (buildmatchesmybuild/, over-ignoring); a correct fix needs proper segment-anchoring + a gitignore test suite, so it's left for a dedicated PR.