Changeset 222563 in webkit for trunk/Source/JavaScriptCore/runtime/JSArray.cpp

Timestamp:

Sep 27, 2017, 11:37:41 AM (8 years ago)

Author:

Yusuke Suzuki

Message:

[DFG] Support ArrayPush with multiple args
​https://bugs.webkit.org/show_bug.cgi?id=175823

Reviewed by Saam Barati.

JSTests:

• microbenchmarks/array-push-0.js: Added.

(arrayPush0):

• microbenchmarks/array-push-1.js: Added.

(arrayPush1):

• microbenchmarks/array-push-2.js: Added.

(arrayPush2):

• microbenchmarks/array-push-3.js: Added.

(arrayPush3):

• stress/array-push-multiple-contiguous.js: Added.

(shouldBe):
(test):

• stress/array-push-multiple-double-nan.js: Added.

(shouldBe):
(test):

• stress/array-push-multiple-double.js: Added.

(shouldBe):
(test):

• stress/array-push-multiple-int32.js: Added.

(shouldBe):
(test):

• stress/array-push-multiple-many-contiguous.js: Added.

(shouldBe):
(test):

• stress/array-push-multiple-many-double.js: Added.

(shouldBe):
(test):

• stress/array-push-multiple-many-int32.js: Added.

(shouldBe):
(test):

• stress/array-push-multiple-many-storage.js: Added.

(shouldBe):
(test):

• stress/array-push-multiple-storage.js: Added.

(shouldBe):
(test):

Source/JavaScriptCore:

This patch implements ArrayPush(with multiple arguments) in DFG and FTL. Previously, they are not handled
by ArrayPush. Then they go to generic direct call to Array#push and it does in slow path. This patch
extends ArrayPush to push multiple arguments in a bulk push manner.

The problem of ArrayPush is that we need to perform ArrayPush atomically: If OSR exit occurs in the middle
of ArrayPush, we incorrectly push pushed elements twice. Once we start pushing values, we should not exit.
But we do not want to iterate elements twice, once for type checks and once for actually pushing it. It
could move elements between registers and memory back and forth.

This patch achieves the above goal by separating type checks from ArrayPush. When starting ArrayPush, type
checks for elements are already done by separately emitted Check nodes.

We also add JSArray::pushInline for DFG operations just calling JSArray::push. And we also use it in
arrayProtoFuncPush's fast path.

This patch significantly improves performance of push(multiple args).

baseline patched

Microbenchmarks:

array-push-0 461.8455+-28.9995 ^{151.3438+-6.5653} definitely 3.0516x faster
array-push-1 133.8845+-7.0349 ? 136.1775+-5.8327 ? might be 1.0171x slower
array-push-2 675.6555+-13.4645 ^{145.8747+-6.4621} definitely 4.6318x faster
array-push-3 849.5284+-15.2540 ^{253.4421+-9.1249} definitely 3.3520x faster

baseline patched

SixSpeed:

spread-literal.es5 90.3482+-6.6514 ^{24.8123+-2.3304} definitely 3.6413x faster

• dfg/DFGByteCodeParser.cpp:

(JSC::DFG::ByteCodeParser::handleIntrinsicCall):

• dfg/DFGFixupPhase.cpp:

(JSC::DFG::FixupPhase::fixupNode):

• dfg/DFGNodeType.h:
• dfg/DFGOperations.cpp:
• dfg/DFGOperations.h:
• dfg/DFGSpeculativeJIT.cpp:

(JSC::DFG::SpeculativeJIT::compileArrayPush):

• dfg/DFGSpeculativeJIT.h:

(JSC::DFG::SpeculativeJIT::callOperation):

• dfg/DFGSpeculativeJIT32_64.cpp:

(JSC::DFG::SpeculativeJIT::compile):

• dfg/DFGSpeculativeJIT64.cpp:

(JSC::DFG::SpeculativeJIT::compile):

• dfg/DFGStoreBarrierInsertionPhase.cpp:
• ftl/FTLLowerDFGToB3.cpp:

(JSC::FTL::DFG::LowerDFGToB3::compileArrayPush):

• jit/JITOperations.h:
• runtime/ArrayPrototype.cpp:

(JSC::arrayProtoFuncPush):

• runtime/JSArray.cpp:

(JSC::JSArray::push):

• runtime/JSArray.h:
• runtime/JSArrayInlines.h:

(JSC::JSArray::pushInline):

File:

• trunk/Source/JavaScriptCore/runtime/JSArray.cpp (modified) (2 diffs)

trunk/Source/JavaScriptCore/runtime/JSArray.cpp

@@ -41 +41 @@
 namespace JSC {
 
-static const char* const LengthExceededTheMaximumArrayLengthError = "Length exceeded the maximum array length";
+const char* const LengthExceededTheMaximumArrayLengthError = "Length exceeded the maximum array length";
 
 STATIC_ASSERT_IS_TRIVIALLY_DESTRUCTIBLE(JSArray);
@@ -726 +726 @@
 //  - we always are writing beyond the current array bounds, so it is always necessary to update m_length & m_numValuesInVector.
 //  - pushing to an array of length 2^32-1 stores the property, but throws a range error.
-void JSArray::push(ExecState* exec, JSValue value)
-{
-    VM& vm = exec->vm();
-    auto scope = DECLARE_THROW_SCOPE(vm);
-
-    Butterfly* butterfly = m_butterfly.getMayBeNull();
-    
-    switch (indexingType()) {
-    case ArrayClass: {
-        createInitialUndecided(vm, 0);
-        FALLTHROUGH;
-    }
-        
-    case ArrayWithUndecided: {
-        convertUndecidedForValue(vm, value);
-        scope.release();
-        push(exec, value);
-        return;
-    }
-        
-    case ArrayWithInt32: {
-        if (!value.isInt32()) {
-            convertInt32ForValue(vm, value);
-            scope.release();
-            push(exec, value);
-            return;
-        }
-
-        unsigned length = butterfly->publicLength();
-        ASSERT(length <= butterfly->vectorLength());
-        if (length < butterfly->vectorLength()) {
-            butterfly->contiguousInt32()[length].setWithoutWriteBarrier(value);
-            butterfly->setPublicLength(length + 1);
-            return;
-        }
-        
-        if (UNLIKELY(length > MAX_ARRAY_INDEX)) {
-            methodTable(vm)->putByIndex(this, exec, length, value, true);
-            if (!scope.exception())
-                throwException(exec, scope, createRangeError(exec, ASCIILiteral(LengthExceededTheMaximumArrayLengthError)));
-            return;
-        }
-
-        scope.release();
-        putByIndexBeyondVectorLengthWithoutAttributes<Int32Shape>(exec, length, value);
-        return;
-    }
-
-    case ArrayWithContiguous: {
-        unsigned length = butterfly->publicLength();
-        ASSERT(length <= butterfly->vectorLength());
-        if (length < butterfly->vectorLength()) {
-            butterfly->contiguous()[length].set(vm, this, value);
-            butterfly->setPublicLength(length + 1);
-            return;
-        }
-        
-        if (UNLIKELY(length > MAX_ARRAY_INDEX)) {
-            methodTable(vm)->putByIndex(this, exec, length, value, true);
-            if (!scope.exception())
-                throwException(exec, scope, createRangeError(exec, ASCIILiteral(LengthExceededTheMaximumArrayLengthError)));
-            return;
-        }
-
-        scope.release();
-        putByIndexBeyondVectorLengthWithoutAttributes<ContiguousShape>(exec, length, value);
-        return;
-    }
-        
-    case ArrayWithDouble: {
-        if (!value.isNumber()) {
-            convertDoubleToContiguous(vm);
-            scope.release();
-            push(exec, value);
-            return;
-        }
-        double valueAsDouble = value.asNumber();
-        if (valueAsDouble != valueAsDouble) {
-            convertDoubleToContiguous(vm);
-            scope.release();
-            push(exec, value);
-            return;
-        }
-
-        unsigned length = butterfly->publicLength();
-        ASSERT(length <= butterfly->vectorLength());
-        if (length < butterfly->vectorLength()) {
-            butterfly->contiguousDouble()[length] = valueAsDouble;
-            butterfly->setPublicLength(length + 1);
-            return;
-        }
-        
-        if (UNLIKELY(length > MAX_ARRAY_INDEX)) {
-            methodTable(vm)->putByIndex(this, exec, length, value, true);
-            if (!scope.exception())
-                throwException(exec, scope, createRangeError(exec, ASCIILiteral(LengthExceededTheMaximumArrayLengthError)));
-            return;
-        }
-
-        scope.release();
-        putByIndexBeyondVectorLengthWithoutAttributes<DoubleShape>(exec, length, value);
-        return;
-    }
-        
-    case ArrayWithSlowPutArrayStorage: {
-        unsigned oldLength = length();
-        bool putResult = false;
-        if (attemptToInterceptPutByIndexOnHole(exec, oldLength, value, true, putResult)) {
-            if (!scope.exception() && oldLength < 0xFFFFFFFFu) {
-                scope.release();
-                setLength(exec, oldLength + 1, true);
-            }
-            return;
-        }
-        FALLTHROUGH;
-    }
-        
-    case ArrayWithArrayStorage: {
-        ArrayStorage* storage = butterfly->arrayStorage();
-
-        // Fast case - push within vector, always update m_length & m_numValuesInVector.
-        unsigned length = storage->length();
-        if (length < storage->vectorLength()) {
-            storage->m_vector[length].set(vm, this, value);
-            storage->setLength(length + 1);
-            ++storage->m_numValuesInVector;
-            return;
-        }
-
-        // Pushing to an array of invalid length (2^31-1) stores the property, but throws a range error.
-        if (UNLIKELY(storage->length() > MAX_ARRAY_INDEX)) {
-            methodTable(vm)->putByIndex(this, exec, storage->length(), value, true);
-            // Per ES5.1 15.4.4.7 step 6 & 15.4.5.1 step 3.d.
-            if (!scope.exception())
-                throwException(exec, scope, createRangeError(exec, ASCIILiteral(LengthExceededTheMaximumArrayLengthError)));
-            return;
-        }
-
-        // Handled the same as putIndex.
-        scope.release();
-        putByIndexBeyondVectorLengthWithArrayStorage(exec, storage->length(), value, true, storage);
-        return;
-    }
-        
-    default:
-        RELEASE_ASSERT_NOT_REACHED();
-    }
+NEVER_INLINE void JSArray::push(ExecState* exec, JSValue value)
+{
+    pushInline(exec, value);
 }