Merge pull request #346 from tirth8205/perf/understand-pipeline

perf(understand): parallelise file I/O in compute-batches + extract-import-map (#76)

tests/skill/understand/test_extract_import_map.test.mjs

@@ -1597,3 +1597,55 @@ describe('extract-import-map.mjs — tree-sitter init graceful failure', () => {
     expect(result.output.stats.totalEdges).toBe(0);
   });
 });
+
+describe('extract-import-map.mjs — deterministic stderr ordering across loaders', () => {
+  let projectRoot;
+
+  afterEach(() => {
+    if (projectRoot) {
+      rmSync(projectRoot, { recursive: true, force: true });
+      projectRoot = null;
+    }
+  });
+
+  // Regression for the parallel-loader stderr-order bug surfaced in
+  // PR #346 review: tsconfig / go.mod / composer.json loaders now run
+  // concurrently, but warnings must still emit in the pre-PR canonical
+  // order (tsconfig → go → php). If the loaders streamed warnings
+  // mid-flight, I/O timing could reorder them — the assertions below
+  // catch that regression.
+  it('emits warnings in canonical order (tsconfig, go, php) regardless of I/O timing', () => {
+    projectRoot = setupTree({
+      'tsconfig.json': '{ "compilerOptions": { "baseUrl": ".", ', // unterminated
+      'composer.json': '{ "autoload": { "psr-4": { "App\\\\": "src/" }, ', // unterminated
+      'src/index.ts': `import { foo } from './foo';\n`,
+      'src/foo.ts': `export const foo = 1;\n`,
+      'src/Http/Controller.php':
+        `<?php\nnamespace App\\Http;\n\nuse App\\Models\\User;\n\nclass Controller { }\n`,
+      'src/Models/User.php':
+        `<?php\nnamespace App\\Models;\nclass User { }\n`,
+    });
+
+    const result = runScript(projectRoot, {
+      projectRoot,
+      files: [
+        { path: 'tsconfig.json', language: 'json', fileCategory: 'config' },
+        { path: 'composer.json', language: 'json', fileCategory: 'config' },
+        { path: 'src/index.ts', language: 'typescript', fileCategory: 'code' },
+        { path: 'src/foo.ts', language: 'typescript', fileCategory: 'code' },
+        { path: 'src/Http/Controller.php', language: 'php', fileCategory: 'code' },
+        { path: 'src/Models/User.php', language: 'php', fileCategory: 'code' },
+      ],
+    });
+
+    expect(result.status).toBe(0);
+    const tsLineIdx = result.stderr.indexOf('tsconfig.json at');
+    const composerLineIdx = result.stderr.indexOf('composer.json at');
+    expect(tsLineIdx).toBeGreaterThanOrEqual(0);
+    expect(composerLineIdx).toBeGreaterThanOrEqual(0);
+    // Canonical order: tsconfig warnings precede composer warnings.
+    // Pre-PR-346 this fell out of sequential loader passes; post-fix it
+    // falls out of buffering + ordered drain in buildResolutionContext.
+    expect(tsLineIdx).toBeLessThan(composerLineIdx);
+  });
+});

understand-anything-plugin/skills/understand/compute-batches.mjs

@@ -13,10 +13,19 @@
  */
 
 import { readFileSync, writeFileSync, existsSync, realpathSync } from 'node:fs';
+import { readFile } from 'node:fs/promises';
 import { dirname, join, resolve } from 'node:path';
 import { fileURLToPath, pathToFileURL } from 'node:url';
 import { createRequire } from 'node:module';
 
+/**
+ * Chunk size for parallel file I/O. Bounded so a 15k-file repo doesn't try
+ * to open every descriptor at once (would hit `EMFILE`) while still keeping
+ * libuv's worker-thread pool saturated. Empirically chosen to keep memory
+ * around tens of MB even when the average file is ~10 KB.
+ */
+const IO_PARALLELISM = 64;
+
 const __filename = fileURLToPath(import.meta.url);
 const PLUGIN_ROOT = resolve(dirname(__filename), '../..');
 const require = createRequire(resolve(PLUGIN_ROOT, 'package.json'));
@@ -57,31 +66,55 @@ async function extractExports(projectRoot, codeFiles) {
   }
 
   const exportsByPath = new Map();
-  for (const file of codeFiles) {
-    const abs = join(projectRoot, file.path);
-    let content;
-    try {
-      content = readFileSync(abs, 'utf-8');
-    } catch (err) {
-      process.stderr.write(
-        `Warning: compute-batches: exports extraction failed for ${file.path} ` +
-        `(read error: ${err.message}) — symbols=[] in neighborMap — ` +
-        `cross-batch edges to this file limited to file-level\n`,
-      );
-      exportsByPath.set(file.path, []);
-      continue;
-    }
-    try {
-      const analysis = registry.analyzeFile(file.path, content);
-      const names = (analysis?.exports || []).map(e => e.name).filter(Boolean);
-      exportsByPath.set(file.path, names);
-    } catch (err) {
-      process.stderr.write(
-        `Warning: compute-batches: exports extraction failed for ${file.path} ` +
-        `(analyze error: ${err.message}) — symbols=[] in neighborMap — ` +
-        `cross-batch edges to this file limited to file-level\n`,
-      );
-      exportsByPath.set(file.path, []);
+
+  // I/O is parallelised in bounded chunks (libuv worker threads handle the
+  // disk reads concurrently) while the actual tree-sitter parse stays on
+  // the main thread, since web-tree-sitter is single-threaded WASM. For a
+  // 15k-file iOS repo (#226), the sequential `readFileSync` loop dominated;
+  // letting reads pipeline drops wall time roughly proportional to the
+  // share of the loop spent waiting on disk.
+  for (let start = 0; start < codeFiles.length; start += IO_PARALLELISM) {
+    const slice = codeFiles.slice(start, start + IO_PARALLELISM);
+
+    // Read every file in the slice concurrently. Errors per file are
+    // captured in-place so a single bad file does not abort the chunk.
+    const reads = await Promise.all(
+      slice.map(async (file) => {
+        const abs = join(projectRoot, file.path);
+        try {
+          const content = await readFile(abs, 'utf-8');
+          return { file, content, readError: null };
+        } catch (err) {
+          return { file, content: null, readError: err };
+        }
+      }),
+    );
+
+    // Serialise the CPU-bound tree-sitter work and the stderr warning emits
+    // so log order remains identical to the previous sequential loop. This
+    // also keeps existing fixture-comparison tests stable.
+    for (const { file, content, readError } of reads) {
+      if (readError) {
+        process.stderr.write(
+          `Warning: compute-batches: exports extraction failed for ${file.path} ` +
+          `(read error: ${readError.message}) — symbols=[] in neighborMap — ` +
+          `cross-batch edges to this file limited to file-level\n`,
+        );
+        exportsByPath.set(file.path, []);
+        continue;
+      }
+      try {
+        const analysis = registry.analyzeFile(file.path, content);
+        const names = (analysis?.exports || []).map(e => e.name).filter(Boolean);
+        exportsByPath.set(file.path, names);
+      } catch (err) {
+        process.stderr.write(
+          `Warning: compute-batches: exports extraction failed for ${file.path} ` +
+          `(analyze error: ${err.message}) — symbols=[] in neighborMap — ` +
+          `cross-batch edges to this file limited to file-level\n`,
+        );
+        exportsByPath.set(file.path, []);
+      }
     }
   }
   return exportsByPath;

understand-anything-plugin/skills/understand/extract-import-map.mjs

@@ -37,6 +37,29 @@ import { createRequire } from 'node:module';
 import { dirname, resolve, join, posix } from 'node:path';
 import { fileURLToPath, pathToFileURL } from 'node:url';
 import { existsSync, readFileSync, realpathSync, writeFileSync } from 'node:fs';
+import { readFile } from 'node:fs/promises';
+
+/**
+ * Read a list of files concurrently while preserving result order. Failures
+ * are returned in-place as `{ raw: null, err }` so callers can emit the same
+ * per-file warnings they did under the previous sequential `readFileSync`
+ * loops.
+ *
+ * `paths` is a list of `{ key, absPath }` pairs; `key` is whatever the caller
+ * wants to attach the result to (typically a project-relative POSIX path).
+ */
+async function readFilesParallel(paths) {
+  return Promise.all(
+    paths.map(async ({ key, absPath }) => {
+      try {
+        const raw = await readFile(absPath, 'utf-8');
+        return { key, raw, err: null };
+      } catch (err) {
+        return { key, raw: null, err };
+      }
+    }),
+  );
+}
 
 const __dirname = dirname(fileURLToPath(import.meta.url));
 // skills/understand/ -> plugin root is two dirs up
@@ -180,20 +203,26 @@ function parseTsConfigText(raw) {
  *      where the stripper damaged a string literal containing `//`.
  *   3. If both fail, warn and skip — that tsconfig contributes no aliases.
  */
-function loadTsConfigs(projectRoot, files) {
+async function loadTsConfigs(projectRoot, files) {
   const out = new Map();
+  const warnings = [];
+  // Collect the candidate paths in the original file order before reading,
+  // so warning emit order matches the previous sequential implementation.
+  const candidates = [];
   for (const f of files) {
     const p = toPosix(f.path);
     const base = p.includes('/') ? p.slice(p.lastIndexOf('/') + 1) : p;
     if (base !== 'tsconfig.json') continue;
     const absPath = join(projectRoot, p);
     if (!existsSync(absPath)) continue;
-    let raw;
-    try {
-      raw = readFileSync(absPath, 'utf-8');
-    } catch (err) {
-      process.stderr.write(
-        `Warning: extract-import-map: tsconfig.json at ${absPath} failed ` +
+    candidates.push({ key: p, absPath });
+  }
+  const reads = await readFilesParallel(candidates);
+  for (const { key: p, raw, err } of reads) {
+    if (err) {
+      // absPath isn't carried through the helper return shape; reconstruct it.
+      warnings.push(
+        `Warning: extract-import-map: tsconfig.json at ${join(projectRoot, p)} failed ` +
         `to read (${err.message}) — path aliases from this config will ` +
         `not be applied — relative imports unaffected\n`,
       );
@@ -201,8 +230,8 @@ function loadTsConfigs(projectRoot, files) {
     }
     const parsed = parseTsConfigText(raw);
     if (!parsed) {
-      process.stderr.write(
-        `Warning: extract-import-map: tsconfig.json at ${absPath} failed ` +
+      warnings.push(
+        `Warning: extract-import-map: tsconfig.json at ${join(projectRoot, p)} failed ` +
         `to parse — path aliases from this config will not be applied ` +
         `— relative imports unaffected\n`,
       );
@@ -210,7 +239,7 @@ function loadTsConfigs(projectRoot, files) {
     }
     out.set(dirOf(p), parsed);
   }
-  return out;
+  return { configs: out, warnings };
 }
 
 /**
@@ -237,20 +266,26 @@ function loadTsConfigs(projectRoot, files) {
  * The resolver uses each module's prefix to translate
  * `import "github.com/foo/bar/x"` into the project-internal `x/<file>.go`.
  */
-function loadGoModules(projectRoot, files) {
+async function loadGoModules(projectRoot, files) {
   const out = new Map();
+  // loadGoModules currently emits no warnings (read failures are silently
+  // skipped — per-file resolvers surface "no ancestor go.mod" later), but
+  // the `{ data, warnings }` shape matches loadTsConfigs / loadPhpAutoloads
+  // so the concurrent caller in buildResolutionContext can drain them
+  // uniformly in canonical order.
+  const warnings = [];
+  const candidates = [];
   for (const f of files) {
     const p = toPosix(f.path);
     const base = p.includes('/') ? p.slice(p.lastIndexOf('/') + 1) : p;
     if (base !== 'go.mod') continue;
     const absPath = join(projectRoot, p);
     if (!existsSync(absPath)) continue;
-    let raw;
-    try {
-      raw = readFileSync(absPath, 'utf-8');
-    } catch {
-      continue;
-    }
+    candidates.push({ key: p, absPath });
+  }
+  const reads = await readFilesParallel(candidates);
+  for (const { key: p, raw, err } of reads) {
+    if (err) continue;
     let moduleName = '';
     for (const line of raw.split(/\r?\n/)) {
       const trimmed = line.replace(/\/\/.*$/, '').trim();
@@ -261,7 +296,7 @@ function loadGoModules(projectRoot, files) {
     if (!moduleName) continue;
     out.set(dirOf(p), moduleName);
   }
-  return out;
+  return { modules: out, warnings };
 }
 
 /**
@@ -306,10 +341,32 @@ function findNearestConfigDir(startDir, configMap) {
  *
  * Build once; pass everywhere.
  */
-function buildResolutionContext(projectRoot, files) {
+async function buildResolutionContext(projectRoot, files) {
   const fileSet = new Set(files.map(f => toPosix(f.path)));
-  const tsConfigs = loadTsConfigs(projectRoot, files);
-  const goModules = loadGoModules(projectRoot, files);
+
+  // The three config-loader passes are independent and each does its own
+  // batched parallel I/O; run them concurrently so the wait for a slow
+  // tsconfig.json read doesn't block go.mod / composer.json scanning.
+  //
+  // Each loader BUFFERS warnings into a private array rather than writing
+  // them to stderr inline. If a loader streamed warnings directly during
+  // the concurrent passes, lines from independent loader families could
+  // interleave based on I/O timing — that would break the pre-PR
+  // deterministic order (ts → go → php) and make stderr-diff verification
+  // flaky. Drain the buffers in canonical order *after* Promise.all, so
+  // a fixture with `(malformed tsconfig.json, malformed composer.json)`
+  // always emits `tsconfig…\ncomposer…\n`, never the reverse.
+  const [tsResult, goResult, phpResult] = await Promise.all([
+    loadTsConfigs(projectRoot, files),
+    loadGoModules(projectRoot, files),
+    loadPhpAutoloads(projectRoot, files),
+  ]);
+  for (const w of tsResult.warnings) process.stderr.write(w);
+  for (const w of goResult.warnings) process.stderr.write(w);
+  for (const w of phpResult.warnings) process.stderr.write(w);
+  const tsConfigs = tsResult.configs;
+  const goModules = goResult.modules;
+  const phpAutoloads = phpResult.autoloads;
 
   // Index .go files by their parent directory so the Go resolver can
   // expand a package-level import to all member .go files in O(1).
@@ -331,8 +388,6 @@ function buildResolutionContext(projectRoot, files) {
   const kotlinIndex = buildSuffixIndex(files, p => p.endsWith('.kt'));
   const csIndex = buildSuffixIndex(files, p => p.endsWith('.cs'));
 
-  const phpAutoloads = loadPhpAutoloads(projectRoot, files);
-
   return {
     projectRoot,
     fileSet,
@@ -1028,20 +1083,23 @@ function parseComposerAutoloadText(raw) {
  * at the bad file and skips it. The rest of the project's PHP imports keep
  * resolving via whichever composer.json files parsed cleanly.
  */
-function loadPhpAutoloads(projectRoot, files) {
+async function loadPhpAutoloads(projectRoot, files) {
   const out = new Map();
+  const warnings = [];
+  const candidates = [];
   for (const f of files) {
     const p = toPosix(f.path);
     const base = p.includes('/') ? p.slice(p.lastIndexOf('/') + 1) : p;
     if (base !== 'composer.json') continue;
     const absPath = join(projectRoot, p);
     if (!existsSync(absPath)) continue;
-    let raw;
-    try {
-      raw = readFileSync(absPath, 'utf-8');
-    } catch (err) {
-      process.stderr.write(
-        `Warning: extract-import-map: composer.json at ${absPath} failed ` +
+    candidates.push({ key: p, absPath });
+  }
+  const reads = await readFilesParallel(candidates);
+  for (const { key: p, raw, err } of reads) {
+    if (err) {
+      warnings.push(
+        `Warning: extract-import-map: composer.json at ${join(projectRoot, p)} failed ` +
         `to read (${err.message}) — PSR-4 namespace mapping from this ` +
         `composer.json unavailable — PHP imports under this package ` +
         `will not resolve\n`,
@@ -1050,8 +1108,8 @@ function loadPhpAutoloads(projectRoot, files) {
     }
     const parsed = parseComposerAutoloadText(raw);
     if (parsed === null) {
-      process.stderr.write(
-        `Warning: extract-import-map: composer.json at ${absPath} failed ` +
+      warnings.push(
+        `Warning: extract-import-map: composer.json at ${join(projectRoot, p)} failed ` +
         `to parse — PSR-4 namespace mapping unavailable — PHP imports ` +
         `under this package will not resolve\n`,
       );
@@ -1059,7 +1117,7 @@ function loadPhpAutoloads(projectRoot, files) {
     }
     out.set(dirOf(p), parsed);
   }
-  return out;
+  return { autoloads: out, warnings };
 }
 
 /**
@@ -1421,8 +1479,10 @@ async function main() {
     );
   }
 
-  // Build resolution context (cached configs)
-  const ctx = buildResolutionContext(projectRoot, files);
+  // Build resolution context (cached configs). The loader pass for the
+  // tsconfig/go.mod/composer.json files inside is parallelised — see
+  // `buildResolutionContext`.
+  const ctx = await buildResolutionContext(projectRoot, files);
 
   const importMap = {};
   let filesWithImports = 0;