1325254 - optimize TimerThread data structures

1325254 P1 Make TimerThread::mTimers store RefPtr<nsTimerImpl> objects.

1325254 P2 Make TimerThread list store an entry struct and just drop nsTimerImpl ref on cancel.

1325254 P3 Sort TimerThread list as a binary heap.

1325254 P4 Dynamically allocate Entry structs stored in TimerThread::mTimers.

1325254 P5 Make nsITimer::Cancel() O(c).

xpcom/threads/TimerThread.cpp

@@ -335,7 +335,7 @@ TimerThread::Shutdown()
     return NS_ERROR_NOT_INITIALIZED;
   }
 
-  nsTArray<nsTimerImpl*> timers;
+  nsTArray<UniquePtr<Entry>> timers;
   {
     // lock scope
     MonitorAutoLock lock(mMonitor);
@@ -354,14 +354,15 @@ TimerThread::Shutdown()
     // might potentially call some code reentering the same lock
     // that leads to unexpected behavior or deadlock.
     // See bug 422472.
-    timers.SwapElements(mTimers);
+    mTimers.SwapElements(timers);
   }
 
   uint32_t timersCount = timers.Length();
   for (uint32_t i = 0; i < timersCount; i++) {
-    nsTimerImpl* timer = timers[i];
-    timer->Cancel();
-    ReleaseTimerInternal(timer);
+    RefPtr<nsTimerImpl> timer = timers[i]->Take();
+    if (timer) {
+      timer->Cancel();
+    }
   }
 
   mThread->Shutdown();    // wait for the thread to die
@@ -432,12 +433,11 @@ TimerThread::Run()
     } else {
       waitFor = PR_INTERVAL_NO_TIMEOUT;
       TimeStamp now = TimeStamp::Now();
-      nsTimerImpl* timer = nullptr;
+
+      RemoveLeadingCanceledTimersInternal();
 
       if (!mTimers.IsEmpty()) {
-        timer = mTimers[0];
-
-        if (now >= timer->mTimeout || forceRunThisTimer) {
+        if (now >= mTimers[0]->Value()->mTimeout || forceRunThisTimer) {
     next:
           // NB: AddRef before the Release under RemoveTimerInternal to avoid
           // mRefCnt passing through zero, in case all other refs than the one
@@ -445,9 +445,8 @@ TimerThread::Run()
           // must be racing with us, blocked in gThread->RemoveTimer waiting
           // for TimerThread::mMonitor, under nsTimerImpl::Release.
 
-          RefPtr<nsTimerImpl> timerRef(timer);
-          RemoveTimerInternal(timer);
-          timer = nullptr;
+          RefPtr<nsTimerImpl> timerRef(mTimers[0]->Take());
+          RemoveFirstTimerInternal();
 
           MOZ_LOG(GetTimerLog(), LogLevel::Debug,
                  ("Timer thread woke up %fms from when it was supposed to\n",
@@ -489,10 +488,10 @@ TimerThread::Run()
         }
       }
 
-      if (!mTimers.IsEmpty()) {
-        timer = mTimers[0];
+      RemoveLeadingCanceledTimersInternal();
 
-        TimeStamp timeout = timer->mTimeout;
+      if (!mTimers.IsEmpty()) {
+        TimeStamp timeout = mTimers[0]->Value()->mTimeout;
 
         // Don't wait at all (even for PR_INTERVAL_NO_WAIT) if the next timer
         // is due now or overdue.
@@ -558,13 +557,12 @@ TimerThread::AddTimer(nsTimerImpl* aTimer)
   }
 
   // Add the timer to our list.
-  int32_t i = AddTimerInternal(aTimer);
-  if (i < 0) {
+  if(!AddTimerInternal(aTimer)) {
     return NS_ERROR_OUT_OF_MEMORY;
   }
 
   // Awaken the timer thread.
-  if (mWaiting && i == 0) {
+  if (mWaiting && mTimers[0]->Value() == aTimer) {
     mNotified = true;
     mMonitor.Notify();
   }
@@ -640,24 +638,23 @@ TimerThread::FindNextFireTimeForCurrentThread(TimeStamp aDefault, uint32_t aSear
 }
 
 // This function must be called from within a lock
-int32_t
+bool
 TimerThread::AddTimerInternal(nsTimerImpl* aTimer)
 {
   mMonitor.AssertCurrentThreadOwns();
   if (mShutdown) {
-    return -1;
+    return false;
   }
 
   TimeStamp now = TimeStamp::Now();
 
-  TimerAdditionComparator c(now, aTimer);
-  nsTimerImpl** insertSlot = mTimers.InsertElementSorted(aTimer, c);
-
-  if (!insertSlot) {
-    return -1;
+  UniquePtr<Entry>* entry = mTimers.AppendElement(
+    MakeUnique<Entry>(now, aTimer->mTimeout, aTimer), mozilla::fallible);
+  if (!entry) {
+    return false;
   }
 
-  NS_ADDREF(aTimer);
+  std::push_heap(mTimers.begin(), mTimers.end(), Entry::UniquePtrLessThan);
 
 #ifdef MOZ_TASK_TRACER
   // Caller of AddTimer is the parent task of its timer event, so we store the
@@ -665,30 +662,55 @@ TimerThread::AddTimerInternal(nsTimerImpl* aTimer)
   aTimer->GetTLSTraceInfo();
 #endif
 
-  return insertSlot - mTimers.Elements();
+  return true;
 }
 
 // Note: this function must be called from within a lock.
 bool
 TimerThread::RemoveTimerInternal(nsTimerImpl* aTimer)
 {
-  mMonitor.AssertCurrentThreadOwns();
-  if (!mTimers.RemoveElement(aTimer)) {
+  if (!aTimer || !aTimer->mHolder) {
     return false;
   }
-
-  ReleaseTimerInternal(aTimer);
+  aTimer->mHolder->Forget(aTimer);
   return true;
 }
 
 void
-TimerThread::ReleaseTimerInternal(nsTimerImpl* aTimer)
+TimerThread::RemoveLeadingCanceledTimersInternal()
 {
-  if (!mShutdown) {
-    // copied to a local array before releasing in shutdown
-    mMonitor.AssertCurrentThreadOwns();
+  mMonitor.AssertCurrentThreadOwns();
+
+  // Move all canceled timers from the front of the list to
+  // the back of the list using std::pop_heap().  We do this
+  // without actually removing them from the list so we can
+  // modify the nsTArray in a single bulk operation.
+  auto sortedEnd = mTimers.end();
+  while (sortedEnd != mTimers.begin() && !mTimers[0]->Value()) {
+    std::pop_heap(mTimers.begin(), sortedEnd, Entry::UniquePtrLessThan);
+    --sortedEnd;
   }
-  NS_RELEASE(aTimer);
+
+  // If there were no canceled timers then we are done.
+  if (sortedEnd == mTimers.end()) {
+    return;
+  }
+
+  // Finally, remove the canceled timers from the back of the
+  // nsTArray.  Note, since std::pop_heap() uses iterators
+  // we must convert to nsTArray indices and number of
+  // elements here.
+  mTimers.RemoveElementsAt(sortedEnd - mTimers.begin(),
+                           mTimers.end() - sortedEnd);
+}
+
+void
+TimerThread::RemoveFirstTimerInternal()
+{
+  mMonitor.AssertCurrentThreadOwns();
+  MOZ_ASSERT(!mTimers.IsEmpty());
+  std::pop_heap(mTimers.begin(), mTimers.end(), Entry::UniquePtrLessThan);
+  mTimers.RemoveElementAt(mTimers.Length() - 1);
 }
 
 already_AddRefed<nsTimerImpl>

xpcom/threads/TimerThread.h

@@ -19,6 +19,8 @@
 #include "mozilla/Attributes.h"
 #include "mozilla/Monitor.h"
 
+#include <algorithm>
+
 namespace mozilla {
 class TimeStamp;
 } // namespace mozilla
@@ -60,12 +62,12 @@ private:
   mozilla::Atomic<bool> mInitInProgress;
   bool    mInitialized;
 
-  // These two internal helper methods must be called while mMonitor is held.
-  // AddTimerInternal returns the position where the timer was added in the
-  // list, or -1 if it failed.
-  int32_t AddTimerInternal(nsTimerImpl* aTimer);
+  // These internal helper methods must be called while mMonitor is held.
+  // AddTimerInternal returns false if the insertion failed.
+  bool    AddTimerInternal(nsTimerImpl* aTimer);
   bool    RemoveTimerInternal(nsTimerImpl* aTimer);
-  void    ReleaseTimerInternal(nsTimerImpl* aTimer);
+  void    RemoveLeadingCanceledTimersInternal();
+  void    RemoveFirstTimerInternal();
 
   already_AddRefed<nsTimerImpl> PostTimerEvent(already_AddRefed<nsTimerImpl> aTimerRef);
 
@@ -81,7 +83,43 @@ private:
   bool mNotified;
   bool mSleeping;
 
-  nsTArray<nsTimerImpl*> mTimers;
+  class Entry final : public nsTimerImplHolder
+  {
+    const TimeStamp mTimeout;
+
+  public:
+    Entry(const TimeStamp& aMinTimeout, const TimeStamp& aTimeout,
+          nsTimerImpl* aTimerImpl)
+      : nsTimerImplHolder(aTimerImpl)
+      , mTimeout(std::max(aMinTimeout, aTimeout))
+    {
+    }
+
+    nsTimerImpl*
+    Value() const
+    {
+      return mTimerImpl;
+    }
+
+    already_AddRefed<nsTimerImpl>
+    Take()
+    {
+      if (mTimerImpl) {
+        mTimerImpl->SetHolder(nullptr);
+      }
+      return mTimerImpl.forget();
+    }
+
+    static bool
+    UniquePtrLessThan(UniquePtr<Entry>& aLeft, UniquePtr<Entry>& aRight)
+    {
+      // This is reversed because std::push_heap() sorts the "largest" to
+      // the front of the heap.  We want that to be the earliest timer.
+      return aRight->mTimeout < aLeft->mTimeout;
+    }
+  };
+
+  nsTArray<UniquePtr<Entry>> mTimers;
 };
 
 struct TimerAdditionComparator

xpcom/threads/nsTimerImpl.cpp

@@ -142,6 +142,7 @@ nsTimer::Release(void)
 }
 
 nsTimerImpl::nsTimerImpl(nsITimer* aTimer) :
+  mHolder(nullptr),
   mGeneration(0),
   mDelay(0),
   mITimer(aTimer),
@@ -647,6 +648,12 @@ nsTimerImpl::LogFiring(const Callback& aCallback, uint8_t aType, uint32_t aDelay
   }
 }
 
+void
+nsTimerImpl::SetHolder(nsTimerImplHolder* aHolder)
+{
+  mHolder = aHolder;
+}
+
 nsTimer::~nsTimer()
 {
 }

xpcom/threads/nsTimerImpl.h

@@ -32,12 +32,18 @@ extern mozilla::LogModule* GetTimerLog();
     {0x84, 0x27, 0xfb, 0xab, 0x44, 0xf2, 0x9b, 0xc8} \
 }
 
+class nsTimerImplHolder;
+
 // TimerThread, nsTimerEvent, and nsTimer have references to these. nsTimer has
 // a separate lifecycle so we can Cancel() the underlying timer when the user of
 // the nsTimer has let go of its last reference.
 class nsTimerImpl
 {
-  ~nsTimerImpl() {}
+  ~nsTimerImpl()
+  {
+    MOZ_ASSERT(!mHolder);
+  }
+
 public:
   typedef mozilla::TimeStamp TimeStamp;
 
@@ -158,6 +164,8 @@ public:
            mType == nsITimer::TYPE_REPEATING_SLACK_LOW_PRIORITY;
   }
 
+  void SetHolder(nsTimerImplHolder* aHolder);
+
   nsCOMPtr<nsIEventTarget> mEventTarget;
 
   void LogFiring(const Callback& aCallback, uint8_t aType, uint32_t aDelay);
@@ -168,6 +176,10 @@ public:
                                       uint32_t aType,
                                       Callback::Name aName);
 
+  // This weak reference must be cleared by the nsTimerImplHolder by calling
+  // SetHolder(nullptr) before the holder is destroyed.
+  nsTimerImplHolder*    mHolder;
+
   // These members are set by the initiating thread, when the timer's type is
   // changed and during the period where it fires on that thread.
   uint8_t               mType;
@@ -219,4 +231,39 @@ private:
   RefPtr<nsTimerImpl> mImpl;
 };
 
+// A class that holds on to an nsTimerImpl.  This lets the nsTimerImpl object
+// directly instruct its holder to forget the timer, avoiding list lookups.
+class nsTimerImplHolder
+{
+public:
+  explicit nsTimerImplHolder(nsTimerImpl* aTimerImpl)
+    : mTimerImpl(aTimerImpl)
+  {
+    if (mTimerImpl) {
+      mTimerImpl->SetHolder(this);
+    }
+  }
+
+  ~nsTimerImplHolder()
+  {
+    if (mTimerImpl) {
+      mTimerImpl->SetHolder(nullptr);
+    }
+  }
+
+  void
+  Forget(nsTimerImpl* aTimerImpl)
+  {
+    if (MOZ_UNLIKELY(!mTimerImpl)) {
+      return;
+    }
+    MOZ_ASSERT(aTimerImpl == mTimerImpl);
+    mTimerImpl->SetHolder(nullptr);
+    mTimerImpl = nullptr;
+  }
+
+protected:
+  RefPtr<nsTimerImpl> mTimerImpl;
+};
+
 #endif /* nsTimerImpl_h___ */