Removing some optimizations to prepare for refactoring (#680)

* Removing some optimizations to prepare for refactoring

- FunctionWasCalled is not used anymore but kept for future reference. It detects if a method is called to release an arguments array to the pull.
- The last binding memoization has been removed.

Jint.Tests/Runtime/EngineTests.cs

@@ -10,6 +10,7 @@ using Jint.Native.Object;
 using Jint.Runtime;
 using Jint.Runtime.Debugger;
 using Xunit;
+using Xunit.Abstractions;
 
 namespace Jint.Tests.Runtime
 {
@@ -19,10 +20,10 @@ namespace Jint.Tests.Runtime
         private int countBreak = 0;
         private StepMode stepMode;
 
-        public EngineTests()
+        public EngineTests(ITestOutputHelper output)
         {
             _engine = new Engine()
-                .SetValue("log", new Action<object>(Console.WriteLine))
+                .SetValue("log", new Action<object>( o => output.WriteLine(o.ToString())))
                 .SetValue("assert", new Action<bool>(Assert.True))
                 .SetValue("equal", new Action<object, object>(Assert.Equal))
                 ;

Jint/Native/Function/ScriptFunctionInstance.cs

@@ -12,7 +12,6 @@ namespace Jint.Native.Function
     public sealed class ScriptFunctionInstance : FunctionInstance, IConstructor
     {
         internal readonly JintFunctionDefinition _function;
-        private LexicalEnvironment _localEnv;
 
 
         /// <summary>
@@ -87,9 +86,7 @@ namespace Jint.Native.Function
                     thisBinding = thisArg;
                 }
 
-                var localEnv = _localEnv ?? LexicalEnvironment.NewDeclarativeEnvironment(_engine, _scope);
-                localEnv.Reset(_scope);
-                _localEnv = null;
+                var localEnv = LexicalEnvironment.NewDeclarativeEnvironment(_engine, _scope);
 
                 _engine.EnterExecutionContext(localEnv, localEnv, thisBinding);
 
@@ -124,7 +121,6 @@ namespace Jint.Native.Function
                 finally
                 {
                     _engine.LeaveExecutionContext();
-                    _localEnv = localEnv;
                 }
 
                 return Undefined;

Jint/Runtime/Environments/Binding.cs

@@ -14,5 +14,7 @@ namespace Jint.Runtime.Environments
         public JsValue Value;
         public readonly bool CanBeDeleted;
         public readonly bool Mutable;
+
+        public bool IsInitialized => !ReferenceEquals(Value, null);
     }
 }

Jint/Runtime/Environments/DeclarativeEnvironmentRecord.cs

@@ -17,112 +17,25 @@ namespace Jint.Runtime.Environments
     /// </summary>
     public sealed class DeclarativeEnvironmentRecord : EnvironmentRecord
     {
-        private StringDictionarySlim<Binding> _dictionary;
-        private bool _set;
-        private Key _key;
-        private Binding _value;
-
-        private Binding _argumentsBinding;
-
-        // false = not accessed, true = accessed, null = values copied
-        private bool? _argumentsBindingWasAccessed = false;
+        private StringDictionarySlim<Binding> _dictionary = new StringDictionarySlim<Binding>();
 
         public DeclarativeEnvironmentRecord(Engine engine) : base(engine)
         {
         }
-
-        internal void Reset()
-        {
-            _dictionary?.Clear();
-            _set = false;
-            _key = default;
-            _value = default;
-            _argumentsBinding = default;
-            _argumentsBindingWasAccessed = false;
-        }
-
-        private void SetItem(in Key key, in Binding value)
-        {
-            if (_set && _key != key)
-            {
-                if (_dictionary == null)
-                {
-                    _dictionary = new StringDictionarySlim<Binding>();
-                }
-
-                _dictionary[_key] = _value;
-            }
-
-            _set = true;
-            _key = key;
-            _value = value;
-
-            if (_dictionary != null)
-            {
-                _dictionary[key] = value;
-            }
-        }
-
-        private ref Binding GetExistingItem(in Key key)
+        
+        private ref Binding GetOrCreateBinding(in Key key)
         {
-            if (_set && _key == key)
-            {
-                return ref _value;
-            }
-
-            if (key == KnownKeys.Arguments)
-            {
-                _argumentsBindingWasAccessed = true;
-                return ref _argumentsBinding;
-            }
-
             return ref _dictionary.GetOrAddValueRef(key);
         }
 
         private bool ContainsKey(in Key key)
         {
-            if (key == KnownKeys.Arguments)
-            {
-                return !ReferenceEquals(_argumentsBinding.Value, null);
-            }
-
-            if (_set && key == _key)
-            {
-                return true;
-            }
-
             return _dictionary?.ContainsKey(key) == true;
         }
 
-        private void Remove(in Key key)
-        {
-            if (_set && key == _key)
-            {
-                _set = false;
-                _key = default;
-                _value = default;
-            }
-
-            if (key == KnownKeys.Arguments)
-            {
-                _argumentsBinding.Value = null;
-            }
-            else
-            {
-                _dictionary?.Remove(key);
-            }
-        }
-
         private bool TryGetValue(in Key key, out Binding value)
         {
-            value = default;
-            if (_set && _key == key)
-            {
-                value = _value;
-                return true;
-            }
-
-            return _dictionary != null && _dictionary.TryGetValue(key, out value);
+            return _dictionary.TryGetValue(key, out value);
         }
 
         public override bool HasBinding(in Key name)
@@ -136,42 +49,19 @@ namespace Jint.Runtime.Environments
             out Binding binding,
             out JsValue value)
         {
-            if (_set && _key == name)
-            {
-                binding = _value;
-                value = UnwrapBindingValue(strict, _value);
-                return true;
-            }
-
-            if (name == KnownKeys.Arguments
-                && !ReferenceEquals(_argumentsBinding.Value, null))
-            {
-                _argumentsBindingWasAccessed = true;
-                binding = _argumentsBinding;
-                value = UnwrapBindingValue(strict, _argumentsBinding);
-                return true;
-            }
-
-            if (_dictionary != null)
-            {
-                var success = _dictionary.TryGetValue(name, out binding);
-                value = success ? UnwrapBindingValue(strict, binding) : default;
-                return success;
-            }
-
-            binding = default;
-            value = default;
-            return false;
+            var success = _dictionary.TryGetValue(name, out binding);
+            value = success ? UnwrapBindingValue(strict, binding) : default;
+            return success;
         }
 
         public override void CreateMutableBinding(in Key name, JsValue value, bool canBeDeleted = false)
         {
-            SetItem(name, new Binding(value, canBeDeleted, mutable: true));
+            _dictionary[name] = new Binding(value, canBeDeleted, mutable: true);
         }
 
         public override void SetMutableBinding(in Key name, JsValue value, bool strict)
         {
-            ref var binding = ref GetExistingItem(name);
+            ref var binding = ref GetOrCreateBinding(name);
 
             if (binding.Mutable)
             {
@@ -188,14 +78,14 @@ namespace Jint.Runtime.Environments
 
         public override JsValue GetBindingValue(in Key name, bool strict)
         {
-            ref var binding = ref GetExistingItem(name);
+            ref var binding = ref GetOrCreateBinding(name);
             return UnwrapBindingValue(strict, binding);
         }
 
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         private JsValue UnwrapBindingValue(bool strict, in Binding binding)
         {
-            if (!binding.Mutable && binding.Value._type == InternalTypes.Undefined)
+            if (!binding.Mutable && !binding.IsInitialized)
             {
                 if (strict)
                 {
@@ -215,9 +105,7 @@ namespace Jint.Runtime.Environments
 
         public override bool DeleteBinding(in Key name)
         {
-            ref Binding binding = ref GetExistingItem(name);
-
-            if (ReferenceEquals(binding.Value, null))
+            if (!_dictionary.TryGetValue(name, out var binding))
             {
                 return true;
             }
@@ -227,7 +115,7 @@ namespace Jint.Runtime.Environments
                 return false;
             }
 
-            Remove(name);
+            _dictionary.Remove(name);
 
             return true;
         }
@@ -240,34 +128,8 @@ namespace Jint.Runtime.Environments
         /// <inheritdoc />
         public override Key[] GetAllBindingNames()
         {
-            int size = _set ? 1 : 0;
-            if (!ReferenceEquals(_argumentsBinding.Value, null))
-            {
-                size += 1;
-            }
-
-            if (_dictionary != null)
-            {
-                size += _dictionary.Count;
-            }
-
-            var keys = size > 0 ? new Key[size] : ArrayExt.Empty<Key>();
-            int n = 0;
-            if (_set)
-            {
-                keys[n++] = _key;
-            }
-
-            if (!ReferenceEquals(_argumentsBinding.Value, null))
-            {
-                keys[n++] = KnownKeys.Arguments;
-            }
-
-            if (_dictionary == null)
-            {
-                return keys;
-            }
-
+            var keys = new Key[_dictionary.Count];
+            var n = 0;
             foreach (var entry in _dictionary)
             {
                 keys[n++] = entry.Key;
@@ -284,19 +146,17 @@ namespace Jint.Runtime.Environments
         {
             var parameters = functionDeclaration.Params;
 
-            bool empty = _dictionary == null && !_set;
+            bool empty = _dictionary.Count == 0;
 
-            if (ReferenceEquals(_argumentsBinding.Value, null)
-                && !(functionInstance is ArrowFunctionInstance))
+            if (!(functionInstance is ArrowFunctionInstance))
             {
-                _argumentsBinding = new Binding(argumentsInstance, canBeDeleted: false, mutable: true);
+                _dictionary[KnownKeys.Arguments] = new Binding(argumentsInstance, canBeDeleted: false, mutable: true);
             }
 
             for (var i = 0; i < parameters.Count; i++)
             {
                 SetFunctionParameter(parameters[i], arguments, i, empty);
             }
-
         }
 
         private void SetFunctionParameter(
@@ -456,21 +316,13 @@ namespace Jint.Runtime.Environments
         {
             if (initiallyEmpty || !TryGetValue(name, out var existing))
             {
-                var binding = new Binding(argument, false, true);
-                if (name == KnownKeys.Arguments)
-                {
-                    _argumentsBinding = binding;
-                }
-                else
-                {
-                    SetItem(name, binding);
-                }
+                _dictionary[name] = new Binding(argument, false, true);
             }
             else
             {
                 if (existing.Mutable)
                 {
-                    ref var b = ref GetExistingItem(name);
+                    ref var b = ref GetOrCreateBinding(name);
                     b.Value = argument;
                 }
                 else
@@ -496,7 +348,7 @@ namespace Jint.Runtime.Environments
                         if (!ContainsKey(dn))
                         {
                             var binding = new Binding(Undefined, canBeDeleted: false, mutable: true);
-                            SetItem(dn, binding);
+                            _dictionary[dn] = binding;
                         }
                     }
                 }
@@ -520,24 +372,29 @@ namespace Jint.Runtime.Environments
         
         internal override void FunctionWasCalled()
         {
-            // we can safely release arguments only if it doesn't have possibility to escape the scope
-            // so check if someone ever accessed it
-            if (!(_argumentsBinding.Value is ArgumentsInstance argumentsInstance))
-            {
-                return;
-            }
-            
-            if (!argumentsInstance._initialized && _argumentsBindingWasAccessed == false)
+            if (_dictionary.TryGetValue(KnownKeys.Arguments, out var arguments) && arguments.Value is ArgumentsInstance argumentsInstance)
             {
-                _engine._argumentsInstancePool.Return(argumentsInstance);
-                _argumentsBinding = default;
-            }
-            else if (_argumentsBindingWasAccessed != null && argumentsInstance._args.Length > 0)
-            {
-                // we need to ensure we hold on to arguments given
                 argumentsInstance.PersistArguments();
-                _argumentsBindingWasAccessed = null;
             }
+
+            // we can safely release arguments only if it doesn't have possibility to escape the scope
+            // so check if someone ever accessed it
+            //if (!(_argumentsBinding.Value is ArgumentsInstance argumentsInstance))
+            //{
+            //    return;
+            //}
+
+            //if (!argumentsInstance._initialized && _argumentsBindingWasAccessed == false)
+            //{
+            //    _engine._argumentsInstancePool.Return(argumentsInstance);
+            //    _argumentsBinding = default;
+            //}
+            //else if (_argumentsBindingWasAccessed != null && argumentsInstance._args.Length > 0)
+            //{
+            //    // we need to ensure we hold on to arguments given
+            //    argumentsInstance.PersistArguments();
+            //    _argumentsBindingWasAccessed = null;
+            //}
         }
 
         private sealed class ArrayPatternProtocol : IteratorProtocol

Jint/Runtime/Environments/LexicalEnvironment.cs

@@ -70,15 +70,10 @@ namespace Jint.Runtime.Environments
         public static LexicalEnvironment NewDeclarativeEnvironment(Engine engine, LexicalEnvironment outer = null)
         {
             var environment = new LexicalEnvironment(engine, null, null);
-            environment.Reset(outer);
-            return environment;
-        }
+            environment._record = new DeclarativeEnvironmentRecord(engine);
+            environment._outer = outer;
 
-        internal void Reset(LexicalEnvironment outer)
-        {
-            _record = _record ?? new DeclarativeEnvironmentRecord(_engine);
-            ((DeclarativeEnvironmentRecord) _record).Reset();
-            _outer = outer;
+            return environment;
         }
 
         public static LexicalEnvironment NewObjectEnvironment(Engine engine, ObjectInstance objectInstance, LexicalEnvironment outer, bool provideThis)