Skip to content

Commit

Permalink
Bug 650379. Add a new XPCOM timer type that is like TYPE_REPEATING_PR…
Browse files Browse the repository at this point in the history
…ECISE 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.
  • Loading branch information
bzbarsky committed Apr 28, 2011
1 parent decad45 commit 6a18f5b
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 12 deletions.
6 changes: 3 additions & 3 deletions layout/base/nsRefreshDriver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion layout/generic/nsTextFrameThebes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down
25 changes: 24 additions & 1 deletion xpcom/threads/nsITimer.idl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
20 changes: 13 additions & 7 deletions xpcom/threads/nsTimerImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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;
Expand Down
11 changes: 11 additions & 0 deletions xpcom/threads/nsTimerImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<nsIEventTarget> mEventTarget;

void * mClosure;
Expand Down

0 comments on commit 6a18f5b

Please sign in to comment.