Bug 1077084 - Improve DNS cache pruning. r=sworkman, a=sledru
authorDaniel Stenberg <daniel@haxx.se>
Mon, 13 Oct 2014 23:07:00 +0200
changeset 225610 e3652317f6f1ad90708ec455e872e095d974d272
parent 225609 da7bdb2b6ee32a2356e65fb159b773167513de8a
child 225611 58d1381493ffba3d5d19c2183beebdcce2f079fb
push id7130
push userryanvm@gmail.com
push dateMon, 20 Oct 2014 17:19:48 +0000
treeherdermozilla-aurora@8949cb909baf [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssworkman, sledru
bugs1077084
milestone35.0a2
Bug 1077084 - Improve DNS cache pruning. r=sworkman, a=sledru
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