Bug 590422: Remove delay line filter from timer thread. r=bz

dom/base/nsGlobalWindow.cpp

@@ -10088,11 +10088,11 @@ nsGlobalWindow::RunTimeout(nsTimeout *aTimeout)
   TimeStamp deadline;
 
   if (aTimeout && aTimeout->mWhen > now) {
-    // The OS timer fired early (yikes!), and possibly out of order
-    // too. Set |deadline| to be the time when the OS timer *should*
-    // have fired so that any timers that *should* have fired before
-    // aTimeout *will* be fired now. This happens most of the time on
-    // Win2k.
+    // The OS timer fired early (which can happen due to the timers
+    // having lower precision than TimeStamp does).  Set |deadline| to
+    // be the time when the OS timer *should* have fired so that any
+    // timers that *should* have fired before aTimeout *will* be fired
+    // now.
 
     deadline = aTimeout->mWhen;
   } else {

xpcom/threads/TimerThread.cpp

@@ -25,9 +25,7 @@ TimerThread::TimerThread() :
   mMonitor("TimerThread.mMonitor"),
   mShutdown(false),
   mWaiting(false),
-  mSleeping(false),
-  mDelayLineCounter(0),
-  mMinTimerPeriod(0)
+  mSleeping(false)
 {
 }
 
@@ -161,60 +159,6 @@ nsresult TimerThread::Shutdown()
   return NS_OK;
 }
 
-// Keep track of how early (positive slack) or late (negative slack) timers
-// are running, and use the filtered slack number to adaptively estimate how
-// early timers should fire to be "on time".
-void TimerThread::UpdateFilter(uint32_t aDelay, TimeStamp aTimeout,
-                               TimeStamp aNow)
-{
-  TimeDuration slack = aTimeout - aNow;
-  double smoothSlack = 0;
-  uint32_t i, filterLength;
-  static TimeDuration kFilterFeedbackMaxTicks =
-    TimeDuration::FromMilliseconds(FILTER_FEEDBACK_MAX);
-  static TimeDuration kFilterFeedbackMinTicks =
-    TimeDuration::FromMilliseconds(-FILTER_FEEDBACK_MAX);
-
-  if (slack > kFilterFeedbackMaxTicks)
-    slack = kFilterFeedbackMaxTicks;
-  else if (slack < kFilterFeedbackMinTicks)
-    slack = kFilterFeedbackMinTicks;
-
-  mDelayLine[mDelayLineCounter & DELAY_LINE_LENGTH_MASK] =
-    slack.ToMilliseconds();
-  if (++mDelayLineCounter < DELAY_LINE_LENGTH) {
-    // Startup mode: accumulate a full delay line before filtering.
-    PR_ASSERT(mTimeoutAdjustment.ToSeconds() == 0);
-    filterLength = 0;
-  } else {
-    // Past startup: compute number of filter taps based on mMinTimerPeriod.
-    if (mMinTimerPeriod == 0) {
-      mMinTimerPeriod = (aDelay != 0) ? aDelay : 1;
-    } else if (aDelay != 0 && aDelay < mMinTimerPeriod) {
-      mMinTimerPeriod = aDelay;
-    }
-
-    filterLength = (uint32_t) (FILTER_DURATION / mMinTimerPeriod);
-    if (filterLength > DELAY_LINE_LENGTH)
-      filterLength = DELAY_LINE_LENGTH;
-    else if (filterLength < 4)
-      filterLength = 4;
-
-    for (i = 1; i <= filterLength; i++)
-      smoothSlack += mDelayLine[(mDelayLineCounter-i) & DELAY_LINE_LENGTH_MASK];
-    smoothSlack /= filterLength;
-
-    // XXXbe do we need amplification?  hacking a fudge factor, need testing...
-    mTimeoutAdjustment = TimeDuration::FromMilliseconds(smoothSlack * 1.5);
-  }
-
-#ifdef DEBUG_TIMERS
-  PR_LOG(GetTimerLog(), PR_LOG_DEBUG,
-         ("UpdateFilter: smoothSlack = %g, filterLength = %u\n",
-          smoothSlack, filterLength));
-#endif
-}
-
 /* void Run(); */
 NS_IMETHODIMP TimerThread::Run()
 {
@@ -259,7 +203,7 @@ NS_IMETHODIMP TimerThread::Run()
       if (!mTimers.IsEmpty()) {
         timer = mTimers[0];
 
-        if (now >= timer->mTimeout + mTimeoutAdjustment) {
+        if (now >= timer->mTimeout) {
     next:
           // NB: AddRef before the Release under RemoveTimerInternal to avoid
           // mRefCnt passing through zero, in case all other refs than the one
@@ -317,7 +261,7 @@ NS_IMETHODIMP TimerThread::Run()
       if (!mTimers.IsEmpty()) {
         timer = mTimers[0];
 
-        TimeStamp timeout = timer->mTimeout + mTimeoutAdjustment;
+        TimeStamp timeout = timer->mTimeout;
 
         // Don't wait at all (even for PR_INTERVAL_NO_WAIT) if the next timer
         // is due now or overdue.
@@ -329,7 +273,7 @@ NS_IMETHODIMP TimerThread::Run()
         double microseconds = (timeout - now).ToMilliseconds()*1000;
         if (microseconds < halfMicrosecondsIntervalResolution)
           goto next; // round down; execute event now
-        waitFor = PR_MicrosecondsToInterval(microseconds);
+        waitFor = PR_MicrosecondsToInterval(static_cast<PRUint32>(microseconds)); // Floor is accurate enough.
         if (waitFor == 0)
           waitFor = 1; // round up, wait the minimum time we can wait
       }
@@ -418,7 +362,7 @@ int32_t TimerThread::AddTimerInternal(nsTimerImpl *aTimer)
 
   TimeStamp now = TimeStamp::Now();
 
-  TimerAdditionComparator c(now, mTimeoutAdjustment, aTimer);
+  TimerAdditionComparator c(now, aTimer);
   nsTimerImpl** insertSlot = mTimers.InsertElementSorted(aTimer, c);
 
   if (!insertSlot)
@@ -461,9 +405,6 @@ void TimerThread::DoAfterSleep()
     timer->SetDelay(delay);
   }
 
-  // nuke the stored adjustments, so they get recalibrated
-  mTimeoutAdjustment = TimeDuration(0);
-  mDelayLineCounter = 0;
   mSleeping = false;
 }
 

xpcom/threads/TimerThread.h

@@ -40,12 +40,6 @@ public:
   nsresult TimerDelayChanged(nsTimerImpl *aTimer);
   nsresult RemoveTimer(nsTimerImpl *aTimer);
 
-#define FILTER_DURATION         1e3     /* one second */
-#define FILTER_FEEDBACK_MAX     100     /* 1/10th of a second */
-
-  void UpdateFilter(uint32_t aDelay, TimeStamp aTimeout,
-                    TimeStamp aNow);
-
   void DoBeforeSleep();
   void DoAfterSleep();
 
@@ -70,23 +64,12 @@ private:
   bool mSleeping;
   
   nsTArray<nsTimerImpl*> mTimers;
-
-#define DELAY_LINE_LENGTH_LOG2  5
-#define DELAY_LINE_LENGTH_MASK  ((1u << DELAY_LINE_LENGTH_LOG2) - 1)
-#define DELAY_LINE_LENGTH       (1u << DELAY_LINE_LENGTH_LOG2)
-
-  int32_t  mDelayLine[DELAY_LINE_LENGTH]; // milliseconds
-  uint32_t mDelayLineCounter;
-  uint32_t mMinTimerPeriod;     // milliseconds
-  TimeDuration mTimeoutAdjustment;
 };
 
 struct TimerAdditionComparator {
   TimerAdditionComparator(const mozilla::TimeStamp &aNow,
-                          const mozilla::TimeDuration &aTimeoutAdjustment,
                           nsTimerImpl *aTimerToInsert) :
-    now(aNow),
-    timeoutAdjustment(aTimeoutAdjustment)
+    now(aNow)
 #ifdef DEBUG
     , timerToInsert(aTimerToInsert)
 #endif
@@ -96,11 +79,7 @@ struct TimerAdditionComparator {
     NS_ABORT_IF_FALSE(newTimer == timerToInsert, "Unexpected timer ordering");
 
     // Skip any overdue timers.
-
-    // XXXbz why?  Given our definition of overdue in terms of
-    // mTimeoutAdjustment, aTimer might be overdue already!  Why not
-    // just fire timers in order?
-    return now >= fromArray->mTimeout + timeoutAdjustment ||
+    return fromArray->mTimeout <= now ||
            fromArray->mTimeout <= newTimer->mTimeout;
   }
 
@@ -110,7 +89,6 @@ struct TimerAdditionComparator {
 
 private:
   const mozilla::TimeStamp &now;
-  const mozilla::TimeDuration &timeoutAdjustment;
 #ifdef DEBUG
   const nsTimerImpl * const timerToInsert;
 #endif

xpcom/threads/nsTimerImpl.cpp

@@ -460,8 +460,6 @@ void nsTimerImpl::Fire()
     // calling Fire().
     timeout -= TimeDuration::FromMilliseconds(mDelay);
   }
-  if (gThread)
-    gThread->UpdateFilter(mDelay, timeout, now);
 
   if (mCallbackType == CALLBACK_TYPE_INTERFACE)
     mTimerCallbackWhileFiring = mCallback.i;

xpcom/threads/nsTimerImpl.h

@@ -52,7 +52,7 @@ public:
   static NS_HIDDEN_(void) Shutdown();
 
   friend class TimerThread;
-  friend class TimerAdditionComparator;
+  friend struct TimerAdditionComparator;
 
   void Fire();
   nsresult PostTimerEvent();