perf(core): cache parsed ASTs in the expression engine#88
Conversation
evaluate() ran extractExpression -> tokenize -> parse on every call, even though the engine is pure (no eval/Function, and evaluateNode never mutates the AST) so a parsed AST is valid forever. In a long-running spec where the same quality gate, invariant, or verification check fires repeatedly, the tokenize/parse work compounds. This is Candidate 2 of issue #46. Add a module-level Map<string, ASTNode> keyed on the raw template string, with a 1000-entry soft cap and LRU eviction (Map insertion order: a hit re-inserts the key, eviction drops the oldest). Only successful parses are cached; a malformed template re-throws on every call exactly as before. evaluateNode still runs per call against the live context, so results stay per-context. clearExpressionCache(), expressionCacheSize(), and expressionCacheHas() are exported for test isolation and diagnostics (pure reads; cacheHas does not affect recency). Behaviour-preserving: cached and uncached evaluations are identical, distinct templates never collide, and eviction at the cap re-parses without corrupting results. Tests cover cache-hit equivalence across contexts, no cross-template collision, cap-bounded eviction, correct re-evaluation of an evicted template, and an LRU-vs-FIFO discriminator (verified to fail under FIFO) proving a hot template survives eviction while the least-recently-used victim is dropped. Proof (npm run bench, evaluate same expression 10,000x, this machine): best 13.2ms before, 1.46ms after (~9x), widening the margin against the 1000ms assertion from ~75x to ~685x. Thresholds left unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 17 minutes and 5 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Issue #46, Candidate 2 (PR 3 of 4). Follows the
__perf__scaffold (#53) and the compiler fix (#87).Summary
evaluate(template, context)ranextractExpression -> tokenize -> parseon every call. The engine is pure (noeval/Function, andevaluateNodenever mutates the AST), so a parsed AST is valid forever and safe to share. In a long-running spec where the same quality gate, invariant, branch condition, or verification check fires repeatedly, the tokenize/parse work compounds.Map<string, ASTNode>keyed on the raw template string, 1000-entry soft cap, LRU eviction via Map insertion order (a hit re-inserts the key; eviction drops the oldest).evaluateNodestill runs per call against the live context, so results remain per-context.clearExpressionCache(),expressionCacheSize(),expressionCacheHas()exported for test isolation and diagnostics (pure reads).Behaviour-preserving
Cached and uncached evaluations are identical, distinct templates never collide, and eviction at the cap re-parses without corrupting results.
Tests (packages/core/expression.test.ts)
Proof (npm run bench, Candidate-2 assertion: evaluate the same expression 10,000x)
~9x faster; the assertion passes with a far wider margin. Thresholds left unchanged per the issue's calibration guidance.
Scope
packages/core/expression.ts+ its tests only.index.tsdeliberately untouched; the new diagnostics are imported directly from./expression.jsby the test.Test Plan
npm run build:core&&npm run buildnpm test(503 passed)npm run typechecknpm run lint(no new errors; 5 pre-existing out-of-scope errors unchanged)node spec/fixtures/run-fixtures.mjs(29 passed)npm run bench(3 passed, Candidate-2 margin widened)🤖 Generated with Claude Code
Summary by cubic
Caches parsed ASTs in the expression engine to skip re-tokenizing/parsing on repeat evaluations. Results are identical; the 10k-evals bench is ~9x faster (13.2ms → 1.46ms), meeting the goals of issue #46.
Map<string, ASTNode>cache keyed by raw template; 1000-entry soft cap with LRU eviction.evaluateNoderuns per call, so results stay per-context.clearExpressionCache,expressionCacheSize, andexpressionCacheHasfor tests and diagnostics.Written for commit 3d7f62c. Summary will update on new commits. Review in cubic