From 7f097bdcab5e2653f075552f16f8b328cb1a8e31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Tue, 24 Mar 2026 09:16:12 +0100 Subject: [PATCH 1/6] feat(core): Replace unbounded expression code cache with LRU --- .../src/__tests__/integration.test.ts | 2 +- .../expression-evaluator-cache.test.ts | 95 +++++++++++++++++++ .../src/evaluator/__tests__/lru-cache.test.ts | 93 ++++++++++++++++++ .../src/evaluator/expression-evaluator.ts | 10 +- .../src/evaluator/lru-cache.ts | 39 ++++++++ .../expression-runtime/src/types/evaluator.ts | 5 + packages/workflow/src/expression.ts | 4 + 7 files changed, 246 insertions(+), 2 deletions(-) create mode 100644 packages/@n8n/expression-runtime/src/evaluator/__tests__/expression-evaluator-cache.test.ts create mode 100644 packages/@n8n/expression-runtime/src/evaluator/__tests__/lru-cache.test.ts create mode 100644 packages/@n8n/expression-runtime/src/evaluator/lru-cache.ts diff --git a/packages/@n8n/expression-runtime/src/__tests__/integration.test.ts b/packages/@n8n/expression-runtime/src/__tests__/integration.test.ts index c46402784379..8c781cd9ba8f 100644 --- a/packages/@n8n/expression-runtime/src/__tests__/integration.test.ts +++ b/packages/@n8n/expression-runtime/src/__tests__/integration.test.ts @@ -9,7 +9,7 @@ describe('Integration: ExpressionEvaluator + IsolatedVmBridge', () => { beforeAll(async () => { const bridge = new IsolatedVmBridge({ timeout: 5000 }); - evaluator = new ExpressionEvaluator({ bridge }); + evaluator = new ExpressionEvaluator({ bridge, maxCodeCacheSize: 1024 }); await evaluator.initialize(); }); diff --git a/packages/@n8n/expression-runtime/src/evaluator/__tests__/expression-evaluator-cache.test.ts b/packages/@n8n/expression-runtime/src/evaluator/__tests__/expression-evaluator-cache.test.ts new file mode 100644 index 000000000000..1816f7cfce75 --- /dev/null +++ b/packages/@n8n/expression-runtime/src/evaluator/__tests__/expression-evaluator-cache.test.ts @@ -0,0 +1,95 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { ExpressionEvaluator } from '../expression-evaluator'; +import type { RuntimeBridge, ObservabilityProvider } from '../../types'; + +function createMockBridge(): RuntimeBridge { + return { + initialize: vi.fn().mockResolvedValue(undefined), + execute: vi.fn().mockReturnValue('result'), + dispose: vi.fn().mockResolvedValue(undefined), + isDisposed: vi.fn().mockReturnValue(false), + }; +} + +function createMockObservability(): ObservabilityProvider { + return { + metrics: { + counter: vi.fn(), + gauge: vi.fn(), + histogram: vi.fn(), + }, + traces: { + startSpan: vi.fn().mockReturnValue({ + setStatus: vi.fn(), + setAttribute: vi.fn(), + recordException: vi.fn(), + end: vi.fn(), + }), + }, + logs: { + error: vi.fn(), + warn: vi.fn(), + info: vi.fn(), + debug: vi.fn(), + }, + }; +} + +describe('ExpressionEvaluator cache', () => { + let bridge: RuntimeBridge; + let observability: ObservabilityProvider; + + beforeEach(() => { + bridge = createMockBridge(); + observability = createMockObservability(); + }); + + it('should emit cache miss on first evaluation', async () => { + const evaluator = new ExpressionEvaluator({ bridge, observability, maxCodeCacheSize: 1024 }); + await evaluator.initialize(); + evaluator.evaluate('={{ $json.email }}', {}); + expect(observability.metrics.counter).toHaveBeenCalledWith('expression.code_cache.miss', 1); + }); + + it('should emit cache hit on repeated evaluation', async () => { + const evaluator = new ExpressionEvaluator({ bridge, observability, maxCodeCacheSize: 1024 }); + await evaluator.initialize(); + evaluator.evaluate('={{ $json.email }}', {}); + evaluator.evaluate('={{ $json.email }}', {}); + expect(observability.metrics.counter).toHaveBeenCalledWith('expression.code_cache.hit', 1); + }); + + it('should emit eviction when cache is full', async () => { + const evaluator = new ExpressionEvaluator({ + bridge, + observability, + maxCodeCacheSize: 2, + }); + await evaluator.initialize(); + evaluator.evaluate('={{ $json.a }}', {}); + evaluator.evaluate('={{ $json.b }}', {}); + evaluator.evaluate('={{ $json.c }}', {}); // evicts first + expect(observability.metrics.counter).toHaveBeenCalledWith('expression.code_cache.eviction', 1); + }); + + it('should work without observability', async () => { + const evaluator = new ExpressionEvaluator({ bridge, maxCodeCacheSize: 1024 }); + await evaluator.initialize(); + expect(() => { + evaluator.evaluate('={{ $json.email }}', {}); + evaluator.evaluate('={{ $json.email }}', {}); + }).not.toThrow(); + }); + + it('should evict least recently used and report miss on re-access', async () => { + const evaluator = new ExpressionEvaluator({ bridge, observability, maxCodeCacheSize: 2 }); + await evaluator.initialize(); + evaluator.evaluate('={{ $json.a }}', {}); + evaluator.evaluate('={{ $json.b }}', {}); + evaluator.evaluate('={{ $json.c }}', {}); + expect(observability.metrics.counter).toHaveBeenCalledWith('expression.code_cache.eviction', 1); + (observability.metrics.counter as ReturnType).mockClear(); + evaluator.evaluate('={{ $json.a }}', {}); + expect(observability.metrics.counter).toHaveBeenCalledWith('expression.code_cache.miss', 1); + }); +}); diff --git a/packages/@n8n/expression-runtime/src/evaluator/__tests__/lru-cache.test.ts b/packages/@n8n/expression-runtime/src/evaluator/__tests__/lru-cache.test.ts new file mode 100644 index 000000000000..f44d25226a6f --- /dev/null +++ b/packages/@n8n/expression-runtime/src/evaluator/__tests__/lru-cache.test.ts @@ -0,0 +1,93 @@ +import { describe, it, expect, vi } from 'vitest'; +import { LruCache } from '../lru-cache'; + +describe('LruCache', () => { + it('should store and retrieve values', () => { + const cache = new LruCache(3); + cache.set('a', 1); + cache.set('b', 2); + expect(cache.get('a')).toBe(1); + expect(cache.get('b')).toBe(2); + }); + + it('should return undefined for missing keys', () => { + const cache = new LruCache(3); + expect(cache.get('missing')).toBeUndefined(); + }); + + it('should evict the least recently used entry when over capacity', () => { + const cache = new LruCache(2); + cache.set('a', 1); + cache.set('b', 2); + cache.set('c', 3); // evicts 'a' + expect(cache.get('a')).toBeUndefined(); + expect(cache.get('b')).toBe(2); + expect(cache.get('c')).toBe(3); + }); + + it('should refresh recency on get', () => { + const cache = new LruCache(2); + cache.set('a', 1); + cache.set('b', 2); + cache.get('a'); // refreshes 'a', now 'b' is oldest + cache.set('c', 3); // evicts 'b' + expect(cache.get('a')).toBe(1); + expect(cache.get('b')).toBeUndefined(); + expect(cache.get('c')).toBe(3); + }); + + it('should refresh recency on set (update)', () => { + const cache = new LruCache(2); + cache.set('a', 1); + cache.set('b', 2); + cache.set('a', 10); // refreshes 'a', now 'b' is oldest + cache.set('c', 3); // evicts 'b' + expect(cache.get('a')).toBe(10); + expect(cache.get('b')).toBeUndefined(); + expect(cache.get('c')).toBe(3); + }); + + it('should call onEvict with evicted key and value', () => { + const onEvict = vi.fn(); + const cache = new LruCache(2, onEvict); + cache.set('a', 1); + cache.set('b', 2); + cache.set('c', 3); // evicts 'a' + expect(onEvict).toHaveBeenCalledWith('a', 1); + }); + + it('should not evict when updating existing keys', () => { + const onEvict = vi.fn(); + const cache = new LruCache(3, onEvict); + cache.set('a', 1); + cache.set('b', 2); + cache.set('a', 10); // update, not a new entry + cache.set('c', 3); + expect(onEvict).not.toHaveBeenCalled(); + expect(cache.get('a')).toBe(10); + expect(cache.get('b')).toBe(2); + expect(cache.get('c')).toBe(3); + }); + + it('should clear all entries', () => { + const cache = new LruCache(3); + cache.set('a', 1); + cache.set('b', 2); + cache.clear(); + expect(cache.get('a')).toBeUndefined(); + expect(cache.get('b')).toBeUndefined(); + }); + + it('should work with capacity 1', () => { + const cache = new LruCache(1); + cache.set('a', 1); + expect(cache.get('a')).toBe(1); + cache.set('b', 2); // evicts 'a' + expect(cache.get('a')).toBeUndefined(); + expect(cache.get('b')).toBe(2); + }); + + it('should throw for capacity less than 1', () => { + expect(() => new LruCache(0)).toThrow('capacity must be at least 1'); + }); +}); diff --git a/packages/@n8n/expression-runtime/src/evaluator/expression-evaluator.ts b/packages/@n8n/expression-runtime/src/evaluator/expression-evaluator.ts index 27da794928c6..34cf558f035b 100644 --- a/packages/@n8n/expression-runtime/src/evaluator/expression-evaluator.ts +++ b/packages/@n8n/expression-runtime/src/evaluator/expression-evaluator.ts @@ -5,6 +5,7 @@ import type { WorkflowData, EvaluateOptions, } from '../types'; +import { LruCache } from './lru-cache'; export class ExpressionEvaluator implements IExpressionEvaluator { private config: EvaluatorConfig; @@ -16,10 +17,13 @@ export class ExpressionEvaluator implements IExpressionEvaluator { // Cache: template expression → tournament-transformed JavaScript code // Cache hit rate in production: ~99.9% (same expressions repeat within a workflow) - private codeCache = new Map(); + private codeCache: LruCache; constructor(config: EvaluatorConfig) { this.config = config; + this.codeCache = new LruCache(config.maxCodeCacheSize, () => { + this.config.observability?.metrics.counter('expression.code_cache.eviction', 1); + }); } async initialize(): Promise { @@ -62,9 +66,12 @@ export class ExpressionEvaluator implements IExpressionEvaluator { private getTransformedCode(expression: string): string { const cached = this.codeCache.get(expression); if (cached !== undefined) { + this.config.observability?.metrics.counter('expression.code_cache.hit', 1); return cached; } + this.config.observability?.metrics.counter('expression.code_cache.miss', 1); + if (!this.tournament) { // Tournament requires an errorHandler but we only use getExpressionCode() // for AST transformation — we never call tournament.execute(), so this @@ -79,6 +86,7 @@ export class ExpressionEvaluator implements IExpressionEvaluator { const [transformedCode] = this.tournament.getExpressionCode(expression); this.codeCache.set(expression, transformedCode); + this.config.observability?.metrics.gauge('expression.code_cache.size', this.codeCache.size); return transformedCode; } diff --git a/packages/@n8n/expression-runtime/src/evaluator/lru-cache.ts b/packages/@n8n/expression-runtime/src/evaluator/lru-cache.ts new file mode 100644 index 000000000000..3d8b942a5276 --- /dev/null +++ b/packages/@n8n/expression-runtime/src/evaluator/lru-cache.ts @@ -0,0 +1,39 @@ +export class LruCache { + private map = new Map(); + + constructor( + private readonly capacity: number, + private readonly onEvict?: (key: K, value: V) => void, + ) { + if (capacity < 1) { + throw new Error('LruCache capacity must be at least 1'); + } + } + + get(key: K): V | undefined { + const value = this.map.get(key); + if (value === undefined) return undefined; + if (this.map.size < this.capacity) return value; + this.map.delete(key); + this.map.set(key, value); + return value; + } + + set(key: K, value: V): void { + this.map.delete(key); + this.map.set(key, value); + if (this.map.size > this.capacity) { + const [oldestKey, oldestValue] = this.map.entries().next().value!; + this.map.delete(oldestKey); + this.onEvict?.(oldestKey, oldestValue); + } + } + + get size(): number { + return this.map.size; + } + + clear(): void { + this.map.clear(); + } +} diff --git a/packages/@n8n/expression-runtime/src/types/evaluator.ts b/packages/@n8n/expression-runtime/src/types/evaluator.ts index 630d337b867b..192d68b314c5 100644 --- a/packages/@n8n/expression-runtime/src/types/evaluator.ts +++ b/packages/@n8n/expression-runtime/src/types/evaluator.ts @@ -30,6 +30,11 @@ export interface EvaluatorConfig { * If omitted, expressions are transformed with no security hooks (dev/testing use). */ hooks?: TournamentHooks; + + /** + * Maximum number of tournament-transformed expressions to cache (LRU). + */ + maxCodeCacheSize: number; } /** diff --git a/packages/workflow/src/expression.ts b/packages/workflow/src/expression.ts index 63f09629d22b..b05fc7d44141 100644 --- a/packages/workflow/src/expression.ts +++ b/packages/workflow/src/expression.ts @@ -211,8 +211,12 @@ export class Expression { // Dynamic import to avoid loading expression-runtime in browser environments const { ExpressionEvaluator, IsolatedVmBridge } = await import('@n8n/expression-runtime'); const bridge = new IsolatedVmBridge({ timeout: 5000 }); + const DEFAULT_MAX_CODE_CACHE_SIZE = 1024; + const parsed = parseInt(process.env.N8N_EXPRESSION_ENGINE_MAX_CODE_CACHE_SIZE ?? '', 10); + const maxCodeCacheSize = parsed || DEFAULT_MAX_CODE_CACHE_SIZE; this.vmEvaluator = new ExpressionEvaluator({ bridge, + maxCodeCacheSize, hooks: { before: [ThisSanitizer], after: [PrototypeSanitizer, DollarSignValidator], From 9426ba7308e281e5001ecd08ddb8b966bea86b84 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Tue, 24 Mar 2026 09:34:28 +0100 Subject: [PATCH 2/6] refactor: Remove premature optimization --- .../src/evaluator/__tests__/lru-cache.test.ts | 11 +++++++++++ .../expression-runtime/src/evaluator/lru-cache.ts | 1 - 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/packages/@n8n/expression-runtime/src/evaluator/__tests__/lru-cache.test.ts b/packages/@n8n/expression-runtime/src/evaluator/__tests__/lru-cache.test.ts index f44d25226a6f..30771c33f888 100644 --- a/packages/@n8n/expression-runtime/src/evaluator/__tests__/lru-cache.test.ts +++ b/packages/@n8n/expression-runtime/src/evaluator/__tests__/lru-cache.test.ts @@ -36,6 +36,17 @@ describe('LruCache', () => { expect(cache.get('c')).toBe(3); }); + it('should refresh recency on get even when below capacity', () => { + const cache = new LruCache(3); + cache.set('a', 1); + cache.set('b', 2); + cache.get('a'); // refreshes 'a' while cache is below capacity + cache.set('c', 3); // fills cache + cache.set('d', 4); // evicts 'b' (oldest), not 'a' + expect(cache.get('a')).toBe(1); + expect(cache.get('b')).toBeUndefined(); + }); + it('should refresh recency on set (update)', () => { const cache = new LruCache(2); cache.set('a', 1); diff --git a/packages/@n8n/expression-runtime/src/evaluator/lru-cache.ts b/packages/@n8n/expression-runtime/src/evaluator/lru-cache.ts index 3d8b942a5276..e0237a7a541a 100644 --- a/packages/@n8n/expression-runtime/src/evaluator/lru-cache.ts +++ b/packages/@n8n/expression-runtime/src/evaluator/lru-cache.ts @@ -13,7 +13,6 @@ export class LruCache { get(key: K): V | undefined { const value = this.map.get(key); if (value === undefined) return undefined; - if (this.map.size < this.capacity) return value; this.map.delete(key); this.map.set(key, value); return value; From b21fdc1297cb1cf5f8158c7d630ea6d74d2e5de5 Mon Sep 17 00:00:00 2001 From: Danny Martini Date: Wed, 25 Mar 2026 12:46:53 +0100 Subject: [PATCH 3/6] fix(core): allow configurable vm evaluator timeout for benchmarks CodSpeed's instruction-counting instrumentation adds significant wall-clock overhead, causing the hardcoded 5s isolated-vm timeout to fire on the 10k-item array benchmark. Adds an optional `timeout` parameter to `initializeVmEvaluator()` (defaults to 5000ms) and passes 60s in the benchmark fixture. Co-Authored-By: Claude Sonnet 4.6 --- .../benchmarks/expression-engine/fixtures/data.ts | 5 ++++- packages/workflow/src/expression.ts | 4 ++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/testing/performance/benchmarks/expression-engine/fixtures/data.ts b/packages/testing/performance/benchmarks/expression-engine/fixtures/data.ts index edc928209b3b..400e81c8307a 100644 --- a/packages/testing/performance/benchmarks/expression-engine/fixtures/data.ts +++ b/packages/testing/performance/benchmarks/expression-engine/fixtures/data.ts @@ -144,5 +144,8 @@ export async function useCurrentEngine(): Promise { export async function useVmEngine(): Promise { Expression.setExpressionEngine('vm'); - await Expression.initializeVmEvaluator(); + // Use a higher timeout for benchmarks — CodSpeed's instruction-counting + // instrumentation adds significant wall-clock overhead that can cause the + // default 5s timeout to fire on larger data set benchmarks (e.g. 10k items). + await Expression.initializeVmEvaluator({ timeout: 60_000 }); } diff --git a/packages/workflow/src/expression.ts b/packages/workflow/src/expression.ts index b05fc7d44141..db0e1febe7e3 100644 --- a/packages/workflow/src/expression.ts +++ b/packages/workflow/src/expression.ts @@ -204,13 +204,13 @@ export class Expression { * Should be called once during application startup. * Only available in Node.js environments (not in browser). */ - static async initializeVmEvaluator(): Promise { + static async initializeVmEvaluator(options?: { timeout?: number }): Promise { if (this.expressionEngine !== 'vm' || IS_FRONTEND) return; if (!this.vmEvaluator) { // Dynamic import to avoid loading expression-runtime in browser environments const { ExpressionEvaluator, IsolatedVmBridge } = await import('@n8n/expression-runtime'); - const bridge = new IsolatedVmBridge({ timeout: 5000 }); + const bridge = new IsolatedVmBridge({ timeout: options?.timeout ?? 5000 }); const DEFAULT_MAX_CODE_CACHE_SIZE = 1024; const parsed = parseInt(process.env.N8N_EXPRESSION_ENGINE_MAX_CODE_CACHE_SIZE ?? '', 10); const maxCodeCacheSize = parsed || DEFAULT_MAX_CODE_CACHE_SIZE; From a4b7f001307c44cbbe3d6200086bcc3bfe2d4b48 Mon Sep 17 00:00:00 2001 From: Danny Martini Date: Wed, 25 Mar 2026 15:43:28 +0100 Subject: [PATCH 4/6] fix(core): emit cache size gauge reset on evaluator disposal Ensures the expression.code_cache.size gauge is set to 0 when the evaluator is disposed, preventing stale values in metrics backends. Co-Authored-By: Claude Sonnet 4.6 --- .../expression-runtime/src/evaluator/expression-evaluator.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/@n8n/expression-runtime/src/evaluator/expression-evaluator.ts b/packages/@n8n/expression-runtime/src/evaluator/expression-evaluator.ts index 34cf558f035b..1a52355d5b17 100644 --- a/packages/@n8n/expression-runtime/src/evaluator/expression-evaluator.ts +++ b/packages/@n8n/expression-runtime/src/evaluator/expression-evaluator.ts @@ -93,6 +93,7 @@ export class ExpressionEvaluator implements IExpressionEvaluator { async dispose(): Promise { this.disposed = true; this.codeCache.clear(); + this.config.observability?.metrics.gauge('expression.code_cache.size', 0); await this.config.bridge.dispose(); } From 648aea91e44ceb936b35ca8e2b5fe8a9aec0aed4 Mon Sep 17 00:00:00 2001 From: Danny Martini Date: Wed, 25 Mar 2026 15:54:32 +0100 Subject: [PATCH 5/6] test(core): replace manual mockClear cast with vi.clearAllMocks() Co-Authored-By: Claude Sonnet 4.6 --- .../src/evaluator/__tests__/expression-evaluator-cache.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/@n8n/expression-runtime/src/evaluator/__tests__/expression-evaluator-cache.test.ts b/packages/@n8n/expression-runtime/src/evaluator/__tests__/expression-evaluator-cache.test.ts index 1816f7cfce75..d378bdf7b833 100644 --- a/packages/@n8n/expression-runtime/src/evaluator/__tests__/expression-evaluator-cache.test.ts +++ b/packages/@n8n/expression-runtime/src/evaluator/__tests__/expression-evaluator-cache.test.ts @@ -40,6 +40,7 @@ describe('ExpressionEvaluator cache', () => { let observability: ObservabilityProvider; beforeEach(() => { + vi.clearAllMocks(); bridge = createMockBridge(); observability = createMockObservability(); }); @@ -88,7 +89,7 @@ describe('ExpressionEvaluator cache', () => { evaluator.evaluate('={{ $json.b }}', {}); evaluator.evaluate('={{ $json.c }}', {}); expect(observability.metrics.counter).toHaveBeenCalledWith('expression.code_cache.eviction', 1); - (observability.metrics.counter as ReturnType).mockClear(); + vi.clearAllMocks(); evaluator.evaluate('={{ $json.a }}', {}); expect(observability.metrics.counter).toHaveBeenCalledWith('expression.code_cache.miss', 1); }); From 2e99c2220fe8204883d4dc9247cf5cabd6036806 Mon Sep 17 00:00:00 2001 From: Danny Martini Date: Wed, 25 Mar 2026 16:16:34 +0100 Subject: [PATCH 6/6] test(core): add tests for expression.code_cache.size gauge emissions Co-Authored-By: Claude Sonnet 4.6 --- .../__tests__/expression-evaluator-cache.test.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/packages/@n8n/expression-runtime/src/evaluator/__tests__/expression-evaluator-cache.test.ts b/packages/@n8n/expression-runtime/src/evaluator/__tests__/expression-evaluator-cache.test.ts index d378bdf7b833..5614a8c94e6d 100644 --- a/packages/@n8n/expression-runtime/src/evaluator/__tests__/expression-evaluator-cache.test.ts +++ b/packages/@n8n/expression-runtime/src/evaluator/__tests__/expression-evaluator-cache.test.ts @@ -82,6 +82,22 @@ describe('ExpressionEvaluator cache', () => { }).not.toThrow(); }); + it('should emit cache size gauge on cache miss', async () => { + const evaluator = new ExpressionEvaluator({ bridge, observability, maxCodeCacheSize: 1024 }); + await evaluator.initialize(); + evaluator.evaluate('={{ $json.email }}', {}); + expect(observability.metrics.gauge).toHaveBeenCalledWith('expression.code_cache.size', 1); + }); + + it('should emit cache size gauge of 0 on dispose', async () => { + const evaluator = new ExpressionEvaluator({ bridge, observability, maxCodeCacheSize: 1024 }); + await evaluator.initialize(); + evaluator.evaluate('={{ $json.email }}', {}); + vi.clearAllMocks(); + await evaluator.dispose(); + expect(observability.metrics.gauge).toHaveBeenCalledWith('expression.code_cache.size', 0); + }); + it('should evict least recently used and report miss on re-access', async () => { const evaluator = new ExpressionEvaluator({ bridge, observability, maxCodeCacheSize: 2 }); await evaluator.initialize();