diff --git a/cli/scripts/benchmark-render.mjs b/cli/scripts/benchmark-render.mjs index 4e18c6e..2df8136 100644 --- a/cli/scripts/benchmark-render.mjs +++ b/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]; diff --git a/cli/scripts/benchmark-server.mjs b/cli/scripts/benchmark-server.mjs index 100b733..4aa8369 100644 --- a/cli/scripts/benchmark-server.mjs +++ b/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; diff --git a/cli/scripts/lib/proc-rss.mjs b/cli/scripts/lib/proc-rss.mjs new file mode 100644 index 0000000..28b8b39 --- /dev/null +++ b/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; +} diff --git a/cli/src/builders/facet-directory.test.ts b/cli/src/builders/facet-directory.test.ts index 49f4cf7..cb7db2f 100644 --- a/cli/src/builders/facet-directory.test.ts +++ b/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,12 +322,35 @@ 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 }); @@ -335,7 +358,7 @@ describe('FACET_PACKAGE_PATH local directory override', () => { 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); diff --git a/cli/src/builders/facet-directory.ts b/cli/src/builders/facet-directory.ts index e1239dd..9050dcd 100644 --- a/cli/src/builders/facet-directory.ts +++ b/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 { 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 = {}; @@ -1003,7 +1003,7 @@ export default defineConfig({ return readFileSync(embeddedPath, 'utf-8'); } - private ensureLocalFacetPackageBuilt(packageRoot: string): void { + private async ensureLocalFacetPackageBuilt(packageRoot: string): Promise { const isCurrent = (): boolean => !needsLocalFacetComponentsBuild(packageRoot) && !needsLocalFacetCssBuild(packageRoot); if (isCurrent()) { @@ -1011,7 +1011,7 @@ export default defineConfig({ 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, + ): Promise { 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((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((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 */ } diff --git a/cli/src/bundler/build-cache.ts b/cli/src/bundler/build-cache.ts index 74d13f0..1245f91 100644 --- a/cli/src/bundler/build-cache.ts +++ b/cli/src/bundler/build-cache.ts @@ -19,25 +19,20 @@ function extension(name: string): string { return index < 0 ? '' : name.slice(index).toLowerCase(); } +interface TreeDigestCacheEntry { + metadataDigest: string; + contentDigest: string; +} + +const treeDigestCache = new Map(); + /** 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; @@ -45,8 +40,7 @@ export function computeTemplateBuildKey( 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)); } } @@ -54,13 +48,48 @@ export function computeTemplateBuildKey( 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. */ diff --git a/cli/src/bundler/ssr-pool.ts b/cli/src/bundler/ssr-pool.ts index da1cdaf..ed13599 100644 --- a/cli/src/bundler/ssr-pool.ts +++ b/cli/src/bundler/ssr-pool.ts @@ -1,5 +1,6 @@ import { spawn, type ChildProcessWithoutNullStreams } from 'node:child_process'; import { createInterface } from 'node:readline'; +import { AsyncLocalStorage } from 'node:async_hooks'; import { selfExecBase } from '../utils/self-exec.js'; import type { Logger } from '../utils/logger.js'; @@ -28,6 +29,7 @@ class SsrLoaderProcess { private stderr = ''; private exited = false; private idleTimer?: ReturnType; + private lastUsedAt = Date.now(); constructor( readonly facetRoot: string, @@ -52,7 +54,8 @@ class SsrLoaderProcess { this.pending.delete(reply.id); if (reply.result) pending.resolve(reply.result); else pending.reject(new Error(reply.error ?? 'Persistent SSR loader failed')); - this.scheduleIdleShutdown(); + this.lastUsedAt = Date.now(); + if (!this.hasPending()) this.scheduleIdleShutdown(); }); const fail = (error: Error): void => { if (this.exited) return; @@ -69,14 +72,23 @@ class SsrLoaderProcess { private scheduleIdleShutdown(): void { if (this.idleTimer) clearTimeout(this.idleTimer); - const idleMs = Math.max(1_000, parseInt(process.env['FACET_SSR_LOADER_IDLE_MS'] ?? '300000', 10)); + const idleMs = validEnvInteger('FACET_SSR_LOADER_IDLE_MS', 300_000, 1_000); this.idleTimer = setTimeout(() => void this.close(), idleMs); this.idleTimer.unref(); } + hasPending(): boolean { + return this.pending.size > 0; + } + + lastUsed(): number { + return this.lastUsedAt; + } + request(data: Record, cacheKey: string, verbose = false): Promise { if (this.exited) return Promise.reject(new Error('Persistent SSR loader is not running')); if (this.idleTimer) clearTimeout(this.idleTimer); + this.lastUsedAt = Date.now(); const id = this.nextId++; return new Promise((resolve, reject) => { this.pending.set(id, { resolve, reject }); @@ -84,6 +96,7 @@ class SsrLoaderProcess { if (!error) return; this.pending.delete(id); reject(error); + if (!this.hasPending()) this.scheduleIdleShutdown(); }); }); } @@ -107,39 +120,108 @@ class SsrLoaderProcess { } } +function validEnvInteger(name: string, fallback: number, minimum: number): number { + const raw = process.env[name]; + if (raw === undefined || raw.trim() === '') return fallback; + const parsed = Number(raw); + if (!Number.isFinite(parsed) || !Number.isInteger(parsed) || parsed < 0) return fallback; + return Math.max(minimum, parsed); +} + +export type PersistentSsrLoaderOwner = object; + const loaders = new Map(); +const closingLoaders = new Set(); +const loaderOwners = new Map>(); +const unownedLoaders = new Set(); +const ownerContext = new AsyncLocalStorage(); + +export function withPersistentSsrLoaderOwner( + owner: PersistentSsrLoaderOwner, + action: () => Promise, +): Promise { + return ownerContext.run(owner, action); +} -export function runPersistentSsrLoader( +export async function runPersistentSsrLoader( request: PersistentLoaderRequest, logger: Logger, ): Promise { let loader = loaders.get(request.facetRoot); if (!loader) { - const maxLoaders = Math.max(1, parseInt(process.env['FACET_MAX_SSR_LOADERS'] ?? '4', 10)); - if (loaders.size >= maxLoaders) { - const oldest = loaders.entries().next().value as [string, SsrLoaderProcess] | undefined; - if (oldest) { - loaders.delete(oldest[0]); - void oldest[1].close(); + const maxLoaders = validEnvInteger('FACET_MAX_SSR_LOADERS', 4, 1); + if (loaders.size + closingLoaders.size >= maxLoaders) { + const oldest = [...loaders.entries()] + .filter(([, candidate]) => !candidate.hasPending()) + .sort(([, left], [, right]) => left.lastUsed() - right.lastUsed())[0]; + if (!oldest) { + throw new Error(`Persistent SSR loader pool exhausted (${maxLoaders} active loaders)`); + } + const [evictedRoot, evictedLoader] = oldest; + if (loaders.get(evictedRoot) === evictedLoader) { + loaders.delete(evictedRoot); + loaderOwners.delete(evictedRoot); + unownedLoaders.delete(evictedRoot); + } + closingLoaders.add(evictedLoader); + try { + await evictedLoader.close(); + } finally { + closingLoaders.delete(evictedLoader); } } - let created!: SsrLoaderProcess; - created = new SsrLoaderProcess(request.facetRoot, logger, () => { - if (loaders.get(request.facetRoot) === created) loaders.delete(request.facetRoot); - }); - loader = created; - loaders.set(request.facetRoot, loader); - logger.debug(`Started persistent SSR loader: ${request.facetRoot}`); + // Another request may have created this root while eviction was closing. + loader = loaders.get(request.facetRoot); + if (!loader) { + let created!: SsrLoaderProcess; + created = new SsrLoaderProcess(request.facetRoot, logger, () => { + if (loaders.get(request.facetRoot) === created) { + loaders.delete(request.facetRoot); + loaderOwners.delete(request.facetRoot); + unownedLoaders.delete(request.facetRoot); + } + }); + loader = created; + loaders.set(request.facetRoot, loader); + logger.debug(`Started persistent SSR loader: ${request.facetRoot}`); + } } else { // Refresh insertion order for simple LRU eviction. loaders.delete(request.facetRoot); loaders.set(request.facetRoot, loader); } + + const owner = ownerContext.getStore(); + if (owner) { + const owners = loaderOwners.get(request.facetRoot) ?? new Set(); + owners.add(owner); + loaderOwners.set(request.facetRoot, owners); + } else { + unownedLoaders.add(request.facetRoot); + } return loader.request(request.data, request.cacheKey, request.verbose); } -export async function shutdownPersistentSsrLoaders(): Promise { - const active = [...loaders.values()]; - loaders.clear(); - await Promise.allSettled(active.map((loader) => loader.close())); +export async function shutdownPersistentSsrLoaders(owner?: PersistentSsrLoaderOwner): Promise { + if (!owner) { + const active = [...loaders.values()]; + loaders.clear(); + loaderOwners.clear(); + unownedLoaders.clear(); + await Promise.allSettled(active.map((loader) => loader.close())); + return; + } + + const closing: SsrLoaderProcess[] = []; + for (const [facetRoot, owners] of loaderOwners) { + owners.delete(owner); + if (owners.size > 0 || unownedLoaders.has(facetRoot)) continue; + loaderOwners.delete(facetRoot); + const loader = loaders.get(facetRoot); + if (loader) { + loaders.delete(facetRoot); + closing.push(loader); + } + } + await Promise.allSettled(closing.map((loader) => loader.close())); } diff --git a/cli/src/bundler/vite-builder.ts b/cli/src/bundler/vite-builder.ts index c897fa3..c0aaf9f 100644 --- a/cli/src/bundler/vite-builder.ts +++ b/cli/src/bundler/vite-builder.ts @@ -223,7 +223,7 @@ async function buildTemplateUnlocked(options: BuildOptions): Promise number { + return (value: string): number => { + const parsed = Number(value); + if (!Number.isSafeInteger(parsed) || parsed < minimum) { + throw new InvalidArgumentError(`Expected an integer greater than or equal to ${minimum}`); + } + return parsed; + }; +} + function parseDataLoaderArgs(): string[] { const dashIndex = process.argv.indexOf('--'); return dashIndex !== -1 ? process.argv.slice(dashIndex + 1) : []; @@ -258,11 +268,11 @@ program .option('-p, --port ', 'Server port', '3010') .option('--templates-dir ', 'Directory containing templates', '.') .option('--workers ', 'Number of browser workers', '2') - .option('--max-renders-per-worker ', 'Recycle Chromium after this many renders (default: 50)') - .option('--max-queue-depth ', 'Maximum requests waiting for a browser (default: 20)') - .option('--max-worker-age ', 'Recycle Chromium after this age in milliseconds (default: 1800000)') - .option('--max-worker-rss ', 'Recycle Chromium above this Linux process-tree RSS (default: 0/off)') - .option('--worker-acquire-timeout ', 'Maximum time to wait for a browser worker (default: 30000)') + .option('--max-renders-per-worker ', 'Recycle Chromium after this many renders (default: 50)', numericOption(1)) + .option('--max-queue-depth ', 'Maximum requests waiting for a browser (default: 20)', numericOption(1)) + .option('--max-worker-age ', 'Recycle Chromium after this age in milliseconds (default: 1800000)', numericOption(1)) + .option('--max-worker-rss ', 'Recycle Chromium above this Linux process-tree RSS (default: 0/off)', numericOption(0)) + .option('--worker-acquire-timeout ', 'Maximum time to wait for a browser worker (default: 30000)', numericOption(1)) .option('--no-persistent-ssr', 'Disable persistent SSR loaders to reduce idle memory') .option('--timeout ', 'Render timeout in milliseconds', '60000') .option('--api-key ', 'API key for authentication') diff --git a/cli/src/loaders/ssr.ts b/cli/src/loaders/ssr.ts index 9d274a1..263f859 100644 --- a/cli/src/loaders/ssr.ts +++ b/cli/src/loaders/ssr.ts @@ -111,20 +111,27 @@ async function load(args: LoaderArgs): Promise { if (!cachedBundle) { const buildDir = cacheKey ? `${outDir}.tmp-${crypto.randomUUID()}` : outDir; if (cacheKey) mkdirSync(cacheRoot, { recursive: true }); - await build({ - configFile: viteConfigPath, - root: facetRoot, - logLevel: verbose ? 'info' : 'error', - build: { ssr: true, outDir: buildDir, emptyOutDir: true }, - }); - if (cacheKey) { - try { - renameSync(buildDir, outDir); - } catch (error) { - // Another process may have completed the same content-addressed build. - rmSync(buildDir, { recursive: true, force: true }); - if (!existsSync(outDir)) throw error; + try { + await build({ + configFile: viteConfigPath, + root: facetRoot, + logLevel: verbose ? 'info' : 'error', + build: { ssr: true, outDir: buildDir, emptyOutDir: true }, + }); + if (cacheKey) { + try { + renameSync(buildDir, outDir); + } catch (error) { + // Another process may have completed the same content-addressed build. + try { rmSync(buildDir, { recursive: true, force: true }); } catch { /* best effort */ } + if (!existsSync(outDir)) throw error; + } + } + } catch (error) { + if (cacheKey) { + try { rmSync(buildDir, { recursive: true, force: true }); } catch { /* preserve build error */ } } + throw error; } } else { const now = new Date(); diff --git a/cli/src/server/config.test.ts b/cli/src/server/config.test.ts new file mode 100644 index 0000000..db866f0 --- /dev/null +++ b/cli/src/server/config.test.ts @@ -0,0 +1,50 @@ +import { afterEach, describe, expect, it } from 'vitest'; +import { loadConfig } from './config.js'; + +const limitEnvNames = [ + 'FACET_MAX_RENDERS_PER_WORKER', + 'FACET_MAX_QUEUE_DEPTH', + 'FACET_MAX_WORKER_AGE_MS', + 'FACET_MAX_WORKER_RSS_MB', + 'FACET_WORKER_ACQUIRE_TIMEOUT_MS', +] as const; +const savedEnv = new Map(limitEnvNames.map((name) => [name, process.env[name]])); + +afterEach(() => { + for (const [name, value] of savedEnv) { + if (value === undefined) delete process.env[name]; + else process.env[name] = value; + } +}); + +describe('loadConfig worker limits', () => { + it('falls back to defaults for invalid environment values', () => { + process.env.FACET_MAX_RENDERS_PER_WORKER = 'NaN'; + process.env.FACET_MAX_QUEUE_DEPTH = '0'; + process.env.FACET_MAX_WORKER_AGE_MS = '1.5'; + process.env.FACET_MAX_WORKER_RSS_MB = '-1'; + process.env.FACET_WORKER_ACQUIRE_TIMEOUT_MS = 'Infinity'; + + const config = loadConfig({}); + expect(config.maxRendersPerWorker).toBe(50); + expect(config.maxQueueDepth).toBe(20); + expect(config.maxWorkerAgeMs).toBe(1_800_000); + expect(config.maxWorkerRssMb).toBe(0); + expect(config.workerAcquireTimeoutMs).toBe(30_000); + }); + + it('accepts safe integers at or above each minimum', () => { + const config = loadConfig({ + maxRendersPerWorker: 2, + maxQueueDepth: 3, + maxWorkerAge: 4, + maxWorkerRss: 0, + workerAcquireTimeout: 5, + }); + expect(config.maxRendersPerWorker).toBe(2); + expect(config.maxQueueDepth).toBe(3); + expect(config.maxWorkerAgeMs).toBe(4); + expect(config.maxWorkerRssMb).toBe(0); + expect(config.workerAcquireTimeoutMs).toBe(5); + }); +}); diff --git a/cli/src/server/config.ts b/cli/src/server/config.ts index 8ec2d6f..81f1a2d 100644 --- a/cli/src/server/config.ts +++ b/cli/src/server/config.ts @@ -32,11 +32,11 @@ export interface ServerCLIFlags { port?: string; templatesDir?: string; workers?: string; - maxRendersPerWorker?: string; - maxQueueDepth?: string; - maxWorkerAge?: string; - maxWorkerRss?: string; - workerAcquireTimeout?: string; + maxRendersPerWorker?: string | number; + maxQueueDepth?: string | number; + maxWorkerAge?: string | number; + maxWorkerRss?: string | number; + workerAcquireTimeout?: string | number; persistentSsr?: boolean; timeout?: string; apiKey?: string; @@ -51,16 +51,22 @@ export interface ServerCLIFlags { sandbox?: string | boolean; } +function workerLimit(value: string | number | undefined, fallback: number, minimum: number): number { + if (value === undefined) return fallback; + const parsed = Number(value); + return Number.isSafeInteger(parsed) && parsed >= minimum ? parsed : fallback; +} + export function loadConfig(flags: ServerCLIFlags): ServerConfig { const config: ServerConfig = { port: parseInt(flags.port ?? process.env['FACET_PORT'] ?? '3010', 10), templatesDir: flags.templatesDir ?? process.env['FACET_TEMPLATES_DIR'] ?? '.', workers: parseInt(flags.workers ?? process.env['FACET_WORKERS'] ?? '2', 10), - maxRendersPerWorker: parseInt(flags.maxRendersPerWorker ?? process.env['FACET_MAX_RENDERS_PER_WORKER'] ?? '50', 10), - maxQueueDepth: parseInt(flags.maxQueueDepth ?? process.env['FACET_MAX_QUEUE_DEPTH'] ?? '20', 10), - maxWorkerAgeMs: parseInt(flags.maxWorkerAge ?? process.env['FACET_MAX_WORKER_AGE_MS'] ?? '1800000', 10), - maxWorkerRssMb: parseInt(flags.maxWorkerRss ?? process.env['FACET_MAX_WORKER_RSS_MB'] ?? '0', 10), - workerAcquireTimeoutMs: parseInt(flags.workerAcquireTimeout ?? process.env['FACET_WORKER_ACQUIRE_TIMEOUT_MS'] ?? '30000', 10), + maxRendersPerWorker: workerLimit(flags.maxRendersPerWorker ?? process.env['FACET_MAX_RENDERS_PER_WORKER'], 50, 1), + maxQueueDepth: workerLimit(flags.maxQueueDepth ?? process.env['FACET_MAX_QUEUE_DEPTH'], 20, 1), + maxWorkerAgeMs: workerLimit(flags.maxWorkerAge ?? process.env['FACET_MAX_WORKER_AGE_MS'], 1_800_000, 1), + maxWorkerRssMb: workerLimit(flags.maxWorkerRss ?? process.env['FACET_MAX_WORKER_RSS_MB'], 0, 0), + workerAcquireTimeoutMs: workerLimit(flags.workerAcquireTimeout ?? process.env['FACET_WORKER_ACQUIRE_TIMEOUT_MS'], 30_000, 1), persistentSsr: flags.persistentSsr ?? process.env['FACET_PERSISTENT_SSR'] !== 'false', renderTimeout: parseInt(flags.timeout ?? process.env['FACET_RENDER_TIMEOUT'] ?? '60000', 10), apiKey: flags.apiKey ?? process.env['FACET_API_KEY'], diff --git a/cli/src/server/preview.ts b/cli/src/server/preview.ts index 42789f0..e5e19ab 100644 --- a/cli/src/server/preview.ts +++ b/cli/src/server/preview.ts @@ -6,7 +6,7 @@ import { loadConfig, type ServerCLIFlags, type ServerConfig } from './config.js' import { checkAuth } from './auth.js'; import { errorResponse } from './errors.js'; import { WorkerPool } from './worker-pool.js'; -import { shutdownPersistentSsrLoaders } from '../bundler/ssr-pool.js'; +import { shutdownPersistentSsrLoaders, withPersistentSsrLoaderOwner } from '../bundler/ssr-pool.js'; import { discoverTemplates, type TemplateInfo } from './templates.js'; import { S3Uploader } from './s3.js'; import { handleHealthz, handleTemplates, handleRender, handleRenderStream, handleResultsRoute } from './routes.js'; @@ -35,6 +35,7 @@ export interface ServerHandle { export async function createServer(config: ServerConfig): Promise { const logger = new Logger(config.verbose); + const ssrLoaderOwner = {}; const templatesDir = resolve(process.cwd(), config.templatesDir); logger.info(`Templates directory: ${templatesDir}`); @@ -124,7 +125,7 @@ export async function createServer(config: ServerConfig): Promise const httpServer = createHttpServer((req, res) => { Promise.resolve() .then(() => nodeToWebRequest(req)) - .then((request) => handleRequest(request)) + .then((request) => withPersistentSsrLoaderOwner(ssrLoaderOwner, () => handleRequest(request))) .then((response) => writeWebResponse(res, response)) .catch((err) => { logger.error(`Request handler error: ${err instanceof Error ? err.message : String(err)}`); @@ -153,7 +154,7 @@ export async function createServer(config: ServerConfig): Promise await new Promise((resolveClose) => httpServer.close(() => resolveClose())); await Promise.all([ pool.shutdown(), - shutdownPersistentSsrLoaders(), + shutdownPersistentSsrLoaders(ssrLoaderOwner), ]); }, }; diff --git a/cli/src/server/routes.ts b/cli/src/server/routes.ts index b137f8f..dd03ec8 100644 --- a/cli/src/server/routes.ts +++ b/cli/src/server/routes.ts @@ -1,4 +1,4 @@ -import { mkdtemp, mkdir, writeFile } from 'fs/promises'; +import { mkdtemp, mkdir, readdir, rm, stat, writeFile } from 'fs/promises'; import { createHash } from 'node:crypto'; import { join, basename, resolve } from 'path'; import { tmpdir } from 'os'; @@ -480,6 +480,22 @@ async function renderHTMLStreamed( } } +async function pruneFragments(fragmentDir: string, keepPath: string): Promise { + const configured = Number(process.env['FACET_FRAGMENT_MAX_AGE_MS'] ?? 86_400_000); + const maxAgeMs = Number.isFinite(configured) && configured >= 60_000 ? configured : 86_400_000; + const cutoff = Date.now() - maxAgeMs; + try { + const entries = await readdir(fragmentDir, { withFileTypes: true }); + await Promise.all(entries + .filter((entry) => entry.isFile() && entry.name.endsWith('.tsx')) + .map(async (entry) => { + const path = join(fragmentDir, entry.name); + if (path === keepPath || (await stat(path)).mtimeMs >= cutoff) return; + await rm(path, { force: true }); + })); + } catch { /* pruning is best effort under concurrent workspace activity */ } +} + async function buildFragment( consumerRoot: string, filename: string, @@ -492,7 +508,9 @@ async function buildFragment( const contentKey = createHash('sha256').update(code).digest('hex').slice(0, 20); const fragmentPath = join('.facet-fragments', `${filename.replace(/\.tsx$/, '')}-${contentKey}.tsx`); await mkdir(fragmentDir, { recursive: true }); - await writeFile(join(consumerRoot, fragmentPath), code, 'utf-8'); + const absoluteFragmentPath = join(consumerRoot, fragmentPath); + await writeFile(absoluteFragmentPath, code, 'utf-8'); + await pruneFragments(fragmentDir, absoluteFragmentPath); const result = await buildTemplate({ templatePath: fragmentPath, data, diff --git a/cli/src/server/worker-pool.test.ts b/cli/src/server/worker-pool.test.ts index 6862da8..217497b 100644 --- a/cli/src/server/worker-pool.test.ts +++ b/cli/src/server/worker-pool.test.ts @@ -15,4 +15,21 @@ describe('WorkerPool queue policy', () => { await expect(pool.acquire()).rejects.toThrow('Too many concurrent requests'); await expect(first).rejects.toThrow('Timed out waiting for a browser worker'); }); + + it('finishes recycle bookkeeping during shutdown', async () => { + const pool = new WorkerPool(0); + const internals = pool as unknown as { + shuttingDown: boolean; + recycle: (worker: unknown) => Promise; + }; + internals.shuttingDown = true; + await internals.recycle({ + browser: { close: async () => undefined }, + renders: 0, + startedAt: Date.now(), + lastRssMb: 0, + lastRssAt: 0, + }); + expect(pool.stats().recycling).toBe(0); + }); }); diff --git a/cli/src/server/worker-pool.ts b/cli/src/server/worker-pool.ts index 3efa6dc..2edae8f 100644 --- a/cli/src/server/worker-pool.ts +++ b/cli/src/server/worker-pool.ts @@ -9,6 +9,7 @@ export interface Worker { renders: number; startedAt: number; lastRssMb: number; + lastRssAt: number; } export interface WorkerPoolOptions { @@ -75,7 +76,7 @@ export class WorkerPool { // Start sequentially to avoid a large transient memory spike. for (let i = 0; i < this.size; i++) { const browser = await launchBrowser(); - this.available.push({ browser, renders: 0, startedAt: Date.now(), lastRssMb: 0 }); + this.available.push({ browser, renders: 0, startedAt: Date.now(), lastRssMb: 0, lastRssAt: 0 }); this.total++; } this.logger.info(`Worker pool ready (${this.total} browsers)`); @@ -123,6 +124,7 @@ export class WorkerPool { worker.renders++; worker.lastRssMb = chromiumTreeRssMb(worker.browser); + worker.lastRssAt = Date.now(); const expired = Date.now() - worker.startedAt >= this.limits.maxWorkerAgeMs; const overMemoryLimit = this.limits.maxWorkerRssMb > 0 && worker.lastRssMb >= this.limits.maxWorkerRssMb; if (overMemoryLimit) { @@ -145,32 +147,42 @@ export class WorkerPool { private async recycle(worker: Worker): Promise { this.recycling++; try { - await worker.browser.close(); - } catch { - // ignore close errors - } - - if (this.shuttingDown) { - this.total--; - return; - } - - try { - const browser = await launchBrowser(); - const fresh: Worker = { browser, renders: 0, startedAt: Date.now(), lastRssMb: 0 }; + try { + await worker.browser.close(); + } catch { + // ignore close errors + } - if (this.waiting.length > 0) { - this.waiting.shift()!.resolve(fresh); - } else { - this.available.push(fresh); + if (this.shuttingDown) { + this.total = Math.max(0, this.total - 1); + return; } - } catch (err) { - this.total--; - this.logger.error(`Failed to recycle browser: ${err}`); - if (this.waiting.length > 0) { - this.waiting.shift()!.reject( - new RenderError('RENDER_FAILED', 'No browsers available', 503), - ); + + try { + const browser = await launchBrowser(); + const fresh: Worker = { + browser, renders: 0, startedAt: Date.now(), lastRssMb: 0, lastRssAt: 0, + }; + + if (this.shuttingDown) { + try { await browser.close(); } catch { /* shutdown is best effort */ } + this.total = Math.max(0, this.total - 1); + return; + } + + if (this.waiting.length > 0) { + this.waiting.shift()!.resolve(fresh); + } else { + this.available.push(fresh); + } + } catch (err) { + this.total = Math.max(0, this.total - 1); + this.logger.error(`Failed to recycle browser: ${err}`); + if (this.waiting.length > 0) { + this.waiting.shift()!.reject( + new RenderError('RENDER_FAILED', 'No browsers available', 503), + ); + } } } finally { this.recycling--; @@ -179,6 +191,7 @@ export class WorkerPool { stats(): { total: number; available: number; active: number; recycling: number; waiting: number; chromiumRssMb: number } { const workers = [...this.available, ...this.active]; + const now = Date.now(); return { total: this.total, available: this.available.length, @@ -186,9 +199,11 @@ export class WorkerPool { recycling: this.recycling, waiting: this.waiting.length, chromiumRssMb: Number(workers.reduce((total, worker) => { - const rss = chromiumTreeRssMb(worker.browser); - worker.lastRssMb = rss; - return total + rss; + if (now - worker.lastRssAt >= 5_000) { + worker.lastRssMb = chromiumTreeRssMb(worker.browser); + worker.lastRssAt = now; + } + return total + worker.lastRssMb; }, 0).toFixed(1)), }; } diff --git a/cli/src/utils/browser-readiness.ts b/cli/src/utils/browser-readiness.ts index 9883554..8fb466a 100644 --- a/cli/src/utils/browser-readiness.ts +++ b/cli/src/utils/browser-readiness.ts @@ -19,13 +19,16 @@ export async function waitForPageReady( await page.evaluate( async ({ timeout, facet }) => { + const deadline = performance.now() + timeout; const withTimeout = async (promise: Promise): Promise => { + const remaining = Math.max(0, deadline - performance.now()); + if (remaining === 0) return; let timer: ReturnType | undefined; try { await Promise.race([ promise, new Promise((resolve) => { - timer = setTimeout(resolve, timeout); + timer = setTimeout(resolve, remaining); }), ]); } finally { diff --git a/cli/src/utils/tailwind.test.ts b/cli/src/utils/tailwind.test.ts index e8a6dd2..1dad08b 100644 --- a/cli/src/utils/tailwind.test.ts +++ b/cli/src/utils/tailwind.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; -import { mkdtempSync, mkdirSync, writeFileSync, rmSync, chmodSync } from 'fs'; +import { mkdtempSync, mkdirSync, writeFileSync, rmSync, chmodSync, statSync, utimesSync } from 'fs'; import { tmpdir } from 'os'; import { join } from 'path'; import { @@ -7,6 +7,7 @@ import { tailwindBinExists, runTailwind, renderedClassKey, + runTailwindCached, TailwindBinNotFoundError, } from './tailwind.js'; @@ -24,6 +25,31 @@ describe('renderedClassKey', () => { }); }); +describe('runTailwindCached', () => { + it('refreshes a cache hit timestamp', async () => { + const facetRoot = mkdtempSync(join(tmpdir(), 'facet-tw-cache-test-')); + try { + const html = '
'; + const cacheDir = join(facetRoot, 'tailwind-cache'); + const cachePath = join(cacheDir, `build-${renderedClassKey(html).key}.css`); + mkdirSync(cacheDir, { recursive: true }); + writeFileSync(cachePath, '.font-bold{font-weight:700}'); + const old = new Date(Date.now() - 60_000); + utimesSync(cachePath, old, old); + + await expect(runTailwindCached({ + facetRoot, + stylesInput: join(facetRoot, 'styles.css'), + html, + buildCacheKey: 'build', + })).resolves.toContain('font-weight'); + expect(statSync(cachePath).mtimeMs).toBeGreaterThan(old.getTime()); + } finally { + rmSync(facetRoot, { recursive: true, force: true }); + } + }); +}); + describe('resolveTailwindBin', () => { const originalPlatform = process.platform; diff --git a/cli/src/utils/tailwind.ts b/cli/src/utils/tailwind.ts index 33a1fa1..767190a 100644 --- a/cli/src/utils/tailwind.ts +++ b/cli/src/utils/tailwind.ts @@ -9,7 +9,7 @@ import { $ } from './shell.js'; import { createHash, randomUUID } from 'node:crypto'; import { existsSync } from 'fs'; -import { mkdir, readFile, readdir, rename, rm, stat, writeFile } from 'node:fs/promises'; +import { mkdir, readFile, readdir, rename, rm, stat, utimes, writeFile } from 'node:fs/promises'; import { join } from 'path'; export class TailwindBinNotFoundError extends Error { @@ -77,14 +77,21 @@ const tailwindLocks = new Map>(); async function pruneTailwindCache(cacheDir: string): Promise { const maxEntries = Math.max(1, parseInt(process.env['FACET_TAILWIND_CACHE_ENTRIES'] ?? '50', 10)); - const names = (await readdir(cacheDir)).filter((name) => name.endsWith('.css')); - if (names.length <= maxEntries) return; - const entries = await Promise.all(names.map(async (name) => { - const path = join(cacheDir, name); - return { path, mtimeMs: (await stat(path)).mtimeMs }; - })); + let entries: Array<{ path: string; mtimeMs: number }>; + try { + const names = (await readdir(cacheDir)).filter((name) => name.endsWith('.css')); + if (names.length <= maxEntries) return; + entries = await Promise.all(names.map(async (name) => { + const path = join(cacheDir, name); + return { path, mtimeMs: (await stat(path)).mtimeMs }; + })); + } catch { + return; + } entries.sort((a, b) => b.mtimeMs - a.mtimeMs); - await Promise.all(entries.slice(maxEntries).map((entry) => rm(entry.path, { force: true }))); + await Promise.all(entries.slice(maxEntries).map((entry) => + rm(entry.path, { force: true }).catch(() => undefined), + )); } /** Extract the data-dependent class set without including unrelated text. */ @@ -112,9 +119,15 @@ export async function runTailwindCached(opts: CachedTailwindOptions): Promise { + let cachedCss: string | undefined; try { - return await readFile(cachePath, 'utf-8'); + cachedCss = await readFile(cachePath, 'utf-8'); } catch { /* cache miss */ } + if (cachedCss !== undefined) { + const now = new Date(); + try { await utimes(cachePath, now, now); } catch { /* cache may be concurrently pruned */ } + return cachedCss; + } await mkdir(cacheDir, { recursive: true }); const nonce = `${process.pid}-${randomUUID()}`;