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.

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

[JSC] TypedArray change-array-by-copy should be C++ #2771

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions JSTests/microbenchmarks/typed-array-subarray-hot.js
@@ -0,0 +1,6 @@
var typedArray = new Uint8Array(1024);
typedArray.fill(253);
var output = typedArray.subarray();
for (let i = 0; i < 100000; i++) {
output = output.subarray();
}
6 changes: 6 additions & 0 deletions JSTests/microbenchmarks/typed-array-to-reversed-hot.js
@@ -0,0 +1,6 @@
var typedArray = new Uint8Array(1024);
typedArray.fill(253);
var output = typedArray.toReversed();
for (let i = 0; i < 100000; i++) {
output = output.toReversed();
}
6 changes: 6 additions & 0 deletions JSTests/microbenchmarks/typed-array-to-sorted-hot.js
@@ -0,0 +1,6 @@
var typedArray = new Uint8Array(1024);
typedArray.fill(253);
var output = typedArray.toSorted();
for (let i = 0; i < 100000; i++) {
output = output.toSorted();
}
6 changes: 6 additions & 0 deletions JSTests/microbenchmarks/typed-array-with-hot.js
@@ -0,0 +1,6 @@
var typedArray = new Uint8Array(1024);
typedArray.fill(253);
var output = typedArray.with(0, 1);
for (let i = 0; i < 100000; i++) {
output = output.with(i & 1023, i);
}
6 changes: 3 additions & 3 deletions Source/JavaScriptCore/builtins/ArrayPrototype.js
Expand Up @@ -818,12 +818,12 @@ function toReversed()
return result;
}

function toSorted(comparefn)
function toSorted(comparator)
{
"use strict";

// Step 1.
if (comparefn !== @undefined && !@isCallable(comparefn))
if (comparator !== @undefined && !@isCallable(comparator))
@throwTypeError("Array.prototype.toSorted requires the comparator argument to be a function or undefined");

// Step 2.
Expand All @@ -840,7 +840,7 @@ function toSorted(comparefn)
@putByValDirect(result, k, array[k]);

// Step 6.
@arraySort.@call(result, comparefn);
@arraySort.@call(result, comparator);

return result;
}
Expand Down
2 changes: 1 addition & 1 deletion Source/JavaScriptCore/builtins/BuiltinNames.h
Expand Up @@ -73,10 +73,10 @@ namespace JSC {
macro(Map) \
macro(throwTypeErrorFunction) \
macro(typedArrayLength) \
macro(typedArrayClone) \
macro(typedArrayContentType) \
macro(typedArraySort) \
macro(typedArrayGetOriginalConstructor) \
macro(typedArraySubarrayCreate) \
macro(BuiltinLog) \
macro(BuiltinDescribe) \
macro(homeObject) \
Expand Down
105 changes: 15 additions & 90 deletions Source/JavaScriptCore/builtins/TypedArrayPrototype.js
Expand Up @@ -213,7 +213,6 @@ function typedArrayMergeSort(array, valueCount, comparator)
}
}

@linkTimeConstant
function sort(comparator)
{
"use strict";
Expand All @@ -237,23 +236,6 @@ function sort(comparator)
return this;
}

function subarray(begin, end)
{
"use strict";

if (!@isTypedArrayView(this))
@throwTypeError("|this| should be a typed array view");

var start = @toIntegerOrInfinity(begin);
var finish;
if (end !== @undefined)
finish = @toIntegerOrInfinity(end);

var constructor = @typedArraySpeciesConstructor(this);

return @typedArraySubarrayCreate.@call(this, start, finish, constructor);
}

function reduce(callback /* [, initialValue] */)
{
// 22.2.3.19
Expand Down Expand Up @@ -393,85 +375,28 @@ function at(index)
return (k >= 0 && k < length) ? this[k] : @undefined;
}

function toReversed()
{
"use strict";

// Step 2-3.
var length = @typedArrayLength(this);

// Step 4.
var constructor = @typedArrayGetOriginalConstructor(this);
var result = new constructor(length);

// Step 5-6.
for (var k = 0; k < length; k++) {
var fromValue = this[length - k - 1];
result[k] = fromValue;
}

return result;
}

function toSorted(comparefn)
function toSorted(comparator)
{
"use strict";

// Step 1.
if (comparefn !== @undefined && !@isCallable(comparefn))
@throwTypeError("TypedArray.prototype.toSorted requires the comparefn argument to be a function or undefined");

// Step 5.
var length = @typedArrayLength(this);

// Step 6.
var constructor = @typedArrayGetOriginalConstructor(this);
var result = new constructor(length);

// Step 11.
for (var k = 0; k < length; k++)
result[k] = this[k];

// Step 9.
@sort.@call(result, comparefn);

return result;
}

function with(index, value)
{
"use strict";

// Step 2-3.
var length = @typedArrayLength(this);

// Step 4.
var relativeIndex = @toIntegerOrInfinity(index);
var actualIndex;

// Step 5-6.
if (relativeIndex >= 0)
actualIndex = relativeIndex;
else
actualIndex = length + relativeIndex;
if (comparator !== @undefined && !@isCallable(comparator))
@throwTypeError("TypedArray.prototype.toSorted requires the comparator argument to be a function or undefined");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like ArrayPrototype.js calls the argument comparefn (following spec naming) but says "comparator" in the error message (which is certainly more pleasant). Should we keep this in sync?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we should use comparator.


// Step 7.
if (@isDetached(this))
@throwRangeError("TypedArray.prototype.with called on an detached array");
if (actualIndex < 0 || actualIndex >= length)
@throwRangeError("Array index out of range")
var result = @typedArrayClone.@call(this);

// Step 8.
var constructor = @typedArrayGetOriginalConstructor(this);
var result = new constructor(length);
var length = @typedArrayLength(result);
if (length < 2)
return result;

// Step 9-10.
for (var k = 0; k < length; k++) {
if (k == actualIndex)
result[k] = value;
else
result[k] = this[k];
}
// typedArraySort is not safe when the other thread is modifying content. So if |result| is SharedArrayBuffer,
// use JS-implemented sorting.
if (comparator !== @undefined || @isSharedTypedArrayView(result)) {
if (comparator === @undefined)
comparator = @typedArrayDefaultComparator;
@typedArrayMergeSort(result, length, comparator);
} else
@typedArraySort(result);

return result;
}
2 changes: 1 addition & 1 deletion Source/JavaScriptCore/bytecode/LinkTimeConstant.h
Expand Up @@ -50,14 +50,14 @@ class JSGlobalObject;
v(makeTypeError, nullptr) \
v(AggregateError, nullptr) \
v(typedArrayLength, nullptr) \
v(typedArrayClone, nullptr) \
v(typedArrayContentType, nullptr) \
v(typedArrayGetOriginalConstructor, nullptr) \
v(typedArraySort, nullptr) \
v(isTypedArrayView, nullptr) \
v(isSharedTypedArrayView, nullptr) \
v(isDetached, nullptr) \
v(typedArrayDefaultComparator, nullptr) \
v(typedArraySubarrayCreate, nullptr) \
v(isBoundFunction, nullptr) \
v(hasInstanceBoundFunction, nullptr) \
v(instanceOf, nullptr) \
Expand Down
10 changes: 5 additions & 5 deletions Source/JavaScriptCore/runtime/IntlObject.cpp
Expand Up @@ -1602,7 +1602,7 @@ const Vector<String>& intlAvailableCalendars()
}

// The AvailableCalendars abstract operation returns a List, ordered as if an Array of the same
// values had been sorted using %Array.prototype.sort% using undefined as comparefn
// values had been sorted using %Array.prototype.sort% using undefined as comparator
std::sort(availableCalendars->begin(), availableCalendars->end(),
[](const String& a, const String& b) {
return WTF::codePointCompare(a, b) < 0;
Expand Down Expand Up @@ -1676,7 +1676,7 @@ static JSArray* availableCollations(JSGlobalObject* globalObject)
}

// The AvailableCollations abstract operation returns a List, ordered as if an Array of the same
// values had been sorted using %Array.prototype.sort% using undefined as comparefn
// values had been sorted using %Array.prototype.sort% using undefined as comparator
std::sort(elements.begin(), elements.end(),
[](const String& a, const String& b) {
return WTF::codePointCompare(a, b) < 0;
Expand Down Expand Up @@ -1732,7 +1732,7 @@ static JSArray* availableCurrencies(JSGlobalObject* globalObject)
}

// The AvailableCurrencies abstract operation returns a List, ordered as if an Array of the same
// values had been sorted using %Array.prototype.sort% using undefined as comparefn
// values had been sorted using %Array.prototype.sort% using undefined as comparator
std::sort(elements.begin(), elements.end(),
[](const String& a, const String& b) {
return WTF::codePointCompare(a, b) < 0;
Expand Down Expand Up @@ -1782,7 +1782,7 @@ static JSArray* availableNumberingSystems(JSGlobalObject* globalObject)
}

// The AvailableNumberingSystems abstract operation returns a List, ordered as if an Array of the same
// values had been sorted using %Array.prototype.sort% using undefined as comparefn
// values had been sorted using %Array.prototype.sort% using undefined as comprator
std::sort(elements.begin(), elements.end(),
[](const String& a, const String& b) {
return WTF::codePointCompare(a, b) < 0;
Expand Down Expand Up @@ -1840,7 +1840,7 @@ const Vector<String>& intlAvailableTimeZones()
}

// The AvailableTimeZones abstract operation returns a List, ordered as if an Array of the same
// values had been sorted using %Array.prototype.sort% using undefined as comparefn
// values had been sorted using %Array.prototype.sort% using undefined as comparator
std::sort(temporary.begin(), temporary.end(),
[](const String& a, const String& b) {
return WTF::codePointCompare(a, b) < 0;
Expand Down