diff --git a/xpcom/threads/TimerThread.cpp b/xpcom/threads/TimerThread.cpp index 336e4d5a8b..d7c6bb10cb 100644 --- a/xpcom/threads/TimerThread.cpp +++ b/xpcom/threads/TimerThread.cpp @@ -335,7 +335,7 @@ TimerThread::Shutdown() return NS_ERROR_NOT_INITIALIZED; } - nsTArray timers; + nsTArray> 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 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 timerRef(timer); - RemoveTimerInternal(timer); - timer = nullptr; + RefPtr 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 = mTimers.AppendElement( + MakeUnique(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 diff --git a/xpcom/threads/TimerThread.h b/xpcom/threads/TimerThread.h index 132fbb3aa9..60ef749698 100644 --- a/xpcom/threads/TimerThread.h +++ b/xpcom/threads/TimerThread.h @@ -19,6 +19,8 @@ #include "mozilla/Attributes.h" #include "mozilla/Monitor.h" +#include + namespace mozilla { class TimeStamp; } // namespace mozilla @@ -60,12 +62,12 @@ private: mozilla::Atomic 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 PostTimerEvent(already_AddRefed aTimerRef); @@ -81,7 +83,43 @@ private: bool mNotified; bool mSleeping; - nsTArray 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 + Take() + { + if (mTimerImpl) { + mTimerImpl->SetHolder(nullptr); + } + return mTimerImpl.forget(); + } + + static bool + UniquePtrLessThan(UniquePtr& aLeft, UniquePtr& 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> mTimers; }; struct TimerAdditionComparator diff --git a/xpcom/threads/nsTimerImpl.cpp b/xpcom/threads/nsTimerImpl.cpp index f8fce63d94..097155905c 100644 --- a/xpcom/threads/nsTimerImpl.cpp +++ b/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() { } diff --git a/xpcom/threads/nsTimerImpl.h b/xpcom/threads/nsTimerImpl.h index 8ce70a175f..0133d52674 100644 --- a/xpcom/threads/nsTimerImpl.h +++ b/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 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 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 mTimerImpl; +}; + #endif /* nsTimerImpl_h___ */