Skip to content

Commit

Permalink
Backport var declaration and reference resolving performance improvem…
Browse files Browse the repository at this point in the history
…ents (#1841)

* Update test262 suite and fix issues
* Run GH Actions workflows for 3.x branch
  • Loading branch information
lahma authored Apr 26, 2024
1 parent ed7e3e7 commit 78bc1ec
Show file tree
Hide file tree
Showing 11 changed files with 111 additions and 76 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ name: Build
on:

push:
branches: [ main ]
branches: [ main, 3.x ]
paths-ignore:
- 'doc/**'
- '**.md'
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/pr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ name: PR Check
on:

pull_request:
branches: [ main, release/2.x ]
branches: [ main, release/2.x, 3.x ]

jobs:

Expand Down
9 changes: 7 additions & 2 deletions Jint.Tests.Test262/Test262Harness.settings.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"SuiteGitSha": "45e740928f9ac3c2144307fee20bb58570fb9588",
"SuiteGitSha": "c2ae5ed5e90d86e17730730b003e9b6fb050693e",
//"SuiteDirectory": "//mnt/c/work/test262",
"TargetPath": "./Generated",
"Namespace": "Jint.Tests.Test262",
Expand All @@ -8,10 +8,12 @@
"Array.fromAsync",
"async-iteration",
"Atomics",
"Float16Array",
"import-assertions",
"iterator-helpers",
"regexp-duplicate-named-groups",
"regexp-lookbehind",
"regexp-modifiers",
"regexp-unicode-property-escapes",
"regexp-v-flag",
"tail-call-optimization",
Expand Down Expand Up @@ -44,6 +46,9 @@
"built-ins/RegExp/lookahead-quantifier-match-groups.js",
"built-ins/RegExp/unicode_full_case_folding.js",

// needs "small" async rewrite
"language/module-code/top-level-await/async-module-does-not-block-sibling-modules.js",

// requires investigation how to process complex function name evaluation for property
"built-ins/Function/prototype/toString/method-computed-property-name.js",
"language/expressions/class/elements/class-name-static-initializer-anonymous.js",
Expand All @@ -68,7 +73,7 @@
// C# can't distinguish 1.797693134862315808e+308 and 1.797693134862315708145274237317e+308
"language/types/number/8.5.1.js",

// inner binding is immutable (from parameters) Expected SameValue(«null», «function() {{ ... }}») to be true
// inner binding is immutable (from parameters) Expected SameValue(«null», «function() {{ ... }}») to be true
"language/expressions/function/scope-name-var-open-non-strict.js",
"language/expressions/function/scope-name-var-open-strict.js",

Expand Down
26 changes: 16 additions & 10 deletions Jint/Engine.cs
Original file line number Diff line number Diff line change
Expand Up @@ -570,6 +570,9 @@ internal JsValue GetValue(object value, bool returnReferenceToPool)
return GetValue(reference, returnReferenceToPool);
}

/// <summary>
/// https://tc39.es/ecma262/#sec-getvalue
/// </summary>
internal JsValue GetValue(Reference reference, bool returnReferenceToPool)
{
var baseValue = reference.Base;
Expand Down Expand Up @@ -607,7 +610,8 @@ internal JsValue GetValue(Reference reference, bool returnReferenceToPool)
return baseObj.PrivateGet((PrivateName) reference.ReferencedName);
}

var v = baseObj.Get(property, reference.ThisValue);
reference.EvaluateAndCachePropertyKey();
var v = baseObj.Get(reference.ReferencedName, reference.ThisValue);
return v;
}

Expand Down Expand Up @@ -684,33 +688,35 @@ private bool TryHandleStringValue(JsValue property, JsString s, ref ObjectInstan
/// </summary>
internal void PutValue(Reference reference, JsValue value)
{
var property = reference.ReferencedName;
if (reference.IsUnresolvableReference)
{
if (reference.Strict && reference.ReferencedName != CommonProperties.Arguments)
if (reference.Strict && property != CommonProperties.Arguments)
{
ExceptionHelper.ThrowReferenceError(Realm, reference);
}

Realm.GlobalObject.Set(reference.ReferencedName, value, throwOnError: false);
Realm.GlobalObject.Set(property, value, throwOnError: false);
}
else if (reference.IsPropertyReference)
{
var baseObject = Runtime.TypeConverter.ToObject(Realm, reference.Base);
if (reference.IsPrivateReference)
{
baseObject.PrivateSet((PrivateName) reference.ReferencedName, value);
baseObject.PrivateSet((PrivateName) property, value);
return;
}

reference.EvaluateAndCachePropertyKey();
var succeeded = baseObject.Set(reference.ReferencedName, value, reference.ThisValue);
if (!succeeded && reference.Strict)
{
ExceptionHelper.ThrowTypeError(Realm, "Cannot assign to read only property '" + reference.ReferencedName + "' of " + baseObject);
ExceptionHelper.ThrowTypeError(Realm, "Cannot assign to read only property '" + property + "' of " + baseObject);
}
}
else
{
((Environment) reference.Base).SetMutableBinding(Runtime.TypeConverter.ToString(reference.ReferencedName), value, reference.Strict);
((Environment) reference.Base).SetMutableBinding(Runtime.TypeConverter.ToString(property), value, reference.Strict);
}
}

Expand Down Expand Up @@ -891,7 +897,7 @@ internal SyntaxElement GetLastSyntaxElement()
public JsValue GetValue(JsValue scope, JsValue property)
{
var reference = _referencePool.Rent(scope, property, _isStrict, thisValue: null);
var jsValue = GetValue(reference, false);
var jsValue = GetValue(reference, returnReferenceToPool: false);
_referencePool.Return(reference);
return jsValue;
}
Expand Down Expand Up @@ -1002,7 +1008,7 @@ private void GlobalDeclarationInstantiation(
for (var i = 0; i < lexNames.Count; i++)
{
var (dn, constant) = lexNames[i];
if (env.HasVarDeclaration(dn) || env.HasLexicalDeclaration(dn) || env.HasRestrictedGlobalProperty(dn))
if (env.HasLexicalDeclaration(dn) || env.HasRestrictedGlobalProperty(dn))
{
ExceptionHelper.ThrowSyntaxError(realm, $"Identifier '{dn}' has already been declared");
}
Expand All @@ -1017,7 +1023,7 @@ private void GlobalDeclarationInstantiation(
}
}

// we need to go trough in reverse order to handle the hoisting correctly
// we need to go through in reverse order to handle the hoisting correctly
for (var i = functionToInitialize.Count - 1; i > -1; i--)
{
var f = functionToInitialize[i];
Expand Down Expand Up @@ -1367,7 +1373,7 @@ internal void EvalDeclarationInstantiation(
{
if (varEnvRec is GlobalEnvironment ger)
{
ger.CreateGlobalVarBinding(vn, true);
ger.CreateGlobalVarBinding(vn, canBeDeleted: true);
}
else
{
Expand Down
13 changes: 12 additions & 1 deletion Jint/Native/Array/ArrayIteratorPrototype.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ namespace Jint.Native.Array;
/// </summary>
internal sealed class ArrayIteratorPrototype : IteratorPrototype
{
private ClrFunction? _originalNextFunction;

internal ArrayIteratorPrototype(
Engine engine,
Realm realm,
Expand All @@ -22,9 +24,10 @@ internal ArrayIteratorPrototype(

protected override void Initialize()
{
_originalNextFunction = new ClrFunction(Engine, "next", Next, 0, PropertyFlag.Configurable);
var properties = new PropertyDictionary(1, checkExistingKeys: false)
{
[KnownKeys.Next] = new(new ClrFunction(Engine, "next", Next, 0, PropertyFlag.Configurable), true, false, true)
[KnownKeys.Next] = new(_originalNextFunction, PropertyFlag.NonEnumerable)
};
SetProperties(properties);

Expand All @@ -37,13 +40,21 @@ protected override void Initialize()

internal IteratorInstance Construct(ObjectInstance array, ArrayIteratorType kind)
{
if (!HasOriginalNext)
{
return new IteratorInstance.ObjectIterator(this);
}

IteratorInstance instance = array is JsArray jsArray
? new ArrayIterator(Engine, jsArray, kind) { _prototype = this }
: new ArrayLikeIterator(Engine, array, kind) { _prototype = this };

return instance;
}

internal bool HasOriginalNext
=> ReferenceEquals(Get(CommonProperties.Next), _originalNextFunction);

private sealed class ArrayIterator : IteratorInstance
{
private readonly ArrayIteratorType _kind;
Expand Down
77 changes: 47 additions & 30 deletions Jint/Native/TypedArray/IntrinsicTypedArrayPrototype.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1313,56 +1313,73 @@ private JsValue Subarray(JsValue thisObject, JsValue[] arguments)
ExceptionHelper.ThrowTypeError(_realm);
}

var begin = arguments.At(0);
var start = arguments.At(0);
var end = arguments.At(1);

var buffer = o._viewedArrayBuffer;
var srcLength = o.GetLength();
var relativeBegin = TypeConverter.ToIntegerOrInfinity(begin);
var srcRecord = MakeTypedArrayWithBufferWitnessRecord(o, ArrayBufferOrder.SeqCst);

double beginIndex;
if (double.IsNegativeInfinity(relativeBegin))
{
beginIndex = 0;
}
else if (relativeBegin < 0)
uint srcLength = 0;
if (!srcRecord.IsTypedArrayOutOfBounds)
{
beginIndex = System.Math.Max(srcLength + relativeBegin, 0);
srcLength = srcRecord.TypedArrayLength;
}
else

var relativeStart = TypeConverter.ToIntegerOrInfinity(start);

double startIndex;
if (double.IsNegativeInfinity(relativeStart))
{
beginIndex = System.Math.Min(relativeBegin, srcLength);
startIndex = 0;
}

double relativeEnd;
if (end.IsUndefined())
else if (relativeStart < 0)
{
relativeEnd = srcLength;
startIndex = System.Math.Max(srcLength + relativeStart, 0);
}
else
{
relativeEnd = TypeConverter.ToIntegerOrInfinity(end);
startIndex = System.Math.Min(relativeStart, srcLength);
}

double endIndex;
if (double.IsNegativeInfinity(relativeEnd))
{
endIndex = 0;
}
else if (relativeEnd < 0)
var elementSize = o._arrayElementType.GetElementSize();
var srcByteOffset = o._byteOffset;
var beginByteOffset = srcByteOffset + startIndex * elementSize;

JsValue[] argumentsList;
if (o._arrayLength == JsTypedArray.LengthAuto && end.IsUndefined())
{
endIndex = System.Math.Max(srcLength + relativeEnd, 0);
argumentsList = [buffer, beginByteOffset];
}
else
{
endIndex = System.Math.Min(relativeEnd, srcLength);
double relativeEnd;
if (end.IsUndefined())
{
relativeEnd = srcLength;
}
else
{
relativeEnd = TypeConverter.ToIntegerOrInfinity(end);
}

double endIndex;
if (double.IsNegativeInfinity(relativeEnd))
{
endIndex = 0;
}
else if (relativeEnd < 0)
{
endIndex = System.Math.Max(srcLength + relativeEnd, 0);
}
else
{
endIndex = System.Math.Min(relativeEnd, srcLength);
}

var newLength = System.Math.Max(endIndex - startIndex, 0);
argumentsList = [buffer, beginByteOffset, newLength];
}

var newLength = System.Math.Max(endIndex - beginIndex, 0);
var elementSize = o._arrayElementType.GetElementSize();
var srcByteOffset = o._byteOffset;
var beginByteOffset = srcByteOffset + beginIndex * elementSize;
var argumentsList = new JsValue[] { buffer, beginByteOffset, newLength };
return _realm.Intrinsics.TypedArray.TypedArraySpeciesCreate(o, argumentsList);
}

Expand Down
27 changes: 9 additions & 18 deletions Jint/Runtime/Environments/GlobalEnvironment.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ public GlobalDeclarativeEnvironment(Engine engine) : base(engine)

// Environment records are needed by debugger
internal readonly GlobalDeclarativeEnvironment _declarativeRecord;
private readonly HashSet<Key> _varNames = [];

public GlobalEnvironment(Engine engine, ObjectInstance global) : base(engine)
{
Expand Down Expand Up @@ -258,15 +257,10 @@ internal override bool DeleteBinding(Key name)
return _declarativeRecord.DeleteBinding(name);
}

if (_global.HasOwnProperty(name.Name))
var n = JsString.Create(name.Name);
if (_global.HasOwnProperty(n))
{
var status = _global.Delete(name.Name);
if (status)
{
_varNames.Remove(name);
}

return status;
return _global.Delete(n);
}

return true;
Expand All @@ -280,8 +274,6 @@ internal override bool DeleteBinding(Key name)

internal override JsValue GetThisBinding() => _global;

internal bool HasVarDeclaration(Key name) => _varNames.Contains(name);

internal bool HasLexicalDeclaration(Key name) => _declarativeRecord.HasBinding(name);

internal bool HasRestrictedGlobalProperty(Key name)
Expand Down Expand Up @@ -334,12 +326,14 @@ public bool CanDeclareGlobalFunction(Key name)

public void CreateGlobalVarBinding(Key name, bool canBeDeleted)
{
if (_global.Extensible && _global._properties!.TryAdd(name, new PropertyDescriptor(Undefined, canBeDeleted
? PropertyFlag.ConfigurableEnumerableWritable | PropertyFlag.MutableBinding
: PropertyFlag.NonConfigurable | PropertyFlag.MutableBinding)))
if (!_global.Extensible)
{
_varNames.Add(name);
return;
}

_global._properties!.TryAdd(name, new PropertyDescriptor(Undefined, canBeDeleted
? PropertyFlag.ConfigurableEnumerableWritable | PropertyFlag.MutableBinding
: PropertyFlag.NonConfigurable | PropertyFlag.MutableBinding));
}

internal void CreateGlobalVarBindings(List<Key> names, bool canBeDeleted)
Expand All @@ -356,8 +350,6 @@ internal void CreateGlobalVarBindings(List<Key> names, bool canBeDeleted)
_global._properties!.TryAdd(name, new PropertyDescriptor(Undefined, canBeDeleted
? PropertyFlag.ConfigurableEnumerableWritable | PropertyFlag.MutableBinding
: PropertyFlag.NonConfigurable | PropertyFlag.MutableBinding));

_varNames.Add(name);
}
}

Expand All @@ -381,7 +373,6 @@ public void CreateGlobalFunctionBinding(Key name, JsValue value, bool canBeDelet

_global.DefinePropertyOrThrow(jsString, desc);
_global.Set(jsString, value, false);
_varNames.Add(name);
}

internal override bool HasBindings()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ private static JsValue HandleArrayPattern(
{
close = true;
var reference = GetReferenceFromMember(context, me);

JsValue value;
if (arrayOperations != null)
{
Expand Down
7 changes: 1 addition & 6 deletions Jint/Runtime/Interpreter/Expressions/JintMemberExpression.cs
Original file line number Diff line number Diff line change
Expand Up @@ -127,12 +127,7 @@ protected override object EvaluateInternal(EvaluationContext context)
return MakePrivateReference(engine, baseValue, property);
}

// only convert if necessary
var propertyKey = property.IsInteger() && baseValue.IsIntegerIndexedArray
? property
: TypeConverter.ToPropertyKey(property);

return context.Engine._referencePool.Rent(baseValue, propertyKey, isStrictModeCode, thisValue: actualThis);
return context.Engine._referencePool.Rent(baseValue, property, isStrictModeCode, thisValue: actualThis);
}

/// <summary>
Expand Down
Loading

0 comments on commit 78bc1ec

Please sign in to comment.