Skip to content

Commit

Permalink
Removing some optimizations to prepare for refactoring (#680)
Browse files Browse the repository at this point in the history 
* 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.
  • Loading branch information
@sebastienros
sebastienros committed Jan 8, 2020
1 parent 5d07e89 commit d53c2ce
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 199 deletions.
5 changes: 3 additions & 2 deletions Jint.Tests/Runtime/EngineTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
using Jint.Runtime;
using Jint.Runtime.Debugger;
using Xunit;
using Xunit.Abstractions;

namespace Jint.Tests.Runtime
{
Expand All @@ -19,10 +20,10 @@ public class EngineTests : IDisposable
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))
;
Expand Down
6 changes: 1 addition & 5 deletions Jint/Native/Function/ScriptFunctionInstance.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ namespace Jint.Native.Function
public sealed class ScriptFunctionInstance : FunctionInstance, IConstructor
{
internal readonly JintFunctionDefinition _function;
private LexicalEnvironment _localEnv;


/// <summary>
Expand Down Expand Up @@ -87,9 +86,7 @@ public override JsValue Call(JsValue thisArg, JsValue[] arguments)
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);

Expand Down Expand Up @@ -124,7 +121,6 @@ public override JsValue Call(JsValue thisArg, JsValue[] arguments)
finally
{
_engine.LeaveExecutionContext();
_localEnv = localEnv;
}

return Undefined;
Expand Down
2 changes: 2 additions & 0 deletions Jint/Runtime/Environments/Binding.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,7 @@ public Binding(JsValue value, bool canBeDeleted, bool mutable)
public JsValue Value;
public readonly bool CanBeDeleted;
public readonly bool Mutable;

public bool IsInitialized => !ReferenceEquals(Value, null);
}
}
225 changes: 41 additions & 184 deletions Jint/Runtime/Environments/DeclarativeEnvironmentRecord.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -136,42 +49,19 @@ public override bool HasBinding(in Key name)
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)
{
Expand All @@ -188,14 +78,14 @@ public override void SetMutableBinding(in Key name, JsValue value, bool strict)

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)
{
Expand All @@ -215,9 +105,7 @@ private void ThrowUninitializedBindingException()

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;
}
Expand All @@ -227,7 +115,7 @@ public override bool DeleteBinding(in Key name)
return false;
}

Remove(name);
_dictionary.Remove(name);

return true;
}
Expand All @@ -240,34 +128,8 @@ public override JsValue ImplicitThisValue()
/// <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;
Expand All @@ -284,19 +146,17 @@ public override Key[] GetAllBindingNames()
{
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(
Expand Down Expand Up @@ -456,21 +316,13 @@ private void SetItemSafely(in Key name, JsValue argument, bool initiallyEmpty)
{
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
Expand All @@ -496,7 +348,7 @@ internal void AddVariableDeclarations(ref NodeList<VariableDeclaration> variable
if (!ContainsKey(dn))
{
var binding = new Binding(Undefined, canBeDeleted: false, mutable: true);
SetItem(dn, binding);
_dictionary[dn] = binding;
}
}
}
Expand All @@ -520,24 +372,29 @@ internal static JsValue HandleAssignmentPatternIfNeeded(IFunction functionDeclar

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
Expand Down

0 comments on commit d53c2ce

Please sign in to comment.