<!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>[184652] 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/184652">184652</a></dd>
<dt>Author</dt> <dd>akling@apple.com</dd>
<dt>Date</dt> <dd>2015-05-20 13:30:42 -0700 (Wed, 20 May 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>Eden collections should extend the IncrementalSweeper work list, not replace it.
<https://webkit.org/b/145213>
<rdar://problem/21002666>
Reviewed by Geoffrey Garen.
After an eden collection, the garbage collector was adding all MarkedBlocks containing
new objects to the IncrementalSweeper's work list, to make sure they didn't have to
wait until the next full collection before getting swept.
Or at least, that's what it thought it was doing. It turns out that IncrementalSweeper's
internal work list is really just a reference to Heap::m_blockSnapshot. I didn't realize
this when writing the post-eden sweep code, and instead made eden collections cancel
all pending sweeps and *replace* them with the list of blocks with new objects.
This made it so that rapidly occurring eden collections could prevent large numbers of
heap blocks from ever getting swept. This would manifest as accumulation of MarkedBlocks
when a system under heavy load was also allocating short lived objects at a high rate.
Things would eventually get cleaned up when there was a lull and a full collection was
allowed to run its heap sweep to completion.
Fix this by moving all management of the block snapshot to Heap. snapshotMarkedSpace()
now handles eden collections by merging the list of blocks with new objects into the
existing block snapshot.
* heap/Heap.cpp:
(JSC::Heap::snapshotMarkedSpace):
(JSC::Heap::notifyIncrementalSweeper):
* heap/IncrementalSweeper.cpp:
(JSC::IncrementalSweeper::startSweeping):
(JSC::IncrementalSweeper::addBlocksAndContinueSweeping): Deleted.
* heap/IncrementalSweeper.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="#trunkSourceJavaScriptCoreheapIncrementalSweepercpp">trunk/Source/JavaScriptCore/heap/IncrementalSweeper.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapIncrementalSweeperh">trunk/Source/JavaScriptCore/heap/IncrementalSweeper.h</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (184651 => 184652)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2015-05-20 20:20:09 UTC (rev 184651)
+++ trunk/Source/JavaScriptCore/ChangeLog 2015-05-20 20:30:42 UTC (rev 184652)
</span><span class="lines">@@ -1,3 +1,38 @@
</span><ins>+2015-05-20 Andreas Kling <akling@apple.com>
+
+ Eden collections should extend the IncrementalSweeper work list, not replace it.
+ <https://webkit.org/b/145213>
+ <rdar://problem/21002666>
+
+ Reviewed by Geoffrey Garen.
+
+ After an eden collection, the garbage collector was adding all MarkedBlocks containing
+ new objects to the IncrementalSweeper's work list, to make sure they didn't have to
+ wait until the next full collection before getting swept.
+
+ Or at least, that's what it thought it was doing. It turns out that IncrementalSweeper's
+ internal work list is really just a reference to Heap::m_blockSnapshot. I didn't realize
+ this when writing the post-eden sweep code, and instead made eden collections cancel
+ all pending sweeps and *replace* them with the list of blocks with new objects.
+
+ This made it so that rapidly occurring eden collections could prevent large numbers of
+ heap blocks from ever getting swept. This would manifest as accumulation of MarkedBlocks
+ when a system under heavy load was also allocating short lived objects at a high rate.
+ Things would eventually get cleaned up when there was a lull and a full collection was
+ allowed to run its heap sweep to completion.
+
+ Fix this by moving all management of the block snapshot to Heap. snapshotMarkedSpace()
+ now handles eden collections by merging the list of blocks with new objects into the
+ existing block snapshot.
+
+ * heap/Heap.cpp:
+ (JSC::Heap::snapshotMarkedSpace):
+ (JSC::Heap::notifyIncrementalSweeper):
+ * heap/IncrementalSweeper.cpp:
+ (JSC::IncrementalSweeper::startSweeping):
+ (JSC::IncrementalSweeper::addBlocksAndContinueSweeping): Deleted.
+ * heap/IncrementalSweeper.h:
+
</ins><span class="cx"> 2015-05-20 Youenn Fablet <youenn.fablet@crf.canon.fr>
</span><span class="cx">
</span><span class="cx"> AudioContext resume/close/suspend should reject promises with a DOM exception in lieu of throwing exceptions
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapHeapcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/Heap.cpp (184651 => 184652)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/Heap.cpp 2015-05-20 20:20:09 UTC (rev 184651)
+++ trunk/Source/JavaScriptCore/heap/Heap.cpp 2015-05-20 20:30:42 UTC (rev 184652)
</span><span class="lines">@@ -1212,9 +1212,12 @@
</span><span class="cx"> {
</span><span class="cx"> GCPHASE(SnapshotMarkedSpace);
</span><span class="cx">
</span><del>- if (m_operationInProgress == EdenCollection)
- m_blockSnapshot = m_objectSpace.blocksWithNewObjects();
- else {
</del><ins>+ if (m_operationInProgress == EdenCollection) {
+ m_blockSnapshot.appendVector(m_objectSpace.blocksWithNewObjects());
+ // Sort and deduplicate the block snapshot since we might be appending to an unfinished work list.
+ std::sort(m_blockSnapshot.begin(), m_blockSnapshot.end());
+ m_blockSnapshot.shrink(std::unique(m_blockSnapshot.begin(), m_blockSnapshot.end()) - m_blockSnapshot.begin());
+ } else {
</ins><span class="cx"> m_blockSnapshot.resizeToFit(m_objectSpace.blocks().set().size());
</span><span class="cx"> MarkedBlockSnapshotFunctor functor(m_blockSnapshot);
</span><span class="cx"> m_objectSpace.forEachBlock(functor);
</span><span class="lines">@@ -1230,13 +1233,13 @@
</span><span class="cx"> void Heap::notifyIncrementalSweeper()
</span><span class="cx"> {
</span><span class="cx"> GCPHASE(NotifyIncrementalSweeper);
</span><del>- if (m_operationInProgress == EdenCollection)
- m_sweeper->addBlocksAndContinueSweeping(WTF::move(m_blockSnapshot));
- else {
</del><ins>+
+ if (m_operationInProgress == FullCollection) {
</ins><span class="cx"> if (!m_logicallyEmptyWeakBlocks.isEmpty())
</span><span class="cx"> m_indexOfNextLogicallyEmptyWeakBlockToSweep = 0;
</span><del>- m_sweeper->startSweeping(WTF::move(m_blockSnapshot));
</del><span class="cx"> }
</span><ins>+
+ m_sweeper->startSweeping();
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void Heap::rememberCurrentlyExecutingCodeBlocks()
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapIncrementalSweepercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/IncrementalSweeper.cpp (184651 => 184652)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/IncrementalSweeper.cpp 2015-05-20 20:20:09 UTC (rev 184651)
+++ trunk/Source/JavaScriptCore/heap/IncrementalSweeper.cpp 2015-05-20 20:30:42 UTC (rev 184652)
</span><span class="lines">@@ -101,21 +101,11 @@
</span><span class="cx"> return m_vm->heap.sweepNextLogicallyEmptyWeakBlock();
</span><span class="cx"> }
</span><span class="cx">
</span><del>-void IncrementalSweeper::startSweeping(Vector<MarkedBlock*>&& blockSnapshot)
</del><ins>+void IncrementalSweeper::startSweeping()
</ins><span class="cx"> {
</span><del>- m_blocksToSweep = WTF::move(blockSnapshot);
</del><span class="cx"> scheduleTimer();
</span><span class="cx"> }
</span><span class="cx">
</span><del>-void IncrementalSweeper::addBlocksAndContinueSweeping(Vector<MarkedBlock*>&& blockSnapshot)
-{
- Vector<MarkedBlock*> blocks = WTF::move(blockSnapshot);
- m_blocksToSweep.appendVector(blocks);
- std::sort(m_blocksToSweep.begin(), m_blocksToSweep.end());
- m_blocksToSweep.shrink(std::unique(m_blocksToSweep.begin(), m_blocksToSweep.end()) - m_blocksToSweep.begin());
- scheduleTimer();
-}
-
</del><span class="cx"> void IncrementalSweeper::willFinishSweeping()
</span><span class="cx"> {
</span><span class="cx"> m_blocksToSweep.clear();
</span><span class="lines">@@ -134,14 +124,10 @@
</span><span class="cx"> {
</span><span class="cx"> }
</span><span class="cx">
</span><del>-void IncrementalSweeper::startSweeping(Vector<MarkedBlock*>&&)
</del><ins>+void IncrementalSweeper::startSweeping()
</ins><span class="cx"> {
</span><span class="cx"> }
</span><span class="cx">
</span><del>-void IncrementalSweeper::addBlocksAndContinueSweeping(Vector<MarkedBlock*>&&)
-{
-}
-
</del><span class="cx"> void IncrementalSweeper::willFinishSweeping()
</span><span class="cx"> {
</span><span class="cx"> }
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapIncrementalSweeperh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/IncrementalSweeper.h (184651 => 184652)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/IncrementalSweeper.h 2015-05-20 20:20:09 UTC (rev 184651)
+++ trunk/Source/JavaScriptCore/heap/IncrementalSweeper.h 2015-05-20 20:30:42 UTC (rev 184652)
</span><span class="lines">@@ -44,8 +44,7 @@
</span><span class="cx"> explicit IncrementalSweeper(VM*);
</span><span class="cx"> #endif
</span><span class="cx">
</span><del>- void startSweeping(Vector<MarkedBlock*>&&);
- void addBlocksAndContinueSweeping(Vector<MarkedBlock*>&&);
</del><ins>+ void startSweeping();
</ins><span class="cx">
</span><span class="cx"> JS_EXPORT_PRIVATE virtual void doWork() override;
</span><span class="cx"> bool sweepNextBlock();
</span></span></pre>
</div>
</div>
</body>
</html>