Cache more script-global information when preparing AST (#1671)

Jint/Collections/HybridDictionary.cs

@@ -51,7 +51,7 @@ namespace Jint.Collections
                 {
                     if (_list.Count >= CutoverPoint - 1)
                     {
-                        SwitchToDictionary(key, value);
+                        SwitchToDictionary(key, value, tryAdd: false);
                     }
                     else
                     {
@@ -97,7 +97,7 @@ namespace Jint.Collections
             }
         }
 
-        private void SwitchToDictionary(Key key, TValue value)
+        private bool SwitchToDictionary(Key key, TValue value, bool tryAdd)
         {
             var dictionary = new StringDictionarySlim<TValue>(InitialDictionarySize);
             foreach (var pair in _list)
@@ -105,9 +105,19 @@ namespace Jint.Collections
                 dictionary[pair.Key] = pair.Value;
             }
 
-            dictionary[key] = value;
+            bool result;
+            if (tryAdd)
+            {
+                result = dictionary.TryAdd(key, value);
+            }
+            else
+            {
+                dictionary[key] = value;
+                result = true;
+            }
             _dictionary = dictionary;
             _list = null;
+            return result;
         }
 
         public int Count
@@ -116,6 +126,27 @@ namespace Jint.Collections
             get => _dictionary?.Count ?? _list?.Count ?? 0;
         }
 
+        public bool TryAdd(Key key, TValue value)
+        {
+            if (_dictionary != null)
+            {
+                return _dictionary.TryAdd(key, value);
+            }
+            else
+            {
+                _list ??= new ListDictionary<TValue>(key, value, _checkExistingKeys);
+
+                if (_list.Count + 1 >= CutoverPoint)
+                {
+                    return SwitchToDictionary(key, value, tryAdd: true);
+                }
+                else
+                {
+                    return _list.Add(key, value, tryAdd: true);
+                }
+            }
+        }
+
         public void Add(Key key, TValue value)
         {
             if (_dictionary != null)
@@ -132,7 +163,7 @@ namespace Jint.Collections
                 {
                     if (_list.Count + 1 >= CutoverPoint)
                     {
-                        SwitchToDictionary(key, value);
+                        SwitchToDictionary(key, value, tryAdd: false);
                     }
                     else
                     {

Jint/Collections/ListDictionary.cs

@@ -99,7 +99,7 @@ namespace Jint.Collections
             get => _count;
         }
 
-        public void Add(Key key, TValue value)
+        public bool Add(Key key, TValue value, bool tryAdd = false)
         {
             DictionaryNode last = null;
             DictionaryNode node;
@@ -109,6 +109,10 @@ namespace Jint.Collections
                 var oldKey = node.Key;
                 if (checkExistingKeys && oldKey == key)
                 {
+                    if (tryAdd)
+                    {
+                        return false;
+                    }
                     ExceptionHelper.ThrowArgumentException();
                 }
 
@@ -116,6 +120,7 @@ namespace Jint.Collections
             }
 
             AddNode(key, value, last);
+            return true;
         }
 
         private void AddNode(Key key, TValue value, DictionaryNode last)

Jint/Collections/StringDictionarySlim.cs

@@ -171,6 +171,23 @@ namespace Jint.Collections
             return ref AddKey(key, bucketIndex);
         }
 
+        public bool TryAdd(Key key, TValue value)
+        {
+            Entry[] entries = _entries;
+            int bucketIndex = key.HashCode & (_buckets.Length - 1);
+            for (int i = _buckets[bucketIndex] - 1;
+                 (uint)i < (uint)entries.Length; i = entries[i].next)
+            {
+                if (key.Name == entries[i].key.Name)
+                {
+                    return false;
+                }
+            }
+
+            AddKey(key, bucketIndex) = value;
+            return true;
+        }
+
         /// <summary>
         /// Adds a new item and expects key to not to exist.
         /// </summary>

Jint/Engine.Ast.cs

@@ -8,7 +8,6 @@ namespace Jint;
 
 public partial class Engine
 {
-
     /// <summary>
     /// Prepares a script for the engine that includes static analysis data to speed up execution during run-time.
     /// </summary>
@@ -70,7 +69,102 @@ public partial class Engine
                     var function = (IFunction) node;
                     node.AssociatedData = JintFunctionDefinition.BuildState(function);
                     break;
+                case Nodes.Program:
+                    node.AssociatedData = new CachedHoistingScope((Program) node);
+                    break;
+            }
+        }
+    }
+}
+
+internal sealed class CachedHoistingScope
+{
+    public CachedHoistingScope(Program program)
+    {
+        Scope = HoistingScope.GetProgramLevelDeclarations(program);
+
+        VarNames = new List<string>();
+        GatherVarNames(Scope, VarNames);
+
+        LexNames = new List<CachedLexicalName>();
+        GatherLexNames(Scope, LexNames);
+    }
+
+    internal static void GatherVarNames(HoistingScope scope, List<string> boundNames)
+    {
+        var varDeclarations = scope._variablesDeclarations;
+        if (varDeclarations != null)
+        {
+            for (var i = 0; i < varDeclarations.Count; i++)
+            {
+                var d = varDeclarations[i];
+                d.GetBoundNames(boundNames);
+            }
+        }
+    }
+
+    internal static void GatherLexNames(HoistingScope scope, List<CachedLexicalName> boundNames)
+    {
+        var lexDeclarations = scope._lexicalDeclarations;
+        if (lexDeclarations != null)
+        {
+            var temp = new List<string>();
+            for (var i = 0; i < lexDeclarations.Count; i++)
+            {
+                var d = lexDeclarations[i];
+                temp.Clear();
+                d.GetBoundNames(temp);
+                foreach (var name in temp)
+                {
+                    boundNames.Add(new CachedLexicalName(name, d.IsConstantDeclaration()));
+                }
             }
         }
     }
+
+    internal readonly record struct CachedLexicalName(string Name, bool Constant);
+
+    public HoistingScope Scope { get; }
+    public List<string> VarNames { get; }
+    public List<CachedLexicalName> LexNames { get; }
+}
+
+internal static class AstPreparationExtensions
+{
+    internal static HoistingScope GetHoistingScope(this Program program)
+    {
+        return program.AssociatedData is CachedHoistingScope cached ? cached.Scope : HoistingScope.GetProgramLevelDeclarations(program);
+    }
+
+    internal static List<string> GetVarNames(this Program program, HoistingScope hoistingScope)
+    {
+        List<string> boundNames;
+        if (program.AssociatedData is CachedHoistingScope cached)
+        {
+            boundNames = cached.VarNames;
+        }
+        else
+        {
+            boundNames = new List<string>();
+            CachedHoistingScope.GatherVarNames(hoistingScope, boundNames);
+        }
+
+        return boundNames;
+    }
+
+    internal static List<CachedHoistingScope.CachedLexicalName> GetLexNames(this Program program, HoistingScope hoistingScope)
+    {
+        List<CachedHoistingScope.CachedLexicalName> boundNames;
+        if (program.AssociatedData is CachedHoistingScope cached)
+        {
+            boundNames = cached.LexNames;
+        }
+        else
+        {
+            boundNames = new List<CachedHoistingScope.CachedLexicalName>();
+            CachedHoistingScope.GatherLexNames(hoistingScope, boundNames);
+        }
+
+        return boundNames;
+    }
 }

Jint/Engine.cs

@@ -917,14 +917,12 @@ namespace Jint
             Script script,
             GlobalEnvironmentRecord env)
         {
-            var strict = _isStrict || StrictModeScope.IsStrictModeCode;
-            var hoistingScope = HoistingScope.GetProgramLevelDeclarations(strict, script);
+            var hoistingScope = script.GetHoistingScope();
             var functionDeclarations = hoistingScope._functionDeclarations;
-            var varDeclarations = hoistingScope._variablesDeclarations;
             var lexDeclarations = hoistingScope._lexicalDeclarations;
 
-            var functionToInitialize = new LinkedList<JintFunctionDefinition>();
-            var declaredFunctionNames = new HashSet<string>();
+            var functionToInitialize = new List<JintFunctionDefinition>();
+            var declaredFunctionNames = new HashSet<string>(StringComparer.Ordinal);
             var declaredVarNames = new List<string>();
 
             var realm = Realm;
@@ -944,74 +942,56 @@ namespace Jint
                         }
 
                         declaredFunctionNames.Add(fn);
-                        functionToInitialize.AddFirst(new JintFunctionDefinition(d));
+                        functionToInitialize.Add(new JintFunctionDefinition(d));
                     }
                 }
             }
 
-            var boundNames = new List<string>();
-            if (varDeclarations != null)
+            var varNames = script.GetVarNames(hoistingScope);
+            for (var j = 0; j < varNames.Count; j++)
             {
-                for (var i = 0; i < varDeclarations.Count; i++)
+                var vn = varNames[j];
+                if (env.HasLexicalDeclaration(vn))
                 {
-                    var d = varDeclarations[i];
-                    boundNames.Clear();
-                    d.GetBoundNames(boundNames);
-                    for (var j = 0; j < boundNames.Count; j++)
-                    {
-                        var vn = boundNames[j];
-
-                        if (env.HasLexicalDeclaration(vn))
-                        {
-                            ExceptionHelper.ThrowSyntaxError(realm, $"Identifier '{vn}' has already been declared");
-                        }
-
-                        if (!declaredFunctionNames.Contains(vn))
-                        {
-                            var vnDefinable = env.CanDeclareGlobalVar(vn);
-                            if (!vnDefinable)
-                            {
-                                ExceptionHelper.ThrowTypeError(realm);
-                            }
+                    ExceptionHelper.ThrowSyntaxError(realm, $"Identifier '{vn}' has already been declared");
+                }
 
-                            declaredVarNames.Add(vn);
-                        }
+                if (!declaredFunctionNames.Contains(vn))
+                {
+                    var vnDefinable = env.CanDeclareGlobalVar(vn);
+                    if (!vnDefinable)
+                    {
+                        ExceptionHelper.ThrowTypeError(realm);
                     }
+
+                    declaredVarNames.Add(vn);
                 }
             }
 
             PrivateEnvironmentRecord? privateEnv = null;
-            if (lexDeclarations != null)
+            var lexNames = script.GetLexNames(hoistingScope);
+            for (var i = 0; i < lexNames.Count; i++)
             {
-                for (var i = 0; i < lexDeclarations.Count; i++)
+                var (dn, constant) = lexNames[i];
+                if (env.HasVarDeclaration(dn) || env.HasLexicalDeclaration(dn) || env.HasRestrictedGlobalProperty(dn))
                 {
-                    var d = lexDeclarations[i];
-                    boundNames.Clear();
-                    d.GetBoundNames(boundNames);
-                    for (var j = 0; j < boundNames.Count; j++)
-                    {
-                        var dn = boundNames[j];
-                        if (env.HasVarDeclaration(dn)
-                            || env.HasLexicalDeclaration(dn)
-                            || env.HasRestrictedGlobalProperty(dn))
-                        {
-                            ExceptionHelper.ThrowSyntaxError(realm, $"Identifier '{dn}' has already been declared");
-                        }
+                    ExceptionHelper.ThrowSyntaxError(realm, $"Identifier '{dn}' has already been declared");
+                }
 
-                        if (d.IsConstantDeclaration())
-                        {
-                            env.CreateImmutableBinding(dn, strict: true);
-                        }
-                        else
-                        {
-                            env.CreateMutableBinding(dn, canBeDeleted: false);
-                        }
-                    }
+                if (constant)
+                {
+                    env.CreateImmutableBinding(dn, strict: true);
+                }
+                else
+                {
+                    env.CreateMutableBinding(dn, canBeDeleted: false);
                 }
             }
 
-            foreach (var f in functionToInitialize)
+            // we need to go trough in reverse order to handle the hoisting correctly
+            for (var i = functionToInitialize.Count - 1; i > -1; i--)
             {
+                var f = functionToInitialize[i];
                 var fn = f.Name!;
 
                 if (env.HasLexicalDeclaration(fn))
@@ -1023,11 +1003,7 @@ namespace Jint
                 env.CreateGlobalFunctionBinding(fn, fo, canBeDeleted: false);
             }
 
-            for (var i = 0; i < declaredVarNames.Count; i++)
-            {
-                var vn = declaredVarNames[i];
-                env.CreateGlobalVarBinding(vn, canBeDeleted: false);
-            }
+            env.CreateGlobalVarBindings(declaredVarNames, canBeDeleted: false);
         }
 
         /// <summary>
@@ -1199,7 +1175,7 @@ namespace Jint
             PrivateEnvironmentRecord? privateEnv,
             bool strict)
         {
-            var hoistingScope = HoistingScope.GetProgramLevelDeclarations(strict, script);
+            var hoistingScope = HoistingScope.GetProgramLevelDeclarations(script);
 
             var lexEnvRec = (DeclarativeEnvironmentRecord) lexEnv;
             var varEnvRec = varEnv;
@@ -1261,7 +1237,7 @@ namespace Jint
 
             var functionDeclarations = hoistingScope._functionDeclarations;
             var functionsToInitialize = new LinkedList<JintFunctionDefinition>();
-            var declaredFunctionNames = new HashSet<string>();
+            var declaredFunctionNames = new HashSet<string>(StringComparer.Ordinal);
 
             if (functionDeclarations != null)
             {

Jint/HoistingScope.cs

@@ -28,12 +28,11 @@ namespace Jint
         }
 
         public static HoistingScope GetProgramLevelDeclarations(
-            bool strict,
             Program script,
             bool collectVarNames = false,
             bool collectLexicalNames = false)
         {
-            var treeWalker = new ScriptWalker(strict, collectVarNames, collectLexicalNames);
+            var treeWalker = new ScriptWalker(collectVarNames, collectLexicalNames);
             treeWalker.Visit(script, null);
 
             return new HoistingScope(
@@ -46,7 +45,7 @@ namespace Jint
 
         public static HoistingScope GetFunctionLevelDeclarations(bool strict, IFunction node)
         {
-            var treeWalker = new ScriptWalker(strict, collectVarNames: true, collectLexicalNames: true);
+            var treeWalker = new ScriptWalker(collectVarNames: true, collectLexicalNames: true);
             treeWalker.Visit(node.Body, null);
 
             return new HoistingScope(
@@ -63,7 +62,7 @@ namespace Jint
             bool collectLexicalNames = false)
         {
             // modules area always strict
-            var treeWalker = new ScriptWalker(strict: true, collectVarNames, collectLexicalNames);
+            var treeWalker = new ScriptWalker(collectVarNames, collectLexicalNames);
             treeWalker.Visit(module, null);
             return new HoistingScope(
                 treeWalker._functions,
@@ -204,7 +203,6 @@ namespace Jint
         {
             internal List<FunctionDeclaration>? _functions;
 
-            private readonly bool _strict;
             private readonly bool _collectVarNames;
             internal List<VariableDeclaration>? _variableDeclarations;
             internal List<Key>? _varNames;
@@ -213,9 +211,8 @@ namespace Jint
             internal List<Declaration>? _lexicalDeclarations;
             internal List<string>? _lexicalNames;
 
-            public ScriptWalker(bool strict, bool collectVarNames, bool collectLexicalNames)
+            public ScriptWalker(bool collectVarNames, bool collectLexicalNames)
             {
-                _strict = strict;
                 _collectVarNames = collectVarNames;
                 _collectLexicalNames = collectLexicalNames;
             }

Jint/Runtime/Environments/EnvironmentRecord.cs

@@ -118,7 +118,6 @@ namespace Jint.Runtime.Environments
         {
             public readonly Key Key;
             public readonly JsString Value;
-            public readonly bool HasEvalOrArguments;
             public readonly JsValue? CalculatedValue;
 
             public BindingName(string value)
@@ -126,7 +125,6 @@ namespace Jint.Runtime.Environments
                 var key = (Key) value;
                 Key = key;
                 Value = JsString.Create(value);
-                HasEvalOrArguments = key == KnownKeys.Eval || key == KnownKeys.Arguments;
                 if (key == KnownKeys.Undefined)
                 {
                     CalculatedValue = Undefined;

Jint/Runtime/Environments/GlobalEnvironmentRecord.cs

@@ -12,20 +12,30 @@ namespace Jint.Runtime.Environments
     /// </summary>
     public sealed class GlobalEnvironmentRecord : EnvironmentRecord
     {
+        /// <summary>
+        /// A sealed class for global usage.
+        /// </summary>
+        internal sealed class GlobalDeclarativeEnvironmentRecord : DeclarativeEnvironmentRecord
+        {
+            public GlobalDeclarativeEnvironmentRecord(Engine engine) : base(engine)
+            {
+            }
+        }
+
         internal readonly ObjectInstance _global;
 
         // we expect it to be GlobalObject, but need to allow to something host-defined, like Window
         private readonly GlobalObject? _globalObject;
 
         // Environment records are needed by debugger
-        internal readonly DeclarativeEnvironmentRecord _declarativeRecord;
-        private readonly HashSet<string> _varNames = new();
+        internal readonly GlobalDeclarativeEnvironmentRecord _declarativeRecord;
+        private readonly HashSet<string> _varNames = new(StringComparer.Ordinal);
 
         public GlobalEnvironmentRecord(Engine engine, ObjectInstance global) : base(engine)
         {
             _global = global;
             _globalObject = global as GlobalObject;
-            _declarativeRecord = new DeclarativeEnvironmentRecord(engine);
+            _declarativeRecord = new GlobalDeclarativeEnvironmentRecord(engine);
         }
 
         public ObjectInstance GlobalThisValue => _global;
@@ -66,7 +76,7 @@ namespace Jint.Runtime.Environments
             out Binding binding,
             [NotNullWhen(true)] out JsValue? value)
         {
-            if (_declarativeRecord.TryGetBinding(name, strict, out binding, out value))
+            if (_declarativeRecord._dictionary is not null && _declarativeRecord.TryGetBinding(name, strict, out binding, out value))
             {
                 return true;
             }
@@ -343,14 +353,31 @@ namespace Jint.Runtime.Environments
         public void CreateGlobalVarBinding(string name, bool canBeDeleted)
         {
             Key key = name;
-            if (!_global._properties!.ContainsKey(key) && _global.Extensible)
-            {
-                _global._properties[key] = new PropertyDescriptor(Undefined, canBeDeleted
+            if (_global.Extensible && _global._properties!.TryAdd(key, new PropertyDescriptor(Undefined, canBeDeleted
                     ? PropertyFlag.ConfigurableEnumerableWritable | PropertyFlag.MutableBinding
-                    : PropertyFlag.NonConfigurable | PropertyFlag.MutableBinding);
+                    : PropertyFlag.NonConfigurable | PropertyFlag.MutableBinding)))
+            {
+                _varNames.Add(name);
+            }
+        }
+
+        internal void CreateGlobalVarBindings(List<string> names, bool canBeDeleted)
+        {
+            if (!_global.Extensible)
+            {
+                return;
             }
 
-            _varNames.Add(name);
+            for (var i = 0; i < names.Count; i++)
+            {
+                var name = names[i];
+
+                _global._properties!.TryAdd(name,new PropertyDescriptor(Undefined, canBeDeleted
+                    ? PropertyFlag.ConfigurableEnumerableWritable | PropertyFlag.MutableBinding
+                    : PropertyFlag.NonConfigurable | PropertyFlag.MutableBinding));
+
+                _varNames.Add(name);
+            }
         }
 
         /// <summary>

Jint/Runtime/Environments/JintEnvironment.cs

@@ -44,7 +44,7 @@ namespace Jint.Runtime.Environments
 
             if (env._outerEnv is null)
             {
-                return env.TryGetBinding(name, strict, out _, out value);
+                return ((GlobalEnvironmentRecord) env).TryGetBinding(name, strict, out _, out value);
             }
 
             while (!ReferenceEquals(record, null))

Jint/Runtime/Interpreter/Expressions/JintIdentifierExpression.cs

@@ -9,28 +9,52 @@ namespace Jint.Runtime.Interpreter.Expressions;
 
 internal sealed class JintIdentifierExpression : JintExpression
 {
+    private EnvironmentRecord.BindingName _identifier = null!;
+
     public JintIdentifierExpression(Identifier expression) : base(expression)
     {
+        _initialized = false;
+    }
+
+    public EnvironmentRecord.BindingName Identifier
+    {
+        get
+        {
+            EnsureIdentifier();
+            return _identifier;
+        }
+    }
+
+    protected override void Initialize(EvaluationContext context)
+    {
+        EnsureIdentifier();
     }
 
-    internal EnvironmentRecord.BindingName Identifier
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
+    private void EnsureIdentifier()
     {
-        get => (EnvironmentRecord.BindingName) (_expression.AssociatedData ??= new EnvironmentRecord.BindingName(((Identifier) _expression).Name));
+        _identifier ??= _expression.AssociatedData as EnvironmentRecord.BindingName ?? new EnvironmentRecord.BindingName(((Identifier) _expression).Name);
     }
 
-    public bool HasEvalOrArguments => Identifier.HasEvalOrArguments;
+    public bool HasEvalOrArguments
+    {
+        get
+        {
+            var name = ((Identifier) _expression).Name;
+            return name is "eval" or "arguments";
+        }
+    }
 
     protected override object EvaluateInternal(EvaluationContext context)
     {
         var engine = context.Engine;
         var env = engine.ExecutionContext.LexicalEnvironment;
         var strict = StrictModeScope.IsStrictModeCode;
-        var identifier = Identifier;
-        var identifierEnvironment = JintEnvironment.TryGetIdentifierEnvironmentWithBinding(env, identifier, out var temp)
+        var identifierEnvironment = JintEnvironment.TryGetIdentifierEnvironmentWithBinding(env, _identifier, out var temp)
             ? temp
             : JsValue.Undefined;
 
-        return engine._referencePool.Rent(identifierEnvironment, identifier.Value, strict, thisValue: null);
+        return engine._referencePool.Rent(identifierEnvironment, _identifier.Value, strict, thisValue: null);
     }
 
     public override JsValue GetValue(EvaluationContext context)
@@ -79,6 +103,6 @@ internal sealed class JintIdentifierExpression : JintExpression
     [MethodImpl(MethodImplOptions.NoInlining)]
     private void ThrowNotInitialized(Engine engine)
     {
-        ExceptionHelper.ThrowReferenceError(engine.Realm, Identifier.Key.Name + " has not been initialized");
+        ExceptionHelper.ThrowReferenceError(engine.Realm, _identifier.Key.Name + " has not been initialized");
     }
 }

Jint/Runtime/Interpreter/Statements/JintExpressionStatement.cs

@@ -7,6 +7,9 @@ internal sealed class JintExpressionStatement : JintStatement<ExpressionStatemen
 {
     private JintExpression _expression = null!;
 
+    // identifiers are queried the most
+    private JintIdentifierExpression? _identifierExpression;
+
     public JintExpressionStatement(ExpressionStatement statement) : base(statement)
     {
     }
@@ -14,10 +17,15 @@ internal sealed class JintExpressionStatement : JintStatement<ExpressionStatemen
     protected override void Initialize(EvaluationContext context)
     {
         _expression = JintExpression.Build(_statement.Expression);
+        _identifierExpression = _expression as JintIdentifierExpression;
     }
 
     protected override Completion ExecuteInternal(EvaluationContext context)
     {
-        return new Completion(context.Completion, _expression.GetValue(context), _statement);
+        var value = _identifierExpression is not null
+            ? _identifierExpression.GetValue(context)
+            : _expression.GetValue(context);
+
+        return new Completion(context.Completion, value, _statement);
     }
 }