Skip to content
Permalink
Browse files
Expand the set of objects we take JSArray::fastSlice() path for
https://bugs.webkit.org/show_bug.cgi?id=234539

Patch by Alexey Shvayka <ashvayka@apple.com> on 2022-01-07
Reviewed by Yusuke Suzuki.

JSTests:

* microbenchmarks/array-slice-call-cloned-arguments.js: Added.
* stress/array-slice-beyond-length.js: Added.
* stress/array-slice-length-lookup.js: Added.

Source/JavaScriptCore:

Currently, Array.prototype's slice() / splice() methods take a fast path only for
JSArray source objects. With this change, gcSafeMemcpy-based path is taken for any
object with ordinary getOwnPropertySlotByIndex() method, which speeds up the common
case of `[].slice.call(arguments)` by 140% (in strict mode only, see ClonedArguments).

Also, once is https://webkit.org/b/234538 resolved, calling Array.prototype.slice()
on a static NodeList, which is a common idiom to acquire map() / filter() methods,
will become faster as well.

This patch was thoroughly evaluated to be spec-perfect and memory-safe:

  - indexing mode check and holesMustForwardToPrototype() guarantee that there
    are no observable userland code to be invoked;
  - fastSlice() signature is upgraded to uint64_t so `nullptr` is returned in case
    of large "length", resulting in a RangeError being thrown on the slow path;
  - to handle the case of source array being shrinked after "length" lookup (see r175420),
    OOB read check is moved to JSArray::fastSlice() and refined to rely on vectorLength()
    so the double "length" lookup is avoided (added a test for this).

All this (and more) is well covered by the test262 suite.

This change improves Speedometer2/EmberJS-Debug-TodoMVC score by 0.5%: although the test
is slow on its own, `[].slice.call(arguments)` is performed ~56k times per run.

* runtime/ArrayPrototype.cpp:
(JSC::JSC_DEFINE_HOST_FUNCTION):
* runtime/JSArray.cpp:
(JSC::JSArray::fastSlice):
* runtime/JSArray.h:


Canonical link: https://commits.webkit.org/245853@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@287800 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
Alexey Shvayka authored and webkit-commit-queue committed Jan 8, 2022
1 parent 85bcf54 commit ce32bc67ad9219feff7cc3540988f46bf534902b
Showing 8 changed files with 136 additions and 16 deletions.
@@ -1,3 +1,14 @@
2022-01-07 Alexey Shvayka <ashvayka@apple.com>

Expand the set of objects we take JSArray::fastSlice() path for
https://bugs.webkit.org/show_bug.cgi?id=234539

Reviewed by Yusuke Suzuki.

* microbenchmarks/array-slice-call-cloned-arguments.js: Added.
* stress/array-slice-beyond-length.js: Added.
* stress/array-slice-length-lookup.js: Added.

2022-01-06 Saam Barati <sbarati@apple.com>

preparePatchpointForExceptions needs to handle tuples
@@ -0,0 +1,14 @@
(function() {
"use strict";
var TEST_LENGTH = 10;

var slice = Array.prototype.slice;
var clonedArguments = (function() { return arguments; }).apply(null, new Array(TEST_LENGTH).fill(1));

var slicedArray;
for (var i = 0; i < 1e6; i++)
slicedArray = slice.call(clonedArguments);

if (slicedArray.length !== TEST_LENGTH)
throw new Error("Bad assertion!");
})();
@@ -0,0 +1,30 @@
function shouldBe(actual, expected) {
if (actual !== expected)
throw new Error(`Bad value: ${actual}!`);
}

(function sourceIsJSArray() {
for (var i = 0; i < 10_000; i++) {
var sourceObj = [0, 1, 2];
var slicedArr = sourceObj.slice(0, 1000);

shouldBe(slicedArr.length, 3);
shouldBe(slicedArr.join(), "0,1,2");
}
})();

const MAX_ARRAY_LENGTH = 2 ** 32 - 1;

(function sourceIsFinalObject() {
for (var i = 0; i < 10_000; i++) {
var sourceObj = {};
sourceObj[0] = "x";
sourceObj[MAX_ARRAY_LENGTH] = "y";
sourceObj.length = MAX_ARRAY_LENGTH + 1;
sourceObj.slice = Array.prototype.slice;
var slicedArr = sourceObj.slice(MAX_ARRAY_LENGTH, MAX_ARRAY_LENGTH + 2);

shouldBe(slicedArr.length, 1);
shouldBe(slicedArr[0], "y");
}
})();
@@ -0,0 +1,23 @@
function shouldBe(actual, expected) {
if (actual !== expected)
throw new Error(`Bad value: ${actual}!`);
}

(function sourceIsFinalObject() {
var lengthLookups = 0;
var sourceObj = {
0: 0, 1: 1, 2: 2,
get length() {
lengthLookups++;
return 3;
},
};

var slicedArr;
for (var i = 0; i < 1e6; i++) {
slicedArr = [].slice.call(sourceObj);
}

shouldBe(slicedArr.length, 3);
shouldBe(lengthLookups, 1e6);
})();
@@ -1,3 +1,40 @@
2022-01-07 Alexey Shvayka <ashvayka@apple.com>

Expand the set of objects we take JSArray::fastSlice() path for
https://bugs.webkit.org/show_bug.cgi?id=234539

Reviewed by Yusuke Suzuki.

Currently, Array.prototype's slice() / splice() methods take a fast path only for
JSArray source objects. With this change, gcSafeMemcpy-based path is taken for any
object with ordinary getOwnPropertySlotByIndex() method, which speeds up the common
case of `[].slice.call(arguments)` by 140% (in strict mode only, see ClonedArguments).

Also, once is https://webkit.org/b/234538 resolved, calling Array.prototype.slice()
on a static NodeList, which is a common idiom to acquire map() / filter() methods,
will become faster as well.

This patch was thoroughly evaluated to be spec-perfect and memory-safe:

- indexing mode check and holesMustForwardToPrototype() guarantee that there
are no observable userland code to be invoked;
- fastSlice() signature is upgraded to uint64_t so `nullptr` is returned in case
of large "length", resulting in a RangeError being thrown on the slow path;
- to handle the case of source array being shrinked after "length" lookup (see r175420),
OOB read check is moved to JSArray::fastSlice() and refined to rely on vectorLength()
so the double "length" lookup is avoided (added a test for this).

All this (and more) is well covered by the test262 suite.

This change improves Speedometer2/EmberJS-Debug-TodoMVC score by 0.5%: although the test
is slow on its own, `[].slice.call(arguments)` is performed ~56k times per run.

* runtime/ArrayPrototype.cpp:
(JSC::JSC_DEFINE_HOST_FUNCTION):
* runtime/JSArray.cpp:
(JSC::JSArray::fastSlice):
* runtime/JSArray.h:

2022-01-07 Tim Horton <timothy_horton@apple.com>

Adopt linkedOnOrAfter() in more places
@@ -1139,10 +1139,8 @@ JSC_DEFINE_HOST_FUNCTION(arrayProtoFuncSlice, (JSGlobalObject* globalObject, Cal
if (UNLIKELY(speciesResult.first == SpeciesConstructResult::Exception))
return { };

bool okToDoFastPath = speciesResult.first == SpeciesConstructResult::FastPath && isJSArray(thisObj) && length == toLength(globalObject, thisObj);
RETURN_IF_EXCEPTION(scope, { });
if (LIKELY(okToDoFastPath)) {
if (JSArray* result = asArray(thisObj)->fastSlice(globalObject, static_cast<uint32_t>(begin), static_cast<uint32_t>(end - begin)))
if (LIKELY(speciesResult.first == SpeciesConstructResult::FastPath)) {
if (JSArray* result = JSArray::fastSlice(globalObject, thisObj, begin, end - begin))
return JSValue::encode(result);
}

@@ -1235,10 +1233,8 @@ JSC_DEFINE_HOST_FUNCTION(arrayProtoFuncSplice, (JSGlobalObject* globalObject, Ca
return JSValue::encode(jsUndefined());

JSObject* result = nullptr;
bool okToDoFastPath = speciesResult.first == SpeciesConstructResult::FastPath && isJSArray(thisObj) && length == toLength(globalObject, thisObj);
RETURN_IF_EXCEPTION(scope, encodedJSValue());
if (LIKELY(okToDoFastPath))
result = asArray(thisObj)->fastSlice(globalObject, static_cast<uint32_t>(actualStart), static_cast<uint32_t>(actualDeleteCount));
if (LIKELY(speciesResult.first == SpeciesConstructResult::FastPath))
result = JSArray::fastSlice(globalObject, thisObj, actualStart, actualDeleteCount);

if (!result) {
if (speciesResult.first == SpeciesConstructResult::CreatedObject)
@@ -725,18 +725,27 @@ NEVER_INLINE void JSArray::push(JSGlobalObject* globalObject, JSValue value)
pushInline(globalObject, value);
}

JSArray* JSArray::fastSlice(JSGlobalObject* globalObject, unsigned startIndex, unsigned count)
JSArray* JSArray::fastSlice(JSGlobalObject* globalObject, JSObject* source, uint64_t startIndex, uint64_t count)
{
VM& vm = globalObject->vm();

ensureWritable(vm);
// FIXME: Avoid converting the source from CoW since we aren't modifying it.
// https://bugs.webkit.org/show_bug.cgi?id=234990
source->ensureWritable(vm);

Structure* sourceStructure = source->structure(vm);
if (sourceStructure->typeInfo().interceptsGetOwnPropertySlotByIndexEvenWhenLengthIsNotZero())
return nullptr;

auto arrayType = indexingMode();
auto arrayType = source->indexingMode() | IsArray;
switch (arrayType) {
case ArrayWithDouble:
case ArrayWithInt32:
case ArrayWithContiguous: {
if (count >= MIN_SPARSE_ARRAY_INDEX || structure(vm)->holesMustForwardToPrototype(vm, this))
if (count >= MIN_SPARSE_ARRAY_INDEX || sourceStructure->holesMustForwardToPrototype(vm, source))
return nullptr;

if (startIndex + count > source->butterfly()->vectorLength())
return nullptr;

Structure* resultStructure = globalObject->arrayStructureForIndexingTypeDuringAllocation(arrayType);
@@ -745,15 +754,15 @@ JSArray* JSArray::fastSlice(JSGlobalObject* globalObject, unsigned startIndex, u

ASSERT(!globalObject->isHavingABadTime());
ObjectInitializationScope scope(vm);
JSArray* resultArray = JSArray::tryCreateUninitializedRestricted(scope, resultStructure, count);
JSArray* resultArray = JSArray::tryCreateUninitializedRestricted(scope, resultStructure, static_cast<uint32_t>(count));
if (UNLIKELY(!resultArray))
return nullptr;

auto& resultButterfly = *resultArray->butterfly();
if (arrayType == ArrayWithDouble)
gcSafeMemcpy(resultButterfly.contiguousDouble().data(), butterfly()->contiguousDouble().data() + startIndex, sizeof(JSValue) * count);
gcSafeMemcpy(resultButterfly.contiguousDouble().data(), source->butterfly()->contiguousDouble().data() + startIndex, sizeof(JSValue) * static_cast<uint32_t>(count));
else
gcSafeMemcpy(resultButterfly.contiguous().data(), butterfly()->contiguous().data() + startIndex, sizeof(JSValue) * count);
gcSafeMemcpy(resultButterfly.contiguous().data(), source->butterfly()->contiguous().data() + startIndex, sizeof(JSValue) * static_cast<uint32_t>(count));

ASSERT(resultButterfly.publicLength() == count);
return resultArray;
@@ -104,7 +104,7 @@ class JSArray : public JSNonFinalObject {
JS_EXPORT_PRIVATE void push(JSGlobalObject*, JSValue);
JS_EXPORT_PRIVATE JSValue pop(JSGlobalObject*);

JSArray* fastSlice(JSGlobalObject*, unsigned startIndex, unsigned count);
static JSArray* fastSlice(JSGlobalObject*, JSObject* source, uint64_t startIndex, uint64_t count);

bool canFastCopy(VM&, JSArray* otherArray);
bool canDoFastIndexedAccess(VM&);

0 comments on commit ce32bc6

Please sign in to comment.