<!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>[185161] 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/185161">185161</a></dd>
<dt>Author</dt> <dd>fpizlo@apple.com</dd>
<dt>Date</dt> <dd>2015-06-03 13:08:01 -0700 (Wed, 03 Jun 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>CallLinkStatus should return takesSlowPath if the GC often cleared the IC
https://bugs.webkit.org/show_bug.cgi?id=145502
Reviewed by Geoffrey Garen.
CallLinkInfo now remembers when it has been cleared by GC. This has some safeguards for when
a call gets cleared by GC only because we hadn't converted it into a closure call; in that
case the GC will just tell us that it should be a closure call. The DFG will not optimize
a call that was cleared by GC, and the DFG will always prefer a closure call if the GC told
us that the specific callee was dead but the executable wasn't.
This guards us from some scenarios that came up in Speedometer. It's neutral on the pure JS
benchmarks, most likely just because those benchmarks aren't real enough to have interesting
GC of code.
* bytecode/CallLinkInfo.cpp:
(JSC::CallLinkInfo::visitWeak):
(JSC::CallLinkInfo::dummy):
* bytecode/CallLinkInfo.h:
(JSC::CallLinkInfo::CallLinkInfo):
* bytecode/CallLinkStatus.cpp:
(JSC::CallLinkStatus::computeFromCallLinkInfo):</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCorebytecodeCallLinkInfocpp">trunk/Source/JavaScriptCore/bytecode/CallLinkInfo.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCorebytecodeCallLinkInfoh">trunk/Source/JavaScriptCore/bytecode/CallLinkInfo.h</a></li>
<li><a href="#trunkSourceJavaScriptCorebytecodeCallLinkStatuscpp">trunk/Source/JavaScriptCore/bytecode/CallLinkStatus.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCorejitRepatchcpp">trunk/Source/JavaScriptCore/jit/Repatch.cpp</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (185160 => 185161)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2015-06-03 20:04:00 UTC (rev 185160)
+++ trunk/Source/JavaScriptCore/ChangeLog 2015-06-03 20:08:01 UTC (rev 185161)
</span><span class="lines">@@ -1,3 +1,28 @@
</span><ins>+2015-06-01 Filip Pizlo <fpizlo@apple.com>
+
+ CallLinkStatus should return takesSlowPath if the GC often cleared the IC
+ https://bugs.webkit.org/show_bug.cgi?id=145502
+
+ Reviewed by Geoffrey Garen.
+
+ CallLinkInfo now remembers when it has been cleared by GC. This has some safeguards for when
+ a call gets cleared by GC only because we hadn't converted it into a closure call; in that
+ case the GC will just tell us that it should be a closure call. The DFG will not optimize
+ a call that was cleared by GC, and the DFG will always prefer a closure call if the GC told
+ us that the specific callee was dead but the executable wasn't.
+
+ This guards us from some scenarios that came up in Speedometer. It's neutral on the pure JS
+ benchmarks, most likely just because those benchmarks aren't real enough to have interesting
+ GC of code.
+
+ * bytecode/CallLinkInfo.cpp:
+ (JSC::CallLinkInfo::visitWeak):
+ (JSC::CallLinkInfo::dummy):
+ * bytecode/CallLinkInfo.h:
+ (JSC::CallLinkInfo::CallLinkInfo):
+ * bytecode/CallLinkStatus.cpp:
+ (JSC::CallLinkStatus::computeFromCallLinkInfo):
+
</ins><span class="cx"> 2015-06-02 Filip Pizlo <fpizlo@apple.com>
</span><span class="cx">
</span><span class="cx"> GetById and PutById profiling should be more precise about it takes slow path
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorebytecodeCallLinkInfocpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/bytecode/CallLinkInfo.cpp (185160 => 185161)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/bytecode/CallLinkInfo.cpp 2015-06-03 20:04:00 UTC (rev 185160)
+++ trunk/Source/JavaScriptCore/bytecode/CallLinkInfo.cpp 2015-06-03 20:08:01 UTC (rev 185161)
</span><span class="lines">@@ -58,6 +58,13 @@
</span><span class="cx">
</span><span class="cx"> void CallLinkInfo::visitWeak(RepatchBuffer& repatchBuffer)
</span><span class="cx"> {
</span><ins>+ auto handleSpecificCallee = [&] (JSFunction* callee) {
+ if (Heap::isMarked(callee->executable()))
+ hasSeenClosure = true;
+ else
+ clearedByGC = true;
+ };
+
</ins><span class="cx"> if (isLinked()) {
</span><span class="cx"> if (stub) {
</span><span class="cx"> if (!stub->visitWeak(repatchBuffer)) {
</span><span class="lines">@@ -68,6 +75,7 @@
</span><span class="cx"> ".\n");
</span><span class="cx"> }
</span><span class="cx"> unlink(repatchBuffer);
</span><ins>+ clearedByGC = true;
</ins><span class="cx"> }
</span><span class="cx"> } else if (!Heap::isMarked(callee.get())) {
</span><span class="cx"> if (Options::verboseOSR()) {
</span><span class="lines">@@ -77,11 +85,14 @@
</span><span class="cx"> callee.get()->executable()->hashFor(specializationKind()),
</span><span class="cx"> ").\n");
</span><span class="cx"> }
</span><ins>+ handleSpecificCallee(callee.get());
</ins><span class="cx"> unlink(repatchBuffer);
</span><span class="cx"> }
</span><span class="cx"> }
</span><del>- if (!!lastSeenCallee && !Heap::isMarked(lastSeenCallee.get()))
</del><ins>+ if (!!lastSeenCallee && !Heap::isMarked(lastSeenCallee.get())) {
+ handleSpecificCallee(lastSeenCallee.get());
</ins><span class="cx"> lastSeenCallee.clear();
</span><ins>+ }
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> CallLinkInfo& CallLinkInfo::dummy()
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorebytecodeCallLinkInfoh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/bytecode/CallLinkInfo.h (185160 => 185161)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/bytecode/CallLinkInfo.h 2015-06-03 20:04:00 UTC (rev 185160)
+++ trunk/Source/JavaScriptCore/bytecode/CallLinkInfo.h 2015-06-03 20:08:01 UTC (rev 185161)
</span><span class="lines">@@ -59,6 +59,7 @@
</span><span class="cx"> : isFTL(false)
</span><span class="cx"> , hasSeenShouldRepatch(false)
</span><span class="cx"> , hasSeenClosure(false)
</span><ins>+ , clearedByGC(false)
</ins><span class="cx"> , callType(None)
</span><span class="cx"> , maxNumArguments(0)
</span><span class="cx"> , slowPathCount(0)
</span><span class="lines">@@ -92,7 +93,8 @@
</span><span class="cx"> bool isFTL : 1;
</span><span class="cx"> bool hasSeenShouldRepatch : 1;
</span><span class="cx"> bool hasSeenClosure : 1;
</span><del>- unsigned callType : 5; // CallType
</del><ins>+ bool clearedByGC : 1;
+ unsigned callType : 4; // CallType
</ins><span class="cx"> unsigned calleeGPR : 8;
</span><span class="cx"> uint8_t maxNumArguments; // Only used for varargs calls.
</span><span class="cx"> uint32_t slowPathCount;
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorebytecodeCallLinkStatuscpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/bytecode/CallLinkStatus.cpp (185160 => 185161)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/bytecode/CallLinkStatus.cpp 2015-06-03 20:04:00 UTC (rev 185160)
+++ trunk/Source/JavaScriptCore/bytecode/CallLinkStatus.cpp 2015-06-03 20:08:01 UTC (rev 185161)
</span><span class="lines">@@ -135,6 +135,9 @@
</span><span class="cx"> CallLinkStatus CallLinkStatus::computeFromCallLinkInfo(
</span><span class="cx"> const ConcurrentJITLocker&, CallLinkInfo& callLinkInfo)
</span><span class="cx"> {
</span><ins>+ if (callLinkInfo.clearedByGC)
+ return takesSlowPath();
+
</ins><span class="cx"> // Note that despite requiring that the locker is held, this code is racy with respect
</span><span class="cx"> // to the CallLinkInfo: it may get cleared while this code runs! This is because
</span><span class="cx"> // CallLinkInfo::unlink() may be called from a different CodeBlock than the one that owns
</span><span class="lines">@@ -148,11 +151,6 @@
</span><span class="cx"> // that is still marginally valid (i.e. the pointers ain't stale). This kind of raciness
</span><span class="cx"> // is probably OK for now.
</span><span class="cx">
</span><del>- // FIXME: If the GC often clears this call, we should probably treat it like it always takes the
- // slow path. We could be smart about this; for example if we cleared a specific callee but the
- // despecified executable was alive then we could note that separately.
- // https://bugs.webkit.org/show_bug.cgi?id=145502
-
</del><span class="cx"> // PolymorphicCallStubRoutine is a GCAwareJITStubRoutine, so if non-null, it will stay alive
</span><span class="cx"> // until next GC even if the CallLinkInfo is concurrently cleared. Also, the variants list is
</span><span class="cx"> // never mutated after the PolymorphicCallStubRoutine is instantiated. We have some conservative
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorejitRepatchcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/jit/Repatch.cpp (185160 => 185161)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/jit/Repatch.cpp 2015-06-03 20:04:00 UTC (rev 185160)
+++ trunk/Source/JavaScriptCore/jit/Repatch.cpp 2015-06-03 20:08:01 UTC (rev 185161)
</span><span class="lines">@@ -1745,6 +1745,9 @@
</span><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+ if (isClosureCall)
+ callLinkInfo.hasSeenClosure = true;
+
</ins><span class="cx"> Vector<PolymorphicCallCase> callCases;
</span><span class="cx">
</span><span class="cx"> // Figure out what our cases are.
</span></span></pre>
</div>
</div>
</body>
</html>