Backport improve environment handling performance and cleanup API (#1846

)

Jint/Runtime/Debugger/DebugScope.cs

@@ -57,7 +57,7 @@ internal DebugScope(DebugScopeType type, Environment record, bool isTopLevel)
         /// <returns>Value of the binding</returns>
         public JsValue? GetBindingValue(string name)
         {
-            _record.TryGetBinding(new Environment.BindingName(name), strict: true, out _, out var result);
+            _record.TryGetBinding(new Environment.BindingName(name), out var result);
             return result;
         }
 

Jint/Runtime/Descriptors/Specialized/ClrAccessDescriptor.cs

@@ -31,14 +31,14 @@ public ClrAccessDescriptor(
 
         private JsValue DoGet(JsValue n)
         {
-            return _env.TryGetBinding(_name, false, out var binding, out _)
-                ? binding.Value
+            return _env.TryGetBinding(_name, out var value)
+                ? value
                 : JsValue.Undefined;
         }
 
         private void DoSet(JsValue n, JsValue o)
         {
-            _env.SetMutableBinding(_name.Key, o, true);
+            _env.SetMutableBinding(_name.Key, o, strict: true);
         }
     }
 }

Jint/Runtime/Environments/DeclarativeEnvironment.cs

@@ -23,16 +23,16 @@ public DeclarativeEnvironment(Engine engine, bool catchEnvironment = false) : ba
 
         internal sealed override bool HasBinding(Key name) => _dictionary is not null && _dictionary.ContainsKey(name);
 
-        internal override bool TryGetBinding(
-            BindingName name,
-            bool strict,
-            out Binding binding,
-            [NotNullWhen(true)] out JsValue? value)
+        internal override bool TryGetBinding(BindingName name, [NotNullWhen(true)] out JsValue? value)
         {
-            binding = default;
-            var success = _dictionary is not null &&_dictionary.TryGetValue(name.Key, out binding);
-            value = success && binding.IsInitialized() ? binding.Value : default;
-            return success;
+            if (_dictionary?.TryGetValue(name.Key, out var binding) == true)
+            {
+                value = binding.Value;
+                return true;
+            }
+
+            value = null;
+            return false;
         }
 
         internal void CreateMutableBindingAndInitialize(Key name, bool canBeDeleted, JsValue value)
@@ -75,6 +75,7 @@ internal sealed override void SetMutableBinding(Key name, JsValue value, bool st
                 {
                     ExceptionHelper.ThrowReferenceNameError(_engine.Realm, name);
                 }
+
                 CreateMutableBindingAndInitialize(name, canBeDeleted: true, value);
                 return;
             }
@@ -169,5 +170,17 @@ public void Clear()
         {
             _dictionary = null;
         }
+
+        internal void TransferTo(List<Key> names, DeclarativeEnvironment env)
+        {
+            var source = _dictionary!;
+            var target = env._dictionary ??= new HybridDictionary<Binding>(names.Count, checkExistingKeys: true);
+            for (var j = 0; j < names.Count; j++)
+            {
+                var bn = names[j];
+                source.TryGetValue(bn, out var lastValue);
+                target[bn] = new Binding(lastValue.Value, canBeDeleted: false, mutable: true, strict: false);
+            }
+        }
     }
 }

Jint/Runtime/Environments/Environment.cs

@@ -28,11 +28,7 @@ protected Environment(Engine engine) : base(InternalTypes.ObjectEnvironmentRecor
 
         internal abstract bool HasBinding(BindingName name);
 
-        internal abstract bool TryGetBinding(
-            BindingName name,
-            bool strict,
-            out Binding binding,
-            [NotNullWhen(true)] out JsValue? value);
+        internal abstract bool TryGetBinding(BindingName name, [NotNullWhen(true)] out JsValue? value);
 
         /// <summary>
         /// Creates a new mutable binding in an environment record.

Jint/Runtime/Environments/GlobalEnvironment.cs

@@ -69,19 +69,14 @@ internal override bool HasBinding(BindingName name)
             return _global.HasProperty(name.Value);
         }
 
-        internal override bool TryGetBinding(
-            BindingName name,
-            bool strict,
-            out Binding binding,
-            [NotNullWhen(true)] out JsValue? value)
+        internal override bool TryGetBinding(BindingName name, [NotNullWhen(true)] out JsValue? value)
         {
-            if (_declarativeRecord._dictionary is not null && _declarativeRecord.TryGetBinding(name, strict, out binding, out value))
+            if (_declarativeRecord._dictionary is not null && _declarativeRecord.TryGetBinding(name, out value))
             {
                 return true;
             }
 
             // we unwrap by name
-            binding = default;
             value = default;
 
             // normal case is to find

Jint/Runtime/Environments/JintEnvironment.cs

@@ -9,7 +9,7 @@ internal static class JintEnvironment
     {
         internal static bool TryGetIdentifierEnvironmentWithBinding(
             Environment env,
-            in Environment.BindingName name,
+            Environment.BindingName name,
             [NotNullWhen(true)] out Environment? record)
         {
             record = env;
@@ -34,8 +34,7 @@ record = record._outerEnv;
 
         internal static bool TryGetIdentifierEnvironmentWithBindingValue(
             Environment env,
-            in Environment.BindingName name,
-            bool strict,
+            Environment.BindingName name,
             [NotNullWhen(true)] out Environment? record,
             [NotNullWhen(true)] out JsValue? value)
         {
@@ -44,16 +43,12 @@ record = env;
 
             if (env._outerEnv is null)
             {
-                return ((GlobalEnvironment) env).TryGetBinding(name, strict, out _, out value);
+                return ((GlobalEnvironment) env).TryGetBinding(name, out value);
             }
 
             while (!ReferenceEquals(record, null))
             {
-                if (record.TryGetBinding(
-                    name,
-                    strict,
-                    out _,
-                    out value))
+                if (record.TryGetBinding(name, out value))
                 {
                     return true;
                 }

Jint/Runtime/Environments/ModuleEnvironment.cs

@@ -47,16 +47,15 @@ internal override JsValue GetBindingValue(Key name, bool strict)
         return base.GetBindingValue(name, strict);
     }
 
-    internal override bool TryGetBinding(BindingName name, bool strict, out Binding binding, [NotNullWhen(true)] out JsValue? value)
+    internal override bool TryGetBinding(BindingName name, [NotNullWhen(true)] out JsValue? value)
     {
         if (_importBindings.TryGetValue(name.Key, out var indirectBinding))
         {
             value = indirectBinding.Module._environment.GetBindingValue(indirectBinding.BindingName, strict: true);
-            binding = new(value, canBeDeleted: false, mutable: false, strict: true);
             return true;
         }
 
-        return base.TryGetBinding(name, strict, out binding, out value);
+        return base.TryGetBinding(name, out value);
     }
 
     /// <summary>

Jint/Runtime/Environments/ObjectEnvironment.cs

@@ -67,15 +67,9 @@ private bool HasProperty(JsValue property)
             return _bindingObject.HasProperty(property);
         }
 
-        internal override bool TryGetBinding(
-            BindingName name,
-            bool strict,
-            out Binding binding,
-            [NotNullWhen(true)] out JsValue? value)
+        internal override bool TryGetBinding(BindingName name, [NotNullWhen(true)] out JsValue? value)
         {
             // we unwrap by name
-            binding = default;
-
             if (!HasProperty(name.Value))
             {
                 value = default;
@@ -153,8 +147,7 @@ internal override void SetMutableBinding(BindingName name, JsValue value, bool s
 
         internal override JsValue GetBindingValue(Key name, bool strict)
         {
-            JsValue value;
-            if (!_bindingObject.TryGetValue(name.Name, out value) && strict)
+            if (!_bindingObject.TryGetValue(name.Name, out var value) && strict)
             {
                 ExceptionHelper.ThrowReferenceNameError(_engine.Realm, name.Name);
             }

Jint/Runtime/Interpreter/Expressions/JintAssignmentExpression.cs

@@ -49,7 +49,6 @@ protected override object EvaluateInternal(EvaluationContext context)
             if (_leftIdentifier is not null && JintEnvironment.TryGetIdentifierEnvironmentWithBindingValue(
                     engine.ExecutionContext.LexicalEnvironment,
                     _leftIdentifier.Identifier,
-                    StrictModeScope.IsStrictModeCode,
                     out var identifierEnvironment,
                     out var temp))
             {

Jint/Runtime/Interpreter/Expressions/JintIdentifierExpression.cs

@@ -54,14 +54,12 @@ public override JsValue GetValue(EvaluationContext context)
             return identifier.CalculatedValue;
         }
 
-        var strict = StrictModeScope.IsStrictModeCode;
         var engine = context.Engine;
         var env = engine.ExecutionContext.LexicalEnvironment;
 
         if (JintEnvironment.TryGetIdentifierEnvironmentWithBindingValue(
                 env,
                 identifier,
-                strict,
                 out _,
                 out var value))
         {
@@ -72,7 +70,7 @@ public override JsValue GetValue(EvaluationContext context)
         }
         else
         {
-            var reference = engine._referencePool.Rent(JsValue.Undefined, identifier.Value, strict, thisValue: null);
+            var reference = engine._referencePool.Rent(JsValue.Undefined, identifier.Value, StrictModeScope.IsStrictModeCode, thisValue: null);
             value = engine.GetValue(reference, returnReferenceToPool: true);
         }
 

Jint/Runtime/Interpreter/Expressions/JintMemberExpression.cs

@@ -57,21 +57,18 @@ protected override object EvaluateInternal(EvaluationContext context)
             }
 
             JsValue? actualThis = null;
-            string? baseReferenceName = null;
+            object? baseReferenceName = null;
             JsValue? baseValue = null;
-            var isStrictModeCode = StrictModeScope.IsStrictModeCode;
 
             var engine = context.Engine;
             if (_objectExpression is JintIdentifierExpression identifierExpression)
             {
                 var identifier = identifierExpression.Identifier;
                 baseReferenceName = identifier.Key.Name;
-                var strict = isStrictModeCode;
                 var env = engine.ExecutionContext.LexicalEnvironment;
                 JintEnvironment.TryGetIdentifierEnvironmentWithBindingValue(
                     env,
                     identifier,
-                    strict,
                     out _,
                     out baseValue);
             }
@@ -96,13 +93,12 @@ protected override object EvaluateInternal(EvaluationContext context)
                 }
                 if (baseReference is Reference reference)
                 {
-                    baseReferenceName = reference.ReferencedName.ToString();
-                    baseValue = engine.GetValue(reference, false);
-                    engine._referencePool.Return(reference);
+                    baseReferenceName = reference.ReferencedName;
+                    baseValue = engine.GetValue(reference, returnReferenceToPool: true);
                 }
                 else
                 {
-                    baseValue = engine.GetValue(baseReference, false);
+                    baseValue = engine.GetValue(baseReference, returnReferenceToPool: false);
                 }
             }
 
@@ -117,7 +113,7 @@ protected override object EvaluateInternal(EvaluationContext context)
                 // we can use base data types securely, object evaluation can mess things up
                 var referenceName = property.IsPrimitive()
                     ? TypeConverter.ToString(property)
-                    : _determinedProperty?.ToString() ?? baseReferenceName;
+                    : _determinedProperty?.ToString() ?? baseReferenceName?.ToString();
 
                 TypeConverter.CheckObjectCoercible(engine, baseValue, _memberExpression.Property, referenceName!);
             }
@@ -127,7 +123,7 @@ protected override object EvaluateInternal(EvaluationContext context)
                 return MakePrivateReference(engine, baseValue, property);
             }
 
-            return context.Engine._referencePool.Rent(baseValue, property, isStrictModeCode, thisValue: actualThis);
+            return context.Engine._referencePool.Rent(baseValue, property, StrictModeScope.IsStrictModeCode, thisValue: actualThis);
         }
 
         /// <summary>

Jint/Runtime/Interpreter/Expressions/JintUpdateExpression.cs

@@ -1,5 +1,6 @@
 using Jint.Native;
 using Jint.Runtime.Environments;
+using Environment = Jint.Runtime.Environments.Environment;
 
 namespace Jint.Runtime.Interpreter.Expressions
 {
@@ -121,20 +122,16 @@ private JsValue UpdateNonIdentifier(EvaluationContext context)
 
         private JsValue? UpdateIdentifier(EvaluationContext context)
         {
-            var strict = StrictModeScope.IsStrictModeCode;
             var name = _leftIdentifier!.Identifier;
-            var engine = context.Engine;
-            var env = engine.ExecutionContext.LexicalEnvironment;
             if (JintEnvironment.TryGetIdentifierEnvironmentWithBindingValue(
-                env,
-                name,
-                strict,
-                out var environmentRecord,
-                out var value))
+                    context.Engine.ExecutionContext.LexicalEnvironment,
+                    name,
+                    out var environmentRecord,
+                    out var value))
             {
-                if (strict && _evalOrArguments)
+                if (_evalOrArguments && StrictModeScope.IsStrictModeCode)
                 {
-                    ExceptionHelper.ThrowSyntaxError(engine.Realm);
+                    ExceptionHelper.ThrowSyntaxError(context.Engine.Realm);
                 }
 
                 var isInteger = value._type == InternalTypes.Integer;
@@ -167,7 +164,7 @@ private JsValue UpdateNonIdentifier(EvaluationContext context)
                     }
                 }
 
-                environmentRecord.SetMutableBinding(name.Key.Name, newValue!, strict);
+                environmentRecord.SetMutableBinding(name.Key, newValue!, StrictModeScope.IsStrictModeCode);
                 if (_prefix)
                 {
                     return newValue;

Jint/Runtime/Interpreter/Statements/JintForStatement.cs

@@ -170,17 +170,10 @@ private Completion ForBodyEvaluation(EvaluationContext context)
         private void CreatePerIterationEnvironment(EvaluationContext context)
         {
             var engine = context.Engine;
-            var lastIterationEnv = engine.ExecutionContext.LexicalEnvironment;
-            var lastIterationEnvRec = lastIterationEnv;
-            var outer = lastIterationEnv._outerEnv;
-            var thisIterationEnv = JintEnvironment.NewDeclarativeEnvironment(engine, outer);
+            var lastIterationEnv = (DeclarativeEnvironment) engine.ExecutionContext.LexicalEnvironment;
+            var thisIterationEnv = JintEnvironment.NewDeclarativeEnvironment(engine, lastIterationEnv._outerEnv);
 
-            for (var j = 0; j < _boundNames!.Count; j++)
-            {
-                var bn = _boundNames[j];
-                var lastValue = lastIterationEnvRec.GetBindingValue(bn, true);
-                thisIterationEnv.CreateMutableBindingAndInitialize(bn, false, lastValue);
-            }
+            lastIterationEnv.TransferTo(_boundNames!, thisIterationEnv);
 
             engine.UpdateLexicalEnvironment(thisIterationEnv);
         }