<!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>[211461] 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/211461">211461</a></dd>
<dt>Author</dt> <dd>jfbastien@apple.com</dd>
<dt>Date</dt> <dd>2017-01-31 17:26:00 -0800 (Tue, 31 Jan 2017)</dd>
</dl>
<h3>Log Message</h3>
<pre>OSR entry: delay outer-loop compilation when at inner-loop
https://bugs.webkit.org/show_bug.cgi?id=167149
Reviewed by Filip Pizlo.
<a href="http://trac.webkit.org/projects/webkit/changeset/211224">r211224</a> was reverted because it caused a massive kraken/ai-astar
regression. This patch instead does the minimally-disruptive
change to fix the original bug as described below, but omits extra
tuning and refactoring which I had before. I'll commit tuning and
refactoring separately, if this sticks. This patch is therefore
very minimal, and layers carefully on top of the complex
spaghetti-logic. The only change it makes is that it uses triggers
to indicate to outer loops that they should compile, which fixes
the immediate bug and seems roughly perf neutral (maybe a small
gain on kraken sometimes, other times a small regression as would
be expected from compiling later).
As of https://bugs.webkit.org/show_bug.cgi?id=155217 OSR
compilation can be kicked off for an entry into an outer-loop,
while executing an inner-loop. This is desirable because often the
codegen from an inner-entry isn't as good as the codegen from an
outer-entry, but execution from an inner-loop is often pretty hot
and likely to kick off compilation. This approach provided nice
speedups on Kraken because we'd select to enter to the outer-loop
very reliably, which reduces variability (the inner-loop was
selected roughly 1/5 times from my unscientific measurements).
When compilation starts we take a snapshot of the JSValues at the
current execution state using OSR's recovery mechanism. These
values are passed to the compiler and are used as way to perform
type profiling, and could be used to observe cell types as well as
to perform predictions such as through constant propagation.
It's therefore desired to enter from the outer-loop when we can,
but we need to be executing from that location to capture the
right JSValues, otherwise we're confusing the compiler and giving
it inaccurate JSValues which can lead it to predict the wrong
things, leading to suboptimal code or recompilation due to
misprediction, or in super-corner-cases a crash.
These effects are pretty hard to measure: Fil points out that
marsalis-osr-entry really needs mustHandleValues (the JSValues
from the point of execution) because right now it just happens to
correctly guess int32. I tried removing mustHandleValues entirely
and saw no slowdowns, but our benchmarks probably aren't
sufficient to reliably find issues, sometimes because we happen to
have sufficient mitigations.
DFG tier-up was added here:
https://bugs.webkit.org/show_bug.cgi?id=112838
* dfg/DFGOperations.cpp:</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGOperationscpp">trunk/Source/JavaScriptCore/dfg/DFGOperations.cpp</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (211460 => 211461)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2017-02-01 01:04:03 UTC (rev 211460)
+++ trunk/Source/JavaScriptCore/ChangeLog 2017-02-01 01:26:00 UTC (rev 211461)
</span><span class="lines">@@ -1,3 +1,58 @@
</span><ins>+2017-01-31 JF Bastien <jfbastien@apple.com>
+
+ OSR entry: delay outer-loop compilation when at inner-loop
+ https://bugs.webkit.org/show_bug.cgi?id=167149
+
+ Reviewed by Filip Pizlo.
+
+ r211224 was reverted because it caused a massive kraken/ai-astar
+ regression. This patch instead does the minimally-disruptive
+ change to fix the original bug as described below, but omits extra
+ tuning and refactoring which I had before. I'll commit tuning and
+ refactoring separately, if this sticks. This patch is therefore
+ very minimal, and layers carefully on top of the complex
+ spaghetti-logic. The only change it makes is that it uses triggers
+ to indicate to outer loops that they should compile, which fixes
+ the immediate bug and seems roughly perf neutral (maybe a small
+ gain on kraken sometimes, other times a small regression as would
+ be expected from compiling later).
+
+ As of https://bugs.webkit.org/show_bug.cgi?id=155217 OSR
+ compilation can be kicked off for an entry into an outer-loop,
+ while executing an inner-loop. This is desirable because often the
+ codegen from an inner-entry isn't as good as the codegen from an
+ outer-entry, but execution from an inner-loop is often pretty hot
+ and likely to kick off compilation. This approach provided nice
+ speedups on Kraken because we'd select to enter to the outer-loop
+ very reliably, which reduces variability (the inner-loop was
+ selected roughly 1/5 times from my unscientific measurements).
+
+ When compilation starts we take a snapshot of the JSValues at the
+ current execution state using OSR's recovery mechanism. These
+ values are passed to the compiler and are used as way to perform
+ type profiling, and could be used to observe cell types as well as
+ to perform predictions such as through constant propagation.
+
+ It's therefore desired to enter from the outer-loop when we can,
+ but we need to be executing from that location to capture the
+ right JSValues, otherwise we're confusing the compiler and giving
+ it inaccurate JSValues which can lead it to predict the wrong
+ things, leading to suboptimal code or recompilation due to
+ misprediction, or in super-corner-cases a crash.
+
+ These effects are pretty hard to measure: Fil points out that
+ marsalis-osr-entry really needs mustHandleValues (the JSValues
+ from the point of execution) because right now it just happens to
+ correctly guess int32. I tried removing mustHandleValues entirely
+ and saw no slowdowns, but our benchmarks probably aren't
+ sufficient to reliably find issues, sometimes because we happen to
+ have sufficient mitigations.
+
+ DFG tier-up was added here:
+ https://bugs.webkit.org/show_bug.cgi?id=112838
+
+ * dfg/DFGOperations.cpp:
+
</ins><span class="cx"> 2017-01-31 Filip Pizlo <fpizlo@apple.com>
</span><span class="cx">
</span><span class="cx"> The mutator should be able to perform increments of GC work
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGOperationscpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGOperations.cpp (211460 => 211461)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGOperations.cpp 2017-02-01 01:04:03 UTC (rev 211460)
+++ trunk/Source/JavaScriptCore/dfg/DFGOperations.cpp 2017-02-01 01:26:00 UTC (rev 211461)
</span><span class="lines">@@ -2351,6 +2351,19 @@
</span><span class="cx"> worklistState = Worklist::NotKnown;
</span><span class="cx">
</span><span class="cx"> JITCode* jitCode = codeBlock->jitCode()->dfg();
</span><ins>+
+ bool triggeredSlowPath = false;
+ auto tierUpEntryTriggers = jitCode->tierUpEntryTriggers.find(osrEntryBytecodeIndex);
+ if (tierUpEntryTriggers != jitCode->tierUpEntryTriggers.end()) {
+ if (tierUpEntryTriggers->value == 1) {
+ // We were asked to enter as soon as possible. Unset this trigger so we don't continually enter.
+ if (Options::verboseOSR())
+ dataLog("EntryTrigger for ", *codeBlock, " forced slow-path.\n");
+ triggeredSlowPath = true;
+ tierUpEntryTriggers->value = 0;
+ }
+ }
+
</ins><span class="cx"> if (worklistState == Worklist::Compiling) {
</span><span class="cx"> CODEBLOCK_LOG_EVENT(codeBlock, "delayFTLCompile", ("still compiling"));
</span><span class="cx"> jitCode->setOptimizationThresholdBasedOnCompilationResult(
</span><span class="lines">@@ -2366,8 +2379,12 @@
</span><span class="cx"> return nullptr;
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+ // The following is only true for triggerTierUpNowInLoop, which can never
+ // be an OSR entry.
+ bool canOSRFromHere = originBytecodeIndex == osrEntryBytecodeIndex;
+
</ins><span class="cx"> // If we can OSR Enter, do it right away.
</span><del>- if (originBytecodeIndex == osrEntryBytecodeIndex) {
</del><ins>+ if (canOSRFromHere) {
</ins><span class="cx"> unsigned streamIndex = jitCode->bytecodeIndexToStreamIndex.get(originBytecodeIndex);
</span><span class="cx"> if (CodeBlock* entryBlock = jitCode->osrEntryBlock()) {
</span><span class="cx"> if (void* address = FTL::prepareOSREntry(exec, codeBlock, entryBlock, originBytecodeIndex, streamIndex)) {
</span><span class="lines">@@ -2381,10 +2398,10 @@
</span><span class="cx"> // - If we do have an FTL code block, then try to enter for a while.
</span><span class="cx"> // - If we couldn't enter for a while, then trigger OSR entry.
</span><span class="cx">
</span><del>- if (!shouldTriggerFTLCompile(codeBlock, jitCode))
</del><ins>+ if (!shouldTriggerFTLCompile(codeBlock, jitCode) && !triggeredSlowPath)
</ins><span class="cx"> return nullptr;
</span><span class="cx">
</span><del>- if (!jitCode->neverExecutedEntry) {
</del><ins>+ if (!jitCode->neverExecutedEntry && !triggeredSlowPath) {
</ins><span class="cx"> triggerFTLReplacementCompile(vm, codeBlock, jitCode);
</span><span class="cx">
</span><span class="cx"> if (!codeBlock->hasOptimizedReplacement())
</span><span class="lines">@@ -2430,13 +2447,28 @@
</span><span class="cx"> return nullptr;
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+ if (!canOSRFromHere) {
+ // We can't OSR from here, or even start a compilation because doing so
+ // calls jitCode->reconstruct which would get the wrong state.
+ if (Options::verboseOSR())
+ dataLog("Non-OSR-able bc#", originBytecodeIndex, " in ", *codeBlock, " setting parent loop's trigger and backing off.\n");
+ jitCode->tierUpEntryTriggers.set(osrEntryBytecodeIndex, 1);
+ jitCode->dontOptimizeAnytimeSoon(codeBlock);
+ return nullptr;
+ }
+
</ins><span class="cx"> unsigned streamIndex = jitCode->bytecodeIndexToStreamIndex.get(osrEntryBytecodeIndex);
</span><del>- auto tierUpHierarchyEntry = jitCode->tierUpInLoopHierarchy.find(osrEntryBytecodeIndex);
- if (tierUpHierarchyEntry != jitCode->tierUpInLoopHierarchy.end()) {
- for (unsigned osrEntryCandidate : tierUpHierarchyEntry->value) {
- if (jitCode->tierUpEntrySeen.contains(osrEntryCandidate)) {
- osrEntryBytecodeIndex = osrEntryCandidate;
- streamIndex = jitCode->bytecodeIndexToStreamIndex.get(osrEntryBytecodeIndex);
</del><ins>+
+ if (!triggeredSlowPath) {
+ auto tierUpHierarchyEntry = jitCode->tierUpInLoopHierarchy.find(osrEntryBytecodeIndex);
+ if (tierUpHierarchyEntry != jitCode->tierUpInLoopHierarchy.end()) {
+ for (unsigned osrEntryCandidate : tierUpHierarchyEntry->value) {
+ if (jitCode->tierUpEntrySeen.contains(osrEntryCandidate)) {
+ // Ask an enclosing loop to compile, instead of doing so here.
+ jitCode->tierUpEntryTriggers.set(osrEntryCandidate, 1);
+ jitCode->dontOptimizeAnytimeSoon(codeBlock);
+ return nullptr;
+ }
</ins><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx"> }
</span></span></pre>
</div>
</div>
</body>
</html>