Bug 1343912 P4 Only execute consecutive timeout handlers for a limit period of time. r=ehsan
authorBen Kelly <ben@wanderview.com>
Fri, 19 May 2017 13:45:55 -0700
changeset 359731 cb29b5cc977c17d747d8277e6e7672a24fbc51e0
parent 359730 598bd9d7eecc1177e56ea8dccc927b8fb35ff243
child 359732 a8582a3560cc1103229eaf7667c11ac533776198
push id31855
push userarchaeopteryx@coole-files.de
push dateSat, 20 May 2017 16:49:13 +0000
treeherdermozilla-central@5b74bbf20e80 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersehsan
bugs1343912
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 1343912 P4 Only execute consecutive timeout handlers for a limit period of time. r=ehsan
dom/base/TimeoutManager.cpp
modules/libpref/init/all.js
--- a/dom/base/TimeoutManager.cpp
+++ b/dom/base/TimeoutManager.cpp
@@ -240,16 +240,21 @@ static int32_t gTimeoutBucketingStrategy
 // timer code can handle, really. See DELAY_INTERVAL_LIMIT in
 // nsTimerImpl.h for details.
 #define DOM_MAX_TIMEOUT_VALUE    DELAY_INTERVAL_LIMIT
 
 uint32_t TimeoutManager::sNestingLevel = 0;
 
 namespace {
 
+// The maximum number of milliseconds to allow consecutive timer callbacks
+// to run in a single event loop runnable.
+#define DEFAULT_MAX_CONSECUTIVE_CALLBACKS_MILLISECONDS 4
+uint32_t gMaxConsecutiveCallbacksMilliseconds;
+
 // The maximum number of timer callbacks we will try to run in a single event
 // loop runnable.
 #define DEFAULT_TARGET_MAX_CONSECUTIVE_CALLBACKS 5
 uint32_t gTargetMaxConsecutiveCallbacks;
 
 // The number of queued runnables within the TabGroup ThrottledEventQueue
 // at which to begin applying back pressure to the window.
 #define DEFAULT_THROTTLED_EVENT_QUEUE_BACK_PRESSURE 5000
@@ -361,16 +366,20 @@ TimeoutManager::Initialize()
                                DEFAULT_BACK_PRESSURE_DELAY_REDUCTION_THRESHOLD_MS);
   Preferences::AddUintVarCache(&gBackPressureDelayMinimumMS,
                                "dom.timeout.back_pressure_delay_minimum_ms",
                                DEFAULT_BACK_PRESSURE_DELAY_MINIMUM_MS);
 
   Preferences::AddUintVarCache(&gTargetMaxConsecutiveCallbacks,
                                "dom.timeout.max_consecutive_callbacks",
                                DEFAULT_TARGET_MAX_CONSECUTIVE_CALLBACKS);
+
+  Preferences::AddUintVarCache(&gMaxConsecutiveCallbacksMilliseconds,
+                               "dom.timeout.max_consecutive_callbacks_ms",
+                               DEFAULT_MAX_CONSECUTIVE_CALLBACKS_MILLISECONDS);
 }
 
 uint32_t
 TimeoutManager::GetTimeoutId(Timeout::Reason aReason)
 {
   switch (aReason) {
     case Timeout::Reason::eIdleCallbackTimeout:
       return ++mIdleCallbackTimeoutCounter;
@@ -584,16 +593,33 @@ void
 TimeoutManager::RunTimeout(Timeout* aTimeout)
 {
   if (mWindow.IsSuspended()) {
     return;
   }
 
   NS_ASSERTION(!mWindow.IsFrozen(), "Timeout running on a window in the bfcache!");
 
+  // Limit the overall time spent in RunTimeout() to reduce jank.
+  uint32_t totalTimeLimitMS = std::max(1u, gMaxConsecutiveCallbacksMilliseconds);
+  const TimeDuration totalTimeLimit = TimeDuration::FromMilliseconds(totalTimeLimitMS);
+
+  // Allow up to 25% of our total time budget to be used figuring out which
+  // timers need to run.  This is the initial loop in this method.
+  const TimeDuration initalTimeLimit =
+    TimeDuration::FromMilliseconds(totalTimeLimit.ToMilliseconds() / 4);
+
+  // Ammortize overhead from from calling TimeStamp::Now() in the initial
+  // 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 start = TimeStamp::Now();
+
   Timeout* last_expired_normal_timeout = nullptr;
   Timeout* last_expired_tracking_timeout = nullptr;
   bool     last_expired_timeout_is_normal = false;
   Timeout* last_normal_insertion_point = nullptr;
   Timeout* last_tracking_insertion_point = nullptr;
 
   uint32_t firingId = CreateFiringId();
   auto guard = MakeScopeExit([&] {
@@ -672,20 +698,32 @@ TimeoutManager::RunTimeout(Timeout* aTim
         // Further timers that are ready will get picked up by their own
         // nsITimer runnables when they execute.
         //
         // For chrome windows, however, we do coalesce all timers and
         // do not yield the main thread.  This is partly because we
         // trust chrome windows not to misbehave and partly because a
         // number of browser chrome tests have races that depend on this
         // coalescing.
-        if (targetTimerSeen &&
-            numTimersToRun >= gTargetMaxConsecutiveCallbacks &&
-            !mWindow.IsChromeWindow()) {
-          break;
+        //
+        // Chrome windows are still subject to our time budget limit,
+        // however.  The time budget allows many timers to coallesce and
+        // chrome script should not hit this limit under normal
+        // circumstances.
+        if (targetTimerSeen) {
+          if (numTimersToRun >= gTargetMaxConsecutiveCallbacks &&
+              !mWindow.IsChromeWindow()) {
+            break;
+          }
+          if (numTimersToRun % kNumTimersPerInitialElapsedCheck == 0) {
+            TimeDuration elapsed(TimeStamp::Now() - start);
+            if (elapsed >= initalTimeLimit) {
+              break;
+            }
+          }
         }
       }
 
       expiredIter.UpdateIterator();
     }
   }
 
   // Maybe the timeout that the event was fired for has been deleted
@@ -729,16 +767,18 @@ TimeoutManager::RunTimeout(Timeout* aTim
 
   last_tracking_insertion_point = mTrackingTimeouts.InsertionPoint();
   if (!last_expired_timeout_is_normal) {
     // If we ever start setting mTrackingTimeoutInsertionPoint to a non-dummy timeout,
     // the logic in ResetTimersForThrottleReduction will need to change.
     mTrackingTimeouts.SetInsertionPoint(dummy_tracking_timeout);
   }
 
+  bool targetTimeoutSeen = false;
+
   // 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
   // dummy timeout for the list that the last expired timeout came from, or it
   // will 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
@@ -784,16 +824,20 @@ TimeoutManager::RunTimeout(Timeout* aTim
         // No context means this window was closed or never properly
         // initialized for this language.  This timer will never fire
         // so just remove it.
         timeout->remove();
         timeout->Release();
         continue;
       }
 
+      if (timeout == aTimeout) {
+        targetTimeoutSeen = true;
+      }
+
       // This timeout is good to run
       bool timeout_was_cleared = mWindow.RunTimeoutHandler(timeout, scx);
       MOZ_LOG(gLog, LogLevel::Debug,
               ("Run%s(TimeoutManager=%p, timeout=%p, aTimeout=%p, tracking=%d) returned %d\n", timeout->mIsInterval ? "Interval" : "Timeout",
                this, timeout, aTimeout,
                int(aTimeout->mIsTracking),
                !!timeout_was_cleared));
 
@@ -849,16 +893,28 @@ TimeoutManager::RunTimeout(Timeout* aTim
           mNormalTimeouts.Insert(timeout,
                                  mWindow.IsFrozen() ? Timeouts::SortBy::TimeRemaining
                                                     : Timeouts::SortBy::TimeWhen);
         }
       }
 
       // Release the timeout struct since it's possibly out of the list
       timeout->Release();
+
+      // Check to see if we have run out of time to execute timeout handlers.
+      // If we've exceeded our time budget then terminate the loop immediately.
+      //
+      // Note, we only do this if we have seen the Timeout object explicitly
+      // passed to RunTimeout().  The target timeout must always be executed.
+      if (targetTimeoutSeen) {
+        TimeDuration elapsed = TimeStamp::Now() - start;
+        if (elapsed >= totalTimeLimit) {
+          break;
+        }
+      }
     }
   }
 
   // Take the dummy timeout off the head of the list
   if (dummy_normal_timeout->isInList()) {
     dummy_normal_timeout->remove();
   }
   timeoutExtraRef1 = nullptr;
--- a/modules/libpref/init/all.js
+++ b/modules/libpref/init/all.js
@@ -5716,16 +5716,20 @@ pref("dom.IntersectionObserver.enabled",
 
 // Whether module scripts (<script type="module">) are enabled for content.
 pref("dom.moduleScripts.enabled", false);
 
 // Maximum number of setTimeout()/setInterval() callbacks to run in a single
 // event loop runnable. Minimum value of 1.
 pref("dom.timeout.max_consecutive_callbacks", 5);
 
+// Maximum amount of time in milliseconds consecutive setTimeout()/setInterval()
+// callback are allowed to run before yielding the event loop.
+pref("dom.timeout.max_consecutive_callbacks_ms", 4);
+
 #ifdef FUZZING
 pref("fuzzing.enabled", false);
 #endif
 
 // Set advanced layers preferences here to have them show up in about:config or
 // to be overridable in reftest.list files. They should pretty much all be set
 // to a value of 2, and the conditional-pref code in gfxPrefs.h will convert
 // it to a boolean as appropriate. In particular, do NOT add ifdefs here to