Skip to content

Restore compiled-expression cache in RuleExpressionParser (fixes #673)#727

Merged
YogeshPraj merged 1 commit into
microsoft:mainfrom
YogeshPraj:fix/restore-expression-parser-cache
May 27, 2026
Merged

Restore compiled-expression cache in RuleExpressionParser (fixes #673)#727
YogeshPraj merged 1 commit into
microsoft:mainfrom
YogeshPraj:fix/restore-expression-parser-cache

Conversation

@YogeshPraj

@YogeshPraj YogeshPraj commented May 27, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #673. The static MemCache around RuleExpressionParser.Compile<T> was removed in v5.0.1 (#501) as a secondary change inside a PR titled "Added option to disable auto type registry." Removing it meant every call to Compile<T> / CompileRuleExpressionParameters re-ran the dynamic expression parser and re-emitted IL via CompileFast/Compile, causing the ~10x slowdown the issue reports.

This PR restores the cache, but as an instance field (the original was static, which silently shared compiled delegates across RulesEngine instances with different CustomTypes / IsExpressionCaseSensitive — a latent correctness bug we don't want to bring back).

Why the cache was originally removed (best reconstruction)

PR #501's commit message ("removed caching from RuleExpressionParser"), the PR description, the CHANGELOG entry, and the linked issue (#500) all give no justification for the removal. #500 is a parsing bug ("array like an enum") whose actual fix is the AutoRegisterInputType flag — completely unrelated to caching.

The most plausible unstated rationale: the same PR introduced two new behavioral toggles (AutoRegisterInputType and IsExpressionCaseSensitive). The original static cache would have silently shared compiled delegates across RuleExpressionParser instances with different settings, defeating those new toggles. Removing the cache "fixed" that latent bug by sledgehammer.

This PR addresses that concern directly rather than by removal:

  • The cache is now an instance field, not static. Different RuleExpressionParser instances cannot share entries.
  • The cache key includes a settings fingerprint covering IsExpressionCaseSensitive, UseFastExpressionCompiler, and CustomTypes. Even within one instance, settings-affecting inputs cannot collide.
  • The new Compile_DifferentSettings_DoNotShareCache test explicitly guards this property and would fail if the bug ever crept back.
  • The AutoRegisterInputType parsing fix from Added option to disable auto type registry #501 is left untouched, so v4 to v5 error: array like an enum #500 stays fixed.

What changed

  • RuleExpressionParser now owns an instance MemCache. Compile<T> and CompileRuleExpressionParameters look up against it before re-parsing.
  • Cache key fingerprints everything that affects the compiled delegate:
    • expression text
    • ordered (paramName, paramType.AssemblyQualifiedName)
    • return type
    • settings: IsExpressionCaseSensitive, UseFastExpressionCompiler, CustomTypes
    • for scoped-params: each ValueExpression.ToString() (so mutating a workflow's LocalParams/GlobalParams correctly invalidates)
  • Honors the existing ReSettings.CacheConfig. SizeLimit == 0 opts out entirely.

Why RulesCache doesn't already mask this

RulesCache keys on workflowName + paramTypes and caches compiled RuleFunc<RuleResultTree>. It works for repeat ExecuteAllRulesAsync calls on a warm engine. It does not cover:

  • Direct use of RuleExpressionParser.Evaluate<T> / Compile<T> (public API).
  • ExecuteActionWorkflowAsync, which calls CompileRule on every invocation (RulesEngine.cs#L131).
  • Workloads where param types vary slightly between calls (cache-key churn).
  • Mutating a workflow and re-adding it.

Benchmark

.NET 8.0.27, Release build. Each scenario: 3 warmup + 5 measured runs; reported value is the median in ms.

# Scenario Before (master) After (this PR) Speedup
A RuleExpressionParser.Evaluate<bool> x 30,000 (same expression + params) 34,730 ms 33 ms ~1,052x
B RuleExpressionParser.Compile<bool> x 30,000 (same expression + params) 34,846 ms 18 ms ~1,936x
C RulesEngine.ExecuteActionWorkflowAsync x 1,000 (30-rule workflow) 1,182 ms 3 ms ~394x
D RulesEngine.ExecuteAllRulesAsync x 1,000, warm engine (30-rule workflow) 11 ms 17 ms ~0.65x
E New RulesEngine + ExecuteAllRulesAsync x 50, fresh engine each call (30 rules) 1,890 ms 1,867 ms ~1.01x

Notes:

  • A, B, C are the paths the v5.0.1 regression actually hits. Restored to v5.0.0 throughput.
  • D picks up ~6 µs/call of cache-key hashing overhead on a path already served by RulesCache. Within noise, negligible vs compile cost.
  • E is unchanged because the new cache is instance-scoped by design. Fresh-engine workloads should reuse the RulesEngine; restoring a process-wide static cache is what caused the original latent bug.

Test plan

  • All 131 existing unit tests pass on net6.0 / net8.0 / net9.0 / net10.0.
  • Three new tests in RuleExpressionParserTests:
    • Compile_SameExpressionAndParams_ReturnsCachedDelegate — verifies the cache returns the same delegate instance.
    • Compile_DifferentSettings_DoNotShareCache — guards against the original static-field cross-settings bug.
    • Compile_CacheDisabled_RecompilesEveryCall — verifies CacheConfig.SizeLimit == 0 opts out.
  • Existing tests that mutate workflow expressions and re-add them (RemoveWorkFlow_ShouldRemoveAllCompiledCache, ClearWorkFlow_ShouldRemoveAllCompiledCache, WorkflowUpdate_GlobalParam_ShouldReflect) still pass — confirms the scoped-params cache key correctly includes the value-expression text.

…osoft#673)

The static MemCache around Compile<T> was removed in v5.0.1 (microsoft#501) along
with the AutoRegisterInputType feature. Removing it caused every call to
Compile<T> / CompileRuleExpressionParameters to re-run the dynamic
expression parser and re-emit IL, causing ~10x slowdowns reported in microsoft#673
and 100x-1900x slowdowns on the public parser and ExecuteActionWorkflowAsync
paths that are not served by the workflow-level RulesCache.

Reintroduce the cache as an instance field (not static, to avoid the latent
cross-settings sharing bug the old static had). The key includes the
expression text, ordered (paramName, paramType), return type, and a settings
fingerprint covering IsExpressionCaseSensitive, UseFastExpressionCompiler,
and CustomTypes. Scoped-params keys also include each value-expression's
text so workflow mutations correctly invalidate.

Honors the existing ReSettings.CacheConfig; SizeLimit == 0 disables.

Benchmark (median of 5 runs, .NET 8, 30-rule workflow):
  Parser.Evaluate<bool> x 30k:           34,730 ms -> 33 ms   (~1052x)
  Parser.Compile<bool>  x 30k:           34,846 ms -> 18 ms   (~1936x)
  ExecuteActionWorkflowAsync x 1000:      1,182 ms ->  3 ms   ( ~394x)
  ExecuteAllRulesAsync x 1000 (warm):        11 ms -> 17 ms   (within noise)
  Fresh engine x 50:                      1,890 ms -> 1,867 ms (unchanged)
@YogeshPraj YogeshPraj merged commit 6e62a1b into microsoft:main May 27, 2026
3 checks passed
YogeshPraj added a commit that referenced this pull request May 27, 2026
Bump <Version> from 6.0.0 to 6.0.1-preview.1 and add a CHANGELOG entry
covering the three PRs landed since 6.0.0: #727 (perf cache restore),
#728 (action-exception propagation, list schema union, OutputExpression
hint, global-param dedup, object-return diagnostics), and #729
(ExecuteActionWorkflowAsync FormatErrorMessages, ActionContext null guard,
deep ErrorMessage interpolation, plus regression guards for #581, #590,
#606, #608).

Co-authored-by: Yogesh Prajapati <yogeshcprajapati@outlook.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Huge Performance Degredation in version 5.0.1

2 participants