diff --git a/CHANGELOG.md b/CHANGELOG.md index 4eb9e73572..4e72e01c7d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Fixes +- `codegraph_explore`, the CLI `codegraph explore` and the prompt hook no longer stall for a minute or more on a large project after a full index; the step that finds the project's busiest file no longer slows to a crawl when a few files are huge, like a generated Android `R.java` (#2571). Thanks @yangfengfan2011. - In VB.NET, every member of a `Structure` is now indexed, including its fields, properties, methods, constructors and nested enums. Before, only the first member was, so the rest could not be found and their callers looked empty. - In VB.NET and C#, what a property's `Get` and `Set` code calls, creates and reads now belongs to that property, as do C#'s `get => …` accessors and `=> …` property bodies. Before, it was dropped, so a method used only from a property looked unused. - In VB.NET, a field or property initializer like `= Compute()` or `As New List(Of Order)` now links what it calls and creates, and so do a `Custom Event`'s `AddHandler`, `RemoveHandler` and `RaiseEvent` blocks. Re-index VB.NET and C# projects after upgrading. diff --git a/__tests__/dominant-file-plan.test.ts b/__tests__/dominant-file-plan.test.ts new file mode 100644 index 0000000000..362dff0705 --- /dev/null +++ b/__tests__/dominant-file-plan.test.ts @@ -0,0 +1,111 @@ +/** + * The dominant-file aggregation must stay one pass over the edges whatever the + * planner's statistics say (#2571). `runMaintenance` samples them + * (`analysis_limit=1000` + `PRAGMA optimize`), and a sample can under-count + * nodes per file — `idx_nodes_file_path` at 6 where a full ANALYZE says 26. + * The plain join then started from nodes and, per node, walked every node in + * its file and probed edges by (source, target): quadratic in nodes per file, + * ~100 s on a repo with a generated R.java. The statistics below are the + * sampled ones from a real index; the test plants them and reads the plan. + */ +import { describe, it, expect, afterEach } from 'vitest'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import CodeGraph from '../src/index'; +import { DOMINANT_FILE_SQL } from '../src/db/queries'; + +/** sqlite_stat1 after a sampled ANALYZE of this repo's own index. */ +const SAMPLED_STATS: Array<[string, string, string]> = [ + ['edges', 'idx_edges_identity', '103526 9 2 2 1 1'], + ['edges', 'idx_edges_kind', '103526 12941'], + ['edges', 'idx_edges_provenance', '103526 51763'], + ['edges', 'idx_edges_source_kind', '103526 9 6'], + ['edges', 'idx_edges_target_kind', '103526 4 3'], + ['nodes', 'idx_nodes_file_line', '30684 26 1'], + ['nodes', 'idx_nodes_file_path', '30684 6'], + ['nodes', 'idx_nodes_kind', '30684 1535 6 1 1'], + ['nodes', 'idx_nodes_language', '30684 1395'], + ['nodes', 'idx_nodes_lower_name', '30684 3'], + ['nodes', 'idx_nodes_name', '30684 2'], + ['nodes', 'idx_nodes_qualified_name', '30684 2'], + ['nodes', 'sqlite_autoindex_nodes_1', '30684 1'], +]; + +let root: string | undefined; +let cg: CodeGraph | undefined; + +afterEach(() => { + cg?.close(); + cg = undefined; + if (root) fs.rmSync(root, { recursive: true, force: true }); + root = undefined; +}); + +describe('dominant file query plan (#2571)', () => { + it('scans the edges first even when sampled statistics under-count nodes per file', async () => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-2571-')); + const body = Array.from({ length: 25 }, (_, i) => `export function f${i}() { return f${i + 1}(); }`).join('\n'); + fs.writeFileSync(path.join(root, 'core.ts'), `${body}\nexport function f25() { return 0; }\n`); + cg = await CodeGraph.init(root, { index: true }); + + const db = (cg as any).db.db; + db.exec('ANALYZE'); + db.exec("DELETE FROM sqlite_stat1 WHERE tbl IN ('nodes', 'edges')"); + const insert = db.prepare('INSERT INTO sqlite_stat1(tbl, idx, stat) VALUES (?, ?, ?)'); + for (const row of SAMPLED_STATS) insert.run(...row); + db.exec('ANALYZE sqlite_schema'); // reload the planted statistics + + const plan = (sql: string) => + (db.prepare(`EXPLAIN QUERY PLAN ${sql}`).all() as Array<{ detail: string }>).map((r) => r.detail); + + // The statement getDominantFile prepares is the exported one. + const queries = (cg as any).queries; + queries.stmts.getDominantFile = undefined; + const prepare = queries.db.prepare.bind(queries.db); + const prepared: string[] = []; + queries.db.prepare = (text: string) => { + prepared.push(text); + return prepare(text); + }; + let dominant; + try { + dominant = queries.getDominantFile(); + } finally { + queries.db.prepare = prepare; + } + expect(dominant?.filePath).toBe('core.ts'); + expect(prepared).toContain(DOMINANT_FILE_SQL); + + const fixed = plan(DOMINANT_FILE_SQL); + expect(fixed[0]).toMatch(/^SCAN e\b/); + expect(fixed.join(' | ')).not.toMatch(/SEARCH m USING INDEX idx_nodes_file_path/); + + // Control: on SQLite 3.51 a plain join takes the quadratic plan under these + // statistics, which is what makes the assertions above discriminate. A + // planner that costs it differently is no failure of this query. + const plain = plan(DOMINANT_FILE_SQL.replace(/CROSS JOIN/g, 'JOIN')).join(' | '); + if (!/SEARCH m USING INDEX idx_nodes_file_path/.test(plain)) { + console.warn(`dominant-file-plan: plain JOIN no longer takes the quadratic plan here (${plain})`); + } + }); + + // The plan no longer decides between files with the same count: the old + // plan happened to emit them by path, the edges-first one does not, and a + // tie at the edge of the 20-row window can decide the dominant file. + it('breaks a tie in edge count by path, whatever the plan', async () => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-2571-')); + const chain = (prefix: string) => + Array.from({ length: 25 }, (_, i) => `export function ${prefix}${i}() { return ${prefix}${i + 1}(); }`).join('\n') + + `\nexport function ${prefix}25() { return 0; }\n`; + fs.mkdirSync(path.join(root, 'src')); + fs.writeFileSync(path.join(root, 'src/zeta.ts'), chain('z')); + fs.writeFileSync(path.join(root, 'src/alpha.ts'), chain('a')); + cg = await CodeGraph.init(root, { index: true }); + (cg as any).db.db.exec('ANALYZE'); + + const dominant = (cg as any).queries.getDominantFile(); + expect(dominant?.edgeCount).toBe(dominant?.nextEdgeCount); + expect(dominant?.filePath).toBe('src/alpha.ts'); + }); +}); diff --git a/src/db/queries.ts b/src/db/queries.ts index 6421eec360..9d7241903d 100644 --- a/src/db/queries.ts +++ b/src/db/queries.ts @@ -225,6 +225,28 @@ function rowToFileRecord(row: FileRow): FileRecord { }; } +/** + * Files ranked by edges between two of their own symbols — the input to + * `getDominantFile`. CROSS JOIN pins the loop order to edges first, then a + * lookup by id for each end: one pass over the edges whatever the statistics + * say. With a plain JOIN the planner may start from nodes and, for each node, + * walk every node in its file and probe edges by (source, target) — quadratic + * in nodes per file. Sampled stats (`analysis_limit` + `PRAGMA optimize`) can + * under-count nodes per file and pick that plan, and a few huge files (a + * generated R.java) then cost ~100 s (#2571). Ties go to the path, so the + * plan never decides between equal counts. + */ +export const DOMINANT_FILE_SQL = ` + SELECT n.file_path AS file_path, COUNT(*) AS edge_count + FROM edges e + CROSS JOIN nodes n ON e.source = n.id + CROSS JOIN nodes m ON e.target = m.id + WHERE n.file_path = m.file_path + GROUP BY n.file_path + ORDER BY edge_count DESC, n.file_path + LIMIT 20 +`; + /** * Query builder for the knowledge graph database */ @@ -1152,16 +1174,8 @@ export class QueryBuilder { // protobuf stub) outranks the real `server/etcdserver/server.go` // (470 edges) by 4×, and the boost would push the agent toward // generated code. - this.stmts.getDominantFile = this.db.prepare(` - SELECT n.file_path AS file_path, COUNT(*) AS edge_count - FROM edges e - JOIN nodes n ON e.source = n.id - JOIN nodes m ON e.target = m.id - WHERE n.file_path = m.file_path - GROUP BY n.file_path - ORDER BY edge_count DESC - LIMIT 20 - `); + // Edges-first and tie-broken by path on purpose: see DOMINANT_FILE_SQL (#2571). + this.stmts.getDominantFile = this.db.prepare(DOMINANT_FILE_SQL); } const rows = this.stmts.getDominantFile.all() as Array<{ file_path: string; edge_count: number }>; const generated = this.getGeneratedPathsAmong(rows.map(r => r.file_path));