mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-05 02:43:32 +00:00
4 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Gergő Magyar
|
96132bd13a
|
perf(scope-resolution): stop re-scanning the ParsedFile store once per language (#3211)
* perf(scope-resolution): stop re-scanning the ParsedFile store once per language Scope resolution calls `loadParsedFilesForPaths` once per language, and every call walks every shard in the store. The skip decision needs the envelope's path listing, and that listing is only trustworthy after the payload digest has been checked -- so a pass that wants 50 Python files still opens and SHA-256s all 413 shards / 301MB of a TypeScript-dominated store to prove it can skip them. A pass wanting a SINGLE file costs 335ms. The cost scales with language count, not with the files that language has, so a polyglot repo pays it worst. `tryLoadV8Cache` now returns the listing it already parsed for that skip decision, and the store memoizes it per run. Later passes skip on the memoized listing without reopening the file. Measured on a 2234-file, 3-language repo, min-of-5: before python 411ms typescript 2872ms javascript 507ms = 3834ms after python 415ms typescript 2805ms javascript 248ms = 3484ms -350ms here, roughly -250ms per additional language elsewhere. The first pass is unchanged by construction -- it is what populates the memo. End-to-end the graph is byte-identical: 51,288 nodes / 163,094 edges / 2106 clusters / 759 flows on a true incremental run. Keyed on size+mtime as well as name. Shard names are content-addressed, so a name collision across different content should be impossible, but that invariant lives in the parse-cache keying rather than here and one stat per shard is a few ms against the hundreds this saves. The memo holds one store directory at a time, so a new repo in a long-lived MCP process drops the previous set instead of accumulating. The failure mode a listing memo introduces is a FALSE SKIP: a pass concludes a shard holds nothing it wants and those files silently never reach the graph -- an exit-0 wrong answer, not a crash. The new test walks four passes with disjoint wants over one store, plus a shard written after the memo is warm; it fails when the skip is forced. Also records the full scopeResolution breakdown in bench/. The headline is that `emit` is 7161ms of the 14.7s phase and ~21% of the edit loop, spread across a fan of passes with no hot inner loop -- so the win there is not running them for unchanged files, which is a design rather than a patch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Address PR review feedback (#3211) Strengthen the shard-listing memo test so a later miss asserts fs.open and v8.deserialize never run for the skipped shard. Key-set checks alone still passed if the memo never skipped. Co-authored-by: Cursor <cursoragent@cursor.com> * chore(autofix): apply prettier + eslint fixes via /autofix command --------- Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> |
||
|
Gergő Magyar
|
0d1aed942f
|
docs(bench): close FTS as an optimization target with measured evidence (#3209)
PR #3208 landed two claims that further measurement disproved. The narrowing is not inconsistent. A true incremental leaf edit rebuilds exactly 8 of the 20 configured indexes -- the tables the writeback DMLs -- and the 20-index run I compared it against was a forced full rebuild (runner-identity trap). Per-index costs for those 8 sum to 7769ms against the 7525ms measured in-analyze. There is nothing to fix in `touchedFts`. The 845ms was not `import('./platform/capabilities.js')`. The CLI already imports that module statically; a cached dynamic import measures 0.035ms. The `await` is the first yield after the native FTS build and absorbs the libuv work still queued behind it. Recorded as a fourth measurement trap, since it invalidates any mark placed on an await that follows native work. What replaces them is a floor, established by probing a copy of the corpus index directly: - narrowing further: nothing left, the 8 tables are exactly the DML'd set - concurrent builds: hard error, one write transaction at a time - connection thread count: flat at 4/8/16/24 (7298/7133/7109/7345ms min-of-3), though the default burns ~60% more CPU for it - dropping `content` from File: 3541ms -> 241ms, but that deletes full-file keyword search (#2317/#2323); capping is a bad trade because the size distribution is flat The one lever left is overlap: the build runs on a libuv thread and hides behind main-thread JS (3337ms for the index plus a 3000ms JS burn, against ~6859ms serial). File rows are `{ name, filePath }` from `processStructure` with content lazy-read at CSV time, so they are known before parsing. What blocks it is that the DB is closed for the whole pipeline and that an early write moves `liveIndexMutationStarted` ahead of it. The `ponytail:` comment in `createSearchFTSIndexes` invited exactly the fix that cannot work -- every caller has already dropped the indexes it passes, so a presence gate would never fire. Replaced with the measured reason. Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Gergő Magyar
|
eba42d2994
|
docs(bench): correct the edit-loop numbers and add the FTS per-index breakdown (#3208)
The numbers this file shipped with were full rebuilds labelled as incremental. Rebuilding or re-copying `dist` between runs changes the analyzer runner identity, and the tool then forces a full rebuild — every measurement taken that way is a full run wearing an incremental label. That is the third "silently fall back to full work" guard in this pipeline, after the non-git corpus and the escalation gate, so the method section now says to read the banner on every run. Corrected, measured on a leaf file with a stable runner identity so the incremental path is genuinely taken: 31.7s, not 36.5s. The graph write is a 3,980-node subgraph rather than the full 51,288, and parse is ~9% of the loop. Adds the per-index FTS breakdown, which is the actionable finding: 10.2s across 20 indexes, of which File.file_fts alone is 3.5s because File nodes carry file content and that index re-tokenizes ~30MB of source to reflect one changed row. Also records that a two-importer leaf edit rebuilt 20 indexes while another run rebuilt 8 — `touchedFts` narrowing is at least inconsistent, and it has a withdrawal path when the index catalog cannot be read. That needs pinning before anyone optimizes against the narrowed set. Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Gergő Magyar
|
f48bf81256
|
perf(parse): tighten the dispatch-round memory bound and unclamp the worker-pool override (#3200)
* docs(parse): record why dispatchGroups is a required interface member Review finding #10 argued dispatchGroups should be optional to match `getQuarantinedPaths?` / `getStats?`. Those are compatibility accommodation for WorkerPool shapes that predate them, not a convention for new members; optional here would force a `?.` plus an unreachable fallback at the single production call site. Documenting the decision so the next reader does not re-litigate it from the neighbouring optional markers. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> (cherry picked from commit addaab647377f3c4553f752fa3ca1388bcb9ca81) * refactor(parse): simplify round accounting and dispatch setup Simplification pass over the dispatch-rounds change. Behavior preserved: identical graph on a full analyze (51,286 nodes / 163,092 edges). - Drop `roundMissBytes`. `roundBufferedBytes` counts the same bytes plus the cache hits, so it is always the greater of the two and the first disjunct of the close condition could never fire on its own. One counter, one reset, one check. - Measure round bytes with `Buffer.byteLength(content, 'utf8')` instead of `String.length`. UTF-16 code units undercount non-ASCII source by up to 3x, so the cap meant to bound main-thread retention was letting a CJK-heavy repo hold well past its nominal budget. Matches `estimateItemBytes` in the pool. - Reset the durable ParsedFile directories for a round's chunks concurrently. Each targets its own chunk-hash directory, and running them serially put N round trips of fs work on the critical path the round exists to shorten. The try/catch stays inside the mapped callback, so one failure still degrades that chunk alone. - Skip the quarantine filter entirely when nothing is quarantined, which is every run without a worker death. It was an identity copy of every group. - `dispatchChunkParseRound` takes `DispatchGroup<...>` rather than re-declaring that shape inline; the type was already imported and used in its body. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> (cherry picked from commit 527d5b6e0ca8ae7bbc6a414c5ac7e27fd85e9995) * refactor(parse): count round misses with the same idiom startRound uses `drainRound` hand-rolled a reduce to count 'miss' entries while `startRound`, one function above, filters the same predicate over the same union. Same integer, one idiom. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> (cherry picked from commit 7eaa193b0cb5fa515844f36ae1401d6fb2fed7b8) * fix(parse): honor GITNEXUS_WORKER_POOL_SIZE above the auto sizing cap The auto pool size is bounded by source bytes so a tiny repo does not spawn a full idle pool. That bound was also clamping the operator's env override, because the env value is read inside `resolveAutoPoolSize()` and the result went through `Math.min(..., workProportionalCap)`. `DEFAULT_POOL_SIZE_CAP`'s own comment offers `GITNEXUS_WORKER_POOL_SIZE` and `--workers <N>` as equivalent escape hatches for operators on bigger machines. They were not. Measured on a 30MB corpus, where the byte-derived cap is 16: --workers 24 -> pool: 24/24 active GITNEXUS_WORKER_POOL_SIZE=24 -> pool: 16/16 active (silently ignored) Both are deliberate operator input, so both now bypass the work-proportional cap, which goes back to bounding only the auto default. After the fix, on the same corpus, with identical graph output (51,286 nodes / 163,092 edges): GITNEXUS_WORKER_POOL_SIZE=24 -> pool: 24/24 active GITNEXUS_WORKER_POOL_SIZE=4 -> pool: 4/4 active unset -> pool: 16/16 active Verified by hand against the pool's own throughput log; not covered by an automated regression test, since the pool size is only observable through that log line and not through the progress stream a test can read. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> (cherry picked from commit 17ed08608c878079b2927da25cfd39c1608a02a2) * fix(parse): bound the durable-reset fan-out and pin the pool-size override Review follow-ups on #3200. The round's durable ParsedFile directory resets went out as one unbounded `Promise.all` — one recursive rm + mkdir per miss chunk, all at once. A round can hold hundreds of small packs, and those resets compete for descriptors with the chunk prefetch this loop already has in flight. `readFileContents` degrades a losing read SILENTLY by documented contract, so a dropped file would vanish from the chunk, from the graph, and from the chunk hash — shipping a narrowed index with exit 0. Now routed through `mapConcurrent` at the same width the file reads use, which keeps the pipelining win and caps in-flight descriptors. An operator's pool size is now also bounded by the number of files there are to parse, so `GITNEXUS_WORKER_POOL_SIZE=100000` on a five-file repo cannot become the literal thread count. This applies to `--workers` and the env var alike, so the parity the previous commit established is intact. It does NOT shrink an incremental re-analyze: `totalParseable` counts every parseable file in the scan, not the changed ones. Adds the regression test a reviewer asked for. The existing coverage (`worker-pool-resilience` calling `resolveAutoPoolSize` directly, `analyze-worker-pool-size` mocking `runFullAnalysis`) never reaches `runChunkedParseAndResolve`'s `effectivePoolSize`, so both stayed green through a revert of the fix. The new test drives the real parse phase with a worker double that writes a per-`threadId` marker, and counts them: verified it fails on the reverted line with `expected [ 'worker-1' ] to have a length of 3 but got 1`, and passes on HEAD. Also corrects the `GITNEXUS_PARSE_ROUND_BYTES` docstring, which still described the cache-miss counter deleted two commits ago. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(parse): skip caching a chunk with a stale durable generation; warn on over-subscription Closes the two findings left open by the review of #3200. When `prepareDurableParsedFileChunk` fails, the previous generation's shards are still on disk, so a later warm hit would union them with the new ones. The chunk is now recorded and its parse-cache write skipped -- the same posture `finalizeWorkerChunk` already takes for a quarantined chunk, and for the same reason: do not cache what we cannot vouch for. The next run re-dispatches into a directory it can actually clear. Bounding the reset fan-out removed the correlated trigger; this closes the individual case. Pool size over-subscription now warns rather than caps. Silently capping is precisely what the override exists to prevent, so an operator's number is still honored -- but an exported GITNEXUS_WORKER_POOL_SIZE applies to every analyze in a long-lived caller (watch auto-sync, the MCP server), including small incremental ones, and that is easy to set once and forget. The warning names the host's usable core count, so it is a hardware fact rather than an invented threshold. `resolveHostParallelism` is extracted from `resolveAutoPoolSize` rather than re-deriving the cgroup-aware fallback at the new call site. Tests: the stale-generation skip is pinned by a new case asserting nothing is written under any key; verified it fails without the guard with `expected 1 to be +0`. 60 unit and 49 integration tests pass across the affected suites. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(parse): guard dispatch-round cadence with a bench, not a wall-clock budget Round boundaries are deliberately invisible to graph output — batching that changed output would be a bug — so nothing in the repo could see the #3196 win regress. It would have come back as a silent ~1.5x on every cold analyze. Two earlier attempts to pin it as a unit test failed for that exact reason: one scraped a logger line the progress stream does not carry, the other asserted graph content that is identical either way. Extracts the round-close fold into `createRoundBudget`, so the decision is a shared unit the bench measures rather than a copy that drifts. The parse loop is streaming and cannot know chunk sizes up front, so an accumulator is the honest shape — not a planner. Four deterministic arms, one ratio, no millisecond gate: - layout_fingerprint — pack membership. Every cache key derives from it, so drift needs a SCHEMA_BUMP, never a lone re-baseline. - packs / single_file_packs — the FLOOR. `rounds` only asserts something while the corpus over-splits (774 packs where the byte budget needs 5). This is bench/import-target's lesson, where four heap arms read 0 B and passed every ceiling: a ceiling says "not too big", nothing said "still measuring". - rounds — the regression signal, both directions. - cjk_rounds vs ascii_rounds — pins UTF-8 byte accounting. The two corpora share a UTF-16 length and differ only in encoded size, so String.length collapses them to equal. This is the arm no unit test could be. - pack_scaling_ratio — (t_4n/t_n)/4, min-of-15. A ratio because wall-clock is runner-speed-dependent and this repo has the scar: callable-value-flow's ms gate failed twice at 2.07 and 1.975 against 1.9 with correct code, on a sub-11ms measurement. Every arm verified to fail before being recorded: close-every-chunk reads 774 rounds, disabling the close reads 1, reverting roundFileBytes to String.length takes cjk_rounds 8 -> 3, and shrinking the corpus trips the shape floor. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(bench): record the analyze phase breakdown and the rejected optimizations Where analyze time actually goes, measured while landing #3194/#3196/#3200, plus the two optimizations that looked compelling and were measured away. The headline is that the parse work is done: a one-file-edit re-analyze is 36.5s, of which parse is 2.8s (8%). scopeResolution is 40% and the unlogged graph emit + FTS rebuild is 49% — neither is incremental, and the ~18s sits outside the phase runner so every phase log is blind to it. Also records the trap that invalidated an earlier measurement: a non-git corpus never records a schema fingerprint, so every run is a forced rebuild and any "warm" number taken that way is fiction. Rejected, with numbers: more workers (16/20/24 land inside run-to-run spread) and bundling the worker entry (~250ms on a normal filesystem; the 8.6s that motivated it was a 9p-mount artifact). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |