More tweaks to reduce call stack size (#1277)

Jint.Tests/Runtime/EngineLimitTests.cs

@@ -10,9 +10,9 @@ public class EngineLimitTests
     public void ShouldAllowReasonableCallStackDepth()
     {
 #if RELEASE
-        const int FunctionNestingCount = 690;
+        const int FunctionNestingCount = 740;
 #else
-        const int FunctionNestingCount = 350;
+        const int FunctionNestingCount = 400;
 #endif
 
         // generate call tree

Jint/Engine.cs

@@ -25,7 +25,7 @@ namespace Jint
     {
         private readonly JavaScriptParser _defaultParser = new(ParserOptions.Default);
 
-        private readonly ExecutionContextStack _executionContexts;
+        internal readonly ExecutionContextStack _executionContexts;
         private JsValue _completionValue = JsValue.Undefined;
         internal EvaluationContext? _activeEvaluationContext;
 
@@ -38,7 +38,7 @@ namespace Jint
 
         // cached access
         internal readonly IObjectConverter[]? _objectConverters;
-        private readonly IConstraint[] _constraints;
+        internal readonly IConstraint[] _constraints;
         internal readonly bool _isDebugMode;
         internal bool _isStrict;
         internal readonly IReferenceResolver _referenceResolver;
@@ -108,13 +108,14 @@ namespace Jint
 
             _constraints = Options.Constraints.Constraints.ToArray();
             _referenceResolver = Options.ReferenceResolver;
-            CallStack = new JintCallStack(Options.Constraints.MaxRecursionDepth >= 0);
 
             _referencePool = new ReferencePool();
             _argumentsInstancePool = new ArgumentsInstancePool(this);
             _jsValueArrayPool = new JsValueArrayPool();
 
             Options.Apply(this);
+
+            CallStack = new JintCallStack(Options.Constraints.MaxRecursionDepth >= 0);
         }
 
         private void Reset()

Jint/Native/Function/FunctionInstance.cs

@@ -319,14 +319,6 @@ namespace Jint.Native.Function
             localEnv.BindThisValue(thisValue);
         }
 
-        /// <summary>
-        /// https://tc39.es/ecma262/#sec-ordinarycallevaluatebody
-        /// </summary>
-        internal Completion OrdinaryCallEvaluateBody(EvaluationContext context, JsValue[] arguments)
-        {
-            return _functionDefinition.EvaluateBody(context, this, arguments);
-        }
-
         /// <summary>
         /// https://tc39.es/ecma262/#sec-prepareforordinarycall
         /// </summary>

Jint/Native/Function/ScriptFunctionInstance.cs

@@ -72,7 +72,8 @@ namespace Jint.Native.Function
 
                     // actual call
                     var context = _engine._activeEvaluationContext ?? new EvaluationContext(_engine);
-                    var result = OrdinaryCallEvaluateBody(context, arguments);
+
+                    var result = _functionDefinition.EvaluateBody(context, this, arguments);
 
                     if (result.Type == CompletionType.Throw)
                     {
@@ -143,7 +144,8 @@ namespace Jint.Native.Function
                 try
                 {
                     var context = _engine._activeEvaluationContext ?? new EvaluationContext(_engine);
-                    var result = OrdinaryCallEvaluateBody(context, arguments);
+
+                    var result = _functionDefinition.EvaluateBody(context, this, arguments);
 
                     // The DebugHandler needs the current execution context before the return for stepping through the return point
                     if (context.DebugMode && result.Type != CompletionType.Throw)

Jint/Runtime/Interpreter/EvaluationContext.cs

@@ -7,19 +7,29 @@ namespace Jint.Runtime.Interpreter
     /// </summary>
     internal sealed class EvaluationContext
     {
+        private readonly bool _shouldRunBeforeExecuteStatementChecks;
+
         public EvaluationContext(Engine engine, in Completion? resumedCompletion = null)
         {
             Engine = engine;
-            DebugMode = engine._isDebugMode;
             ResumedCompletion = resumedCompletion ?? default; // TODO later
             OperatorOverloadingAllowed = engine.Options.Interop.AllowOperatorOverloading;
+            _shouldRunBeforeExecuteStatementChecks = engine._constraints.Length > 0 || engine._isDebugMode;
         }
 
         public Engine Engine { get; }
         public Completion ResumedCompletion { get; }
-        public bool DebugMode { get; }
+        public bool DebugMode => Engine._isDebugMode;
 
         public SyntaxElement LastSyntaxElement { get; set; } = null!;
         public bool OperatorOverloadingAllowed { get; }
+
+        public void RunBeforeExecuteStatementChecks(Statement statement)
+        {
+            if (_shouldRunBeforeExecuteStatementChecks)
+            {
+                Engine.RunBeforeExecuteStatementChecks(statement);
+            }
+        }
     }
 }

Jint/Runtime/Interpreter/Expressions/JintCallExpression.cs

@@ -25,11 +25,14 @@ namespace Jint.Runtime.Interpreter.Expressions
         protected override void Initialize(EvaluationContext context)
         {
             var engine = context.Engine;
+
             var expression = (CallExpression) _expression;
+            ref readonly var expressionArguments = ref expression.Arguments;
+
             _calleeExpression = Build(engine, expression.Callee);
             var cachedArgumentsHolder = new CachedArgumentsHolder
             {
-                JintArguments = new JintExpression[expression.Arguments.Count]
+                JintArguments = new JintExpression[expressionArguments.Count]
             };
 
             static bool CanSpread(Node? e)
@@ -37,18 +40,19 @@ namespace Jint.Runtime.Interpreter.Expressions
                 return e?.Type == Nodes.SpreadElement || e is AssignmentExpression { Right.Type: Nodes.SpreadElement };
             }
 
-            bool cacheable = true;
-            for (var i = 0; i < expression.Arguments.Count; i++)
+            var cacheable = true;
+            for (var i = 0; i < expressionArguments.Count; i++)
             {
-                var expressionArgument = expression.Arguments[i];
+                var expressionArgument = expressionArguments[i];
                 cachedArgumentsHolder.JintArguments[i] = Build(engine, expressionArgument);
                 cacheable &= expressionArgument.Type == Nodes.Literal;
                 _hasSpreads |= CanSpread(expressionArgument);
                 if (expressionArgument is ArrayExpression ae)
                 {
-                    for (var elementIndex = 0; elementIndex < ae.Elements.Count; elementIndex++)
+                    ref readonly var elements = ref ae.Elements;
+                    for (var elementIndex = 0; elementIndex < elements.Count; elementIndex++)
                     {
-                        _hasSpreads |= CanSpread(ae.Elements[elementIndex]);
+                        _hasSpreads |= CanSpread(elements[elementIndex]);
                     }
                 }
             }
@@ -94,10 +98,10 @@ namespace Jint.Runtime.Interpreter.Expressions
             }
 
             var referenceRecord = reference as Reference;
-            if (referenceRecord != null
+            if (ReferenceEquals(func, engine.Realm.Intrinsics.Eval)
+                && referenceRecord != null
                 && !referenceRecord.IsPropertyReference()
-                && referenceRecord.GetReferencedName() == CommonProperties.Eval
-                && ReferenceEquals(func, engine.Realm.Intrinsics.Eval))
+                && referenceRecord.GetReferencedName() == CommonProperties.Eval)
             {
                 return HandleEval(context, func, engine, referenceRecord);
             }
@@ -140,7 +144,7 @@ namespace Jint.Runtime.Interpreter.Expressions
 
             if (!func.IsObject() && !engine._referenceResolver.TryGetCallable(engine, reference, out func))
             {
-                ThrowMemberisNotFunction(referenceRecord, reference, engine);
+                ThrowMemberIsNotFunction(referenceRecord, reference, engine);
             }
 
             var callable = func as ICallable;
@@ -206,7 +210,7 @@ namespace Jint.Runtime.Interpreter.Expressions
 
         [DoesNotReturn]
         [MethodImpl(MethodImplOptions.NoInlining)]
-        private static void ThrowMemberisNotFunction(Reference? referenceRecord1, object reference, Engine engine)
+        private static void ThrowMemberIsNotFunction(Reference? referenceRecord1, object reference, Engine engine)
         {
             var message = referenceRecord1 == null
                 ? reference + " is not a function"
@@ -244,7 +248,7 @@ namespace Jint.Runtime.Interpreter.Expressions
             }
 
             var argList = ArgumentListEvaluation(context);
-            var result = ((IConstructor) func).Construct(argList, newTarget ?? JsValue.Undefined);
+            var result = ((IConstructor) func).Construct(argList, newTarget);
             var thisER = (FunctionEnvironmentRecord) engine.ExecutionContext.GetThisEnvironment();
             return thisER.BindThisValue(result);
         }

Jint/Runtime/Interpreter/Expressions/JintExpression.cs

@@ -469,19 +469,17 @@ namespace Jint.Runtime.Interpreter.Expressions
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         protected static void BuildArguments(EvaluationContext context, JintExpression[] jintExpressions, JsValue[] targetArray)
         {
-            for (var i = 0; i < jintExpressions.Length; i++)
+            for (uint i = 0; i < (uint) jintExpressions.Length; i++)
             {
-                var completion = jintExpressions[i].GetValue(context);
-                targetArray[i] = completion.Value.Clone();
+                targetArray[i] = jintExpressions[i].GetValue(context).Value.Clone();
             }
         }
 
         protected static JsValue[] BuildArgumentsWithSpreads(EvaluationContext context, JintExpression[] jintExpressions)
         {
             var args = new List<JsValue>(jintExpressions.Length);
-            for (var i = 0; i < jintExpressions.Length; i++)
+            foreach (var jintExpression in jintExpressions)
             {
-                var jintExpression = jintExpressions[i];
                 if (jintExpression is JintSpreadExpression jse)
                 {
                     jse.GetValueAndCheckIterator(context, out var objectInstance, out var iterator);

Jint/Runtime/Interpreter/JintFunctionDefinition.cs

@@ -35,28 +35,36 @@ namespace Jint.Runtime.Interpreter
         public FunctionThisMode ThisMode => Strict ? FunctionThisMode.Strict : FunctionThisMode.Global;
 
         /// <summary>
-        /// https://tc39.es/ecma262/#sec-runtime-semantics-evaluatebody
+        /// https://tc39.es/ecma262/#sec-ordinarycallevaluatebody
         /// </summary>
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         internal Completion EvaluateBody(EvaluationContext context, FunctionInstance functionObject, JsValue[] argumentsList)
         {
             Completion result;
+            var argumentsInstance = _engine.FunctionDeclarationInstantiation(functionObject, argumentsList);
             if (Function.Expression)
             {
-                result = EvaluateConciseBody(context, functionObject, argumentsList);
+                // https://tc39.es/ecma262/#sec-runtime-semantics-evaluateconcisebody
+                _bodyExpression ??= JintExpression.Build(_engine, (Expression) Function.Body);
+                var jsValue = _bodyExpression.GetValue(context).GetValueOrDefault().Clone();
+                result = new Completion(CompletionType.Return, jsValue, null, Function.Body);
             }
             else if (Function.Generator)
             {
-                result = EvaluateFunctionBody(context, functionObject, argumentsList);
                 // TODO generators
                 // result = EvaluateGeneratorBody(functionObject, argumentsList);
+                _bodyStatementList ??= new JintStatementList(Function);
+                result = _bodyStatementList.Execute(context);
             }
             else
             {
-                result = EvaluateFunctionBody(context, functionObject, argumentsList);
+                // https://tc39.es/ecma262/#sec-runtime-semantics-evaluatefunctionbody
+                _bodyStatementList ??= new JintStatementList(Function);
+                result = _bodyStatementList.Execute(context);
             }
 
-            return new Completion(result.Type, result.GetValueOrDefault().Clone(), result.Target, result._source);
+            argumentsInstance?.FunctionWasCalled();
+            return result;
         }
 
         /// <summary>
@@ -68,31 +76,6 @@ namespace Jint.Runtime.Interpreter
             return default;
         }
 
-        /// <summary>
-        /// https://tc39.es/ecma262/#sec-runtime-semantics-evaluateconcisebody
-        /// </summary>
-        private Completion EvaluateConciseBody(EvaluationContext context, FunctionInstance functionObject, JsValue[] argumentsList)
-        {
-            var argumentsInstance = _engine.FunctionDeclarationInstantiation(functionObject, argumentsList);
-            _bodyExpression ??= JintExpression.Build(_engine, (Expression) Function.Body);
-            var jsValue = _bodyExpression?.GetValue(context).Value ?? Undefined.Instance;
-            argumentsInstance?.FunctionWasCalled();
-            return new Completion(CompletionType.Return, jsValue, null, Function.Body);
-        }
-
-        /// <summary>
-        /// https://tc39.es/ecma262/#sec-runtime-semantics-evaluatefunctionbody
-        /// </summary>
-        private Completion EvaluateFunctionBody(EvaluationContext context, FunctionInstance functionObject, JsValue[] argumentsList)
-        {
-            var argumentsInstance = _engine.FunctionDeclarationInstantiation(functionObject, argumentsList);
-            _bodyStatementList ??= new JintStatementList(Function);
-            var completion = _bodyStatementList.Execute(context);
-            argumentsInstance?.FunctionWasCalled();
-
-            return completion;
-        }
-
         internal State Initialize(FunctionInstance functionInstance)
         {
             return _state ??= DoInitialize(functionInstance);

Jint/Runtime/Interpreter/JintStatementList.cs

@@ -71,15 +71,14 @@ namespace Jint.Runtime.Interpreter
                 _initialized = true;
             }
 
-            var engine = context.Engine;
-            if (_statement != null)
+            if (_statement is not null)
             {
                 context.LastSyntaxElement = _statement;
-                engine.RunBeforeExecuteStatementChecks(_statement);
+                context.RunBeforeExecuteStatementChecks(_statement);
             }
 
             JintStatement? s = null;
-            var c = new Completion(CompletionType.Normal, null!, null, context.LastSyntaxElement);
+            Completion c = default;
             Completion sl = c;
 
             // The value of a StatementList is the value of the last value-producing item in the StatementList
@@ -89,33 +88,57 @@ namespace Jint.Runtime.Interpreter
                 foreach (var pair in _jintStatements!)
                 {
                     s = pair.Statement;
-                    c = pair.Value ?? s.Execute(context);
+                    c = pair.Value.GetValueOrDefault();
+                    if (c.Value is null)
+                    {
+                        c = s.Execute(context);
+                    }
 
                     if (c.Type != CompletionType.Normal)
                     {
                         return new Completion(
                             c.Type,
-                            c.Value ?? sl.Value,
+                            c.Value ?? sl.Value!,
                             c.Target,
                             c._source);
                     }
                     sl = c;
-                    lastValue = c.Value ?? lastValue;
+                    if (c.Value is not null)
+                    {
+                        lastValue = c.Value;
+                    }
+                }
+            }
+            catch (Exception ex)
+            {
+                if (ex is JintException)
+                {
+                    return HandleException(context, ex, s);
                 }
+
+                throw;
             }
-            catch (JavaScriptException v)
+
+            return new Completion(c.Type, lastValue ?? JsValue.Undefined, c.Target, c._source!);
+        }
+
+        private static Completion HandleException(EvaluationContext context, Exception exception, JintStatement? s)
+        {
+            if (exception is JavaScriptException javaScriptException)
             {
-                return CreateThrowCompletion(s, v);
+                return CreateThrowCompletion(s, javaScriptException);
             }
-            catch (TypeErrorException e)
+            if (exception is TypeErrorException typeErrorException)
             {
-                return CreateThrowCompletion(engine.Realm.Intrinsics.TypeError, e, s!);
+                return CreateThrowCompletion(context.Engine.Realm.Intrinsics.TypeError, typeErrorException, s!);
             }
-            catch (RangeErrorException e)
+            if (exception is RangeErrorException rangeErrorException)
             {
-                return CreateThrowCompletion(engine.Realm.Intrinsics.RangeError, e, s!);
+                return CreateThrowCompletion(context.Engine.Realm.Intrinsics.RangeError, rangeErrorException, s!);
             }
-            return new Completion(c.Type, lastValue ?? JsValue.Undefined, c.Target, c._source);
+
+            // should not happen unless there's problem in the engine
+            throw exception;
         }
 
         private static Completion CreateThrowCompletion(ErrorConstructor errorConstructor, Exception e, JintStatement s)

Jint/Runtime/Interpreter/Statements/JintExpressionStatement.cs

@@ -1,32 +1,23 @@
 using Esprima.Ast;
 using Jint.Runtime.Interpreter.Expressions;
-using Jint.Runtime.References;
 
-namespace Jint.Runtime.Interpreter.Statements
-{
-    internal sealed class JintExpressionStatement : JintStatement<ExpressionStatement>
-    {
-        private JintExpression _expression = null!;
-
-        public JintExpressionStatement(ExpressionStatement statement) : base(statement)
-        {
-        }
+namespace Jint.Runtime.Interpreter.Statements;
 
-        protected override void Initialize(EvaluationContext context)
-        {
-            _expression = JintExpression.Build(context.Engine, _statement.Expression);
-        }
+internal sealed class JintExpressionStatement : JintStatement<ExpressionStatement>
+{
+    private JintExpression _expression = null!;
 
-        protected override Completion ExecuteInternal(EvaluationContext context)
-        {
-            var result = _expression.Evaluate(context);
+    public JintExpressionStatement(ExpressionStatement statement) : base(statement)
+    {
+    }
 
-            if (result.Type != ExpressionCompletionType.Reference)
-            {
-                return new Completion(result);
-            }
+    protected override void Initialize(EvaluationContext context)
+    {
+        _expression = JintExpression.Build(context.Engine, _statement.Expression);
+    }
 
-            return new Completion(CompletionType.Normal, context.Engine.GetValue((Reference) result.Value, true), null, _statement);
-        }
+    protected override Completion ExecuteInternal(EvaluationContext context)
+    {
+        return _expression.GetValue(context);
     }
 }

Jint/Runtime/Interpreter/Statements/JintStatement.cs

@@ -33,7 +33,7 @@ namespace Jint.Runtime.Interpreter.Statements
             if (_statement.Type != Nodes.BlockStatement)
             {
                 context.LastSyntaxElement = _statement;
-                context.Engine.RunBeforeExecuteStatementChecks(_statement);
+                context.RunBeforeExecuteStatementChecks(_statement);
             }
 
             if (!_initialized)