Bug 1478732 - Backed out changeset 9de74f5039a4 r=backout a=backout
authorValentin Gosu <valentin.gosu@gmail.com>
Thu, 09 Aug 2018 18:56:02 +0200
changeset 485940 937c64cef999
parent 485939 25a65f1051c2
child 485953 f8c756895a37
push id9719
push userffxbld-merge
push dateFri, 24 Aug 2018 17:49:46 +0000
treeherdermozilla-beta@719ec98fba77 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbackout, backout
bugs1478732
milestone63.0a1
first release with
nightly linux32
937c64cef999 / 63.0a1 / 20180809220138 / files
nightly linux64
937c64cef999 / 63.0a1 / 20180809220138 / files
nightly mac
937c64cef999 / 63.0a1 / 20180809220138 / files
nightly win32
937c64cef999 / 63.0a1 / 20180809220138 / files
nightly win64
937c64cef999 / 63.0a1 / 20180809220138 / files
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
releases
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1478732 - Backed out changeset 9de74f5039a4 r=backout a=backout
modules/libpref/init/all.js
netwerk/dns/nsHostResolver.cpp
netwerk/dns/nsHostResolver.h
--- a/modules/libpref/init/all.js
+++ b/modules/libpref/init/all.js
@@ -2083,17 +2083,17 @@ pref("network.dns.localDomains", "");
 // domains mapped to localhost with localDomains stay localhost.
 pref("network.dns.forceResolve", "");
 
 // Contols whether or not "localhost" should resolve when offline
 pref("network.dns.offline-localhost", true);
 
 // Defines how much longer resolver threads should stay idle before are shut down.
 // A negative value will keep the thread alive forever.
-pref("network.dns.resolver-thread-extra-idle-time-seconds", -1);
+pref("network.dns.resolver-thread-extra-idle-time-seconds", 60);
 
 // The maximum allowed length for a URL - 1MB default
 pref("network.standard-url.max-length", 1048576);
 
 // Whether nsIURI.host/.hostname/.spec should return a punycode string
 // If set to false we will revert to previous behaviour and return a unicode string.
 pref("network.standard-url.punycode-host", true);
 
--- a/netwerk/dns/nsHostResolver.cpp
+++ b/netwerk/dns/nsHostResolver.cpp
@@ -543,18 +543,20 @@ NS_IMPL_ISUPPORTS0(nsHostResolver)
 
 nsHostResolver::nsHostResolver(uint32_t maxCacheEntries,
                                uint32_t defaultCacheEntryLifetime,
                                uint32_t defaultGracePeriod)
     : mMaxCacheEntries(maxCacheEntries)
     , mDefaultCacheLifetime(defaultCacheEntryLifetime)
     , mDefaultGracePeriod(defaultGracePeriod)
     , mLock("nsHostResolver.mLock")
+    , mIdleTaskCV(mLock, "nsHostResolver.mIdleTaskCV")
     , mEvictionQSize(0)
     , mShutdown(true)
+    , mNumIdleTasks(0)
     , mActiveTaskCount(0)
     , mActiveAnyThreadCount(0)
     , mPendingCount(0)
 {
     mCreationTime = PR_Now();
 
     mLongIdleTimeout  = TimeDuration::FromSeconds(LongIdleTimeoutSeconds);
     mShortIdleTimeout = TimeDuration::FromSeconds(ShortIdleTimeoutSeconds);
@@ -598,17 +600,17 @@ nsHostResolver::Init()
     static int initCount = 0;
     if (initCount++ > 0) {
         LOG(("Calling 'res_ninit'.\n"));
         res_ninit(&_res);
     }
 #endif
 
     // We can configure the threadpool to keep threads alive for a while after
-    // the last ResolveHostTask has been executed.
+    // the last ThreadFunc task has been executed.
     int32_t poolTimeoutSecs = Preferences::GetInt(kPrefThreadIdleTime, 60);
     uint32_t poolTimeoutMs;
     if (poolTimeoutSecs < 0) {
         // This means never shut down the idle threads
         poolTimeoutMs = UINT32_MAX;
     } else {
         // We clamp down the idle time between 0 and one hour.
         poolTimeoutMs = mozilla::clamped<uint32_t>(poolTimeoutSecs * 1000,
@@ -700,16 +702,19 @@ nsHostResolver::Shutdown()
         pendingQHigh = std::move(mHighQ);
         pendingQMed = std::move(mMediumQ);
         pendingQLow = std::move(mLowQ);
         evictionQ = std::move(mEvictionQ);
 
         mEvictionQSize = 0;
         mPendingCount = 0;
 
+        if (mNumIdleTasks)
+            mIdleTaskCV.NotifyAll();
+
         // empty host database
         mRecordDB.Clear();
     }
 
     ClearPendingQueue(pendingQHigh);
     ClearPendingQueue(pendingQMed);
     ClearPendingQueue(pendingQLow);
 
@@ -1006,23 +1011,25 @@ nsHostResolver::ResolveHost(const nsACSt
 
                     if (IsHighPriority(flags) &&
                         !IsHighPriority(rec->flags)) {
                         // Move from (low|med) to high.
                         NS_ASSERTION(rec->onQueue, "Moving Host Record Not Currently Queued");
                         rec->remove();
                         mHighQ.insertBack(rec);
                         rec->flags = flags;
+                        ConditionallyCreateThread(rec);
                     } else if (IsMediumPriority(flags) &&
                                IsLowPriority(rec->flags)) {
                         // Move from low to med.
                         NS_ASSERTION(rec->onQueue, "Moving Host Record Not Currently Queued");
                         rec->remove();
                         mMediumQ.insertBack(rec);
                         rec->flags = flags;
+                        mIdleTaskCV.Notify();
                     }
                 }
             }
         }
     }
 
     if (result) {
         if (callback->isInList()) {
@@ -1071,16 +1078,41 @@ nsHostResolver::DetachCallback(const nsA
 
     // complete callback with the given status code; this would only be done if
     // the record was in the process of being resolved.
     if (rec) {
         callback->OnResolveHostComplete(this, rec, status);
     }
 }
 
+nsresult
+nsHostResolver::ConditionallyCreateThread(nsHostRecord *rec)
+{
+    if (mNumIdleTasks) {
+        // wake up idle tasks to process this lookup
+        mIdleTaskCV.Notify();
+    }
+    else if ((mActiveTaskCount < HighThreadThreshold) ||
+             (IsHighPriority(rec->flags) && mActiveTaskCount < MAX_RESOLVER_THREADS)) {
+        nsCOMPtr<nsIRunnable> event =
+            mozilla::NewRunnableMethod("nsHostResolver::ThreadFunc",
+                                       this,
+                                       &nsHostResolver::ThreadFunc);
+        mActiveTaskCount++;
+        nsresult rv = mResolverThreads->Dispatch(event, nsIEventTarget::DISPATCH_NORMAL);
+        if (NS_FAILED(rv)) {
+            mActiveTaskCount--;
+        }
+    }
+    else {
+        LOG(("  Unable to find a thread for looking up host [%s].\n", rec->host.get()));
+    }
+    return NS_OK;
+}
+
 // make sure the mTrrLock is held when this is used!
 #define TRROutstanding() ((rec->mTrrA || rec->mTrrAAAA))
 
 nsresult
 nsHostResolver::TrrLookup_unlocked(nsHostRecord *rec, TRR *pushedTRR)
 {
     MutexAutoLock lock(mLock);
     return TrrLookup(rec, pushedTRR);
@@ -1210,26 +1242,25 @@ nsHostResolver::NativeLookup(nsHostRecor
     }
     mPendingCount++;
 
     rec->mNative = true;
     rec->mNativeUsed = true;
     rec->onQueue = true;
     rec->mResolving++;
 
-    LOG (("  DNS thread counters: total=%d any-live=%d pending=%d\n",
+    nsresult rv = ConditionallyCreateThread(rec);
+
+    LOG (("  DNS thread counters: total=%d any-live=%d idle=%d pending=%d\n",
           static_cast<uint32_t>(mActiveTaskCount),
           static_cast<uint32_t>(mActiveAnyThreadCount),
+          static_cast<uint32_t>(mNumIdleTasks),
           static_cast<uint32_t>(mPendingCount)));
 
-    nsCOMPtr<nsIRunnable> event =
-        mozilla::NewRunnableMethod("nsHostResolver::ResolveHostTask",
-                                   this,
-                                   &nsHostResolver::ResolveHostTask);
-    return mResolverThreads->Dispatch(event, nsIEventTarget::DISPATCH_NORMAL);
+    return rv;
 }
 
 ResolverMode
 nsHostResolver::Mode()
 {
     if (gTRRService) {
         return static_cast<ResolverMode>(gTRRService->Mode());
     }
@@ -1304,44 +1335,83 @@ nsHostResolver::DeQueue(LinkedList<RefPt
     mPendingCount--;
     rec.forget(aResult);
     (*aResult)->onQueue = false;
 }
 
 bool
 nsHostResolver::GetHostToLookup(nsHostRecord **result)
 {
+    bool timedOut = false;
+    TimeDuration timeout;
+    TimeStamp epoch, now;
+
     MutexAutoLock lock(mLock);
 
+    timeout = (mNumIdleTasks >= HighThreadThreshold) ? mShortIdleTimeout : mLongIdleTimeout;
+    epoch = TimeStamp::Now();
+
+    while (!mShutdown) {
+        // remove next record from Q; hand over owning reference. Check high, then med, then low
+
 #define SET_GET_TTL(var, val) (var)->mGetTtl = sGetTtlEnabled && (val)
 
-    if (!mHighQ.isEmpty()) {
-        DeQueue(mHighQ, result);
-        SET_GET_TTL(*result, false);
-        return true;
-    }
-
-    if (mActiveAnyThreadCount < HighThreadThreshold) {
-        if (!mMediumQ.isEmpty()) {
-            DeQueue(mMediumQ, result);
-            mActiveAnyThreadCount++;
-            (*result)->usingAnyThread = true;
-            SET_GET_TTL(*result, true);
+        if (!mHighQ.isEmpty()) {
+            DeQueue (mHighQ, result);
+            SET_GET_TTL(*result, false);
             return true;
         }
 
-        if (!mLowQ.isEmpty()) {
-            DeQueue(mLowQ, result);
-            mActiveAnyThreadCount++;
-            (*result)->usingAnyThread = true;
-            SET_GET_TTL(*result, true);
-            return true;
+        if (mActiveAnyThreadCount < HighThreadThreshold) {
+            if (!mMediumQ.isEmpty()) {
+                DeQueue (mMediumQ, result);
+                mActiveAnyThreadCount++;
+                (*result)->usingAnyThread = true;
+                SET_GET_TTL(*result, true);
+                return true;
+            }
+
+            if (!mLowQ.isEmpty()) {
+                DeQueue (mLowQ, result);
+                mActiveAnyThreadCount++;
+                (*result)->usingAnyThread = true;
+                SET_GET_TTL(*result, true);
+                return true;
+            }
+        }
+
+        // Determining timeout is racy, so allow one cycle through checking the queues
+        // before exiting.
+        if (timedOut)
+            break;
+
+        // wait for one or more of the following to occur:
+        //  (1) the pending queue has a host record to process
+        //  (2) the shutdown flag has been set
+        //  (3) the thread has been idle for too long
+
+        mNumIdleTasks++;
+        mIdleTaskCV.Wait(timeout);
+        mNumIdleTasks--;
+
+        now = TimeStamp::Now();
+
+        if (now - epoch >= timeout) {
+            timedOut = true;
+        } else {
+            // It is possible that CondVar::Wait() was interrupted and returned
+            // early, in which case we will loop back and re-enter it. In that
+            // case we want to do so with the new timeout reduced to reflect
+            // time already spent waiting.
+            timeout -= now - epoch;
+            epoch = now;
         }
     }
 
+    // tell thread to exit...
     return false;
 }
 
 void
 nsHostResolver::PrepareRecordExpiration(nsHostRecord* rec) const
 {
     // NOTE: rec->addr_info_lock is already held by parent
     MOZ_ASSERT(((bool)rec->addr_info) != rec->negative);
@@ -1751,33 +1821,37 @@ nsHostResolver::SizeOfIncludingThis(Mall
     // - mHighQ, mMediumQ, mLowQ, mEvictionQ, because they just point to
     //   nsHostRecords that also pointed to by entries |mRecordDB|, and
     //   measured when |mRecordDB| is measured.
 
     return n;
 }
 
 void
-nsHostResolver::ResolveHostTask()
+nsHostResolver::ThreadFunc()
 {
-    mActiveTaskCount++;
     LOG(("DNS lookup thread - starting execution.\n"));
 
 #if defined(RES_RETRY_ON_FAILURE)
     nsResState rs;
 #endif
     RefPtr<nsHostRecord> rec;
     AddrInfo *ai = nullptr;
 
-    if (!GetHostToLookup(getter_AddRefs(rec))) {
-        NS_WARNING("Could not find any host to resolve");
-        return;
-    }
+    do {
+        if (!rec) {
+            RefPtr<nsHostRecord> tmpRec;
+            if (!GetHostToLookup(getter_AddRefs(tmpRec))) {
+                break; // thread shutdown signal
+            }
+            // GetHostToLookup() returns an owning reference
+            MOZ_ASSERT(tmpRec);
+            rec.swap(tmpRec);
+        }
 
-    do {
         LOG(("DNS lookup thread - Calling getaddrinfo for host [%s].\n",
              rec->host.get()));
 
         TimeStamp startTime = TimeStamp::Now();
         bool getTtl = rec->mGetTtl;
         TimeDuration inQueue = startTime - rec->mNativeStart;
         uint32_t ms = static_cast<uint32_t>(inQueue.ToMilliseconds());
         Telemetry::Accumulate(Telemetry::DNS_NATIVE_QUEUING, ms);
@@ -1822,17 +1896,17 @@ nsHostResolver::ResolveHostTask()
              ai ? "success" : "failure: unknown host"));
 
         if (LOOKUP_RESOLVEAGAIN == CompleteLookup(rec, status, ai, rec->pb)) {
             // leave 'rec' assigned and loop to make a renewed host resolve
             LOG(("DNS lookup thread - Re-resolving host [%s].\n", rec->host.get()));
         } else {
             rec = nullptr;
         }
-    } while(rec);
+    } while(true);
 
     mActiveTaskCount--;
     LOG(("DNS lookup thread - queue empty, task finished.\n"));
 }
 
 void
 nsHostResolver::SetCacheLimits(uint32_t aMaxCacheEntries,
                                uint32_t aDefaultCacheEntryLifetime,
--- a/netwerk/dns/nsHostResolver.h
+++ b/netwerk/dns/nsHostResolver.h
@@ -416,52 +416,55 @@ private:
     bool     GetHostToLookup(nsHostRecord **m);
 
     // Removes the first element from the list and returns it AddRef-ed in aResult
     // Should not be called for an empty linked list.
     void     DeQueue(mozilla::LinkedList<RefPtr<nsHostRecord>>& aQ, nsHostRecord **aResult);
     // Cancels host records in the pending queue and also
     // calls CompleteLookup with the NS_ERROR_ABORT result code.
     void     ClearPendingQueue(mozilla::LinkedList<RefPtr<nsHostRecord>>& aPendingQ);
+    nsresult ConditionallyCreateThread(nsHostRecord *rec);
 
     /**
      * Starts a new lookup in the background for entries that are in the grace
      * period with a failed connect or all cached entries are negative.
      */
     nsresult ConditionallyRefreshRecord(nsHostRecord *rec, const nsACString &host);
 
-    void ResolveHostTask();
+    void ThreadFunc();
 
     enum {
         METHOD_HIT = 1,
         METHOD_RENEWAL = 2,
         METHOD_NEGATIVE_HIT = 3,
         METHOD_LITERAL = 4,
         METHOD_OVERFLOW = 5,
         METHOD_NETWORK_FIRST = 6,
         METHOD_NETWORK_SHARED = 7
     };
 
     uint32_t      mMaxCacheEntries;
     uint32_t      mDefaultCacheLifetime; // granularity seconds
     uint32_t      mDefaultGracePeriod; // granularity seconds
     mutable Mutex mLock;    // mutable so SizeOfIncludingThis can be const
+    CondVar       mIdleTaskCV;
     nsRefPtrHashtable<nsGenericHashKey<nsHostKey>, nsHostRecord> mRecordDB;
     mozilla::LinkedList<RefPtr<nsHostRecord>> mHighQ;
     mozilla::LinkedList<RefPtr<nsHostRecord>> mMediumQ;
     mozilla::LinkedList<RefPtr<nsHostRecord>> mLowQ;
     mozilla::LinkedList<RefPtr<nsHostRecord>> mEvictionQ;
     uint32_t      mEvictionQSize;
     PRTime        mCreationTime;
     mozilla::TimeDuration mLongIdleTimeout;
     mozilla::TimeDuration mShortIdleTimeout;
 
     RefPtr<nsIThreadPool> mResolverThreads;
 
     mozilla::Atomic<bool>     mShutdown;
+    mozilla::Atomic<uint32_t> mNumIdleTasks;
     mozilla::Atomic<uint32_t> mActiveTaskCount;
     mozilla::Atomic<uint32_t> mActiveAnyThreadCount;
     mozilla::Atomic<uint32_t> mPendingCount;
 
     // Set the expiration time stamps appropriately.
     void PrepareRecordExpiration(nsHostRecord* rec) const;
 
 public: