fix(perf): share jiti module cache across extension loads (#3308)
* fix(perf): share jiti module cache across extension loads (#2108) Each extension was creating a new jiti instance with moduleCache: false, causing shared dependencies to be recompiled for every extension. Use a shared singleton with moduleCache: true so shared modules are compiled once. Export resetExtensionLoaderCache() for test teardown and explicit reload. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: correct loader path in extension-load-perf test (4 → 3 levels up) The test file is at src/tests/ (2 levels deep from repo root), so fileURLToPath(import.meta.url) + 3x'..' reaches the repo root. Using 4 levels exits the repo into the GitHub Actions workspace parent, causing ERR_MODULE_NOT_FOUND for loader.js in dist/. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: use process.cwd() for loader path in perf test (source/compiled portability) import.meta.url resolves to different depths in source (src/tests/) vs compiled (dist-test/src/tests/), so relative '../' navigation produces the wrong path in the build phase. process.cwd() is always the repo root in CI regardless of where the test file is compiled to. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: trek-e <trek-e@users.noreply.github.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
e9dabdc649
commit
4cb6252b9b
3 changed files with 160 additions and 5 deletions
|
|
@ -4,7 +4,7 @@ import * as fs from "node:fs";
|
|||
import * as os from "node:os";
|
||||
import * as path from "node:path";
|
||||
import { isProjectTrusted, trustProject, getUntrustedExtensionPaths } from "./project-trust.js";
|
||||
import { containsTypeScriptSyntax, loadExtensions } from "./loader.js";
|
||||
import { containsTypeScriptSyntax, loadExtensions, resetExtensionLoaderCache } from "./loader.js";
|
||||
|
||||
// ─── helpers ──────────────────────────────────────────────────────────────────
|
||||
|
||||
|
|
@ -235,3 +235,41 @@ describe("loadExtensions", () => {
|
|||
);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── resetExtensionLoaderCache ───────────────────────────────────────────────
|
||||
|
||||
describe("resetExtensionLoaderCache", () => {
|
||||
let tmpDir: string;
|
||||
|
||||
beforeEach(() => {
|
||||
tmpDir = makeTempDir();
|
||||
// Always start with a clean cache so tests are independent
|
||||
resetExtensionLoaderCache();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
resetExtensionLoaderCache();
|
||||
cleanDir(tmpDir);
|
||||
});
|
||||
|
||||
it("clears the jiti singleton so a fresh instance is created on next load", async () => {
|
||||
// Write a minimal valid extension that returns a name
|
||||
const extPath = path.join(tmpDir, "cache-ext.ts");
|
||||
fs.writeFileSync(
|
||||
extPath,
|
||||
`export default function activate(api: any) { return { name: "cache-ext" }; }\n`,
|
||||
);
|
||||
|
||||
// First load — creates the jiti singleton and caches the module
|
||||
const result1 = await loadExtensions([extPath], tmpDir);
|
||||
assert.equal(result1.extensions.length, 1, "first load should succeed");
|
||||
|
||||
// Reset the cache — nulls the singleton
|
||||
resetExtensionLoaderCache();
|
||||
|
||||
// Second load — should create a new jiti instance (not reuse the old one)
|
||||
// and still successfully load the extension
|
||||
const result2 = await loadExtensions([extPath], tmpDir);
|
||||
assert.equal(result2.extensions.length, 1, "load after reset should succeed with fresh jiti");
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -624,6 +624,39 @@ export function containsTypeScriptSyntax(source: string): boolean {
|
|||
return TS_SYNTAX_PATTERNS.some((pattern) => pattern.test(source));
|
||||
}
|
||||
|
||||
/**
|
||||
* Shared jiti instance for loading extension modules.
|
||||
*
|
||||
* Before this fix (#2108), each extension created a NEW jiti instance with
|
||||
* `moduleCache: false`, causing shared dependencies (e.g. @gsd/pi-agent-core)
|
||||
* to be recompiled for every extension — turning a ~3s parallel load into a
|
||||
* ~15-30s serial compilation bottleneck.
|
||||
*
|
||||
* Using a single shared instance with `moduleCache: true` means shared modules
|
||||
* are compiled once and reused across all extensions.
|
||||
*/
|
||||
let _extensionLoaderJiti: ReturnType<typeof createJiti> | null = null;
|
||||
|
||||
/**
|
||||
* Reset the shared jiti singleton so the next call to getExtensionLoaderJiti()
|
||||
* creates a fresh instance. This prevents memory leaks in long-running daemon
|
||||
* processes (every loaded module stays cached forever) and ensures stale modules
|
||||
* are not returned when extension source changes on disk.
|
||||
*/
|
||||
export function resetExtensionLoaderCache(): void {
|
||||
_extensionLoaderJiti = null;
|
||||
}
|
||||
|
||||
function getExtensionLoaderJiti() {
|
||||
if (!_extensionLoaderJiti) {
|
||||
_extensionLoaderJiti = createJiti(import.meta.url, {
|
||||
moduleCache: true,
|
||||
...getJitiOptions(),
|
||||
});
|
||||
}
|
||||
return _extensionLoaderJiti;
|
||||
}
|
||||
|
||||
async function loadExtensionModule(extensionPath: string) {
|
||||
// Pre-compiled extension loading: if the source is .ts and a sibling .js
|
||||
// file exists with matching or newer mtime, use native import() to skip
|
||||
|
|
@ -643,10 +676,7 @@ async function loadExtensionModule(extensionPath: string) {
|
|||
}
|
||||
}
|
||||
|
||||
const jiti = createJiti(import.meta.url, {
|
||||
moduleCache: false,
|
||||
...getJitiOptions(),
|
||||
});
|
||||
const jiti = getExtensionLoaderJiti();
|
||||
|
||||
const module = await jiti.import(extensionPath, { default: true });
|
||||
const factory = module as ExtensionFactory;
|
||||
|
|
|
|||
87
src/tests/extension-load-perf.test.ts
Normal file
87
src/tests/extension-load-perf.test.ts
Normal file
|
|
@ -0,0 +1,87 @@
|
|||
/**
|
||||
* Extension loading performance test
|
||||
*
|
||||
* Regression test for https://github.com/gsd-build/gsd-2/issues/2108
|
||||
*
|
||||
* Verifies that loading multiple extensions sharing common dependencies
|
||||
* does NOT re-compile those dependencies for each extension. The jiti
|
||||
* module cache must be shared across extension loads so that shared
|
||||
* modules are compiled once.
|
||||
*
|
||||
* Uses the built dist/ (not raw TS source) because pi-coding-agent uses
|
||||
* TypeScript features unsupported by --experimental-strip-types.
|
||||
*/
|
||||
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import { mkdtempSync, writeFileSync, mkdirSync, rmSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
import { tmpdir } from "node:os";
|
||||
|
||||
// Import loadExtensions from the compiled dist (it IS re-exported from the
|
||||
// core/extensions barrel but not from the top-level index).
|
||||
// Use process.cwd() rather than import.meta.url-relative navigation — the
|
||||
// compiled test lands in dist-test/src/tests/, so relative paths differ between
|
||||
// source and compiled contexts. process.cwd() is always the repo root in CI.
|
||||
const loaderPath = join(
|
||||
process.cwd(),
|
||||
"packages", "pi-coding-agent", "dist", "core", "extensions", "loader.js",
|
||||
);
|
||||
|
||||
test("loadExtensions shares module cache across extensions (perf regression #2108)", async () => {
|
||||
const { loadExtensions } = await import(loaderPath);
|
||||
|
||||
// Create a temp directory with two extensions that import a shared helper
|
||||
const tmp = mkdtempSync(join(tmpdir(), "gsd-perf-test-"));
|
||||
|
||||
try {
|
||||
// Shared helper module
|
||||
const sharedDir = join(tmp, "shared");
|
||||
mkdirSync(sharedDir, { recursive: true });
|
||||
writeFileSync(
|
||||
join(sharedDir, "helper.ts"),
|
||||
`export const SHARED_VALUE = "shared-${Date.now()}";\n`,
|
||||
);
|
||||
|
||||
// Extension A — imports the shared helper
|
||||
const extADir = join(tmp, "ext-a");
|
||||
mkdirSync(extADir, { recursive: true });
|
||||
writeFileSync(
|
||||
join(extADir, "index.ts"),
|
||||
`import { SHARED_VALUE } from "${join(sharedDir, "helper.ts").replace(/\\/g, "/")}";\n` +
|
||||
`export default function(api: any) {\n` +
|
||||
` api.registerCommand("ext-a-cmd", { description: "test A " + SHARED_VALUE, handler: async () => {} });\n` +
|
||||
`}\n`,
|
||||
);
|
||||
|
||||
// Extension B — imports the same shared helper
|
||||
const extBDir = join(tmp, "ext-b");
|
||||
mkdirSync(extBDir, { recursive: true });
|
||||
writeFileSync(
|
||||
join(extBDir, "index.ts"),
|
||||
`import { SHARED_VALUE } from "${join(sharedDir, "helper.ts").replace(/\\/g, "/")}";\n` +
|
||||
`export default function(api: any) {\n` +
|
||||
` api.registerCommand("ext-b-cmd", { description: "test B " + SHARED_VALUE, handler: async () => {} });\n` +
|
||||
`}\n`,
|
||||
);
|
||||
|
||||
const paths = [join(extADir, "index.ts"), join(extBDir, "index.ts")];
|
||||
const start = Date.now();
|
||||
const result = await loadExtensions(paths, tmp);
|
||||
const elapsed = Date.now() - start;
|
||||
|
||||
// Both extensions should load without errors
|
||||
assert.strictEqual(result.errors.length, 0, `Extension errors: ${JSON.stringify(result.errors)}`);
|
||||
assert.strictEqual(result.extensions.length, 2, "Expected 2 extensions to load");
|
||||
|
||||
// With shared jiti cache, loading 2 trivial extensions that share a
|
||||
// dependency should complete in well under 5 seconds.
|
||||
assert.ok(
|
||||
elapsed < 5000,
|
||||
`Extension loading took ${elapsed}ms — expected < 5000ms. ` +
|
||||
`This suggests jiti module caching is not shared across extensions.`,
|
||||
);
|
||||
} finally {
|
||||
try { rmSync(tmp, { recursive: true, force: true, maxRetries: 3 }); } catch { /* cleanup */ }
|
||||
}
|
||||
});
|
||||
Loading…
Add table
Reference in a new issue