<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[159826] trunk/Source/JavaScriptCore</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/159826">159826</a></dd>
<dt>Author</dt> <dd>fpizlo@apple.com</dd>
<dt>Date</dt> <dd>2013-11-27 16:22:43 -0800 (Wed, 27 Nov 2013)</dd>
</dl>
<h3>Log Message</h3>
<pre>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:</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGJITCompilerh">trunk/Source/JavaScriptCore/dfg/DFGJITCompiler.h</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGOSREntrycpp">trunk/Source/JavaScriptCore/dfg/DFGOSREntry.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGOSREntryh">trunk/Source/JavaScriptCore/dfg/DFGOSREntry.h</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapHeapcpp">trunk/Source/JavaScriptCore/heap/Heap.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreinterpreterJSStackcpp">trunk/Source/JavaScriptCore/interpreter/JSStack.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreinterpreterJSStackh">trunk/Source/JavaScriptCore/interpreter/JSStack.h</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (159825 => 159826)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2013-11-27 23:15:48 UTC (rev 159825)
+++ trunk/Source/JavaScriptCore/ChangeLog 2013-11-28 00:22:43 UTC (rev 159826)
</span><span class="lines">@@ -1,3 +1,41 @@
</span><ins>+2013-11-27 Filip Pizlo <fpizlo@apple.com>
+
+ 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:
+
</ins><span class="cx"> 2013-11-26 Filip Pizlo <fpizlo@apple.com>
</span><span class="cx">
</span><span class="cx"> Do bytecode validation as part of testing
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGJITCompilerh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGJITCompiler.h (159825 => 159826)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGJITCompiler.h 2013-11-27 23:15:48 UTC (rev 159825)
+++ trunk/Source/JavaScriptCore/dfg/DFGJITCompiler.h 2013-11-28 00:22:43 UTC (rev 159826)
</span><span class="lines">@@ -268,6 +268,8 @@
</span><span class="cx"> entry->m_expectedValues.local(local).makeHeapTop();
</span><span class="cx"> else {
</span><span class="cx"> VariableAccessData* variable = node->variableAccessData();
</span><ins>+ entry->m_machineStackUsed.set(variable->machineLocal().toLocal());
+
</ins><span class="cx"> switch (variable->flushFormat()) {
</span><span class="cx"> case FlushedDouble:
</span><span class="cx"> entry->m_localsForcedDouble.set(local);
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGOSREntrycpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGOSREntry.cpp (159825 => 159826)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGOSREntry.cpp 2013-11-27 23:15:48 UTC (rev 159825)
+++ trunk/Source/JavaScriptCore/dfg/DFGOSREntry.cpp 2013-11-28 00:22:43 UTC (rev 159826)
</span><span class="lines">@@ -52,6 +52,9 @@
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> VM* vm = &exec->vm();
</span><ins>+
+ vm->interpreter->stack().sanitizeStack();
+
</ins><span class="cx"> if (codeBlock->jitType() != JITCode::DFGJIT) {
</span><span class="cx"> RELEASE_ASSERT(codeBlock->jitType() == JITCode::FTLJIT);
</span><span class="cx">
</span><span class="lines">@@ -181,7 +184,8 @@
</span><span class="cx"> // it seems silly: you'd be diverting the program to error handling when it
</span><span class="cx"> // would have otherwise just kept running albeit less quickly.
</span><span class="cx">
</span><del>- if (!vm->interpreter->stack().grow(&exec->registers()[virtualRegisterForLocal(jitCode->common.requiredRegisterCountForExecutionAndExit()).offset()])) {
</del><ins>+ unsigned frameSize = jitCode->common.requiredRegisterCountForExecutionAndExit();
+ if (!vm->interpreter->stack().grow(&exec->registers()[virtualRegisterForLocal(frameSize).offset()])) {
</ins><span class="cx"> if (Options::verboseOSR())
</span><span class="cx"> dataLogF(" OSR failed because stack growth failed.\n");
</span><span class="cx"> return 0;
</span><span class="lines">@@ -207,11 +211,20 @@
</span><span class="cx"> for (unsigned i = entry->m_reshufflings.size(); i--;)
</span><span class="cx"> registers[entry->m_reshufflings[i].toOffset] = temporaryLocals[i];
</span><span class="cx">
</span><del>- // 5) Fix the call frame.
</del><ins>+ // 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.
</ins><span class="cx">
</span><ins>+ for (unsigned i = frameSize; i--;) {
+ if (entry->m_machineStackUsed.get(i))
+ continue;
+ registers[virtualRegisterForLocal(i).offset()] = JSValue::encode(JSValue());
+ }
+
+ // 6) Fix the call frame.
+
</ins><span class="cx"> exec->setCodeBlock(codeBlock);
</span><span class="cx">
</span><del>- // 6) Find and return the destination machine code address.
</del><ins>+ // 7) Find and return the destination machine code address.
</ins><span class="cx">
</span><span class="cx"> void* result = codeBlock->jitCode()->executableAddressAtOffset(entry->m_machineCodeOffset);
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGOSREntryh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGOSREntry.h (159825 => 159826)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGOSREntry.h 2013-11-27 23:15:48 UTC (rev 159825)
+++ trunk/Source/JavaScriptCore/dfg/DFGOSREntry.h 2013-11-28 00:22:43 UTC (rev 159826)
</span><span class="lines">@@ -59,6 +59,7 @@
</span><span class="cx"> BitVector m_localsForcedDouble;
</span><span class="cx"> BitVector m_localsForcedMachineInt;
</span><span class="cx"> Vector<OSREntryReshuffling> m_reshufflings;
</span><ins>+ BitVector m_machineStackUsed;
</ins><span class="cx"> };
</span><span class="cx">
</span><span class="cx"> inline unsigned getOSREntryDataBytecodeIndex(OSREntryData* osrEntryData)
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapHeapcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/Heap.cpp (159825 => 159826)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/Heap.cpp 2013-11-27 23:15:48 UTC (rev 159825)
+++ trunk/Source/JavaScriptCore/heap/Heap.cpp 2013-11-28 00:22:43 UTC (rev 159826)
</span><span class="lines">@@ -463,6 +463,7 @@
</span><span class="cx"> {
</span><span class="cx"> GCPHASE(GatherStackRoots);
</span><span class="cx"> stack().gatherConservativeRoots(stackRoots, m_jitStubRoutines, m_codeBlocks);
</span><ins>+ stack().sanitizeStack();
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> #if ENABLE(DFG_JIT)
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreinterpreterJSStackcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/interpreter/JSStack.cpp (159825 => 159826)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/interpreter/JSStack.cpp 2013-11-27 23:15:48 UTC (rev 159825)
+++ trunk/Source/JavaScriptCore/interpreter/JSStack.cpp 2013-11-28 00:22:43 UTC (rev 159826)
</span><span class="lines">@@ -52,6 +52,8 @@
</span><span class="cx"> m_reservation = PageReservation::reserve(roundUpAllocationSize(capacity * sizeof(Register), commitSize), OSAllocator::JSVMStackPages);
</span><span class="cx"> updateStackLimit(highAddress());
</span><span class="cx"> m_commitEnd = highAddress();
</span><ins>+
+ m_lastStackTop = getBaseOfStack();
</ins><span class="cx">
</span><span class="cx"> disableErrorStackReserve();
</span><span class="cx">
</span><span class="lines">@@ -101,6 +103,19 @@
</span><span class="cx"> conservativeRoots.add(getBaseOfStack(), getTopOfStack(), jitStubRoutines, codeBlocks);
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+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();
+}
+
</ins><span class="cx"> void JSStack::releaseExcessCapacity()
</span><span class="cx"> {
</span><span class="cx"> ptrdiff_t delta = reinterpret_cast<uintptr_t>(highAddress()) - reinterpret_cast<uintptr_t>(m_commitEnd);
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreinterpreterJSStackh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/interpreter/JSStack.h (159825 => 159826)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/interpreter/JSStack.h 2013-11-27 23:15:48 UTC (rev 159825)
+++ trunk/Source/JavaScriptCore/interpreter/JSStack.h 2013-11-28 00:22:43 UTC (rev 159826)
</span><span class="lines">@@ -81,6 +81,7 @@
</span><span class="cx">
</span><span class="cx"> void gatherConservativeRoots(ConservativeRoots&);
</span><span class="cx"> void gatherConservativeRoots(ConservativeRoots&, JITStubRoutineSet&, CodeBlockSet&);
</span><ins>+ void sanitizeStack();
</ins><span class="cx">
</span><span class="cx"> Register* getBaseOfStack() const
</span><span class="cx"> {
</span><span class="lines">@@ -154,6 +155,7 @@
</span><span class="cx"> Register* m_useableEnd;
</span><span class="cx"> PageReservation m_reservation;
</span><span class="cx"> CallFrame*& m_topCallFrame;
</span><ins>+ Register* m_lastStackTop;
</ins><span class="cx">
</span><span class="cx"> friend class LLIntOffsetsExtractor;
</span><span class="cx"> };
</span></span></pre>
</div>
</div>
</body>
</html>