Bug 1409542 part 1 - telemetry on why a resource is marked prefetchable or not. draft
authorNicholas Hurley <hurley@mozilla.com>
Tue, 17 Oct 2017 14:46:56 -0700
changeset 684955 adc9fd67ecc0b6ed9e31a89b512b7b0838e8bf8f
parent 684703 d49501f258b105c5e2dcd0a59896ec1ceabf726b
child 684956 3408c5ab78e0f283318ccc620fc03a2dc08e5bbe
push id85774
push userbmo:hurley@mozilla.com
push dateMon, 23 Oct 2017 19:46:05 +0000
bugs1409542
milestone58.0a1
Bug 1409542 part 1 - telemetry on why a resource is marked prefetchable or not. 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"
   },