Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (184122 => 184123)
--- trunk/Source/_javascript_Core/ChangeLog 2015-05-11 21:20:46 UTC (rev 184122)
+++ trunk/Source/_javascript_Core/ChangeLog 2015-05-11 21:21:17 UTC (rev 184123)
@@ -1,3 +1,56 @@
+2015-05-11 Ryosuke Niwa <[email protected]>
+
+ REGRESSION(r180595): same-callee profiling no longer works
+ https://bugs.webkit.org/show_bug.cgi?id=144787
+
+ Reviewed by Michael Saboff.
+
+ This patch introduces a DFG optimization to use NewObject node when the callee of op_create_this is
+ always the same JSFunction. This condition doesn't hold when the byte code creates multiple
+ JSFunction objects at runtime as in: function y() { return function () {} }; new y(); new y();
+
+ To enable this optimization, LLint and baseline JIT now store the last callee we saw in the newly
+ added fourth operand of op_create_this. We use this JSFunction's structure in DFG after verifying
+ our speculation that the callee is the same. To avoid recompiling the same code for different callee
+ objects in the polymorphic case, the special value of seenMultipleCalleeObjects() is set in
+ LLint and baseline JIT when multiple callees are observed.
+
+ Tests: stress/create-this-with-callee-variants.js
+
+ * bytecode/BytecodeList.json: Increased the number of operands to 5.
+ * bytecode/BytecodeUseDef.h:
+ (JSC::computeUsesForBytecodeOffset): op_create_this uses 2nd (constructor) and 4th (callee cache)
+ operands.
+ * bytecode/CodeBlock.cpp:
+ (JSC::CodeBlock::dumpBytecode): Dump the newly added callee cache.
+ (JSC::CodeBlock::finalizeUnconditionally): Clear the callee cache if the callee is no longer alive.
+ * bytecompiler/BytecodeGenerator.cpp:
+ (JSC::BytecodeGenerator::emitCreateThis): Add the instruction to propertyAccessInstructions so that
+ we can clear the callee cache in CodeBlock::finalizeUnconditionally. Also initialize the newly added
+ operand.
+ * dfg/DFGByteCodeParser.cpp:
+ (JSC::DFG::ByteCodeParser::parseBlock): Implement the optimization. Speculate the actual callee to
+ match the cache. Use the cached callee's structure if the speculation succeeds. Otherwise, OSR exit.
+ * jit/JITOpcodes.cpp:
+ (JSC::JIT::emit_op_create_this): Go to the slow path to update the cache unless it's already marked
+ as seenMultipleCalleeObjects() to indicate the polymorphic behavior.
+ (JSC::JIT::emitSlow_op_create_this):
+ * jit/JITOpcodes32_64.cpp:
+ (JSC::JIT::emit_op_create_this): Ditto.
+ (JSC::JIT::emitSlow_op_create_this):
+ * llint/LowLevelInterpreter32_64.asm:
+ (_llint_op_create_this): Ditto.
+ * llint/LowLevelInterpreter64.asm:
+ (_llint_op_create_this): Ditto.
+ * runtime/CommonSlowPaths.cpp:
+ (slow_path_create_this): Set the callee cache to the actual callee if it's not set. If the cache has
+ been set to a JSFunction* different from the actual callee, set it to seenMultipleCalleeObjects().
+ * runtime/JSCell.h:
+ (JSC::JSCell::seenMultipleCalleeObjects): Added.
+ * runtime/WriteBarrier.h:
+ (JSC::WriteBarrierBase::unvalidatedGet): Removed the compile guard around it.
+ * tests/stress/create-this-with-callee-variants.js: Added.
+
2015-05-11 Andreas Kling <[email protected]>
PropertyNameArray should use a Vector when there are few entries.
Modified: trunk/Source/_javascript_Core/bytecode/BytecodeList.json (184122 => 184123)
--- trunk/Source/_javascript_Core/bytecode/BytecodeList.json 2015-05-11 21:20:46 UTC (rev 184122)
+++ trunk/Source/_javascript_Core/bytecode/BytecodeList.json 2015-05-11 21:21:17 UTC (rev 184123)
@@ -9,7 +9,7 @@
{ "name" : "op_create_direct_arguments", "length" : 2 },
{ "name" : "op_create_scoped_arguments", "length" : 3 },
{ "name" : "op_create_out_of_band_arguments", "length" : 2 },
- { "name" : "op_create_this", "length" : 4 },
+ { "name" : "op_create_this", "length" : 5 },
{ "name" : "op_to_this", "length" : 4 },
{ "name" : "op_check_tdz", "length" : 2 },
{ "name" : "op_new_object", "length" : 4 },
Modified: trunk/Source/_javascript_Core/bytecode/BytecodeUseDef.h (184122 => 184123)
--- trunk/Source/_javascript_Core/bytecode/BytecodeUseDef.h 2015-05-11 21:20:46 UTC (rev 184122)
+++ trunk/Source/_javascript_Core/bytecode/BytecodeUseDef.h 2015-05-11 21:21:17 UTC (rev 184123)
@@ -142,7 +142,6 @@
case op_not:
case op_mov:
case op_new_array_with_size:
- case op_create_this:
case op_del_by_id:
case op_unsigned:
case op_new_func:
@@ -183,6 +182,11 @@
functor(codeBlock, instruction, opcodeID, instruction[3].u.operand);
return;
}
+ case op_create_this: {
+ functor(codeBlock, instruction, opcodeID, instruction[2].u.operand);
+ functor(codeBlock, instruction, opcodeID, instruction[4].u.operand);
+ return;
+ }
case op_has_structure_property:
case op_construct_varargs:
case op_call_varargs: {
Modified: trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp (184122 => 184123)
--- trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp 2015-05-11 21:20:46 UTC (rev 184122)
+++ trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp 2015-05-11 21:21:17 UTC (rev 184123)
@@ -795,8 +795,9 @@
int r0 = (++it)->u.operand;
int r1 = (++it)->u.operand;
unsigned inferredInlineCapacity = (++it)->u.operand;
+ unsigned cachedFunction = (++it)->u.operand;
printLocationAndOp(out, exec, location, it, "create_this");
- out.printf("%s, %s, %u", registerName(r0).data(), registerName(r1).data(), inferredInlineCapacity);
+ out.printf("%s, %s, %u, %u", registerName(r0).data(), registerName(r1).data(), inferredInlineCapacity, cachedFunction);
break;
}
case op_to_this: {
@@ -2553,6 +2554,18 @@
curInstruction[3].u.toThisStatus = merge(
curInstruction[3].u.toThisStatus, ToThisClearedByGC);
break;
+ case op_create_this: {
+ auto& cacheWriteBarrier = curInstruction[4].u.jsCell;
+ if (!cacheWriteBarrier || cacheWriteBarrier.unvalidatedGet() == JSCell::seenMultipleCalleeObjects())
+ break;
+ JSCell* cachedFunction = cacheWriteBarrier.get();
+ if (Heap::isMarked(cachedFunction))
+ break;
+ if (Options::verboseOSR())
+ dataLogF("Clearing LLInt create_this with cached callee %p.\n", cachedFunction);
+ cacheWriteBarrier.clear();
+ break;
+ }
case op_resolve_scope: {
// Right now this isn't strictly necessary. Any symbol tables that this will refer to
// are for outer functions, and we refer to those functions strongly, and they refer
Modified: trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp (184122 => 184123)
--- trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp 2015-05-11 21:20:46 UTC (rev 184122)
+++ trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp 2015-05-11 21:21:17 UTC (rev 184123)
@@ -1653,10 +1653,12 @@
size_t begin = instructions().size();
m_staticPropertyAnalyzer.createThis(m_thisRegister.index(), begin + 3);
+ m_codeBlock->addPropertyAccessInstruction(instructions().size());
emitOpcode(op_create_this);
instructions().append(m_thisRegister.index());
instructions().append(m_thisRegister.index());
instructions().append(0);
+ instructions().append(0);
return dst;
}
Modified: trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp (184122 => 184123)
--- trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2015-05-11 21:20:46 UTC (rev 184122)
+++ trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2015-05-11 21:21:17 UTC (rev 184123)
@@ -2669,8 +2669,24 @@
case op_create_this: {
int calleeOperand = currentInstruction[2].u.operand;
Node* callee = get(VirtualRegister(calleeOperand));
+
+ JSFunction* function = callee->dynamicCastConstant<JSFunction*>();
+ if (!function) {
+ JSCell* cachedFunction = currentInstruction[4].u.jsCell.unvalidatedGet();
+ RELEASE_ASSERT(cachedFunction); // LLint and BaselineJIT always set it to a JSFunction* or seenMultipleCalleeObjects().
+ if (cachedFunction != JSCell::seenMultipleCalleeObjects()) {
+ ASSERT(cachedFunction->inherits(JSFunction::info()));
+
+ FrozenValue* frozen = m_graph.freeze(cachedFunction);
+ addToGraph(CheckCell, OpInfo(frozen), callee);
+ set(VirtualRegister(currentInstruction[1].u.operand), addToGraph(JSConstant, OpInfo(frozen)));
+
+ function = static_cast<JSFunction*>(cachedFunction);
+ }
+ }
+
bool alreadyEmitted = false;
- if (JSFunction* function = callee->dynamicCastConstant<JSFunction*>()) {
+ if (function) {
if (FunctionRareData* rareData = function->rareData()) {
if (Structure* structure = rareData->allocationStructure()) {
m_graph.freeze(rareData);
Modified: trunk/Source/_javascript_Core/jit/JITOpcodes.cpp (184122 => 184123)
--- trunk/Source/_javascript_Core/jit/JITOpcodes.cpp 2015-05-11 21:20:46 UTC (rev 184122)
+++ trunk/Source/_javascript_Core/jit/JITOpcodes.cpp 2015-05-11 21:21:17 UTC (rev 184123)
@@ -705,11 +705,13 @@
void JIT::emit_op_create_this(Instruction* currentInstruction)
{
int callee = currentInstruction[2].u.operand;
+ WriteBarrierBase<JSCell>* cachedFunction = ¤tInstruction[4].u.jsCell;
RegisterID calleeReg = regT0;
- RegisterID rareDataReg = regT0;
+ RegisterID rareDataReg = regT4;
RegisterID resultReg = regT0;
RegisterID allocatorReg = regT1;
RegisterID structureReg = regT2;
+ RegisterID cachedFunctionReg = regT4;
RegisterID scratchReg = regT3;
emitGetVirtualRegister(callee, calleeReg);
@@ -719,6 +721,11 @@
loadPtr(Address(rareDataReg, FunctionRareData::offsetOfAllocationProfile() + ObjectAllocationProfile::offsetOfStructure()), structureReg);
addSlowCase(branchTestPtr(Zero, allocatorReg));
+ loadPtr(cachedFunction, cachedFunctionReg);
+ Jump hasSeenMultipleCallees = branchPtr(Equal, cachedFunctionReg, TrustedImmPtr(JSCell::seenMultipleCalleeObjects()));
+ addSlowCase(branchPtr(NotEqual, calleeReg, cachedFunctionReg));
+ hasSeenMultipleCallees.link(this);
+
emitAllocateJSObject(allocatorReg, structureReg, resultReg, scratchReg);
emitPutVirtualRegister(currentInstruction[1].u.operand);
}
@@ -728,6 +735,7 @@
linkSlowCase(iter); // doesn't have rare data
linkSlowCase(iter); // doesn't have an allocation profile
linkSlowCase(iter); // allocation failed
+ linkSlowCase(iter); // cached function didn't match
JITSlowPathCall slowPathCall(this, currentInstruction, slow_path_create_this);
slowPathCall.call();
Modified: trunk/Source/_javascript_Core/jit/JITOpcodes32_64.cpp (184122 => 184123)
--- trunk/Source/_javascript_Core/jit/JITOpcodes32_64.cpp 2015-05-11 21:20:46 UTC (rev 184122)
+++ trunk/Source/_javascript_Core/jit/JITOpcodes32_64.cpp 2015-05-11 21:21:17 UTC (rev 184123)
@@ -936,11 +936,13 @@
void JIT::emit_op_create_this(Instruction* currentInstruction)
{
int callee = currentInstruction[2].u.operand;
+ WriteBarrierBase<JSCell>* cachedFunction = ¤tInstruction[4].u.jsCell;
RegisterID calleeReg = regT0;
- RegisterID rareDataReg = regT0;
+ RegisterID rareDataReg = regT4;
RegisterID resultReg = regT0;
RegisterID allocatorReg = regT1;
RegisterID structureReg = regT2;
+ RegisterID cachedFunctionReg = regT4;
RegisterID scratchReg = regT3;
emitLoadPayload(callee, calleeReg);
@@ -950,6 +952,11 @@
loadPtr(Address(rareDataReg, FunctionRareData::offsetOfAllocationProfile() + ObjectAllocationProfile::offsetOfStructure()), structureReg);
addSlowCase(branchTestPtr(Zero, allocatorReg));
+ loadPtr(cachedFunction, cachedFunctionReg);
+ Jump hasSeenMultipleCallees = branchPtr(Equal, cachedFunctionReg, TrustedImmPtr(JSCell::seenMultipleCalleeObjects()));
+ addSlowCase(branchPtr(NotEqual, calleeReg, cachedFunctionReg));
+ hasSeenMultipleCallees.link(this);
+
emitAllocateJSObject(allocatorReg, structureReg, resultReg, scratchReg);
emitStoreCell(currentInstruction[1].u.operand, resultReg);
}
@@ -959,6 +966,7 @@
linkSlowCase(iter); // doesn't have rare data
linkSlowCase(iter); // doesn't have an allocation profile
linkSlowCase(iter); // allocation failed
+ linkSlowCase(iter); // cached function didn't match
JITSlowPathCall slowPathCall(this, currentInstruction, slow_path_create_this);
slowPathCall.call();
Modified: trunk/Source/_javascript_Core/llint/LowLevelInterpreter32_64.asm (184122 => 184123)
--- trunk/Source/_javascript_Core/llint/LowLevelInterpreter32_64.asm 2015-05-11 21:20:46 UTC (rev 184122)
+++ trunk/Source/_javascript_Core/llint/LowLevelInterpreter32_64.asm 2015-05-11 21:21:17 UTC (rev 184123)
@@ -745,15 +745,19 @@
loadp FunctionRareData::m_allocationProfile + ObjectAllocationProfile::m_allocator[t4], t1
loadp FunctionRareData::m_allocationProfile + ObjectAllocationProfile::m_structure[t4], t2
btpz t1, .opCreateThisSlow
+ loadpFromInstruction(4, t4)
+ bpeq t4, 1, .hasSeenMultipleCallee
+ bpneq t4, t0, .opCreateThisSlow
+.hasSeenMultipleCallee:
allocateJSObject(t1, t2, t0, t3, .opCreateThisSlow)
loadi 4[PC], t1
storei CellTag, TagOffset[cfr, t1, 8]
storei t0, PayloadOffset[cfr, t1, 8]
- dispatch(4)
+ dispatch(5)
.opCreateThisSlow:
callSlowPath(_slow_path_create_this)
- dispatch(4)
+ dispatch(5)
_llint_op_to_this:
Modified: trunk/Source/_javascript_Core/llint/LowLevelInterpreter64.asm (184122 => 184123)
--- trunk/Source/_javascript_Core/llint/LowLevelInterpreter64.asm 2015-05-11 21:20:46 UTC (rev 184122)
+++ trunk/Source/_javascript_Core/llint/LowLevelInterpreter64.asm 2015-05-11 21:21:17 UTC (rev 184123)
@@ -631,14 +631,18 @@
loadp FunctionRareData::m_allocationProfile + ObjectAllocationProfile::m_allocator[t4], t1
loadp FunctionRareData::m_allocationProfile + ObjectAllocationProfile::m_structure[t4], t2
btpz t1, .opCreateThisSlow
+ loadpFromInstruction(4, t4)
+ bpeq t4, 1, .hasSeenMultipleCallee
+ bpneq t4, t0, .opCreateThisSlow
+.hasSeenMultipleCallee:
allocateJSObject(t1, t2, t0, t3, .opCreateThisSlow)
loadisFromInstruction(1, t1)
storeq t0, [cfr, t1, 8]
- dispatch(4)
+ dispatch(5)
.opCreateThisSlow:
callSlowPath(_slow_path_create_this)
- dispatch(4)
+ dispatch(5)
_llint_op_to_this:
Modified: trunk/Source/_javascript_Core/runtime/CommonSlowPaths.cpp (184122 => 184123)
--- trunk/Source/_javascript_Core/runtime/CommonSlowPaths.cpp 2015-05-11 21:20:46 UTC (rev 184122)
+++ trunk/Source/_javascript_Core/runtime/CommonSlowPaths.cpp 2015-05-11 21:21:17 UTC (rev 184123)
@@ -235,6 +235,12 @@
ASSERT(constructor->methodTable()->getConstructData(constructor, constructData) == ConstructTypeJS);
#endif
+ auto& cacheWriteBarrier = pc[4].u.jsCell;
+ if (!cacheWriteBarrier)
+ cacheWriteBarrier.set(exec->vm(), exec->codeBlock()->ownerExecutable(), constructor);
+ else if (cacheWriteBarrier.unvalidatedGet() != JSCell::seenMultipleCalleeObjects() && cacheWriteBarrier.get() != constructor)
+ cacheWriteBarrier.setWithoutWriteBarrier(JSCell::seenMultipleCalleeObjects());
+
size_t inlineCapacity = pc[3].u.operand;
Structure* structure = constructor->rareData(exec, inlineCapacity)->allocationProfile()->structure();
RETURN(constructEmptyObject(exec, structure));
Modified: trunk/Source/_javascript_Core/runtime/JSCell.h (184122 => 184123)
--- trunk/Source/_javascript_Core/runtime/JSCell.h 2015-05-11 21:20:46 UTC (rev 184122)
+++ trunk/Source/_javascript_Core/runtime/JSCell.h 2015-05-11 21:21:17 UTC (rev 184123)
@@ -74,6 +74,8 @@
static const bool needsDestruction = false;
+ static JSCell* seenMultipleCalleeObjects() { return bitwise_cast<JSCell*>(static_cast<uintptr_t>(1)); }
+
enum CreatingEarlyCellTag { CreatingEarlyCell };
JSCell(CreatingEarlyCellTag);
Modified: trunk/Source/_javascript_Core/runtime/WriteBarrier.h (184122 => 184123)
--- trunk/Source/_javascript_Core/runtime/WriteBarrier.h 2015-05-11 21:20:46 UTC (rev 184122)
+++ trunk/Source/_javascript_Core/runtime/WriteBarrier.h 2015-05-11 21:21:17 UTC (rev 184123)
@@ -126,9 +126,7 @@
this->m_cell = reinterpret_cast<JSCell*>(value);
}
-#if ENABLE(GC_VALIDATION)
T* unvalidatedGet() const { return reinterpret_cast<T*>(static_cast<void*>(m_cell)); }
-#endif
private:
JSCell* m_cell;
Added: trunk/Source/_javascript_Core/tests/stress/create-this-with-callee-variants.js (0 => 184123)
--- trunk/Source/_javascript_Core/tests/stress/create-this-with-callee-variants.js (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/create-this-with-callee-variants.js 2015-05-11 21:21:17 UTC (rev 184123)
@@ -0,0 +1,18 @@
+function createInLoop(x, count) {
+ noInline(x)
+ for (var i = 0; i < 5000; i++) {
+ var obj = new x;
+ if (!(obj instanceof x))
+ throw "Failed to instantiate the right object";
+ }
+}
+
+function y() { return function () {} }
+
+createInLoop(y());
+
+function z() { return function () {} }
+
+createInLoop(z());
+createInLoop(z());
+createInLoop(z());
