From 2e9a3ad04f8b704fbe5a3ffad07041e2e8e93b8f Mon Sep 17 00:00:00 2001
From: Marko Lahma <marko.lahma@gmail.com>
Date: Mon, 17 Aug 2026 02:16:38 +0300
Subject: [PATCH] Trim per-evaluation work in the JavaScript evaluator (#7892)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* perf(javascript): trim per-evaluation work in the JavaScript evaluator

A Jint engine is built for every expression evaluation, so anything done during
setup is paid for on every evaluation. Four pieces of that work are avoidable:

* The three `IObjectConverter` implementations are stateless but were allocated
  fresh for every engine. They are now shared static instances.

* Every prepared-script cache lookup — including hits — computed a SHA-256 hash
  of the expression text, base64-encoded it and concatenated a prefix, purely to
  build the cache key. Using a dedicated key type instead keeps the entries
  distinct from other users of the shared cache while letting the expression
  itself be the key, so a hit is a dictionary lookup. Looking the entry up
  directly rather than through `GetOrCreate` also keeps the factory closure off
  the hit path.

* `ObjectConverterHelper.ConvertToJsObject` built an explicit `PropertyDescriptor`
  per property and called `DefineOwnProperty`. `CreateDataProperty` is public,
  produces exactly the same writable/enumerable/configurable descriptor, and is
  the engine's fast path for it.

* The variable write-back resolved the workflow input names — walking the whole
  activity execution context ancestor chain — before checking whether there was
  anything to write back. Only variables the expression actually referenced are
  copied into the engine, so for the common case of an expression that never
  mentions `variables.` the container is empty and all of that work is wasted.
  The input names are also now looked up through a set rather than a list.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0179sA2T7HuRfRfSc2JirFik

* test(javascript): pin the parse-failure test to the same exception every time

Calling `ThrowsAnyAsync<Exception>` twice only proved that both evaluations threw
something, which is exactly the assertion a poisoned cache would still satisfy:
had the failed preparation left a null or half-built entry behind, the second
evaluation would have failed too, just with a different exception. The test now
captures both exceptions and asserts they are the same type with the same
message, so "keeps reporting the same parse failure" is what is actually checked.

The message is stable to compare: it is `Could not prepare script: Unexpected end
of input (1:9)`, and since both evaluations run the identical script literal the
position is identical as well. No file or path detail is involved.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0179sA2T7HuRfRfSc2JirFik

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 .../Handlers/ConfigureEngineWithVariables.cs  |  8 ++-
 .../Helpers/ObjectConverterHelper.cs          | 12 ++---
 .../Services/JintJavaScriptEvaluator.cs       | 40 ++++++++------
 .../ScriptCacheTests.cs                       | 52 +++++++++++++++++++
 4 files changed, 86 insertions(+), 26 deletions(-)
 create mode 100644 test/integration/Elsa.JavaScript.IntegrationTests/ScriptCacheTests.cs

diff --git a/src/modules/Elsa.Expressions.JavaScript/Handlers/ConfigureEngineWithVariables.cs b/src/modules/Elsa.Expressions.JavaScript/Handlers/ConfigureEngineWithVariables.cs
index 13400e509..042e43eb9 100644
--- a/src/modules/Elsa.Expressions.JavaScript/Handlers/ConfigureEngineWithVariables.cs
+++ b/src/modules/Elsa.Expressions.JavaScript/Handlers/ConfigureEngineWithVariables.cs
@@ -46,7 +46,13 @@ public partial class ConfigureEngineWithVariables(IOptions<JintOptions> options)
         var context = notification.Context;
         var engine = notification.Engine;
         var variablesContainer = (IDictionary<string, object?>)engine.GetValue("variables").ToObject()!;
-        var inputNames = GetInputNames(context).FilterInvalidVariableNames().Distinct().ToList();
+
+        // Only the variables the expression actually referenced were copied in, so an expression that never
+        // mentions "variables." has nothing to copy back and does not need the input names resolved at all.
+        if (variablesContainer.Count == 0)
+            return;
+
+        var inputNames = GetInputNames(context).FilterInvalidVariableNames().ToHashSet(StringComparer.Ordinal);
 
         foreach (var (variableName, variableValue) in variablesContainer)
         {
diff --git a/src/modules/Elsa.Expressions.JavaScript/Helpers/ObjectConverterHelper.cs b/src/modules/Elsa.Expressions.JavaScript/Helpers/ObjectConverterHelper.cs
index 28d9ecdcc..e3e2a8282 100644
--- a/src/modules/Elsa.Expressions.JavaScript/Helpers/ObjectConverterHelper.cs
+++ b/src/modules/Elsa.Expressions.JavaScript/Helpers/ObjectConverterHelper.cs
@@ -1,19 +1,14 @@
 using System.Collections;
 using System.Dynamic;
 using Elsa.Extensions;
-using Elsa.Expressions.JavaScript.Options;
 using Jint;
 using Jint.Native;
 using Jint.Native.Object;
-using Jint.Runtime.Descriptors;
-using Microsoft.Extensions.Options;
 
 namespace Elsa.Expressions.JavaScript.Helpers;
 
 internal static class ObjectConverterHelper
 {
-    
-    
     public static object? ProcessVariableValue(Engine engine, object? variableValue)
     {
         if (variableValue == null)
@@ -31,10 +26,9 @@ internal static class ObjectConverterHelper
 
         foreach (var kvp in expando)
         {
-            var value = kvp.Value;
-            var jsValue = ConvertToJsValue(engine, value);
-            var propertyDescriptor = new PropertyDescriptor(jsValue, true, true, true);
-            jsObject.DefineOwnProperty(kvp.Key, propertyDescriptor);
+            // CreateDataProperty defines a writable, enumerable and configurable property, which is what the
+            // explicit descriptor used to spell out, and takes the engine's fast path for doing so.
+            jsObject.CreateDataProperty(kvp.Key, ConvertToJsValue(engine, kvp.Value));
         }
 
         return jsObject;
diff --git a/src/modules/Elsa.Expressions.JavaScript/Services/JintJavaScriptEvaluator.cs b/src/modules/Elsa.Expressions.JavaScript/Services/JintJavaScriptEvaluator.cs
index bbc395fc6..15c1818cb 100644
--- a/src/modules/Elsa.Expressions.JavaScript/Services/JintJavaScriptEvaluator.cs
+++ b/src/modules/Elsa.Expressions.JavaScript/Services/JintJavaScriptEvaluator.cs
@@ -1,6 +1,4 @@
 using System.Diagnostics.CodeAnalysis;
-using System.Security.Cryptography;
-using System.Text;
 using Acornima.Ast;
 using Elsa.Expressions.Helpers;
 using Elsa.Expressions.Models;
@@ -25,6 +23,9 @@ namespace Elsa.Expressions.JavaScript.Services;
 public class JintJavaScriptEvaluator(IConfiguration configuration, INotificationSender mediator, IOptions<JintOptions> scriptOptions, IMemoryCache memoryCache)
     : IJavaScriptEvaluator
 {
+    // The converters are stateless, so a single instance of each can serve every engine.
+    private static readonly IObjectConverter[] ObjectConverters = [new ByteArrayConverter(), new EnumToStringConverter(), new JsonElementConverter()];
+
     private readonly JintOptions _jintOptions = scriptOptions.Value;
 
     /// <inheritdoc />
@@ -97,7 +98,7 @@ public class JintJavaScriptEvaluator(IConfiguration configuration, INotification
 
     private void ConfigureObjectConverters(Jint.Options options)
     {
-        options.Interop.ObjectConverters.AddRange([new ByteArrayConverter(), new EnumToStringConverter(), new JsonElementConverter()]);
+        options.Interop.ObjectConverters.AddRange(ObjectConverters);
     }
 
     private void ConfigureArgumentGetters(Engine engine, ExpressionEvaluatorOptions options)
@@ -124,15 +125,24 @@ public class JintJavaScriptEvaluator(IConfiguration configuration, INotification
 
     private Prepared<Script> GetOrCreatePrepareScript(string expression)
     {
-        var cacheKey = "jint:script:" + Hash(expression);
+        // The key type keeps these entries distinct from any other consumer of the shared cache, so the
+        // expression itself can be used as the key. A cache hit then costs a dictionary lookup rather than
+        // a hash of the entire expression plus the allocations needed to render that hash as a string.
+        var cacheKey = new ScriptCacheKey(expression);
 
-        return memoryCache.GetOrCreate(cacheKey, entry =>
-        {
-            if (_jintOptions.ScriptCacheTimeout.HasValue)
-                entry.SetSlidingExpiration(_jintOptions.ScriptCacheTimeout.Value);
+        // Looking the entry up directly rather than through GetOrCreate keeps the factory closure off the
+        // hot path: it is only needed on a miss.
+        if (memoryCache.TryGetValue(cacheKey, out Prepared<Script> cachedScript))
+            return cachedScript;
 
-            return PrepareScript(expression);
-        })!;
+        using var entry = memoryCache.CreateEntry(cacheKey);
+
+        if (_jintOptions.ScriptCacheTimeout.HasValue)
+            entry.SetSlidingExpiration(_jintOptions.ScriptCacheTimeout.Value);
+
+        var preparedScript = PrepareScript(expression);
+        entry.Value = preparedScript;
+        return preparedScript;
     }
 
     private Prepared<Script> PrepareScript(string expression)
@@ -147,10 +157,8 @@ public class JintJavaScriptEvaluator(IConfiguration configuration, INotification
         return Engine.PrepareScript(expression, options: prepareOptions);
     }
 
-    private string Hash(string input)
-    {
-        var bytes = Encoding.UTF8.GetBytes(input);
-        var hash = SHA256.HashData(bytes);
-        return Convert.ToBase64String(hash);
-    }
+    /// <summary>
+    /// Identifies a prepared script in the shared memory cache.
+    /// </summary>
+    private readonly record struct ScriptCacheKey(string Expression);
 }
\ No newline at end of file
diff --git a/test/integration/Elsa.JavaScript.IntegrationTests/ScriptCacheTests.cs b/test/integration/Elsa.JavaScript.IntegrationTests/ScriptCacheTests.cs
new file mode 100644
index 000000000..03b9cdb3c
--- /dev/null
+++ b/test/integration/Elsa.JavaScript.IntegrationTests/ScriptCacheTests.cs
@@ -0,0 +1,52 @@
+using Elsa.Expressions.JavaScript.Contracts;
+using Elsa.Expressions.Models;
+using Elsa.Testing.Shared;
+using Microsoft.Extensions.DependencyInjection;
+using Xunit;
+using Xunit.Abstractions;
+
+namespace Elsa.JavaScript.IntegrationTests;
+
+/// <summary>
+/// Verifies the behaviour of the prepared-script cache.
+/// </summary>
+public class ScriptCacheTests
+{
+    private readonly IServiceProvider _serviceProvider;
+    private readonly IJavaScriptEvaluator _evaluator;
+
+    public ScriptCacheTests(ITestOutputHelper testOutputHelper)
+    {
+        _serviceProvider = new TestApplicationBuilder(testOutputHelper).Build();
+        _evaluator = _serviceProvider.GetRequiredService<IJavaScriptEvaluator>();
+    }
+
+    [Fact(DisplayName = "Repeated evaluation of the same expression produces the same result")]
+    public async Task RepeatedEvaluationIsStable()
+    {
+        Assert.Equal("3", await EvaluateAsync("return '' + (1 + 2);"));
+        Assert.Equal("3", await EvaluateAsync("return '' + (1 + 2);"));
+        Assert.Equal("7", await EvaluateAsync("return '' + (3 + 4);"));
+        Assert.Equal("3", await EvaluateAsync("return '' + (1 + 2);"));
+    }
+
+    [Fact(DisplayName = "An expression that fails to parse reports the failure on every evaluation")]
+    public async Task ExpressionThatFailsToParseKeepsFailing()
+    {
+        var first = await Assert.ThrowsAnyAsync<Exception>(() => EvaluateAsync("return ("));
+        var second = await Assert.ThrowsAnyAsync<Exception>(() => EvaluateAsync("return ("));
+
+        // Every evaluation has to report the same parse failure. Merely throwing twice would also be satisfied
+        // by a cache that stored a null or half-built entry for the failed preparation, because the second
+        // evaluation would then fail too — just with a different exception. Comparing the two exceptions rules
+        // that out. The script is identical on both evaluations, so any position the message carries is too.
+        Assert.IsType(first.GetType(), second);
+        Assert.Equal(first.Message, second.Message);
+    }
+
+    private async Task<string?> EvaluateAsync(string script)
+    {
+        var context = new ExpressionExecutionContext(_serviceProvider, new());
+        return (string?)await _evaluator.EvaluateAsync(script, typeof(string), context);
+    }
+}