Add PLT measurement to Resource Prefetching for Mobile Web

chrome/browser/predictors/resource_prefetch_predictor.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/predictors/resource_prefetch_predictor.h"
 
 #include <map>
 #include <set>
 #include <utility>
 
@@ skipping 16 matching lines @@
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/url_constants.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_source.h"
 #include "content/public/browser/notification_types.h"
 #include "content/public/browser/resource_request_info.h"
 #include "content/public/browser/web_contents.h"
 #include "net/base/mime_util.h"
+#include "net/base/network_change_notifier.h"
 #include "net/http/http_response_headers.h"
 #include "net/url_request/url_request.h"
 #include "net/url_request/url_request_context_getter.h"
 
 using content::BrowserThread;
 
 namespace {
 
 // For reporting whether a subresource is handled or not, and for what reasons.
 enum ResourceStatus {
@@ skipping 36 matching lines @@
   REPORTING_EVENT_PARTIAL_HISTORY_CLEARED = 1,
   REPORTING_EVENT_COUNT = 2
 };
 
 void RecordNavigationEvent(NavigationEvent event) {
   UMA_HISTOGRAM_ENUMERATION("ResourcePrefetchPredictor.NavigationEvent",
                             event,
                             NAVIGATION_EVENT_COUNT);
 }
 
+enum AdditionalConnectionType {
+    CONNECTION_CELLULAR = net::NetworkChangeNotifier::CONNECTION_LAST + 1,

[Alexei Svitkine (slow), 2014/10/14 19:27:27]

Nit: Indentation is wrong.

[Zhen Wang, 2014/10/16 18:12:22]

Done.

+    CONNECTION_ALL = net::NetworkChangeNotifier::CONNECTION_LAST + 2,
+    CONNECTION_COUNT = net::NetworkChangeNotifier::CONNECTION_LAST + 3
+  };
+
 }  // namespace
 
 namespace predictors {
 
 ////////////////////////////////////////////////////////////////////////////////
 // History lookup task.
 
 // Used to fetch the visit count for a URL from the History database.
 class GetUrlVisitCountTask : public history::HistoryDBTask {
  public:
@@ skipping 415 matching lines @@
 
   NavigationMap::iterator nav_it =
       inflight_navigations_.find(navigation_id);
   if (nav_it == inflight_navigations_.end()) {
     RecordNavigationEvent(NAVIGATION_EVENT_ONLOAD_UNTRACKED_URL);
     return;
   }
   RecordNavigationEvent(NAVIGATION_EVENT_ONLOAD_TRACKED_URL);
 
   // Report any stats.
+  base::TimeDelta plt = base::TimeTicks::Now() - navigation_id.creation_time;
+  ReportPageLoadTimeStats(plt);
   if (prefetch_manager_.get()) {
     ResultsMap::iterator results_it = results_map_.find(navigation_id);
     bool have_prefetch_results = results_it != results_map_.end();
     UMA_HISTOGRAM_BOOLEAN("ResourcePrefetchPredictor.HavePrefetchResults",
                           have_prefetch_results);
     if (have_prefetch_results) {
       ReportAccuracyStats(results_it->second->key_type,
                           *(nav_it->second),
                           results_it->second->requests.get());
+      ReportPageLoadTimePrefetchedStats(results_it->second->key_type, plt);
+    } else {
+      ReportPageLoadTimeNotPrefetchedStats(plt);
     }
   } else {
     scoped_ptr<ResourcePrefetcher::RequestVector> requests(
         new ResourcePrefetcher::RequestVector);
     PrefetchKeyType key_type;
     if (GetPrefetchData(navigation_id, requests.get(), &key_type)) {
       RecordNavigationEvent(NAVIGATION_EVENT_HAVE_PREDICTIONS_FOR_URL);
       ReportPredictedAccuracyStats(key_type,
                                    *(nav_it->second),
                                    *requests);
@@ skipping 449 matching lines @@
     BrowserThread::PostTask(
         BrowserThread::DB, FROM_HERE,
         base::Bind(&ResourcePrefetchPredictorTables::UpdateData,
                    tables_,
                    url_data,
                    host_data));
   }
 }
 
 ////////////////////////////////////////////////////////////////////////////////
-// Accuracy measurement.
+// Page load time and accuracy measurement.
+
+#define REPORT_PLT(name, page_load_time) \
+  UMA_HISTOGRAM_CUSTOM_TIMES( \
+      name, \
+      page_load_time, \
+      base::TimeDelta::FromMilliseconds(10), \
+      base::TimeDelta::FromSeconds(60), \

[Alexei Svitkine (slow), 2014/10/14 19:27:27]

I don't think 60s is a good upper bound - there are probably pages that take
longer to load.

I suggest using UMA_HISTOGRAM_MEDIUM_TIMES(), which gives you an upper bound of
3minutes. It is 50 buckets, rather than 100, but I think given that you're
adding so many histograms, its wasteful to have them all be 100 buckets, in
terms of memory used on the client and data used in logs.

[Zhen Wang, 2014/10/16 18:12:22]

The granularity of 3 minutes with 50 buckets is too coarse (3.6s). The
granularity of 1 minute with 100 buckets (0.6s) seems most useful. For current
stage, we do not care too much about super long PLT pages. Although there are
many histograms for PLT, they are usually exclusive to each other because there
is only one network type for each page load.

And this is consistent with the implementation in prerender_histograms
(https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/pre...).

[Alexei Svitkine (slow), 2014/10/16 20:05:36]

Are you aware that these histograms will have bucket sizes that are
exponentially increasing? You can't just divide 3 minutes by 50, since it's not
a linear distribution. Instead, you'll get more granularity at the low range and
coarser granularity closer to the max value.

[Zhen Wang, 2014/10/16 21:57:34]

Oh, thanks for pointing it out. Using medium times now.

+      100)
+
+void ResourcePrefetchPredictor::ReportPageLoadTimeStats(
+    base::TimeDelta plt) const {
+  net::NetworkChangeNotifier::ConnectionType connection_type =
+      net::NetworkChangeNotifier::GetConnectionType();
+
+  REPORT_PLT("ResourcePrefetchPredictor.PLT", plt);
+  if (net::NetworkChangeNotifier::IsConnectionCellular(connection_type))
+    REPORT_PLT("ResourcePrefetchPredictor.PLT.Cellular", plt);
+  switch (connection_type) {
+    case net::NetworkChangeNotifier::CONNECTION_ETHERNET:
+      REPORT_PLT("ResourcePrefetchPredictor.PLT.Ethernet", plt);
+      break;
+    case net::NetworkChangeNotifier::CONNECTION_WIFI:
+      REPORT_PLT("ResourcePrefetchPredictor.PLT.WiFi", plt);
+      break;
+    case net::NetworkChangeNotifier::CONNECTION_2G:
+      REPORT_PLT("ResourcePrefetchPredictor.PLT.2G", plt);
+      break;
+    case net::NetworkChangeNotifier::CONNECTION_3G:
+      REPORT_PLT("ResourcePrefetchPredictor.PLT.3G", plt);
+      break;
+    case net::NetworkChangeNotifier::CONNECTION_4G:
+      REPORT_PLT("ResourcePrefetchPredictor.PLT.4G", plt);
+      break;
+    case net::NetworkChangeNotifier::CONNECTION_NONE:
+      REPORT_PLT("ResourcePrefetchPredictor.PLT.None", plt);
+      break;
+    case net::NetworkChangeNotifier::CONNECTION_BLUETOOTH:
+      REPORT_PLT("ResourcePrefetchPredictor.PLT.Bluetooth", plt);
+      break;
+    case net::NetworkChangeNotifier::CONNECTION_UNKNOWN:
+    default:
+      REPORT_PLT("ResourcePrefetchPredictor.PLT.Unknown", plt);
+      break;
+  }
+}
+
+void ResourcePrefetchPredictor::ReportPageLoadTimePrefetchedStats(
+    PrefetchKeyType key_type,
+    base::TimeDelta plt) const {
+  net::NetworkChangeNotifier::ConnectionType connection_type =
+      net::NetworkChangeNotifier::GetConnectionType();
+  bool on_cellular =
+      net::NetworkChangeNotifier::IsConnectionCellular(connection_type);
+
+  UMA_HISTOGRAM_ENUMERATION(
+      "ResourcePrefetchPredictor.PagePrefetchedNetworkType",

[Alexei Svitkine (slow), 2014/10/14 19:27:27]

Please make a helper function for this histogram which takes the enum value as a
param. Since each histogram macro block expands to a lot of code, this will help
reduce some bloat.

[Zhen Wang, 2014/10/16 18:12:22]

Done.

+      CONNECTION_ALL,
+      CONNECTION_COUNT);
+  UMA_HISTOGRAM_ENUMERATION(
+      "ResourcePrefetchPredictor.PagePrefetchedNetworkType",
+      net::NetworkChangeNotifier::GetConnectionType(),
+      CONNECTION_COUNT);
+  if (on_cellular) {
+    UMA_HISTOGRAM_ENUMERATION(
+        "ResourcePrefetchPredictor.PagePrefetchedNetworkType",
+        CONNECTION_CELLULAR,
+        CONNECTION_COUNT);
+  }
+
+  REPORT_PLT("ResourcePrefetchPredictor.PLT.PagePrefetched", plt);
+  if (on_cellular)
+    REPORT_PLT("ResourcePrefetchPredictor.PLT.PagePrefetched.Cellular", plt);
+  switch (connection_type) {
+    case net::NetworkChangeNotifier::CONNECTION_ETHERNET:
+      REPORT_PLT("ResourcePrefetchPredictor.PLT.PagePrefetched.Ethernet", plt);
+      break;
+    case net::NetworkChangeNotifier::CONNECTION_WIFI:
+      REPORT_PLT("ResourcePrefetchPredictor.PLT.PagePrefetched.WiFi", plt);
+      break;
+    case net::NetworkChangeNotifier::CONNECTION_2G:
+      REPORT_PLT("ResourcePrefetchPredictor.PLT.PagePrefetched.2G", plt);
+      break;
+    case net::NetworkChangeNotifier::CONNECTION_3G:
+      REPORT_PLT("ResourcePrefetchPredictor.PLT.PagePrefetched.3G", plt);
+      break;
+    case net::NetworkChangeNotifier::CONNECTION_4G:
+      REPORT_PLT("ResourcePrefetchPredictor.PLT.PagePrefetched.4G", plt);
+      break;
+    case net::NetworkChangeNotifier::CONNECTION_NONE:
+      REPORT_PLT("ResourcePrefetchPredictor.PLT.PagePrefetched.None", plt);
+      break;
+    case net::NetworkChangeNotifier::CONNECTION_BLUETOOTH:
+      REPORT_PLT("ResourcePrefetchPredictor.PLT.PagePrefetched.Bluetooth", plt);
+      break;
+    case net::NetworkChangeNotifier::CONNECTION_UNKNOWN:
+    default:
+      REPORT_PLT(
+          "ResourcePrefetchPredictor.PLT.PagePrefetched.Unknown", plt);
+      break;
+  }
+
+  if (key_type == PREFETCH_KEY_TYPE_HOST) {
+    REPORT_PLT("ResourcePrefetchPredictor.PLT.PagePrefetched.Host", plt);
+    if (on_cellular) {
+      REPORT_PLT(
+          "ResourcePrefetchPredictor.PLT.PagePrefetched.Host.Cellular", plt);
+    }
+    switch (connection_type) {
+      case net::NetworkChangeNotifier::CONNECTION_ETHERNET:
+        REPORT_PLT(
+            "ResourcePrefetchPredictor.PLT.PagePrefetched.Host.Ethernet", plt);
+        break;
+      case net::NetworkChangeNotifier::CONNECTION_WIFI:
+        REPORT_PLT(
+            "ResourcePrefetchPredictor.PLT.PagePrefetched.Host.WiFi", plt);
+        break;
+      case net::NetworkChangeNotifier::CONNECTION_2G:
+        REPORT_PLT(
+            "ResourcePrefetchPredictor.PLT.PagePrefetched.Host.2G", plt);
+        break;
+      case net::NetworkChangeNotifier::CONNECTION_3G:
+        REPORT_PLT(
+            "ResourcePrefetchPredictor.PLT.PagePrefetched.Host.3G", plt);
+        break;
+      case net::NetworkChangeNotifier::CONNECTION_4G:
+        REPORT_PLT(
+            "ResourcePrefetchPredictor.PLT.PagePrefetched.Host.4G", plt);
+        break;
+      case net::NetworkChangeNotifier::CONNECTION_NONE:
+        REPORT_PLT(
+            "ResourcePrefetchPredictor.PLT.PagePrefetched.Host.None", plt);
+        break;
+      case net::NetworkChangeNotifier::CONNECTION_BLUETOOTH:
+        REPORT_PLT(
+            "ResourcePrefetchPredictor.PLT.PagePrefetched.Host.Bluetooth", plt);
+        break;
+      case net::NetworkChangeNotifier::CONNECTION_UNKNOWN:
+      default:
+        REPORT_PLT(
+            "ResourcePrefetchPredictor.PLT.PagePrefetched.Host.Unknown", plt);
+        break;
+    }
+  } else {
+    REPORT_PLT("ResourcePrefetchPredictor.PLT.PagePrefetched.Url", plt);

[Alexei Svitkine (slow), 2014/10/14 19:27:27]

Honestly, I feel like it would be better to just use the underlying Histogram*
API directly, rather than going through the macros. Then, you can avoid all of
this duplication and just have helper functions that return suffix strings for
the particular enums.

[Zhen Wang, 2014/10/16 18:12:22]

I originally have a more concise version (patch 1). e.g.

REPORT_PLT("ResourcePrefetchPredictor.PLT.PagePrefetched." + histogram_type +
"." + GetNetworkTypeString(), plt);

But it turns out that each macro should records a const name. So I have to
expand it to this duplication. Or is there a better histogram API for me to use?

[Alexei Svitkine (slow), 2014/10/16 20:05:36]

Yeah, you can't use the macro. You need to use the FactoryGet() API of a
histogram directly. You can look at what the UMA macro expands to and use that
minus the static var that's used for caching.

[Zhen Wang, 2014/10/16 21:57:34]

Using self-defined RPP_HISTOGRAM_MEDIUM_TIMES now.

+    if (on_cellular) {
+      REPORT_PLT(
+          "ResourcePrefetchPredictor.PLT.PagePrefetched.Url.Cellular", plt);
+    }
+    switch (connection_type) {
+      case net::NetworkChangeNotifier::CONNECTION_ETHERNET:
+        REPORT_PLT(
+            "ResourcePrefetchPredictor.PLT.PagePrefetched.Url.Ethernet", plt);
+        break;
+      case net::NetworkChangeNotifier::CONNECTION_WIFI:
+        REPORT_PLT(
+            "ResourcePrefetchPredictor.PLT.PagePrefetched.Url.WiFi", plt);
+        break;
+      case net::NetworkChangeNotifier::CONNECTION_2G:
+        REPORT_PLT(
+            "ResourcePrefetchPredictor.PLT.PagePrefetched.Url.2G", plt);
+        break;
+      case net::NetworkChangeNotifier::CONNECTION_3G:
+        REPORT_PLT(
+            "ResourcePrefetchPredictor.PLT.PagePrefetched.Url.3G", plt);
+        break;
+      case net::NetworkChangeNotifier::CONNECTION_4G:
+        REPORT_PLT(
+            "ResourcePrefetchPredictor.PLT.PagePrefetched.Url.4G", plt);
+        break;
+      case net::NetworkChangeNotifier::CONNECTION_NONE:
+        REPORT_PLT(
+            "ResourcePrefetchPredictor.PLT.PagePrefetched.Url.None", plt);
+        break;
+      case net::NetworkChangeNotifier::CONNECTION_BLUETOOTH:
+        REPORT_PLT(
+            "ResourcePrefetchPredictor.PLT.PagePrefetched.Url.Bluetooth", plt);
+        break;
+      case net::NetworkChangeNotifier::CONNECTION_UNKNOWN:
+      default:
+        REPORT_PLT(
+            "ResourcePrefetchPredictor.PLT.PagePrefetched.Url.Unknown", plt);
+        break;
+    }
+  }
+}
+
+void ResourcePrefetchPredictor::ReportPageLoadTimeNotPrefetchedStats(
+    base::TimeDelta plt) const {
+  net::NetworkChangeNotifier::ConnectionType connection_type =
+      net::NetworkChangeNotifier::GetConnectionType();
+  bool on_cellular =
+      net::NetworkChangeNotifier::IsConnectionCellular(connection_type);
+
+  UMA_HISTOGRAM_ENUMERATION(
+      "ResourcePrefetchPredictor.PageNotPrefetchedNetworkType",
+      CONNECTION_ALL,
+      CONNECTION_COUNT);
+  UMA_HISTOGRAM_ENUMERATION(
+      "ResourcePrefetchPredictor.PageNotPrefetchedNetworkType",
+      net::NetworkChangeNotifier::GetConnectionType(),
+      CONNECTION_COUNT);
+  if (on_cellular) {
+    UMA_HISTOGRAM_ENUMERATION(
+        "ResourcePrefetchPredictor.PagePrefetchedNetworkType",
+        CONNECTION_CELLULAR,
+        CONNECTION_COUNT);
+  }
+
+  REPORT_PLT("ResourcePrefetchPredictor.PLT.PageNotPrefetched", plt);
+  if (on_cellular)
+    REPORT_PLT("ResourcePrefetchPredictor.PLT.PageNotPrefetched.Cellular", plt);
+  switch (connection_type) {
+    case net::NetworkChangeNotifier::CONNECTION_ETHERNET:
+      REPORT_PLT(
+          "ResourcePrefetchPredictor.PLT.PageNotPrefetched.Ethernet", plt);
+      break;
+    case net::NetworkChangeNotifier::CONNECTION_WIFI:
+      REPORT_PLT("ResourcePrefetchPredictor.PLT.PageNotPrefetched.WiFi", plt);
+      break;
+    case net::NetworkChangeNotifier::CONNECTION_2G:
+      REPORT_PLT("ResourcePrefetchPredictor.PLT.PageNotPrefetched.2G", plt);
+      break;
+    case net::NetworkChangeNotifier::CONNECTION_3G:
+      REPORT_PLT("ResourcePrefetchPredictor.PLT.PageNotPrefetched.3G", plt);
+      break;
+    case net::NetworkChangeNotifier::CONNECTION_4G:
+      REPORT_PLT("ResourcePrefetchPredictor.PLT.PageNotPrefetched.4G", plt);
+      break;
+    case net::NetworkChangeNotifier::CONNECTION_NONE:
+      REPORT_PLT("ResourcePrefetchPredictor.PLT.PageNotPrefetched.None", plt);
+      break;
+    case net::NetworkChangeNotifier::CONNECTION_BLUETOOTH:
+      REPORT_PLT(
+          "ResourcePrefetchPredictor.PLT.PageNotPrefetched.Bluetooth", plt);
+      break;
+    case net::NetworkChangeNotifier::CONNECTION_UNKNOWN:
+    default:
+      REPORT_PLT(
+          "ResourcePrefetchPredictor.PLT.PageNotPrefetched.Unknown", plt);
+      break;
+  }
+}
 
 void ResourcePrefetchPredictor::ReportAccuracyStats(
     PrefetchKeyType key_type,
     const std::vector<URLRequestSummary>& actual,
     ResourcePrefetcher::RequestVector* prefetched) const {
   // Annotate the results.
   std::map<GURL, bool> actual_resources;
   for (std::vector<URLRequestSummary>::const_iterator it = actual.begin();
        it != actual.end(); ++it) {
     actual_resources[it->resource_url] = it->was_cached;
@@ skipping 198 matching lines @@
     RPP_PREDICTED_HISTOGRAM_PERCENTAGE(
         "PrefetchFromNetworkPercentOfTotalFromNetwork",
         prefetch_network * 100.0 / total_resources_fetched_from_network);
   }
 
 #undef RPP_PREDICTED_HISTOGRAM_PERCENTAGE
 #undef RPP_PREDICTED_HISTOGRAM_COUNTS
 }
 
 }  // namespace predictors