- Revision
- 159826
- Author
- [email protected]
- Date
- 2013-11-27 16:22:43 -0800 (Wed, 27 Nov 2013)
Log Message
Finally fix some obvious Bartlett bugs
https://bugs.webkit.org/show_bug.cgi?id=124951
Reviewed by Mark Hahnenberg.
Sanitize the stack (i.e. zero parts of it known to be dead) at three key points:
- GC.
- At beginning of OSR entry.
- Just as we finish preparing OSR entry. This clears those slots on the stack that
could have been live in baseline but that are known to be dead in DFG.
This is as much as a 2x speed-up on splay if you run it in certain modes, and run it
for a long enough interval. It appears to fix all instances of the dreaded exponential
heap growth that splay gets into when some stale pointer stays around.
This doesn't have much of an effect on real-world programs. This bug has only ever
manifested in splay and for that reason we thus far opted against fixing it. But splay
is, for what it's worth, the premiere GC stress test in _javascript_ - so making sure we
can run it without pathologies - even when you tweak its configuration - is probably
fairly important.
* dfg/DFGJITCompiler.h:
(JSC::DFG::JITCompiler::noticeOSREntry):
* dfg/DFGOSREntry.cpp:
(JSC::DFG::prepareOSREntry):
* dfg/DFGOSREntry.h:
* heap/Heap.cpp:
(JSC::Heap::markRoots):
* interpreter/JSStack.cpp:
(JSC::JSStack::JSStack):
(JSC::JSStack::sanitizeStack):
* interpreter/JSStack.h:
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (159825 => 159826)
--- trunk/Source/_javascript_Core/ChangeLog 2013-11-27 23:15:48 UTC (rev 159825)
+++ trunk/Source/_javascript_Core/ChangeLog 2013-11-28 00:22:43 UTC (rev 159826)
@@ -1,3 +1,41 @@
+2013-11-27 Filip Pizlo <[email protected]>
+
+ Finally fix some obvious Bartlett bugs
+ https://bugs.webkit.org/show_bug.cgi?id=124951
+
+ Reviewed by Mark Hahnenberg.
+
+ Sanitize the stack (i.e. zero parts of it known to be dead) at three key points:
+
+ - GC.
+
+ - At beginning of OSR entry.
+
+ - Just as we finish preparing OSR entry. This clears those slots on the stack that
+ could have been live in baseline but that are known to be dead in DFG.
+
+ This is as much as a 2x speed-up on splay if you run it in certain modes, and run it
+ for a long enough interval. It appears to fix all instances of the dreaded exponential
+ heap growth that splay gets into when some stale pointer stays around.
+
+ This doesn't have much of an effect on real-world programs. This bug has only ever
+ manifested in splay and for that reason we thus far opted against fixing it. But splay
+ is, for what it's worth, the premiere GC stress test in _javascript_ - so making sure we
+ can run it without pathologies - even when you tweak its configuration - is probably
+ fairly important.
+
+ * dfg/DFGJITCompiler.h:
+ (JSC::DFG::JITCompiler::noticeOSREntry):
+ * dfg/DFGOSREntry.cpp:
+ (JSC::DFG::prepareOSREntry):
+ * dfg/DFGOSREntry.h:
+ * heap/Heap.cpp:
+ (JSC::Heap::markRoots):
+ * interpreter/JSStack.cpp:
+ (JSC::JSStack::JSStack):
+ (JSC::JSStack::sanitizeStack):
+ * interpreter/JSStack.h:
+
2013-11-26 Filip Pizlo <[email protected]>
Do bytecode validation as part of testing
Modified: trunk/Source/_javascript_Core/dfg/DFGJITCompiler.h (159825 => 159826)
--- trunk/Source/_javascript_Core/dfg/DFGJITCompiler.h 2013-11-27 23:15:48 UTC (rev 159825)
+++ trunk/Source/_javascript_Core/dfg/DFGJITCompiler.h 2013-11-28 00:22:43 UTC (rev 159826)
@@ -268,6 +268,8 @@
entry->m_expectedValues.local(local).makeHeapTop();
else {
VariableAccessData* variable = node->variableAccessData();
+ entry->m_machineStackUsed.set(variable->machineLocal().toLocal());
+
switch (variable->flushFormat()) {
case FlushedDouble:
entry->m_localsForcedDouble.set(local);
Modified: trunk/Source/_javascript_Core/dfg/DFGOSREntry.cpp (159825 => 159826)
--- trunk/Source/_javascript_Core/dfg/DFGOSREntry.cpp 2013-11-27 23:15:48 UTC (rev 159825)
+++ trunk/Source/_javascript_Core/dfg/DFGOSREntry.cpp 2013-11-28 00:22:43 UTC (rev 159826)
@@ -52,6 +52,9 @@
}
VM* vm = &exec->vm();
+
+ vm->interpreter->stack().sanitizeStack();
+
if (codeBlock->jitType() != JITCode::DFGJIT) {
RELEASE_ASSERT(codeBlock->jitType() == JITCode::FTLJIT);
@@ -181,7 +184,8 @@
// it seems silly: you'd be diverting the program to error handling when it
// would have otherwise just kept running albeit less quickly.
- if (!vm->interpreter->stack().grow(&exec->registers()[virtualRegisterForLocal(jitCode->common.requiredRegisterCountForExecutionAndExit()).offset()])) {
+ unsigned frameSize = jitCode->common.requiredRegisterCountForExecutionAndExit();
+ if (!vm->interpreter->stack().grow(&exec->registers()[virtualRegisterForLocal(frameSize).offset()])) {
if (Options::verboseOSR())
dataLogF(" OSR failed because stack growth failed.\n");
return 0;
@@ -207,11 +211,20 @@
for (unsigned i = entry->m_reshufflings.size(); i--;)
registers[entry->m_reshufflings[i].toOffset] = temporaryLocals[i];
- // 5) Fix the call frame.
+ // 5) Clear those parts of the call frame that the DFG ain't using. This helps GC on some
+ // programs by eliminating some stale pointer pathologies.
+ for (unsigned i = frameSize; i--;) {
+ if (entry->m_machineStackUsed.get(i))
+ continue;
+ registers[virtualRegisterForLocal(i).offset()] = JSValue::encode(JSValue());
+ }
+
+ // 6) Fix the call frame.
+
exec->setCodeBlock(codeBlock);
- // 6) Find and return the destination machine code address.
+ // 7) Find and return the destination machine code address.
void* result = codeBlock->jitCode()->executableAddressAtOffset(entry->m_machineCodeOffset);
Modified: trunk/Source/_javascript_Core/dfg/DFGOSREntry.h (159825 => 159826)
--- trunk/Source/_javascript_Core/dfg/DFGOSREntry.h 2013-11-27 23:15:48 UTC (rev 159825)
+++ trunk/Source/_javascript_Core/dfg/DFGOSREntry.h 2013-11-28 00:22:43 UTC (rev 159826)
@@ -59,6 +59,7 @@
BitVector m_localsForcedDouble;
BitVector m_localsForcedMachineInt;
Vector<OSREntryReshuffling> m_reshufflings;
+ BitVector m_machineStackUsed;
};
inline unsigned getOSREntryDataBytecodeIndex(OSREntryData* osrEntryData)
Modified: trunk/Source/_javascript_Core/heap/Heap.cpp (159825 => 159826)
--- trunk/Source/_javascript_Core/heap/Heap.cpp 2013-11-27 23:15:48 UTC (rev 159825)
+++ trunk/Source/_javascript_Core/heap/Heap.cpp 2013-11-28 00:22:43 UTC (rev 159826)
@@ -463,6 +463,7 @@
{
GCPHASE(GatherStackRoots);
stack().gatherConservativeRoots(stackRoots, m_jitStubRoutines, m_codeBlocks);
+ stack().sanitizeStack();
}
#if ENABLE(DFG_JIT)
Modified: trunk/Source/_javascript_Core/interpreter/JSStack.cpp (159825 => 159826)
--- trunk/Source/_javascript_Core/interpreter/JSStack.cpp 2013-11-27 23:15:48 UTC (rev 159825)
+++ trunk/Source/_javascript_Core/interpreter/JSStack.cpp 2013-11-28 00:22:43 UTC (rev 159826)
@@ -52,6 +52,8 @@
m_reservation = PageReservation::reserve(roundUpAllocationSize(capacity * sizeof(Register), commitSize), OSAllocator::JSVMStackPages);
updateStackLimit(highAddress());
m_commitEnd = highAddress();
+
+ m_lastStackTop = getBaseOfStack();
disableErrorStackReserve();
@@ -101,6 +103,19 @@
conservativeRoots.add(getBaseOfStack(), getTopOfStack(), jitStubRoutines, codeBlocks);
}
+void JSStack::sanitizeStack()
+{
+ ASSERT(getTopOfStack() <= getBaseOfStack());
+
+ if (m_lastStackTop < getTopOfStack()) {
+ char* begin = reinterpret_cast<char*>(m_lastStackTop);
+ char* end = reinterpret_cast<char*>(getTopOfStack());
+ memset(begin, 0, end - begin);
+ }
+
+ m_lastStackTop = getTopOfStack();
+}
+
void JSStack::releaseExcessCapacity()
{
ptrdiff_t delta = reinterpret_cast<uintptr_t>(highAddress()) - reinterpret_cast<uintptr_t>(m_commitEnd);
Modified: trunk/Source/_javascript_Core/interpreter/JSStack.h (159825 => 159826)
--- trunk/Source/_javascript_Core/interpreter/JSStack.h 2013-11-27 23:15:48 UTC (rev 159825)
+++ trunk/Source/_javascript_Core/interpreter/JSStack.h 2013-11-28 00:22:43 UTC (rev 159826)
@@ -81,6 +81,7 @@
void gatherConservativeRoots(ConservativeRoots&);
void gatherConservativeRoots(ConservativeRoots&, JITStubRoutineSet&, CodeBlockSet&);
+ void sanitizeStack();
Register* getBaseOfStack() const
{
@@ -154,6 +155,7 @@
Register* m_useableEnd;
PageReservation m_reservation;
CallFrame*& m_topCallFrame;
+ Register* m_lastStackTop;
friend class LLIntOffsetsExtractor;
};
