Merge pull request #6320 from elsa-workflows/perf/js-variables

Add option to disable variable copying in Jint engine

src/apps/Elsa.Server.Web/Program.cs

@@ -89,6 +89,7 @@
 const bool useAgents = false;
 const bool useSecrets = false;
 const bool disableVariableWrappers = false;
+const bool disableVariableCopying = false;
 
 var builder = WebApplication.CreateBuilder(args);
 var services = builder.Services;
@@ -355,6 +356,7 @@
             {
                 options.AllowClrAccess = true;
                 options.DisableWrappers = disableVariableWrappers;
+                options.DisableVariableCopying = disableVariableCopying;
                 options.RegisterType<OrderReceived>();
                 options.ConfigureEngine(engine =>
                 {

src/modules/Elsa.JavaScript/Extensions/EngineExtensions.cs

@@ -24,12 +24,12 @@ public static class EngineExtensions
 
     internal static void SyncVariablesContainer(this Engine engine, IOptions<JintOptions> options, string name, object? value)
     {
-        if (!options.Value.DisableWrappers)
-        {
-            // To ensure both variable accessor syntaxes work, we need to update the variables container in the engine as well as the context to keep them in sync.
-            var variablesContainer = (IDictionary<string, object?>)engine.GetValue("variables").ToObject()!;
-            variablesContainer[name] = ObjectConverterHelper.ProcessVariableValue(engine, value);
-            engine.SetValue("variables", variablesContainer);
-        }
+        if (options.Value.DisableWrappers || options.Value.DisableVariableCopying)
+            return;
+
+        // To ensure both variable accessor syntaxes work, we need to update the variables container in the engine as well as the context to keep them in sync.
+        var variablesContainer = (IDictionary<string, object?>)engine.GetValue("variables").ToObject()!;
+        variablesContainer[name] = ObjectConverterHelper.ProcessVariableValue(engine, value);
+        engine.SetValue("variables", variablesContainer);
     }
 }

src/modules/Elsa.JavaScript/Handlers/ConfigureEngineWithVariables.cs

@@ -1,4 +1,5 @@
 using System.Dynamic;
+using System.Text.RegularExpressions;
 using Elsa.Expressions.Models;
 using Elsa.Extensions;
 using Elsa.JavaScript.Extensions;
@@ -8,7 +9,6 @@
 using Elsa.Mediator.Contracts;
 using Elsa.Workflows.Activities;
 using JetBrains.Annotations;
-using Jint;
 using Jint.Native;
 using Microsoft.Extensions.Options;
 
@@ -18,12 +18,14 @@ namespace Elsa.JavaScript.Handlers;
 /// A handler that configures the Jint engine with workflow variables.
 /// </summary>
 [UsedImplicitly]
-public class ConfigureEngineWithVariables(IOptions<JintOptions> options) : INotificationHandler<EvaluatingJavaScript>, INotificationHandler<EvaluatedJavaScript>
+public partial class ConfigureEngineWithVariables(IOptions<JintOptions> options) : INotificationHandler<EvaluatingJavaScript>, INotificationHandler<EvaluatedJavaScript>
 {
+    private bool IsEnabled => options.Value is { DisableWrappers: false, DisableVariableCopying: false };
+
     /// <inheritdoc />
     public Task HandleAsync(EvaluatingJavaScript notification, CancellationToken cancellationToken)
     {
-        if (options.Value.DisableWrappers)
+        if (!IsEnabled)
             return Task.CompletedTask;
 
         CopyVariablesIntoEngine(notification);
@@ -32,7 +34,7 @@ public Task HandleAsync(EvaluatingJavaScript notification, CancellationToken can
 
     public Task HandleAsync(EvaluatedJavaScript notification, CancellationToken cancellationToken)
     {
-        if (options.Value.DisableWrappers)
+        if (!IsEnabled)
             return Task.CompletedTask;
 
         CopyVariablesIntoWorkflowExecutionContext(notification);
@@ -60,7 +62,8 @@ private void CopyVariablesIntoEngine(EvaluatingJavaScript notification)
     {
         var engine = notification.Engine;
         var context = notification.Context;
-        var variableNames = context.GetVariableNamesInScope().FilterInvalidVariableNames().ToList();
+        var expression = notification.Expression;
+        var variableNames = GetUsedVariableNames(context, expression).ToList();
         var variablesContainer = (IDictionary<string, object?>)new ExpandoObject();
 
         foreach (var variableName in variableNames)
@@ -73,6 +76,17 @@ private void CopyVariablesIntoEngine(EvaluatingJavaScript notification)
         engine.SetValue("variables", variablesContainer);
     }
 
+    private IEnumerable<string> GetUsedVariableNames(ExpressionExecutionContext context, string expression)
+    {
+        var variableNames = context.GetVariableNamesInScope().FilterInvalidVariableNames();
+
+        var variableNamesInScript = ExtractVariableNamesRegex().Matches(expression)
+            .Select(m => m.Groups[1].Value)
+            .ToList();
+
+        return variableNames.Where(x => variableNamesInScript.Contains(x));
+    }
+
     private IEnumerable<string> GetInputNames(ExpressionExecutionContext context)
     {
         var activityExecutionContext = context.TryGetActivityExecutionContext(out var aec) ? aec : null;
@@ -95,4 +109,7 @@ private IEnumerable<string> GetInputNames(ExpressionExecutionContext context)
             activityExecutionContext = activityExecutionContext.ParentActivityExecutionContext;
         }
     }
+
+    [GeneratedRegex(@"variables\.(\w+)(?:\.\w+)*")]
+    private static partial Regex ExtractVariableNamesRegex();
 }

src/modules/Elsa.JavaScript/Options/JintOptions.cs

@@ -47,6 +47,12 @@ public class JintOptions
     /// </summary>
     public bool DisableWrappers { get; set; }
 
+    /// <summary>
+    /// Disables copying workflow variables into the Jint engine and copying them back into the workflow execution context.
+    /// Disabling this option will increase performance but will also prevent you from accessing workflow variables from within JavaScript expressions using the <c>variables.MyVariable</c> syntax.
+    /// </summary>
+    public bool DisableVariableCopying { get; set; }
+
     /// <summary>
     /// Configures the Jint engine options.
     /// </summary>

src/modules/Elsa.Workflows.Core/Extensions/ExpressionExecutionContextExtensions.cs

@@ -119,7 +119,7 @@ public static IDictionary<object, object> CreateTriggerIndexingPropertiesFrom(Wo
     public static Variable? GetVariable(this ExpressionExecutionContext context, string name, bool localScopeOnly = false)
     {
         var block = context.GetVariableBlock(name, localScopeOnly);
-        return block?.Metadata is VariableBlockMetadata metadata ? metadata.Variable : default;
+        return block?.Metadata is VariableBlockMetadata metadata ? metadata.Variable : null;
     }
 
     private static MemoryBlock? GetVariableBlock(this ExpressionExecutionContext context, string name, bool localScopeOnly = false)
@@ -138,12 +138,12 @@ public static IDictionary<object, object> CreateTriggerIndexingPropertiesFrom(Wo
     /// Creates a named variable in the context.
     /// </summary>
     public static Variable CreateVariable<T>(this ExpressionExecutionContext context, string name, T? value, Type? storageDriverType = null,
-        Action<MemoryBlock>? configure = default)
+        Action<MemoryBlock>? configure = null)
     {
         var existingVariable = context.GetVariable(name, localScopeOnly: true);
 
         if (existingVariable != null)
-            throw new Exception($"Variable {name} already exists in the context.");
+            throw new($"Variable {name} already exists in the context.");
 
         var variable = new Variable(name, value)
         {
@@ -173,7 +173,7 @@ public static ExpressionExecutionContext GetVariableContainerContext(this Expres
     /// <summary>
     /// Sets the value of a named variable in the context.
     /// </summary>
-    public static Variable SetVariable<T>(this ExpressionExecutionContext context, string name, T? value, Action<MemoryBlock>? configure = default)
+    public static Variable SetVariable<T>(this ExpressionExecutionContext context, string name, T? value, Action<MemoryBlock>? configure = null)
     {
         var variable = context.GetVariable(name);
 
@@ -193,7 +193,7 @@ public static Variable SetVariable<T>(this ExpressionExecutionContext context, s
     /// <summary>
     /// Sets the output to the specified value.
     /// </summary>
-    public static void Set(this ExpressionExecutionContext context, Output? output, object? value, Action<MemoryBlock>? configure = default)
+    public static void Set(this ExpressionExecutionContext context, Output? output, object? value, Action<MemoryBlock>? configure = null)
     {
         if (output != null)
         {
@@ -412,11 +412,11 @@ private static JsonSerializerOptions GetSerializerOptions(ExpressionExecutionCon
         // Otherwise, return the input.
         var workflowExecutionContext = context.GetWorkflowExecutionContext();
         var input = workflowExecutionContext.Input;
-        return input.TryGetValue(name, out var value) ? value : default;
+        return input.TryGetValue(name, out var value) ? value : null;
     }
 
     /// <summary>
-    /// Returns the value of the specified input.
+    /// Returns the value of the specified output.
     /// </summary>
     /// <param name="context"></param>
     /// <param name="activityIdOrName">The ID or name of the activity.</param>
@@ -433,9 +433,9 @@ private static JsonSerializerOptions GetSerializerOptions(ExpressionExecutionCon
             throw new InvalidOperationException("Activity not found.");
 
         var outputRegister = workflowExecutionContext.GetActivityOutputRegister();
-        var outputRecordCandidates = outputRegister.FindMany(x => x.ActivityId == activity.Id && x.OutputName == outputName).ToList();
+        var outputRecordCandidates = outputRegister.FindMany(activity.Id, outputName);
         var containerIds = activityExecutionContext.GetAncestors().Select(x => x.Id).ToList();
-        var filteredOutputRecordCandidates = outputRecordCandidates.Where(x => containerIds.Contains(x.ContainerId)).ToList();
+        var filteredOutputRecordCandidates = outputRecordCandidates.Where(x => containerIds.Contains(x.ContainerId));
         var outputRecord = filteredOutputRecordCandidates.FirstOrDefault();
         return outputRecord?.Value;
     }
@@ -464,7 +464,7 @@ public static async IAsyncEnumerable<ActivityOutputs> GetActivityOutputs(this Ex
             foreach (var output in activityDescriptor.Outputs)
             {
                 var outputPascalName = output.Name.Pascalize();
-                yield return new ActivityOutputs(activity.Id, activityIdPascalName, [
+                yield return new(activity.Id, activityIdPascalName, [
                     outputPascalName
                 ]);
             }
@@ -508,7 +508,7 @@ public static IEnumerable<WorkflowInput> GetWorkflowInputs(this ExpressionExecut
             {
                 var inputPascalName = inputEntry.Key.Pascalize();
                 var inputValue = inputEntry.Value;
-                yield return new WorkflowInput(inputPascalName, inputValue);
+                yield return new(inputPascalName, inputValue);
             }
         }
         else
@@ -522,7 +522,7 @@ public static IEnumerable<WorkflowInput> GetWorkflowInputs(this ExpressionExecut
 
                 var variable = variableBlockMetadata.Variable;
                 var variablePascalName = variable.Name.Pascalize();
-                yield return new WorkflowInput(variablePascalName, block.Value);
+                yield return new(variablePascalName, block.Value);
             }
         }
     }

src/modules/Elsa.Workflows.Core/Models/ActivityOutputRegister.cs

@@ -5,7 +5,8 @@ namespace Elsa.Workflows.Models;
 /// </summary>
 public class ActivityOutputRegister
 {
-    private readonly ICollection<ActivityOutputRecord> _records = new List<ActivityOutputRecord>();
+    private readonly Dictionary<string, List<ActivityOutputRecord>> _recordsByActivityIdAndOutputName = new();
+    private readonly Dictionary<string, ActivityOutputRecord> _recordsByActivityInstanceIdAndOutputName = new();
 
     /// <summary>
     /// The default output name.
@@ -19,7 +20,7 @@ public class ActivityOutputRegister
     /// <param name="outputValue">The output value.</param>
     public void Record(ActivityExecutionContext activityExecutionContext, object? outputValue)
     {
-        Record(activityExecutionContext, default, outputValue);
+        Record(activityExecutionContext, null, outputValue);
     }
 
     /// <summary>
@@ -39,30 +40,46 @@ public void Record(ActivityExecutionContext activityExecutionContext, string? ou
         // Inspect the output descriptor to see if the specified output name matches any PropertyInfo's name.
         // If so, use that descriptor's name instead.
         var outputDescriptor = activityExecutionContext.ActivityDescriptor.Outputs.FirstOrDefault(x => x.PropertyInfo?.Name == outputName);
-        
+
         if (outputDescriptor != null)
             outputName = outputDescriptor.Name;
 
         var record = new ActivityOutputRecord(containerId, activityId, activityInstanceId, outputName, outputValue);
 
-        _records.Add(record);
+        _recordsByActivityInstanceIdAndOutputName[CreateActivityInstanceIdLookupKey(activityInstanceId, outputName)] = record;
+
+        var scopedRecordsKey = CreateActivityIdLookupKey(activityId, outputName);
+
+        if (!_recordsByActivityIdAndOutputName.TryGetValue(scopedRecordsKey, out var scopedRecords))
+        {
+            scopedRecords = new();
+            _recordsByActivityIdAndOutputName[scopedRecordsKey] = scopedRecords;
+        }
+
+        scopedRecords.Add(record);
     }
 
     /// <summary>
-    /// Finds all output records matching the specified predicate.
+    ///  Finds all output records for the specified activity ID and output name.
     /// </summary>
-    public IEnumerable<ActivityOutputRecord> FindMany(Func<ActivityOutputRecord, bool> predicate) => _records.Where(predicate);
+    public IEnumerable<ActivityOutputRecord> FindMany(string activityId, string? outputName = null)
+    {
+        var key = CreateActivityIdLookupKey(activityId, outputName);
+        return _recordsByActivityIdAndOutputName.TryGetValue(key, out var records) ? records : Enumerable.Empty<ActivityOutputRecord>();
+    }
 
     /// <summary>
     /// Gets the output value for the specified activity ID.
     /// </summary>
     /// <param name="activityId">The activity ID.</param>
     /// <param name="outputName">Name of the output.</param>
     /// <returns>The output value.</returns>
-    public object? FindOutputByActivityId(string activityId, string? outputName = default)
+    public object? FindOutputByActivityId(string activityId, string? outputName = null)
     {
-        var record = _records.LastOrDefault(x => x.ActivityId == activityId && x.OutputName == (outputName ?? DefaultOutputName));
-        return record?.Value;
+        var key = CreateActivityIdLookupKey(activityId, outputName);
+        return !_recordsByActivityIdAndOutputName.TryGetValue(key, out var records)
+            ? null
+            : records.FirstOrDefault()?.Value;
     }
 
     /// <summary>
@@ -71,9 +88,14 @@ public void Record(ActivityExecutionContext activityExecutionContext, string? ou
     /// <param name="activityInstanceId">The activity instance ID.</param>
     /// <param name="outputName"></param>
     /// <returns>The output value.</returns>
-    public object? FindOutputByActivityInstanceId(string activityInstanceId, string? outputName = default)
+    public object? FindOutputByActivityInstanceId(string activityInstanceId, string? outputName = null)
     {
-        var record = _records.LastOrDefault(x => x.ActivityInstanceId == activityInstanceId && x.OutputName == (outputName ?? DefaultOutputName));
-        return record?.Value;
+        var key = CreateActivityInstanceIdLookupKey(activityInstanceId, outputName);
+        return !_recordsByActivityInstanceIdAndOutputName.TryGetValue(key, out var record)
+            ? null
+            : record.Value;
     }
+
+    private string CreateActivityIdLookupKey(string activityId, string? outputName) => $"{activityId}:{outputName ?? DefaultOutputName}";
+    private string CreateActivityInstanceIdLookupKey(string activityInstanceId, string? outputName) => $"{activityInstanceId}:{outputName ?? DefaultOutputName}";
 }

src/modules/Elsa.Workflows.Core/Models/PropertyDescriptor.cs

@@ -11,13 +11,13 @@ public abstract class PropertyDescriptor
     /// <summary>
     /// The name.
     /// </summary>
-    public string Name { get; set; } = default!;
+    public string Name { get; set; } = null!;
 
     /// <summary>
     /// The .NET type.
     /// </summary>
     [JsonPropertyName("typeName")]
-    public Type Type { get; set; } = default!;
+    public Type Type { get; set; } = null!;
 
     /// <summary>
     /// The user friendly name of the input. Used by UI tools.
@@ -53,13 +53,13 @@ public abstract class PropertyDescriptor
     /// Returns the value of the input property for the specified activity.
     /// </summary>
     [JsonIgnore]
-    public Func<IActivity, object?> ValueGetter { get; set; } = default!;
+    public Func<IActivity, object?> ValueGetter { get; set; } = null!;
 
     /// <summary>
     /// Sets the value of the input property for the specified activity.
     /// </summary>
     [JsonIgnore]
-    public Action<IActivity, object?> ValueSetter { get; set; } = default!;
+    public Action<IActivity, object?> ValueSetter { get; set; } = null!;
 
     /// <summary>
     /// The source of the property, if any.