diff --git a/.changeset/perf-cached-workflow-script.md b/.changeset/perf-cached-workflow-script.md new file mode 100644 index 0000000000..0dd223117d --- /dev/null +++ b/.changeset/perf-cached-workflow-script.md @@ -0,0 +1,5 @@ +--- +"@workflow/core": patch +--- + +Cache the compiled workflow-bundle `vm.Script` per process so replays reuse the compiled bundle instead of re-parsing it on every iteration. diff --git a/packages/core/src/vm/script-cache.test.ts b/packages/core/src/vm/script-cache.test.ts new file mode 100644 index 0000000000..399f6b2c69 --- /dev/null +++ b/packages/core/src/vm/script-cache.test.ts @@ -0,0 +1,202 @@ +import { runInContext } from 'node:vm'; +import { afterEach, describe, expect, it } from 'vitest'; +import { createContext } from './index.js'; +import { + clearWorkflowScriptCache, + getCachedWorkflowScript, + runCachedWorkflowScript, + workflowScriptCacheSize, +} from './script-cache.js'; + +const seed = 'script-cache seed'; +const fixedTimestamp = 1234567890000; + +const SAMPLE_BUNDLE = ` +globalThis.__private_workflows = new Map(); +globalThis.__private_workflows.set('my/workflow', async function workflow(name) { + return 'hello,' + name + ',' + Math.random() + ',' + Date.now(); +}); +`; + +/** + * Builds a realistic multi-workflow bundle: many registered workflow functions + * (the production shape) with a distinguishing `marker` baked in so two bundles + * built with different markers are genuinely different code (not the trivial + * `1 + 1` / `2 + 2` strings). Each workflow returns a value derived from the + * marker so a mis-served Script would produce a detectably wrong result. + */ +function buildBundle(marker: string, workflowCount = 12): string { + const defs: string[] = []; + for (let i = 0; i < workflowCount; i++) { + defs.push( + `globalThis.__private_workflows.set('app/workflow-${i}', async function workflow${i}(name) {\n` + + ` return '${marker}:' + ${i} + ':' + name + ':' + Math.random();\n` + + `});` + ); + } + return `globalThis.__private_workflows = new Map();\n${defs.join('\n')}\n`; +} + +describe('script-cache', () => { + afterEach(() => { + clearWorkflowScriptCache(); + }); + + it('returns the same compiled Script for identical (code, filename)', () => { + const a = getCachedWorkflowScript(SAMPLE_BUNDLE, 'workflows/a.ts'); + const b = getCachedWorkflowScript(SAMPLE_BUNDLE, 'workflows/a.ts'); + expect(a).toBe(b); + }); + + it('returns distinct Scripts for the same code under different filenames', () => { + const a = getCachedWorkflowScript(SAMPLE_BUNDLE, 'workflows/a.ts'); + const b = getCachedWorkflowScript(SAMPLE_BUNDLE, 'workflows/b.ts'); + expect(a).not.toBe(b); + }); + + it('returns distinct Scripts for different code under the same filename', () => { + const a = getCachedWorkflowScript('1 + 1', 'workflows/a.ts'); + const b = getCachedWorkflowScript('2 + 2', 'workflows/a.ts'); + expect(a).not.toBe(b); + }); + + it('produces a byte-identical workflow result vs. uncached runInContext', async () => { + // Cached path: run the bundle then look up the workflow, mirroring + // runWorkflow's two-step evaluation. + const { context: cachedCtx } = createContext({ seed, fixedTimestamp }); + runCachedWorkflowScript(SAMPLE_BUNDLE, 'workflows/a.ts', cachedCtx); + const cachedFn = runCachedWorkflowScript( + `globalThis.__private_workflows?.get('my/workflow')`, + 'workflows/a.ts', + cachedCtx + ); + expect(cachedFn).toBeTypeOf('function'); + const cachedResult = await (cachedFn as (n: string) => Promise)( + 'world' + ); + + // Uncached path: the original combined-string approach. + const { context: plainCtx } = createContext({ seed, fixedTimestamp }); + const plainFn = runInContext( + `${SAMPLE_BUNDLE}; globalThis.__private_workflows?.get('my/workflow')`, + plainCtx, + { filename: 'workflows/a.ts' } + ); + const plainResult = await (plainFn as (n: string) => Promise)( + 'world' + ); + + expect(cachedResult).toEqual(plainResult); + }); + + it('reuses the compiled Script across multiple runs against fresh contexts', async () => { + const script = getCachedWorkflowScript(SAMPLE_BUNDLE, 'workflows/a.ts'); + + const results: string[] = []; + for (let i = 0; i < 3; i++) { + const { context } = createContext({ seed, fixedTimestamp }); + runCachedWorkflowScript(SAMPLE_BUNDLE, 'workflows/a.ts', context); + // The same cached Script object is used every iteration. + expect(getCachedWorkflowScript(SAMPLE_BUNDLE, 'workflows/a.ts')).toBe( + script + ); + const fn = runInContext( + `globalThis.__private_workflows?.get('my/workflow')`, + context + ) as (n: string) => Promise; + results.push(await fn('world')); + } + + // Deterministic context => identical results every run. + expect(results[0]).toEqual(results[1]); + expect(results[1]).toEqual(results[2]); + }); + + it('bounds the bundle cache and evicts least-recently-used bundles', () => { + // Insert far more distinct bundles than any sane cap, simulating a long + // dev/watch session where every edit produces a new bundle string. The + // cache must NOT grow monotonically with edit count. + const editCount = 100; + const filename = 'workflows/a.ts'; + for (let i = 0; i < editCount; i++) { + getCachedWorkflowScript(buildBundle(`edit-${i}`), filename); + } + + const size = workflowScriptCacheSize(); + expect(size).toBeGreaterThan(0); + // Bounded well below the number of edits — the whole point of the LRU. + expect(size).toBeLessThan(editCount); + + // The cache still serves correctly after heavy churn: the most-recently + // inserted bundle is retained and repeated lookups return the same Script. + const latest = buildBundle(`edit-${editCount - 1}`); + expect(getCachedWorkflowScript(latest, filename)).toBe( + getCachedWorkflowScript(latest, filename) + ); + }); + + it('keeps the most-recently-used bundle and evicts the stale one', () => { + const filename = 'workflows/a.ts'; + // Seed an "old" bundle, then keep it hot by re-touching it while many + // unrelated bundles churn through. LRU must NOT evict the bundle we keep + // using, even though it was inserted first. + const hot = buildBundle('hot'); + const hotScript = getCachedWorkflowScript(hot, filename); + + for (let i = 0; i < 50; i++) { + getCachedWorkflowScript(buildBundle(`cold-${i}`), filename); + // Re-access the hot bundle so it stays most-recently-used. + expect(getCachedWorkflowScript(hot, filename)).toBe(hotScript); + } + + // After all that churn the hot bundle is still the *same* cached Script — + // proving LRU recency (touch-on-access), not mere insertion order, governs + // eviction. + expect(getCachedWorkflowScript(hot, filename)).toBe(hotScript); + }); + + it('never returns the wrong Script across realistic multi-workflow bundles', async () => { + // Two genuinely different bundles (production-shape: many workflows each), + // distinguished by their marker, plus two filenames. Every distinct + // (code, filename) must map to its own Script, and running each must + // produce results derived from its own marker — never another bundle's. + const bundleX = buildBundle('bundle-X'); + const bundleY = buildBundle('bundle-Y'); + const fileA = 'workflows/a.ts'; + const fileB = 'workflows/b.ts'; + + const xa = getCachedWorkflowScript(bundleX, fileA); + const xb = getCachedWorkflowScript(bundleX, fileB); + const ya = getCachedWorkflowScript(bundleY, fileA); + const yb = getCachedWorkflowScript(bundleY, fileB); + + // All four (code, filename) combinations are distinct Script objects. + const scripts = [xa, xb, ya, yb]; + for (let i = 0; i < scripts.length; i++) { + for (let j = i + 1; j < scripts.length; j++) { + expect(scripts[i]).not.toBe(scripts[j]); + } + } + + // Same (code, filename) is stable across lookups. + expect(getCachedWorkflowScript(bundleX, fileA)).toBe(xa); + expect(getCachedWorkflowScript(bundleY, fileB)).toBe(yb); + + // Running each bundle yields its OWN marker, confirming no cross-wiring. + const { context: ctxX } = createContext({ seed, fixedTimestamp }); + runCachedWorkflowScript(bundleX, fileA, ctxX); + const fnX = runInContext( + `globalThis.__private_workflows?.get('app/workflow-3')`, + ctxX + ) as (n: string) => Promise; + expect(await fnX('z')).toContain('bundle-X:3:z'); + + const { context: ctxY } = createContext({ seed, fixedTimestamp }); + runCachedWorkflowScript(bundleY, fileA, ctxY); + const fnY = runInContext( + `globalThis.__private_workflows?.get('app/workflow-3')`, + ctxY + ) as (n: string) => Promise; + expect(await fnY('z')).toContain('bundle-Y:3:z'); + }); +}); diff --git a/packages/core/src/vm/script-cache.ts b/packages/core/src/vm/script-cache.ts new file mode 100644 index 0000000000..d23bbc1624 --- /dev/null +++ b/packages/core/src/vm/script-cache.ts @@ -0,0 +1,153 @@ +import { type Context, Script } from 'node:vm'; + +/** + * Module-level cache of compiled workflow-bundle `vm.Script` objects. + * + * Why this exists + * --------------- + * Replaying a workflow re-evaluates the workflow bundle against a fresh VM + * context on every iteration of the inline replay loop (see + * `runWorkflow` in `../workflow.ts`). The bundle is a single string that + * contains every workflow function in the app and registers them on + * `globalThis.__private_workflows`. Previously each replay called + * `vm.runInContext(workflowCode, context, { filename })`, which RE-PARSES and + * RE-COMPILES the entire bundle every time — O(N) full re-parses for a + * sequential workflow of N steps, plus the same parse cost repeated across + * every invocation in the process. + * + * Compilation is a pure function of `(code, filename)`: a `vm.Script` carries + * no realm/context state — it is only bound to a context at `runInContext` + * time. So a single compiled `Script` can be reused across replays AND across + * workflow invocations in the same process without affecting determinism: the + * produced workflow function is identical to the previous re-parse-every-time + * behaviour, with identical `filename` source attribution (see the precise + * claim — and its one caveat — in `runWorkflow`). + * + * Keying + * ------ + * Keyed by `code` then `filename`. The `filename` is part of the key on + * purpose, NOT as a dedupe key: it is baked into the compiled script's source + * attribution and surfaces in stack traces, where `remapErrorStack` keys on it + * to map frames back to the user's source. Two workflows in the same bundle + * share the same `code` but have different `filename`s, so they intentionally + * compile to distinct `Script`s — collapsing them onto a single shared `Script` + * would misattribute one workflow's stack frames to another file. The cost of + * keeping them distinct is that the whole bundle is compiled once per distinct + * `filename` (not once per bundle); in practice that is bounded by the number + * of source files that define a workflow, and because V8 lazily compiles + * function bodies the duplicated work is the (cheap) top-level parse, not full + * per-workflow codegen. + * + * We use a nested Map (code -> filename -> Script) so that evicting a bundle + * (e.g. a new deployment/hot-reload producing a different `code`) drops the old + * code string and all of its per-filename scripts together. + * + * Bounding + * -------- + * The top-level (`code`-keyed) map is an insertion-ordered LRU capped at + * `MAX_BUNDLES` entries. In production this bound is never reached: a + * deployment is its own process serving exactly one build-time bundle literal + * (skew protection runs old versions as separate processes), so there is a + * single `code` key for the process lifetime. The bound exists for dev/watch + * mode, where the dev route re-reads `workflowCode` from disk and re-invokes + * the entrypoint on every edit — each edit produces a NEW bundle string, which + * without a bound would pin every historical version forever (~0.8MB per edit, + * growing monotonically with edit count). The dev path only ever needs the + * latest bundle, so an LRU that keeps the few most-recent bundles and evicts + * the rest preserves the pre-cache GC behaviour while still serving the + * steady-state single-bundle case for free. The per-`filename` inner map is not + * separately bounded: it is naturally bounded by the (small) number of workflow + * source files in a bundle and is dropped wholesale when its parent `code` + * entry is evicted. + */ +const scriptCache = new Map>(); + +/** + * Max number of distinct bundle (`code`) versions to retain. One is enough for + * production; a handful covers pathological dev hot-reload / repeated-rebuild + * churn within a single long-lived process (e.g. a watch session or a test + * file) without unbounded growth. Kept deliberately small — there is no value + * in retaining stale bundles, only a memory cost. + */ +const MAX_BUNDLES = 8; + +/** + * Looks up the per-filename map for `code`, marking it most-recently-used. + * Relies on `Map` preserving insertion order: deleting and re-inserting an + * existing key moves it to the end (newest), so the first key is always the + * least-recently-used eviction candidate. + */ +function touchBundle(code: string): Map | undefined { + const byFilename = scriptCache.get(code); + if (byFilename === undefined) { + return undefined; + } + // Move to the most-recently-used position (end of insertion order). + scriptCache.delete(code); + scriptCache.set(code, byFilename); + return byFilename; +} + +/** + * Returns a compiled `vm.Script` for the given workflow bundle code and + * filename, compiling and caching it on first use. Subsequent calls with the + * same `(code, filename)` return the cached `Script`. + * + * The returned `Script` is not yet bound to any context; the caller runs it + * against a specific VM context via `script.runInContext(context)`. This is + * equivalent to `vm.runInContext(code, context, { filename })` but skips the + * recompile. + */ +export function getCachedWorkflowScript( + code: string, + filename: string +): Script { + let byFilename = touchBundle(code); + if (byFilename === undefined) { + byFilename = new Map(); + scriptCache.set(code, byFilename); + // Evict the least-recently-used bundle(s) when over the cap. New bundles + // are appended at the end, so the oldest live at the front. + while (scriptCache.size > MAX_BUNDLES) { + const oldest = scriptCache.keys().next().value; + if (oldest === undefined) { + break; + } + scriptCache.delete(oldest); + } + } + let script = byFilename.get(filename); + if (script === undefined) { + script = new Script(code, { filename }); + byFilename.set(filename, script); + } + return script; +} + +/** + * Runs the cached workflow-bundle `Script` against `context`. Compiles and + * caches the `Script` on first use for the given `(code, filename)`. + */ +export function runCachedWorkflowScript( + code: string, + filename: string, + context: Context +): unknown { + return getCachedWorkflowScript(code, filename).runInContext(context); +} + +/** + * Clears the compiled-script cache. Intended for tests that want to assert + * compile-vs-cache behaviour in isolation; not used on the hot path. + */ +export function clearWorkflowScriptCache(): void { + scriptCache.clear(); +} + +/** + * Number of distinct bundle (`code`) versions currently retained. Exposed for + * tests asserting the LRU bound; not used on the hot path. + */ +export function workflowScriptCacheSize(): number { + return scriptCache.size; +} diff --git a/packages/core/src/workflow.ts b/packages/core/src/workflow.ts index 7a041015f5..6d3af6a89f 100644 --- a/packages/core/src/workflow.ts +++ b/packages/core/src/workflow.ts @@ -1,4 +1,3 @@ -import { runInContext } from 'node:vm'; import { ERROR_SLUGS, ReplayDivergenceError, @@ -38,15 +37,16 @@ import * as Attribute from './telemetry/semantic-conventions.js'; import { trace } from './telemetry.js'; import { getWorkflowRunStreamId } from './util.js'; import { createContext } from './vm/index.js'; +import { runCachedWorkflowScript } from './vm/script-cache.js'; import { createAbortSignalStatics, createCreateAbortController, } from './workflow/abort-controller.js'; +import { createSetAttributes } from './workflow/attribute-dispatcher.js'; import type { WorkflowMetadata } from './workflow/get-workflow-metadata.js'; import { WORKFLOW_CONTEXT_SYMBOL } from './workflow/get-workflow-metadata.js'; import { createCreateHook } from './workflow/hook.js'; import { createSleep } from './workflow/sleep.js'; -import { createSetAttributes } from './workflow/attribute-dispatcher.js'; /** * Drain pending queue items at workflow completion (success or failure). @@ -769,10 +769,29 @@ export async function runWorkflow( const parsedName = parseWorkflowName(workflowRun.workflowName); const filename = parsedName?.moduleSpecifier || workflowRun.workflowName; - const workflowFn = runInContext( - `${workflowCode}; globalThis.__private_workflows?.get(${JSON.stringify(workflowRun.workflowName)})`, - context, - { filename } + // Evaluate the workflow bundle against the fresh context using a + // process-wide cache of the compiled `vm.Script`. The bundle is the same + // string for every replay and every invocation in this process, and + // compilation is a pure function of `(code, filename)`, so reusing the + // compiled Script across replays is determinism-safe: it produces the same + // workflow function and the same `filename` source attribution as + // re-parsing the bundle every time, but skips the (expensive) re-parse. + // Evaluating the bundle registers every workflow on + // `globalThis.__private_workflows`; the trailing lookup expression then + // retrieves the requested workflow function. The lookup is evaluated as a + // separate cached Script under the same `filename`, so error stack frames + // still attribute to the workflow's source file (`remapErrorStack` keys on + // `filename`). The one behavioural difference from the previous + // single-combined-string approach is the *line number* of an error thrown + // by the lookup expression itself: it now reports line 1 of the lookup + // Script rather than the line just past the end of the bundle. That path + // is rare (it requires the lookup `?.get(...)` expression to throw) and + // does not affect the workflow function or replay determinism. + runCachedWorkflowScript(workflowCode, filename, context); + const workflowFn = runCachedWorkflowScript( + `globalThis.__private_workflows?.get(${JSON.stringify(workflowRun.workflowName)})`, + filename, + context ); if (typeof workflowFn !== 'function') {