Diff
Modified: trunk/JSTests/ChangeLog (222381 => 222382)
--- trunk/JSTests/ChangeLog 2017-09-22 09:51:38 UTC (rev 222381)
+++ trunk/JSTests/ChangeLog 2017-09-22 10:25:58 UTC (rev 222382)
@@ -1,3 +1,17 @@
+2017-09-22 Commit Queue <[email protected]>
+
+ Unreviewed, rolling out r222380.
+ https://bugs.webkit.org/show_bug.cgi?id=177352
+
+ Octane/box2d shows 8% regression (Requested by yusukesuzuki on
+ #webkit).
+
+ Reverted changeset:
+
+ "[DFG][FTL] Profile array vector length for array allocation"
+ https://bugs.webkit.org/show_bug.cgi?id=177051
+ http://trac.webkit.org/changeset/222380
+
2017-09-21 Yusuke Suzuki <[email protected]>
[DFG][FTL] Profile array vector length for array allocation
Deleted: trunk/JSTests/microbenchmarks/new-array-buffer-vector-profile.js (222381 => 222382)
--- trunk/JSTests/microbenchmarks/new-array-buffer-vector-profile.js 2017-09-22 09:51:38 UTC (rev 222381)
+++ trunk/JSTests/microbenchmarks/new-array-buffer-vector-profile.js 2017-09-22 10:25:58 UTC (rev 222382)
@@ -1,12 +0,0 @@
-function target()
-{
- var array = [0];
- array.push(1);
- array.push(2);
- array.push(3);
- array.push(4);
- return array;
-}
-noInline(target);
-for (var i = 0; i < 1e6; ++i)
- target();
Modified: trunk/Source/_javascript_Core/ChangeLog (222381 => 222382)
--- trunk/Source/_javascript_Core/ChangeLog 2017-09-22 09:51:38 UTC (rev 222381)
+++ trunk/Source/_javascript_Core/ChangeLog 2017-09-22 10:25:58 UTC (rev 222382)
@@ -1,3 +1,17 @@
+2017-09-22 Commit Queue <[email protected]>
+
+ Unreviewed, rolling out r222380.
+ https://bugs.webkit.org/show_bug.cgi?id=177352
+
+ Octane/box2d shows 8% regression (Requested by yusukesuzuki on
+ #webkit).
+
+ Reverted changeset:
+
+ "[DFG][FTL] Profile array vector length for array allocation"
+ https://bugs.webkit.org/show_bug.cgi?id=177051
+ http://trac.webkit.org/changeset/222380
+
2017-09-21 Yusuke Suzuki <[email protected]>
[DFG][FTL] Profile array vector length for array allocation
Modified: trunk/Source/_javascript_Core/bytecode/ArrayAllocationProfile.cpp (222381 => 222382)
--- trunk/Source/_javascript_Core/bytecode/ArrayAllocationProfile.cpp 2017-09-22 09:51:38 UTC (rev 222381)
+++ trunk/Source/_javascript_Core/bytecode/ArrayAllocationProfile.cpp 2017-09-22 10:25:58 UTC (rev 222382)
@@ -30,7 +30,7 @@
namespace JSC {
-void ArrayAllocationProfile::updateProfile()
+void ArrayAllocationProfile::updateIndexingType()
{
// This is awkwardly racy but totally sound even when executed concurrently. The
// worst cases go something like this:
@@ -49,11 +49,9 @@
JSArray* lastArray = m_lastArray;
if (!lastArray)
return;
- if (LIKELY(Options::useArrayAllocationProfiling())) {
+ if (LIKELY(Options::useArrayAllocationProfiling()))
m_currentIndexingType = leastUpperBoundOfIndexingTypes(m_currentIndexingType, lastArray->indexingType());
- m_largestSeenVectorLength = std::min(std::max(m_largestSeenVectorLength, lastArray->getVectorLength()), BASE_CONTIGUOUS_VECTOR_LEN_MAX);
- }
- m_lastArray = nullptr;
+ m_lastArray = 0;
}
} // namespace JSC
Modified: trunk/Source/_javascript_Core/bytecode/ArrayAllocationProfile.h (222381 => 222382)
--- trunk/Source/_javascript_Core/bytecode/ArrayAllocationProfile.h 2017-09-22 09:51:38 UTC (rev 222381)
+++ trunk/Source/_javascript_Core/bytecode/ArrayAllocationProfile.h 2017-09-22 10:25:58 UTC (rev 222382)
@@ -32,22 +32,19 @@
class ArrayAllocationProfile {
public:
+ ArrayAllocationProfile()
+ : m_currentIndexingType(ArrayWithUndecided)
+ , m_lastArray(0)
+ {
+ }
+
IndexingType selectIndexingType()
{
JSArray* lastArray = m_lastArray;
if (lastArray && UNLIKELY(lastArray->indexingType() != m_currentIndexingType))
- updateProfile();
+ updateIndexingType();
return m_currentIndexingType;
}
-
- // vector length hint becomes [0, BASE_CONTIGUOUS_VECTOR_LEN_MAX].
- unsigned vectorLengthHint()
- {
- JSArray* lastArray = m_lastArray;
- if (lastArray && (m_largestSeenVectorLength != BASE_CONTIGUOUS_VECTOR_LEN_MAX) && UNLIKELY(lastArray->getVectorLength() > m_largestSeenVectorLength))
- updateProfile();
- return m_largestSeenVectorLength;
- }
JSArray* updateLastAllocation(JSArray* lastArray)
{
@@ -55,7 +52,7 @@
return lastArray;
}
- JS_EXPORT_PRIVATE void updateProfile();
+ JS_EXPORT_PRIVATE void updateIndexingType();
static IndexingType selectIndexingTypeFor(ArrayAllocationProfile* profile)
{
@@ -73,9 +70,8 @@
private:
- IndexingType m_currentIndexingType { ArrayWithUndecided };
- unsigned m_largestSeenVectorLength { 0 };
- JSArray* m_lastArray { nullptr };
+ IndexingType m_currentIndexingType;
+ JSArray* m_lastArray;
};
} // namespace JSC
Modified: trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp (222381 => 222382)
--- trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp 2017-09-22 09:51:38 UTC (rev 222381)
+++ trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp 2017-09-22 10:25:58 UTC (rev 222382)
@@ -2568,7 +2568,7 @@
// Don't count these either, for similar reasons.
for (unsigned i = m_arrayAllocationProfiles.size(); i--;)
- m_arrayAllocationProfiles[i].updateProfile();
+ m_arrayAllocationProfiles[i].updateIndexingType();
}
void CodeBlock::updateAllPredictions()
Modified: trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp (222381 => 222382)
--- trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2017-09-22 09:51:38 UTC (rev 222381)
+++ trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2017-09-22 10:25:58 UTC (rev 222382)
@@ -4399,7 +4399,6 @@
data.startConstant = m_inlineStackTop->m_constantBufferRemap[startConstant];
data.numConstants = numConstants;
data.indexingType = profile->selectIndexingType();
- data.vectorLengthHint = std::max<unsigned>(profile->vectorLengthHint(), numConstants);
// If this statement has never executed, we'll have the wrong indexing type in the profile.
for (int i = 0; i < numConstants; ++i) {
@@ -4409,7 +4408,7 @@
m_codeBlock->constantBuffer(data.startConstant)[i]);
}
- m_graph.m_newArrayBufferData.append(WTFMove(data));
+ m_graph.m_newArrayBufferData.append(data);
set(VirtualRegister(currentInstruction[1].u.operand), addToGraph(NewArrayBuffer, OpInfo(&m_graph.m_newArrayBufferData.last())));
NEXT_OPCODE(op_new_array_buffer);
}
Modified: trunk/Source/_javascript_Core/dfg/DFGGraph.cpp (222381 => 222382)
--- trunk/Source/_javascript_Core/dfg/DFGGraph.cpp 2017-09-22 09:51:38 UTC (rev 222381)
+++ trunk/Source/_javascript_Core/dfg/DFGGraph.cpp 2017-09-22 10:25:58 UTC (rev 222382)
@@ -321,7 +321,6 @@
for (unsigned i = 0; i < node->numConstants(); ++i)
out.print(anotherComma, pointerDumpInContext(freeze(m_codeBlock->constantBuffer(node->startConstant())[i]), context));
out.print("]");
- out.print(comma, "vectorLengthHint = ", node->vectorLengthHint());
}
if (node->hasLazyJSValue())
out.print(comma, node->lazyJSValue());
Modified: trunk/Source/_javascript_Core/dfg/DFGNode.h (222381 => 222382)
--- trunk/Source/_javascript_Core/dfg/DFGNode.h 2017-09-22 09:51:38 UTC (rev 222381)
+++ trunk/Source/_javascript_Core/dfg/DFGNode.h 2017-09-22 10:25:58 UTC (rev 222382)
@@ -99,7 +99,6 @@
struct NewArrayBufferData {
unsigned startConstant;
unsigned numConstants;
- unsigned vectorLengthHint;
IndexingType indexingType;
};
@@ -1116,11 +1115,6 @@
{
return newArrayBufferData()->numConstants;
}
-
- unsigned vectorLengthHint()
- {
- return newArrayBufferData()->vectorLengthHint;
- }
bool hasIndexingType()
{
Modified: trunk/Source/_javascript_Core/dfg/DFGOperations.cpp (222381 => 222382)
--- trunk/Source/_javascript_Core/dfg/DFGOperations.cpp 2017-09-22 09:51:38 UTC (rev 222381)
+++ trunk/Source/_javascript_Core/dfg/DFGOperations.cpp 2017-09-22 10:25:58 UTC (rev 222382)
@@ -1311,25 +1311,6 @@
return bitwise_cast<char*>(result);
}
-char* JIT_OPERATION operationNewArrayWithSizeAndHint(ExecState* exec, Structure* arrayStructure, int32_t size, int32_t vectorLengthHint, Butterfly* butterfly)
-{
- VM& vm = exec->vm();
- NativeCallFrameTracer tracer(&vm, exec);
- auto scope = DECLARE_THROW_SCOPE(vm);
-
- if (UNLIKELY(size < 0))
- return bitwise_cast<char*>(throwException(exec, scope, createRangeError(exec, ASCIILiteral("Array size is not a small enough positive integer."))));
-
- JSArray* result;
- if (butterfly)
- result = JSArray::createWithButterfly(vm, nullptr, arrayStructure, butterfly);
- else {
- result = JSArray::tryCreate(vm, arrayStructure, size, vectorLengthHint);
- ASSERT(result);
- }
- return bitwise_cast<char*>(result);
-}
-
char* JIT_OPERATION operationNewArrayBuffer(ExecState* exec, Structure* arrayStructure, size_t start, size_t size)
{
VM& vm = exec->vm();
Modified: trunk/Source/_javascript_Core/dfg/DFGOperations.h (222381 => 222382)
--- trunk/Source/_javascript_Core/dfg/DFGOperations.h 2017-09-22 09:51:38 UTC (rev 222381)
+++ trunk/Source/_javascript_Core/dfg/DFGOperations.h 2017-09-22 10:25:58 UTC (rev 222382)
@@ -81,7 +81,6 @@
char* JIT_OPERATION operationNewArrayBuffer(ExecState*, Structure*, size_t, size_t) WTF_INTERNAL;
char* JIT_OPERATION operationNewEmptyArray(ExecState*, Structure*) WTF_INTERNAL;
char* JIT_OPERATION operationNewArrayWithSize(ExecState*, Structure*, int32_t, Butterfly*) WTF_INTERNAL;
-char* JIT_OPERATION operationNewArrayWithSizeAndHint(ExecState*, Structure*, int32_t, int32_t, Butterfly*) WTF_INTERNAL;
char* JIT_OPERATION operationNewInt8ArrayWithSize(ExecState*, Structure*, int32_t, char*) WTF_INTERNAL;
char* JIT_OPERATION operationNewInt8ArrayWithOneArgument(ExecState*, Structure*, EncodedJSValue) WTF_INTERNAL;
char* JIT_OPERATION operationNewInt16ArrayWithSize(ExecState*, Structure*, int32_t, char*) WTF_INTERNAL;
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp (222381 => 222382)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp 2017-09-22 09:51:38 UTC (rev 222381)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp 2017-09-22 10:25:58 UTC (rev 222382)
@@ -4238,8 +4238,6 @@
IndexingType indexingType = node->indexingType();
if (!globalObject->isHavingABadTime() && !hasAnyArrayStorage(indexingType)) {
unsigned numElements = node->numConstants();
- unsigned vectorLengthHint = node->vectorLengthHint();
- ASSERT(vectorLengthHint >= numElements);
GPRTemporary result(this);
GPRTemporary storage(this);
@@ -4247,7 +4245,7 @@
GPRReg resultGPR = result.gpr();
GPRReg storageGPR = storage.gpr();
- emitAllocateRawObject(resultGPR, m_jit.graph().registerStructure(globalObject->arrayStructureForIndexingTypeDuringAllocation(indexingType)), storageGPR, numElements, vectorLengthHint);
+ emitAllocateRawObject(resultGPR, m_jit.graph().registerStructure(globalObject->arrayStructureForIndexingTypeDuringAllocation(indexingType)), storageGPR, numElements, numElements);
DFG_ASSERT(m_jit.graph(), node, indexingType & IsArray);
JSValue* data = ""
Modified: trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (222381 => 222382)
--- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2017-09-22 09:51:38 UTC (rev 222381)
+++ trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2017-09-22 10:25:58 UTC (rev 222382)
@@ -4189,7 +4189,7 @@
m_out.select(m_out.equal(indexingType, m_out.constInt32(ArrayWithContiguous)),
weakStructure(m_graph.registerStructure(globalObject->arrayStructureForIndexingTypeDuringAllocation(ArrayWithContiguous))),
weakStructure(m_graph.registerStructure(globalObject->arrayStructureForIndexingTypeDuringAllocation(ArrayWithDouble)))));
- arrayResult = allocateJSArray(resultLength, resultLength, structure, indexingType, false, false);
+ arrayResult = allocateJSArray(resultLength, structure, indexingType, false, false);
}
LBasicBlock loop = m_out.newBlock();
@@ -5114,11 +5114,9 @@
if (!globalObject->isHavingABadTime() && !hasAnyArrayStorage(m_node->indexingType())) {
unsigned numElements = m_node->numConstants();
- unsigned vectorLengthHint = m_node->vectorLengthHint();
-
- ASSERT(vectorLengthHint >= numElements);
+
ArrayValues arrayValues =
- allocateUninitializedContiguousJSArray(numElements, vectorLengthHint, structure);
+ allocateUninitializedContiguousJSArray(m_out.constInt32(numElements), structure);
JSValue* data = ""
for (unsigned index = 0; index < m_node->numConstants(); ++index) {
@@ -5157,7 +5155,7 @@
IndexingType indexingType = m_node->indexingType();
setJSValue(
allocateJSArray(
- publicLength, publicLength, weakPointer(globalObject->arrayStructureForIndexingTypeDuringAllocation(indexingType)), m_out.constInt32(indexingType)).array);
+ publicLength, weakPointer(globalObject->arrayStructureForIndexingTypeDuringAllocation(indexingType)), m_out.constInt32(indexingType)).array);
mutatorFence();
return;
}
@@ -11443,7 +11441,7 @@
LValue butterfly;
};
- ArrayValues allocateJSArray(LValue publicLength, LValue vectorLength, LValue structure, LValue indexingType, bool shouldInitializeElements = true, bool shouldLargeArraySizeCreateArrayStorage = true)
+ ArrayValues allocateJSArray(LValue publicLength, LValue structure, LValue indexingType, bool shouldInitializeElements = true, bool shouldLargeArraySizeCreateArrayStorage = true)
{
JSGlobalObject* globalObject = m_graph.globalObjectFor(m_node->origin.semantic);
if (indexingType->hasInt32()) {
@@ -11462,18 +11460,6 @@
LBasicBlock slowCase = m_out.newBlock();
LBasicBlock lastNext = m_out.insertNewBlocksBefore(fastCase);
-
- if (vectorLength->hasInt32() && structure->hasIntPtr()) {
- unsigned vectorLengthConst = static_cast<unsigned>(vectorLength->asInt32());
- if (vectorLengthConst <= MAX_STORAGE_VECTOR_LENGTH) {
- vectorLength = m_out.constInt32(
- Butterfly::optimalContiguousVectorLength(
- bitwise_cast<Structure*>(structure->asIntPtr())->outOfLineCapacity(), vectorLengthConst));
- }
- } else {
- // We don't compute the optimal vector length for new Array(blah) where blah is not
- // statically known, since the compute effort of doing it here is probably not worth it.
- }
ValueFromBlock noButterfly = m_out.anchor(m_out.intPtrZero);
@@ -11486,6 +11472,22 @@
m_out.branch(predicate, rarely(largeCase), usually(fastCase));
m_out.appendTo(fastCase, largeCase);
+
+ LValue vectorLength = nullptr;
+ if (publicLength->hasInt32() && structure->hasIntPtr()) {
+ unsigned publicLengthConst = static_cast<unsigned>(publicLength->asInt32());
+ if (publicLengthConst <= MAX_STORAGE_VECTOR_LENGTH) {
+ vectorLength = m_out.constInt32(
+ Butterfly::optimalContiguousVectorLength(
+ bitwise_cast<Structure*>(structure->asIntPtr())->outOfLineCapacity(), publicLengthConst));
+ }
+ }
+
+ if (!vectorLength) {
+ // We don't compute the optimal vector length for new Array(blah) where blah is not
+ // statically known, since the compute effort of doing it here is probably not worth it.
+ vectorLength = publicLength;
+ }
LValue payloadSize =
m_out.shl(m_out.zeroExt(vectorLength, pointerType()), m_out.constIntPtr(3));
@@ -11531,10 +11533,10 @@
LValue slowResultValue = lazySlowPath(
[=, &vm] (const Vector<Location>& locations) -> RefPtr<LazySlowPath::Generator> {
return createLazyCallGenerator(vm,
- operationNewArrayWithSizeAndHint, locations[0].directGPR(),
- locations[1].directGPR(), locations[2].directGPR(), locations[3].directGPR(), locations[4].directGPR());
+ operationNewArrayWithSize, locations[0].directGPR(),
+ locations[1].directGPR(), locations[2].directGPR(), locations[3].directGPR());
},
- structureValue, publicLength, vectorLength, butterflyValue);
+ structureValue, publicLength, butterflyValue);
ValueFromBlock slowResult = m_out.anchor(slowResultValue);
ValueFromBlock slowButterfly = m_out.anchor(
m_out.loadPtr(slowResultValue, m_heaps.JSObject_butterfly));
@@ -11546,25 +11548,14 @@
m_out.phi(pointerType(), fastButterfly, slowButterfly));
}
- ArrayValues allocateUninitializedContiguousJSArrayInternal(LValue publicLength, LValue vectorLength, RegisteredStructure structure)
+ ArrayValues allocateUninitializedContiguousJSArray(LValue publicLength, RegisteredStructure structure)
{
bool shouldInitializeElements = false;
bool shouldLargeArraySizeCreateArrayStorage = false;
return allocateJSArray(
- publicLength, vectorLength, weakStructure(structure), m_out.constInt32(structure->indexingType()), shouldInitializeElements,
+ publicLength, weakStructure(structure), m_out.constInt32(structure->indexingType()), shouldInitializeElements,
shouldLargeArraySizeCreateArrayStorage);
}
-
- ArrayValues allocateUninitializedContiguousJSArray(LValue publicLength, RegisteredStructure structure)
- {
- return allocateUninitializedContiguousJSArrayInternal(publicLength, publicLength, structure);
- }
-
- ArrayValues allocateUninitializedContiguousJSArray(unsigned publicLength, unsigned vectorLength, RegisteredStructure structure)
- {
- ASSERT(vectorLength >= publicLength);
- return allocateUninitializedContiguousJSArrayInternal(m_out.constInt32(publicLength), m_out.constInt32(vectorLength), structure);
- }
LValue ensureShadowChickenPacket()
{
Modified: trunk/Source/_javascript_Core/runtime/ArrayConventions.h (222381 => 222382)
--- trunk/Source/_javascript_Core/runtime/ArrayConventions.h 2017-09-22 09:51:38 UTC (rev 222381)
+++ trunk/Source/_javascript_Core/runtime/ArrayConventions.h 2017-09-22 10:25:58 UTC (rev 222382)
@@ -77,8 +77,6 @@
// for an array that was created with a sepcified length (e.g. a = new Array(123))
#define BASE_CONTIGUOUS_VECTOR_LEN 3U
#define BASE_CONTIGUOUS_VECTOR_LEN_EMPTY 5U
-#define BASE_CONTIGUOUS_VECTOR_LEN_MIN 3U
-#define BASE_CONTIGUOUS_VECTOR_LEN_MAX 25U
#define BASE_ARRAY_STORAGE_VECTOR_LEN 4U
// The upper bound to the size we'll grow a zero length array when the first element
Modified: trunk/Source/_javascript_Core/runtime/JSArray.h (222381 => 222382)
--- trunk/Source/_javascript_Core/runtime/JSArray.h 2017-09-22 09:51:38 UTC (rev 222381)
+++ trunk/Source/_javascript_Core/runtime/JSArray.h 2017-09-22 10:25:58 UTC (rev 222382)
@@ -54,7 +54,6 @@
public:
static JSArray* tryCreate(VM&, Structure*, unsigned initialLength = 0);
- static JSArray* tryCreate(VM&, Structure*, unsigned initialLength, unsigned vectorLengthHint);
static JSArray* create(VM&, Structure*, unsigned initialLength = 0);
static JSArray* createWithButterfly(VM&, GCDeferralContext*, Structure*, Butterfly*);
@@ -216,9 +215,8 @@
Butterfly* createArrayButterflyInDictionaryIndexingMode(
VM&, JSCell* intendedOwner, unsigned initialLength);
-inline JSArray* JSArray::tryCreate(VM& vm, Structure* structure, unsigned initialLength, unsigned vectorLengthHint)
+inline JSArray* JSArray::tryCreate(VM& vm, Structure* structure, unsigned initialLength)
{
- ASSERT(vectorLengthHint >= initialLength);
unsigned outOfLineStorage = structure->outOfLineCapacity();
Butterfly* butterfly;
@@ -230,10 +228,10 @@
|| hasDouble(indexingType)
|| hasContiguous(indexingType));
- if (UNLIKELY(vectorLengthHint > MAX_STORAGE_VECTOR_LENGTH))
+ if (UNLIKELY(initialLength > MAX_STORAGE_VECTOR_LENGTH))
return nullptr;
- unsigned vectorLength = Butterfly::optimalContiguousVectorLength(structure, vectorLengthHint);
+ unsigned vectorLength = Butterfly::optimalContiguousVectorLength(structure, initialLength);
void* temp = vm.jsValueGigacageAuxiliarySpace.tryAllocate(nullptr, Butterfly::totalSize(0, outOfLineStorage, true, vectorLength * sizeof(EncodedJSValue)));
if (!temp)
return nullptr;
@@ -258,11 +256,6 @@
return createWithButterfly(vm, nullptr, structure, butterfly);
}
-inline JSArray* JSArray::tryCreate(VM& vm, Structure* structure, unsigned initialLength)
-{
- return tryCreate(vm, structure, initialLength, initialLength);
-}
-
inline JSArray* JSArray::create(VM& vm, Structure* structure, unsigned initialLength)
{
JSArray* result = JSArray::tryCreate(vm, structure, initialLength);
