<!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>[190599] 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/190599">190599</a></dd>
<dt>Author</dt> <dd>fpizlo@apple.com</dd>
<dt>Date</dt> <dd>2015-10-05 18:33:39 -0700 (Mon, 05 Oct 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>GC shouldn't cancel every FTL compilation
https://bugs.webkit.org/show_bug.cgi?id=149821
Reviewed by Saam Barati.
During one of the CodeBlock GC refactorings, we messed up the GC's compilation cancellation
code. The GC should be able to cancel compilation plans if it determines that the plan will
be DOA. But, prior to this fix, that code was killing every FTL compilation. This happened
because the meaning of CodeBlock::isKnownToBeLiveDuringGC() changed.
It's funny that this didn't show up as a bigger slow-down. Basically, those benchmarks that
GC a lot usually don't rely on good compilation, while those benchmarks that do rely on good
compilation usually don't GC a lot. That's probably why this wasn't super obvious when we
broke it.
This change just changes the cancellation logic so that it only cancels plans if the owning
executable is dead. This is safe; in fact the relevant method would be correct even if it
always returned true. It would also be correct if it always returned false. So, compared to
what we had before we changed isKnownToBeLiveDuringGC(), this new code will cancel fewer
compilations. But, that's better than cancelling every compilation. I've filed a bug and
written a FIXME for investigating ways to resurrect the old behavior:
https://bugs.webkit.org/show_bug.cgi?id=149823
Nonetheless, this change looks like it might be a 1% speed-up on Octane. It improves earley
and gbemu.
* dfg/DFGPlan.cpp:
(JSC::DFG::Plan::isKnownToBeLiveDuringGC):</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGPlancpp">trunk/Source/JavaScriptCore/dfg/DFGPlan.cpp</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (190598 => 190599)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2015-10-06 01:31:03 UTC (rev 190598)
+++ trunk/Source/JavaScriptCore/ChangeLog 2015-10-06 01:33:39 UTC (rev 190599)
</span><span class="lines">@@ -1,3 +1,34 @@
</span><ins>+2015-10-05 Filip Pizlo <fpizlo@apple.com>
+
+ GC shouldn't cancel every FTL compilation
+ https://bugs.webkit.org/show_bug.cgi?id=149821
+
+ Reviewed by Saam Barati.
+
+ During one of the CodeBlock GC refactorings, we messed up the GC's compilation cancellation
+ code. The GC should be able to cancel compilation plans if it determines that the plan will
+ be DOA. But, prior to this fix, that code was killing every FTL compilation. This happened
+ because the meaning of CodeBlock::isKnownToBeLiveDuringGC() changed.
+
+ It's funny that this didn't show up as a bigger slow-down. Basically, those benchmarks that
+ GC a lot usually don't rely on good compilation, while those benchmarks that do rely on good
+ compilation usually don't GC a lot. That's probably why this wasn't super obvious when we
+ broke it.
+
+ This change just changes the cancellation logic so that it only cancels plans if the owning
+ executable is dead. This is safe; in fact the relevant method would be correct even if it
+ always returned true. It would also be correct if it always returned false. So, compared to
+ what we had before we changed isKnownToBeLiveDuringGC(), this new code will cancel fewer
+ compilations. But, that's better than cancelling every compilation. I've filed a bug and
+ written a FIXME for investigating ways to resurrect the old behavior:
+ https://bugs.webkit.org/show_bug.cgi?id=149823
+
+ Nonetheless, this change looks like it might be a 1% speed-up on Octane. It improves earley
+ and gbemu.
+
+ * dfg/DFGPlan.cpp:
+ (JSC::DFG::Plan::isKnownToBeLiveDuringGC):
+
</ins><span class="cx"> 2015-10-05 Sukolsak Sakshuwong <sukolsak@gmail.com>
</span><span class="cx">
</span><span class="cx"> [Intl] Change the return type of canonicalizeLocaleList() from JSArray* to Vector<String>
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGPlancpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGPlan.cpp (190598 => 190599)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGPlan.cpp 2015-10-06 01:31:03 UTC (rev 190598)
+++ trunk/Source/JavaScriptCore/dfg/DFGPlan.cpp 2015-10-06 01:33:39 UTC (rev 190599)
</span><span class="lines">@@ -641,12 +641,26 @@
</span><span class="cx"> {
</span><span class="cx"> if (stage == Cancelled)
</span><span class="cx"> return false;
</span><ins>+
+ // NOTE: From here on, this method can return anything and still be sound. It's sound to return
+ // false because then we'll just cancel the compilation. It's always sound to do that. It's sound
+ // to return true because then we'll just keep alive everything that the compilation needs to
+ // have live. The only thing you have to worry about is performance. The goal of returning false
+ // is to try to minimize the likelihood that we expend effort compiling something that will be
+ // DOA - that is, the code being compiled is only reachable from the compilation worklist itself
+ // or the code being compiled is going to have weak references to things that are otherwise dead.
+ // If you return false too often, then you'll regress performance because you might be killing a
+ // compilation that wouldn't have been DOA, and so you're losing opportunities to run faster
+ // code. If you return true too often, then you'll regress performance because you might expend
+ // too much effort compiling something that will be DOA.
+
</ins><span class="cx"> if (!Heap::isMarked(codeBlock->ownerExecutable()))
</span><span class="cx"> return false;
</span><del>- if (!codeBlock->alternative()->isKnownToBeLiveDuringGC())
- return false;
- if (!!profiledDFGCodeBlock && !profiledDFGCodeBlock->isKnownToBeLiveDuringGC())
- return false;
</del><ins>+
+ // FIXME: We could detect if the alternate CodeBlock or the profiled DFG CodeBlock have
+ // experienced troubles and may be jettisoned.
+ // https://bugs.webkit.org/show_bug.cgi?id=149823
+
</ins><span class="cx"> return true;
</span><span class="cx"> }
</span><span class="cx">
</span></span></pre>
</div>
</div>
</body>
</html>