fix: harden render concurrency, cache lifecycle, worker limits, and benchmark portability (#64)

* fix: harden render concurrency, cache lifecycle, worker limits, and benchmark portability

* chore: address review comments

Author: yashmehrotra

cli/scripts/benchmark-render.mjs

@@ -1,30 +1,22 @@
 #!/usr/bin/env node
 import { spawn } from 'node:child_process';
 import { mkdir, rm } from 'node:fs/promises';
-import { readFileSync } from 'node:fs';
 import { resolve } from 'node:path';
+import { fileURLToPath } from 'node:url';
 import { performance } from 'node:perf_hooks';
+import { processTreeRss } from './lib/proc-rss.mjs';
 
-const cli = resolve('../dist/facet');
-const template = resolve(process.argv[2] ?? 'examples/SimpleReport.tsx');
-const data = resolve(process.argv[3] ?? 'examples/simple-data.json');
+const scriptDir = fileURLToPath(new URL('.', import.meta.url));
+const cli = resolve(scriptDir, '../../dist/facet');
+const template = process.argv[2]
+  ? resolve(process.argv[2])
+  : resolve(scriptDir, '../examples/SimpleReport.tsx');
+const data = process.argv[3]
+  ? resolve(process.argv[3])
+  : resolve(scriptDir, '../examples/simple-data.json');
 const output = resolve('.benchmark-output');
 const iterations = Math.max(1, Number(process.env.FACET_BENCH_ITERATIONS ?? 5));
 
-function processTreeRss(pid, seen = new Set()) {
-  if (process.platform !== 'linux' || seen.has(pid)) return 0;
-  seen.add(pid);
-  let rss = 0;
-  try {
-    rss = Number(readFileSync(`/proc/${pid}/statm`, 'utf8').trim().split(/\s+/)[1] ?? 0) * 4096;
-  } catch { return 0; }
-  try {
-    const children = readFileSync(`/proc/${pid}/task/${pid}/children`, 'utf8').trim().split(/\s+/).filter(Boolean);
-    for (const child of children) rss += processTreeRss(Number(child), seen);
-  } catch { /* process exited while sampling */ }
-  return rss;
-}
-
 function run(format, cold) {
   return new Promise((resolveRun, reject) => {
     const args = [cli, format, template, '--data', data, '--output', output];

cli/scripts/benchmark-server.mjs

@@ -1,13 +1,17 @@
 #!/usr/bin/env node
 import { spawn } from 'node:child_process';
 import { mkdtemp, rm } from 'node:fs/promises';
-import { readFileSync } from 'node:fs';
 import { tmpdir } from 'node:os';
 import { join, resolve } from 'node:path';
+import { fileURLToPath } from 'node:url';
 import { performance } from 'node:perf_hooks';
+import { processTreeRss } from './lib/proc-rss.mjs';
 
-const cli = resolve('../dist/facet');
-const templatesDir = resolve(process.argv[2] ?? 'examples');
+const scriptDir = fileURLToPath(new URL('.', import.meta.url));
+const cli = resolve(scriptDir, '../../dist/facet');
+const templatesDir = process.argv[2]
+  ? resolve(process.argv[2])
+  : resolve(scriptDir, '../examples');
 const iterations = Math.max(1, Number(process.env.FACET_BENCH_ITERATIONS ?? 5));
 const sectionCount = Math.max(1, Number(process.env.FACET_BENCH_SECTIONS ?? 1));
 const templateName = process.env.FACET_BENCH_TEMPLATE ?? 'SimpleReport';
@@ -17,20 +21,6 @@ if (formats.length === 0) throw new Error('FACET_BENCH_FORMATS must contain html
 const port = Number(process.env.FACET_BENCH_PORT ?? 39123);
 const cacheDir = await mkdtemp(join(tmpdir(), 'facet-server-bench-'));
 
-function processTreeRss(pid, seen = new Set()) {
-  if (process.platform !== 'linux' || seen.has(pid)) return 0;
-  seen.add(pid);
-  let rss = 0;
-  try {
-    rss = Number(readFileSync(`/proc/${pid}/statm`, 'utf8').trim().split(/\s+/)[1] ?? 0) * 4096;
-  } catch { return 0; }
-  try {
-    const children = readFileSync(`/proc/${pid}/task/${pid}/children`, 'utf8').trim().split(/\s+/).filter(Boolean);
-    for (const child of children) rss += processTreeRss(Number(child), seen);
-  } catch { /* process exited while sampling */ }
-  return rss;
-}
-
 const server = spawn(cli, [
   'serve', '--port', String(port), '--templates-dir', templatesDir,
   '--workers', '1', '--cache-dir', cacheDir,
@@ -45,6 +35,7 @@ const sampler = setInterval(() => {
   peakRss = Math.max(peakRss, rss);
   requestPeakRss = Math.max(requestPeakRss, rss);
 }, 25);
+sampler.unref();
 
 async function waitForServer() {
   const deadline = Date.now() + 30_000;

cli/scripts/lib/proc-rss.mjs

@@ -0,0 +1,16 @@
+import { readFileSync } from 'node:fs';
+
+export function processTreeRss(pid, seen = new Set()) {
+  if (process.platform !== 'linux' || seen.has(pid)) return 0;
+  seen.add(pid);
+  let rss = 0;
+  try {
+    rss = Number(readFileSync(`/proc/${pid}/statm`, 'utf8').trim().split(/\s+/)[1] ?? 0) * 4096;
+  } catch { return 0; }
+  try {
+    const children = readFileSync(`/proc/${pid}/task/${pid}/children`, 'utf8')
+      .trim().split(/\s+/).filter(Boolean);
+    for (const child of children) rss += processTreeRss(Number(child), seen);
+  } catch { /* process exited while sampling */ }
+  return rss;
+}

cli/src/builders/facet-directory.test.ts

@@ -229,7 +229,7 @@ describe('FacetDirectory.generateViteConfig remark plugins', () => {
 describe('FacetDirectory.generatePackageJson .npmrc', () => {
   it('disables modules-purge confirmation so non-interactive pnpm runs do not abort', async () => {
     await writeFile(join(consumerRoot, 'package.json'), JSON.stringify({ name: 'consumer', version: '0.0.0' }));
-    newFacetDir().generatePackageJson();
+    await newFacetDir().generatePackageJson();
     const npmrc = await readFile(join(facetRoot, '.npmrc'), 'utf-8');
     expect(npmrc).toContain('confirm-modules-purge=false');
   });
@@ -260,7 +260,7 @@ describe('FACET_PACKAGE_PATH local directory override', () => {
     const localRoot = await writeLocalFacetPackage();
     process.env.FACET_PACKAGE_PATH = localRoot;
 
-    newFacetDir().generatePackageJson();
+    await newFacetDir().generatePackageJson();
 
     const generated = JSON.parse(await readFile(join(facetRoot, 'package.json'), 'utf-8'));
     expect(generated.dependencies['@flanksource/facet']).toBe(`file:${localRoot}`);
@@ -277,7 +277,7 @@ describe('FACET_PACKAGE_PATH local directory override', () => {
     await writeFile(tarball, 'fake tarball');
     process.env.FACET_PACKAGE_PATH = tarball;
 
-    newFacetDir().generatePackageJson();
+    await newFacetDir().generatePackageJson();
 
     const generated = JSON.parse(await readFile(join(facetRoot, 'package.json'), 'utf-8'));
     expect(generated.dependencies['@flanksource/facet']).toBe(`file:${tarball}`);
@@ -322,20 +322,43 @@ describe('FACET_PACKAGE_PATH local directory override', () => {
     expect(needsLocalFacetCssBuild(localRoot)).toBe(true);
   });
 
+  it('waits for a local build lock without blocking the event loop', async () => {
+    const localRoot = await writeLocalFacetPackage();
+    process.env.FACET_PACKAGE_PATH = localRoot;
+    const future = new Date(Date.now() + 120_000);
+    utimesSync(join(localRoot, 'src/components/index.tsx'), future, future);
+    const lockPath = join(localRoot, '.facet-local-build.lock');
+    await writeFile(lockPath, 'other-process\n');
+    let timerRan = false;
+    const timer = setTimeout(() => {
+      timerRan = true;
+      void rm(lockPath, { force: true });
+    }, 25);
+
+    try {
+      await newFacetDir().generatePackageJson();
+    } finally {
+      clearTimeout(timer);
+    }
+
+    expect(timerRan).toBe(true);
+    expect(existsSync(lockPath)).toBe(false);
+  });
+
   it('removes a stale installed local package copy when package.json is unchanged', async () => {
     const localRoot = await writeLocalFacetPackage();
     process.env.FACET_PACKAGE_PATH = localRoot;
     const facetDir = newFacetDir();
 
-    facetDir.generatePackageJson();
+    await facetDir.generatePackageJson();
 
     const installedRoot = join(facetRoot, 'node_modules/@flanksource/facet');
     await mkdir(join(installedRoot, 'dist/components'), { recursive: true });
     await writeFile(join(installedRoot, 'package.json'), await readFile(join(localRoot, 'package.json'), 'utf-8'));
     await writeFile(join(installedRoot, 'dist/components/index.js'), 'export const Marker = "stale";\n');
     await writeFile(join(facetRoot, 'pnpm-lock.yaml'), 'lockfileVersion: 9.0\n');
 
-    facetDir.generatePackageJson();
+    await facetDir.generatePackageJson();
 
     expect(existsSync(installedRoot)).toBe(false);
     expect(existsSync(join(facetRoot, 'pnpm-lock.yaml'))).toBe(false);

cli/src/builders/facet-directory.ts

@@ -690,12 +690,12 @@ export default defineConfig({
    * Generate package.json with all required build dependencies
    * Reads versions from embedded root-package.json and merges with consumer's dependencies
    */
-  generatePackageJson(): void {
+  async generatePackageJson(): Promise<void> {
     this.logger.debug('Generating package.json');
 
     const facetOverride = resolveFacetPackageOverride();
     if (facetOverride?.kind === 'directory') {
-      this.ensureLocalFacetPackageBuilt(facetOverride.path);
+      await this.ensureLocalFacetPackageBuilt(facetOverride.path);
     }
 
     let dependencies: Record<string, string> = {};
@@ -1003,15 +1003,15 @@ export default defineConfig({
     return readFileSync(embeddedPath, 'utf-8');
   }
 
-  private ensureLocalFacetPackageBuilt(packageRoot: string): void {
+  private async ensureLocalFacetPackageBuilt(packageRoot: string): Promise<void> {
     const isCurrent = (): boolean =>
       !needsLocalFacetComponentsBuild(packageRoot) && !needsLocalFacetCssBuild(packageRoot);
     if (isCurrent()) {
       this.logger.debug(`FACET_PACKAGE_PATH build outputs are current: ${packageRoot}`);
       return;
     }
 
-    this.withLocalFacetBuildLock(packageRoot, () => {
+    await this.withLocalFacetBuildLock(packageRoot, () => {
       // Another process may have completed the build while this process waited.
       if (isCurrent()) return;
       const shouldBuildCss = needsLocalFacetCssBuild(packageRoot);
@@ -1026,14 +1026,17 @@ export default defineConfig({
     });
   }
 
-  private withLocalFacetBuildLock(packageRoot: string, action: () => void): void {
+  private async withLocalFacetBuildLock(
+    packageRoot: string,
+    action: () => void | Promise<void>,
+  ): Promise<void> {
     const lockPath = join(packageRoot, '.facet-local-build.lock');
     const deadline = Date.now() + LOCAL_BUILD_TIMEOUT_MS;
     let fd: number | undefined;
     while (fd === undefined) {
+      let acquiredFd: number;
       try {
-        fd = openSync(lockPath, 'wx');
-        writeFileSync(fd, `${process.pid}\n`);
+        acquiredFd = openSync(lockPath, 'wx');
       } catch (error) {
         const code = (error as NodeJS.ErrnoException).code;
         if (code !== 'EEXIST') throw error;
@@ -1042,15 +1045,31 @@ export default defineConfig({
             unlinkSync(lockPath);
             continue;
           }
-        } catch { continue; }
+        } catch {
+          if (Date.now() >= deadline) {
+            throw new Error(`Timed out waiting for local Facet build lock: ${lockPath}`);
+          }
+          await new Promise<void>((resolveWait) => setTimeout(resolveWait, 100));
+          continue;
+        }
         if (Date.now() >= deadline) {
           throw new Error(`Timed out waiting for local Facet build lock: ${lockPath}`);
         }
-        Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, 100);
+        await new Promise<void>((resolveWait) => setTimeout(resolveWait, 100));
+        continue;
+      }
+
+      try {
+        writeFileSync(acquiredFd, `${process.pid}\n`);
+        fd = acquiredFd;
+      } catch (error) {
+        try { closeSync(acquiredFd); } catch { /* preserve the write error */ }
+        try { unlinkSync(lockPath); } catch { /* preserve the write error */ }
+        throw error;
       }
     }
     try {
-      action();
+      await action();
     } finally {
       closeSync(fd);
       try { unlinkSync(lockPath); } catch { /* already cleaned up */ }

cli/src/bundler/build-cache.ts

@@ -19,48 +19,77 @@ function extension(name: string): string {
   return index < 0 ? '' : name.slice(index).toLowerCase();
 }
 
+interface TreeDigestCacheEntry {
+  metadataDigest: string;
+  contentDigest: string;
+}
+
+const treeDigestCache = new Map<string, TreeDigestCacheEntry>();
+
 /** Hash template sources and build metadata, intentionally excluding render data. */
 export function computeTemplateBuildKey(
   consumerRoot: string,
   facetVersion: string,
   templatePath?: string,
 ): string {
-  const hash = createHash('sha256');
-  hash.update(`facet:${facetVersion}\0`);
-  if (templatePath) {
-    hash.update(`entry:${templatePath}\0`);
-    // Content-addressed generated fragments are excluded from the general tree
-    // to avoid invalidating the main template, so hash the selected entry here.
-    try {
-      hash.update(readFileSync(join(consumerRoot, templatePath)));
-      hash.update('\0');
-    } catch { /* the normal source traversal will report/build missing entries */ }
-  }
   const files: string[] = [];
-
   const visit = (dir: string): void => {
     for (const entry of readdirSync(dir, { withFileTypes: true })) {
       if (entry.isSymbolicLink()) continue;
       if (entry.isDirectory()) {
         if (!EXCLUDED_DIRS.has(entry.name)) visit(join(dir, entry.name));
         continue;
       }
-      if (!entry.isFile()) continue;
-      if (INCLUDED_METADATA.has(entry.name) || SOURCE_EXTENSIONS.has(extension(entry.name))) {
+      if (entry.isFile() && (INCLUDED_METADATA.has(entry.name) || SOURCE_EXTENSIONS.has(extension(entry.name)))) {
         files.push(join(dir, entry.name));
       }
     }
   };
 
   visit(consumerRoot);
   files.sort();
+  const selectedEntry = templatePath ? join(consumerRoot, templatePath) : undefined;
+  const metadataHash = createHash('sha256');
+  if (selectedEntry) {
+    try {
+      const stats = statSync(selectedEntry, { bigint: true });
+      metadataHash.update(`entry:${templatePath}\0${stats.size}:${stats.mtimeNs}\0`);
+    } catch { /* the normal source traversal will report/build missing entries */ }
+  }
   for (const file of files) {
-    hash.update(relative(consumerRoot, file));
-    hash.update('\0');
-    hash.update(readFileSync(file));
-    hash.update('\0');
+    const stats = statSync(file, { bigint: true });
+    metadataHash.update(relative(consumerRoot, file));
+    metadataHash.update(`\0${stats.size}:${stats.mtimeNs}\0`);
   }
-  return hash.digest('hex').slice(0, 24);
+
+  const cacheKey = `${consumerRoot}\0${templatePath ?? ''}`;
+  const metadataDigest = metadataHash.digest('hex');
+  let contentDigest = treeDigestCache.get(cacheKey)?.metadataDigest === metadataDigest
+    ? treeDigestCache.get(cacheKey)!.contentDigest
+    : undefined;
+  if (!contentDigest) {
+    const contentHash = createHash('sha256');
+    if (selectedEntry) {
+      try {
+        contentHash.update(`entry:${templatePath}\0`);
+        contentHash.update(readFileSync(selectedEntry));
+        contentHash.update('\0');
+      } catch { /* the normal source traversal will report/build missing entries */ }
+    }
+    for (const file of files) {
+      contentHash.update(relative(consumerRoot, file));
+      contentHash.update('\0');
+      contentHash.update(readFileSync(file));
+      contentHash.update('\0');
+    }
+    contentDigest = contentHash.digest('hex');
+    treeDigestCache.set(cacheKey, { metadataDigest, contentDigest });
+    if (treeDigestCache.size > 100) treeDigestCache.delete(treeDigestCache.keys().next().value!);
+  }
+
+  return createHash('sha256')
+    .update(`facet:${facetVersion}\0entry:${templatePath ?? ''}\0${contentDigest}`)
+    .digest('hex').slice(0, 24);
 }
 
 /** Keep the newest cache entries within a configurable count. */