<!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>[199382] trunk/Source</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/199382">199382</a></dd>
<dt>Author</dt> <dd>fpizlo@apple.com</dd>
<dt>Date</dt> <dd>2016-04-12 13:06:26 -0700 (Tue, 12 Apr 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>PolymorphicAccess should buffer AccessCases before regenerating
https://bugs.webkit.org/show_bug.cgi?id=156457
Reviewed by Benjamin Poulain.
Source/JavaScriptCore:
Prior to this change, whenever we added an AccessCase to a PolymorphicAccess, we would
regenerate the whole stub. That meant that we'd do O(N^2) work for N access cases.
One way to fix this is to have each AccessCase generate a stub just for itself, which
cascades down to the already-generated cases. But that removes the binary switch
optimization, which makes the IC perform great even when there are many cases.
This change fixes the issue by buffering access cases. When we take slow path and try to add
a new case, the StructureStubInfo will usually just buffer the new case without generating
new code. We simply guarantee that after we buffer a case, we will take at most
Options::repatchBufferingCountdown() slow path calls before generating code for it. That
option is currently 7. Taking 7 more slow paths means that we have 7 more opportunities to
gather more access cases, or to realize that this IC is too crazy to bother with.
This change ensures that the DFG still gets the same kind of profiling. This is because the
buffered AccessCases are still part of PolymorphicAccess and so are still scanned by
GetByIdStatus and PutByIdStatus. The fact that the AccessCases hadn't been generated and so
hadn't executed doesn't change much. Mainly, it increases the likelihood that the DFG will
see an access case that !couldStillSucceed(). The DFG's existing profile parsing logic can
handle this just fine.
There are a bunch of algorithmic changes here. StructureStubInfo now caches the set of
structures that it has seen as a guard to prevent adding lots of redundant cases, in case
we see the same 7 cases after buffering the first one. This cache means we won't wastefully
allocate 7 identical AccessCase instances. PolymorphicAccess is now restructured around
having separate addCase() and regenerate() calls. That means a bit more moving data around.
So far that seems OK for performance, probably since it's O(N) work rather than O(N^2) work.
There is room for improvement for future patches, to be sure.
This is benchmarking as slightly positive or neutral on JS benchmarks. It's meant to reduce
pathologies I saw in page loads.
* bytecode/GetByIdStatus.cpp:
(JSC::GetByIdStatus::computeForStubInfoWithoutExitSiteFeedback):
* bytecode/PolymorphicAccess.cpp:
(JSC::PolymorphicAccess::PolymorphicAccess):
(JSC::PolymorphicAccess::~PolymorphicAccess):
(JSC::PolymorphicAccess::addCases):
(JSC::PolymorphicAccess::addCase):
(JSC::PolymorphicAccess::visitWeak):
(JSC::PolymorphicAccess::dump):
(JSC::PolymorphicAccess::commit):
(JSC::PolymorphicAccess::regenerate):
(JSC::PolymorphicAccess::aboutToDie):
(WTF::printInternal):
(JSC::PolymorphicAccess::regenerateWithCases): Deleted.
(JSC::PolymorphicAccess::regenerateWithCase): Deleted.
* bytecode/PolymorphicAccess.h:
(JSC::AccessCase::isGetter):
(JSC::AccessCase::callLinkInfo):
(JSC::AccessGenerationResult::AccessGenerationResult):
(JSC::AccessGenerationResult::madeNoChanges):
(JSC::AccessGenerationResult::gaveUp):
(JSC::AccessGenerationResult::buffered):
(JSC::AccessGenerationResult::generatedNewCode):
(JSC::AccessGenerationResult::generatedFinalCode):
(JSC::AccessGenerationResult::shouldGiveUpNow):
(JSC::AccessGenerationResult::generatedSomeCode):
(JSC::PolymorphicAccess::isEmpty):
(JSC::PolymorphicAccess::size):
(JSC::PolymorphicAccess::at):
* bytecode/PutByIdStatus.cpp:
(JSC::PutByIdStatus::computeForStubInfo):
* bytecode/StructureStubInfo.cpp:
(JSC::StructureStubInfo::StructureStubInfo):
(JSC::StructureStubInfo::addAccessCase):
(JSC::StructureStubInfo::reset):
(JSC::StructureStubInfo::visitWeakReferences):
* bytecode/StructureStubInfo.h:
(JSC::StructureStubInfo::considerCaching):
(JSC::StructureStubInfo::willRepatch): Deleted.
(JSC::StructureStubInfo::willCoolDown): Deleted.
* jit/JITOperations.cpp:
* jit/Repatch.cpp:
(JSC::tryCacheGetByID):
(JSC::repatchGetByID):
(JSC::tryCachePutByID):
(JSC::repatchPutByID):
(JSC::tryRepatchIn):
(JSC::repatchIn):
* runtime/JSCJSValue.h:
* runtime/JSCJSValueInlines.h:
(JSC::JSValue::putByIndex):
(JSC::JSValue::structureOrNull):
(JSC::JSValue::structureOrUndefined):
* runtime/Options.h:
Source/WTF:
* wtf/TinyPtrSet.h:
(WTF::TinyPtrSet::add): Add a helpful comment because I had forgotten what the bool return meant.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCorebytecodeGetByIdStatuscpp">trunk/Source/JavaScriptCore/bytecode/GetByIdStatus.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCorebytecodePolymorphicAccesscpp">trunk/Source/JavaScriptCore/bytecode/PolymorphicAccess.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCorebytecodePolymorphicAccessh">trunk/Source/JavaScriptCore/bytecode/PolymorphicAccess.h</a></li>
<li><a href="#trunkSourceJavaScriptCorebytecodePutByIdStatuscpp">trunk/Source/JavaScriptCore/bytecode/PutByIdStatus.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCorebytecodeStructureStubInfocpp">trunk/Source/JavaScriptCore/bytecode/StructureStubInfo.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCorebytecodeStructureStubInfoh">trunk/Source/JavaScriptCore/bytecode/StructureStubInfo.h</a></li>
<li><a href="#trunkSourceJavaScriptCorejitJITOperationscpp">trunk/Source/JavaScriptCore/jit/JITOperations.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCorejitRepatchcpp">trunk/Source/JavaScriptCore/jit/Repatch.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeJSCJSValueh">trunk/Source/JavaScriptCore/runtime/JSCJSValue.h</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeJSCJSValueInlinesh">trunk/Source/JavaScriptCore/runtime/JSCJSValueInlines.h</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeOptionsh">trunk/Source/JavaScriptCore/runtime/Options.h</a></li>
<li><a href="#trunkSourceWTFChangeLog">trunk/Source/WTF/ChangeLog</a></li>
<li><a href="#trunkSourceWTFwtfTinyPtrSeth">trunk/Source/WTF/wtf/TinyPtrSet.h</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (199381 => 199382)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2016-04-12 19:37:37 UTC (rev 199381)
+++ trunk/Source/JavaScriptCore/ChangeLog 2016-04-12 20:06:26 UTC (rev 199382)
</span><span class="lines">@@ -1,3 +1,97 @@
</span><ins>+2016-04-11 Filip Pizlo <fpizlo@apple.com>
+
+ PolymorphicAccess should buffer AccessCases before regenerating
+ https://bugs.webkit.org/show_bug.cgi?id=156457
+
+ Reviewed by Benjamin Poulain.
+
+ Prior to this change, whenever we added an AccessCase to a PolymorphicAccess, we would
+ regenerate the whole stub. That meant that we'd do O(N^2) work for N access cases.
+
+ One way to fix this is to have each AccessCase generate a stub just for itself, which
+ cascades down to the already-generated cases. But that removes the binary switch
+ optimization, which makes the IC perform great even when there are many cases.
+
+ This change fixes the issue by buffering access cases. When we take slow path and try to add
+ a new case, the StructureStubInfo will usually just buffer the new case without generating
+ new code. We simply guarantee that after we buffer a case, we will take at most
+ Options::repatchBufferingCountdown() slow path calls before generating code for it. That
+ option is currently 7. Taking 7 more slow paths means that we have 7 more opportunities to
+ gather more access cases, or to realize that this IC is too crazy to bother with.
+
+ This change ensures that the DFG still gets the same kind of profiling. This is because the
+ buffered AccessCases are still part of PolymorphicAccess and so are still scanned by
+ GetByIdStatus and PutByIdStatus. The fact that the AccessCases hadn't been generated and so
+ hadn't executed doesn't change much. Mainly, it increases the likelihood that the DFG will
+ see an access case that !couldStillSucceed(). The DFG's existing profile parsing logic can
+ handle this just fine.
+
+ There are a bunch of algorithmic changes here. StructureStubInfo now caches the set of
+ structures that it has seen as a guard to prevent adding lots of redundant cases, in case
+ we see the same 7 cases after buffering the first one. This cache means we won't wastefully
+ allocate 7 identical AccessCase instances. PolymorphicAccess is now restructured around
+ having separate addCase() and regenerate() calls. That means a bit more moving data around.
+ So far that seems OK for performance, probably since it's O(N) work rather than O(N^2) work.
+ There is room for improvement for future patches, to be sure.
+
+ This is benchmarking as slightly positive or neutral on JS benchmarks. It's meant to reduce
+ pathologies I saw in page loads.
+
+ * bytecode/GetByIdStatus.cpp:
+ (JSC::GetByIdStatus::computeForStubInfoWithoutExitSiteFeedback):
+ * bytecode/PolymorphicAccess.cpp:
+ (JSC::PolymorphicAccess::PolymorphicAccess):
+ (JSC::PolymorphicAccess::~PolymorphicAccess):
+ (JSC::PolymorphicAccess::addCases):
+ (JSC::PolymorphicAccess::addCase):
+ (JSC::PolymorphicAccess::visitWeak):
+ (JSC::PolymorphicAccess::dump):
+ (JSC::PolymorphicAccess::commit):
+ (JSC::PolymorphicAccess::regenerate):
+ (JSC::PolymorphicAccess::aboutToDie):
+ (WTF::printInternal):
+ (JSC::PolymorphicAccess::regenerateWithCases): Deleted.
+ (JSC::PolymorphicAccess::regenerateWithCase): Deleted.
+ * bytecode/PolymorphicAccess.h:
+ (JSC::AccessCase::isGetter):
+ (JSC::AccessCase::callLinkInfo):
+ (JSC::AccessGenerationResult::AccessGenerationResult):
+ (JSC::AccessGenerationResult::madeNoChanges):
+ (JSC::AccessGenerationResult::gaveUp):
+ (JSC::AccessGenerationResult::buffered):
+ (JSC::AccessGenerationResult::generatedNewCode):
+ (JSC::AccessGenerationResult::generatedFinalCode):
+ (JSC::AccessGenerationResult::shouldGiveUpNow):
+ (JSC::AccessGenerationResult::generatedSomeCode):
+ (JSC::PolymorphicAccess::isEmpty):
+ (JSC::PolymorphicAccess::size):
+ (JSC::PolymorphicAccess::at):
+ * bytecode/PutByIdStatus.cpp:
+ (JSC::PutByIdStatus::computeForStubInfo):
+ * bytecode/StructureStubInfo.cpp:
+ (JSC::StructureStubInfo::StructureStubInfo):
+ (JSC::StructureStubInfo::addAccessCase):
+ (JSC::StructureStubInfo::reset):
+ (JSC::StructureStubInfo::visitWeakReferences):
+ * bytecode/StructureStubInfo.h:
+ (JSC::StructureStubInfo::considerCaching):
+ (JSC::StructureStubInfo::willRepatch): Deleted.
+ (JSC::StructureStubInfo::willCoolDown): Deleted.
+ * jit/JITOperations.cpp:
+ * jit/Repatch.cpp:
+ (JSC::tryCacheGetByID):
+ (JSC::repatchGetByID):
+ (JSC::tryCachePutByID):
+ (JSC::repatchPutByID):
+ (JSC::tryRepatchIn):
+ (JSC::repatchIn):
+ * runtime/JSCJSValue.h:
+ * runtime/JSCJSValueInlines.h:
+ (JSC::JSValue::putByIndex):
+ (JSC::JSValue::structureOrNull):
+ (JSC::JSValue::structureOrUndefined):
+ * runtime/Options.h:
+
</ins><span class="cx"> 2016-04-12 Saam barati <sbarati@apple.com>
</span><span class="cx">
</span><span class="cx"> There is a race with the compiler thread and the main thread with result profiles
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorebytecodeGetByIdStatuscpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/bytecode/GetByIdStatus.cpp (199381 => 199382)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/bytecode/GetByIdStatus.cpp 2016-04-12 19:37:37 UTC (rev 199381)
+++ trunk/Source/JavaScriptCore/bytecode/GetByIdStatus.cpp 2016-04-12 20:06:26 UTC (rev 199382)
</span><span class="lines">@@ -220,11 +220,11 @@
</span><span class="cx"> break;
</span><span class="cx"> }
</span><span class="cx"> case AccessCase::Getter: {
</span><del>- CallLinkInfo* callLinkInfo = access.callLinkInfo();
- ASSERT(callLinkInfo);
- callLinkStatus = std::make_unique<CallLinkStatus>(
- CallLinkStatus::computeFor(
- locker, profiledBlock, *callLinkInfo, callExitSiteData));
</del><ins>+ callLinkStatus = std::make_unique<CallLinkStatus>();
+ if (CallLinkInfo* callLinkInfo = access.callLinkInfo()) {
+ *callLinkStatus = CallLinkStatus::computeFor(
+ locker, profiledBlock, *callLinkInfo, callExitSiteData);
+ }
</ins><span class="cx"> break;
</span><span class="cx"> }
</span><span class="cx"> default: {
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorebytecodePolymorphicAccesscpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/bytecode/PolymorphicAccess.cpp (199381 => 199382)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/bytecode/PolymorphicAccess.cpp 2016-04-12 19:37:37 UTC (rev 199381)
+++ trunk/Source/JavaScriptCore/bytecode/PolymorphicAccess.cpp 2016-04-12 20:06:26 UTC (rev 199382)
</span><span class="lines">@@ -1392,7 +1392,7 @@
</span><span class="cx"> PolymorphicAccess::PolymorphicAccess() { }
</span><span class="cx"> PolymorphicAccess::~PolymorphicAccess() { }
</span><span class="cx">
</span><del>-AccessGenerationResult PolymorphicAccess::regenerateWithCases(
</del><ins>+AccessGenerationResult PolymorphicAccess::addCases(
</ins><span class="cx"> VM& vm, CodeBlock* codeBlock, StructureStubInfo& stubInfo, const Identifier& ident,
</span><span class="cx"> Vector<std::unique_ptr<AccessCase>> originalCasesToAdd)
</span><span class="cx"> {
</span><span class="lines">@@ -1438,76 +1438,26 @@
</span><span class="cx"> if (casesToAdd.isEmpty())
</span><span class="cx"> return AccessGenerationResult::MadeNoChanges;
</span><span class="cx">
</span><del>- // Now construct the list of cases as they should appear if we are successful. This means putting
- // all of the previous cases in this list in order but excluding those that can be replaced, and
- // then adding the new cases.
- ListType newCases;
- for (auto& oldCase : m_list) {
- // Ignore old cases that cannot possibly succeed anymore.
- if (!oldCase->couldStillSucceed())
- continue;
-
- // Figure out if this is replaced by any new cases.
- bool found = false;
- for (auto& caseToAdd : casesToAdd) {
- if (caseToAdd->canReplace(*oldCase)) {
- found = true;
- break;
- }
- }
- if (found)
- continue;
-
- newCases.append(oldCase->clone());
</del><ins>+ // Now add things to the new list. Note that at this point, we will still have old cases that
+ // may be replaced by the new ones. That's fine. We will sort that out when we regenerate.
+ for (auto& caseToAdd : casesToAdd) {
+ commit(vm, m_watchpoints, codeBlock, stubInfo, ident, *caseToAdd);
+ m_list.append(WTFMove(caseToAdd));
</ins><span class="cx"> }
</span><del>- for (auto& caseToAdd : casesToAdd)
- newCases.append(WTFMove(caseToAdd));
-
</del><ins>+
</ins><span class="cx"> if (verbose)
</span><del>- dataLog("newCases: ", listDump(newCases), "\n");
-
- // See if we are close to having too many cases and if some of those cases can be subsumed by a
- // megamorphic load.
- if (newCases.size() >= Options::maxAccessVariantListSize()) {
- unsigned numSelfLoads = 0;
- for (auto& newCase : newCases) {
- if (newCase->canBeReplacedByMegamorphicLoad())
- numSelfLoads++;
- }
-
- if (numSelfLoads >= Options::megamorphicLoadCost()) {
- if (auto mega = AccessCase::megamorphicLoad(vm, codeBlock)) {
- newCases.removeAllMatching(
- [&] (std::unique_ptr<AccessCase>& newCase) -> bool {
- return newCase->canBeReplacedByMegamorphicLoad();
- });
-
- newCases.append(WTFMove(mega));
- }
- }
- }
</del><ins>+ dataLog("After addCases: m_list: ", listDump(m_list), "\n");
</ins><span class="cx">
</span><del>- if (newCases.size() > Options::maxAccessVariantListSize()) {
- if (verbose)
- dataLog("Too many cases.\n");
- return AccessGenerationResult::GaveUp;
- }
-
- MacroAssemblerCodePtr result = regenerate(vm, codeBlock, stubInfo, ident, newCases);
- if (!result)
- return AccessGenerationResult::GaveUp;
-
- m_list = WTFMove(newCases);
- return result;
</del><ins>+ return AccessGenerationResult::Buffered;
</ins><span class="cx"> }
</span><span class="cx">
</span><del>-AccessGenerationResult PolymorphicAccess::regenerateWithCase(
</del><ins>+AccessGenerationResult PolymorphicAccess::addCase(
</ins><span class="cx"> VM& vm, CodeBlock* codeBlock, StructureStubInfo& stubInfo, const Identifier& ident,
</span><span class="cx"> std::unique_ptr<AccessCase> newAccess)
</span><span class="cx"> {
</span><span class="cx"> Vector<std::unique_ptr<AccessCase>> newAccesses;
</span><span class="cx"> newAccesses.append(WTFMove(newAccess));
</span><del>- return regenerateWithCases(vm, codeBlock, stubInfo, ident, WTFMove(newAccesses));
</del><ins>+ return addCases(vm, codeBlock, stubInfo, ident, WTFMove(newAccesses));
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> bool PolymorphicAccess::visitWeak(VM& vm) const
</span><span class="lines">@@ -1534,14 +1484,32 @@
</span><span class="cx"> out.print("]");
</span><span class="cx"> }
</span><span class="cx">
</span><del>-MacroAssemblerCodePtr PolymorphicAccess::regenerate(
- VM& vm, CodeBlock* codeBlock, StructureStubInfo& stubInfo, const Identifier& ident,
- PolymorphicAccess::ListType& cases)
</del><ins>+void PolymorphicAccess::commit(
+ VM& vm, std::unique_ptr<WatchpointsOnStructureStubInfo>& watchpoints, CodeBlock* codeBlock,
+ StructureStubInfo& stubInfo, const Identifier& ident, AccessCase& accessCase)
</ins><span class="cx"> {
</span><ins>+ // NOTE: We currently assume that this is relatively rare. It mainly arises for accesses to
+ // properties on DOM nodes. For sure we cache many DOM node accesses, but even in
+ // Real Pages (TM), we appear to spend most of our time caching accesses to properties on
+ // vanilla objects or exotic objects from within JSC (like Arguments, those are super popular).
+ // Those common kinds of JSC object accesses don't hit this case.
+
+ for (WatchpointSet* set : accessCase.commit(vm, ident)) {
+ Watchpoint* watchpoint =
+ WatchpointsOnStructureStubInfo::ensureReferenceAndAddWatchpoint(
+ watchpoints, codeBlock, &stubInfo, ObjectPropertyCondition());
+
+ set->add(watchpoint);
+ }
+}
+
+AccessGenerationResult PolymorphicAccess::regenerate(
+ VM& vm, CodeBlock* codeBlock, StructureStubInfo& stubInfo, const Identifier& ident)
+{
</ins><span class="cx"> SuperSamplerScope superSamplerScope(false);
</span><span class="cx">
</span><span class="cx"> if (verbose)
</span><del>- dataLog("Generating code for cases: ", listDump(cases), "\n");
</del><ins>+ dataLog("Regenerate with m_list: ", listDump(m_list), "\n");
</ins><span class="cx">
</span><span class="cx"> AccessGenerationState state;
</span><span class="cx">
</span><span class="lines">@@ -1572,14 +1540,73 @@
</span><span class="cx"> state.preservedReusedRegisterState =
</span><span class="cx"> allocator.preserveReusedRegistersByPushing(jit, ScratchRegisterAllocator::ExtraStackSpace::NoExtraSpace);
</span><span class="cx">
</span><ins>+ // Regenerating is our opportunity to figure out what our list of cases should look like. We
+ // do this here. The newly produced 'cases' list may be smaller than m_list. We don't edit
+ // m_list in-place because we may still fail, in which case we want the PolymorphicAccess object
+ // to be unmutated. For sure, we want it to hang onto any data structures that may be referenced
+ // from the code of the current stub (aka previous).
+ ListType cases;
+ for (unsigned i = 0; i < m_list.size(); ++i) {
+ AccessCase& someCase = *m_list[i];
+ // Ignore cases that cannot possibly succeed anymore.
+ if (!someCase.couldStillSucceed())
+ continue;
+
+ // Figure out if this is replaced by any later case.
+ bool found = false;
+ for (unsigned j = i + 1; j < m_list.size(); ++j) {
+ if (m_list[j]->canReplace(someCase)) {
+ found = true;
+ break;
+ }
+ }
+ if (found)
+ continue;
+
+ // FIXME: Do we have to clone cases that aren't generated? Maybe we can just take those
+ // from m_list, since we don't have to keep those alive if we fail.
+ // https://bugs.webkit.org/show_bug.cgi?id=156493
+ cases.append(someCase.clone());
+ }
+
+ if (verbose)
+ dataLog("In regenerate: cases: ", listDump(cases), "\n");
+
+ // Now that we've removed obviously unnecessary cases, we can check if the megamorphic load
+ // optimization is applicable. Note that we basically tune megamorphicLoadCost according to code
+ // size. It would be faster to just allow more repatching with many load cases, and avoid the
+ // megamorphicLoad optimization, if we had infinite executable memory.
+ if (cases.size() >= Options::megamorphicLoadCost()) {
+ unsigned numSelfLoads = 0;
+ for (auto& newCase : cases) {
+ if (newCase->canBeReplacedByMegamorphicLoad())
+ numSelfLoads++;
+ }
+
+ if (numSelfLoads >= Options::megamorphicLoadCost()) {
+ if (auto mega = AccessCase::megamorphicLoad(vm, codeBlock)) {
+ cases.removeAllMatching(
+ [&] (std::unique_ptr<AccessCase>& newCase) -> bool {
+ return newCase->canBeReplacedByMegamorphicLoad();
+ });
+
+ cases.append(WTFMove(mega));
+ }
+ }
+ }
+
+ if (verbose)
+ dataLog("Optimized cases: ", listDump(cases), "\n");
+
+ // At this point we're convinced that 'cases' contains the cases that we want to JIT now and we
+ // won't change that set anymore.
+
</ins><span class="cx"> bool allGuardedByStructureCheck = true;
</span><span class="cx"> bool hasJSGetterSetterCall = false;
</span><del>- for (auto& entry : cases) {
- for (WatchpointSet* set : entry->commit(vm, ident))
- set->add(state.addWatchpoint());
-
- allGuardedByStructureCheck &= entry->guardedByStructureCheck();
- if (entry->type() == AccessCase::Getter || entry->type() == AccessCase::Setter)
</del><ins>+ for (auto& newCase : cases) {
+ commit(vm, state.watchpoints, codeBlock, stubInfo, ident, *newCase);
+ allGuardedByStructureCheck &= newCase->guardedByStructureCheck();
+ if (newCase->type() == AccessCase::Getter || newCase->type() == AccessCase::Setter)
</ins><span class="cx"> hasJSGetterSetterCall = true;
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -1677,7 +1704,7 @@
</span><span class="cx"> if (linkBuffer.didFailToAllocate()) {
</span><span class="cx"> if (verbose)
</span><span class="cx"> dataLog("Did fail to allocate.\n");
</span><del>- return MacroAssemblerCodePtr();
</del><ins>+ return AccessGenerationResult::GaveUp;
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> CodeLocationLabel successLabel =
</span><span class="lines">@@ -1707,12 +1734,22 @@
</span><span class="cx"> m_weakReferences = std::make_unique<Vector<WriteBarrier<JSCell>>>(WTFMove(state.weakReferences));
</span><span class="cx"> if (verbose)
</span><span class="cx"> dataLog("Returning: ", code.code(), "\n");
</span><del>- return code.code();
</del><ins>+
+ m_list = WTFMove(cases);
+
+ AccessGenerationResult::Kind resultKind;
+ if (m_list.size() >= Options::maxAccessVariantListSize())
+ resultKind = AccessGenerationResult::GeneratedFinalCode;
+ else
+ resultKind = AccessGenerationResult::GeneratedNewCode;
+
+ return AccessGenerationResult(resultKind, code.code());
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void PolymorphicAccess::aboutToDie()
</span><span class="cx"> {
</span><del>- m_stubRoutine->aboutToDie();
</del><ins>+ if (m_stubRoutine)
+ m_stubRoutine->aboutToDie();
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> } // namespace JSC
</span><span class="lines">@@ -1730,9 +1767,15 @@
</span><span class="cx"> case AccessGenerationResult::GaveUp:
</span><span class="cx"> out.print("GaveUp");
</span><span class="cx"> return;
</span><ins>+ case AccessGenerationResult::Buffered:
+ out.print("Buffered");
+ return;
</ins><span class="cx"> case AccessGenerationResult::GeneratedNewCode:
</span><span class="cx"> out.print("GeneratedNewCode");
</span><span class="cx"> return;
</span><ins>+ case AccessGenerationResult::GeneratedFinalCode:
+ out.print("GeneratedFinalCode");
+ return;
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> RELEASE_ASSERT_NOT_REACHED();
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorebytecodePolymorphicAccessh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/bytecode/PolymorphicAccess.h (199381 => 199382)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/bytecode/PolymorphicAccess.h 2016-04-12 19:37:37 UTC (rev 199381)
+++ trunk/Source/JavaScriptCore/bytecode/PolymorphicAccess.h 2016-04-12 20:06:26 UTC (rev 199382)
</span><span class="lines">@@ -216,6 +216,8 @@
</span><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+ // This can return null even for a getter/setter, if it hasn't been generated yet. That's
+ // actually somewhat likely because of how we do buffering of new cases.
</ins><span class="cx"> CallLinkInfo* callLinkInfo() const
</span><span class="cx"> {
</span><span class="cx"> if (!m_rareData)
</span><span class="lines">@@ -309,7 +311,9 @@
</span><span class="cx"> enum Kind {
</span><span class="cx"> MadeNoChanges,
</span><span class="cx"> GaveUp,
</span><del>- GeneratedNewCode
</del><ins>+ Buffered,
+ GeneratedNewCode,
+ GeneratedFinalCode // Generated so much code that we never want to generate code again.
</ins><span class="cx"> };
</span><span class="cx">
</span><span class="cx"> AccessGenerationResult()
</span><span class="lines">@@ -319,13 +323,15 @@
</span><span class="cx"> AccessGenerationResult(Kind kind)
</span><span class="cx"> : m_kind(kind)
</span><span class="cx"> {
</span><del>- ASSERT(kind != GeneratedNewCode);
</del><ins>+ RELEASE_ASSERT(kind != GeneratedNewCode);
+ RELEASE_ASSERT(kind != GeneratedFinalCode);
</ins><span class="cx"> }
</span><span class="cx">
</span><del>- AccessGenerationResult(MacroAssemblerCodePtr code)
- : m_kind(GeneratedNewCode)
</del><ins>+ AccessGenerationResult(Kind kind, MacroAssemblerCodePtr code)
+ : m_kind(kind)
</ins><span class="cx"> , m_code(code)
</span><span class="cx"> {
</span><ins>+ RELEASE_ASSERT(kind == GeneratedNewCode || kind == GeneratedFinalCode);
</ins><span class="cx"> RELEASE_ASSERT(code);
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -350,8 +356,16 @@
</span><span class="cx">
</span><span class="cx"> bool madeNoChanges() const { return m_kind == MadeNoChanges; }
</span><span class="cx"> bool gaveUp() const { return m_kind == GaveUp; }
</span><ins>+ bool buffered() const { return m_kind == Buffered; }
</ins><span class="cx"> bool generatedNewCode() const { return m_kind == GeneratedNewCode; }
</span><ins>+ bool generatedFinalCode() const { return m_kind == GeneratedFinalCode; }
</ins><span class="cx">
</span><ins>+ // If we gave up on this attempt to generate code, or if we generated the "final" code, then we
+ // should give up after this.
+ bool shouldGiveUpNow() const { return gaveUp() || generatedFinalCode(); }
+
+ bool generatedSomeCode() const { return generatedNewCode() || generatedFinalCode(); }
+
</ins><span class="cx"> void dump(PrintStream&) const;
</span><span class="cx">
</span><span class="cx"> private:
</span><span class="lines">@@ -366,15 +380,16 @@
</span><span class="cx"> PolymorphicAccess();
</span><span class="cx"> ~PolymorphicAccess();
</span><span class="cx">
</span><del>- // This may return null, in which case the old stub routine is left intact. You are required to
- // pass a vector of non-null access cases. This will prune the access cases by rejecting any case
- // in the list that is subsumed by a later case in the list.
- AccessGenerationResult regenerateWithCases(
</del><ins>+ // When this fails (returns GaveUp), this will leave the old stub intact but you should not try
+ // to call this method again for that PolymorphicAccess instance.
+ AccessGenerationResult addCases(
</ins><span class="cx"> VM&, CodeBlock*, StructureStubInfo&, const Identifier&, Vector<std::unique_ptr<AccessCase>>);
</span><span class="cx">
</span><del>- AccessGenerationResult regenerateWithCase(
</del><ins>+ AccessGenerationResult addCase(
</ins><span class="cx"> VM&, CodeBlock*, StructureStubInfo&, const Identifier&, std::unique_ptr<AccessCase>);
</span><span class="cx">
</span><ins>+ AccessGenerationResult regenerate(VM&, CodeBlock*, StructureStubInfo&, const Identifier&);
+
</ins><span class="cx"> bool isEmpty() const { return m_list.isEmpty(); }
</span><span class="cx"> unsigned size() const { return m_list.size(); }
</span><span class="cx"> const AccessCase& at(unsigned i) const { return *m_list[i]; }
</span><span class="lines">@@ -401,6 +416,10 @@
</span><span class="cx"> friend struct AccessGenerationState;
</span><span class="cx">
</span><span class="cx"> typedef Vector<std::unique_ptr<AccessCase>, 2> ListType;
</span><ins>+
+ void commit(
+ VM&, std::unique_ptr<WatchpointsOnStructureStubInfo>&, CodeBlock*, StructureStubInfo&,
+ const Identifier&, AccessCase&);
</ins><span class="cx">
</span><span class="cx"> MacroAssemblerCodePtr regenerate(
</span><span class="cx"> VM&, CodeBlock*, StructureStubInfo&, const Identifier&, ListType& cases);
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorebytecodePutByIdStatuscpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/bytecode/PutByIdStatus.cpp (199381 => 199382)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/bytecode/PutByIdStatus.cpp 2016-04-12 19:37:37 UTC (rev 199381)
+++ trunk/Source/JavaScriptCore/bytecode/PutByIdStatus.cpp 2016-04-12 20:06:26 UTC (rev 199382)
</span><span class="lines">@@ -225,12 +225,12 @@
</span><span class="cx"> return PutByIdStatus(slowPathState);
</span><span class="cx">
</span><span class="cx"> case ComplexGetStatus::Inlineable: {
</span><del>- CallLinkInfo* callLinkInfo = access.callLinkInfo();
- ASSERT(callLinkInfo);
</del><span class="cx"> std::unique_ptr<CallLinkStatus> callLinkStatus =
</span><del>- std::make_unique<CallLinkStatus>(
- CallLinkStatus::computeFor(
- locker, profiledBlock, *callLinkInfo, callExitSiteData));
</del><ins>+ std::make_unique<CallLinkStatus>();
+ if (CallLinkInfo* callLinkInfo = access.callLinkInfo()) {
+ *callLinkStatus = CallLinkStatus::computeFor(
+ locker, profiledBlock, *callLinkInfo, callExitSiteData);
+ }
</ins><span class="cx">
</span><span class="cx"> variant = PutByIdVariant::setter(
</span><span class="cx"> structure, complexGetStatus.offset(), complexGetStatus.conditionSet(),
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorebytecodeStructureStubInfocpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/bytecode/StructureStubInfo.cpp (199381 => 199382)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/bytecode/StructureStubInfo.cpp 2016-04-12 19:37:37 UTC (rev 199381)
+++ trunk/Source/JavaScriptCore/bytecode/StructureStubInfo.cpp 2016-04-12 20:06:26 UTC (rev 199382)
</span><span class="lines">@@ -33,6 +33,9 @@
</span><span class="cx"> namespace JSC {
</span><span class="cx">
</span><span class="cx"> #if ENABLE(JIT)
</span><ins>+
+static const bool verbose = false;
+
</ins><span class="cx"> StructureStubInfo::StructureStubInfo(AccessType accessType)
</span><span class="cx"> : callSiteIndex(UINT_MAX)
</span><span class="cx"> , accessType(accessType)
</span><span class="lines">@@ -40,6 +43,7 @@
</span><span class="cx"> , countdown(1) // For a totally clear stub, we'll patch it after the first execution.
</span><span class="cx"> , repatchCount(0)
</span><span class="cx"> , numberOfCoolDowns(0)
</span><ins>+ , bufferingCountdown(0)
</ins><span class="cx"> , resetByGC(false)
</span><span class="cx"> , tookSlowPath(false)
</span><span class="cx"> , everConsidered(false)
</span><span class="lines">@@ -109,38 +113,91 @@
</span><span class="cx"> {
</span><span class="cx"> VM& vm = *codeBlock->vm();
</span><span class="cx">
</span><ins>+ if (verbose)
+ dataLog("Adding access case: ", accessCase, "\n");
+
</ins><span class="cx"> if (!accessCase)
</span><span class="cx"> return AccessGenerationResult::GaveUp;
</span><span class="cx">
</span><ins>+ AccessGenerationResult result;
+
</ins><span class="cx"> if (cacheType == CacheType::Stub) {
</span><del>- SuperSamplerScope superSamplerScope(true);
</del><ins>+ result = u.stub->addCase(vm, codeBlock, *this, ident, WTFMove(accessCase));
+
+ if (verbose)
+ dataLog("Had stub, result: ", result, "\n");
+
+ if (!result.buffered()) {
+ bufferedStructures.clear();
+ return result;
+ }
+ } else {
+ std::unique_ptr<PolymorphicAccess> access = std::make_unique<PolymorphicAccess>();
+
+ Vector<std::unique_ptr<AccessCase>> accessCases;
+
+ std::unique_ptr<AccessCase> previousCase =
+ AccessCase::fromStructureStubInfo(vm, codeBlock, *this);
+ if (previousCase)
+ accessCases.append(WTFMove(previousCase));
+
+ accessCases.append(WTFMove(accessCase));
+
+ result = access->addCases(vm, codeBlock, *this, ident, WTFMove(accessCases));
+
+ if (verbose)
+ dataLog("Created stub, result: ", result, "\n");
+
+ if (!result.buffered()) {
+ bufferedStructures.clear();
+ return result;
+ }
+
+ initStub(codeBlock, WTFMove(access));
+ }
</ins><span class="cx">
</span><del>- return u.stub->regenerateWithCase(vm, codeBlock, *this, ident, WTFMove(accessCase));
</del><ins>+ RELEASE_ASSERT(!result.generatedSomeCode());
+
+ // If we didn't buffer any cases then bail. If this made no changes then we'll just try again
+ // subject to cool-down.
+ if (!result.buffered()) {
+ if (verbose)
+ dataLog("Didn't buffer anything, bailing.\n");
+ bufferedStructures.clear();
+ return result;
</ins><span class="cx"> }
</span><span class="cx">
</span><del>- std::unique_ptr<PolymorphicAccess> access = std::make_unique<PolymorphicAccess>();
</del><ins>+ // The buffering countdown tells us if we should be repatching now.
+ if (bufferingCountdown) {
+ if (verbose)
+ dataLog("Countdown is too high: ", bufferingCountdown, ".\n");
+ return result;
+ }
</ins><span class="cx">
</span><del>- Vector<std::unique_ptr<AccessCase>> accessCases;
</del><ins>+ // Forget the buffered structures so that all future attempts to cache get fully handled by the
+ // PolymorphicAccess.
+ bufferedStructures.clear();
</ins><span class="cx">
</span><del>- std::unique_ptr<AccessCase> previousCase =
- AccessCase::fromStructureStubInfo(vm, codeBlock, *this);
- if (previousCase)
- accessCases.append(WTFMove(previousCase));
-
- accessCases.append(WTFMove(accessCase));
-
- AccessGenerationResult result =
- access->regenerateWithCases(vm, codeBlock, *this, ident, WTFMove(accessCases));
-
- if (!result.generatedNewCode())
</del><ins>+ result = u.stub->regenerate(vm, codeBlock, *this, ident);
+
+ if (verbose)
+ dataLog("Regeneration result: ", result, "\n");
+
+ RELEASE_ASSERT(!result.buffered());
+
+ if (!result.generatedSomeCode())
</ins><span class="cx"> return result;
</span><del>-
- initStub(codeBlock, WTFMove(access));
</del><ins>+
+ // If we generated some code then we don't want to attempt to repatch in the future until we
+ // gather enough cases.
+ bufferingCountdown = Options::repatchBufferingCountdown();
</ins><span class="cx"> return result;
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void StructureStubInfo::reset(CodeBlock* codeBlock)
</span><span class="cx"> {
</span><ins>+ bufferedStructures.clear();
+
</ins><span class="cx"> if (cacheType == CacheType::Unset)
</span><span class="cx"> return;
</span><span class="cx">
</span><span class="lines">@@ -172,6 +229,8 @@
</span><span class="cx"> void StructureStubInfo::visitWeakReferences(CodeBlock* codeBlock)
</span><span class="cx"> {
</span><span class="cx"> VM& vm = *codeBlock->vm();
</span><ins>+
+ bufferedStructures.clear();
</ins><span class="cx">
</span><span class="cx"> switch (cacheType) {
</span><span class="cx"> case CacheType::GetByIdSelf:
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorebytecodeStructureStubInfoh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/bytecode/StructureStubInfo.h (199381 => 199382)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/bytecode/StructureStubInfo.h 2016-04-12 19:37:37 UTC (rev 199381)
+++ trunk/Source/JavaScriptCore/bytecode/StructureStubInfo.h 2016-04-12 20:06:26 UTC (rev 199382)
</span><span class="lines">@@ -35,6 +35,7 @@
</span><span class="cx"> #include "Options.h"
</span><span class="cx"> #include "RegisterSet.h"
</span><span class="cx"> #include "Structure.h"
</span><ins>+#include "StructureSet.h"
</ins><span class="cx"> #include "StructureStubClearingWatchpoint.h"
</span><span class="cx">
</span><span class="cx"> namespace JSC {
</span><span class="lines">@@ -81,13 +82,26 @@
</span><span class="cx"> // either entirely or just enough to ensure that those dead pointers don't get used anymore.
</span><span class="cx"> void visitWeakReferences(CodeBlock*);
</span><span class="cx">
</span><del>- ALWAYS_INLINE bool considerCaching()
</del><ins>+ ALWAYS_INLINE bool considerCaching(Structure* structure)
</ins><span class="cx"> {
</span><ins>+ // We never cache non-cells.
+ if (!structure)
+ return false;
+
+ // This method is called from the Optimize variants of IC slow paths. The first part of this
+ // method tries to determine if the Optimize variant should really behave like the
+ // non-Optimize variant and leave the IC untouched.
+ //
+ // If we determine that we should do something to the IC then the next order of business is
+ // to determine if this Structure would impact the IC at all. We know that it won't, if we
+ // have already buffered something on its behalf. That's what the bufferedStructures set is
+ // for.
+
</ins><span class="cx"> everConsidered = true;
</span><span class="cx"> if (!countdown) {
</span><span class="cx"> // Check if we have been doing repatching too frequently. If so, then we should cool off
</span><span class="cx"> // for a while.
</span><del>- willRepatch();
</del><ins>+ WTF::incrementWithSaturation(repatchCount);
</ins><span class="cx"> if (repatchCount > Options::repatchCountForCoolDown()) {
</span><span class="cx"> // We've been repatching too much, so don't do it now.
</span><span class="cx"> repatchCount = 0;
</span><span class="lines">@@ -99,25 +113,33 @@
</span><span class="cx"> static_cast<uint8_t>(Options::initialCoolDownCount()),
</span><span class="cx"> numberOfCoolDowns,
</span><span class="cx"> static_cast<uint8_t>(std::numeric_limits<uint8_t>::max() - 1));
</span><del>- willCoolDown();
- return false;
</del><ins>+ WTF::incrementWithSaturation(numberOfCoolDowns);
+
+ // We may still have had something buffered. Trigger generation now.
+ bufferingCountdown = 0;
+ return true;
</ins><span class="cx"> }
</span><del>- return true;
</del><ins>+
+ // We don't want to return false due to buffering indefinitely.
+ if (!bufferingCountdown) {
+ // Note that when this returns true, it's possible that we will not even get an
+ // AccessCase because this may cause Repatch.cpp to simply do an in-place
+ // repatching.
+ return true;
+ }
+
+ bufferingCountdown--;
+
+ // Now protect the IC buffering. We want to proceed only if this is a structure that
+ // we don't already have a case buffered for. Note that if this returns true but the
+ // bufferingCountdown is not zero then we will buffer the access case for later without
+ // immediately generating code for it.
+ return bufferedStructures.add(structure);
</ins><span class="cx"> }
</span><span class="cx"> countdown--;
</span><span class="cx"> return false;
</span><span class="cx"> }
</span><span class="cx">
</span><del>- ALWAYS_INLINE void willRepatch()
- {
- WTF::incrementWithSaturation(repatchCount);
- }
-
- ALWAYS_INLINE void willCoolDown()
- {
- WTF::incrementWithSaturation(numberOfCoolDowns);
- }
-
</del><span class="cx"> CodeLocationCall callReturnLocation;
</span><span class="cx">
</span><span class="cx"> CodeOrigin codeOrigin;
</span><span class="lines">@@ -132,7 +154,13 @@
</span><span class="cx"> } byIdSelf;
</span><span class="cx"> PolymorphicAccess* stub;
</span><span class="cx"> } u;
</span><del>-
</del><ins>+
+ // Represents those structures that already have buffered AccessCases in the PolymorphicAccess.
+ // Note that it's always safe to clear this. If we clear it prematurely, then if we see the same
+ // structure again during this buffering countdown, we will create an AccessCase object for it.
+ // That's not so bad - we'll get rid of the redundant ones once we regenerate.
+ StructureSet bufferedStructures;
+
</ins><span class="cx"> struct {
</span><span class="cx"> int8_t baseGPR;
</span><span class="cx"> #if USE(JSVALUE32_64)
</span><span class="lines">@@ -158,6 +186,7 @@
</span><span class="cx"> uint8_t countdown; // We repatch only when this is zero. If not zero, we decrement.
</span><span class="cx"> uint8_t repatchCount;
</span><span class="cx"> uint8_t numberOfCoolDowns;
</span><ins>+ uint8_t bufferingCountdown;
</ins><span class="cx"> bool resetByGC : 1;
</span><span class="cx"> bool tookSlowPath : 1;
</span><span class="cx"> bool everConsidered : 1;
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorejitJITOperationscpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/jit/JITOperations.cpp (199381 => 199382)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/jit/JITOperations.cpp 2016-04-12 19:37:37 UTC (rev 199381)
+++ trunk/Source/JavaScriptCore/jit/JITOperations.cpp 2016-04-12 20:06:26 UTC (rev 199382)
</span><span class="lines">@@ -195,7 +195,7 @@
</span><span class="cx"> PropertySlot slot(baseValue, PropertySlot::InternalMethodType::VMInquiry);
</span><span class="cx">
</span><span class="cx"> baseValue.getPropertySlot(exec, ident, slot);
</span><del>- if (stubInfo->considerCaching() && !slot.isTaintedByProxy() && (slot.isCacheableValue() || slot.isCacheableGetter() || slot.isUnset()))
</del><ins>+ if (stubInfo->considerCaching(baseValue.structureOrNull()) && !slot.isTaintedByProxy() && (slot.isCacheableValue() || slot.isCacheableGetter() || slot.isUnset()))
</ins><span class="cx"> repatchGetByID(exec, baseValue, ident, slot, *stubInfo, GetByIDKind::Pure);
</span><span class="cx">
</span><span class="cx"> return JSValue::encode(slot.getPureResult());
</span><span class="lines">@@ -245,7 +245,7 @@
</span><span class="cx"> PropertySlot slot(baseValue, PropertySlot::InternalMethodType::Get);
</span><span class="cx">
</span><span class="cx"> bool hasResult = baseValue.getPropertySlot(exec, ident, slot);
</span><del>- if (stubInfo->considerCaching())
</del><ins>+ if (stubInfo->considerCaching(baseValue.structureOrNull()))
</ins><span class="cx"> repatchGetByID(exec, baseValue, ident, slot, *stubInfo, GetByIDKind::Normal);
</span><span class="cx">
</span><span class="cx"> return JSValue::encode(hasResult? slot.getValue(exec, ident) : jsUndefined());
</span><span class="lines">@@ -272,7 +272,7 @@
</span><span class="cx">
</span><span class="cx"> RELEASE_ASSERT(accessType == stubInfo->accessType);
</span><span class="cx">
</span><del>- if (stubInfo->considerCaching())
</del><ins>+ if (stubInfo->considerCaching(asObject(base)->structure()))
</ins><span class="cx"> repatchIn(exec, base, ident, result, slot, *stubInfo);
</span><span class="cx">
</span><span class="cx"> return JSValue::encode(jsBoolean(result));
</span><span class="lines">@@ -393,7 +393,7 @@
</span><span class="cx"> if (accessType != static_cast<AccessType>(stubInfo->accessType))
</span><span class="cx"> return;
</span><span class="cx">
</span><del>- if (stubInfo->considerCaching())
</del><ins>+ if (stubInfo->considerCaching(structure))
</ins><span class="cx"> repatchPutByID(exec, baseValue, structure, ident, slot, *stubInfo, NotDirect);
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -418,7 +418,7 @@
</span><span class="cx"> if (accessType != static_cast<AccessType>(stubInfo->accessType))
</span><span class="cx"> return;
</span><span class="cx">
</span><del>- if (stubInfo->considerCaching())
</del><ins>+ if (stubInfo->considerCaching(structure))
</ins><span class="cx"> repatchPutByID(exec, baseValue, structure, ident, slot, *stubInfo, NotDirect);
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -443,7 +443,7 @@
</span><span class="cx"> if (accessType != static_cast<AccessType>(stubInfo->accessType))
</span><span class="cx"> return;
</span><span class="cx">
</span><del>- if (stubInfo->considerCaching())
</del><ins>+ if (stubInfo->considerCaching(structure))
</ins><span class="cx"> repatchPutByID(exec, baseObject, structure, ident, slot, *stubInfo, Direct);
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -468,7 +468,7 @@
</span><span class="cx"> if (accessType != static_cast<AccessType>(stubInfo->accessType))
</span><span class="cx"> return;
</span><span class="cx">
</span><del>- if (stubInfo->considerCaching())
</del><ins>+ if (stubInfo->considerCaching(structure))
</ins><span class="cx"> repatchPutByID(exec, baseObject, structure, ident, slot, *stubInfo, Direct);
</span><span class="cx"> }
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorejitRepatchcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/jit/Repatch.cpp (199381 => 199382)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/jit/Repatch.cpp 2016-04-12 19:37:37 UTC (rev 199381)
+++ trunk/Source/JavaScriptCore/jit/Repatch.cpp 2016-04-12 20:06:26 UTC (rev 199382)
</span><span class="lines">@@ -364,17 +364,14 @@
</span><span class="cx">
</span><span class="cx"> AccessGenerationResult result = stubInfo.addAccessCase(codeBlock, propertyName, WTFMove(newCase));
</span><span class="cx">
</span><del>- if (result.gaveUp())
- return GiveUpOnCache;
- if (result.madeNoChanges())
- return RetryCacheLater;
</del><ins>+ if (result.generatedSomeCode()) {
+ LOG_IC((ICEvent::GetByIdReplaceWithJump, baseValue.classInfoOrNull(), propertyName));
+
+ RELEASE_ASSERT(result.code());
+ replaceWithJump(stubInfo, result.code());
+ }
</ins><span class="cx">
</span><del>- LOG_IC((ICEvent::GetByIdReplaceWithJump, baseValue.classInfoOrNull(), propertyName));
-
- RELEASE_ASSERT(result.code());
- replaceWithJump(stubInfo, result.code());
-
- return RetryCacheLater;
</del><ins>+ return result.shouldGiveUpNow() ? GiveUpOnCache : RetryCacheLater;
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void repatchGetByID(ExecState* exec, JSValue baseValue, const Identifier& propertyName, const PropertySlot& slot, StructureStubInfo& stubInfo, GetByIDKind kind)
</span><span class="lines">@@ -515,21 +512,18 @@
</span><span class="cx">
</span><span class="cx"> AccessGenerationResult result = stubInfo.addAccessCase(codeBlock, ident, WTFMove(newCase));
</span><span class="cx">
</span><del>- if (result.gaveUp())
- return GiveUpOnCache;
- if (result.madeNoChanges())
- return RetryCacheLater;
-
- LOG_IC((ICEvent::PutByIdReplaceWithJump, structure->classInfo(), ident));
-
- RELEASE_ASSERT(result.code());
- resetPutByIDCheckAndLoad(stubInfo);
- MacroAssembler::repatchJump(
- stubInfo.callReturnLocation.jumpAtOffset(
- stubInfo.patch.deltaCallToJump),
- CodeLocationLabel(result.code()));
</del><ins>+ if (result.generatedSomeCode()) {
+ LOG_IC((ICEvent::PutByIdReplaceWithJump, structure->classInfo(), ident));
+
+ RELEASE_ASSERT(result.code());
+ resetPutByIDCheckAndLoad(stubInfo);
+ MacroAssembler::repatchJump(
+ stubInfo.callReturnLocation.jumpAtOffset(
+ stubInfo.patch.deltaCallToJump),
+ CodeLocationLabel(result.code()));
+ }
</ins><span class="cx">
</span><del>- return RetryCacheLater;
</del><ins>+ return result.shouldGiveUpNow() ? GiveUpOnCache : RetryCacheLater;
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void repatchPutByID(ExecState* exec, JSValue baseValue, Structure* structure, const Identifier& propertyName, const PutPropertySlot& slot, StructureStubInfo& stubInfo, PutKind putKind)
</span><span class="lines">@@ -579,19 +573,17 @@
</span><span class="cx"> vm, codeBlock, wasFound ? AccessCase::InHit : AccessCase::InMiss, structure, conditionSet);
</span><span class="cx">
</span><span class="cx"> AccessGenerationResult result = stubInfo.addAccessCase(codeBlock, ident, WTFMove(newCase));
</span><del>- if (result.gaveUp())
- return GiveUpOnCache;
- if (result.madeNoChanges())
- return RetryCacheLater;
-
- LOG_IC((ICEvent::InReplaceWithJump, structure->classInfo(), ident));
-
- RELEASE_ASSERT(result.code());
- MacroAssembler::repatchJump(
- stubInfo.callReturnLocation.jumpAtOffset(stubInfo.patch.deltaCallToJump),
- CodeLocationLabel(result.code()));
</del><span class="cx">
</span><del>- return RetryCacheLater;
</del><ins>+ if (result.generatedSomeCode()) {
+ LOG_IC((ICEvent::InReplaceWithJump, structure->classInfo(), ident));
+
+ RELEASE_ASSERT(result.code());
+ MacroAssembler::repatchJump(
+ stubInfo.callReturnLocation.jumpAtOffset(stubInfo.patch.deltaCallToJump),
+ CodeLocationLabel(result.code()));
+ }
+
+ return result.shouldGiveUpNow() ? GiveUpOnCache : RetryCacheLater;
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void repatchIn(
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeJSCJSValueh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/JSCJSValue.h (199381 => 199382)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/JSCJSValue.h 2016-04-12 19:37:37 UTC (rev 199381)
+++ trunk/Source/JavaScriptCore/runtime/JSCJSValue.h 2016-04-12 20:06:26 UTC (rev 199382)
</span><span class="lines">@@ -308,6 +308,7 @@
</span><span class="cx"> JSCell* asCell() const;
</span><span class="cx"> JS_EXPORT_PRIVATE bool isValidCallee();
</span><span class="cx">
</span><ins>+ Structure* structureOrNull() const;
</ins><span class="cx"> JSValue structureOrUndefined() const;
</span><span class="cx">
</span><span class="cx"> JS_EXPORT_PRIVATE void dump(PrintStream&) const;
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeJSCJSValueInlinesh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/JSCJSValueInlines.h (199381 => 199382)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/JSCJSValueInlines.h 2016-04-12 19:37:37 UTC (rev 199381)
+++ trunk/Source/JavaScriptCore/runtime/JSCJSValueInlines.h 2016-04-12 20:06:26 UTC (rev 199382)
</span><span class="lines">@@ -849,6 +849,13 @@
</span><span class="cx"> return asCell()->methodTable(exec->vm())->putByIndex(asCell(), exec, propertyName, value, shouldThrow);
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+inline Structure* JSValue::structureOrNull() const
+{
+ if (isCell())
+ return asCell()->structure();
+ return nullptr;
+}
+
</ins><span class="cx"> inline JSValue JSValue::structureOrUndefined() const
</span><span class="cx"> {
</span><span class="cx"> if (isCell())
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeOptionsh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/Options.h (199381 => 199382)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/Options.h 2016-04-12 19:37:37 UTC (rev 199381)
+++ trunk/Source/JavaScriptCore/runtime/Options.h 2016-04-12 20:06:26 UTC (rev 199382)
</span><span class="lines">@@ -125,6 +125,7 @@
</span><span class="cx"> \
</span><span class="cx"> v(unsigned, repatchCountForCoolDown, 10, nullptr) \
</span><span class="cx"> v(unsigned, initialCoolDownCount, 20, nullptr) \
</span><ins>+ v(unsigned, repatchBufferingCountdown, 7, nullptr) \
</ins><span class="cx"> \
</span><span class="cx"> v(bool, dumpGeneratedBytecodes, false, nullptr) \
</span><span class="cx"> v(bool, dumpBytecodeLivenessResults, false, nullptr) \
</span></span></pre></div>
<a id="trunkSourceWTFChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WTF/ChangeLog (199381 => 199382)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WTF/ChangeLog 2016-04-12 19:37:37 UTC (rev 199381)
+++ trunk/Source/WTF/ChangeLog 2016-04-12 20:06:26 UTC (rev 199382)
</span><span class="lines">@@ -1,3 +1,13 @@
</span><ins>+2016-04-12 Filip Pizlo <fpizlo@apple.com>
+
+ PolymorphicAccess should buffer AccessCases before regenerating
+ https://bugs.webkit.org/show_bug.cgi?id=156457
+
+ Reviewed by Benjamin Poulain.
+
+ * wtf/TinyPtrSet.h:
+ (WTF::TinyPtrSet::add): Add a helpful comment because I had forgotten what the bool return meant.
+
</ins><span class="cx"> 2016-04-12 Tomas Popela <tpopela@redhat.com>
</span><span class="cx">
</span><span class="cx"> S390X and PPC64 architectures detection is wrong
</span></span></pre></div>
<a id="trunkSourceWTFwtfTinyPtrSeth"></a>
<div class="modfile"><h4>Modified: trunk/Source/WTF/wtf/TinyPtrSet.h (199381 => 199382)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WTF/wtf/TinyPtrSet.h 2016-04-12 19:37:37 UTC (rev 199381)
+++ trunk/Source/WTF/wtf/TinyPtrSet.h 2016-04-12 20:06:26 UTC (rev 199382)
</span><span class="lines">@@ -101,6 +101,7 @@
</span><span class="cx"> return result;
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+ // Returns true if the value was added, or false if the value was already there.
</ins><span class="cx"> bool add(T value)
</span><span class="cx"> {
</span><span class="cx"> ASSERT(value);
</span></span></pre>
</div>
</div>
</body>
</html>