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.

Sign up for GitHub

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

Jump to bottom

Optimize Array.pop #1680

Merged
merged 1 commit into from
Nov 12, 2023
Merged

Optimize Array.pop #1680

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion Jint/Engine.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -618,7 +618,7 @@ internal JsValue GetValue(Reference reference, bool returnReferenceToPool)
return JsValue.Undefined;
}

var callable = (ICallable) getter.AsObject();
var callable = (ICallable) getter;
return callable.Call(baseValue, Arguments.Empty);
}
}
Expand Down
98 changes: 66 additions & 32 deletions Jint/Native/Array/ArrayInstance.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ public class ArrayInstance : ObjectInstance, IEnumerable<JsValue>

private ObjectChangeFlags _objectChangeFlags;

private ArrayConstructor? _constructor;

private protected ArrayInstance(Engine engine, InternalTypes type) : base(engine, type: type)
{
_dense = System.Array.Empty<JsValue?>();
Expand Down Expand Up @@ -54,7 +56,8 @@ private protected ArrayInstance(Engine engine, JsValue[] items) : base(engine, t

private void InitializePrototypeAndValidateCapacity(Engine engine, uint capacity)
{
_prototype = engine.Realm.Intrinsics.Array.PrototypeObject;
_constructor = engine.Realm.Intrinsics.Array;
_prototype = _constructor.PrototypeObject;

if (capacity > 0 && capacity > engine.Options.Constraints.MaxArraySize)
{
Expand All @@ -67,7 +70,7 @@ private void InitializePrototypeAndValidateCapacity(Engine engine, uint capacity
public sealed override bool IsArray() => true;

internal sealed override bool HasOriginalIterator
=> ReferenceEquals(Get(GlobalSymbolRegistry.Iterator), _engine.Realm.Intrinsics.Array.PrototypeObject._originalIteratorFunction);
=> ReferenceEquals(Get(GlobalSymbolRegistry.Iterator), _constructor?.PrototypeObject._originalIteratorFunction);

/// <summary>
/// Checks whether there have been changes to object prototype chain which could render fast access patterns impossible.
Expand All @@ -83,7 +86,7 @@ internal bool CanUseFastAccess
}

if (_prototype is not ArrayPrototype arrayPrototype
|| !ReferenceEquals(_prototype, _engine.Realm.Intrinsics.Array.PrototypeObject))
|| !ReferenceEquals(_prototype, _constructor?.PrototypeObject))
{
// somebody has switched prototype
return false;
Expand All @@ -96,7 +99,7 @@ internal bool CanUseFastAccess
}

if (arrayPrototype.Prototype is not ObjectPrototype arrayPrototypePrototype
|| !ReferenceEquals(arrayPrototypePrototype, _engine.Realm.Intrinsics.Array.PrototypeObject.Prototype))
|| !ReferenceEquals(arrayPrototypePrototype, _constructor.PrototypeObject.Prototype))
{
return false;
}
Expand Down Expand Up @@ -177,7 +180,7 @@ private bool DefineLength(PropertyDescriptor desc)
{
for (uint keyIndex = 0; keyIndex < _dense.Length; ++keyIndex)
{
if (_dense[keyIndex] == null)
if (_dense[keyIndex] is null)
{
continue;
}
Expand Down Expand Up @@ -284,15 +287,10 @@ private bool DefineOwnProperty(uint index, PropertyDescriptor desc)
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal uint GetLength()
{
if (_length is null)
{
return 0;
}
internal uint GetLength() => (uint) GetJsNumberLength()._value;

return (uint) ((JsNumber) _length._value!)._value;
}
[MethodImpl(MethodImplOptions.AggressiveInlining)]
private JsNumber GetJsNumberLength() => _length is null ? JsNumber.PositiveZero : (JsNumber) _length._value!;

protected sealed override void AddProperty(JsValue property, PropertyDescriptor descriptor)
{
Expand Down Expand Up @@ -330,7 +328,7 @@ public sealed override List<JsValue> GetOwnPropertyKeys(Types types = Types.None
var length = System.Math.Min(temp.Length, GetLength());
for (var i = 0; i < length; i++)
{
if (temp[i] != null)
if (temp[i] is not null)
{
properties.Add(JsString.Create(i));
}
Expand Down Expand Up @@ -380,7 +378,7 @@ public sealed override List<JsValue> GetOwnPropertyKeys(Types types = Types.None
for (uint i = 0; i < length; i++)
{
var value = temp[i];
if (value != null)
if (value is not null)
{
if (_sparse is null || !_sparse.TryGetValue(i, out var descriptor) || descriptor is null)
{
Expand Down Expand Up @@ -633,17 +631,19 @@ private void EnsureCorrectLength(uint index)
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal void SetLength(ulong length)
internal void SetLength(ulong length) => SetLength(JsNumber.Create(length));

[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal void SetLength(JsNumber length)
{
var number = JsNumber.Create(length);
if (Extensible && _length!._flags == PropertyFlag.OnlyWritable)
{
_length!.Value = number;
_length!.Value = length;
}
else
{
// slow path
Set(CommonProperties.Length, number, true);
Set(CommonProperties.Length, length, true);
}
}

Expand Down Expand Up @@ -677,33 +677,44 @@ internal bool DeletePropertyOrThrow(uint index)
return true;
}

internal bool Delete(uint index)
private bool Delete(uint index) => Delete(index, unwrapFromNonDataDescriptor: false, out _);

private bool Delete(uint index, bool unwrapFromNonDataDescriptor, out JsValue? deletedValue)
{
TryGetDescriptor(index, createIfMissing: false, out var desc);

// check fast path
var temp = _dense;
if (temp != null)
{
if (index < (uint) temp.Length)
{
if (!TryGetDescriptor(index, createIfMissing: false, out var descriptor) || descriptor.Configurable)
if (desc is null || desc.Configurable)
{
deletedValue = temp[index];
temp[index] = null;
return true;
}
}
}

if (!TryGetDescriptor(index, createIfMissing: false, out var desc))
if (desc is null)
{
deletedValue = null;
return true;
}

if (desc.Configurable)
{
DeleteAt(index);
_sparse!.Remove(index);
deletedValue = desc.IsDataDescriptor() || unwrapFromNonDataDescriptor
? UnwrapJsValue(desc)
: null;

return true;
}

deletedValue = null;
return false;
}

Expand All @@ -728,21 +739,23 @@ internal bool DeleteAt(uint index)

private bool TryGetDescriptor(uint index, bool createIfMissing, [NotNullWhen(true)] out PropertyDescriptor? descriptor)
{
if (!createIfMissing && _sparse is null)
{
descriptor = null;
return false;
}

descriptor = null;
var temp = _dense;
if (temp != null)
{
if (index < (uint) temp.Length)
{
var value = temp[index];
if (value != null)
if (value is not null)
{
if (_sparse is null || !_sparse.TryGetValue(index, out descriptor) || descriptor is null)
{
if (!createIfMissing)
{
return false;
}
_sparse ??= new Dictionary<uint, PropertyDescriptor?>();
_sparse[index] = descriptor = new PropertyDescriptor(value, PropertyFlag.ConfigurableEnumerableWritable);
}
Expand Down Expand Up @@ -913,7 +926,7 @@ private void ConvertToSparse()
for (uint i = 0; i < (uint) temp.Length; ++i)
{
var value = temp[i];
if (value != null)
if (value is not null)
{
_sparse.TryGetValue(i, out var descriptor);
descriptor ??= new PropertyDescriptor(value, PropertyFlag.ConfigurableEnumerableWritable);
Expand Down Expand Up @@ -1004,7 +1017,7 @@ private IEnumerable<IndexedEntry> Enumerate()
for (var i = 0; i < length; i++)
{
var value = temp[i];
if (value != null)
if (value is not null)
{
yield return new IndexedEntry(i, value);
}
Expand Down Expand Up @@ -1107,6 +1120,27 @@ public uint Push(JsValue[] values)
return (uint) n;
}

public JsValue Pop()
{
var len = GetJsNumberLength();
if (JsNumber.PositiveZero.Equals(len))
{
SetLength(len);
return Undefined;
}

var newLength = (uint) len._value - 1;

if (!Delete(newLength, unwrapFromNonDataDescriptor: true, out var element))
{
ExceptionHelper.ThrowTypeError(_engine.Realm);
}

SetLength(newLength);

return element ?? Undefined;
}

private bool CanSetLength()
{
if (!_length!.IsAccessorDescriptor())
Expand Down Expand Up @@ -1138,7 +1172,7 @@ internal JsArray Map(JsValue[] arguments)
var len = GetLength();

var callable = GetCallable(callbackfn);
var a = Engine.Realm.Intrinsics.Array.ArrayCreate(len);
var a = _engine.Realm.Intrinsics.Array.ArrayCreate(len);
var args = _engine._jsValueArrayPool.RentArray(3);
args[2] = this;
for (uint k = 0; k < len; k++)
Expand Down Expand Up @@ -1306,7 +1340,7 @@ internal void CopyValues(JsArray source, uint sourceStartIndex, uint targetStart
for (var i = sourceStartIndex; i < sourceStartIndex + length; ++i, j++)
{
JsValue? sourceValue;
if (i < (uint) sourceDense.Length && sourceDense[i] != null)
if (i < (uint) sourceDense.Length && sourceDense[i] is not null)
{
sourceValue = sourceDense[i];
}
Expand Down
7 changes: 6 additions & 1 deletion Jint/Native/Array/ArrayPrototype.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1633,6 +1633,11 @@ public JsValue Push(JsValue thisObject, JsValue[] arguments)

public JsValue Pop(JsValue thisObject, JsValue[] arguments)
{
if (thisObject is JsArray { CanUseFastAccess: true } array)
{
return array.Pop();
}

var o = ArrayOperations.For(_realm, thisObject);
ulong len = o.GetLongLength();
if (len == 0)
Expand All @@ -1641,7 +1646,7 @@ public JsValue Pop(JsValue thisObject, JsValue[] arguments)
return Undefined;
}

len = len - 1;
len -= 1;
JsValue element = o.Get(len);
o.DeletePropertyOrThrow(len);
o.SetLength(len);
Expand Down