Bug 1368286 - Take the idle queue into account in nsThread::HasPendingEvents(); r=smaug
authorEhsan Akhgari <ehsan@mozilla.com>
Fri, 02 Jun 2017 20:40:12 -0400
changeset 410718 16798a4167a180349f1c566fad9bc87bcfd0e992
parent 410717 551c2fc04c72cb29ba18af90af2c4deac9261f21
child 410719 216d4d5cbb0c924b71616e7d721d99845962fb4f
push id7391
push usermtabara@mozilla.com
push dateMon, 12 Jun 2017 13:08:53 +0000
treeherdermozilla-beta@2191d7f87e2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1368286
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1368286 - Take the idle queue into account in nsThread::HasPendingEvents(); r=smaug
xpcom/threads/nsThread.cpp
xpcom/threads/nsThread.h
--- a/xpcom/threads/nsThread.cpp
+++ b/xpcom/threads/nsThread.cpp
@@ -639,16 +639,17 @@ nsThread::nsThread(MainThreadFlag aMainT
   , mNestedEventLoopDepth(0)
   , mStackSize(aStackSize)
   , mShutdownContext(nullptr)
   , mShutdownRequired(false)
   , mEventsAreDoomed(false)
   , mIsMainThread(aMainThread)
   , mLastUnlabeledRunnable(TimeStamp::Now())
   , mCanInvokeJS(false)
+  , mHasPendingEventsPromisedIdleEvent(false)
 {
 }
 
 nsThread::~nsThread()
 {
   NS_ASSERTION(mRequestedShutdownContexts.IsEmpty(),
                "shouldn't be waiting on other threads to shutdown");
 #ifdef DEBUG
@@ -1032,26 +1033,77 @@ nsThread::Shutdown()
       return !context->mAwaitingShutdownAck;
     }, context->mJoiningThread);
 
   ShutdownComplete(context);
 
   return NS_OK;
 }
 
+TimeStamp
+nsThread::GetIdleDeadline()
+{
+  TimeStamp idleDeadline;
+  {
+    // Releasing the lock temporarily since getting the idle period
+    // might need to lock the timer thread. Unlocking here might make
+    // us receive an event on the main queue, but we've committed to
+    // run an idle event anyhow.
+    MutexAutoUnlock unlock(mLock);
+    mIdlePeriod->GetIdlePeriodHint(&idleDeadline);
+  }
+
+  // If HasPendingEvents() has been called and it has returned true because of
+  // pending idle events, there is a risk that we may decide here that we aren't
+  // idle and return null, in which case HasPendingEvents() has effectively
+  // lied.  Since we can't go back and fix the past, we have to adjust what we
+  // do here and forcefully pick the idle queue task here.  Note that this means
+  // that we are choosing to run a task from the idle queue when we would
+  // normally decide that we aren't in an idle period, but this can only happen
+  // if we fall out of the idle period in between the call to HasPendingEvents()
+  // and here, which should hopefully be quite rare.  We are effectively
+  // choosing to prioritize the sanity of our API semantics over the optimal
+  // scheduling.
+  if (!mHasPendingEventsPromisedIdleEvent &&
+      (!idleDeadline || idleDeadline < TimeStamp::Now())) {
+    return TimeStamp();
+  }
+  if (mHasPendingEventsPromisedIdleEvent && !idleDeadline) {
+    // If HasPendingEvents() has been called and it has returned true, but we're no
+    // longer in the idle period, we must return a valid timestamp to pretend that
+    // we are still in the idle period.
+    return TimeStamp::Now();
+  }
+  return idleDeadline;
+}
+
 NS_IMETHODIMP
 nsThread::HasPendingEvents(bool* aResult)
 {
   if (NS_WARN_IF(PR_GetCurrentThread() != mThread)) {
     return NS_ERROR_NOT_SAME_THREAD;
   }
 
   {
     MutexAutoLock lock(mLock);
-    *aResult = mEvents->HasPendingEvent(lock);
+    mHasPendingEventsPromisedIdleEvent = false;
+    bool hasPendingEvent = mEvents->HasPendingEvent(lock);
+    bool hasPendingIdleEvent = false;
+    if (!hasPendingEvent) {
+      // Note that GetIdleDeadline() checks mHasPendingEventsPromisedIdleEvent,
+      // but that's OK since we set it to false in the beginning of this method!
+      TimeStamp idleDeadline = GetIdleDeadline();
+
+      // Only examine the idle queue if we are in an idle period.
+      if (idleDeadline) {
+        hasPendingIdleEvent = mIdleEvents.HasPendingEvent(lock);
+        mHasPendingEventsPromisedIdleEvent = hasPendingIdleEvent;
+      }
+    }
+    *aResult = hasPendingEvent || hasPendingIdleEvent;
   }
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsThread::RegisterIdlePeriod(already_AddRefed<nsIIdlePeriod> aIdlePeriod)
 {
   if (NS_WARN_IF(PR_GetCurrentThread() != mThread)) {
@@ -1149,31 +1201,23 @@ void canary_alarm_handler(int signum)
 
 void
 nsThread::GetIdleEvent(nsIRunnable** aEvent, MutexAutoLock& aProofOfLock)
 {
   MOZ_ASSERT(PR_GetCurrentThread() == mThread);
   MOZ_ASSERT(aEvent);
 
   if (!mIdleEvents.HasPendingEvent(aProofOfLock)) {
+    MOZ_ASSERT(!mHasPendingEventsPromisedIdleEvent);
     aEvent = nullptr;
     return;
   }
 
-  TimeStamp idleDeadline;
-  {
-    // Releasing the lock temporarily since getting the idle period
-    // might need to lock the timer thread. Unlocking here might make
-    // us receive an event on the main queue, but we've committed to
-    // run an idle event anyhow.
-    MutexAutoUnlock unlock(mLock);
-    mIdlePeriod->GetIdlePeriodHint(&idleDeadline);
-  }
-
-  if (!idleDeadline || idleDeadline < TimeStamp::Now()) {
+  TimeStamp idleDeadline = GetIdleDeadline();
+  if (!idleDeadline) {
     aEvent = nullptr;
     return;
   }
 
   mIdleEvents.GetEvent(false, aEvent, aProofOfLock);
 
   if (*aEvent) {
     nsCOMPtr<nsIIdleRunnable> idleEvent(do_QueryInterface(*aEvent));
@@ -1186,16 +1230,20 @@ nsThread::GetIdleEvent(nsIRunnable** aEv
 void
 nsThread::GetEvent(bool aWait, nsIRunnable** aEvent,
                    unsigned short* aPriority,
                    MutexAutoLock& aProofOfLock)
 {
   MOZ_ASSERT(PR_GetCurrentThread() == mThread);
   MOZ_ASSERT(aEvent);
 
+  MakeScopeExit([&] {
+    mHasPendingEventsPromisedIdleEvent = false;
+  });
+
   // We'll try to get an event to execute in three stages.
   // [1] First we just try to get it from the regular queue without waiting.
   mEvents->GetEvent(false, aEvent, aPriority, aProofOfLock);
 
   // [2] If we didn't get an event from the regular queue, try to
   // get one from the idle queue
   if (!*aEvent) {
     // Since events in mEvents have higher priority than idle
--- a/xpcom/threads/nsThread.h
+++ b/xpcom/threads/nsThread.h
@@ -91,16 +91,18 @@ public:
   };
 
   static bool SaveMemoryReportNearOOM(ShouldSaveMemoryReport aShouldSave);
 #endif
 
 private:
   void DoMainThreadSpecificProcessing(bool aReallyWait);
 
+  // Returns a null TimeStamp if we're not in the idle period.
+  mozilla::TimeStamp GetIdleDeadline();
   void GetIdleEvent(nsIRunnable** aEvent, mozilla::MutexAutoLock& aProofOfLock);
   void GetEvent(bool aWait, nsIRunnable** aEvent,
                 unsigned short* aPriority,
                 mozilla::MutexAutoLock& aProofOfLock);
 
 protected:
   class nsChainedEventQueue;
 
@@ -273,16 +275,20 @@ protected:
   MainThreadFlag mIsMainThread;
 
   // The time when we last ran an unlabeled runnable (one not associated with a
   // SchedulerGroup).
   mozilla::TimeStamp mLastUnlabeledRunnable;
 
   // Set to true if this thread creates a JSRuntime.
   bool mCanInvokeJS;
+  // Set to true if HasPendingEvents() has been called and returned true because
+  // of a pending idle event.  This is used to remember to return that idle
+  // event from GetIdleEvent() to ensure that HasPendingEvents() never lies.
+  bool mHasPendingEventsPromisedIdleEvent;
 };
 
 #if defined(XP_UNIX) && !defined(ANDROID) && !defined(DEBUG) && HAVE_UALARM \
   && defined(_GNU_SOURCE)
 # define MOZ_CANARY
 
 extern int sCanaryOutputFD;
 #endif