From 3bffc52f84903ac6f0e287da24a5f0bd9ebb5de1 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Thu, 9 Apr 2026 00:51:17 -0600 Subject: [PATCH 1/2] perf(native): fix full-build regression from NativeDbProxy overhead (#903) The NativeDbProxy introduced in #897 has per-statement napi serialization overhead (2 round-trips per .run()) that causes a full-build regression when multiplied across 667 files (9.4s vs 5.2s in 3.9.1). Three fixes: 1. Enable bulkInsertCfg native fast path in buildCFGData. The Rust method handles delete-before-insert atomically on a single rusqlite connection, eliminating the dual-connection WAL concern that originally blocked it. 2. After the Rust orchestrator completes, hand off to better-sqlite3 for JS post-processing (structure + analysis) instead of routing through the slow proxy. Wire suspendJsDb/resumeJsDb WAL callbacks and transfer the advisory lock to the new connection. 3. Optimize NativeDbProxy.run() to only query last_insert_rowid() for INSERT statements, halving per-statement overhead for DELETE/UPDATE. Closes #903 --- src/domain/graph/builder/native-db-proxy.ts | 14 +++-- src/domain/graph/builder/pipeline.ts | 34 ++++++++++-- src/features/cfg.ts | 57 ++++++++++++++++++--- 3 files changed, 92 insertions(+), 13 deletions(-) diff --git a/src/domain/graph/builder/native-db-proxy.ts b/src/domain/graph/builder/native-db-proxy.ts index 13b4295b..83f4b0b0 100644 --- a/src/domain/graph/builder/native-db-proxy.ts +++ b/src/domain/graph/builder/native-db-proxy.ts @@ -27,6 +27,9 @@ export class NativeDbProxy implements BetterSqlite3Database { prepare(sql: string): SqliteStatement { const ndb = this.#ndb; + // Only INSERT statements need last_insert_rowid — skip the extra napi + // call for UPDATE/DELETE/other DML to halve per-statement overhead. + const isInsert = sql.trimStart().substring(0, 6).toUpperCase() === 'INSERT'; const stmt: SqliteStatement = { all(...params: unknown[]): TRow[] { return ndb.queryAll(sql, sanitize(params)) as TRow[]; @@ -36,10 +39,13 @@ export class NativeDbProxy implements BetterSqlite3Database { }, run(...params: unknown[]): { changes: number; lastInsertRowid: number | bigint } { ndb.queryAll(sql, sanitize(params)); - // Retrieve last_insert_rowid via SQLite scalar function so callers - // that depend on it (e.g. CFG block edge mapping) get correct values. - const row = ndb.queryGet('SELECT last_insert_rowid() AS rid', []) as { rid: number } | null; - return { changes: 0, lastInsertRowid: row?.rid ?? 0 }; + if (isInsert) { + const row = ndb.queryGet('SELECT last_insert_rowid() AS rid', []) as { + rid: number; + } | null; + return { changes: 0, lastInsertRowid: row?.rid ?? 0 }; + } + return { changes: 0, lastInsertRowid: 0 }; }, iterate(): IterableIterator { throw new Error('iterate() is not supported via NativeDbProxy'); diff --git a/src/domain/graph/builder/pipeline.ts b/src/domain/graph/builder/pipeline.ts index 65cc17c9..e12f0509 100644 --- a/src/domain/graph/builder/pipeline.ts +++ b/src/domain/graph/builder/pipeline.ts @@ -477,8 +477,25 @@ async function runPostNativeAnalysis( const native = loadNative(); if (native?.NativeDatabase) { try { + // Checkpoint JS WAL before opening native connection so both + // connections see the same DB state (structure writes are flushed). + ctx.db.pragma('wal_checkpoint(TRUNCATE)'); ctx.nativeDb = native.NativeDatabase.openReadWrite(ctx.dbPath); - if (ctx.engineOpts) ctx.engineOpts.nativeDb = ctx.nativeDb; + if (ctx.engineOpts) { + ctx.engineOpts.nativeDb = ctx.nativeDb; + ctx.engineOpts.suspendJsDb = () => { + ctx.db.pragma('wal_checkpoint(TRUNCATE)'); + }; + ctx.engineOpts.resumeJsDb = () => { + try { + ctx.nativeDb?.exec('PRAGMA wal_checkpoint(TRUNCATE)'); + } catch (e) { + debug( + `resumeJsDb: WAL checkpoint failed (nativeDb may already be closed): ${toErrorMessage(e)}`, + ); + } + }; + } } catch { ctx.nativeDb = undefined; if (ctx.engineOpts) ctx.engineOpts.nativeDb = undefined; @@ -633,11 +650,22 @@ async function tryNativeOrchestrator( const needsStructure = !result.structureHandled; if (needsAnalysis || needsStructure) { - // In native-first mode the proxy is already wired — no WAL handoff needed. - if (!ctx.nativeFirstProxy && !handoffWalAfterNativeBuild(ctx)) { + // Always hand off to better-sqlite3 for JS post-processing. + // The NativeDbProxy has per-statement napi serialization overhead that + // makes structure/analysis phases significantly slower than direct + // better-sqlite3. Native bulk-insert methods (bulkInsertCfg, etc.) + // are wired through engineOpts.nativeDb in runPostNativeAnalysis. + const lockPath = ctx.nativeFirstProxy + ? (ctx.db as unknown as { __lockPath?: string }).__lockPath + : undefined; + if (ctx.nativeFirstProxy) ctx.nativeFirstProxy = false; + if (!handoffWalAfterNativeBuild(ctx)) { // DB reopen failed — return partial result return formatNativeTimingResult(p, 0, analysisTiming); } + // Transfer advisory lock ownership from the proxy to the new better-sqlite3 + // connection so closeDbPair releases it at the end. + if (lockPath) (ctx.db as unknown as { __lockPath?: string }).__lockPath = lockPath; // When structure was handled by Rust, we only need changed files for // analysis — no need to load the entire graph from DB. When structure diff --git a/src/features/cfg.ts b/src/features/cfg.ts index 923b3c7e..3b7558da 100644 --- a/src/features/cfg.ts +++ b/src/features/cfg.ts @@ -369,7 +369,7 @@ export async function buildCFGData( db: BetterSqlite3Database, fileSymbols: Map, rootDir: string, - _engineOpts?: { + engineOpts?: { nativeDb?: { bulkInsertCfg?(entries: Array>): number }; suspendJsDb?: () => void; resumeJsDb?: () => void; @@ -379,11 +379,56 @@ export async function buildCFGData( // skip WASM parser init, tree parsing, and JS visitor entirely — just persist. const allNative = allCfgNative(fileSymbols); - // NOTE: nativeDb.bulkInsertCfg is intentionally NOT used here. - // The CFG path requires delete-before-insert (deleteCfgForNode) which creates - // a dual-connection WAL conflict when deletes go through JS (better-sqlite3) - // and inserts go through native (rusqlite). The JS-only persistNativeFileCfg - // path below handles both on a single connection safely. + // ── Native bulk-insert fast path ────────────────────────────────────── + // The Rust bulkInsertCfg handles delete-before-insert atomically on a + // single rusqlite connection, so there is no dual-connection WAL conflict. + const nativeDb = engineOpts?.nativeDb; + if (allNative && nativeDb?.bulkInsertCfg) { + const entries: Array> = []; + for (const [relPath, symbols] of fileSymbols) { + const ext = path.extname(relPath).toLowerCase(); + if (!CFG_EXTENSIONS.has(ext)) continue; + + for (const def of symbols.definitions) { + if (def.kind !== 'function' && def.kind !== 'method') continue; + if (!def.line) continue; + + const nodeId = getFunctionNodeId(db, def.name, relPath, def.line); + if (!nodeId) continue; + + const cfg = def.cfg as { blocks?: CfgBuildBlock[]; edges?: CfgBuildEdge[] } | undefined; + if (!cfg?.blocks?.length) continue; + + entries.push({ + nodeId, + blocks: cfg.blocks.map((b) => ({ + index: b.index, + blockType: b.type, + startLine: b.startLine ?? null, + endLine: b.endLine ?? null, + label: b.label ?? null, + })), + edges: (cfg.edges || []).map((e) => ({ + sourceIndex: e.sourceIndex, + targetIndex: e.targetIndex, + kind: e.kind, + })), + }); + } + } + + if (entries.length > 0) { + let inserted: number; + try { + engineOpts?.suspendJsDb?.(); + inserted = nativeDb.bulkInsertCfg(entries); + } finally { + engineOpts?.resumeJsDb?.(); + } + info(`CFG (native bulk): ${inserted} functions analyzed`); + } + return; + } const extToLang = buildExtToLangMap(); let parsers: unknown = null; From bc6906345efda1fa7a81464d582fb80163609180 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Thu, 9 Apr 2026 01:30:31 -0600 Subject: [PATCH 2/2] =?UTF-8?q?fix:=20address=20review=20feedback=20?= =?UTF-8?q?=E2=80=94=20remove=20redundant=20lock-path=20transfer=20and=20d?= =?UTF-8?q?ead=20branch=20(#906)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove redundant lockPath capture and transfer in tryNativeOrchestrator (openDb already sets __lockPath on the new connection) - Remove dead else-if branch in runPostNativeAnalysis (ctx.nativeFirstProxy is always false after the unconditional reset in tryNativeOrchestrator) - Initialize inserted = 0 in buildCFGData for explicit intent --- src/domain/graph/builder/pipeline.ts | 8 -------- src/features/cfg.ts | 2 +- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/src/domain/graph/builder/pipeline.ts b/src/domain/graph/builder/pipeline.ts index e12f0509..fe3d9970 100644 --- a/src/domain/graph/builder/pipeline.ts +++ b/src/domain/graph/builder/pipeline.ts @@ -501,8 +501,6 @@ async function runPostNativeAnalysis( if (ctx.engineOpts) ctx.engineOpts.nativeDb = undefined; } } - } else if (ctx.engineOpts) { - ctx.engineOpts.nativeDb = ctx.nativeDb; } try { @@ -655,17 +653,11 @@ async function tryNativeOrchestrator( // makes structure/analysis phases significantly slower than direct // better-sqlite3. Native bulk-insert methods (bulkInsertCfg, etc.) // are wired through engineOpts.nativeDb in runPostNativeAnalysis. - const lockPath = ctx.nativeFirstProxy - ? (ctx.db as unknown as { __lockPath?: string }).__lockPath - : undefined; if (ctx.nativeFirstProxy) ctx.nativeFirstProxy = false; if (!handoffWalAfterNativeBuild(ctx)) { // DB reopen failed — return partial result return formatNativeTimingResult(p, 0, analysisTiming); } - // Transfer advisory lock ownership from the proxy to the new better-sqlite3 - // connection so closeDbPair releases it at the end. - if (lockPath) (ctx.db as unknown as { __lockPath?: string }).__lockPath = lockPath; // When structure was handled by Rust, we only need changed files for // analysis — no need to load the entire graph from DB. When structure diff --git a/src/features/cfg.ts b/src/features/cfg.ts index 3b7558da..65bc745a 100644 --- a/src/features/cfg.ts +++ b/src/features/cfg.ts @@ -418,7 +418,7 @@ export async function buildCFGData( } if (entries.length > 0) { - let inserted: number; + let inserted = 0; try { engineOpts?.suspendJsDb?.(); inserted = nativeDb.bulkInsertCfg(entries);