- Revision
- 199566
- Author
- [email protected]
- Date
- 2016-04-14 16:08:07 -0700 (Thu, 14 Apr 2016)
Log Message
PolymorphicAccess should try to generate a stub only once
https://bugs.webkit.org/show_bug.cgi?id=156555
Reviewed by Geoffrey Garen.
This changes the PolymorphicAccess heuristics to reduce the amount of code generation even
more than before. We used to always generate a monomorphic stub for the first case we saw.
This change disables that. This change also increases the buffering countdown to match the
cool-down repatch count. This means that we will allow for ten slow paths for adding cases,
then we will generate a stub, and then we will go into cool-down and the repatching slow
paths will not even attempt repatching for a while. After we emerge from cool-down - which
requires a bunch of slow path calls - we will again wait for ten slow paths to get new
cases. Note that it only takes 13 cases to cause the stub to give up on future repatching
entirely. Also, most stubs don't ever get to 10 cases. Therefore, for most stubs this change
means that each IC will repatch once. If they make it to two repatching, then the likelihood
of a third becomes infinitesimal because of all of the rules that come into play at that
point (the size limit being 13, the fact that we go into exponential cool-down every time we
generate code, and the fact that if we have lots of self cases then we will create a
catch-all megamorphic load case).
This also undoes a change to the megamorphic optimization that I think was unintentional.
As in the change that originally introduced megamorphic loads, we want to do this only if we
would otherwise exhaust the max size of the IC. This is because megamorphic loads are pretty
expensive and it's best to use them only if we know that the alternative is giving up on
caching.
This is neutral on JS benchmarks, but looks like it's another speed-up for page loading.
* bytecode/PolymorphicAccess.cpp:
(JSC::AccessCase::canBeReplacedByMegamorphicLoad):
(JSC::AccessCase::canReplace):
(JSC::AccessCase::dump):
(JSC::PolymorphicAccess::regenerate):
* bytecode/StructureStubInfo.cpp:
(JSC::StructureStubInfo::StructureStubInfo):
* runtime/Options.h:
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (199565 => 199566)
--- trunk/Source/_javascript_Core/ChangeLog 2016-04-14 23:04:26 UTC (rev 199565)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-04-14 23:08:07 UTC (rev 199566)
@@ -1,3 +1,42 @@
+2016-04-14 Filip Pizlo <[email protected]>
+
+ PolymorphicAccess should try to generate a stub only once
+ https://bugs.webkit.org/show_bug.cgi?id=156555
+
+ Reviewed by Geoffrey Garen.
+
+ This changes the PolymorphicAccess heuristics to reduce the amount of code generation even
+ more than before. We used to always generate a monomorphic stub for the first case we saw.
+ This change disables that. This change also increases the buffering countdown to match the
+ cool-down repatch count. This means that we will allow for ten slow paths for adding cases,
+ then we will generate a stub, and then we will go into cool-down and the repatching slow
+ paths will not even attempt repatching for a while. After we emerge from cool-down - which
+ requires a bunch of slow path calls - we will again wait for ten slow paths to get new
+ cases. Note that it only takes 13 cases to cause the stub to give up on future repatching
+ entirely. Also, most stubs don't ever get to 10 cases. Therefore, for most stubs this change
+ means that each IC will repatch once. If they make it to two repatching, then the likelihood
+ of a third becomes infinitesimal because of all of the rules that come into play at that
+ point (the size limit being 13, the fact that we go into exponential cool-down every time we
+ generate code, and the fact that if we have lots of self cases then we will create a
+ catch-all megamorphic load case).
+
+ This also undoes a change to the megamorphic optimization that I think was unintentional.
+ As in the change that originally introduced megamorphic loads, we want to do this only if we
+ would otherwise exhaust the max size of the IC. This is because megamorphic loads are pretty
+ expensive and it's best to use them only if we know that the alternative is giving up on
+ caching.
+
+ This is neutral on JS benchmarks, but looks like it's another speed-up for page loading.
+
+ * bytecode/PolymorphicAccess.cpp:
+ (JSC::AccessCase::canBeReplacedByMegamorphicLoad):
+ (JSC::AccessCase::canReplace):
+ (JSC::AccessCase::dump):
+ (JSC::PolymorphicAccess::regenerate):
+ * bytecode/StructureStubInfo.cpp:
+ (JSC::StructureStubInfo::StructureStubInfo):
+ * runtime/Options.h:
+
2016-04-14 Mark Lam <[email protected]>
Update treatment of invoking RegExp.prototype methods on RegExp.prototype.
Modified: trunk/Source/_javascript_Core/bytecode/PolymorphicAccess.cpp (199565 => 199566)
--- trunk/Source/_javascript_Core/bytecode/PolymorphicAccess.cpp 2016-04-14 23:04:26 UTC (rev 199565)
+++ trunk/Source/_javascript_Core/bytecode/PolymorphicAccess.cpp 2016-04-14 23:08:07 UTC (rev 199566)
@@ -472,6 +472,9 @@
bool AccessCase::canBeReplacedByMegamorphicLoad() const
{
+ if (type() == MegamorphicLoad)
+ return true;
+
return type() == Load
&& !viaProxy()
&& conditionSet().isEmpty()
@@ -481,17 +484,23 @@
bool AccessCase::canReplace(const AccessCase& other) const
{
- // We could do a lot better here, but for now we just do something obvious.
-
- if (type() == MegamorphicLoad && other.canBeReplacedByMegamorphicLoad())
- return true;
+ // This puts in a good effort to try to figure out if 'other' is made superfluous by '*this'.
+ // It's fine for this to return false if it's in doubt.
- if (!guardedByStructureCheck() || !other.guardedByStructureCheck()) {
- // FIXME: Implement this!
- return false;
+ switch (type()) {
+ case MegamorphicLoad:
+ return other.canBeReplacedByMegamorphicLoad();
+ case ArrayLength:
+ case StringLength:
+ case DirectArgumentsLength:
+ case ScopedArgumentsLength:
+ return other.type() == type();
+ default:
+ if (!guardedByStructureCheck() || !other.guardedByStructureCheck())
+ return false;
+
+ return structure() == other.structure();
}
-
- return structure() == other.structure();
}
void AccessCase::dump(PrintStream& out) const
@@ -1587,7 +1596,7 @@
// 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()) {
+ if (cases.size() >= Options::maxAccessVariantListSize()) {
unsigned numSelfLoads = 0;
for (auto& newCase : cases) {
if (newCase->canBeReplacedByMegamorphicLoad())
@@ -1658,9 +1667,14 @@
// of something that isn't patchable. The slow path will decrement "countdown" and will only
// patch things if the countdown reaches zero. We increment the slow path count here to ensure
// that the slow path does not try to patch.
+#if CPU(X86) || CPU(X86_64)
+ jit.move(CCallHelpers::TrustedImmPtr(&stubInfo.countdown), state.scratchGPR);
+ jit.add8(CCallHelpers::TrustedImm32(1), CCallHelpers::Address(state.scratchGPR));
+#else
jit.load8(&stubInfo.countdown, state.scratchGPR);
jit.add32(CCallHelpers::TrustedImm32(1), state.scratchGPR);
jit.store8(state.scratchGPR, &stubInfo.countdown);
+#endif
}
CCallHelpers::JumpList failure;
Modified: trunk/Source/_javascript_Core/bytecode/StructureStubInfo.cpp (199565 => 199566)
--- trunk/Source/_javascript_Core/bytecode/StructureStubInfo.cpp 2016-04-14 23:04:26 UTC (rev 199565)
+++ trunk/Source/_javascript_Core/bytecode/StructureStubInfo.cpp 2016-04-14 23:08:07 UTC (rev 199566)
@@ -43,7 +43,7 @@
, countdown(1) // For a totally clear stub, we'll patch it after the first execution.
, repatchCount(0)
, numberOfCoolDowns(0)
- , bufferingCountdown(0)
+ , bufferingCountdown(Options::repatchBufferingCountdown())
, resetByGC(false)
, tookSlowPath(false)
, everConsidered(false)
Modified: trunk/Source/_javascript_Core/runtime/Options.h (199565 => 199566)
--- trunk/Source/_javascript_Core/runtime/Options.h 2016-04-14 23:04:26 UTC (rev 199565)
+++ trunk/Source/_javascript_Core/runtime/Options.h 2016-04-14 23:08:07 UTC (rev 199566)
@@ -125,7 +125,7 @@
\
v(unsigned, repatchCountForCoolDown, 10, nullptr) \
v(unsigned, initialCoolDownCount, 20, nullptr) \
- v(unsigned, repatchBufferingCountdown, 7, nullptr) \
+ v(unsigned, repatchBufferingCountdown, 10, nullptr) \
\
v(bool, dumpGeneratedBytecodes, false, nullptr) \
v(bool, dumpBytecodeLivenessResults, false, nullptr) \
