Bug 590422: Remove delay line filter from timer thread. r=bz
☠☠ backed out by 4ee88e30412b ☠ ☠
authorAvi Halachmi <avihpit@yahoo.com>
Wed, 20 Feb 2013 20:21:09 +0200
changeset 123068 00ed3d264438713fe8450d6c713b1f2040ee33ea
parent 123067 a1d01526d34421e59928bd4e9babd3e5bdb46a25
child 123069 13ccc41033ec691658a2f08d88589b73c63a3ddb
push id24372
push useremorley@mozilla.com
push dateWed, 27 Feb 2013 13:22:59 +0000
treeherdermozilla-central@0a91da5f5eab [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs590422
milestone22.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 590422: Remove delay line filter from timer thread. r=bz
dom/base/nsGlobalWindow.cpp
xpcom/threads/TimerThread.cpp
xpcom/threads/TimerThread.h
xpcom/threads/nsTimerImpl.cpp
xpcom/threads/nsTimerImpl.h
--- a/dom/base/nsGlobalWindow.cpp
+++ b/dom/base/nsGlobalWindow.cpp
@@ -10083,21 +10083,21 @@ nsGlobalWindow::RunTimeout(nsTimeout *aT
   nsCOMPtr<nsIScriptGlobalObject> windowKungFuDeathGrip(this);
 
   // A native timer has gone off. See which of our timeouts need
   // servicing
   TimeStamp now = TimeStamp::Now();
   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 {
     deadline = now;
   }
 
   // The timeout list is kept in deadline order. Discover the latest
   // timeout whose deadline has expired. On some platforms, native
--- a/xpcom/threads/TimerThread.cpp
+++ b/xpcom/threads/TimerThread.cpp
@@ -20,19 +20,17 @@ using namespace mozilla;
 NS_IMPL_THREADSAFE_ISUPPORTS2(TimerThread, nsIRunnable, nsIObserver)
 
 TimerThread::TimerThread() :
   mInitInProgress(0),
   mInitialized(false),
   mMonitor("TimerThread.mMonitor"),
   mShutdown(false),
   mWaiting(false),
-  mSleeping(false),
-  mDelayLineCounter(0),
-  mMinTimerPeriod(0)
+  mSleeping(false)
 {
 }
 
 TimerThread::~TimerThread()
 {
   mThread = nullptr;
 
   NS_ASSERTION(mTimers.IsEmpty(), "Timers remain in TimerThread::~TimerThread");
@@ -156,70 +154,16 @@ nsresult TimerThread::Shutdown()
   }
 
   mThread->Shutdown();    // wait for the thread to die
 
   PR_LOG(GetTimerLog(), PR_LOG_DEBUG, ("TimerThread::Shutdown end\n"));
   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()
 {
   PR_SetCurrentThreadName("Timer");
 
   MonitorAutoLock lock(mMonitor);
 
   // We need to know how many microseconds give a positive PRIntervalTime. This
@@ -254,17 +198,17 @@ NS_IMETHODIMP TimerThread::Run()
     } else {
       waitFor = PR_INTERVAL_NO_TIMEOUT;
       TimeStamp now = TimeStamp::Now();
       nsTimerImpl *timer = nullptr;
 
       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
           // from mTimers have gone away (the last non-mTimers[i]-ref's Release
           // must be racing with us, blocked in gThread->RemoveTimer waiting
           // for TimerThread::mMonitor, under nsTimerImpl::Release.
 
           NS_ADDREF(timer);
@@ -312,29 +256,29 @@ NS_IMETHODIMP TimerThread::Run()
           // tick or two, and we may goto next below.
           now = TimeStamp::Now();
         }
       }
 
       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.
         //
         // Note that we can only sleep for integer values of a certain
         // resolution. We use halfMicrosecondsIntervalResolution, calculated
         // before, to do the optimal rounding (i.e., of how to decide what
         // interval is so small we should not wait at all).
         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
       }
 
 #ifdef DEBUG_TIMERS
       if (PR_LOG_TEST(GetTimerLog(), PR_LOG_DEBUG)) {
         if (waitFor == PR_INTERVAL_NO_TIMEOUT)
           PR_LOG(GetTimerLog(), PR_LOG_DEBUG,
@@ -413,17 +357,17 @@ nsresult TimerThread::RemoveTimer(nsTime
 // This function must be called from within a lock
 int32_t TimerThread::AddTimerInternal(nsTimerImpl *aTimer)
 {
   if (mShutdown)
     return -1;
 
   TimeStamp now = TimeStamp::Now();
 
-  TimerAdditionComparator c(now, mTimeoutAdjustment, aTimer);
+  TimerAdditionComparator c(now, aTimer);
   nsTimerImpl** insertSlot = mTimers.InsertElementSorted(aTimer, c);
 
   if (!insertSlot)
     return -1;
 
   aTimer->mArmed = true;
   NS_ADDREF(aTimer);
   return insertSlot - mTimers.Elements();
@@ -456,19 +400,16 @@ void TimerThread::DoAfterSleep()
   for (uint32_t i = 0; i < mTimers.Length(); i ++) {
     nsTimerImpl *timer = mTimers[i];
     // get and set the delay to cause its timeout to be recomputed
     uint32_t delay;
     timer->GetDelay(&delay);
     timer->SetDelay(delay);
   }
 
-  // nuke the stored adjustments, so they get recalibrated
-  mTimeoutAdjustment = TimeDuration(0);
-  mDelayLineCounter = 0;
   mSleeping = false;
 }
 
 
 /* void observe (in nsISupports aSubject, in string aTopic, in wstring aData); */
 NS_IMETHODIMP
 TimerThread::Observe(nsISupports* /* aSubject */, const char *aTopic, const PRUnichar* /* aData */)
 {
--- a/xpcom/threads/TimerThread.h
+++ b/xpcom/threads/TimerThread.h
@@ -35,22 +35,16 @@ public:
   
   NS_HIDDEN_(nsresult) Init();
   NS_HIDDEN_(nsresult) Shutdown();
 
   nsresult AddTimer(nsTimerImpl *aTimer);
   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();
 
 private:
   ~TimerThread();
 
   int32_t mInitInProgress;
   bool    mInitialized;
@@ -65,55 +59,39 @@ private:
   nsCOMPtr<nsIThread> mThread;
   Monitor mMonitor;
 
   bool mShutdown;
   bool mWaiting;
   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
   {}
 
   PRBool LessThan(nsTimerImpl *fromArray, nsTimerImpl *newTimer) const {
     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;
   }
 
   PRBool Equals(nsTimerImpl* fromArray, nsTimerImpl* newTimer) const {
     return PR_FALSE;
   }
 
 private:
   const mozilla::TimeStamp &now;
-  const mozilla::TimeDuration &timeoutAdjustment;
 #ifdef DEBUG
   const nsTimerImpl * const timerToInsert;
 #endif
 };
 
 #endif /* TimerThread_h___ */
--- a/xpcom/threads/nsTimerImpl.cpp
+++ b/xpcom/threads/nsTimerImpl.cpp
@@ -455,18 +455,16 @@ void nsTimerImpl::Fire()
 #endif
 
   TimeStamp timeout = mTimeout;
   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)
     mTimerCallbackWhileFiring = mCallback.i;
   mFiring = true;
   
   // Handle callbacks that re-init the timer, but avoid leaking.
   // See bug 330128.
   CallbackUnion callback = mCallback;
--- a/xpcom/threads/nsTimerImpl.h
+++ b/xpcom/threads/nsTimerImpl.h
@@ -47,17 +47,17 @@ public:
   typedef mozilla::TimeStamp TimeStamp;
 
   nsTimerImpl();
 
   static NS_HIDDEN_(nsresult) Startup();
   static NS_HIDDEN_(void) Shutdown();
 
   friend class TimerThread;
-  friend class TimerAdditionComparator;
+  friend struct TimerAdditionComparator;
 
   void Fire();
   nsresult PostTimerEvent();
   void SetDelayInternal(uint32_t aDelay);
 
   NS_DECL_ISUPPORTS
   NS_DECL_NSITIMER