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

Remove some unnecessary JsString allocations #1728

Merged
merged 1 commit into from
Jan 6, 2024
Merged

Remove some unnecessary JsString allocations #1728

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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion Jint.Tests/Runtime/FunctionTests.cs
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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