Bug 882516 - Use AF_UNSPEC cached entries for AF_INET and AF_INET6 DNS requests, if possible. r=sworkman
authorAdrian Lungu <alungu@mozilla.com>
Fri, 16 Aug 2013 11:49:57 -0700
changeset 142925 9ab1c5795b4f4decafe03f5baad1ad2fa68b7c36
parent 142924 627e7121da84a0d4ac45f94c7fc6fee823b99164
child 142926 7c097d951a23c199d308fffcff9b8f6acadf9149
push id25116
push userphilringnalda@gmail.com
push dateSat, 17 Aug 2013 15:35:37 +0000
treeherdermozilla-central@74fe1012de43 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssworkman
bugs882516
milestone26.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 882516 - Use AF_UNSPEC cached entries for AF_INET and AF_INET6 DNS requests, if possible. r=sworkman
netwerk/dns/DNS.cpp
netwerk/dns/DNS.h
netwerk/dns/nsHostResolver.cpp
netwerk/dns/nsHostResolver.h
--- a/netwerk/dns/DNS.cpp
+++ b/netwerk/dns/DNS.cpp
@@ -166,50 +166,77 @@ bool IsIPAddrV4Mapped(const NetAddr *add
   return false;
 }
 
 NetAddrElement::NetAddrElement(const PRNetAddr *prNetAddr)
 {
   PRNetAddrToNetAddr(prNetAddr, &mAddress);
 }
 
+NetAddrElement::NetAddrElement(const NetAddrElement& netAddr)
+{
+  mAddress = netAddr.mAddress;
+}
+
 NetAddrElement::~NetAddrElement()
 {
 }
 
 AddrInfo::AddrInfo(const char *host, const PRAddrInfo *prAddrInfo,
                    bool disableIPv4, const char *cname)
 {
-  size_t hostlen = strlen(host);
-  mHostName = static_cast<char*>(moz_xmalloc(hostlen + 1));
-  memcpy(mHostName, host, hostlen + 1);
-  if (cname) {
-      size_t cnameLen = strlen(cname);
-      mCanonicalName = static_cast<char*>(moz_xmalloc(cnameLen + 1));
-      memcpy(mCanonicalName, cname, cnameLen + 1);
-  }
-  else {
-      mCanonicalName = nullptr;
-  }
+  MOZ_ASSERT(prAddrInfo, "Cannot construct AddrInfo with a null prAddrInfo pointer!");
 
+  Init(host, cname);
   PRNetAddr tmpAddr;
   void *iter = nullptr;
   do {
     iter = PR_EnumerateAddrInfo(iter, prAddrInfo, 0, &tmpAddr);
     if (iter && (!disableIPv4 || tmpAddr.raw.family != PR_AF_INET)) {
       NetAddrElement *addrElement = new NetAddrElement(&tmpAddr);
       mAddresses.insertBack(addrElement);
     }
   } while (iter);
 }
 
+AddrInfo::AddrInfo(const char *host, const char *cname)
+{
+  Init(host, cname);
+}
+
 AddrInfo::~AddrInfo()
 {
   NetAddrElement *addrElement;
   while ((addrElement = mAddresses.popLast())) {
     delete addrElement;
   }
   moz_free(mHostName);
   moz_free(mCanonicalName);
 }
 
+void
+AddrInfo::Init(const char *host, const char *cname)
+{
+  MOZ_ASSERT(host, "Cannot initialize AddrInfo with a null host pointer!");
+
+  size_t hostlen = strlen(host);
+  mHostName = static_cast<char*>(moz_xmalloc(hostlen + 1));
+  memcpy(mHostName, host, hostlen + 1);
+  if (cname) {
+    size_t cnameLen = strlen(cname);
+    mCanonicalName = static_cast<char*>(moz_xmalloc(cnameLen + 1));
+    memcpy(mCanonicalName, cname, cnameLen + 1);
+  }
+  else {
+    mCanonicalName = nullptr;
+  }
+}
+
+void
+AddrInfo::AddAddress(NetAddrElement *address)
+{
+  MOZ_ASSERT(address, "Cannot add the address to an uninitialized list");
+
+  mAddresses.insertBack(address);
+}
+
 } // namespace dns
 } // namespace mozilla
--- a/netwerk/dns/DNS.h
+++ b/netwerk/dns/DNS.h
@@ -109,30 +109,41 @@ union NetAddr {
 };
 
 // This class wraps a NetAddr union to provide C++ linked list
 // capabilities and other methods. It is created from a PRNetAddr,
 // which is converted to a mozilla::dns::NetAddr.
 class NetAddrElement : public LinkedListElement<NetAddrElement> {
 public:
   NetAddrElement(const PRNetAddr *prNetAddr);
+  NetAddrElement(const NetAddrElement& netAddr);
   ~NetAddrElement();
 
   NetAddr mAddress;
 };
 
 class AddrInfo {
 public:
+  // Creates an AddrInfo object. It calls the AddrInfo(const char*, const char*)
+  // to initialize the host and the cname.
   AddrInfo(const char *host, const PRAddrInfo *prAddrInfo, bool disableIPv4,
            const char *cname);
+
+  // Creates a basic AddrInfo object (initialize only the host and the cname).
+  AddrInfo(const char *host, const char *cname);
   ~AddrInfo();
 
+  void AddAddress(NetAddrElement *address);
+
   char *mHostName;
   char *mCanonicalName;
   LinkedList<NetAddrElement> mAddresses;
+
+private:
+  void Init(const char *host, const char *cname);
 };
 
 // Copies the contents of a PRNetAddr to a NetAddr.
 // Does not do a ptr safety check!
 void PRNetAddrToNetAddr(const PRNetAddr *prAddr, NetAddr *addr);
 
 // Copies the contents of a NetAddr to a PRNetAddr.
 // Does not do a ptr safety check!
--- a/netwerk/dns/nsHostResolver.cpp
+++ b/netwerk/dns/nsHostResolver.cpp
@@ -559,32 +559,19 @@ nsHostResolver::ResolveHost(const char  
                      he->rec->HasUsableResult(flags) &&
                      TimeStamp::NowLoRes() <= (he->rec->expiration + TimeDuration::FromSeconds(mGracePeriod * 60))) {
                 LOG(("Using cached record for host [%s].\n", host));
                 // put reference to host record on stack...
                 result = he->rec;
                 Telemetry::Accumulate(Telemetry::DNS_LOOKUP_METHOD2, METHOD_HIT);
 
                 // For entries that are in the grace period with a failed connect,
-                // or all cached negative entries, use the cache but start a new lookup in
-                // the background
-                if ((((TimeStamp::NowLoRes() > he->rec->expiration) &&
-                      he->rec->mBlacklistedItems.Length()) ||
-                     he->rec->negative) && !he->rec->resolving) {
-                    LOG(("Using %s cache entry for host [%s] but starting async renewal.",
-                         he->rec->negative ? "negative" :"positive", host));
-                    IssueLookup(he->rec);
-
-                    if (!he->rec->negative) {
-                        // negative entries are constantly being refreshed, only
-                        // track positive grace period induced renewals
-                        Telemetry::Accumulate(Telemetry::DNS_LOOKUP_METHOD2,
-                                              METHOD_RENEWAL);
-                    }
-                }
+                // or all cached negative entries, use the cache but start a new
+                // lookup in the background
+                ConditionallyRefreshRecord(he->rec, host);
                 
                 if (he->rec->negative) {
                     Telemetry::Accumulate(Telemetry::DNS_LOOKUP_METHOD2,
                                           METHOD_NEGATIVE_HIT);
                     status = NS_ERROR_UNKNOWN_HOST;
                 }
             }
             // if the host name is an IP address literal and has been parsed,
@@ -614,45 +601,115 @@ nsHostResolver::ResolveHost(const char  
                                       METHOD_OVERFLOW);
                 // This is a lower priority request and we are swamped, so refuse it.
                 rv = NS_ERROR_DNS_LOOKUP_QUEUE_FULL;
             }
             else if (flags & RES_OFFLINE) {
                 rv = NS_ERROR_OFFLINE;
             }
 
-            // otherwise, hit the resolver...
-            else {
-                // Add callback to the list of pending callbacks.
-                PR_APPEND_LINK(callback, &he->rec->callbacks);
+            // If this is an IPV4 or IPV6 specific request, check if there is
+            // an AF_UNSPEC entry we can use. Otherwise, hit the resolver...
+            else if (!he->rec->resolving) {
+                if (!(flags & RES_BYPASS_CACHE) &&
+                    ((af == PR_AF_INET) || (af == PR_AF_INET6))) {
+                    // First, search for an entry with AF_UNSPEC
+                    const nsHostKey unspecKey = { host, flags, PR_AF_UNSPEC };
+                    nsHostDBEnt *unspecHe = static_cast<nsHostDBEnt *>
+                        (PL_DHashTableOperate(&mDB, &unspecKey, PL_DHASH_LOOKUP));
+                    NS_ASSERTION(PL_DHASH_ENTRY_IS_FREE(unspecHe) ||
+                                 (PL_DHASH_ENTRY_IS_BUSY(unspecHe) &&
+                                  unspecHe->rec),
+                                "Valid host entries should contain a record");
+                    if (PL_DHASH_ENTRY_IS_BUSY(unspecHe) &&
+                        unspecHe->rec &&
+                        unspecHe->rec->HasUsableResult(flags) &&
+                        TimeStamp::NowLoRes() <= (he->rec->expiration +
+                            TimeDuration::FromSeconds(mGracePeriod * 60))) {
+                        LOG(("Specific DNS request (%s) for an unspecified "
+                             "cached record",
+                            (af == PR_AF_INET) ? "AF_INET" : "AF_INET6"));
 
-                if (!he->rec->resolving) {
+                        // Search for any valid address in the AF_UNSPEC entry
+                        // in the cache (not blacklisted and from the right
+                        // family).
+                        NetAddrElement *addrIter =
+                            unspecHe->rec->addr_info->mAddresses.getFirst();
+                        he->rec->addr_info = nullptr;
+                        while (addrIter) {
+                            if ((af == addrIter->mAddress.inet.family) &&
+                                 !unspecHe->rec->Blacklisted(&addrIter->mAddress)) {
+                                if (!he->rec->addr_info) {
+                                    he->rec->addr_info = new AddrInfo(
+                                        unspecHe->rec->addr_info->mHostName,
+                                        unspecHe->rec->addr_info->mCanonicalName);
+                                }
+                                he->rec->addr_info->AddAddress(
+                                    new NetAddrElement(*addrIter));
+                            }
+                            addrIter = addrIter->getNext();
+                        }
+                        if (he->rec->HasUsableResult(flags)) {
+                            result = he->rec;
+                            Telemetry::Accumulate(Telemetry::DNS_LOOKUP_METHOD2,
+                                                  METHOD_HIT);
+                            ConditionallyRefreshRecord(he->rec, host);
+                        }
+                        // For AF_INET6, a new lookup means another AF_UNSPEC
+                        // lookup. We have already iterated through the
+                        // AF_UNSPEC addresses, so we mark this record as
+                        // negative.
+                        else if (af == PR_AF_INET6) {
+                            result = he->rec;
+                            he->rec->negative = true;
+                            status = NS_ERROR_UNKNOWN_HOST;
+                            Telemetry::Accumulate(Telemetry::DNS_LOOKUP_METHOD2,
+                                                  METHOD_NEGATIVE_HIT);
+                        }
+                    }
+                }
+                // If no valid address was found in the cache or this is an
+                // AF_UNSPEC request, then start a new lookup.
+                if (!result) {
+                    LOG(("No valid address was found in the cache for the "
+                         "requested IP family"));
+                    // Add callback to the list of pending callbacks.
+                    PR_APPEND_LINK(callback, &he->rec->callbacks);
                     he->rec->flags = flags;
-                    rv = IssueLookup(he->rec);
+                    IssueLookup(he->rec);
                     Telemetry::Accumulate(Telemetry::DNS_LOOKUP_METHOD2,
                                           METHOD_NETWORK_FIRST);
-                    if (NS_FAILED(rv))
+                    if (NS_FAILED(rv)) {
                         PR_REMOVE_AND_INIT_LINK(callback);
-                    else
-                        LOG(("DNS lookup for host [%s] blocking pending 'getaddrinfo' query.", host));
+                    }
+                    else {
+                        LOG(("DNS lookup for host [%s] blocking pending "
+                             "'getaddrinfo' query.", host));
+                    }
                 }
-                else if (he->rec->onQueue) {
+            }
+            else {
+                // The record is being resolved. Append our callback.
+                PR_APPEND_LINK(callback, &he->rec->callbacks);
+                if (he->rec->onQueue) {
                     Telemetry::Accumulate(Telemetry::DNS_LOOKUP_METHOD2,
                                           METHOD_NETWORK_SHARED);
 
-                    // Consider the case where we are on a pending queue of 
+                    // Consider the case where we are on a pending queue of
                     // lower priority than the request is being made at.
                     // In that case we should upgrade to the higher queue.
 
-                    if (IsHighPriority(flags) && !IsHighPriority(he->rec->flags)) {
+                    if (IsHighPriority(flags) &&
+                        !IsHighPriority(he->rec->flags)) {
                         // Move from (low|med) to high.
                         MoveQueue(he->rec, mHighQ);
                         he->rec->flags = flags;
                         ConditionallyCreateThread(he->rec);
-                    } else if (IsMediumPriority(flags) && IsLowPriority(he->rec->flags)) {
+                    } else if (IsMediumPriority(flags) &&
+                               IsLowPriority(he->rec->flags)) {
                         // Move from low to med.
                         MoveQueue(he->rec, mMediumQ);
                         he->rec->flags = flags;
                         mIdleThreadCV.Notify();
                     }
                 }
             }
         }
@@ -765,16 +822,36 @@ nsHostResolver::IssueLookup(nsHostRecord
           mThreadCount,
           mActiveAnyThreadCount,
           mNumIdleThreads,
           mPendingCount));
 
     return rv;
 }
 
+nsresult
+nsHostResolver::ConditionallyRefreshRecord(nsHostRecord *rec, const char *host)
+{
+    if ((((TimeStamp::NowLoRes() > rec->expiration) &&
+        rec->mBlacklistedItems.Length()) ||
+        rec->negative) && !rec->resolving) {
+        LOG(("Using %s cache entry for host [%s] but starting async renewal.",
+            rec->negative ? "negative" :"positive", host));
+        IssueLookup(rec);
+
+        if (!rec->negative) {
+            // negative entries are constantly being refreshed, only
+            // track positive grace period induced renewals
+            Telemetry::Accumulate(Telemetry::DNS_LOOKUP_METHOD2,
+                METHOD_RENEWAL);
+        }
+    }
+    return NS_OK;
+}
+
 void
 nsHostResolver::DeQueue(PRCList &aQ, nsHostRecord **aResult)
 {
     *aResult = static_cast<nsHostRecord *>(aQ.next);
     PR_REMOVE_AND_INIT_LINK(*aResult);
     mPendingCount--;
     (*aResult)->onQueue = false;
 }
--- a/netwerk/dns/nsHostResolver.h
+++ b/netwerk/dns/nsHostResolver.h
@@ -239,16 +239,22 @@ private:
 
     nsresult Init();
     nsresult IssueLookup(nsHostRecord *);
     bool     GetHostToLookup(nsHostRecord **m);
     void     OnLookupComplete(nsHostRecord *, nsresult, mozilla::net::AddrInfo *);
     void     DeQueue(PRCList &aQ, nsHostRecord **aResult);
     void     ClearPendingQueue(PRCList *aPendingQueue);
     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 char *host);
     
     static void  MoveQueue(nsHostRecord *aRec, PRCList &aDestQ);
     
     static void ThreadFunc(void *);
 
     enum {
         METHOD_HIT = 1,
         METHOD_RENEWAL = 2,