Bug 650379. Add a new XPCOM timer type that is like TYPE_REPEATING_PRECISE but does not swamp the event queue if the callback takes longer than the timer interval to run. r=cjones, sr=brendan
authorBoris Zbarsky <bzbarsky@mit.edu>
Thu, 28 Apr 2011 19:33:52 -0400
changeset 68737 a78968ac5bbbc912fd1b933ceb085f08510ac388
parent 68736 b7e0c1485f60e6865d0ba3dcdbb38d701338ae60
child 68738 4a6f98909da538d865e7beeae6020fcbaacaa562
push id19728
push userbzbarsky@mozilla.com
push dateThu, 28 Apr 2011 23:35:48 +0000
treeherdermozilla-central@a78968ac5bbb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerscjones, brendan
bugs650379
milestone6.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 650379. Add a new XPCOM timer type that is like TYPE_REPEATING_PRECISE but does not swamp the event queue if the callback takes longer than the timer interval to run. r=cjones, sr=brendan This implements proposal 3 from bug 650379 comment 13. The main difference between TYPE_REPEATING_PRECISE and TYPE_REPEATING_PRECISE_CAN_SKIP is to not AddTimer the REPEATING_PRECISE_CAN_SKIP timer until after the callback has run; this guarantees that no more timer events will be posted until after the callback finishes executing. A secondary change is to make REPEATING_PRECISE_CAN_SKIP timers advance their firing time to mDelay from when PostTimerEvent is called, not mDelay from the old mTimeout. While this arguably makes them less precise, the alternative is that if a timer is significantly delayed for some reason (e.g. because the user puts the computer to sleep for a while) it will then fire a whole bunch of times to "catch up" to where it's supposed to be, advancing its firing time by mDelay at a time. That seems undesirable. An alternate approach would have been to readd the timer from inside PostTimerEvent, but only if we're not in the middle of firing the timer. That would allow more precise timers in the case when the callback is not taking too long, but still handle gracefully the case when the callback is slow. Unfortunately this falls down if something _else_ is hogging the main thread event loop (e.g. some other timer has a slow callback, or whatever); in that case we would post multiple events for the one precise timer while the event-loop-hogging operation is running. So I don't think we should do that.
layout/base/nsRefreshDriver.cpp
layout/generic/nsTextFrameThebes.cpp
xpcom/threads/nsITimer.idl
xpcom/threads/nsTimerImpl.cpp
xpcom/threads/nsTimerImpl.h
--- a/layout/base/nsRefreshDriver.cpp
+++ b/layout/base/nsRefreshDriver.cpp
@@ -91,17 +91,17 @@ nsRefreshDriver::GetRefreshTimerInterval
 
 PRInt32
 nsRefreshDriver::GetRefreshTimerType() const
 {
   if (mThrottled) {
     return nsITimer::TYPE_ONE_SHOT;
   }
   if (HaveAnimationFrameListeners() || sPrecisePref) {
-    return nsITimer::TYPE_REPEATING_PRECISE;
+    return nsITimer::TYPE_REPEATING_PRECISE_CAN_SKIP;
   }
   return nsITimer::TYPE_REPEATING_SLACK;
 }
 
 nsRefreshDriver::nsRefreshDriver(nsPresContext *aPresContext)
   : mPresContext(aPresContext),
     mFrozen(false),
     mThrottled(false),
@@ -192,17 +192,17 @@ nsRefreshDriver::EnsureTimerStarted(bool
   }
 
   mTimer = do_CreateInstance(NS_TIMER_CONTRACTID);
   if (!mTimer) {
     return;
   }
 
   PRInt32 timerType = GetRefreshTimerType();
-  mTimerIsPrecise = (timerType == nsITimer::TYPE_REPEATING_PRECISE);
+  mTimerIsPrecise = (timerType == nsITimer::TYPE_REPEATING_PRECISE_CAN_SKIP);
 
   nsresult rv = mTimer->InitWithCallback(this,
                                          GetRefreshTimerInterval(),
                                          timerType);
   if (NS_FAILED(rv)) {
     mTimer = nsnull;
   }
 }
@@ -386,17 +386,17 @@ nsRefreshDriver::Notify(nsITimer *aTimer
           NS_RELEASE(shell);
         }
       }
     }
   }
 
   if (mThrottled ||
       (mTimerIsPrecise !=
-       (GetRefreshTimerType() == nsITimer::TYPE_REPEATING_PRECISE))) {
+       (GetRefreshTimerType() == nsITimer::TYPE_REPEATING_PRECISE_CAN_SKIP))) {
     // Stop the timer now and restart it here.  Stopping is in the mThrottled
     // case ok because either it's already one-shot, and it just fired, and all
     // we need to do is null it out, or it's repeating and we need to reset it
     // to be one-shot.  Stopping and restarting in the case when we need to
     // switch from precise to slack timers or vice versa is unfortunately
     // required.
 
     // Note that the EnsureTimerStarted() call here is ok because
--- a/layout/generic/nsTextFrameThebes.cpp
+++ b/layout/generic/nsTextFrameThebes.cpp
@@ -3031,17 +3031,17 @@ nsBlinkTimer::~nsBlinkTimer()
   sTextBlinker = nsnull;
 }
 
 void nsBlinkTimer::Start()
 {
   nsresult rv;
   mTimer = do_CreateInstance("@mozilla.org/timer;1", &rv);
   if (NS_OK == rv) {
-    mTimer->InitWithCallback(this, 250, nsITimer::TYPE_REPEATING_PRECISE);
+    mTimer->InitWithCallback(this, 250, nsITimer::TYPE_REPEATING_PRECISE_CAN_SKIP);
   }
 }
 
 void nsBlinkTimer::Stop()
 {
   if (nsnull != mTimer) {
     mTimer->Cancel();
     mTimer = nsnull;
--- a/xpcom/threads/nsITimer.idl
+++ b/xpcom/threads/nsITimer.idl
@@ -104,21 +104,44 @@ interface nsITimer : nsISupports
   const short TYPE_REPEATING_SLACK    = 1;
   
   /**
    * An TYPE_REPEATING_PRECISE repeating timer aims to have constant period
    * between firings.  The processing time for each timer callback should not
    * influence the timer period.  However, if the processing for the last
    * timer firing could not be completed until just before the next firing
    * occurs, then you could have two timer notification routines being
-   * executed in quick succession.
+   * executed in quick succession.  Furthermore, if your callback processing
+   * time is longer than the timer period, then the timer will post more
+   * notifications while your callback is running.  For example, if a
+   * REPEATING_PRECISE timer has a 10ms period and a callback takes 50ms,
+   * then by the time the callback is done there will be 5 events to run the
+   * timer callback in the event queue.  Furthermore, the next scheduled time
+   * will always advance by exactly the delay every time the timer fires.
+   * This means that if the clock increments without the timer thread running
+   * (e.g. the computer is asleep) when the timer thread gets to run again it
+   * will post all the events that it "missed" while it wasn't running.  Use
+   * this timer type with extreme caution.  Chances are, this is not what you
+   * want.
    */
   const short TYPE_REPEATING_PRECISE  = 2;
 
   /**
+   * A TYPE_REPEATING_PRECISE_CAN_SKIP repeating timer aims to have constant
+   * period between firings.  The processing time for each timer callback
+   * should not influence the timer period.  However this timer type
+   * guarantees that it will not queue up new events to fire the callback
+   * until the previous callback event finishes firing.  If the callback
+   * takes a long time, then the next callback will be scheduled immediately
+   * afterward, but only once, unlike TYPE_REPEATING_PRECISE.  If you want a
+   * non-slack timer, you probably want this one.
+   */
+  const short TYPE_REPEATING_PRECISE_CAN_SKIP  = 3;
+
+  /**
    * Initialize a timer that will fire after the said delay.
    * A user must keep a reference to this timer till it is 
    * is no longer needed or has been cancelled.
    *
    * @param aObserver   the callback object that observes the 
    *                    ``timer-callback'' topic with the subject being
    *                    the timer itself when the timer fires:
    *
--- a/xpcom/threads/nsTimerImpl.cpp
+++ b/xpcom/threads/nsTimerImpl.cpp
@@ -392,17 +392,17 @@ void nsTimerImpl::Fire()
     PR_LOG(gTimerLog, PR_LOG_DEBUG, ("[this=%p]     delta           %4dms\n", this, (a > b) ? (PRInt32)d : -(PRInt32)d));
 
     mStart = mStart2;
     mStart2 = TimeStamp();
   }
 #endif
 
   TimeStamp timeout = mTimeout;
-  if (mType == TYPE_REPEATING_PRECISE) {
+  if (IsRepeatingPrecisely()) {
     // Precise repeating timers advance mTimeout by mDelay without fail before
     // calling Fire().
     timeout -= TimeDuration::FromMilliseconds(mDelay);
   }
   if (gThread)
     gThread->UpdateFilter(mDelay, timeout, now);
 
   if (mCallbackType == CALLBACK_TYPE_INTERFACE)
@@ -454,20 +454,24 @@ void nsTimerImpl::Fire()
 #ifdef DEBUG_TIMERS
   if (PR_LOG_TEST(gTimerLog, PR_LOG_DEBUG)) {
     PR_LOG(gTimerLog, PR_LOG_DEBUG,
            ("[this=%p] Took %fms to fire timer callback\n",
             this, (TimeStamp::Now() - now).ToMilliseconds()));
   }
 #endif
 
-  // Reschedule REPEATING_SLACK timers, but make sure that we aren't armed
-  // already (which can happen if the callback reinitialized the timer).
-  if (mType == TYPE_REPEATING_SLACK && !mArmed) {
-    SetDelayInternal(mDelay); // force mTimeout to be recomputed.
+  // Reschedule repeating timers, except REPEATING_PRECISE which already did
+  // that in PostTimerEvent, but make sure that we aren't armed already (which
+  // can happen if the callback reinitialized the timer).
+  if (IsRepeating() && mType != TYPE_REPEATING_PRECISE && !mArmed) {
+    if (mType == TYPE_REPEATING_SLACK)
+      SetDelayInternal(mDelay); // force mTimeout to be recomputed.  For
+                                // REPEATING_PRECISE_CAN_SKIP timers this has
+                                // already happened.
     if (gThread)
       gThread->AddTimer(this);
   }
 }
 
 
 class nsTimerEvent : public nsRunnable {
 public:
@@ -534,19 +538,21 @@ nsresult nsTimerImpl::PostTimerEvent()
 #ifdef DEBUG_TIMERS
   if (PR_LOG_TEST(gTimerLog, PR_LOG_DEBUG)) {
     event->mInitTime = TimeStamp::Now();
   }
 #endif
 
   // If this is a repeating precise timer, we need to calculate the time for
   // the next timer to fire before we make the callback.
-  if (mType == TYPE_REPEATING_PRECISE) {
+  if (IsRepeatingPrecisely()) {
     SetDelayInternal(mDelay);
-    if (gThread) {
+
+    // But only re-arm REPEATING_PRECISE timers.
+    if (gThread && mType == TYPE_REPEATING_PRECISE) {
       nsresult rv = gThread->AddTimer(this);
       if (NS_FAILED(rv))
         return rv;
     }
   }
 
   nsresult rv = mEventTarget->Dispatch(event, NS_DISPATCH_NORMAL);
   if (NS_FAILED(rv) && gThread)
--- a/xpcom/threads/nsTimerImpl.h
+++ b/xpcom/threads/nsTimerImpl.h
@@ -109,16 +109,27 @@ private:
     mCallbackType = CALLBACK_TYPE_UNKNOWN; 
 
     if (cbType == CALLBACK_TYPE_INTERFACE)
       NS_RELEASE(mCallback.i);
     else if (cbType == CALLBACK_TYPE_OBSERVER)
       NS_RELEASE(mCallback.o);
   }
 
+  bool IsRepeating() const {
+    PR_STATIC_ASSERT(TYPE_ONE_SHOT < TYPE_REPEATING_SLACK);
+    PR_STATIC_ASSERT(TYPE_REPEATING_SLACK < TYPE_REPEATING_PRECISE);
+    PR_STATIC_ASSERT(TYPE_REPEATING_PRECISE < TYPE_REPEATING_PRECISE_CAN_SKIP);
+    return mType >= TYPE_REPEATING_SLACK;
+  }
+
+  bool IsRepeatingPrecisely() const {
+    return mType >= TYPE_REPEATING_PRECISE;
+  }
+
   nsCOMPtr<nsIEventTarget> mEventTarget;
 
   void *                mClosure;
 
   union CallbackUnion {
     nsTimerCallbackFunc c;
     nsITimerCallback *  i;
     nsIObserver *       o;