Bug 1355746 - Part 2. Polish IdleTaskRunner and reuse it for background parsing. draft
authorHenry Chang <hchang@mozilla.com>
Tue, 27 Jun 2017 11:43:07 -0700
changeset 601763 7a82e81c8fac78c9482b7585b73a146bce6f1665
parent 601762 27754367ed8baabcce8f920fbf5be18a19fd8227
child 601764 8a954d0aab7ce7061f403390dab5fe7c7d046d2e
child 602676 101077d1686392fd2756a58e4bae61d36885ac76
push id66200
push userhchang@mozilla.com
push dateThu, 29 Jun 2017 03:53:43 +0000
bugs1355746
milestone56.0a1
Bug 1355746 - Part 2. Polish IdleTaskRunner and reuse it for background parsing. This patch is mainly to make IdleTaskRunner reusable by nsHtml5TreeOpExecutor. The only necessary work to that purpose is to remove the dependency of sShuttingDown, which was a static variable in nsJSEnvironment.cpp. The idea is to have a "ShouldCancel" as a callback for the consumer to return sShuttingDown. In addition to sShuttingDown, we use std::function<bool()> as the runner main callback type. MozReview-Commit-ID: FT2X1unSvPS
dom/base/nsJSEnvironment.cpp
parser/html/nsHtml5TreeOpExecutor.cpp
xpcom/threads/IdleTaskRunner.cpp
xpcom/threads/IdleTaskRunner.h
--- a/dom/base/nsJSEnvironment.cpp
+++ b/dom/base/nsJSEnvironment.cpp
@@ -1598,17 +1598,17 @@ nsJSContext::ClearMaxCCSliceTime()
 
 uint32_t
 nsJSContext::GetMaxCCSliceTimeSinceClear()
 {
   return gCCStats.mMaxSliceTimeSinceClear;
 }
 
 static bool
-ICCRunnerFired(TimeStamp aDeadline, void* aData)
+ICCRunnerFired(TimeStamp aDeadline)
 {
   if (sDidShutdown) {
     return false;
   }
 
   // Ignore ICC timer fires during IGC. Running ICC during an IGC will cause us
   // to synchronously finish the GC, which is bad.
 
@@ -1639,18 +1639,18 @@ nsJSContext::BeginCycleCollectionCallbac
   KillCCRunner();
 
   gCCStats.RunForgetSkippable();
 
   MOZ_ASSERT(!sICCRunner, "Tried to create a new ICC timer when one already existed.");
 
   // Create an ICC timer even if ICC is globally disabled, because we could be manually triggering
   // an incremental collection, and we want to be sure to finish it.
-  sICCRunner = CollectorRunner::Create(ICCRunnerFired, kICCIntersliceDelay,
-                                       kIdleICCSliceBudget, true);
+  sICCRunner = IdleTaskRunner::Create(ICCRunnerFired, kICCIntersliceDelay,
+                                      kIdleICCSliceBudget, true, []{ return sShuttingDown; });
 }
 
 static_assert(NS_GC_DELAY > kMaxICCDuration, "A max duration ICC shouldn't reduce GC delay to 0");
 
 //static
 void
 nsJSContext::EndCycleCollectionCallback(CycleCollectorResults &aResults)
 {
@@ -1852,21 +1852,19 @@ InterSliceGCRunnerFired(TimeStamp aDeadl
 }
 
 // static
 void
 GCTimerFired(nsITimer *aTimer, void *aClosure)
 {
   nsJSContext::KillGCTimer();
   // Now start the actual GC after initial timer has fired.
-  sInterSliceGCRunner = CollectorRunner::Create(InterSliceGCRunnerFired,
-                                                NS_INTERSLICE_GC_DELAY,
-                                                sActiveIntersliceGCBudget,
-                                                false,
-                                                aClosure);
+  sInterSliceGCRunner = IdleTaskRunner::Create([aClosure](TimeStamp aDeadline) {
+    return InterSliceGCRunnerFired(aDeadline, aClosure);
+  }, NS_INTERSLICE_GC_DELAY, sActiveIntersliceGCBudget, false, []{ return sShuttingDown; });
 }
 
 // static
 void
 ShrinkingGCTimerFired(nsITimer* aTimer, void* aClosure)
 {
   nsJSContext::KillShrinkingGCTimer();
   sIsCompactingOnUserInactive = true;
@@ -1880,17 +1878,17 @@ ShouldTriggerCC(uint32_t aSuspected)
 {
   return sNeedsFullCC ||
          aSuspected > NS_CC_PURPLE_LIMIT ||
          (aSuspected > NS_CC_FORCED_PURPLE_LIMIT &&
           TimeUntilNow(sLastCCEndTime) > NS_CC_FORCED);
 }
 
 static bool
-CCRunnerFired(TimeStamp aDeadline, void* aData)
+CCRunnerFired(TimeStamp aDeadline)
 {
   if (sDidShutdown) {
     return false;
   }
 
   static uint32_t ccDelay = NS_CC_DELAY;
   if (sCCLockedOut) {
     ccDelay = NS_CC_DELAY / 3;
@@ -2037,23 +2035,23 @@ nsJSContext::RunNextCollectorTimer()
   }
 
   // Check the CC timers after the GC timers, because the CC timers won't do
   // anything if a GC is in progress.
   MOZ_ASSERT(!sCCLockedOut, "Don't check the CC timers if the CC is locked out.");
 
   if (sCCRunner) {
     if (ReadyToTriggerExpensiveCollectorTimer()) {
-      CCRunnerFired(TimeStamp(), nullptr);
+      CCRunnerFired(TimeStamp());
     }
     return;
   }
 
   if (sICCRunner) {
-    ICCRunnerFired(TimeStamp(), nullptr);
+    ICCRunnerFired(TimeStamp());
     return;
   }
 }
 
 // static
 void
 nsJSContext::PokeGC(JS::gcreason::Reason aReason,
                     JSObject* aObj,
@@ -2150,18 +2148,19 @@ nsJSContext::MaybePokeCC()
 
   if (ShouldTriggerCC(nsCycleCollector_suspectedCount())) {
     sCCRunnerFireCount = 0;
 
     // We can kill some objects before running forgetSkippable.
     nsCycleCollector_dispatchDeferredDeletion();
 
     sCCRunner =
-      CollectorRunner::Create(CCRunnerFired, NS_CC_SKIPPABLE_DELAY,
-                              kForgetSkippableSliceDuration, true);
+      IdleTaskRunner::Create(CCRunnerFired, NS_CC_SKIPPABLE_DELAY,
+                             kForgetSkippableSliceDuration, true,
+                             []{ return sShuttingDown; });
   }
 }
 
 //static
 void
 nsJSContext::KillGCTimer()
 {
   if (sGCTimer) {
@@ -2335,18 +2334,19 @@ DOMGCSliceCallback(JSContext* aCx, JS::G
     case JS::GC_SLICE_END:
       sGCUnnotifiedTotalTime +=
         aDesc.lastSliceEnd(aCx) - aDesc.lastSliceStart(aCx);
 
       // Schedule another GC slice if the GC has more work to do.
       nsJSContext::KillInterSliceGCRunner();
       if (!sShuttingDown && !aDesc.isComplete_) {
         sInterSliceGCRunner =
-          CollectorRunner::Create(InterSliceGCRunnerFired, NS_INTERSLICE_GC_DELAY,
-                                  sActiveIntersliceGCBudget, false);
+          IdleTaskRunner::Create([](TimeStamp aDeadline) {
+            return InterSliceGCRunnerFired(aDeadline, nullptr);
+          }, NS_INTERSLICE_GC_DELAY, sActiveIntersliceGCBudget, false, []{ return sShuttingDown; });
       }
 
       if (ShouldTriggerCC(nsCycleCollector_suspectedCount())) {
         nsCycleCollector_dispatchDeferredDeletion();
       }
 
       if (sPostGCEventsToConsole) {
         nsString prefix, gcstats;
--- a/parser/html/nsHtml5TreeOpExecutor.cpp
+++ b/parser/html/nsHtml5TreeOpExecutor.cpp
@@ -28,16 +28,17 @@
 #include "mozilla/css/Loader.h"
 #include "GeckoProfiler.h"
 #include "nsIScriptError.h"
 #include "nsIScriptContext.h"
 #include "mozilla/Preferences.h"
 #include "nsIHTMLDocument.h"
 #include "nsIViewSourceChannel.h"
 #include "xpcpublic.h"
+#include "mozilla/IdleTaskRunner.h"
 
 using namespace mozilla;
 
 NS_INTERFACE_TABLE_HEAD_CYCLE_COLLECTION_INHERITED(nsHtml5TreeOpExecutor)
   NS_INTERFACE_TABLE_INHERITED(nsHtml5TreeOpExecutor,
                                nsIContentSink)
 NS_INTERFACE_TABLE_TAIL_INHERITING(nsHtml5DocumentBuilder)
 
@@ -57,17 +58,17 @@ class nsHtml5ExecutorReflusher : public 
     NS_IMETHOD Run() override
     {
       mExecutor->RunFlushLoop();
       return NS_OK;
     }
 };
 
 static mozilla::LinkedList<nsHtml5TreeOpExecutor>* gBackgroundFlushList = nullptr;
-static nsITimer* gFlushTimer = nullptr;
+StaticRefPtr<IdleTaskRunner> gBackgroundFlushRunner;
 
 nsHtml5TreeOpExecutor::nsHtml5TreeOpExecutor()
   : nsHtml5DocumentBuilder(false)
   , mSuppressEOF(false)
   , mReadingFromStage(false)
   , mStreamParser(nullptr)
   , mPreloadedURLs(23)  // Mean # of preloadable resources per page on dmoz
   , mSpeculationReferrerPolicy(mozilla::net::RP_Unset)
@@ -81,19 +82,19 @@ nsHtml5TreeOpExecutor::nsHtml5TreeOpExec
 nsHtml5TreeOpExecutor::~nsHtml5TreeOpExecutor()
 {
   if (gBackgroundFlushList && isInList()) {
     mOpQueue.Clear();
     removeFrom(*gBackgroundFlushList);
     if (gBackgroundFlushList->isEmpty()) {
       delete gBackgroundFlushList;
       gBackgroundFlushList = nullptr;
-      if (gFlushTimer) {
-        gFlushTimer->Cancel();
-        NS_RELEASE(gFlushTimer);
+      if (gBackgroundFlushRunner) {
+        gBackgroundFlushRunner->Cancel();
+        gBackgroundFlushRunner = nullptr;
       }
     }
   }
   NS_ASSERTION(mOpQueue.IsEmpty(), "Somehow there's stuff in the op queue.");
 }
 
 // nsIContentSink
 NS_IMETHODIMP
@@ -246,29 +247,31 @@ nsHtml5TreeOpExecutor::MarkAsBroken(nsre
                                       TaskCategory::Network,
                                       terminator.forget()))) {
       NS_WARNING("failed to dispatch executor flush event");
     }
   }
   return aReason;
 }
 
-void
-FlushTimerCallback(nsITimer* aTimer, void* aClosure)
+static bool
+BackgroundFlushCallback(TimeStamp /*aDeadline*/)
 {
   RefPtr<nsHtml5TreeOpExecutor> ex = gBackgroundFlushList->popFirst();
   if (ex) {
     ex->RunFlushLoop();
   }
   if (gBackgroundFlushList && gBackgroundFlushList->isEmpty()) {
     delete gBackgroundFlushList;
     gBackgroundFlushList = nullptr;
-    gFlushTimer->Cancel();
-    NS_RELEASE(gFlushTimer);
+    gBackgroundFlushRunner->Cancel();
+    gBackgroundFlushRunner = nullptr;
+    return true;
   }
+  return true;
 }
 
 void
 nsHtml5TreeOpExecutor::ContinueInterruptedParsingAsync()
 {
   if (!mDocument || !mDocument->IsInBackgroundWindow()) {
     nsCOMPtr<nsIRunnable> flusher = new nsHtml5ExecutorReflusher(this);
     if (NS_FAILED(mDocument->Dispatch("nsHtml5ExecutorReflusher",
@@ -278,26 +281,27 @@ nsHtml5TreeOpExecutor::ContinueInterrupt
     }
   } else {
     if (!gBackgroundFlushList) {
       gBackgroundFlushList = new mozilla::LinkedList<nsHtml5TreeOpExecutor>();
     }
     if (!isInList()) {
       gBackgroundFlushList->insertBack(this);
     }
-    if (!gFlushTimer) {
-      nsCOMPtr<nsITimer> t = do_CreateInstance("@mozilla.org/timer;1");
-      t.swap(gFlushTimer);
-      // The timer value 50 should not hopefully slow down background pages too
-      // much, yet lets event loop to process enough between ticks.
-      // See bug 734015.
-      gFlushTimer->InitWithNamedFuncCallback(FlushTimerCallback, nullptr,
-                                             50, nsITimer::TYPE_REPEATING_SLACK,
-                                             "FlushTimerCallback");
+    if (gBackgroundFlushRunner) {
+      NS_WARNING("We've already scheduled a task for background list flush.");
+      return;
     }
+    // Now we set up a repetitive idle scheduler for flushing background list.
+    gBackgroundFlushRunner =
+      IdleTaskRunner::Create(&BackgroundFlushCallback,
+                             50, // The hard deadline: 250ms.
+                             nsContentSink::sInteractiveParseTime / 1000, // Required budget.
+                             true, // repeating
+                             []{ return false; }); // MaybeContinueProcess
   }
 }
 
 void
 nsHtml5TreeOpExecutor::FlushSpeculativeLoads()
 {
   nsTArray<nsHtml5SpeculativeLoad> speculativeLoadQueue;
   mStage.MoveSpeculativeLoadsTo(speculativeLoadQueue);
--- a/xpcom/threads/IdleTaskRunner.cpp
+++ b/xpcom/threads/IdleTaskRunner.cpp
@@ -5,58 +5,68 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "IdleTaskRunner.h"
 #include "nsRefreshDriver.h"
 
 namespace mozilla {
 
 already_AddRefed<IdleTaskRunner>
-IdleTaskRunner::Create(IdleTaskRunnerCallback aCallback, uint32_t aDelay,
-                       int64_t aBudget, bool aRepeating, void* aData)
+IdleTaskRunner::Create(const CallbackType& aCallback, uint32_t aDelay,
+                       int64_t aBudget, bool aRepeating,
+                       const MayContinueProcessingCallbackType& aMaybeContinueProcessing)
 {
-  if (sShuttingDown) {
+  if (aMaybeContinueProcessing && aMaybeContinueProcessing()) {
     return nullptr;
   }
 
   RefPtr<IdleTaskRunner> runner =
-    new IdleTaskRunner(aCallback, aDelay, aBudget, aRepeating, aData);
+    new IdleTaskRunner(aCallback, aDelay, aBudget, aRepeating, aMaybeContinueProcessing);
   runner->Schedule(false); // Initial scheduling shouldn't use idle dispatch.
   return runner.forget();
 }
 
-IdleTaskRunner::IdleTaskRunner(IdleTaskRunnerCallback aCallback,
+IdleTaskRunner::IdleTaskRunner(const CallbackType& aCallback,
                                uint32_t aDelay, int64_t aBudget,
-                               bool aRepeating, void* aData)
+                               bool aRepeating,
+                               const MayContinueProcessingCallbackType& aMaybeContinueProcessing)
   : mCallback(aCallback), mDelay(aDelay)
   , mBudget(TimeDuration::FromMilliseconds(aBudget))
-  , mRepeating(aRepeating), mTimerActive(false), mData(aData)
+  , mRepeating(aRepeating), mTimerActive(false)
+  , mMaybeContinueProcessing(aMaybeContinueProcessing)
 {
 }
 
 NS_IMETHODIMP
 IdleTaskRunner::Run()
 {
   if (!mCallback) {
     return NS_OK;
   }
 
   // Deadline is null when called from timer.
+  TimeStamp now = TimeStamp::Now();
   bool deadLineWasNull = mDeadline.IsNull();
   bool didRun = false;
-  if (deadLineWasNull || ((TimeStamp::Now() + mBudget) < mDeadline)) {
+  bool allowIdleDispatch = false;
+  if (deadLineWasNull || ((now + mBudget) < mDeadline)) {
     CancelTimer();
-    didRun = mCallback(mDeadline, mData);
+    didRun = mCallback(mDeadline);
+    // If we didn't do meaningful work, don't schedule using immediate
+    // idle dispatch, since that could lead to a loop until the idle
+    // period ends.
+    allowIdleDispatch = didRun;
+  } else if (now >= mDeadline) {
+    allowIdleDispatch = true;
   }
 
   if (mCallback && (mRepeating || !didRun)) {
-    // If we didn't do meaningful work, don't schedule using immediate
-    // idle dispatch, since that could lead to a loop until the idle
-    // period ends.
-    Schedule(didRun);
+    Schedule(allowIdleDispatch);
+  } else {
+    mCallback = nullptr;
   }
 
   return NS_OK;
 }
 
 static void
 TimedOut(nsITimer* aTimer, void* aClosure)
 {
@@ -109,21 +119,22 @@ ScheduleTimedOut(nsITimer* aTimer, void*
 
 void
 IdleTaskRunner::Schedule(bool aAllowIdleDispatch)
 {
   if (!mCallback) {
     return;
   }
 
-  if (sShuttingDown) {
+  if (mMaybeContinueProcessing && mMaybeContinueProcessing()) {
     Cancel();
     return;
   }
 
+  TimeStamp deadline = mDeadline;
   mDeadline = TimeStamp();
   TimeStamp now = TimeStamp::Now();
   TimeStamp hint = nsRefreshDriver::GetIdleDeadlineHint(now);
   if (hint != now) {
     // RefreshDriver is ticking, let it schedule the idle dispatch.
     nsRefreshDriver::DispatchIdleRunnableAfterTick(this, mDelay);
     // Ensure we get called at some point, even if RefreshDriver is stopped.
     SetTimer(mDelay, mTarget);
@@ -140,17 +151,24 @@ IdleTaskRunner::Schedule(bool aAllowIdle
           return;
         }
       } else {
         mScheduleTimer->Cancel();
       }
 
       // We weren't allowed to do idle dispatch immediately, do it after a
       // short timeout.
-      mScheduleTimer->InitWithFuncCallback(ScheduleTimedOut, this, 16,
+      uint32_t lateScheduleDelayMs;
+      if (deadline.IsNull()) {
+        lateScheduleDelayMs = 16;
+      } else {
+        lateScheduleDelayMs =
+          (uint32_t)((deadline - now).ToMilliseconds());
+      }
+      mScheduleTimer->InitWithFuncCallback(ScheduleTimedOut, this, lateScheduleDelayMs,
                                            nsITimer::TYPE_ONE_SHOT_LOW_PRIORITY);
     }
   }
 }
 
 IdleTaskRunner::~IdleTaskRunner()
 {
   CancelTimer();
--- a/xpcom/threads/IdleTaskRunner.h
+++ b/xpcom/threads/IdleTaskRunner.h
@@ -6,49 +6,61 @@
 
 #ifndef IdleTaskRunner_h
 #define IdleTaskRunner_h
 
 #include "nsThreadUtils.h"
 
 namespace mozilla {
 
-// Return true if some meaningful work was done.
-typedef bool (*IdleTaskRunnerCallback) (TimeStamp aDeadline, void* aData);
-
-// Repeating callback runner for CC and GC.
+// A general purpose repeating callback runner (it can be configured
+// to a one-time runner, too.) If it is running repeatedly,
+// one has to either explicitly Cancel() the runner or have
+// MayContinueProcessing() callback return false to completely remove
+// the runner.
 class IdleTaskRunner final : public IdleRunnable
 {
 public:
+  // Return true if some meaningful work was done.
+  using CallbackType = std::function<bool(TimeStamp aDeadline)>;
+
+  // A callback for "continue process" decision. Return false to
+  // stop processing. This can be an alternative to Cancel() or
+  // work together in different way.
+  using MayContinueProcessingCallbackType = std::function<bool()>;
+
+public:
   static already_AddRefed<IdleTaskRunner>
-  Create(IdleTaskRunnerCallback aCallback, uint32_t aDelay,
-         int64_t aBudget, bool aRepeating, void* aData = nullptr);
+  Create(const CallbackType& aCallback, uint32_t aDelay,
+         int64_t aBudget, bool aRepeating,
+         const MayContinueProcessingCallbackType& aMaybeContinueProcessing);
 
   NS_IMETHOD Run() override;
 
   void SetDeadline(mozilla::TimeStamp aDeadline) override;
   void SetTimer(uint32_t aDelay, nsIEventTarget* aTarget) override;
 
   nsresult Cancel() override;
   void Schedule(bool aAllowIdleDispatch);
 
 private:
-  explicit IdleTaskRunner(IdleTaskRunnerCallback aCallback,
+  explicit IdleTaskRunner(const CallbackType& aCallback,
                           uint32_t aDelay, int64_t aBudget,
-                          bool aRepeating, void* aData);
+                          bool aRepeating,
+                          const MayContinueProcessingCallbackType& aMaybeContinueProcessing);
   ~IdleTaskRunner();
   void CancelTimer();
 
   nsCOMPtr<nsITimer> mTimer;
   nsCOMPtr<nsITimer> mScheduleTimer;
   nsCOMPtr<nsIEventTarget> mTarget;
-  IdleTaskRunnerCallback mCallback;
+  CallbackType mCallback;
   uint32_t mDelay;
   TimeStamp mDeadline;
   TimeDuration mBudget;
   bool mRepeating;
   bool mTimerActive;
-  void* mData;
+  MayContinueProcessingCallbackType mMaybeContinueProcessing;
 };
 
 } // end of unnamed namespace.
 
 #endif