Bug 1409542 part 1 - telemetry on why a resource is marked prefetchable or not. r=francois,valentin
authorNicholas Hurley <hurley@mozilla.com>
Tue, 17 Oct 2017 14:46:56 -0700
changeset 388013 a120a00b3f2e559418668798ed9f0f895ab19a6f
parent 388012 e036a5afa8548d84ca07871f2b3c411d8dac8ef6
child 388014 7935635f721f66fa9d4533f9f7008c701211283b
push id32739
push useracraciun@mozilla.com
push dateWed, 25 Oct 2017 09:29:21 +0000
treeherdermozilla-central@252a8528c5ab [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfrancois, valentin
bugs1409542
milestone58.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 1409542 part 1 - telemetry on why a resource is marked prefetchable or not. r=francois,valentin MozReview-Commit-ID: IdSyFv8RSbY
netwerk/base/Predictor.cpp
netwerk/base/Predictor.h
toolkit/components/telemetry/Histograms.json
--- a/netwerk/base/Predictor.cpp
+++ b/netwerk/base/Predictor.cpp
@@ -46,16 +46,18 @@
 #include "LoadContextInfo.h"
 #include "mozilla/ipc/URIUtils.h"
 #include "SerializedLoadContext.h"
 #include "mozilla/net/NeckoChild.h"
 
 #include "mozilla/dom/ContentParent.h"
 #include "mozilla/ClearOnShutdown.h"
 
+#include "CacheControlParser.h"
+
 using namespace mozilla;
 
 namespace mozilla {
 namespace net {
 
 Predictor *Predictor::sSelf = nullptr;
 
 static LazyLogModule gPredictorLog("NetworkPredictor");
@@ -2604,50 +2606,53 @@ Predictor::UpdateCacheability(nsIURI *so
     PREDICTOR_LOG(("Predictor::UpdateCacheability non-http(s) uri"));
     return;
   }
 
   RefPtr<Predictor> self = sSelf;
   if (self) {
     nsAutoCString method;
     requestHead.Method(method);
+
     nsAutoCString vary;
     Unused << responseHead->GetHeader(nsHttp::Vary, vary);
+
+    nsAutoCString cacheControlHeader;
+    Unused << responseHead->GetHeader(nsHttp::Cache_Control, cacheControlHeader);
+    CacheControlParser cacheControl(cacheControlHeader);
+
     self->UpdateCacheabilityInternal(sourceURI, targetURI, httpStatus,
                                      method, *lci->OriginAttributesPtr(),
-                                     isTracking, !vary.IsEmpty());
+                                     isTracking, !vary.IsEmpty(),
+                                     cacheControl.NoStore());
   }
 }
 
 void
 Predictor::UpdateCacheabilityInternal(nsIURI *sourceURI, nsIURI *targetURI,
                                       uint32_t httpStatus,
                                       const nsCString &method,
                                       const OriginAttributes& originAttributes,
-                                      bool isTracking, bool couldVary)
+                                      bool isTracking, bool couldVary,
+                                      bool isNoStore)
 {
   PREDICTOR_LOG(("Predictor::UpdateCacheability httpStatus=%u", httpStatus));
 
   nsresult rv;
 
   if (!mInitialized) {
     PREDICTOR_LOG(("    not initialized"));
     return;
   }
 
   if (!mEnabled) {
     PREDICTOR_LOG(("    not enabled"));
     return;
   }
 
-  if (!mEnablePrefetch) {
-    PREDICTOR_LOG(("    prefetch not enabled"));
-    return;
-  }
-
   nsCOMPtr<nsICacheStorage> cacheDiskStorage;
 
   RefPtr<LoadContextInfo> lci =
     new LoadContextInfo(false, originAttributes);
 
   rv = mCacheStorageService->DiskCacheStorage(lci, false,
                                              getter_AddRefs(cacheDiskStorage));
   if (NS_FAILED(rv)) {
@@ -2655,17 +2660,17 @@ Predictor::UpdateCacheabilityInternal(ns
     return;
   }
 
   uint32_t openFlags = nsICacheStorage::OPEN_READONLY |
                        nsICacheStorage::OPEN_SECRETLY |
                        nsICacheStorage::CHECK_MULTITHREADED;
   RefPtr<Predictor::CacheabilityAction> action =
     new Predictor::CacheabilityAction(targetURI, httpStatus, method, isTracking,
-                                      couldVary, this);
+                                      couldVary, isNoStore, this);
   nsAutoCString uri;
   targetURI->GetAsciiSpec(uri);
   PREDICTOR_LOG(("    uri=%s action=%p", uri.get(), action.get()));
   cacheDiskStorage->AsyncOpenURI(sourceURI, EmptyCString(), openFlags, action);
 }
 
 NS_IMPL_ISUPPORTS(Predictor::CacheabilityAction,
                   nsICacheEntryOpenCallback,
@@ -2675,16 +2680,28 @@ NS_IMETHODIMP
 Predictor::CacheabilityAction::OnCacheEntryCheck(nsICacheEntry *entry,
                                                  nsIApplicationCache *appCache,
                                                  uint32_t *result)
 {
   *result = nsICacheEntryOpenCallback::ENTRY_WANTED;
   return NS_OK;
 }
 
+namespace {
+enum PrefetchDecisionReason {
+  PREFETCHABLE,
+  STATUS_NOT_200,
+  METHOD_NOT_GET,
+  URL_HAS_QUERY_STRING,
+  RESOURCE_IS_TRACKING,
+  RESOURCE_COULD_VARY,
+  RESOURCE_IS_NO_STORE
+};
+}
+
 NS_IMETHODIMP
 Predictor::CacheabilityAction::OnCacheEntryAvailable(nsICacheEntry *entry,
                                                      bool isNew,
                                                      nsIApplicationCache *appCache,
                                                      nsresult result)
 {
   MOZ_ASSERT(NS_IsMainThread());
   // This is being opened read-only, so isNew should always be false
@@ -2730,20 +2747,44 @@ Predictor::CacheabilityAction::OnCacheEn
 
     if (!mPredictor->ParseMetaDataEntry(key, value, uri, hitCount, lastHit,
                                         flags)) {
       PREDICTOR_LOG(("    failed to parse key=%s value=%s", key, value));
       continue;
     }
 
     if (strTargetURI.Equals(uri)) {
-      if (mHttpStatus == 200 && mMethod.EqualsLiteral("GET") &&
-          !hasQueryString &&
-          !mIsTracking &&
-          !mCouldVary) {
+      bool prefetchable = true;
+      PrefetchDecisionReason reason = PREFETCHABLE;
+
+      if (mHttpStatus != 200) {
+        prefetchable = false;
+        reason = STATUS_NOT_200;
+      } else if (!mMethod.EqualsLiteral("GET")) {
+        prefetchable = false;
+        reason = METHOD_NOT_GET;
+      } else if (hasQueryString) {
+        prefetchable = false;
+        reason = URL_HAS_QUERY_STRING;
+      } else if (mIsTracking) {
+        prefetchable = false;
+        reason = RESOURCE_IS_TRACKING;
+      } else if (mCouldVary) {
+        prefetchable = false;
+        reason = RESOURCE_COULD_VARY;
+      } else if (mIsNoStore) {
+        // We don't set prefetchable = false yet, because we just want to know
+        // what kind of effect this would have on prefetching.
+        reason = RESOURCE_IS_NO_STORE;
+      }
+
+      Telemetry::Accumulate(Telemetry::PREDICTOR_PREFETCH_DECISION_REASON,
+                            reason);
+
+      if (prefetchable) {
         PREDICTOR_LOG(("    marking %s cacheable", key));
         flags |= FLAG_PREFETCHABLE;
       } else {
         PREDICTOR_LOG(("    marking %s uncacheable", key));
         flags &= ~FLAG_PREFETCHABLE;
       }
       nsCString newValue;
       MakeMetadataEntry(hitCount, lastHit, flags, newValue);
--- a/netwerk/base/Predictor.h
+++ b/netwerk/base/Predictor.h
@@ -135,33 +135,35 @@ private:
   {
   public:
     NS_DECL_THREADSAFE_ISUPPORTS
     NS_DECL_NSICACHEENTRYOPENCALLBACK
     NS_DECL_NSICACHEENTRYMETADATAVISITOR
 
     CacheabilityAction(nsIURI *targetURI, uint32_t httpStatus,
                        const nsCString &method, bool isTracking, bool couldVary,
-                       Predictor *predictor)
+                       bool isNoStore, Predictor *predictor)
       :mTargetURI(targetURI)
       ,mHttpStatus(httpStatus)
       ,mMethod(method)
       ,mIsTracking(isTracking)
       ,mCouldVary(couldVary)
+      ,mIsNoStore(isNoStore)
       ,mPredictor(predictor)
     { }
 
   private:
     virtual ~CacheabilityAction() { }
 
     nsCOMPtr<nsIURI> mTargetURI;
     uint32_t mHttpStatus;
     nsCString mMethod;
     bool mIsTracking;
     bool mCouldVary;
+    bool mIsNoStore;
     RefPtr<Predictor> mPredictor;
     nsTArray<nsCString> mKeysToCheck;
     nsTArray<nsCString> mValuesToCheck;
   };
 
   class Resetter : public nsICacheEntryOpenCallback,
                    public nsICacheEntryMetaDataVisitor,
                    public nsICacheStorageVisitor
@@ -423,17 +425,18 @@ private:
                           uint32_t &flags);
 
   // Used to update whether a particular URI was cacheable or not.
   // sourceURI and targetURI are the same as the arguments to Learn
   // and httpStatus is the status code we got while loading targetURI.
   void UpdateCacheabilityInternal(nsIURI *sourceURI, nsIURI *targetURI,
                                   uint32_t httpStatus, const nsCString &method,
                                   const OriginAttributes& originAttributes,
-                                  bool isTracking, bool couldVary);
+                                  bool isTracking, bool couldVary,
+                                  bool isNoStore);
 
   // Make sure our prefs are in their expected range of values
   void SanitizePrefs();
 
   // Our state
   bool mInitialized;
 
   bool mEnabled;
--- a/toolkit/components/telemetry/Histograms.json
+++ b/toolkit/components/telemetry/Histograms.json
@@ -3903,16 +3903,25 @@
   "PREDICTOR_PREDICT_TIME_TO_INACTION": {
     "record_in_processes": ["main", "content"],
     "expires_in_version": "never",
     "kind": "exponential",
     "high": 3000,
     "n_buckets": 10,
     "description": "How long it takes from the time Predict() is called to the time we figure out there's nothing to do"
   },
+  "PREDICTOR_PREFETCH_DECISION_REASON": {
+    "record_in_processes": ["main"],
+    "expires_in_version": "60",
+    "kind": "enumerated",
+    "n_values": 15,
+    "alert_emails": ["hurley@mozilla.com"],
+    "bug_numbers": [1409542],
+    "description": "Why the predictor determined a particular resource was eligible for future prefetch (or not). See PrefetchDecisionReason in Predictor.cpp for value meanings"
+  },
   "HTTPCONNMGR_TOTAL_SPECULATIVE_CONN": {
     "record_in_processes": ["main", "content"],
     "expires_in_version": "never",
     "kind": "exponential",
     "high": 1000000,
     "n_buckets": 50,
     "description": "How many speculative http connections are created"
   },