<!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>[181060] 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/181060">181060</a></dd>
<dt>Author</dt> <dd>akling@apple.com</dd>
<dt>Date</dt> <dd>2015-03-04 18:19:14 -0800 (Wed, 04 Mar 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>GC should compute stack bounds and dump registers at the earliest opportunity.
<https://webkit.org/b/142310>
<rdar://problem/20045624>
Reviewed by Geoffrey Garen.
Make Heap::collect() a wrapper function around a collectImpl() where the work is actually done.
The wrapper function that grabs a snapshot of the current stack boundaries and register values
on entry, and sanitizes the stack on exit.
This is a speculative fix for what appears to be overly conservative behavior in the garbage
collector following <a href="http://trac.webkit.org/projects/webkit/changeset/178364">r178364</a> which caused a measurable regression in memory usage on Membuster.
The theory being that we were putting pointers to dead things on the stack before scanning it,
and by doing that ended up marking things that we'd otherwise discover to be garbage.
* heap/Heap.cpp:
(JSC::Heap::markRoots):
(JSC::Heap::gatherStackRoots):
(JSC::Heap::collect):
(JSC::Heap::collectImpl):
* heap/Heap.h:
* heap/MachineStackMarker.cpp:
(JSC::MachineThreads::gatherFromCurrentThread):
(JSC::MachineThreads::gatherConservativeRoots):
* heap/MachineStackMarker.h:</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapHeapcpp">trunk/Source/JavaScriptCore/heap/Heap.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapHeaph">trunk/Source/JavaScriptCore/heap/Heap.h</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapMachineStackMarkercpp">trunk/Source/JavaScriptCore/heap/MachineStackMarker.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapMachineStackMarkerh">trunk/Source/JavaScriptCore/heap/MachineStackMarker.h</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (181059 => 181060)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2015-03-05 01:51:57 UTC (rev 181059)
+++ trunk/Source/JavaScriptCore/ChangeLog 2015-03-05 02:19:14 UTC (rev 181060)
</span><span class="lines">@@ -1,3 +1,31 @@
</span><ins>+2015-03-04 Andreas Kling <akling@apple.com>
+
+ GC should compute stack bounds and dump registers at the earliest opportunity.
+ <https://webkit.org/b/142310>
+ <rdar://problem/20045624>
+
+ Reviewed by Geoffrey Garen.
+
+ Make Heap::collect() a wrapper function around a collectImpl() where the work is actually done.
+ The wrapper function that grabs a snapshot of the current stack boundaries and register values
+ on entry, and sanitizes the stack on exit.
+
+ This is a speculative fix for what appears to be overly conservative behavior in the garbage
+ collector following r178364 which caused a measurable regression in memory usage on Membuster.
+ The theory being that we were putting pointers to dead things on the stack before scanning it,
+ and by doing that ended up marking things that we'd otherwise discover to be garbage.
+
+ * heap/Heap.cpp:
+ (JSC::Heap::markRoots):
+ (JSC::Heap::gatherStackRoots):
+ (JSC::Heap::collect):
+ (JSC::Heap::collectImpl):
+ * heap/Heap.h:
+ * heap/MachineStackMarker.cpp:
+ (JSC::MachineThreads::gatherFromCurrentThread):
+ (JSC::MachineThreads::gatherConservativeRoots):
+ * heap/MachineStackMarker.h:
+
</ins><span class="cx"> 2015-03-04 Debarshi Ray <debarshir@gnome.org>
</span><span class="cx">
</span><span class="cx"> Silence GCC's -Wstrict-prototypes
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapHeapcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/Heap.cpp (181059 => 181060)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/Heap.cpp 2015-03-05 01:51:57 UTC (rev 181059)
+++ trunk/Source/JavaScriptCore/heap/Heap.cpp 2015-03-05 02:19:14 UTC (rev 181060)
</span><span class="lines">@@ -513,7 +513,7 @@
</span><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx">
</span><del>-void Heap::markRoots(double gcStartTime)
</del><ins>+void Heap::markRoots(double gcStartTime, void* stackOrigin, void* stackTop, MachineThreads::RegisterState& calleeSavedRegisters)
</ins><span class="cx"> {
</span><span class="cx"> SamplingRegion samplingRegion("Garbage Collection: Marking");
</span><span class="cx">
</span><span class="lines">@@ -534,15 +534,11 @@
</span><span class="cx">
</span><span class="cx"> // We gather conservative roots before clearing mark bits because conservative
</span><span class="cx"> // gathering uses the mark bits to determine whether a reference is valid.
</span><del>- void* dummy;
- ALLOCATE_AND_GET_REGISTER_STATE(registers);
</del><span class="cx"> ConservativeRoots conservativeRoots(&m_objectSpace.blocks(), &m_storageSpace);
</span><del>- gatherStackRoots(conservativeRoots, &dummy, registers);
</del><ins>+ gatherStackRoots(conservativeRoots, stackOrigin, stackTop, calleeSavedRegisters);
</ins><span class="cx"> gatherJSStackRoots(conservativeRoots);
</span><span class="cx"> gatherScratchBufferRoots(conservativeRoots);
</span><span class="cx">
</span><del>- sanitizeStackForVM(m_vm);
-
</del><span class="cx"> clearLivenessData();
</span><span class="cx">
</span><span class="cx"> m_sharedData.didStartMarking();
</span><span class="lines">@@ -598,11 +594,11 @@
</span><span class="cx"> m_storageSpace.doneCopying();
</span><span class="cx"> }
</span><span class="cx">
</span><del>-void Heap::gatherStackRoots(ConservativeRoots& roots, void** dummy, MachineThreads::RegisterState& registers)
</del><ins>+void Heap::gatherStackRoots(ConservativeRoots& roots, void* stackOrigin, void* stackTop, MachineThreads::RegisterState& calleeSavedRegisters)
</ins><span class="cx"> {
</span><span class="cx"> GCPHASE(GatherStackRoots);
</span><span class="cx"> m_jitStubRoutines.clearMarks();
</span><del>- m_machineThreads.gatherConservativeRoots(roots, m_jitStubRoutines, m_codeBlocks, dummy, registers);
</del><ins>+ m_machineThreads.gatherConservativeRoots(roots, m_jitStubRoutines, m_codeBlocks, stackOrigin, stackTop, calleeSavedRegisters);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void Heap::gatherJSStackRoots(ConservativeRoots& roots)
</span><span class="lines">@@ -1003,8 +999,18 @@
</span><span class="cx">
</span><span class="cx"> static double minute = 60.0;
</span><span class="cx">
</span><del>-void Heap::collect(HeapOperation collectionType)
</del><ins>+NEVER_INLINE void Heap::collect(HeapOperation collectionType)
</ins><span class="cx"> {
</span><ins>+ void* stackTop;
+ ALLOCATE_AND_GET_REGISTER_STATE(registers);
+
+ collectImpl(collectionType, wtfThreadData().stack().origin(), &stackTop, registers);
+
+ sanitizeStackForVM(m_vm);
+}
+
+NEVER_INLINE void Heap::collectImpl(HeapOperation collectionType, void* stackOrigin, void* stackTop, MachineThreads::RegisterState& calleeSavedRegisters)
+{
</ins><span class="cx"> #if ENABLE(ALLOCATION_LOGGING)
</span><span class="cx"> dataLogF("JSC GC starting collection.\n");
</span><span class="cx"> #endif
</span><span class="lines">@@ -1048,7 +1054,7 @@
</span><span class="cx"> stopAllocation();
</span><span class="cx"> flushWriteBarrierBuffer();
</span><span class="cx">
</span><del>- markRoots(gcStartTime);
</del><ins>+ markRoots(gcStartTime, stackOrigin, stackTop, calleeSavedRegisters);
</ins><span class="cx">
</span><span class="cx"> if (m_verifier) {
</span><span class="cx"> m_verifier->gatherLiveObjects(HeapVerifier::Phase::AfterMarking);
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapHeaph"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/Heap.h (181059 => 181060)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/Heap.h 2015-03-05 01:51:57 UTC (rev 181059)
+++ trunk/Source/JavaScriptCore/heap/Heap.h 2015-03-05 02:19:14 UTC (rev 181060)
</span><span class="lines">@@ -273,6 +273,8 @@
</span><span class="cx"> JS_EXPORT_PRIVATE bool isValidAllocation(size_t);
</span><span class="cx"> JS_EXPORT_PRIVATE void reportExtraMemoryCostSlowCase(size_t);
</span><span class="cx">
</span><ins>+ void collectImpl(HeapOperation, void* stackOrigin, void* stackTop, MachineThreads::RegisterState&);
+
</ins><span class="cx"> void suspendCompilerThreads();
</span><span class="cx"> void willStartCollection(HeapOperation collectionType);
</span><span class="cx"> void deleteOldCode(double gcStartTime);
</span><span class="lines">@@ -280,8 +282,8 @@
</span><span class="cx"> void flushWriteBarrierBuffer();
</span><span class="cx"> void stopAllocation();
</span><span class="cx">
</span><del>- void markRoots(double gcStartTime);
- void gatherStackRoots(ConservativeRoots&, void** dummy, MachineThreads::RegisterState& registers);
</del><ins>+ void markRoots(double gcStartTime, void* stackOrigin, void* stackTop, MachineThreads::RegisterState&);
+ void gatherStackRoots(ConservativeRoots&, void* stackOrigin, void* stackTop, MachineThreads::RegisterState&);
</ins><span class="cx"> void gatherJSStackRoots(ConservativeRoots&);
</span><span class="cx"> void gatherScratchBufferRoots(ConservativeRoots&);
</span><span class="cx"> void clearLivenessData();
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapMachineStackMarkercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/MachineStackMarker.cpp (181059 => 181060)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/MachineStackMarker.cpp 2015-03-05 01:51:57 UTC (rev 181059)
+++ trunk/Source/JavaScriptCore/heap/MachineStackMarker.cpp 2015-03-05 02:19:14 UTC (rev 181060)
</span><span class="lines">@@ -276,15 +276,13 @@
</span><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx">
</span><del>-void MachineThreads::gatherFromCurrentThread(ConservativeRoots& conservativeRoots, JITStubRoutineSet& jitStubRoutines, CodeBlockSet& codeBlocks, void* stackCurrent, RegisterState& registers)
</del><ins>+void MachineThreads::gatherFromCurrentThread(ConservativeRoots& conservativeRoots, JITStubRoutineSet& jitStubRoutines, CodeBlockSet& codeBlocks, void* stackOrigin, void* stackTop, RegisterState& calleeSavedRegisters)
</ins><span class="cx"> {
</span><del>- void* registersBegin = &registers;
- void* registersEnd = reinterpret_cast<void*>(roundUpToMultipleOf<sizeof(void*)>(reinterpret_cast<uintptr_t>(&registers + 1)));
</del><ins>+ void* registersBegin = &calleeSavedRegisters;
+ void* registersEnd = reinterpret_cast<void*>(roundUpToMultipleOf<sizeof(void*)>(reinterpret_cast<uintptr_t>(&calleeSavedRegisters + 1)));
</ins><span class="cx"> conservativeRoots.add(registersBegin, registersEnd, jitStubRoutines, codeBlocks);
</span><span class="cx">
</span><del>- void* stackBegin = stackCurrent;
- void* stackEnd = wtfThreadData().stack().origin();
- conservativeRoots.add(stackBegin, stackEnd, jitStubRoutines, codeBlocks);
</del><ins>+ conservativeRoots.add(stackTop, stackOrigin, jitStubRoutines, codeBlocks);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> static inline bool suspendThread(const PlatformThread& platformThread)
</span><span class="lines">@@ -614,9 +612,9 @@
</span><span class="cx"> *buffer = fastMalloc(*capacity);
</span><span class="cx"> }
</span><span class="cx">
</span><del>-void MachineThreads::gatherConservativeRoots(ConservativeRoots& conservativeRoots, JITStubRoutineSet& jitStubRoutines, CodeBlockSet& codeBlocks, void* stackCurrent, RegisterState& currentThreadRegisters)
</del><ins>+void MachineThreads::gatherConservativeRoots(ConservativeRoots& conservativeRoots, JITStubRoutineSet& jitStubRoutines, CodeBlockSet& codeBlocks, void* stackOrigin, void* stackTop, RegisterState& calleeSavedRegisters)
</ins><span class="cx"> {
</span><del>- gatherFromCurrentThread(conservativeRoots, jitStubRoutines, codeBlocks, stackCurrent, currentThreadRegisters);
</del><ins>+ gatherFromCurrentThread(conservativeRoots, jitStubRoutines, codeBlocks, stackOrigin, stackTop, calleeSavedRegisters);
</ins><span class="cx">
</span><span class="cx"> size_t size;
</span><span class="cx"> size_t capacity = 0;
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapMachineStackMarkerh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/MachineStackMarker.h (181059 => 181060)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/MachineStackMarker.h 2015-03-05 01:51:57 UTC (rev 181059)
+++ trunk/Source/JavaScriptCore/heap/MachineStackMarker.h 2015-03-05 02:19:14 UTC (rev 181060)
</span><span class="lines">@@ -42,14 +42,14 @@
</span><span class="cx"> MachineThreads(Heap*);
</span><span class="cx"> ~MachineThreads();
</span><span class="cx">
</span><del>- void gatherConservativeRoots(ConservativeRoots&, JITStubRoutineSet&, CodeBlockSet&, void* stackCurrent, RegisterState& registers);
</del><ins>+ void gatherConservativeRoots(ConservativeRoots&, JITStubRoutineSet&, CodeBlockSet&, void* stackOrigin, void* stackTop, RegisterState& calleeSavedRegisters);
</ins><span class="cx">
</span><span class="cx"> JS_EXPORT_PRIVATE void addCurrentThread(); // Only needs to be called by clients that can use the same heap from multiple threads.
</span><span class="cx">
</span><span class="cx"> private:
</span><span class="cx"> class Thread;
</span><span class="cx">
</span><del>- void gatherFromCurrentThread(ConservativeRoots&, JITStubRoutineSet&, CodeBlockSet&, void* stackCurrent, RegisterState& registers);
</del><ins>+ void gatherFromCurrentThread(ConservativeRoots&, JITStubRoutineSet&, CodeBlockSet&, void* stackOrigin, void* stackTop, RegisterState& calleeSavedRegisters);
</ins><span class="cx">
</span><span class="cx"> void tryCopyOtherThreadStack(Thread*, void*, size_t capacity, size_t*);
</span><span class="cx"> bool tryCopyOtherThreadStacks(MutexLocker&, void*, size_t capacity, size_t*);
</span></span></pre>
</div>
</div>
</body>
</html>