<!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>[192721] 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/192721">192721</a></dd>
<dt>Author</dt> <dd>akling@apple.com</dd>
<dt>Date</dt> <dd>2015-11-20 20:40:40 -0800 (Fri, 20 Nov 2015)</dd>
</dl>

<h3>Log Message</h3>
<pre>GC timers should carry on gracefully when Heap claims it grew from GC.
&lt;https://webkit.org/b/151521&gt;

Reviewed by Mark Lam.

TL;DR the Heap &quot;extra memory&quot; reporting APIs are hard to use 100% correctly
and GC scheduling shouldn't break if someone makes a mistake with it.

The JSC::Heap allows you to report an extra memory cost for any GC object.
This is reported first when allocating the memory, and then each time the
object is visited during the marking phase.

When reporting an allocation, it's added to the Heap's &quot;bytes allocated in
this cycle&quot; counter. This contributes to the computed heap size at the start
of a collection.

When visiting a GC object that reports extra memory, it's added to the Heap's
&quot;extra memory visited in this collection&quot; counter. This contributes to the
computed heap size at the end of a collection.

As you can see, this means that visiting more memory than we said we allocated
can lead to the Heap thinking it's bigger after a collection than it was before.

Clients of this API do some sketchy things to compute costs, for instance
StringImpl cost is determined by dividing the number of bytes used for the
characters, and dividing it by the StringImpl's ref count. Since a JSString
could be backed by any StringImpl, any code that modifies a StringImpl's
ref count during collection will change the extra memory reported by all
JSString objects that wrap that StringImpl.

So anyways...

The object death rate, which is the basis for when to schedule the next
collection is computed like so:

    deathRate = (sizeBeforeGC - sizeAfterGC) / sizeBeforeGC

This patch adds a safety mechanism that returns a zero death rate when the Heap
claims it grew from collection.

* heap/EdenGCActivityCallback.cpp:
(JSC::EdenGCActivityCallback::deathRate):
* heap/FullGCActivityCallback.cpp:
(JSC::FullGCActivityCallback::deathRate):</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapEdenGCActivityCallbackcpp">trunk/Source/JavaScriptCore/heap/EdenGCActivityCallback.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapFullGCActivityCallbackcpp">trunk/Source/JavaScriptCore/heap/FullGCActivityCallback.cpp</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (192720 => 192721)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog        2015-11-21 03:57:19 UTC (rev 192720)
+++ trunk/Source/JavaScriptCore/ChangeLog        2015-11-21 04:40:40 UTC (rev 192721)
</span><span class="lines">@@ -1,3 +1,50 @@
</span><ins>+2015-11-20  Andreas Kling  &lt;akling@apple.com&gt;
+
+        GC timers should carry on gracefully when Heap claims it grew from GC.
+        &lt;https://webkit.org/b/151521&gt;
+
+        Reviewed by Mark Lam.
+
+        TL;DR the Heap &quot;extra memory&quot; reporting APIs are hard to use 100% correctly
+        and GC scheduling shouldn't break if someone makes a mistake with it.
+
+        The JSC::Heap allows you to report an extra memory cost for any GC object.
+        This is reported first when allocating the memory, and then each time the
+        object is visited during the marking phase.
+
+        When reporting an allocation, it's added to the Heap's &quot;bytes allocated in
+        this cycle&quot; counter. This contributes to the computed heap size at the start
+        of a collection.
+
+        When visiting a GC object that reports extra memory, it's added to the Heap's
+        &quot;extra memory visited in this collection&quot; counter. This contributes to the
+        computed heap size at the end of a collection.
+
+        As you can see, this means that visiting more memory than we said we allocated
+        can lead to the Heap thinking it's bigger after a collection than it was before.
+
+        Clients of this API do some sketchy things to compute costs, for instance
+        StringImpl cost is determined by dividing the number of bytes used for the
+        characters, and dividing it by the StringImpl's ref count. Since a JSString
+        could be backed by any StringImpl, any code that modifies a StringImpl's
+        ref count during collection will change the extra memory reported by all
+        JSString objects that wrap that StringImpl.
+
+        So anyways...
+
+        The object death rate, which is the basis for when to schedule the next
+        collection is computed like so:
+
+            deathRate = (sizeBeforeGC - sizeAfterGC) / sizeBeforeGC
+
+        This patch adds a safety mechanism that returns a zero death rate when the Heap
+        claims it grew from collection.
+
+        * heap/EdenGCActivityCallback.cpp:
+        (JSC::EdenGCActivityCallback::deathRate):
+        * heap/FullGCActivityCallback.cpp:
+        (JSC::FullGCActivityCallback::deathRate):
+
</ins><span class="cx"> 2015-11-20  Mark Lam  &lt;mark.lam@apple.com&gt;
</span><span class="cx"> 
</span><span class="cx">         New JSC tests introduced in r192664 fail on ARM.
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapEdenGCActivityCallbackcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/EdenGCActivityCallback.cpp (192720 => 192721)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/EdenGCActivityCallback.cpp        2015-11-21 03:57:19 UTC (rev 192720)
+++ trunk/Source/JavaScriptCore/heap/EdenGCActivityCallback.cpp        2015-11-21 04:40:40 UTC (rev 192721)
</span><span class="lines">@@ -54,6 +54,12 @@
</span><span class="cx">     size_t sizeAfter = heap-&gt;sizeAfterLastEdenCollection();
</span><span class="cx">     if (!sizeBefore)
</span><span class="cx">         return 1.0;
</span><ins>+    if (sizeAfter &gt; sizeBefore) {
+        // GC caused the heap to grow(!)
+        // This could happen if the we visited more extra memory than was reported allocated.
+        // We don't return a negative death rate, since that would schedule the next GC in the past.
+        return 0;
+    }
</ins><span class="cx">     return static_cast&lt;double&gt;(sizeBefore - sizeAfter) / static_cast&lt;double&gt;(sizeBefore);
</span><span class="cx"> }
</span><span class="cx"> 
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapFullGCActivityCallbackcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/FullGCActivityCallback.cpp (192720 => 192721)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/FullGCActivityCallback.cpp        2015-11-21 03:57:19 UTC (rev 192720)
+++ trunk/Source/JavaScriptCore/heap/FullGCActivityCallback.cpp        2015-11-21 04:40:40 UTC (rev 192721)
</span><span class="lines">@@ -70,6 +70,12 @@
</span><span class="cx">     size_t sizeAfter = heap-&gt;sizeAfterLastFullCollection();
</span><span class="cx">     if (!sizeBefore)
</span><span class="cx">         return 1.0;
</span><ins>+    if (sizeAfter &gt; sizeBefore) {
+        // GC caused the heap to grow(!)
+        // This could happen if the we visited more extra memory than was reported allocated.
+        // We don't return a negative death rate, since that would schedule the next GC in the past.
+        return 0;
+    }
</ins><span class="cx">     return static_cast&lt;double&gt;(sizeBefore - sizeAfter) / static_cast&lt;double&gt;(sizeBefore);
</span><span class="cx"> }
</span><span class="cx"> 
</span></span></pre>
</div>
</div>

</body>
</html>