Bug 1377223 - RCWN - Should we revalidate when racing and the cache wins, r=valentin, data-r=bsmedberg
authorMichal Novotny <michal.novotny@gmail.com>
Mon, 24 Jul 2017 20:35:36 +0200
changeset 419457 69e573abaccb3eba8446c669328f133b7c503a48
parent 419456 2f8b9d3feecbe55eb6faf5f419870aca27c8023d
child 419458 c2ebc796b97b5903fb55e449419d03d585c99aec
push id7566
push usermtabara@mozilla.com
push dateWed, 02 Aug 2017 08:25:16 +0000
treeherdermozilla-beta@86913f512c3c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersvalentin
bugs1377223
milestone56.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 1377223 - RCWN - Should we revalidate when racing and the cache wins, r=valentin, data-r=bsmedberg This patch adds telemetry probes to find out how often the cache wins the race but the entry cannot be used because it needs to be revalidated and we cannot send a conditional request.
netwerk/protocol/http/nsHttpChannel.cpp
toolkit/components/telemetry/Histograms.json
--- a/netwerk/protocol/http/nsHttpChannel.cpp
+++ b/netwerk/protocol/http/nsHttpChannel.cpp
@@ -525,21 +525,28 @@ nsHttpChannel::Connect()
             if (!mFallbackChannel && !mFallbackKey.IsEmpty()) {
                 return AsyncCall(&nsHttpChannel::HandleAsyncFallback);
             }
             return NS_ERROR_DOCUMENT_NOT_CACHED;
         }
         // otherwise, let's just proceed without using the cache.
     }
 
+    if (mRaceCacheWithNetwork && mCacheEntry && !mCachedContentIsValid &&
+        (mDidReval || mCachedContentIsPartial)) {
+        // We won't send the conditional request because the unconditional
+        // request was already sent (see bug 1377223).
+        AccumulateCategorical(Telemetry::LABELS_NETWORK_RACE_CACHE_VALIDATION::NotSent);
+    }
+
     // When racing, if OnCacheEntryAvailable is called before AsyncOpenURI
     // returns, then we may not have started reading from the cache.
     // If the content is valid, we should attempt to do so, as technically the
     // cache has won the race.
-    if (sRCWNEnabled && mCachedContentIsValid && mNetworkTriggered) {
+    if (mRaceCacheWithNetwork && mCachedContentIsValid) {
         Unused << ReadFromCache(true);
     }
 
     return TriggerNetwork(0);
 }
 
 nsresult
 nsHttpChannel::TryHSTSPriming()
@@ -2397,16 +2404,17 @@ nsHttpChannel::ContinueProcessResponse2(
         return NS_OK;
     }
 
     rv = NS_OK;
 
     uint32_t httpStatus = mResponseHead->Status();
 
     bool successfulReval = false;
+    bool partialContentUsed = false;
 
     // handle different server response categories.  Note that we handle
     // caching or not caching of error pages in
     // nsHttpResponseHead::MustValidate; if you change this switch, update that
     // one
     switch (httpStatus) {
     case 200:
     case 203:
@@ -2420,19 +2428,22 @@ nsHttpChannel::ContinueProcessResponse2(
             rv = CallOnStartRequest();
             break;
         }
         // these can normally be cached
         rv = ProcessNormal();
         MaybeInvalidateCacheEntryForSubsequentGet();
         break;
     case 206:
-        if (mCachedContentIsPartial) // an internal byte range request...
+        if (mCachedContentIsPartial) { // an internal byte range request...
             rv = ProcessPartialContent();
-        else {
+            if (NS_SUCCEEDED(rv)) {
+                partialContentUsed = true;
+            }
+        } else {
             mCacheInputStream.CloseAndRelease();
             rv = ProcessNormal();
         }
         break;
     case 300:
     case 301:
     case 302:
     case 307:
@@ -2540,16 +2551,26 @@ nsHttpChannel::ContinueProcessResponse2(
         }
         break;
     default:
         rv = ProcessNormal();
         MaybeInvalidateCacheEntryForSubsequentGet();
         break;
     }
 
+    if (mRaceDelay && !mRaceCacheWithNetwork &&
+        (mCachedContentIsPartial || mDidReval)) {
+        if (successfulReval || partialContentUsed) {
+            AccumulateCategorical(Telemetry::LABELS_NETWORK_RACE_CACHE_VALIDATION::CachedContentUsed);
+        } else {
+            AccumulateCategorical(Telemetry::LABELS_NETWORK_RACE_CACHE_VALIDATION::CachedContentNotUsed);
+        }
+    }
+
+
     if (gHttpHandler->IsTelemetryEnabled()) {
         CacheDisposition cacheDisposition;
         if (!mDidReval) {
             cacheDisposition = kCacheMissed;
         } else if (successfulReval) {
             cacheDisposition = kCacheHitViaReval;
         } else {
             cacheDisposition = kCacheMissedViaReval;
@@ -4525,17 +4546,24 @@ nsHttpChannel::OnCacheEntryAvailableInte
         return rv;
     }
 
     // We may be waiting for more callbacks...
     if (AwaitingCacheCallbacks()) {
         return NS_OK;
     }
 
-    if (mCachedContentIsValid && mNetworkTriggered) {
+    if (mRaceCacheWithNetwork && mCacheEntry && !mCachedContentIsValid &&
+        (mDidReval || mCachedContentIsPartial)) {
+        // We won't send the conditional request because the unconditional
+        // request was already sent (see bug 1377223).
+        AccumulateCategorical(Telemetry::LABELS_NETWORK_RACE_CACHE_VALIDATION::NotSent);
+    }
+
+    if (mRaceCacheWithNetwork && mCachedContentIsValid) {
         Unused << ReadFromCache(true);
     }
 
     return TriggerNetwork(0);
 }
 
 nsresult
 nsHttpChannel::OnNormalCacheEntryAvailable(nsICacheEntry *aEntry,
--- a/toolkit/components/telemetry/Histograms.json
+++ b/toolkit/components/telemetry/Histograms.json
@@ -2486,16 +2486,25 @@
     "kind": "exponential",
     "low": 32,
     "high": 16777216,
     "n_buckets": 100,
     "description": "Amount of bytes received when we decide not to race cache with network.",
     "alert_emails": ["necko@mozilla.com", "bsmedberg@mozilla.com"],
     "bug_numbers": [1354405]
   },
+  "NETWORK_RACE_CACHE_VALIDATION": {
+    "record_in_processes": ["main"],
+    "expires_in_version": "62",
+    "alert_emails": ["necko@mozilla.com"],
+    "bug_numbers": [1377223],
+    "kind": "categorical",
+    "labels": ["NotSent", "CachedContentUsed", "CachedContentNotUsed"],
+    "description": "Stats for validation requests when cache won the race."
+  },
   "HTTP_AUTH_DIALOG_STATS_2": {
     "record_in_processes": ["main", "content"],
     "expires_in_version": "61",
     "alert_emails": ["necko@mozilla.com"],
     "bug_numbers": [1357835],
     "kind": "enumerated",
     "n_values": 32,
     "description": "Stats about what kind of resource requested http authentication. (29=top-level doc, 30=same origin subresources, 31=same origin xhr, (nsIContentPolicy type)=cross-origin subresources per nsIContentPolicy type)"