Create ExpressionCache to unify constant lookups (#2037)

Jint/Runtime/Interpreter/Expressions/ExpressionCache.cs

@@ -0,0 +1,190 @@
+using System.Runtime.CompilerServices;
+using Jint.Native;
+using Jint.Native.Iterator;
+
+namespace Jint.Runtime.Interpreter.Expressions;
+
+/// <summary>
+/// Optimizes constants values from expression array and only returns the actual JsValue in consecutive calls.
+/// </summary>
+internal sealed class ExpressionCache
+{
+    private object?[] _expressions = [];
+    private bool _fullyCached;
+
+    internal bool HasSpreads { get; private set; }
+
+    internal void Initialize(EvaluationContext context, ReadOnlySpan<Expression> arguments)
+    {
+        if (arguments.Length == 0)
+        {
+            _fullyCached = true;
+            _expressions = [];
+            return;
+        }
+
+        _expressions = new object?[arguments.Length];
+        _fullyCached = true;
+        for (var i = 0; i < (uint) arguments.Length; i++)
+        {
+            var argument = arguments[i];
+            if (argument is null)
+            {
+                _fullyCached = false;
+                continue;
+            }
+
+            var expression = JintExpression.Build(argument);
+
+            if (argument.Type == NodeType.Literal)
+            {
+                _expressions[i] = expression.GetValue(context).Clone();
+                continue;
+            }
+
+            _expressions[i] = expression;
+            _fullyCached &= argument.Type == NodeType.Literal;
+            HasSpreads |= CanSpread(argument);
+
+            if (argument.Type == NodeType.ArrayExpression)
+            {
+                ref readonly var elements = ref ((ArrayExpression) argument).Elements;
+                foreach (var e in elements.AsSpan())
+                {
+                    HasSpreads |= CanSpread(e);
+                }
+            }
+        }
+    }
+
+    public JsValue[] ArgumentListEvaluation(EvaluationContext context, out bool rented)
+    {
+        rented = false;
+        if (_fullyCached)
+        {
+            return Unsafe.As<JsValue[]>(_expressions);
+        }
+
+        if (HasSpreads)
+        {
+            var args = new List<JsValue>(_expressions.Length);
+            BuildArgumentsWithSpreads(context, args);
+            return args.ToArray();
+        }
+
+        var arguments = context.Engine._jsValueArrayPool.RentArray(_expressions.Length);
+        rented = true;
+
+        BuildArguments(context, arguments);
+
+        return arguments;
+    }
+
+    internal void BuildArguments(EvaluationContext context, JsValue[] targetArray)
+    {
+        var expressions = _expressions;
+        for (uint i = 0; i < (uint) expressions.Length; i++)
+        {
+            targetArray[i] = GetValue(context, expressions[i])!;
+        }
+    }
+
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
+    public JsValue GetValue(EvaluationContext context, int index)
+    {
+        return GetValue(context, _expressions[index]);
+    }
+
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
+    private static JsValue GetValue(EvaluationContext context, object? value)
+    {
+        return value switch
+        {
+            JintExpression expression => expression.GetValue(context).Clone(),
+            _ => (JsValue) value!,
+        };
+    }
+
+    public bool IsAnonymousFunctionDefinition(int index)
+    {
+        var expressions = _expressions;
+        return index < expressions.Length && (expressions[index] as JintExpression)?._expression.IsAnonymousFunctionDefinition() == true;
+    }
+
+    private static bool CanSpread(Node? e)
+    {
+        if (e is null)
+        {
+            return false;
+        }
+
+        return e.Type == NodeType.SpreadElement || e is AssignmentExpression { Right.Type: NodeType.SpreadElement };
+    }
+
+    internal JsValue[] DefaultSuperCallArgumentListEvaluation(EvaluationContext context)
+    {
+        // This branch behaves similarly to constructor(...args) { super(...args); }.
+        // The most notable distinction is that while the aforementioned ECMAScript source text observably calls
+        // the @@iterator method on %Array.prototype%, this function does not.
+
+        var spreadExpression = (JintSpreadExpression) _expressions[0]!;
+        var array = (JsArray) spreadExpression._argument.GetValue(context);
+        var length = array.GetLength();
+        var args = new List<JsValue>((int) length);
+        for (uint j = 0; j < length; ++j)
+        {
+            array.TryGetValue(j, out var value);
+            args.Add(value);
+        }
+
+        return args.ToArray();
+    }
+
+    internal void BuildArgumentsWithSpreads(EvaluationContext context, List<JsValue> target)
+    {
+        foreach (var expression in _expressions)
+        {
+            if (expression is JintSpreadExpression jse)
+            {
+                jse.GetValueAndCheckIterator(context, out var objectInstance, out var iterator);
+                // optimize for array unless someone has touched the iterator
+                if (objectInstance is JsArray { HasOriginalIterator: true } ai)
+                {
+                    var length = ai.GetLength();
+                    for (uint j = 0; j < length; ++j)
+                    {
+                        ai.TryGetValue(j, out var value);
+                        target.Add(value);
+                    }
+                }
+                else
+                {
+                    var protocol = new ArraySpreadProtocol(context.Engine, target, iterator!);
+                    protocol.Execute();
+                }
+            }
+            else
+            {
+                target.Add(GetValue(context, expression)!);
+            }
+        }
+    }
+
+    private sealed class ArraySpreadProtocol : IteratorProtocol
+    {
+        private readonly List<JsValue> _instance;
+
+        public ArraySpreadProtocol(
+            Engine engine,
+            List<JsValue> instance,
+            IteratorInstance iterator) : base(engine, iterator, 0)
+        {
+            _instance = instance;
+        }
+
+        protected override void ProcessItem(JsValue[] arguments, JsValue currentValue)
+        {
+            _instance.Add(currentValue);
+        }
+    }
+}

Jint/Runtime/Interpreter/Expressions/JintArrayExpression.cs

@@ -5,8 +5,7 @@ namespace Jint.Runtime.Interpreter.Expressions;
 
 internal sealed class JintArrayExpression : JintExpression
 {
-    private JintExpression?[] _expressions = Array.Empty<JintExpression?>();
-    private bool _hasSpreads;
+    private readonly ExpressionCache _arguments = new();
     private bool _initialized;
 
     private JintArrayExpression(ArrayExpression expression) : base(expression)
@@ -20,20 +19,9 @@ public static JintExpression Build(ArrayExpression expression)
             : new JintArrayExpression(expression);
     }
 
-    private void Initialize()
+    private void Initialize(EvaluationContext context)
     {
-        ref readonly var elements = ref ((ArrayExpression) _expression).Elements;
-        var expressions = _expressions = new JintExpression[((ArrayExpression) _expression).Elements.Count];
-        for (var n = 0; n < expressions.Length; n++)
-        {
-            var expr = elements[n];
-            if (expr != null)
-            {
-                var expression = Build(expr);
-                expressions[n] = expression;
-                _hasSpreads |= expr.Type == NodeType.SpreadElement;
-            }
-        }
+        _arguments.Initialize(context, ((ArrayExpression) _expression).Elements.AsSpan()!);
 
         // we get called from nested spread expansion in call
         _initialized = true;
@@ -43,95 +31,41 @@ protected override object EvaluateInternal(EvaluationContext context)
     {
         if (!_initialized)
         {
-            Initialize();
+            Initialize(context);
             _initialized = true;
         }
 
+        var expressions = ((ArrayExpression) _expression).Elements.AsSpan();
         var engine = context.Engine;
-        var a = engine.Realm.Intrinsics.Array.ArrayCreate(_hasSpreads ? 0 : (uint) _expressions.Length);
-
-        uint arrayIndexCounter = 0;
-        foreach (var expr in _expressions)
-        {
-            if (expr == null)
-            {
-                a.SetIndexValue(arrayIndexCounter++, null, updateLength: false);
-            }
-            else if (_hasSpreads && expr is JintSpreadExpression jse)
-            {
-                jse.GetValueAndCheckIterator(context, out var objectInstance, out var iterator);
-                // optimize for array
-                if (objectInstance is JsArray ai)
-                {
-                    var length = ai.GetLength();
-                    var newLength = arrayIndexCounter + length;
-                    a.EnsureCapacity(newLength);
-                    a.CopyValues(ai, sourceStartIndex: 0, targetStartIndex: arrayIndexCounter, length);
-                    arrayIndexCounter += length;
-                    a.SetLength(newLength);
-                }
-                else
-                {
-                    var protocol = new ArraySpreadProtocol(engine, a, iterator!, arrayIndexCounter);
-                    protocol.Execute();
-                    arrayIndexCounter += protocol._addedCount;
-                }
-            }
-            else
-            {
-                var value = expr.GetValue(context);
-                a.SetIndexValue(arrayIndexCounter++, value, updateLength: false);
-            }
-        }
-
-        if (_hasSpreads)
+        if (!_arguments.HasSpreads)
         {
-            a.SetLength(arrayIndexCounter);
+            var values = new JsValue[expressions.Length];
+            _arguments.BuildArguments(context, values);
+            return new JsArray(engine, values);
         }
 
-        return a;
-    }
-
-    private sealed class ArraySpreadProtocol : IteratorProtocol
-    {
-        private readonly JsArray _instance;
-        private long _index;
-        internal uint _addedCount;
-
-        public ArraySpreadProtocol(
-            Engine engine,
-            JsArray instance,
-            IteratorInstance iterator,
-            long startIndex) : base(engine, iterator, 0)
-        {
-            _instance = instance;
-            _index = startIndex - 1;
-        }
-
-        protected override void ProcessItem(JsValue[] arguments, JsValue currentValue)
-        {
-            _index++;
-            _addedCount++;
-            _instance.SetIndexValue((uint) _index, currentValue, updateLength: false);
-        }
+        var array = new List<JsValue>();
+        _arguments.BuildArgumentsWithSpreads(context, array);
+        return new JsArray(engine, array.ToArray());
     }
 
     internal sealed class JintEmptyArrayExpression : JintExpression
     {
-        public static JintEmptyArrayExpression Instance = new(new ArrayExpression(NodeList.From(Array.Empty<Expression?>())));
+        public static JintEmptyArrayExpression Instance =
+            new(new ArrayExpression(NodeList.From(Array.Empty<Expression?>())));
 
         private JintEmptyArrayExpression(Expression expression) : base(expression)
         {
         }
 
         protected override object EvaluateInternal(EvaluationContext context)
         {
-            return new JsArray(context.Engine, Array.Empty<JsValue>());
+            return new JsArray(context.Engine, []);
         }
 
         public override JsValue GetValue(EvaluationContext context)
         {
-            return new JsArray(context.Engine, Array.Empty<JsValue>());
+            return new JsArray(context.Engine, []);
         }
     }
 }