Bug 1370537 P2 Remove TimeoutManager::RunTimeout()'s last expired timeout reference. r=ehsan
authorBen Kelly <ben@wanderview.com>
Thu, 08 Jun 2017 05:51:59 -0700
changeset 413447 e3fbe145dafcc919baa422c1de5b2ed6935344e6
parent 413446 505c2ccffae36c0226dd946c50771928b83c2d3d
child 413448 280dedc5b8c803a9c8ac42a90c38062123da12a7
push id1490
push usermtabara@mozilla.com
push dateMon, 31 Jul 2017 14:08:16 +0000
treeherdermozilla-release@70e32e6bf15e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersehsan
bugs1370537
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 1370537 P2 Remove TimeoutManager::RunTimeout()'s last expired timeout reference. r=ehsan
dom/base/OrderedTimeoutIterator.h
dom/base/TimeoutManager.cpp
--- a/dom/base/OrderedTimeoutIterator.h
+++ b/dom/base/OrderedTimeoutIterator.h
@@ -16,82 +16,72 @@ namespace dom {
 
 // This class implements and iterator which iterates the normal and tracking
 // timeouts lists simultaneously in the mWhen order.
 class MOZ_STACK_CLASS OrderedTimeoutIterator final {
 public:
   typedef TimeoutManager::Timeouts Timeouts;
   typedef Timeouts::TimeoutList    TimeoutList;
 
-  // Passing null for aNormalStopAt or aTrackingStopAt means that the
-  // corresponding timeout list should be iterated all the way to the end.
   OrderedTimeoutIterator(Timeouts& aNormalTimeouts,
-                         Timeouts& aTrackingTimeouts,
-                         Timeout* aNormalStopAt,
-                         Timeout* aTrackingStopAt)
+                         Timeouts& aTrackingTimeouts)
     : mNormalTimeouts(aNormalTimeouts.mTimeoutList),
       mTrackingTimeouts(aTrackingTimeouts.mTimeoutList),
       mNormalIter(mNormalTimeouts.getFirst()),
       mTrackingIter(mTrackingTimeouts.getFirst()),
-      mNormalStopAt(aNormalStopAt),
-      mTrackingStopAt(aTrackingStopAt),
       mKind(Kind::None),
       mUpdateIteratorCalled(true)
   {
   }
 
   // Return the current timeout and move to the next one.
   // Unless this is the first time calling Next(), you must call
   // UpdateIterator() before calling this method.
   Timeout* Next()
   {
     MOZ_ASSERT(mUpdateIteratorCalled);
-    MOZ_ASSERT_IF(mNormalIter && mNormalIter != mNormalStopAt,
-                  mNormalIter->isInList());
-    MOZ_ASSERT_IF(mTrackingIter && mTrackingIter != mTrackingStopAt,
-                  mTrackingIter->isInList());
+    MOZ_ASSERT_IF(mNormalIter, mNormalIter->isInList());
+    MOZ_ASSERT_IF(mTrackingIter, mTrackingIter->isInList());
 
     mUpdateIteratorCalled = false;
     mKind = Kind::None;
     Timeout* timeout = nullptr;
-    if (mNormalIter == mNormalStopAt) {
-      if (mTrackingIter == mTrackingStopAt) {
+    if (!mNormalIter) {
+      if (!mTrackingIter) {
         // We have reached the end of both lists.  Bail out!
         return nullptr;
       } else {
         // We have reached the end of the normal timeout list, select the next
         // tracking timeout.
         timeout = mTrackingIter;
         mKind = Kind::Tracking;
       }
-    } else if (mTrackingIter == mTrackingStopAt) {
+    } else if (!mTrackingIter) {
       // We have reached the end of the tracking timeout list, select the next
       // normal timeout.
       timeout = mNormalIter;
       mKind = Kind::Normal;
     } else {
       // If we have a normal and a tracking timer, return the one with the
       // smaller mWhen (and prefer the timeout with a lower ID in case they are
       // equal.) Otherwise, return whichever iterator has an item left,
       // preferring a non-tracking timeout again.  Note that in practice, even
       // if a web page calls setTimeout() twice in a row, it should get
       // different mWhen values, so in practice we shouldn't fall back to
       // comparing timeout IDs.
       if (mNormalIter && mTrackingIter &&
-          mNormalIter != mNormalStopAt &&
-          mTrackingIter != mTrackingStopAt &&
           (mTrackingIter->When() < mNormalIter->When() ||
            (mTrackingIter->When() == mNormalIter->When() &&
             mTrackingIter->mTimeoutId < mNormalIter->mTimeoutId))) {
         timeout = mTrackingIter;
         mKind = Kind::Tracking;
-      } else if (mNormalIter && mNormalIter != mNormalStopAt) {
+      } else if (mNormalIter) {
         timeout = mNormalIter;
         mKind = Kind::Normal;
-      } else if (mTrackingIter && mTrackingIter != mTrackingStopAt) {
+      } else if (mTrackingIter) {
         timeout = mTrackingIter;
         mKind = Kind::Tracking;
       }
     }
     if (!timeout) {
       // We didn't find any suitable iterator.  This can happen for example
       // when getNext() in UpdateIterator() returns nullptr and then Next()
       // gets called.  Bail out!
@@ -115,24 +105,22 @@ public:
   {
     MOZ_ASSERT(mKind != Kind::None);
     // Update the winning iterator to point to the next element.  Also check to
     // see if the other iterator is still valid, otherwise reset it to the
     // beginning of the list.  This is needed in case a timeout handler removes
     // the timeout pointed to from one of our iterators.
     if (mKind == Kind::Normal) {
       mNormalIter = mCurrent->getNext();
-      if (mTrackingIter && mTrackingIter != mTrackingStopAt &&
-          !mTrackingIter->isInList()) {
+      if (mTrackingIter && !mTrackingIter->isInList()) {
         mTrackingIter = mTrackingTimeouts.getFirst();
       }
     } else {
       mTrackingIter = mCurrent->getNext();
-      if (mNormalIter && mNormalIter != mNormalStopAt &&
-          !mNormalIter->isInList()) {
+      if (mNormalIter && !mNormalIter->isInList()) {
         mNormalIter = mNormalTimeouts.getFirst();
       }
     }
 
     mUpdateIteratorCalled = true;
   }
 
   // This function resets the iterator to a defunct state.  It should only be
@@ -168,18 +156,16 @@ public:
     return mKind == Kind::Tracking;
   }
 
 private:
   TimeoutList& mNormalTimeouts;          // The list of normal timeouts.
   TimeoutList& mTrackingTimeouts;        // The list of tracking timeouts.
   RefPtr<Timeout> mNormalIter;           // The iterator over the normal timeout list.
   RefPtr<Timeout> mTrackingIter;         // The iterator over the tracking timeout list.
-  void* mNormalStopAt;                   // Where to stop iterating the normal list at.
-  void* mTrackingStopAt;                 // Where to stop iterating the tracking list at.
   RefPtr<Timeout> mCurrent;              // The current timeout that Next() just found.
   enum class Kind { Normal, Tracking, None };
   Kind mKind;                            // The kind of iterator picked the last time.
   DebugOnly<bool> mUpdateIteratorCalled; // Whether we have called UpdateIterator() before calling Next().
 };
 
 }
 }
--- a/dom/base/TimeoutManager.cpp
+++ b/dom/base/TimeoutManager.cpp
@@ -520,20 +520,17 @@ TimeoutManager::ClearTimeout(int32_t aTi
   if (!firstTimeout) {
     return;
   }
 
   // If the first timeout was cancelled we need to stop the executor and
   // restart at the next soonest deadline.
   mExecutor->Cancel();
 
-  OrderedTimeoutIterator iter(mNormalTimeouts,
-                              mTrackingTimeouts,
-                              nullptr,
-                              nullptr);
+  OrderedTimeoutIterator iter(mNormalTimeouts, mTrackingTimeouts);
   Timeout* nextTimeout = iter.Next();
   if (nextTimeout) {
     MOZ_ALWAYS_SUCCEEDS(mExecutor->MaybeSchedule(nextTimeout->When()));
   }
 }
 
 void
 TimeoutManager::RunTimeout(const TimeStamp& aNow, const TimeStamp& aTargetDeadline)
@@ -560,20 +557,16 @@ TimeoutManager::RunTimeout(const TimeSta
   // loop, though, by only checking for an elapsed limit every N timeouts.
   const uint32_t kNumTimersPerInitialElapsedCheck = 100;
 
   // Start measuring elapsed time immediately.  We won't potentially expire
   // the time budget until at least one Timeout has run, though.
   TimeStamp now(aNow);
   TimeStamp start = now;
 
-  Timeout* last_expired_normal_timeout = nullptr;
-  Timeout* last_expired_tracking_timeout = nullptr;
-  bool     last_expired_timeout_is_normal = false;
-
   uint32_t firingId = CreateFiringId();
   auto guard = MakeScopeExit([&] {
     DestroyFiringId(firingId);
   });
 
   // Make sure that the window and the script context don't go away as
   // a result of running timeouts
   nsCOMPtr<nsIScriptGlobalObject> windowKungFuDeathGrip(&mWindow);
@@ -593,52 +586,42 @@ TimeoutManager::RunTimeout(const TimeSta
     // timers that *should* have fired *will* be fired now.
 
     deadline = aTargetDeadline;
   } else {
     deadline = now;
   }
 
   TimeStamp nextDeadline;
+  uint32_t numTimersToRun = 0;
 
   // The timeout list is kept in deadline order. Discover the latest timeout
   // whose deadline has expired. On some platforms, native timeout events fire
   // "early", but we handled that above by setting deadline to aTargetDeadline
   // if the timer fired early.  So we can stop walking if we get to timeouts
   // whose When() is greater than deadline, since once that happens we know
   // nothing past that point is expired.
   {
     // Use a nested scope in order to make sure the strong references held by
     // the iterator are freed after the loop.
-    OrderedTimeoutIterator expiredIter(mNormalTimeouts,
-                                       mTrackingTimeouts,
-                                       nullptr,
-                                       nullptr);
-
-    uint32_t numTimersToRun = 0;
+    OrderedTimeoutIterator expiredIter(mNormalTimeouts, mTrackingTimeouts);
 
     while (true) {
       Timeout* timeout = expiredIter.Next();
       if (!timeout || timeout->When() > deadline) {
         if (timeout) {
           nextDeadline = timeout->When();
         }
         break;
       }
 
       if (IsInvalidFiringId(timeout->mFiringId)) {
         // Mark any timeouts that are on the list to be fired with the
         // firing depth so that we can reentrantly run timeouts
         timeout->mFiringId = firingId;
-        last_expired_timeout_is_normal = expiredIter.PickedNormalIter();
-        if (last_expired_timeout_is_normal) {
-          last_expired_normal_timeout = timeout;
-        } else {
-          last_expired_tracking_timeout = timeout;
-        }
 
         numTimersToRun += 1;
 
         // Run only a limited number of timers based on the configured maximum.
         if (numTimersToRun % kNumTimersPerInitialElapsedCheck == 0) {
           now = TimeStamp::Now();
           TimeDuration elapsed(now - start);
           if (elapsed >= initalTimeLimit) {
@@ -661,38 +644,31 @@ TimeoutManager::RunTimeout(const TimeSta
   // in order for timeouts to fire properly.
   if (!nextDeadline.IsNull()) {
     MOZ_ALWAYS_SUCCEEDS(mExecutor->MaybeSchedule(nextDeadline));
   }
 
   // Maybe the timeout that the event was fired for has been deleted
   // and there are no others timeouts with deadlines that make them
   // eligible for execution yet. Go away.
-  if (!last_expired_normal_timeout && !last_expired_tracking_timeout) {
+  if (!numTimersToRun) {
     return;
   }
 
   // Now we need to search the normal and tracking timer list at the same
   // time to run the timers in the scheduled order.
 
   // We stop iterating each list when we go past the last expired timeout from
   // that list that we have observed above.  That timeout will either be the
   // next item after the last timeout we looked at or nullptr if we have
   // exhausted the entire list while looking for the last expired timeout.
   {
     // Use a nested scope in order to make sure the strong references held by
     // the iterator are freed after the loop.
-    OrderedTimeoutIterator runIter(mNormalTimeouts,
-                                   mTrackingTimeouts,
-                                   last_expired_normal_timeout ?
-                                     last_expired_normal_timeout->getNext() :
-                                     nullptr,
-                                   last_expired_tracking_timeout ?
-                                     last_expired_tracking_timeout->getNext() :
-                                     nullptr);
+    OrderedTimeoutIterator runIter(mNormalTimeouts, mTrackingTimeouts);
     while (true) {
       RefPtr<Timeout> timeout = runIter.Next();
       if (!timeout) {
         // We have run out of timeouts!
         break;
       }
       runIter.UpdateIterator();
 
@@ -856,20 +832,17 @@ TimeoutManager::ResetTimersForThrottleRe
                                                                 *this,
                                                                 sortBy);
   NS_ENSURE_SUCCESS(rv, rv);
   rv = mTrackingTimeouts.ResetTimersForThrottleReduction(aPreviousThrottleDelayMS,
                                                          *this,
                                                          sortBy);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  OrderedTimeoutIterator iter(mNormalTimeouts,
-                              mTrackingTimeouts,
-                              nullptr,
-                              nullptr);
+  OrderedTimeoutIterator iter(mNormalTimeouts, mTrackingTimeouts);
   Timeout* firstTimeout = iter.Next();
   if (firstTimeout) {
     rv = mExecutor->MaybeSchedule(firstTimeout->When());
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
   return NS_OK;
 }