Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions src/domain/graph/builder/native-db-proxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ export class NativeDbProxy implements BetterSqlite3Database {

prepare<TRow = unknown>(sql: string): SqliteStatement<TRow> {
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<TRow> = {
all(...params: unknown[]): TRow[] {
return ndb.queryAll(sql, sanitize(params)) as TRow[];
Expand All @@ -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<TRow> {
throw new Error('iterate() is not supported via NativeDbProxy');
Expand Down
30 changes: 25 additions & 5 deletions src/domain/graph/builder/pipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -477,15 +477,30 @@ 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;
}
}
} else if (ctx.engineOpts) {
ctx.engineOpts.nativeDb = ctx.nativeDb;
}

try {
Expand Down Expand Up @@ -633,8 +648,13 @@ 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.
if (ctx.nativeFirstProxy) ctx.nativeFirstProxy = false;
if (!handoffWalAfterNativeBuild(ctx)) {
// DB reopen failed — return partial result
return formatNativeTimingResult(p, 0, analysisTiming);
}
Expand Down
57 changes: 51 additions & 6 deletions src/features/cfg.ts
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ export async function buildCFGData(
db: BetterSqlite3Database,
fileSymbols: Map<string, FileSymbols>,
rootDir: string,
_engineOpts?: {
engineOpts?: {
nativeDb?: { bulkInsertCfg?(entries: Array<Record<string, unknown>>): number };
suspendJsDb?: () => void;
resumeJsDb?: () => void;
Expand All @@ -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<Record<string, unknown>> = [];
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 = 0;
try {
engineOpts?.suspendJsDb?.();
inserted = nativeDb.bulkInsertCfg(entries);
} finally {
engineOpts?.resumeJsDb?.();
}
info(`CFG (native bulk): ${inserted} functions analyzed`);
}
Comment on lines +420 to +429
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 inserted is uninitialized if suspendJsDb throws before bulkInsertCfg runs

TypeScript accepts this because a throw inside the try block means info() is unreachable, so the compiler knows inserted is assigned when info() is actually executed. Runtime behaviour is correct. However, initialising inserted = 0 makes the intent explicit and avoids potential confusion if a catch block is ever added later.

Suggested change
if (entries.length > 0) {
let inserted: number;
try {
engineOpts?.suspendJsDb?.();
inserted = nativeDb.bulkInsertCfg(entries);
} finally {
engineOpts?.resumeJsDb?.();
}
info(`CFG (native bulk): ${inserted} functions analyzed`);
}
if (entries.length > 0) {
let inserted = 0;
try {
engineOpts?.suspendJsDb?.();
inserted = nativeDb.bulkInsertCfg(entries);
} finally {
engineOpts?.resumeJsDb?.();
}
info(`CFG (native bulk): ${inserted} functions analyzed`);
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — initialized inserted = 0 for explicit intent, matching the suggestion exactly.

return;
}

const extToLang = buildExtToLangMap();
let parsers: unknown = null;
Expand Down
Loading