Bug 1077084 - improved DNS cache pruning. r=sworkman
authorDaniel Stenberg <daniel@haxx.se>
Mon, 13 Oct 2014 23:07:00 +0200
changeset 210331 073e83cef2381484ade504b58df6f845b796ed84
parent 210330 7a05af7d4902a2d83fdf2be43928290b5a567970
child 210332 441c734a3643e0a25c4ac2a1f85df8d2f617b9eb
push id27651
push userkwierso@gmail.com
push dateWed, 15 Oct 2014 00:18:02 +0000
treeherdermozilla-central@62f0b771583c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssworkman
bugs1077084
milestone36.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1077084 - improved DNS cache pruning. r=sworkman
netwerk/dns/nsHostResolver.cpp
netwerk/dns/nsHostResolver.h
--- a/netwerk/dns/nsHostResolver.cpp
+++ b/netwerk/dns/nsHostResolver.cpp
@@ -459,29 +459,34 @@ nsHostRecord::GetPriority(uint16_t aFlag
         return nsHostRecord::DNS_PRIORITY_HIGH;
     } else if (IsMediumPriority(aFlags)) {
         return nsHostRecord::DNS_PRIORITY_MEDIUM;
     }
 
     return nsHostRecord::DNS_PRIORITY_LOW;
 }
 
-// Returns true if the entry can be removed, or false if it was marked to get
-// refreshed.
+// Returns true if the entry can be removed, or false if it should be left.
+// Sets mResolveAgain true for entries being resolved right now.
 bool
 nsHostRecord::RemoveOrRefresh()
 {
-  // This condition implies that the request has been passed to the OS
-  // resolver. The resultant DNS record should be considered stale and not
-  // trusted; set a flag to ensure it is called again.
-    if (resolving && !onQueue) {
-        mResolveAgain = true;
+    if (resolving) {
+        if (!onQueue) {
+            // The request has been passed to the OS resolver. The resultant DNS
+            // record should be considered stale and not trusted; set a flag to
+            // ensure it is called again.
+            mResolveAgain = true;
+        }
+        // if Onqueue is true, the host entry is already added to the cache
+        // but is still pending to get resolved: just leave it in hash.
         return false;
     }
-    return true; // can be removed now
+    // Already resolved; not in a pending state; remove from cache.
+    return true;
 }
 
 //----------------------------------------------------------------------------
 
 struct nsHostDBEnt : PLDHashEntryHdr
 {
     nsHostRecord *rec;
 };
@@ -594,19 +599,19 @@ HostDB_RemoveEntry(PLDHashTable *table,
 
 static PLDHashOperator
 HostDB_PruneEntry(PLDHashTable *table,
                   PLDHashEntryHdr *hdr,
                   uint32_t number,
                   void *arg)
 {
     nsHostDBEnt* ent = static_cast<nsHostDBEnt *>(hdr);
-
     // Try to remove the record, or mark it for refresh
     if (ent->rec->RemoveOrRefresh()) {
+        PR_REMOVE_LINK(ent->rec);
         return PL_DHASH_REMOVE;
     }
     return PL_DHASH_NEXT;
 }
 
 //----------------------------------------------------------------------------
 
 #if TTL_AVAILABLE
@@ -781,36 +786,34 @@ nsHostResolver::ClearPendingQueue(PRCLis
 // This function removes all existing resolved host entries from the hash.
 // Names that are in the pending queues can be left there. Entries in the
 // cache that have 'Resolve' set true but not 'onQueue' are being resolved
 // right now, so we need to mark them to get re-resolved on completion!
 
 void
 nsHostResolver::FlushCache()
 {
-    PRCList evictionQ;
-    PR_INIT_CLIST(&evictionQ);
-
-    {
-        MutexAutoLock lock(mLock);
-        MoveCList(mEvictionQ, evictionQ);
-        mEvictionQSize = 0;
+  MutexAutoLock lock(mLock);
+  mEvictionQSize = 0;
 
-        // prune the hash from all hosts already resolved
-        PL_DHashTableEnumerate(&mDB, HostDB_PruneEntry, nullptr);
-    }
+  // Clear the evictionQ and remove all its corresponding entries from
+  // the cache first
+  if (!PR_CLIST_IS_EMPTY(&mEvictionQ)) {
+      PRCList *node = mEvictionQ.next;
+      while (node != &mEvictionQ) {
+          nsHostRecord *rec = static_cast<nsHostRecord *>(node);
+          node = node->next;
+          PR_REMOVE_AND_INIT_LINK(rec);
+          PL_DHashTableOperate(&mDB, (nsHostKey *) rec, PL_DHASH_REMOVE);
+          NS_RELEASE(rec);
+      }
+  }
 
-    if (!PR_CLIST_IS_EMPTY(&evictionQ)) {
-        PRCList *node = evictionQ.next;
-        while (node != &evictionQ) {
-            nsHostRecord *rec = static_cast<nsHostRecord *>(node);
-            node = node->next;
-            NS_RELEASE(rec);
-        }
-    }
+  // Refresh the cache entries that are resolving RIGHT now, remove the rest.
+  PL_DHashTableEnumerate(&mDB, HostDB_PruneEntry, nullptr);
 }
 
 void
 nsHostResolver::Shutdown()
 {
     LOG(("Shutting down host resolver.\n"));
 
 #if TTL_AVAILABLE
@@ -1454,47 +1457,43 @@ nsHostResolver::OnLookupComplete(nsHostR
                     TimeDuration age = TimeStamp::NowLoRes() - head->mValidStart;
                     Telemetry::Accumulate(Telemetry::DNS_CLEANUP_AGE,
                                           static_cast<uint32_t>(age.ToSeconds() / 60));
                 }
 
                 // release reference to rec owned by mEvictionQ
                 NS_RELEASE(head);
             }
+#if TTL_AVAILABLE
+            if (!rec->mGetTtl && sDnsVariant != DNS_EXP_VARIANT_CONTROL
+                && !rec->resolving) {
+                LOG(("Issuing second async lookup for TTL for %s.", rec->host));
+                rec->flags =
+                  (rec->flags & ~RES_PRIORITY_MEDIUM) | RES_PRIORITY_LOW;
+                DebugOnly<nsresult> rv = IssueLookup(rec);
+                NS_WARN_IF_FALSE(NS_SUCCEEDED(rv),
+                                 "Could not issue second async lookup for TTL.");
+            }
+#endif
         }
     }
 
     MOZ_EVENT_TRACER_DONE(rec, "net::dns::resolve");
 
     if (!PR_CLIST_IS_EMPTY(&cbs)) {
         PRCList *node = cbs.next;
         while (node != &cbs) {
             nsResolveHostCallback *callback =
                     static_cast<nsResolveHostCallback *>(node);
             node = node->next;
             callback->OnLookupComplete(this, rec, status);
             // NOTE: callback must not be dereferenced after this point!!
         }
     }
 
-#if TTL_AVAILABLE
-    {
-        MutexAutoLock lock(mLock);
-        if (!mShutdown && !rec->mGetTtl
-                && sDnsVariant != DNS_EXP_VARIANT_CONTROL && !rec->resolving) {
-            LOG(("Issuing second async lookup for TTL for %s.", rec->host));
-            rec->flags =
-                (rec->flags & ~RES_PRIORITY_MEDIUM) | RES_PRIORITY_LOW;
-            DebugOnly<nsresult> rv = IssueLookup(rec);
-            NS_WARN_IF_FALSE(NS_SUCCEEDED(rv),
-                             "Could not issue second async lookup for TTL.");
-        }
-    }
-#endif
-
     NS_RELEASE(rec);
 
     return LOOKUP_OK;
 }
 
 void
 nsHostResolver::CancelAsyncRequest(const char            *host,
                                    uint16_t               flags,
--- a/netwerk/dns/nsHostResolver.h
+++ b/netwerk/dns/nsHostResolver.h
@@ -114,18 +114,18 @@ public:
 
     enum DnsPriority {
         DNS_PRIORITY_LOW,
         DNS_PRIORITY_MEDIUM,
         DNS_PRIORITY_HIGH,
     };
     static DnsPriority GetPriority(uint16_t aFlags);
 
-    bool RemoveOrRefresh(); // Returns whether the host record can be removed
-                            // or needs to be refreshed
+    bool RemoveOrRefresh(); // Mark records currently being resolved as needed
+                            // to resolve again.
 
 private:
     friend class nsHostResolver;
 
 
     PRCList callbacks; /* list of callbacks */
 
     bool    resolving; /* true if this record is being resolved, which means