From 6a18f5b1a79e3f64b7e170ca1ed804058d3fbd78 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Thu, 28 Apr 2011 19:33:52 -0400 Subject: [PATCH] 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 | 6 +++--- layout/generic/nsTextFrameThebes.cpp | 2 +- xpcom/threads/nsITimer.idl | 25 ++++++++++++++++++++++++- xpcom/threads/nsTimerImpl.cpp | 20 +++++++++++++------- xpcom/threads/nsTimerImpl.h | 11 +++++++++++ 5 files changed, 52 insertions(+), 12 deletions(-) diff --git a/layout/base/nsRefreshDriver.cpp b/layout/base/nsRefreshDriver.cpp index 65adc383e2375..11a045b6bef17 100644 --- a/layout/base/nsRefreshDriver.cpp +++ b/layout/base/nsRefreshDriver.cpp @@ -96,7 +96,7 @@ nsRefreshDriver::GetRefreshTimerType() const 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; } @@ -197,7 +197,7 @@ nsRefreshDriver::EnsureTimerStarted(bool aAdjustingTimer) } PRInt32 timerType = GetRefreshTimerType(); - mTimerIsPrecise = (timerType == nsITimer::TYPE_REPEATING_PRECISE); + mTimerIsPrecise = (timerType == nsITimer::TYPE_REPEATING_PRECISE_CAN_SKIP); nsresult rv = mTimer->InitWithCallback(this, GetRefreshTimerInterval(), @@ -391,7 +391,7 @@ nsRefreshDriver::Notify(nsITimer *aTimer) 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 diff --git a/layout/generic/nsTextFrameThebes.cpp b/layout/generic/nsTextFrameThebes.cpp index 46de4a60b1f30..edfa147c69780 100644 --- a/layout/generic/nsTextFrameThebes.cpp +++ b/layout/generic/nsTextFrameThebes.cpp @@ -3036,7 +3036,7 @@ 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); } } diff --git a/xpcom/threads/nsITimer.idl b/xpcom/threads/nsITimer.idl index bcea228515592..8ebd9dd452bb7 100644 --- a/xpcom/threads/nsITimer.idl +++ b/xpcom/threads/nsITimer.idl @@ -109,10 +109,33 @@ interface nsITimer : nsISupports * 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 diff --git a/xpcom/threads/nsTimerImpl.cpp b/xpcom/threads/nsTimerImpl.cpp index 200e1283fce8f..0f993d314ba64 100644 --- a/xpcom/threads/nsTimerImpl.cpp +++ b/xpcom/threads/nsTimerImpl.cpp @@ -397,7 +397,7 @@ void nsTimerImpl::Fire() #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); @@ -459,10 +459,14 @@ void nsTimerImpl::Fire() } #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); } @@ -539,9 +543,11 @@ nsresult nsTimerImpl::PostTimerEvent() // 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; diff --git a/xpcom/threads/nsTimerImpl.h b/xpcom/threads/nsTimerImpl.h index 1b1e3369e2b01..d146d68108e46 100644 --- a/xpcom/threads/nsTimerImpl.h +++ b/xpcom/threads/nsTimerImpl.h @@ -114,6 +114,17 @@ class nsTimerImpl : public nsITimer 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 mEventTarget; void * mClosure;