Skip to content
Permalink
Browse files
[JSC] Allow the same array for fast concat
https://bugs.webkit.org/show_bug.cgi?id=247038
rdar://101569531

Reviewed by Alexey Shvayka.

This patch allows the same array used for concat. We added this check in 42bc733,
but this is necessary only for concat's @appendMemcpy path since it modifies |this|. So we should not prohibit using
the fast code in the concat's fast path (@concatMemcpy) since it creates a new array. We also clean up the holesMustForwardToPrototype
implementation to use already-configured WatchpointSet.

                                  ToT                     Patched

    same-array-concat       15.1125+-0.0547     ^      6.7147+-0.0203        ^ definitely 2.2507x faster

* JSTests/microbenchmarks/same-array-concat.js: Added.
(test):
* Source/JavaScriptCore/runtime/ArrayPrototype.cpp:
(JSC::moveElements):
* Source/JavaScriptCore/runtime/JSArray.cpp:
(JSC::JSArray::appendMemcpy):
(JSC::JSArray::shiftCountWithAnyIndexingType):
* Source/JavaScriptCore/runtime/JSArray.h:
* Source/JavaScriptCore/runtime/JSArrayInlines.h:
(JSC::JSArray::holesMustForwardToPrototype const):
(JSC::JSArray::canFastCopy const):
(JSC::JSArray::canFastAppend const):
(JSC::JSArray::canDoFastIndexedAccess const):
(JSC::JSArray::canFastCopy): Deleted.
(JSC::JSArray::canDoFastIndexedAccess): Deleted.

Canonical link: https://commits.webkit.org/256040@main
  • Loading branch information
Constellation committed Oct 26, 2022
1 parent acd5f11 commit 9285fb4a8ca01205fa4043717926c887b6bc51f1
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 14 deletions.
@@ -0,0 +1,14 @@
function test(array1, array2)
{
return array1.concat(array2);
}
noInline(test);

var array0 = [];
var array1 = [0, 1, 2];
var array2 = ["String", 2];
for (var i = 0; i < 1e5; ++i) {
test(array0, array0);
test(array1, array1);
test(array2, array2);
}
@@ -1434,7 +1434,7 @@ static bool moveElements(JSGlobalObject* globalObject, VM& vm, JSArray* target,
{
auto scope = DECLARE_THROW_SCOPE(vm);

if (LIKELY(!hasAnyArrayStorage(source->indexingType()) && !holesMustForwardToPrototype(source))) {
if (LIKELY(!hasAnyArrayStorage(source->indexingType()) && !source->holesMustForwardToPrototype())) {
for (unsigned i = 0; i < sourceLength; ++i) {
JSValue value = source->tryGetIndexQuickly(i);
if (value) {
@@ -483,7 +483,7 @@ bool JSArray::appendMemcpy(JSGlobalObject* globalObject, VM& vm, unsigned startI
{
auto scope = DECLARE_THROW_SCOPE(vm);

if (!canFastCopy(otherArray))
if (!canFastAppend(otherArray))
return false;

IndexingType type = indexingType();
@@ -926,7 +926,7 @@ bool JSArray::shiftCountWithAnyIndexingType(JSGlobalObject* globalObject, unsign
unsigned end = oldLength - count;
unsigned moveCount = end - startIndex;
if (moveCount) {
if (UNLIKELY(this->structure()->holesMustForwardToPrototype(this))) {
if (UNLIKELY(holesMustForwardToPrototype())) {
for (unsigned i = startIndex; i < end; ++i) {
JSValue v = butterfly->contiguous().at(this, i + count).get();
if (UNLIKELY(!v)) {
@@ -971,7 +971,7 @@ bool JSArray::shiftCountWithAnyIndexingType(JSGlobalObject* globalObject, unsign
unsigned end = oldLength - count;
unsigned moveCount = end - startIndex;
if (moveCount) {
if (UNLIKELY(this->structure()->holesMustForwardToPrototype(this))) {
if (UNLIKELY(holesMustForwardToPrototype())) {
for (unsigned i = startIndex; i < end; ++i) {
double v = butterfly->contiguousDouble().at(this, i + count);
if (UNLIKELY(v != v)) {
@@ -106,8 +106,10 @@ class JSArray : public JSNonFinalObject {

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

bool canFastCopy(JSArray* otherArray);
bool canDoFastIndexedAccess();
bool holesMustForwardToPrototype() const;
bool canFastCopy(JSArray* otherArray) const;
bool canFastAppend(JSArray* otherArray) const;
bool canDoFastIndexedAccess() const;
// This function returns NonArray if the indexing types are not compatable for copying.
IndexingType mergeIndexingTypeForCopying(IndexingType other);
bool appendMemcpy(JSGlobalObject*, VM&, unsigned startIndex, JSArray* otherArray);
@@ -61,21 +61,36 @@ inline IndexingType JSArray::mergeIndexingTypeForCopying(IndexingType other)
return type;
}

inline bool JSArray::canFastCopy(JSArray* otherArray)
ALWAYS_INLINE bool JSArray::holesMustForwardToPrototype() const
{
Structure* structure = this->structure();
if (LIKELY(type() == ArrayType)) {
ASSERT(!structure->mayInterceptIndexedAccesses());
JSGlobalObject* globalObject = structure->globalObject();
if (LIKELY(structure->hasMonoProto() && structure->storedPrototype() == globalObject->arrayPrototype() && globalObject->arrayPrototypeChainIsSane()))
return false;
}
return structure->holesMustForwardToPrototype(const_cast<JSArray*>(this));
}

inline bool JSArray::canFastCopy(JSArray* otherArray) const
{
if (otherArray == this)
return false;
if (hasAnyArrayStorage(indexingType()) || hasAnyArrayStorage(otherArray->indexingType()))
return false;
// FIXME: We should have a watchpoint for indexed properties on Array.prototype and Object.prototype
// instead of walking the prototype chain. https://bugs.webkit.org/show_bug.cgi?id=155592
if (structure()->holesMustForwardToPrototype(this)
|| otherArray->structure()->holesMustForwardToPrototype(otherArray))
if (holesMustForwardToPrototype() || otherArray->holesMustForwardToPrototype())
return false;
return true;
}

inline bool JSArray::canDoFastIndexedAccess()
inline bool JSArray::canFastAppend(JSArray* otherArray) const
{
// Append can modify itself, thus, we cannot do fast-append if |this| and otherArray are the same.
if (otherArray == this)
return false;
return canFastCopy(otherArray);
}

inline bool JSArray::canDoFastIndexedAccess() const
{
JSGlobalObject* globalObject = this->globalObject();
if (!globalObject->arrayPrototypeChainIsSane())

0 comments on commit 9285fb4

Please sign in to comment.