<!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>[242459] releases/WebKitGTK/webkit-2.24/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/242459">242459</a></dd>
<dt>Author</dt> <dd>carlosgc@webkit.org</dd>
<dt>Date</dt> <dd>2019-03-05 04:42:04 -0800 (Tue, 05 Mar 2019)</dd>
</dl>

<h3>Log Message</h3>
<pre>Merge <a href="http://trac.webkit.org/projects/webkit/changeset/242070">r242070</a> - [JSC] Revert <a href="http://trac.webkit.org/projects/webkit/changeset/226885">r226885</a> to make SlotVisitor creation lazy
https://bugs.webkit.org/show_bug.cgi?id=195013

Reviewed by Saam Barati.

We once changed SlotVisitor creation apriori to drop the lock. Also, it turns out that SlotVisitor is memory-consuming.
We should defer SlotVisitor creation until it is actually required. This patch reverts <a href="http://trac.webkit.org/projects/webkit/changeset/226885">r226885</a>. Even with this patch,
we still hold many SlotVisitors after we execute many parallel markers at least once. But recovering the feature of
dynamically allocating SlotVisitors helps further memory optimizations in this area.

* heap/Heap.cpp:
(JSC::Heap::Heap):
(JSC::Heap::runBeginPhase):
* heap/Heap.h:
* heap/HeapInlines.h:
(JSC::Heap::forEachSlotVisitor):
(JSC::Heap::numberOfSlotVisitors):
* heap/MarkingConstraintSolver.cpp:
(JSC::MarkingConstraintSolver::didVisitSomething const):
* heap/SlotVisitor.h:</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#releasesWebKitGTKwebkit224SourceJavaScriptCoreChangeLog">releases/WebKitGTK/webkit-2.24/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#releasesWebKitGTKwebkit224SourceJavaScriptCoreheapHeapcpp">releases/WebKitGTK/webkit-2.24/Source/JavaScriptCore/heap/Heap.cpp</a></li>
<li><a href="#releasesWebKitGTKwebkit224SourceJavaScriptCoreheapHeaph">releases/WebKitGTK/webkit-2.24/Source/JavaScriptCore/heap/Heap.h</a></li>
<li><a href="#releasesWebKitGTKwebkit224SourceJavaScriptCoreheapHeapInlinesh">releases/WebKitGTK/webkit-2.24/Source/JavaScriptCore/heap/HeapInlines.h</a></li>
<li><a href="#releasesWebKitGTKwebkit224SourceJavaScriptCoreheapMarkingConstraintSolvercpp">releases/WebKitGTK/webkit-2.24/Source/JavaScriptCore/heap/MarkingConstraintSolver.cpp</a></li>
<li><a href="#releasesWebKitGTKwebkit224SourceJavaScriptCoreheapSlotVisitorh">releases/WebKitGTK/webkit-2.24/Source/JavaScriptCore/heap/SlotVisitor.h</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="releasesWebKitGTKwebkit224SourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: releases/WebKitGTK/webkit-2.24/Source/JavaScriptCore/ChangeLog (242458 => 242459)</h4>
<pre class="diff"><span>
<span class="info">--- releases/WebKitGTK/webkit-2.24/Source/JavaScriptCore/ChangeLog   2019-03-05 12:42:00 UTC (rev 242458)
+++ releases/WebKitGTK/webkit-2.24/Source/JavaScriptCore/ChangeLog      2019-03-05 12:42:04 UTC (rev 242459)
</span><span class="lines">@@ -1,5 +1,28 @@
</span><span class="cx"> 2019-02-25  Yusuke Suzuki  <ysuzuki@apple.com>
</span><span class="cx"> 
</span><ins>+        [JSC] Revert r226885 to make SlotVisitor creation lazy
+        https://bugs.webkit.org/show_bug.cgi?id=195013
+
+        Reviewed by Saam Barati.
+
+        We once changed SlotVisitor creation apriori to drop the lock. Also, it turns out that SlotVisitor is memory-consuming.
+        We should defer SlotVisitor creation until it is actually required. This patch reverts r226885. Even with this patch,
+        we still hold many SlotVisitors after we execute many parallel markers at least once. But recovering the feature of
+        dynamically allocating SlotVisitors helps further memory optimizations in this area.
+
+        * heap/Heap.cpp:
+        (JSC::Heap::Heap):
+        (JSC::Heap::runBeginPhase):
+        * heap/Heap.h:
+        * heap/HeapInlines.h:
+        (JSC::Heap::forEachSlotVisitor):
+        (JSC::Heap::numberOfSlotVisitors):
+        * heap/MarkingConstraintSolver.cpp:
+        (JSC::MarkingConstraintSolver::didVisitSomething const):
+        * heap/SlotVisitor.h:
+
+2019-02-25  Yusuke Suzuki  <ysuzuki@apple.com>
+
</ins><span class="cx">         [JSC] stress/function-constructor-reading-from-global-lexical-environment.js fails in 32bit arch
</span><span class="cx">         https://bugs.webkit.org/show_bug.cgi?id=195030
</span><span class="cx">         <rdar://problem/48385088>
</span></span></pre></div>
<a id="releasesWebKitGTKwebkit224SourceJavaScriptCoreheapHeapcpp"></a>
<div class="modfile"><h4>Modified: releases/WebKitGTK/webkit-2.24/Source/JavaScriptCore/heap/Heap.cpp (242458 => 242459)</h4>
<pre class="diff"><span>
<span class="info">--- releases/WebKitGTK/webkit-2.24/Source/JavaScriptCore/heap/Heap.cpp       2019-03-05 12:42:00 UTC (rev 242458)
+++ releases/WebKitGTK/webkit-2.24/Source/JavaScriptCore/heap/Heap.cpp  2019-03-05 12:42:04 UTC (rev 242459)
</span><span class="lines">@@ -300,14 +300,6 @@
</span><span class="cx">     , m_threadCondition(AutomaticThreadCondition::create())
</span><span class="cx"> {
</span><span class="cx">     m_worldState.store(0);
</span><del>-
-    for (unsigned i = 0, numberOfParallelThreads = heapHelperPool().numberOfThreads(); i < numberOfParallelThreads; ++i) {
-        std::unique_ptr<SlotVisitor> visitor = std::make_unique<SlotVisitor>(*this, toCString("P", i + 1));
-        if (Options::optimizeParallelSlotVisitorsForStoppedMutator())
-            visitor->optimizeForStoppedMutator();
-        m_availableParallelSlotVisitors.append(visitor.get());
-        m_parallelSlotVisitors.append(WTFMove(visitor));
-    }
</del><span class="cx">     
</span><span class="cx">     if (Options::useConcurrentGC()) {
</span><span class="cx">         if (Options::useStochasticMutatorScheduler())
</span><span class="lines">@@ -1261,8 +1253,19 @@
</span><span class="cx">             SlotVisitor* slotVisitor;
</span><span class="cx">             {
</span><span class="cx">                 LockHolder locker(m_parallelSlotVisitorLock);
</span><del>-                RELEASE_ASSERT_WITH_MESSAGE(!m_availableParallelSlotVisitors.isEmpty(), "Parallel SlotVisitors are allocated apriori");
-                slotVisitor = m_availableParallelSlotVisitors.takeLast();
</del><ins>+                if (m_availableParallelSlotVisitors.isEmpty()) {
+                    std::unique_ptr<SlotVisitor> newVisitor = std::make_unique<SlotVisitor>(
+                        *this, toCString("P", m_parallelSlotVisitors.size() + 1));
+                    
+                    if (Options::optimizeParallelSlotVisitorsForStoppedMutator())
+                        newVisitor->optimizeForStoppedMutator();
+                    
+                    newVisitor->didStartMarking();
+                    
+                    slotVisitor = newVisitor.get();
+                    m_parallelSlotVisitors.append(WTFMove(newVisitor));
+                } else
+                    slotVisitor = m_availableParallelSlotVisitors.takeLast();
</ins><span class="cx">             }
</span><span class="cx"> 
</span><span class="cx">             WTF::registerGCThread(GCThreadType::Helper);
</span></span></pre></div>
<a id="releasesWebKitGTKwebkit224SourceJavaScriptCoreheapHeaph"></a>
<div class="modfile"><h4>Modified: releases/WebKitGTK/webkit-2.24/Source/JavaScriptCore/heap/Heap.h (242458 => 242459)</h4>
<pre class="diff"><span>
<span class="info">--- releases/WebKitGTK/webkit-2.24/Source/JavaScriptCore/heap/Heap.h 2019-03-05 12:42:00 UTC (rev 242458)
+++ releases/WebKitGTK/webkit-2.24/Source/JavaScriptCore/heap/Heap.h    2019-03-05 12:42:04 UTC (rev 242459)
</span><span class="lines">@@ -393,6 +393,7 @@
</span><span class="cx"> 
</span><span class="cx">     template<typename Func>
</span><span class="cx">     void forEachSlotVisitor(const Func&);
</span><ins>+    unsigned numberOfSlotVisitors();
</ins><span class="cx">     
</span><span class="cx">     Seconds totalGCTime() const { return m_totalGCTime; }
</span><span class="cx"> 
</span></span></pre></div>
<a id="releasesWebKitGTKwebkit224SourceJavaScriptCoreheapHeapInlinesh"></a>
<div class="modfile"><h4>Modified: releases/WebKitGTK/webkit-2.24/Source/JavaScriptCore/heap/HeapInlines.h (242458 => 242459)</h4>
<pre class="diff"><span>
<span class="info">--- releases/WebKitGTK/webkit-2.24/Source/JavaScriptCore/heap/HeapInlines.h  2019-03-05 12:42:00 UTC (rev 242458)
+++ releases/WebKitGTK/webkit-2.24/Source/JavaScriptCore/heap/HeapInlines.h     2019-03-05 12:42:04 UTC (rev 242459)
</span><span class="lines">@@ -275,6 +275,7 @@
</span><span class="cx"> template<typename Func>
</span><span class="cx"> void Heap::forEachSlotVisitor(const Func& func)
</span><span class="cx"> {
</span><ins>+    auto locker = holdLock(m_parallelSlotVisitorLock);
</ins><span class="cx">     func(*m_collectorSlotVisitor);
</span><span class="cx">     func(*m_mutatorSlotVisitor);
</span><span class="cx">     for (auto& slotVisitor : m_parallelSlotVisitors)
</span><span class="lines">@@ -281,4 +282,10 @@
</span><span class="cx">         func(*slotVisitor);
</span><span class="cx"> }
</span><span class="cx"> 
</span><ins>+inline unsigned Heap::numberOfSlotVisitors()
+{
+    auto locker = holdLock(m_parallelSlotVisitorLock);
+    return m_parallelSlotVisitors.size() + 2; // m_collectorSlotVisitor and m_mutatorSlotVisitor
+}
+
</ins><span class="cx"> } // namespace JSC
</span></span></pre></div>
<a id="releasesWebKitGTKwebkit224SourceJavaScriptCoreheapMarkingConstraintSolvercpp"></a>
<div class="modfile"><h4>Modified: releases/WebKitGTK/webkit-2.24/Source/JavaScriptCore/heap/MarkingConstraintSolver.cpp (242458 => 242459)</h4>
<pre class="diff"><span>
<span class="info">--- releases/WebKitGTK/webkit-2.24/Source/JavaScriptCore/heap/MarkingConstraintSolver.cpp    2019-03-05 12:42:00 UTC (rev 242458)
+++ releases/WebKitGTK/webkit-2.24/Source/JavaScriptCore/heap/MarkingConstraintSolver.cpp       2019-03-05 12:42:04 UTC (rev 242459)
</span><span class="lines">@@ -51,6 +51,10 @@
</span><span class="cx">         if (visitCounter.visitCount())
</span><span class="cx">             return true;
</span><span class="cx">     }
</span><ins>+    // If the number of SlotVisitors increases after creating m_visitCounters,
+    // we conservatively say there could be something visited by added SlotVisitors.
+    if (m_heap.numberOfSlotVisitors() > m_visitCounters.size())
+        return true;
</ins><span class="cx">     return false;
</span><span class="cx"> }
</span><span class="cx"> 
</span></span></pre></div>
<a id="releasesWebKitGTKwebkit224SourceJavaScriptCoreheapSlotVisitorh"></a>
<div class="modfile"><h4>Modified: releases/WebKitGTK/webkit-2.24/Source/JavaScriptCore/heap/SlotVisitor.h (242458 => 242459)</h4>
<pre class="diff"><span>
<span class="info">--- releases/WebKitGTK/webkit-2.24/Source/JavaScriptCore/heap/SlotVisitor.h  2019-03-05 12:42:00 UTC (rev 242458)
+++ releases/WebKitGTK/webkit-2.24/Source/JavaScriptCore/heap/SlotVisitor.h     2019-03-05 12:42:04 UTC (rev 242459)
</span><span class="lines">@@ -259,8 +259,6 @@
</span><span class="cx">     MarkingConstraint* m_currentConstraint { nullptr };
</span><span class="cx">     MarkingConstraintSolver* m_currentSolver { nullptr };
</span><span class="cx">     
</span><del>-    // Put padding here to mitigate false sharing between multiple SlotVisitors.
-    char padding[64];
</del><span class="cx"> public:
</span><span class="cx"> #if !ASSERT_DISABLED
</span><span class="cx">     bool m_isCheckingForDefaultMarkViolation;
</span></span></pre>
</div>
</div>

</body>
</html>