<!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>[195877] 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/195877">195877</a></dd>
<dt>Author</dt> <dd>fpizlo@apple.com</dd>
<dt>Date</dt> <dd>2016-01-29 18:36:14 -0800 (Fri, 29 Jan 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>CallLinkStatus should trust BadCell exit sites whenever there is no stub
https://bugs.webkit.org/show_bug.cgi?id=153691
Reviewed by Benjamin Poulain.
This fixes a regression in our treatment of inlining failure exit sites when deciding if we
should inline a call.
A long time ago, a BadCell exit site would ensure that a CallLinkStatus returned
takesSlowPath.
But then we added closure calls. A BadCell exit site might just mean that we should do
closure call inlining. We added a BadExecutable exit site to indicate that even closure call
inlining had failed. BadCell would no longer force CallLinkStatus to return takesSlowPath,
but BadExecutable would stuff do so.
But then we unified the IR for checking functions and executables, and the DFG stopped using
the BadExecutable exit kind. Probably this change disabled our ability to use exit site
counting for deciding when to takesSlowPath. But this isn't necessarily a disaster, since
any time you exited in this way, you'd be taken to a baseline call inline cache, and that
inline cache would record the slow path.
But then we introduced polymorphic call inlining. Polymorphic call inlining means that call
unlinking, like when one of the callees is optimized, will reset the stub. We also made it
so that the stub is like a gate for the slow path count. A clear inline cache must first
cause the creation of a stub and then cause it to overflow before the slow path is counted.
So, if the DFG or FTL exits on a cell check associated with a particular callsite being
speculatively inlined, then it's possible that nobody will know about the exit because:
- The exit kind is BadCell but CallLinkStatus needs BadExecutable to disable inlining.
- Between when we tiered up to the DFG (or FTL) and when the exit happened, one of the
callees was tiered up, causing the baseline CallLinkInfo to be unlinked. Therefore, after
the exit, the inline cache is in a reset state and will not count the call as taking slow
path.
The challenge when dealing with this is that often, we will have an super early compilation
of a minimorphic call site before we have seen all of its small set of callees. For example
we may have seen only one of two possible callees. That early compilation will OSR exit, and
in trunk, will be recompiled with bimorphic speculative inlining. That's a pretty good
outcome. Basically, we are trusting that if during the time that the function has been
running prior to a given compilation, a callsite has only seen a small number of callees,
then it's safe to assume that it won't see another one anytime soon.
So, simply forcing the CallLinkStatus to set takesSlowPath any time there was a BadCell exit
would hurt our performance in some cases, because trunk prior to this change would have their
final compilation use speculative inlining, and this change would make guarded inlining
instead.
The compromise that I came up with relies on the fact that a CallLinkInfo must be reset quite
frequently for it to routinely happen in between tier-up to DFG (or FTL) and an exit. So,
most likely, such a CallLinkInfo will also show up as being clear when the CallLinkStatus
is built during DFG profiling. The CallLinkStatus will then fall back on the CallLinkInfo's
lastSeenCallee field, which is persistent across resets. This change just makes it so that
CallLinkStatus sets takesSlowPath if there is a BadCell exit and the status had to be
inferred from the lastSeenCallee.
This change reduces pointless recompilations in gbemu. It's an 11% speed-up on gbemu. It
doesn't seem to hurt any benchmarks.
* bytecode/CallLinkStatus.cpp:
(JSC::CallLinkStatus::computeFor):
(JSC::CallLinkStatus::computeExitSiteData):
(JSC::CallLinkStatus::computeFromCallLinkInfo):
* bytecode/CallLinkStatus.h:
(JSC::CallLinkStatus::CallLinkStatus):
(JSC::CallLinkStatus::at):
(JSC::CallLinkStatus::operator[]):
(JSC::CallLinkStatus::isProved):
(JSC::CallLinkStatus::isBasedOnStub):
(JSC::CallLinkStatus::canOptimize):
(JSC::CallLinkStatus::maxNumArguments):
(JSC::CallLinkStatus::ExitSiteData::ExitSiteData): Deleted.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCorebytecodeCallLinkStatuscpp">trunk/Source/JavaScriptCore/bytecode/CallLinkStatus.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCorebytecodeCallLinkStatush">trunk/Source/JavaScriptCore/bytecode/CallLinkStatus.h</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (195876 => 195877)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2016-01-30 02:05:17 UTC (rev 195876)
+++ trunk/Source/JavaScriptCore/ChangeLog 2016-01-30 02:36:14 UTC (rev 195877)
</span><span class="lines">@@ -1,3 +1,80 @@
</span><ins>+2016-01-29 Filip Pizlo <fpizlo@apple.com>
+
+ CallLinkStatus should trust BadCell exit sites whenever there is no stub
+ https://bugs.webkit.org/show_bug.cgi?id=153691
+
+ Reviewed by Benjamin Poulain.
+
+ This fixes a regression in our treatment of inlining failure exit sites when deciding if we
+ should inline a call.
+
+ A long time ago, a BadCell exit site would ensure that a CallLinkStatus returned
+ takesSlowPath.
+
+ But then we added closure calls. A BadCell exit site might just mean that we should do
+ closure call inlining. We added a BadExecutable exit site to indicate that even closure call
+ inlining had failed. BadCell would no longer force CallLinkStatus to return takesSlowPath,
+ but BadExecutable would stuff do so.
+
+ But then we unified the IR for checking functions and executables, and the DFG stopped using
+ the BadExecutable exit kind. Probably this change disabled our ability to use exit site
+ counting for deciding when to takesSlowPath. But this isn't necessarily a disaster, since
+ any time you exited in this way, you'd be taken to a baseline call inline cache, and that
+ inline cache would record the slow path.
+
+ But then we introduced polymorphic call inlining. Polymorphic call inlining means that call
+ unlinking, like when one of the callees is optimized, will reset the stub. We also made it
+ so that the stub is like a gate for the slow path count. A clear inline cache must first
+ cause the creation of a stub and then cause it to overflow before the slow path is counted.
+
+ So, if the DFG or FTL exits on a cell check associated with a particular callsite being
+ speculatively inlined, then it's possible that nobody will know about the exit because:
+
+ - The exit kind is BadCell but CallLinkStatus needs BadExecutable to disable inlining.
+
+ - Between when we tiered up to the DFG (or FTL) and when the exit happened, one of the
+ callees was tiered up, causing the baseline CallLinkInfo to be unlinked. Therefore, after
+ the exit, the inline cache is in a reset state and will not count the call as taking slow
+ path.
+
+ The challenge when dealing with this is that often, we will have an super early compilation
+ of a minimorphic call site before we have seen all of its small set of callees. For example
+ we may have seen only one of two possible callees. That early compilation will OSR exit, and
+ in trunk, will be recompiled with bimorphic speculative inlining. That's a pretty good
+ outcome. Basically, we are trusting that if during the time that the function has been
+ running prior to a given compilation, a callsite has only seen a small number of callees,
+ then it's safe to assume that it won't see another one anytime soon.
+
+ So, simply forcing the CallLinkStatus to set takesSlowPath any time there was a BadCell exit
+ would hurt our performance in some cases, because trunk prior to this change would have their
+ final compilation use speculative inlining, and this change would make guarded inlining
+ instead.
+
+ The compromise that I came up with relies on the fact that a CallLinkInfo must be reset quite
+ frequently for it to routinely happen in between tier-up to DFG (or FTL) and an exit. So,
+ most likely, such a CallLinkInfo will also show up as being clear when the CallLinkStatus
+ is built during DFG profiling. The CallLinkStatus will then fall back on the CallLinkInfo's
+ lastSeenCallee field, which is persistent across resets. This change just makes it so that
+ CallLinkStatus sets takesSlowPath if there is a BadCell exit and the status had to be
+ inferred from the lastSeenCallee.
+
+ This change reduces pointless recompilations in gbemu. It's an 11% speed-up on gbemu. It
+ doesn't seem to hurt any benchmarks.
+
+ * bytecode/CallLinkStatus.cpp:
+ (JSC::CallLinkStatus::computeFor):
+ (JSC::CallLinkStatus::computeExitSiteData):
+ (JSC::CallLinkStatus::computeFromCallLinkInfo):
+ * bytecode/CallLinkStatus.h:
+ (JSC::CallLinkStatus::CallLinkStatus):
+ (JSC::CallLinkStatus::at):
+ (JSC::CallLinkStatus::operator[]):
+ (JSC::CallLinkStatus::isProved):
+ (JSC::CallLinkStatus::isBasedOnStub):
+ (JSC::CallLinkStatus::canOptimize):
+ (JSC::CallLinkStatus::maxNumArguments):
+ (JSC::CallLinkStatus::ExitSiteData::ExitSiteData): Deleted.
+
</ins><span class="cx"> 2016-01-29 Saam barati <sbarati@apple.com>
</span><span class="cx">
</span><span class="cx"> Pack FunctionExecutable and UnlinkedFunctionExecutable harder
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorebytecodeCallLinkStatuscpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/bytecode/CallLinkStatus.cpp (195876 => 195877)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/bytecode/CallLinkStatus.cpp 2016-01-30 02:05:17 UTC (rev 195876)
+++ trunk/Source/JavaScriptCore/bytecode/CallLinkStatus.cpp 2016-01-30 02:36:14 UTC (rev 195877)
</span><span class="lines">@@ -1,5 +1,5 @@
</span><span class="cx"> /*
</span><del>- * Copyright (C) 2012-2015 Apple Inc. All rights reserved.
</del><ins>+ * Copyright (C) 2012-2016 Apple Inc. All rights reserved.
</ins><span class="cx"> *
</span><span class="cx"> * Redistribution and use in source and binary forms, with or without
</span><span class="cx"> * modification, are permitted provided that the following conditions
</span><span class="lines">@@ -90,7 +90,7 @@
</span><span class="cx">
</span><span class="cx"> CallLinkInfo* callLinkInfo = map.get(CodeOrigin(bytecodeIndex));
</span><span class="cx"> if (!callLinkInfo) {
</span><del>- if (exitSiteData.m_takesSlowPath)
</del><ins>+ if (exitSiteData.takesSlowPath)
</ins><span class="cx"> return takesSlowPath();
</span><span class="cx"> return computeFromLLInt(locker, profiledBlock, bytecodeIndex);
</span><span class="cx"> }
</span><span class="lines">@@ -107,10 +107,10 @@
</span><span class="cx"> ExitSiteData exitSiteData;
</span><span class="cx">
</span><span class="cx"> #if ENABLE(DFG_JIT)
</span><del>- exitSiteData.m_takesSlowPath =
</del><ins>+ exitSiteData.takesSlowPath =
</ins><span class="cx"> profiledBlock->hasExitSite(locker, DFG::FrequentExitSite(bytecodeIndex, BadType))
</span><span class="cx"> || profiledBlock->hasExitSite(locker, DFG::FrequentExitSite(bytecodeIndex, BadExecutable));
</span><del>- exitSiteData.m_badFunction =
</del><ins>+ exitSiteData.badFunction =
</ins><span class="cx"> profiledBlock->hasExitSite(locker, DFG::FrequentExitSite(bytecodeIndex, BadCell));
</span><span class="cx"> #else
</span><span class="cx"> UNUSED_PARAM(locker);
</span><span class="lines">@@ -208,6 +208,7 @@
</span><span class="cx"> CallLinkStatus result;
</span><span class="cx"> result.m_variants = variants;
</span><span class="cx"> result.m_couldTakeSlowPath = !!totalCallsToUnknown;
</span><ins>+ result.m_isBasedOnStub = true;
</ins><span class="cx"> return result;
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -230,9 +231,18 @@
</span><span class="cx"> ExitSiteData exitSiteData)
</span><span class="cx"> {
</span><span class="cx"> CallLinkStatus result = computeFor(locker, profiledBlock, callLinkInfo);
</span><del>- if (exitSiteData.m_badFunction)
- result.makeClosureCall();
- if (exitSiteData.m_takesSlowPath)
</del><ins>+ if (exitSiteData.badFunction) {
+ if (result.isBasedOnStub()) {
+ // If we have a polymorphic stub, then having an exit site is not quite so useful. In
+ // most cases, the information in the stub has higher fidelity.
+ result.makeClosureCall();
+ } else {
+ // We might not have a polymorphic stub for any number of reasons. When this happens, we
+ // are in less certain territory, so exit sites mean a lot.
+ result.m_couldTakeSlowPath = true;
+ }
+ }
+ if (exitSiteData.takesSlowPath)
</ins><span class="cx"> result.m_couldTakeSlowPath = true;
</span><span class="cx">
</span><span class="cx"> return result;
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorebytecodeCallLinkStatush"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/bytecode/CallLinkStatus.h (195876 => 195877)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/bytecode/CallLinkStatus.h 2016-01-30 02:05:17 UTC (rev 195876)
+++ trunk/Source/JavaScriptCore/bytecode/CallLinkStatus.h 2016-01-30 02:36:14 UTC (rev 195877)
</span><span class="lines">@@ -1,5 +1,5 @@
</span><span class="cx"> /*
</span><del>- * Copyright (C) 2012-2015 Apple Inc. All rights reserved.
</del><ins>+ * Copyright (C) 2012-2016 Apple Inc. All rights reserved.
</ins><span class="cx"> *
</span><span class="cx"> * Redistribution and use in source and binary forms, with or without
</span><span class="cx"> * modification, are permitted provided that the following conditions
</span><span class="lines">@@ -48,8 +48,6 @@
</span><span class="cx"> WTF_MAKE_FAST_ALLOCATED;
</span><span class="cx"> public:
</span><span class="cx"> CallLinkStatus()
</span><del>- : m_couldTakeSlowPath(false)
- , m_isProved(false)
</del><span class="cx"> {
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -64,8 +62,6 @@
</span><span class="cx">
</span><span class="cx"> CallLinkStatus(CallVariant variant)
</span><span class="cx"> : m_variants(1, variant)
</span><del>- , m_couldTakeSlowPath(false)
- , m_isProved(false)
</del><span class="cx"> {
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -73,14 +69,8 @@
</span><span class="cx"> CodeBlock*, unsigned bytecodeIndex, const CallLinkInfoMap&);
</span><span class="cx">
</span><span class="cx"> struct ExitSiteData {
</span><del>- ExitSiteData()
- : m_takesSlowPath(false)
- , m_badFunction(false)
- {
- }
-
- bool m_takesSlowPath;
- bool m_badFunction;
</del><ins>+ bool takesSlowPath { false };
+ bool badFunction { false };
</ins><span class="cx"> };
</span><span class="cx"> static ExitSiteData computeExitSiteData(const ConcurrentJITLocker&, CodeBlock*, unsigned bytecodeIndex);
</span><span class="cx">
</span><span class="lines">@@ -116,8 +106,9 @@
</span><span class="cx"> CallVariant at(unsigned i) const { return m_variants[i]; }
</span><span class="cx"> CallVariant operator[](unsigned i) const { return at(i); }
</span><span class="cx"> bool isProved() const { return m_isProved; }
</span><ins>+ bool isBasedOnStub() const { return m_isBasedOnStub; }
</ins><span class="cx"> bool canOptimize() const { return !m_variants.isEmpty(); }
</span><del>-
</del><ins>+
</ins><span class="cx"> bool isClosureCall() const; // Returns true if any callee is a closure call.
</span><span class="cx">
</span><span class="cx"> unsigned maxNumArguments() const { return m_maxNumArguments; }
</span><span class="lines">@@ -134,9 +125,10 @@
</span><span class="cx"> #endif
</span><span class="cx">
</span><span class="cx"> CallVariantList m_variants;
</span><del>- bool m_couldTakeSlowPath;
- bool m_isProved;
- unsigned m_maxNumArguments;
</del><ins>+ bool m_couldTakeSlowPath { false };
+ bool m_isProved { false };
+ bool m_isBasedOnStub { false };
+ unsigned m_maxNumArguments { 0 };
</ins><span class="cx"> };
</span><span class="cx">
</span><span class="cx"> } // namespace JSC
</span></span></pre>
</div>
</div>
</body>
</html>