From 0fe04e2c916e8a1e699b0c1affba2303adea0010 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Tue, 11 Aug 2026 01:48:14 -0700 Subject: [PATCH] perf(sqlite): cache prepared statements in SyncDatabase (#13769) prepare() recompiled every statement, so orchestration reads re-parsed the same SQL on the main thread. Adds a bounded LRU keyed by SQL, cleared on close() and before schema-changing exec(). Wildcard selects are excluded: node:sqlite builds the first post-schema-change row from stale column names, so a reused SELECT * can silently drop a freshly added column. PRAGMAs stay uncached. Co-authored-by: Orca --- src/main/sqlite/sync-database.test.ts | 153 ++++++++++++++++++++++++++ src/main/sqlite/sync-database.ts | 36 +++++- 2 files changed, 188 insertions(+), 1 deletion(-) create mode 100644 src/main/sqlite/sync-database.test.ts diff --git a/src/main/sqlite/sync-database.test.ts b/src/main/sqlite/sync-database.test.ts new file mode 100644 index 00000000000..34677095b26 --- /dev/null +++ b/src/main/sqlite/sync-database.test.ts @@ -0,0 +1,153 @@ +import { mkdtemp, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, describe, expect, it } from 'vitest' +import SyncDatabase from './sync-database' + +const temporaryDirectories: string[] = [] +const openDatabases: SyncDatabase.Database[] = [] + +async function createDatabase(): Promise { + const directory = await mkdtemp(join(tmpdir(), 'orca-sync-database-')) + temporaryDirectories.push(directory) + const db = new SyncDatabase(join(directory, 'test.db')) + openDatabases.push(db) + db.exec( + 'CREATE TABLE items (id TEXT PRIMARY KEY, label TEXT); ' + + "INSERT INTO items (id, label) VALUES ('a', 'alpha'), ('b', 'beta')" + ) + return db +} + +afterEach(async () => { + for (const db of openDatabases.splice(0)) { + try { + db.close() + } catch { + // already closed by the test + } + } + await Promise.all( + temporaryDirectories.splice(0).map((dir) => rm(dir, { recursive: true, force: true })) + ) +}) + +describe('SyncDatabase statement cache', () => { + it('reuses the same statement object for identical SQL', async () => { + const db = await createDatabase() + const sql = 'SELECT label FROM items WHERE id = ?' + + expect(db.prepare(sql)).toBe(db.prepare(sql)) + expect(db.prepare('SELECT id FROM items WHERE id = ?')).not.toBe(db.prepare(sql)) + }) + + it('returns correct rows when a reused statement is bound to different values', async () => { + const db = await createDatabase() + const sql = 'SELECT label FROM items WHERE id = ?' + + expect(db.prepare(sql).get('a')).toEqual({ label: 'alpha' }) + expect(db.prepare(sql).get('b')).toEqual({ label: 'beta' }) + expect(db.prepare(sql).get('missing')).toBeUndefined() + expect(db.prepare(sql).all('a')).toEqual([{ label: 'alpha' }]) + }) + + it('bounds the cache so per-arity SQL cannot grow it without limit', async () => { + const db = await createDatabase() + const first = 'SELECT 0 AS n' + const firstStatement = db.prepare(first) + for (let index = 1; index <= 256; index += 1) { + db.prepare(`SELECT ${index} AS n`) + } + + expect(db.prepare(first)).not.toBe(firstStatement) + expect(db.prepare('SELECT 256 AS n')).toBe(db.prepare('SELECT 256 AS n')) + }) + + it('drops cached statements on close', async () => { + const db = await createDatabase() + db.prepare('SELECT label FROM items WHERE id = ?') + db.close() + + const cache = (db as unknown as { statementCache: Map }).statementCache + expect(cache.size).toBe(0) + }) + + it('does not serve a stale statement after DDL adds a column', async () => { + const db = await createDatabase() + expect(db.prepare('SELECT * FROM items WHERE id = ?').all('a')).toEqual([ + { id: 'a', label: 'alpha' } + ]) + + db.exec("ALTER TABLE items ADD COLUMN note TEXT; UPDATE items SET note = 'noted'") + + expect(db.prepare('SELECT * FROM items WHERE id = ?').all('a')).toEqual([ + { id: 'a', label: 'alpha', note: 'noted' } + ]) + expect(db.prepare('SELECT label FROM items WHERE id = ?').all('a')).toEqual([ + { label: 'alpha' } + ]) + }) + + it('keeps a cached statement correct when another connection changes the schema', async () => { + const directory = await mkdtemp(join(tmpdir(), 'orca-sync-database-')) + temporaryDirectories.push(directory) + const path = join(directory, 'shared.db') + const writer = new SyncDatabase(path) + const reader = new SyncDatabase(path) + openDatabases.push(writer, reader) + writer.exec("CREATE TABLE items (id TEXT PRIMARY KEY); INSERT INTO items VALUES ('a')") + const sql = 'SELECT id FROM items ORDER BY id' + expect(reader.prepare(sql).all()).toEqual([{ id: 'a' }]) + + writer.exec("ALTER TABLE items ADD COLUMN note TEXT; INSERT INTO items VALUES ('b', 'noted')") + + expect(reader.prepare(sql).all()).toEqual([{ id: 'a' }, { id: 'b' }]) + expect(reader.prepare('SELECT note FROM items WHERE id = ?').all('b')).toEqual([ + { note: 'noted' } + ]) + }) + + it('does not cache wildcard selects or pragma statements', async () => { + const db = await createDatabase() + + expect(db.prepare('SELECT * FROM items')).not.toBe(db.prepare('SELECT * FROM items')) + expect(db.prepare('PRAGMA table_info(items)')).not.toBe(db.prepare('PRAGMA table_info(items)')) + expect(db.prepare('SELECT COUNT(*) AS n FROM items')).toBe( + db.prepare('SELECT COUNT(*) AS n FROM items') + ) + }) + + it('keeps cached statements across transaction control and other non-DDL exec calls', async () => { + const db = await createDatabase() + const sql = 'SELECT label FROM items WHERE id = ?' + const statement = db.prepare(sql) + + db.exec('BEGIN IMMEDIATE') + db.exec("INSERT INTO items (id, label) VALUES ('c', 'gamma')") + db.exec('COMMIT') + + expect(db.prepare(sql)).toBe(statement) + expect(statement.get('c')).toEqual({ label: 'gamma' }) + }) + + it('preserves pragma and exec behavior', async () => { + const db = await createDatabase() + + expect(db.pragma('journal_mode', { simple: true })).toBe('delete') + expect(db.pragma('table_info(items)')).toEqual([ + expect.objectContaining({ name: 'id' }), + expect.objectContaining({ name: 'label' }) + ]) + expect(db.pragma('table_info(missing_table)')).toEqual([]) + expect(db.pragma('table_info(missing_table)', { simple: true })).toBeUndefined() + }) + + it('rejects a missing file when fileMustExist is set', async () => { + const directory = await mkdtemp(join(tmpdir(), 'orca-sync-database-')) + temporaryDirectories.push(directory) + + expect(() => new SyncDatabase(join(directory, 'absent.db'), { fileMustExist: true })).toThrow( + /does not exist/ + ) + }) +}) diff --git a/src/main/sqlite/sync-database.ts b/src/main/sqlite/sync-database.ts index 70d8fa1c06a..68faef8b83c 100644 --- a/src/main/sqlite/sync-database.ts +++ b/src/main/sqlite/sync-database.ts @@ -15,6 +15,18 @@ type PragmaOptions = { export type SqliteStatement = StatementSync +// Why: dynamic `IN (?,?,…)` clauses mint a new SQL string per arity, so the cache must stay bounded. +const STATEMENT_CACHE_LIMIT = 256 +const AGGREGATE_STAR = /\(\s*\*\s*\)/g +const PRAGMA_STATEMENT = /^\s*PRAGMA\b/i +const SCHEMA_CHANGING_SQL = /\b(?:ALTER|CREATE|DROP|REINDEX|VACUUM|ATTACH|DETACH)\b/i + +// Why: node:sqlite builds the first post-schema-change row from stale column names, so a reused +// wildcard SELECT can drop a freshly added column; PRAGMAs are one-shot config, never hot-path. +function isStatementCacheable(sql: string): boolean { + return !PRAGMA_STATEMENT.test(sql) && !sql.replace(AGGREGATE_STAR, '').includes('*') +} + // Why: SSH companions target Node 18 and import this adapter without opening SQLite. function loadDatabaseSync(): typeof DatabaseSync { if (typeof process.getBuiltinModule !== 'function') { @@ -26,6 +38,7 @@ function loadDatabaseSync(): typeof DatabaseSync { class SyncDatabase { private readonly db: DatabaseSync + private readonly statementCache = new Map() constructor(path: SqlitePath, options: SyncDatabaseOptions = {}) { if ( @@ -44,11 +57,31 @@ class SyncDatabase { } exec(sql: string): void { + // Why: drop cached statements before DDL lands so a partially applied batch cannot leave stale ones. + if (SCHEMA_CHANGING_SQL.test(sql)) { + this.statementCache.clear() + } this.db.exec(sql) } prepare(sql: string): StatementSync { - return this.db.prepare(sql) + const cached = this.statementCache.get(sql) + if (cached) { + this.statementCache.delete(sql) + this.statementCache.set(sql, cached) + return cached + } + const statement = this.db.prepare(sql) + if (isStatementCacheable(sql)) { + if (this.statementCache.size >= STATEMENT_CACHE_LIMIT) { + const oldest = this.statementCache.keys().next().value + if (oldest !== undefined) { + this.statementCache.delete(oldest) + } + } + this.statementCache.set(sql, statement) + } + return statement } pragma(sql: string, options?: PragmaOptions): unknown { @@ -64,6 +97,7 @@ class SyncDatabase { } close(): void { + this.statementCache.clear() this.db.close() } }