Skip to content

Commit

Permalink
Remove some unnecessary JsString allocations (#1728)
Browse files Browse the repository at this point in the history
  • Loading branch information
lahma authored Jan 6, 2024
1 parent 8d2d986 commit 4b23f03
Show file tree
Hide file tree
Showing 16 changed files with 153 additions and 65 deletions.
1 change: 0 additions & 1 deletion Jint.Tests/Runtime/FunctionTests.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
using Jint.Native;
using Jint.Native.Array;
using Jint.Native.Function;
using Jint.Runtime;
using Jint.Runtime.Interop;
Expand Down
1 change: 0 additions & 1 deletion Jint.Tests/Runtime/RegExpTests.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
using System.Text.RegularExpressions;
using Jint.Native;
using Jint.Native.Array;

namespace Jint.Tests.Runtime;

Expand Down
2 changes: 1 addition & 1 deletion Jint/Native/AggregateError/AggregateErrorConstructor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public override ObjectInstance Construct(JsValue[] arguments, JsValue newTarget)
if (!message.IsUndefined())
{
var msg = TypeConverter.ToString(message);
o.CreateNonEnumerableDataPropertyOrThrow("message", msg);
o.CreateNonEnumerableDataPropertyOrThrow(CommonProperties.Message, msg);
}

o.InstallErrorCause(options);
Expand Down
11 changes: 5 additions & 6 deletions Jint/Native/Array/ArrayConstructor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -82,22 +82,21 @@ private JsValue From(JsValue thisObject, JsValue[] arguments)
return instance;
}

var objectInstance = TypeConverter.ToObject(_realm, items);
if (objectInstance is IObjectWrapper { Target: IEnumerable enumerable })
if (items is IObjectWrapper { Target: IEnumerable enumerable })
{
return ConstructArrayFromIEnumerable(enumerable);
}

return ConstructArrayFromArrayLike(thisObject, objectInstance, callable, thisArg);
var source = ArrayOperations.For(_realm, items, forWrite: false);
return ConstructArrayFromArrayLike(thisObject, source, callable, thisArg);
}

private ObjectInstance ConstructArrayFromArrayLike(
JsValue thisObj,
ObjectInstance objectInstance,
ArrayOperations source,
ICallable? callable,
JsValue thisArg)
{
var source = ArrayOperations.For(objectInstance);
var length = source.GetLength();

ObjectInstance a;
Expand Down Expand Up @@ -311,7 +310,7 @@ private JsArray Construct(JsValue[] arguments, ulong capacity, ObjectInstance pr
break;
case JsArray array:
// direct copy
instance = (JsArray) ConstructArrayFromArrayLike(Undefined, array, null, this);
instance = (JsArray) ConstructArrayFromArrayLike(Undefined, ArrayOperations.For(array), callable: null, this);
break;
default:
instance = ArrayCreate(capacity, prototypeObject);
Expand Down
96 changes: 81 additions & 15 deletions Jint/Native/Array/ArrayOperations.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using System.Collections;
using Jint.Native.Number;
using Jint.Native.Object;
using Jint.Native.String;
using Jint.Native.TypedArray;
using Jint.Runtime;

Expand All @@ -11,25 +12,37 @@ internal abstract class ArrayOperations : IEnumerable<JsValue>
protected internal const ulong MaxArrayLength = 4294967295;
protected internal const ulong MaxArrayLikeLength = NumberConstructor.MaxSafeInteger;

public static ArrayOperations For(Realm realm, JsValue value, bool forWrite)
{
if (!forWrite)
{
if (value.IsString())
{
return new JsStringOperations(realm, (JsString) value);
}

if (value is StringInstance stringInstance)
{
return new JsStringOperations(realm, stringInstance);
}
}

return For(TypeConverter.ToObject(realm, value));
}

public static ArrayOperations For(ObjectInstance instance)
{
if (instance is JsArray { CanUseFastAccess: true } arrayInstance)
{
return new ArrayInstanceOperations(arrayInstance);
return new JsArrayOperations(arrayInstance);
}

if (instance is JsTypedArray typedArrayInstance)
{
return new TypedArrayInstanceOperations(typedArrayInstance);
return new JsTypedArrayOperations(typedArrayInstance);
}

return new ObjectInstanceOperations(instance);
}

public static ArrayOperations For(Realm realm, JsValue thisObj)
{
var instance = TypeConverter.ToObject(realm, thisObj);
return For(instance);
return new ObjectOperations(instance);
}

public abstract ObjectInstance Target { get; }
Expand Down Expand Up @@ -135,9 +148,9 @@ public void Reset()
}
}

private sealed class ObjectInstanceOperations : ArrayOperations<ObjectInstance>
private sealed class ObjectOperations : ArrayOperations<ObjectInstance>
{
public ObjectInstanceOperations(ObjectInstance target) : base(target)
public ObjectOperations(ObjectInstance target) : base(target)
{
}

Expand Down Expand Up @@ -204,9 +217,9 @@ public override void DeletePropertyOrThrow(ulong index)
public override bool HasProperty(ulong index) => Target.HasProperty(index);
}

private sealed class ArrayInstanceOperations : ArrayOperations<JsArray>
private sealed class JsArrayOperations : ArrayOperations<JsArray>
{
public ArrayInstanceOperations(JsArray target) : base(target)
public JsArrayOperations(JsArray target) : base(target)
{
}

Expand Down Expand Up @@ -274,11 +287,11 @@ public override void Set(ulong index, JsValue value, bool updateLength = false,
public override bool HasProperty(ulong index) => _target.HasProperty(index);
}

private sealed class TypedArrayInstanceOperations : ArrayOperations
private sealed class JsTypedArrayOperations : ArrayOperations
{
private readonly JsTypedArray _target;

public TypedArrayInstanceOperations(JsTypedArray target)
public JsTypedArrayOperations(JsTypedArray target)
{
_target = target;
}
Expand Down Expand Up @@ -339,6 +352,59 @@ public override void DeletePropertyOrThrow(ulong index)
public override bool HasProperty(ulong index) => _target.HasProperty(index);
}

private sealed class JsStringOperations : ArrayOperations
{
private readonly Realm _realm;
private readonly JsString _target;
private ObjectInstance? _wrappedTarget;

public JsStringOperations(Realm realm, JsString target)
{
_realm = realm;
_target = target;
}

public JsStringOperations(Realm realm, StringInstance stringInstance) : this(realm, stringInstance.StringData)
{
_wrappedTarget = stringInstance;
}

public override ObjectInstance Target => _wrappedTarget ??= _realm.Intrinsics.String.Construct(_target);

public override ulong GetSmallestIndex(ulong length) => 0;

public override uint GetLength() => (uint) _target.Length;

public override ulong GetLongLength() => GetLength();

public override void SetLength(ulong length) => throw new NotSupportedException();

public override void EnsureCapacity(ulong capacity)
{
}

public override JsValue Get(ulong index) => index < (ulong) _target.Length ? _target[(int) index] : JsValue.Undefined;

public override bool TryGetValue(ulong index, out JsValue value)
{
if (index < (ulong) _target.Length)
{
value = _target[(int) index];
return true;
}

value = JsValue.Undefined;
return false;
}

public override bool HasProperty(ulong index) => index < (ulong) _target.Length;

public override void CreateDataPropertyOrThrow(ulong index, JsValue value) => throw new NotSupportedException();

public override void Set(ulong index, JsValue value, bool updateLength = false, bool throwOnError = true) => throw new NotSupportedException();

public override void DeletePropertyOrThrow(ulong index) => throw new NotSupportedException();
}
}

/// <summary>
Expand Down
36 changes: 18 additions & 18 deletions Jint/Native/Array/ArrayPrototype.cs
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ private JsValue CopyWithin(JsValue thisObject, JsValue[] arguments)
/// </summary>
private JsValue LastIndexOf(JsValue thisObject, JsValue[] arguments)
{
var o = ArrayOperations.For(_realm, thisObject);
var o = ArrayOperations.For(_realm, thisObject, forWrite: false);
var len = o.GetLongLength();
if (len == 0)
{
Expand Down Expand Up @@ -379,7 +379,7 @@ private JsValue Reduce(JsValue thisObject, JsValue[] arguments)
var callbackfn = arguments.At(0);
var initialValue = arguments.At(1);

var o = ArrayOperations.For(_realm, thisObject);
var o = ArrayOperations.For(_realm, thisObject, forWrite: false);
var len = o.GetLength();

var callable = GetCallable(callbackfn);
Expand Down Expand Up @@ -441,7 +441,7 @@ private JsValue Filter(JsValue thisObject, JsValue[] arguments)
var callbackfn = arguments.At(0);
var thisArg = arguments.At(1);

var o = ArrayOperations.For(_realm, thisObject);
var o = ArrayOperations.For(_realm, thisObject, forWrite: false);
var len = o.GetLength();

var callable = GetCallable(callbackfn);
Expand Down Expand Up @@ -484,7 +484,7 @@ private JsValue Map(JsValue thisObject, JsValue[] arguments)
return arrayInstance.Map(arguments);
}

var o = ArrayOperations.For(_realm, thisObject);
var o = ArrayOperations.For(_realm, thisObject, forWrite: false);
var len = o.GetLongLength();

if (len > ArrayOperations.MaxArrayLength)
Expand Down Expand Up @@ -639,7 +639,7 @@ private JsValue ForEach(JsValue thisObject, JsValue[] arguments)
var callbackfn = arguments.At(0);
var thisArg = arguments.At(1);

var o = ArrayOperations.For(_realm, thisObject);
var o = ArrayOperations.For(_realm, thisObject, forWrite: false);
var len = o.GetLength();

var callable = GetCallable(callbackfn);
Expand All @@ -665,7 +665,7 @@ private JsValue ForEach(JsValue thisObject, JsValue[] arguments)
/// </summary>
private JsValue Includes(JsValue thisObject, JsValue[] arguments)
{
var o = ArrayOperations.For(_realm, thisObject);
var o = ArrayOperations.For(_realm, thisObject, forWrite: false);
var len = (long) o.GetLongLength();

if (len == 0)
Expand Down Expand Up @@ -722,7 +722,7 @@ private JsValue Some(JsValue thisObject, JsValue[] arguments)
/// </summary>
private JsValue Every(JsValue thisObject, JsValue[] arguments)
{
var o = ArrayOperations.For(_realm, thisObject);
var o = ArrayOperations.For(_realm, thisObject, forWrite: false);
ulong len = o.GetLongLength();

if (len == 0)
Expand Down Expand Up @@ -759,7 +759,7 @@ private JsValue Every(JsValue thisObject, JsValue[] arguments)
/// </summary>
private JsValue IndexOf(JsValue thisObject, JsValue[] arguments)
{
var o = ArrayOperations.For(_realm, thisObject);
var o = ArrayOperations.For(_realm, thisObject, forWrite: false);
var len = o.GetLongLength();
if (len == 0)
{
Expand Down Expand Up @@ -896,7 +896,7 @@ private JsValue Splice(JsValue thisObject, JsValue[] arguments)
var deleteCount = arguments.At(1);

var obj = TypeConverter.ToObject(_realm, thisObject);
var o = ArrayOperations.For(_realm, obj);
var o = ArrayOperations.For(_realm, obj, forWrite: false);
var len = o.GetLongLength();
var relativeStart = TypeConverter.ToInteger(start);

Expand Down Expand Up @@ -1012,7 +1012,7 @@ private JsValue Splice(JsValue thisObject, JsValue[] arguments)
/// </summary>
private JsValue Unshift(JsValue thisObject, JsValue[] arguments)
{
var o = ArrayOperations.For(_realm, thisObject);
var o = ArrayOperations.For(_realm, thisObject, forWrite: true);
var len = o.GetLongLength();
var argCount = (uint) arguments.Length;

Expand Down Expand Up @@ -1104,7 +1104,7 @@ private JsValue Slice(JsValue thisObject, JsValue[] arguments)
var start = arguments.At(0);
var end = arguments.At(1);

var o = ArrayOperations.For(_realm, thisObject);
var o = ArrayOperations.For(_realm, thisObject, forWrite: false);
var len = o.GetLongLength();

var relativeStart = TypeConverter.ToInteger(start);
Expand Down Expand Up @@ -1164,7 +1164,7 @@ private JsValue Slice(JsValue thisObject, JsValue[] arguments)

private JsValue Shift(JsValue thisObject, JsValue[] arg2)
{
var o = ArrayOperations.For(_realm, thisObject);
var o = ArrayOperations.For(_realm, thisObject, forWrite: true);
var len = o.GetLength();
if (len == 0)
{
Expand Down Expand Up @@ -1197,7 +1197,7 @@ private JsValue Shift(JsValue thisObject, JsValue[] arg2)
/// </summary>
private JsValue Reverse(JsValue thisObject, JsValue[] arguments)
{
var o = ArrayOperations.For(_realm, thisObject);
var o = ArrayOperations.For(_realm, thisObject, forWrite: true);
var len = o.GetLongLength();
var middle = (ulong) System.Math.Floor(len / 2.0);
uint lower = 0;
Expand Down Expand Up @@ -1241,7 +1241,7 @@ private JsValue Reverse(JsValue thisObject, JsValue[] arguments)
private JsValue Join(JsValue thisObject, JsValue[] arguments)
{
var separator = arguments.At(0);
var o = ArrayOperations.For(_realm, thisObject);
var o = ArrayOperations.For(_realm, thisObject, forWrite: false);
var len = o.GetLength();

var sep = TypeConverter.ToString(separator.IsUndefined() ? JsString.CommaString : separator);
Expand Down Expand Up @@ -1281,7 +1281,7 @@ static string StringFromJsValue(JsValue value)

private JsValue ToLocaleString(JsValue thisObject, JsValue[] arguments)
{
var array = ArrayOperations.For(_realm, thisObject);
var array = ArrayOperations.For(_realm, thisObject, forWrite: false);
var len = array.GetLength();
const string Separator = ",";
if (len == 0)
Expand Down Expand Up @@ -1434,7 +1434,7 @@ private JsValue ToSpliced(JsValue thisObject, JsValue[] arguments)
var start = arguments.At(0);
var deleteCount = arguments.At(1);

var o = ArrayOperations.For(_realm, TypeConverter.ToObject(_realm, thisObject));
var o = ArrayOperations.For(_realm, TypeConverter.ToObject(_realm, thisObject), forWrite: false);
var len = o.GetLongLength();
var relativeStart = TypeConverter.ToIntegerOrInfinity(start);

Expand Down Expand Up @@ -1554,7 +1554,7 @@ private JsValue ReduceRight(JsValue thisObject, JsValue[] arguments)
var callbackfn = arguments.At(0);
var initialValue = arguments.At(1);

var o = ArrayOperations.For(TypeConverter.ToObject(_realm, thisObject));
var o = ArrayOperations.For(_realm, thisObject, forWrite: false);
var len = o.GetLongLength();

var callable = GetCallable(callbackfn);
Expand Down Expand Up @@ -1640,7 +1640,7 @@ public JsValue Pop(JsValue thisObject, JsValue[] arguments)
return array.Pop();
}

var o = ArrayOperations.For(_realm, thisObject);
var o = ArrayOperations.For(_realm, thisObject, forWrite: true);
ulong len = o.GetLongLength();
if (len == 0)
{
Expand Down
1 change: 0 additions & 1 deletion Jint/Native/Constructor.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
using Jint.Native.Function;
using Jint.Native.Object;
using Jint.Runtime;

Expand Down
5 changes: 2 additions & 3 deletions Jint/Native/Error/ErrorConstructor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,8 @@ public override ObjectInstance Construct(JsValue[] arguments, JsValue newTarget)
var jsValue = arguments.At(0);
if (!jsValue.IsUndefined())
{
var msg = TypeConverter.ToString(jsValue);
var msgDesc = new PropertyDescriptor(msg, true, false, true);
o.DefinePropertyOrThrow("message", msgDesc);
var msg = TypeConverter.ToJsString(jsValue);
o.CreateNonEnumerableDataPropertyOrThrow(CommonProperties.Message, msg);
}

var stackString = BuildStackString();
Expand Down
Loading

0 comments on commit 4b23f03

Please sign in to comment.