- Revision
- 287800
- Author
- [email protected]
- Date
- 2022-01-07 17:52:18 -0800 (Fri, 07 Jan 2022)
Log Message
Expand the set of objects we take JSArray::fastSlice() path for
https://bugs.webkit.org/show_bug.cgi?id=234539
Patch by Alexey Shvayka <[email protected]> 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/_javascript_Core:
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:
Modified Paths
Added Paths
Diff
Modified: trunk/JSTests/ChangeLog (287799 => 287800)
--- trunk/JSTests/ChangeLog 2022-01-08 01:00:43 UTC (rev 287799)
+++ trunk/JSTests/ChangeLog 2022-01-08 01:52:18 UTC (rev 287800)
@@ -1,3 +1,14 @@
+2022-01-07 Alexey Shvayka <[email protected]>
+
+ 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 <[email protected]>
preparePatchpointForExceptions needs to handle tuples
Added: trunk/JSTests/microbenchmarks/array-slice-call-cloned-arguments.js (0 => 287800)
--- trunk/JSTests/microbenchmarks/array-slice-call-cloned-arguments.js (rev 0)
+++ trunk/JSTests/microbenchmarks/array-slice-call-cloned-arguments.js 2022-01-08 01:52:18 UTC (rev 287800)
@@ -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!");
+})();
Added: trunk/JSTests/stress/array-slice-beyond-length.js (0 => 287800)
--- trunk/JSTests/stress/array-slice-beyond-length.js (rev 0)
+++ trunk/JSTests/stress/array-slice-beyond-length.js 2022-01-08 01:52:18 UTC (rev 287800)
@@ -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");
+ }
+})();
Added: trunk/JSTests/stress/array-slice-length-lookup.js (0 => 287800)
--- trunk/JSTests/stress/array-slice-length-lookup.js (rev 0)
+++ trunk/JSTests/stress/array-slice-length-lookup.js 2022-01-08 01:52:18 UTC (rev 287800)
@@ -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);
+})();
Modified: trunk/Source/_javascript_Core/ChangeLog (287799 => 287800)
--- trunk/Source/_javascript_Core/ChangeLog 2022-01-08 01:00:43 UTC (rev 287799)
+++ trunk/Source/_javascript_Core/ChangeLog 2022-01-08 01:52:18 UTC (rev 287800)
@@ -1,3 +1,40 @@
+2022-01-07 Alexey Shvayka <[email protected]>
+
+ 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 <[email protected]>
Adopt linkedOnOrAfter() in more places
Modified: trunk/Source/_javascript_Core/runtime/ArrayPrototype.cpp (287799 => 287800)
--- trunk/Source/_javascript_Core/runtime/ArrayPrototype.cpp 2022-01-08 01:00:43 UTC (rev 287799)
+++ trunk/Source/_javascript_Core/runtime/ArrayPrototype.cpp 2022-01-08 01:52:18 UTC (rev 287800)
@@ -1139,10 +1139,8 @@
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 @@
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)
Modified: trunk/Source/_javascript_Core/runtime/JSArray.cpp (287799 => 287800)
--- trunk/Source/_javascript_Core/runtime/JSArray.cpp 2022-01-08 01:00:43 UTC (rev 287799)
+++ trunk/Source/_javascript_Core/runtime/JSArray.cpp 2022-01-08 01:52:18 UTC (rev 287800)
@@ -725,20 +725,29 @@
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);
- auto arrayType = indexingMode();
+ Structure* sourceStructure = source->structure(vm);
+ if (sourceStructure->typeInfo().interceptsGetOwnPropertySlotByIndexEvenWhenLengthIsNotZero())
+ return nullptr;
+
+ 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);
if (UNLIKELY(hasAnyArrayStorage(resultStructure->indexingType())))
return nullptr;
@@ -745,15 +754,15 @@
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;
Modified: trunk/Source/_javascript_Core/runtime/JSArray.h (287799 => 287800)
--- trunk/Source/_javascript_Core/runtime/JSArray.h 2022-01-08 01:00:43 UTC (rev 287799)
+++ trunk/Source/_javascript_Core/runtime/JSArray.h 2022-01-08 01:52:18 UTC (rev 287800)
@@ -104,7 +104,7 @@
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&);
