Add metrics for calculating precision/recall of link navigation pre-connect triggers.

chrome/browser/net/predictor.cc

Index: chrome/browser/net/predictor.cc
diff --git a/chrome/browser/net/predictor.cc b/chrome/browser/net/predictor.cc
index 6bedafb1b9b54ee2bb44123167c200e9b9591065..7ab6d1f8448cae20324c7b9457a7d9b35146b029 100644
--- a/chrome/browser/net/predictor.cc
+++ b/chrome/browser/net/predictor.cc
@@ -9,9 +9,11 @@
 #include <set>
 #include <sstream>
 
+#include "base/basictypes.h"
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
+#include "base/containers/mru_cache.h"
 #include "base/metrics/histogram.h"
 #include "base/prefs/pref_service.h"
 #include "base/stl_util.h"
@@ -125,6 +127,136 @@ class Predictor::LookupRequest {
   DISALLOW_COPY_AND_ASSIGN(LookupRequest);
 };
 
+// This records UMAs for preconnect usage based on navigation URLs to

[jar (doing other things), 2013/08/01 22:56:47]

FWIW: The long standing bug in this algorithm is that we monitor navigations,
but we don't monitor socket (connection) utilization count.

As a result, if a top level resource sequentially(!) needs 6 subresources, then
it is incorrectly tagged as as needing 6 preconnects.  This comments suggests
that we're really not addressing that issue... which may greatly improve
preconnection hit rate, and diminish the count of wasteful connections (that
never get used).

[kouhei (in TOK), 2013/08/02 06:18:03]

My understanding is that per-socket utilization count is measured at
Net.PreconnectUtilization2.
https://code.google.com/p/chromium/codesearch#chromium/src/net/socket/stream_...

I wanted to specifically measure navigations-oriented precision/recall here, as
my intent is to study and improve accuracy of the navigation based preconnect
triggers.

The problem of sequentially accessing subresources is very interesting, may be
we can improve Predictor::LearnFromNavigation? Although I would do it in
separate CL.

+// gather precision/recall for user-event based preconnect triggers.
+// Stats are gathered via a LRU cach that remembers all preconnect within the

[jar (doing other things), 2013/08/01 22:56:47]

nit: cache-->cache

[kouhei (in TOK), 2013/08/02 06:18:03]

Done.

+// last N seconds.
+// A preconnect trigger is considered as used iff a navigation including
+// access to the preconnected host occurs within a time period specified by

[jar (doing other things), 2013/08/01 22:56:47]

Per comment: We count access (still), and not simultaneous connection use :-/

+// kMaxUnusedSocketLifetimeSecondsWithoutAGet.
+class Predictor::PreconnectUsage {
+ public:
+  PreconnectUsage();
+  ~PreconnectUsage();
+
+  // Record a preconnect trigger to |url|.
+  void ObservePreconnect(const GURL& url);
+
+  // Record a user navigation with its redirect history, |url_chain|.
+  // We are uncertain if this is actually a link navigation.
+  void ObserveNavigationChain(const std::vector<GURL>& url_chain,
+                              bool is_subresource);
+
+  // Record a user link navigation to |final_url|.
+  // We are certain that this is a user-triggered link navigation.
+  void ObserveLinkNavigation(const GURL& final_url);
+
+ private:
+  // This tracks whether a preconnect was used in some navigation or not
+  class PreconnectPrecisionStat {
+   public:
+    PreconnectPrecisionStat()
+        : timestamp_(base::TimeTicks::Now()),
+          was_used_(false) {
+    }
+
+    const base::TimeTicks& timestamp() { return timestamp_; }
+
+    void set_was_used() { was_used_ = true; }
+    bool was_used() const { return was_used_; }
+
+   private:
+    base::TimeTicks timestamp_;
+    bool was_used_;
+  };
+
+  typedef base::MRUCache<GURL, PreconnectPrecisionStat> MRUPreconnects;
+  MRUPreconnects mru_preconnects_;
+
+  // The longest time an entry can persist in mru_preconnect_
+  const base::TimeDelta max_duration_;
+
+  std::vector<GURL> recent_navigation_chain_;
+
+  DISALLOW_COPY_AND_ASSIGN(PreconnectUsage);
+};
+
+Predictor::PreconnectUsage::PreconnectUsage()
+    : mru_preconnects_(MRUPreconnects::NO_AUTO_EVICT),
+      max_duration_(base::TimeDelta::FromSeconds(
+          Predictor::kMaxUnusedSocketLifetimeSecondsWithoutAGet)) {
+}
+
+Predictor::PreconnectUsage::~PreconnectUsage() {}
+
+void Predictor::PreconnectUsage::ObservePreconnect(const GURL& url) {
+  // Evict any overly old entries and record stats

[jar (doing other things), 2013/08/01 22:56:47]

nit: Period at end of sentences.

[kouhei (in TOK), 2013/08/02 06:18:03]

Done.

+  base::TimeTicks now = base::TimeTicks::Now();
+
+  MRUPreconnects::reverse_iterator eldest_preconnect =
+      mru_preconnects_.rbegin();
+  while (!mru_preconnects_.empty()) {
+    DCHECK(eldest_preconnect == mru_preconnects_.rbegin());
+    if (now - eldest_preconnect->second.timestamp() < max_duration_)
+      break;
+
+    UMA_HISTOGRAM_BOOLEAN("Net.PreconnectTriggerUsed",
+                          eldest_preconnect->second.was_used());
+    eldest_preconnect = mru_preconnects_.Erase(eldest_preconnect);
+  }
+
+  // Add new entry

[jar (doing other things), 2013/08/01 22:56:47]

nit: period

[kouhei (in TOK), 2013/08/02 06:18:03]

Done.

+  GURL canonical_url(Predictor::CanonicalizeUrl(url));
+  mru_preconnects_.Put(canonical_url, PreconnectPrecisionStat());
+}
+
+void Predictor::PreconnectUsage::ObserveNavigationChain(
+    const std::vector<GURL>& url_chain, bool is_subresource) {

[jar (doing other things), 2013/08/01 22:56:47]

nit: one arg per line.

[kouhei (in TOK), 2013/08/02 06:18:03]

Done.

+  if (url_chain.empty())
+    return;
+
+  if (!is_subresource)
+    recent_navigation_chain_ = url_chain;
+
+  GURL canonical_url(Predictor::CanonicalizeUrl(url_chain.back()));
+
+  // Record the preconnect trigger for the url as used if exist
+  MRUPreconnects::iterator itPreconnect = mru_preconnects_.Peek(canonical_url);
+  bool was_preconnected = (itPreconnect != mru_preconnects_.end());
+  if (was_preconnected)
+    itPreconnect->second.set_was_used();
+
+  // This is an UMA which was named incorrectly. This actually measures the
+  // ratio of URLRequests which have used a preconnected session.
+  UMA_HISTOGRAM_BOOLEAN("Net.PreconnectedNavigation", was_preconnected);
+}
+
+void Predictor::PreconnectUsage::ObserveLinkNavigation(const GURL& url) {
+  if (recent_navigation_chain_.empty() ||
+      url != recent_navigation_chain_.back()) {
+    // The navigation chain is not available for this navigation.
+    recent_navigation_chain_.clear();
+    recent_navigation_chain_.push_back(url);
+  }
+
+  // See if the link navigation involved preconnected session.
+  bool did_use_preconnect = false;
+  for (std::vector<GURL>::const_iterator it = recent_navigation_chain_.begin();
+       it != recent_navigation_chain_.end();
+       ++it) {
+    GURL canonical_url(Predictor::CanonicalizeUrl(*it));
+
+    // Record the preconnect trigger for the url as used if exist
+    MRUPreconnects::iterator itPreconnect =
+        mru_preconnects_.Peek(canonical_url);
+    bool was_preconnected = (itPreconnect != mru_preconnects_.end());
+    if (was_preconnected)
+      did_use_preconnect = true;
+  }
+
+  UMA_HISTOGRAM_BOOLEAN("Net.PreconnectedLinkNavigations", did_use_preconnect);
+}
+
 Predictor::Predictor(bool preconnect_enabled)
     : url_request_context_getter_(NULL),
       predictor_enabled_(true),
@@ -136,8 +268,6 @@ Predictor::Predictor(bool preconnect_enabled)
       host_resolver_(NULL),
       preconnect_enabled_(preconnect_enabled),
       consecutive_omnibox_preconnect_count_(0),
-      recent_preconnects_(
-          TimeDelta::FromSeconds(kMaxUnusedSocketLifetimeSecondsWithoutAGet)),
       next_trim_time_(base::TimeTicks::Now() +
                       TimeDelta::FromHours(kDurationBetweenTrimmingsHours)) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
@@ -668,6 +798,7 @@ void Predictor::FinalizeInitializationOnIOThread(
   predictor_enabled_ = predictor_enabled;
   initial_observer_.reset(new InitialObserver());
   host_resolver_ = io_thread->globals()->host_resolver.get();
+  preconnect_usage_.reset(new PreconnectUsage());
 
   // base::WeakPtrFactory instances need to be created and destroyed
   // on the same thread. The predictor lives on the IO thread and will die
@@ -844,11 +975,10 @@ void Predictor::PreconnectUrl(const GURL& url,
 }
 
 void Predictor::PreconnectUrlOnIOThread(
-    const GURL& url, const GURL& first_party_for_cookies,
-    UrlInfo::ResolutionMotivation motivation, int count) {
-  GURL canonical_url(CanonicalizeUrl(url));
-  recent_preconnects_.SetRecentlySeen(canonical_url);
-
+    const GURL& url,
+    const GURL& first_party_for_cookies,
+    UrlInfo::ResolutionMotivation motivation,
+    int count) {
   PreconnectOnIOThread(url,
                        first_party_for_cookies,
                        motivation,
@@ -856,10 +986,54 @@ void Predictor::PreconnectUrlOnIOThread(
                        url_request_context_getter_.get());
 }
 
-void Predictor::RecordPreconnectNavigationStats(const GURL& url) {
-  UMA_HISTOGRAM_BOOLEAN(
-      "Net.PreconnectedNavigation",
-      recent_preconnects_.WasRecentlySeen(url));
+void Predictor::RecordPreconnectTrigger(const GURL& url) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI) ||
+         BrowserThread::CurrentlyOn(BrowserThread::IO));
+
+  if (BrowserThread::CurrentlyOn(BrowserThread::IO)) {
+    RecordPreconnectTriggerOnIOThread(url);
+  } else {
+    BrowserThread::PostTask(
+        BrowserThread::IO,
+        FROM_HERE,
+        base::Bind(&Predictor::RecordPreconnectTriggerOnIOThread,
+                   base::Unretained(this), url));
+  }
+}
+
+void Predictor::RecordPreconnectTriggerOnIOThread(const GURL& url) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  if (preconnect_usage_)
+    preconnect_usage_->ObservePreconnect(url);
+}
+
+void Predictor::RecordPreconnectNavigationStat(
+    const std::vector<GURL>& url_chain, bool is_subresource) {

[jar (doing other things), 2013/08/01 22:56:47]

nit: one arg per line.

[kouhei (in TOK), 2013/08/02 06:18:03]

Done.

+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+
+  if (preconnect_usage_)
+    preconnect_usage_->ObserveNavigationChain(url_chain, is_subresource);
+}
+
+void Predictor::RecordLinkNavigation(const GURL& url) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI) ||
+         BrowserThread::CurrentlyOn(BrowserThread::IO));
+
+  if (BrowserThread::CurrentlyOn(BrowserThread::IO)) {
+    RecordLinkNavigationOnIOThread(url);
+  } else {
+    BrowserThread::PostTask(
+        BrowserThread::IO,
+        FROM_HERE,
+        base::Bind(&Predictor::RecordLinkNavigationOnIOThread,
+                   base::Unretained(this), url));
+  }
+}
+
+void Predictor::RecordLinkNavigationOnIOThread(const GURL& url) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  if (preconnect_usage_)
+    preconnect_usage_->ObserveLinkNavigation(url);
 }
 
 void Predictor::PredictFrameSubresources(const GURL& url,