From 2c7b0b5f89938f7f03f1e3147522450dcaf33132 Mon Sep 17 00:00:00 2001 From: Valentin Gosu Date: Wed, 4 Jul 2018 20:36:58 +0200 Subject: [PATCH] Bug 1471280 - Use nsThreadPool for DNS resolver threads r=bagder Instead of creating and deleteing each thread, we use a nsThreadPool with a max of 8 resolver threads. Whereas before each thread would run ThreadFunc exactly once then shut down, the threads may now remain active for a while. During this time we may post another task(runnable) to the thread. MozReview-Commit-ID: FiE370ic1ah --HG-- extra : rebase_source : 065bbf1f9867dfb38ac7b13816d4d06824d3a05d --- netwerk/dns/nsHostResolver.cpp | 74 ++++++++++++++++------------------ netwerk/dns/nsHostResolver.h | 9 +++-- 2 files changed, 41 insertions(+), 42 deletions(-) diff --git a/netwerk/dns/nsHostResolver.cpp b/netwerk/dns/nsHostResolver.cpp index 40f40bc703d8..c95bdb5f8872 100644 --- a/netwerk/dns/nsHostResolver.cpp +++ b/netwerk/dns/nsHostResolver.cpp @@ -20,7 +20,9 @@ #include "nsISupportsUtils.h" #include "nsIThreadManager.h" #include "nsAutoPtr.h" +#include "nsComponentManagerUtils.h" #include "nsPrintfCString.h" +#include "nsXPCOMCIDInternal.h" #include "prthread.h" #include "prerror.h" #include "prtime.h" @@ -536,11 +538,11 @@ nsHostResolver::nsHostResolver(uint32_t maxCacheEntries, , mDefaultCacheLifetime(defaultCacheEntryLifetime) , mDefaultGracePeriod(defaultGracePeriod) , mLock("nsHostResolver.mLock") - , mIdleThreadCV(mLock, "nsHostResolver.mIdleThreadCV") + , mIdleTaskCV(mLock, "nsHostResolver.mIdleTaskCV") , mEvictionQSize(0) , mShutdown(true) - , mNumIdleThreads(0) - , mThreadCount(0) + , mNumIdleTasks(0) + , mActiveTaskCount(0) , mActiveAnyThreadCount(0) , mPendingCount(0) { @@ -592,6 +594,13 @@ nsHostResolver::Init() } #endif + nsCOMPtr threadPool = do_CreateInstance(NS_THREADPOOL_CONTRACTID); + MOZ_ALWAYS_SUCCEEDS(threadPool->SetThreadLimit(MAX_RESOLVER_THREADS)); + MOZ_ALWAYS_SUCCEEDS(threadPool->SetIdleThreadLimit(MAX_RESOLVER_THREADS)); + MOZ_ALWAYS_SUCCEEDS(threadPool->SetThreadStackSize(nsIThreadManager::kThreadPoolStackSize)); + MOZ_ALWAYS_SUCCEEDS(threadPool->SetName(NS_LITERAL_CSTRING("DNS Resolver"))); + mResolverThreads = threadPool.forget(); + return NS_OK; } @@ -674,8 +683,8 @@ nsHostResolver::Shutdown() mEvictionQSize = 0; mPendingCount = 0; - if (mNumIdleThreads) - mIdleThreadCV.NotifyAll(); + if (mNumIdleTasks) + mIdleTaskCV.NotifyAll(); // empty host database mRecordDB.Clear(); @@ -705,12 +714,12 @@ nsHostResolver::Shutdown() // Use this approach instead of PR_JoinThread() because that does // not allow a timeout which may be necessary for a semi-responsive // shutdown if the thread is blocked on a very slow DNS resolution. - // mThreadCount is read outside of mLock, but the worst case + // mActiveTaskCount is read outside of mLock, but the worst case // scenario for that race is one extra 25ms sleep. PRIntervalTime delay = PR_MillisecondsToInterval(25); PRIntervalTime stopTime = PR_IntervalNow() + PR_SecondsToInterval(20); - while (mThreadCount && PR_IntervalNow() < stopTime) + while (mActiveTaskCount && PR_IntervalNow() < stopTime) PR_Sleep(delay); #endif @@ -719,6 +728,8 @@ nsHostResolver::Shutdown() NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), "Failed to shutdown GetAddrInfo"); } + + mResolverThreads->Shutdown(); } nsresult @@ -989,7 +1000,7 @@ nsHostResolver::ResolveHost(const nsACString &aHost, rec->remove(); mMediumQ.insertBack(rec); rec->flags = flags; - mIdleThreadCV.Notify(); + mIdleTaskCV.Notify(); } } } @@ -1051,31 +1062,20 @@ nsHostResolver::DetachCallback(const nsACString &host, nsresult nsHostResolver::ConditionallyCreateThread(nsHostRecord *rec) { - if (mNumIdleThreads) { - // wake up idle thread to process this lookup - mIdleThreadCV.Notify(); + if (mNumIdleTasks) { + // wake up idle tasks to process this lookup + mIdleTaskCV.Notify(); } - else if ((mThreadCount < HighThreadThreshold) || - (IsHighPriority(rec->flags) && mThreadCount < MAX_RESOLVER_THREADS)) { - static nsThreadPoolNaming naming; - nsCString name = naming.GetNextThreadName("DNS Resolver"); - - // dispatch new worker thread - nsCOMPtr thread; - nsresult rv = NS_NewNamedThread(name, getter_AddRefs(thread), nullptr, - nsIThreadManager::kThreadPoolStackSize); - if (NS_WARN_IF(NS_FAILED(rv)) || !thread) { - return rv; - } - + else if ((mActiveTaskCount < HighThreadThreshold) || + (IsHighPriority(rec->flags) && mActiveTaskCount < MAX_RESOLVER_THREADS)) { nsCOMPtr event = mozilla::NewRunnableMethod("nsHostResolver::ThreadFunc", this, &nsHostResolver::ThreadFunc); - mThreadCount++; - rv = thread->Dispatch(event, nsIEventTarget::DISPATCH_NORMAL); + mActiveTaskCount++; + nsresult rv = mResolverThreads->Dispatch(event, nsIEventTarget::DISPATCH_NORMAL); if (NS_FAILED(rv)) { - mThreadCount--; + mActiveTaskCount--; } } else { @@ -1226,9 +1226,9 @@ nsHostResolver::NativeLookup(nsHostRecord *aRec) nsresult rv = ConditionallyCreateThread(rec); LOG ((" DNS thread counters: total=%d any-live=%d idle=%d pending=%d\n", - static_cast(mThreadCount), + static_cast(mActiveTaskCount), static_cast(mActiveAnyThreadCount), - static_cast(mNumIdleThreads), + static_cast(mNumIdleTasks), static_cast(mPendingCount))); return rv; @@ -1322,7 +1322,7 @@ nsHostResolver::GetHostToLookup(nsHostRecord **result) MutexAutoLock lock(mLock); - timeout = (mNumIdleThreads >= HighThreadThreshold) ? mShortIdleTimeout : mLongIdleTimeout; + timeout = (mNumIdleTasks >= HighThreadThreshold) ? mShortIdleTimeout : mLongIdleTimeout; epoch = TimeStamp::Now(); while (!mShutdown) { @@ -1364,9 +1364,9 @@ nsHostResolver::GetHostToLookup(nsHostRecord **result) // (2) the shutdown flag has been set // (3) the thread has been idle for too long - mNumIdleThreads++; - mIdleThreadCV.Wait(timeout); - mNumIdleThreads--; + mNumIdleTasks++; + mIdleTaskCV.Wait(timeout); + mNumIdleTasks--; now = TimeStamp::Now(); @@ -1879,12 +1879,8 @@ nsHostResolver::ThreadFunc() } } while(true); - nsCOMPtr thread = NS_GetCurrentThread(); - NS_DispatchToMainThread(NS_NewRunnableFunction("nsHostResolver::ThreadFunc::AsyncShutdown", [thread]() { - thread->AsyncShutdown(); - })); - mThreadCount--; - LOG(("DNS lookup thread - queue empty, thread finished.\n")); + mActiveTaskCount--; + LOG(("DNS lookup thread - queue empty, task finished.\n")); } void diff --git a/netwerk/dns/nsHostResolver.h b/netwerk/dns/nsHostResolver.h index 99f6336ad514..8839e566550a 100644 --- a/netwerk/dns/nsHostResolver.h +++ b/netwerk/dns/nsHostResolver.h @@ -23,6 +23,7 @@ #include "mozilla/TimeStamp.h" #include "mozilla/UniquePtr.h" #include "nsRefPtrHashtable.h" +#include "nsIThreadPool.h" class nsHostResolver; class nsResolveHostCallback; @@ -444,7 +445,7 @@ private: uint32_t mDefaultCacheLifetime; // granularity seconds uint32_t mDefaultGracePeriod; // granularity seconds mutable Mutex mLock; // mutable so SizeOfIncludingThis can be const - CondVar mIdleThreadCV; + CondVar mIdleTaskCV; nsRefPtrHashtable, nsHostRecord> mRecordDB; mozilla::LinkedList> mHighQ; mozilla::LinkedList> mMediumQ; @@ -455,9 +456,11 @@ private: mozilla::TimeDuration mLongIdleTimeout; mozilla::TimeDuration mShortIdleTimeout; + RefPtr mResolverThreads; + mozilla::Atomic mShutdown; - mozilla::Atomic mNumIdleThreads; - mozilla::Atomic mThreadCount; + mozilla::Atomic mNumIdleTasks; + mozilla::Atomic mActiveTaskCount; mozilla::Atomic mActiveAnyThreadCount; mozilla::Atomic mPendingCount;