PolymorphicAccess should try to generate a stub only once
authorfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 14 Apr 2016 23:08:07 +0000 (23:08 +0000)
committerfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 14 Apr 2016 23:08:07 +0000 (23:08 +0000)
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:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@199566 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/bytecode/PolymorphicAccess.cpp
Source/JavaScriptCore/bytecode/StructureStubInfo.cpp
Source/JavaScriptCore/runtime/Options.h

index eae301d..e4c28ea 100644 (file)
@@ -1,3 +1,42 @@
+2016-04-14  Filip Pizlo  <fpizlo@apple.com>
+
+        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  <mark.lam@apple.com>
 
         Update treatment of invoking RegExp.prototype methods on RegExp.prototype.
index 6a4206c..fb3f26f 100644 (file)
@@ -472,6 +472,9 @@ bool AccessCase::couldStillSucceed() const
 
 bool AccessCase::canBeReplacedByMegamorphicLoad() const
 {
+    if (type() == MegamorphicLoad)
+        return true;
+    
     return type() == Load
         && !viaProxy()
         && conditionSet().isEmpty()
@@ -481,17 +484,23 @@ bool AccessCase::canBeReplacedByMegamorphicLoad() const
 
 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 @@ AccessGenerationResult PolymorphicAccess::regenerate(
     // 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 @@ AccessGenerationResult PolymorphicAccess::regenerate(
         // 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;
index f2f90ba..d925cfc 100644 (file)
@@ -43,7 +43,7 @@ StructureStubInfo::StructureStubInfo(AccessType accessType)
     , 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)
index 05af2e0..2ba5aea 100644 (file)
@@ -125,7 +125,7 @@ typedef const char* optionString;
     \
     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) \