perf: sub-100ms incremental rebuilds (466ms → 67-80ms)#644
Conversation
… compat 9.1 — Warn on graph load when DB was built with a different codegraph version. The check runs once per process in openReadonlyOrFail() and suggests `build --no-incremental`. 9.2 — Barrel-only files now emit reexport edges during build. Previously the entire file was skipped in buildImportEdges; now only non-reexport imports are skipped, so `codegraph exports` can follow re-export chains. 9.3 — Demote "Failed to parse tsconfig.json" from warn to debug level so it no longer clutters every build output. 9.4 — Document EXTENSIONS/IGNORE_DIRS Array→Set breaking change in CHANGELOG. Add .toArray() convenience method and export ArrayCompatSet type for consumers migrating from the pre-3.4 array API.
…ge duplication (#634) - Move _versionWarned flag outside mismatch conditional to avoid redundant build_meta queries when versions match. - Wrap SUPPORTED_EXTENSIONS in new Set() to avoid mutating the sibling module's export. - Delete outgoing edges for barrel-only files before re-adding them to fileSymbols during incremental builds, preventing duplicate reexport edges.
Delete the 39-LOC manual ambient type declarations for better-sqlite3 and use the community @types/better-sqlite3 package instead. The vendor file was a migration-era shim (allowJs is long gone from tsconfig). - Replace all BetterSqlite3.Database → BetterSqlite3Database (types.ts) - Replace all BetterSqlite3.Statement → SqliteStatement (types.ts) - Simplify constructor casts in connection.ts, branch-compare.ts, snapshot.ts (no longer needed with proper @types) - Clean up watcher.ts double-cast and info.ts @ts-expect-error - Widen transaction() return type for @types compatibility
- Restore warn level for tsconfig/jsconfig parse errors (P1: was incorrectly downgraded to debug; ENOENT is already guarded by existsSync before the try block) - Simplify openReadonlyOrFail constructor cast to match openDb pattern (P2) - Use Object.assign in withArrayCompat instead of cast-then-mutate (P2) - Remove unused BetterSqlite3Database import from branch-compare.ts - Remove stale biome-ignore suppression from snapshot.ts
Four optimizations for small incremental builds (≤5 changed files): 1. Scope barrel re-parsing to related barrels only (resolve-imports.ts) Instead of parsing ALL barrel files one-by-one (~93ms), only re-parse barrels imported by or re-exporting from changed files, batch-parsed in one call (~11ms). 2. Fast-path structure metrics (build-structure.ts) For ≤5 changed files on large codebases (>20 files), use targeted per-file SQL queries (~2ms) instead of loading ALL definitions from DB and recomputing ALL metrics (~35ms). 3. Skip unnecessary finalize work (finalize.ts) - Skip setBuildMeta writes for ≤5 files (avoids WAL transaction) - Skip drift detection for ≤3 files - Skip auto-registration dynamic import for incremental builds - Move timing measurement before db.close() 4. Deferred db.close() for small incremental builds (connection.ts) WAL checkpoint in db.close() costs ~250ms on Windows NTFS. Defer to next event loop tick so buildGraph() returns immediately. Includes flushDeferredClose() for test compatibility and auto-flush on openDb().
Two optimizations for small incremental builds (≤5 changed files): 1. collectFiles: reconstruct file list from DB file_hashes + journal deltas instead of full recursive filesystem scan (~7ms savings) 2. buildEdges: scope node loading query to only relevant files (changed files + import targets) with lazy SQL fallback for global name lookups (~5ms savings on 6K→400 nodes) Combined improvement: 78-90ms → 67-80ms for 1-file incremental on 473-file codebase.
|
Claude finished @carlos-alm's task —— View job PR Review AnalysisTodo List
Phase 1: Root Cause Analysis ✅Problem Being Solved: Incremental rebuild performance bottleneck (466ms → 67-80ms). The PR addresses a real performance issue where small incremental changes trigger expensive operations that don't scale with change size. Why This Approach: The optimizations are surgical and well-justified:
Risk Assessment: LOW - All optimizations include proper fallback mechanisms and maintain correctness guarantees. Phase 2: Configuration Change Detection ✅No High-Risk Configuration Changes Detected:
Phase 3: Test Integrity Check ✅No Red Flags Found:
Phase 4: Code Quality Review ✅Strong Implementation Quality:
No Security/Correctness Concerns:
Phase 5: Backlog Compliance Assessment ✅Backlog Compliance
Critical ConcernsNone identified. This is a high-quality performance optimization that maintains correctness while delivering substantial user value. Final Recommendation
|
Greptile SummaryThis PR delivers a focused set of incremental-rebuild performance optimizations, reducing 1-file build time from ~466 ms to 67–80 ms on a 473-file codebase. The changes touch three pipeline stages:\n\n- Confidence Score: 5/5Safe to merge — no blocking logic errors; prior scoping-alignment concern is resolved and a latent drift-detection bug is fixed. All 2129 tests pass, the previous P0 scopedLoad/threshold misalignment is resolved via the tuple refactor, the finalize.ts threshold correction fixes a real (low-severity) stale-count bug, and the remaining comments are non-blocking style suggestions. Fast paths are guarded by clear conditions that fall back safely on any edge case. No files require special attention; build-edges.ts monkey-patch is worth a follow-up refactor but does not block merge. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[collectFiles stage] --> B{ctx.incremental &&
!forceFullRebuild?}
B -- No --> C[collectFilesUtil
full filesystem scan]
B -- Yes --> D[tryFastCollect]
D --> E{file_hashes
has entries?}
E -- No --> C
E -- Yes --> F{journal valid
& has entries?}
F -- No --> C
F -- Yes --> G[Load relative paths
from file_hashes]
G --> H[Apply journal deltas
+changed / -removed]
H --> I[Convert to absolute
paths → allFiles]
J[buildEdges stage] --> K[loadNodes ctx]
K --> L{!isFullBuild &&
fileSymbols.size <= 5?}
L -- No --> M[SELECT all nodes
scoped=false]
L -- Yes --> N{existingFileCount
> 20?}
N -- No --> M
N -- Yes --> O[Build relevantFiles
changed + imports + barrels]
O --> P[SELECT nodes WHERE
file IN relevantFiles
scoped=true]
P --> Q[addLazyFallback
patch nodesByName.get]
M --> R[buildEdgesTx]
Q --> R
Reviews (2): Last reviewed commit: "Merge branch 'feat/dogfood-fixes-9.1-9.4..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const allNodesBefore = loadNodes(ctx); | ||
| const scopedLoad = !ctx.isFullBuild && ctx.fileSymbols.size <= 5 && allNodesBefore.length < 5000; | ||
| setupNodeLookups(ctx, allNodesBefore); | ||
| addLazyFallback(ctx, scopedLoad); |
There was a problem hiding this comment.
loadNodes and scopedLoad
loadNodes decides to scope the query when existingFileCount > 20 (line 579), meaning it only loads nodes from relevant files. However, scopedLoad — which controls whether the lazy SQL fallback is applied — uses a different gate: allNodesBefore.length < 5000 (line 636).
If existingFileCount > 20 (scoped load executes) but the scoped result happens to return ≥5000 nodes, scopedLoad is false and addLazyFallback is never called. In that state, nodesByName contains only the scoped subset but has no lazy fallback for global name lookups (resolveByMethodOrGlobal, supplementReceiverEdges). Nodes outside the scoped set become silently invisible, potentially dropping call/usage edges.
The two gates should align:
const scopedLoad =
!ctx.isFullBuild &&
ctx.fileSymbols.size <= 5 &&
allNodesBefore.length < 5000 &&
existingFileCount > 20;In practice this is unlikely to fire (≥5000 nodes from ≤5 files + their direct imports is extreme), but the asymmetry is a latent correctness risk worth closing.
There was a problem hiding this comment.
Fixed — refactored loadNodes to return a { rows, scoped } tuple so the scopedLoad flag is derived directly from whether the scoped SQL path actually executed, rather than re-deriving it with a separate (misaligned) threshold. This eliminates the latent correctness risk entirely.
…lback (#644) Return scoped flag directly from loadNodes instead of re-deriving it with a different threshold, ensuring addLazyFallback is always called when the query was actually scoped.
…e tmpdir shadow (#644) - Lower setBuildMeta gate from >5 to >3 to match drift detection gate, preventing stale baseline counts after series of 4-5 file builds. - Remove redundant dynamic import of tmpdir (already imported statically). - Add comment clarifying finalizeMs metric placement.
…e/codegraph into feat/dogfood-fixes-9.1-9.4
|
Addressed all Greptile feedback in two commits: 1. Inconsistent scoping gate in 2. Stale drift detection baseline in 3. 4. All 2129 tests pass, lint clean (1 pre-existing warning). |
|
Addressed all Greptile feedback in two commits: 1. Inconsistent scoping gate in build-edges.ts (inline comment) 2. Stale drift detection baseline in finalize.ts 3. tmpdir shadow variable in finalize.ts 4. finalizeMs metric placement in finalize.ts All 2129 tests pass, lint clean (1 pre-existing warning). |
1 participant
Summary
Optimizes 1-file incremental rebuild performance on 473-file codegraph codebase from ~466ms baseline down to 67-80ms wall-clock time.
Optimizations applied (in order of impact):
Deferred DB close (
finalize.ts,connection.ts): WAL checkpoint indb.close()costs ~250ms on Windows NTFS. For small incremental builds (≤5 files), defer close to next event loop tick viasetImmediate. Auto-flushed on nextopenDb().Scoped barrel re-parsing (
resolve-imports.ts): For ≤5 changed files, only re-parse barrel files related to changes (imported by or re-exporting from changed files) instead of all barrels. Batch-parse in one call.Structure fast path (
build-structure.ts): For ≤5 changed files on large codebases (>20 files), use targeted per-file SQL queries instead of loading ALL definitions from DB.Skip finalize overhead (
finalize.ts): Skip drift detection (≤3 files), build metadata persistence (≤5 files), and auto-registration (incremental builds).DB-cached file list (
collect-files.ts): ReconstructallFilesfrom DBfile_hashes+ journal deltas instead of full recursive filesystem scan (~7ms savings).Scoped node loading (
build-edges.ts): Load nodes only from relevant files (changed + import targets) with lazy SQL fallback for global lookups (~5ms savings, 6K→400 nodes).Benchmark (5 runs, 1-file change on 473-file codebase):
Test plan