Skip to content

Commit

Permalink
Optimize Array.pop (#1680)
Browse files Browse the repository at this point in the history
  • Loading branch information
@lahma
lahma authored Nov 12, 2023
1 parent 675ad49 commit 4bff351
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 34 deletions.
2 changes: 1 addition & 1 deletion Jint/Engine.cs
View file
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
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 IEnumerable<KeyValuePair<JsValue, PropertyDescriptor>> Ge
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
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

0 comments on commit 4bff351

Please sign in to comment.