Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport improve environment handling performance and cleanup API #1846

Merged
merged 1 commit into from
Apr 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Jint/Runtime/Debugger/DebugScope.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
6 changes: 3 additions & 3 deletions Jint/Runtime/Descriptors/Specialized/ClrAccessDescriptor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}
31 changes: 22 additions & 9 deletions Jint/Runtime/Environments/DeclarativeEnvironment.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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);
}
}
}
}
6 changes: 1 addition & 5 deletions Jint/Runtime/Environments/Environment.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
9 changes: 2 additions & 7 deletions Jint/Runtime/Environments/GlobalEnvironment.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
13 changes: 4 additions & 9 deletions Jint/Runtime/Environments/JintEnvironment.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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)
{
Expand All @@ -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;
}
Expand Down
5 changes: 2 additions & 3 deletions Jint/Runtime/Environments/ModuleEnvironment.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
11 changes: 2 additions & 9 deletions Jint/Runtime/Environments/ObjectEnvironment.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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))
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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))
{
Expand All @@ -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);
}

Expand Down
16 changes: 6 additions & 10 deletions Jint/Runtime/Interpreter/Expressions/JintMemberExpression.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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);
}
}

Expand All @@ -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!);
}
Expand All @@ -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>
Expand Down
19 changes: 8 additions & 11 deletions Jint/Runtime/Interpreter/Expressions/JintUpdateExpression.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using Jint.Native;
using Jint.Runtime.Environments;
using Environment = Jint.Runtime.Environments.Environment;

namespace Jint.Runtime.Interpreter.Expressions
{
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
13 changes: 3 additions & 10 deletions Jint/Runtime/Interpreter/Statements/JintForStatement.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down