Bug 1325254 P4 Dynamically allocate Entry structs stored in TimerThread::mTimers. r=froydnj
authorBen Kelly <ben@wanderview.com>
Thu, 20 Apr 2017 17:56:07 -0400
changeset 566343 4b6b6aad7688b3e1a506c898e760997877f8802d
parent 566342 a501885ead3537098c84d651f0a0d9662a2c339f
child 566344 030533bed090ea932ca32d6af60ba22bde4345a8
push id55180
push userjjong@mozilla.com
push dateFri, 21 Apr 2017 09:36:13 +0000
reviewersfroydnj
bugs1325254
milestone55.0a1
Bug 1325254 P4 Dynamically allocate Entry structs stored in TimerThread::mTimers. r=froydnj
xpcom/threads/TimerThread.cpp
xpcom/threads/TimerThread.h
--- a/xpcom/threads/TimerThread.cpp
+++ b/xpcom/threads/TimerThread.cpp
@@ -340,17 +340,17 @@ nsresult
 TimerThread::Shutdown()
 {
   MOZ_LOG(GetTimerLog(), LogLevel::Debug, ("TimerThread::Shutdown begin\n"));
 
   if (!mThread) {
     return NS_ERROR_NOT_INITIALIZED;
   }
 
-  nsTArray<Entry> timers;
+  nsTArray<UniquePtr<Entry>> timers;
   {
     // lock scope
     MonitorAutoLock lock(mMonitor);
 
     mShutdown = true;
 
     // notify the cond var so that Run() can return
     if (mWaiting) {
@@ -364,17 +364,17 @@ TimerThread::Shutdown()
     // might potentially call some code reentering the same lock
     // that leads to unexpected behavior or deadlock.
     // See bug 422472.
     mTimers.SwapElements(timers);
   }
 
   uint32_t timersCount = timers.Length();
   for (uint32_t i = 0; i < timersCount; i++) {
-    RefPtr<nsTimerImpl> timer = timers[i].mTimerImpl.forget();
+    RefPtr<nsTimerImpl> timer = timers[i]->mTimerImpl.forget();
     if (timer) {
       timer->Cancel();
     }
   }
 
   mThread->Shutdown();    // wait for the thread to die
 
   nsTimerEvent::Shutdown();
@@ -443,17 +443,17 @@ TimerThread::Run()
     } else {
       waitFor = PR_INTERVAL_NO_TIMEOUT;
       TimeStamp now = TimeStamp::Now();
       nsTimerImpl* timer = nullptr;
 
       RemoveLeadingCanceledTimersInternal();
 
       if (!mTimers.IsEmpty()) {
-        timer = mTimers[0].mTimerImpl;
+        timer = mTimers[0]->mTimerImpl;
 
         if (now >= timer->mTimeout || forceRunThisTimer) {
     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.
@@ -500,17 +500,17 @@ TimerThread::Run()
           // tick or two, and we may goto next below.
           now = TimeStamp::Now();
         }
       }
 
       RemoveLeadingCanceledTimersInternal();
 
       if (!mTimers.IsEmpty()) {
-        timer = mTimers[0].mTimerImpl;
+        timer = mTimers[0]->mTimerImpl;
 
         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
@@ -573,17 +573,17 @@ TimerThread::AddTimer(nsTimerImpl* aTime
   }
 
   // Add the timer to our list.
   if(!AddTimerInternal(aTimer)) {
     return NS_ERROR_OUT_OF_MEMORY;
   }
 
   // Awaken the timer thread.
-  if (mWaiting && mTimers[0].mTimerImpl == aTimer) {
+  if (mWaiting && mTimers[0]->mTimerImpl == aTimer) {
     mNotified = true;
     mMonitor.Notify();
   }
 
   return NS_OK;
 }
 
 nsresult
@@ -613,58 +613,58 @@ TimerThread::AddTimerInternal(nsTimerImp
 {
   mMonitor.AssertCurrentThreadOwns();
   if (mShutdown) {
     return false;
   }
 
   TimeStamp now = TimeStamp::Now();
 
-  Entry* entry = mTimers.AppendElement(Entry(now, aTimer->mTimeout, aTimer),
-                                       mozilla::fallible);
+  UniquePtr<Entry>* entry = mTimers.AppendElement(
+    MakeUnique<Entry>(now, aTimer->mTimeout, aTimer), mozilla::fallible);
   if (!entry) {
     return false;
   }
 
-  std::push_heap(mTimers.begin(), mTimers.end());
+  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
   // TraceInfo here for later used.
   aTimer->GetTLSTraceInfo();
 #endif
 
   return true;
 }
 
 bool
 TimerThread::RemoveTimerInternal(nsTimerImpl* aTimer)
 {
   mMonitor.AssertCurrentThreadOwns();
   for (uint32_t i = 0; i < mTimers.Length(); ++i) {
-    if (mTimers[i].mTimerImpl == aTimer) {
-      mTimers[i].mTimerImpl = nullptr;
+    if (mTimers[i]->mTimerImpl == aTimer) {
+      mTimers[i]->mTimerImpl = nullptr;
       return true;
     }
   }
   return false;
 }
 
 void
 TimerThread::RemoveLeadingCanceledTimersInternal()
 {
   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].mTimerImpl) {
-    std::pop_heap(mTimers.begin(), sortedEnd);
+  while (sortedEnd != mTimers.begin() && !mTimers[0]->mTimerImpl) {
+    std::pop_heap(mTimers.begin(), sortedEnd, Entry::UniquePtrLessThan);
     --sortedEnd;
   }
 
   // If there were no canceled timers then we are done.
   if (sortedEnd == mTimers.end()) {
     return;
   }
 
@@ -676,17 +676,17 @@ TimerThread::RemoveLeadingCanceledTimers
                            mTimers.end() - sortedEnd);
 }
 
 void
 TimerThread::RemoveFirstTimerInternal()
 {
   mMonitor.AssertCurrentThreadOwns();
   MOZ_ASSERT(!mTimers.IsEmpty());
-  std::pop_heap(mTimers.begin(), mTimers.end());
+  std::pop_heap(mTimers.begin(), mTimers.end(), Entry::UniquePtrLessThan);
   mTimers.RemoveElementAt(mTimers.Length() - 1);
 }
 
 already_AddRefed<nsTimerImpl>
 TimerThread::PostTimerEvent(already_AddRefed<nsTimerImpl> aTimerRef)
 {
   mMonitor.AssertCurrentThreadOwns();
 
--- a/xpcom/threads/TimerThread.h
+++ b/xpcom/threads/TimerThread.h
@@ -76,42 +76,35 @@ private:
 
   bool mShutdown;
   bool mWaiting;
   bool mNotified;
   bool mSleeping;
 
   struct Entry
   {
-    TimeStamp mTimeout;
+    const TimeStamp mTimeout;
     RefPtr<nsTimerImpl> mTimerImpl;
 
     Entry(const TimeStamp& aMinTimeout, const TimeStamp& aTimeout,
           nsTimerImpl* aTimerImpl)
       : mTimeout(std::max(aMinTimeout, aTimeout)),
       mTimerImpl(aTimerImpl)
     { }
 
-    Entry(Entry&& aRight) = default;
-    Entry& operator=(Entry&& aRight) = default;
-
-    bool operator<(const Entry& aRight) const
+    static bool
+    UniquePtrLessThan(UniquePtr<Entry>& aLeft, UniquePtr<Entry>& aRight)
     {
-      // Reverse logic since we are inserting into a max heap
-      // that sorts the "largest" value to index 0.
-      return mTimeout > aRight.mTimeout;
-    }
-
-    bool operator==(const Entry& aRight) const
-    {
-      return mTimeout == aRight.mTimeout;
+      // 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<Entry> mTimers;
+  nsTArray<UniquePtr<Entry>> mTimers;
 };
 
 struct TimerAdditionComparator
 {
   TimerAdditionComparator(const mozilla::TimeStamp& aNow,
                           nsTimerImpl* aTimerToInsert) :
     now(aNow)
 #ifdef DEBUG