Add in-memory caching of network quality estimates across network changes.

net/base/network_quality_estimator.cc

Index: net/base/network_quality_estimator.cc
diff --git a/net/base/network_quality_estimator.cc b/net/base/network_quality_estimator.cc
index 6415ee6e6d7eb36b47ab4fd568d3537283b27729..8a26b777b53dff05b5eb8777cb28a14d85383fae 100644
--- a/net/base/network_quality_estimator.cc
+++ b/net/base/network_quality_estimator.cc
@@ -4,15 +4,28 @@
 
 #include "net/base/network_quality_estimator.h"
 
-#include <string>
-
 #include "base/logging.h"
+#include "base/memory/scoped_ptr.h"
 #include "base/metrics/histogram.h"
+#include "build/build_config.h"
 #include "net/base/net_util.h"
-#include "net/base/network_quality.h"
+#include "net/base/network_interfaces.h"
 #include "net/url_request/url_request.h"
 #include "url/gurl.h"
 
+#if defined(OS_ANDROID)
+#include "net/android/network_library.h"
+#endif  // OS_ANDROID
+
+namespace {
+
+// Maximum size of the cache that holds network quality estimates.
+// Smaller size may reduce the cache hit rate due to frequent evictions.
+// Larger size may affect performance.
+const uint32_t kMaximumNetworkQualityCacheSize = 10;
+
+}  // namespace
+
 namespace net {
 
 NetworkQualityEstimator::NetworkQualityEstimator()
@@ -23,12 +36,16 @@ NetworkQualityEstimator::NetworkQualityEstimator(
     bool allow_local_host_requests_for_tests)
     : allow_localhost_requests_(allow_local_host_requests_for_tests),
       last_connection_change_(base::TimeTicks::Now()),
-      current_connection_type_(NetworkChangeNotifier::GetConnectionType()),
       bytes_read_since_last_connection_change_(false),
-      peak_kbps_since_last_connection_change_(0) {
+      peak_kbps_since_last_connection_change_(0),
+      current_network_id_(NetworkID(NetworkChangeNotifier::GetConnectionType(),
+                                    std::string())) {
   static_assert(kMinRequestDurationMicroseconds > 0,
                 "Minimum request duration must be > 0");
+  static_assert(kMaximumNetworkQualityCacheSize > 0,
+                "Size of the network quality cache must be > 0");
   NetworkChangeNotifier::AddConnectionTypeObserver(this);
+  current_network_id_.id = GetCurrentNetworkName();
 }
 
 NetworkQualityEstimator::~NetworkQualityEstimator() {
@@ -77,7 +94,7 @@ void NetworkQualityEstimator::OnConnectionTypeChanged(
     NetworkChangeNotifier::ConnectionType type) {
   DCHECK(thread_checker_.CalledOnValidThread());
   if (bytes_read_since_last_connection_change_) {
-    switch (current_connection_type_) {
+    switch (current_network_id_.type) {
       case NetworkChangeNotifier::CONNECTION_UNKNOWN:
         UMA_HISTOGRAM_TIMES("NQE.FastestRTT.Unknown",
                             fastest_RTT_since_last_connection_change_);
@@ -111,13 +128,14 @@ void NetworkQualityEstimator::OnConnectionTypeChanged(
                             fastest_RTT_since_last_connection_change_);
         break;
       default:
-        NOTREACHED();
+        NOTREACHED() << "Unexpected connection type = "
+                     << current_network_id_.type;
         break;
     }
   }
 
   if (peak_kbps_since_last_connection_change_) {
-    switch (current_connection_type_) {
+    switch (current_network_id_.type) {
       case NetworkChangeNotifier::CONNECTION_UNKNOWN:
         UMA_HISTOGRAM_COUNTS("NQE.PeakKbps.Unknown",
                              peak_kbps_since_last_connection_change_);
@@ -151,15 +169,27 @@ void NetworkQualityEstimator::OnConnectionTypeChanged(
                              peak_kbps_since_last_connection_change_);
         break;
       default:
-        NOTREACHED();
+        NOTREACHED() << "Unexpected connection type = "
+                     << current_network_id_.type;
         break;
     }
   }
 
+  // Write the estimates of the previous network to the cache.
+  CacheNetworkQualityEstimate();

[mmenke, 2015/06/09 19:48:07]

Should we verify that the network actually changed before caching anything?

[tbansal1, 2015/06/10 00:23:28]

I think that would complicate the logic without much benefit. Right now, either
both events (caching and clearing of NQE local state) happen or neither of them.


If we decouple them, that means: we clear local state but do not write
to cache. In this case, we still need to add an entry to the 
circular ring buffer (deque) to seed the estimator. This would require
completely different logic than ReadCachedNQE().

I think when the device goes from 1 network to another, there 
is an intermediate notification with ConnectionType of None.
So, it seems it would be very rare that OnConnectionTypeChanged()
is called but there is no change in ConnectionType.

WDYT?

+
   last_connection_change_ = base::TimeTicks::Now();
   bytes_read_since_last_connection_change_ = false;
   peak_kbps_since_last_connection_change_ = 0;
-  current_connection_type_ = type;
+  current_network_id_.type = type;
+  current_network_id_.id = GetCurrentNetworkName();
+
+  // Read any cached estimates for the new network.
+  ReadCachedNetworkQualityEstimate();
+}
+
+size_t NetworkQualityEstimator::GetCacheSizeForTests() const {
+  return cached_network_quality_.size();
 }
 
 NetworkQuality NetworkQualityEstimator::GetEstimate() const {
@@ -177,4 +207,113 @@ NetworkQuality NetworkQualityEstimator::GetEstimate() const {
                         peak_kbps_since_last_connection_change_, 0.1);
 }
 
+std::string NetworkQualityEstimator::GetCurrentNetworkName() const {
+  // TODO(tbansal): crbug.com/498068 Add NetworkQualityEstimatorAndroid class
+  // that overrides this method on Android platform.
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  switch (current_network_id_.type) {
+    case NetworkChangeNotifier::ConnectionType::CONNECTION_UNKNOWN:
+    case NetworkChangeNotifier::ConnectionType::CONNECTION_NONE:
+    case NetworkChangeNotifier::ConnectionType::CONNECTION_BLUETOOTH:
+      return std::string();
+    case NetworkChangeNotifier::ConnectionType::CONNECTION_ETHERNET:
+      return std::string();
+    case NetworkChangeNotifier::ConnectionType::CONNECTION_WIFI:
+#if defined(OS_ANDROID) || defined(OS_LINUX) || defined(OS_CHROMEOS)
+      return GetWifiSSID();
+#endif  // defined(OS_ANDROID) || defined(OS_LINUX) || defined(OS_CHROMEOS)
+      return std::string();
+    case NetworkChangeNotifier::ConnectionType::CONNECTION_2G:
+    case NetworkChangeNotifier::ConnectionType::CONNECTION_3G:
+    case NetworkChangeNotifier::ConnectionType::CONNECTION_4G:
+#if defined(OS_ANDROID)
+      return android::GetTelephonyNetworkOperator();
+#endif  // OS_ANDROID
+      return std::string();
+    default:
+      NOTREACHED() << "Unexpected connection type = "
+                   << current_network_id_.type;
+      return std::string();
+  }
+}
+
+bool NetworkQualityEstimator::ReadCachedNetworkQualityEstimate() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  CachedQualities::iterator it =
+      cached_network_quality_.find(current_network_id_);
+  if (it != cached_network_quality_.end()) {
+    // TOOD(tbansal): Populate these values back into the median computing
+    // algorithm.
+    // Add UMA to record how frequently match happens.
+    // int64 median_kbps = network_quality.median_kbps;
+    // int64 median_rtt_msec = network_quality.median_rtt_milliseconds;
+    // Ensure that the estimates read are non-zero before populating them into
+    // the median computing algorithm.
+    return true;
+  }
+  return false;
+}
+
+void NetworkQualityEstimator::CacheNetworkQualityEstimate() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK_LE(cached_network_quality_.size(), kMaximumNetworkQualityCacheSize);
+
+  // TODO(tbansal): Following variables should be initialized using the median
+  /// values reported by NetworkQualityEstimator.
+  int median_kbps = 0;
+  base::TimeDelta median_rtt;
+
+  // If this network is already in the cache, overwrite that entry.
+  CachedQualities::iterator it =
+      cached_network_quality_.find(current_network_id_);
+  if (it != cached_network_quality_.end()) {
+    (it->second)->UpdateNetworkQuality(median_kbps, median_rtt);
+    return;
+  }
+
+  if (cached_network_quality_.size() == kMaximumNetworkQualityCacheSize) {
+    // Remove the oldest entry.
+    CachedQualities::iterator oldest_entry_iterator =
+        cached_network_quality_.begin();
+
+    for (CachedQualities::iterator iterator = cached_network_quality_.begin();
+         iterator != cached_network_quality_.end(); ++iterator) {
+      if ((oldest_entry_iterator->second)->last_update_time() >
+          (iterator->second)->last_update_time()) {
+        oldest_entry_iterator = iterator;
+      }
+    }
+    cached_network_quality_.erase(oldest_entry_iterator);
+  }
+
+  DCHECK_LT(cached_network_quality_.size(), kMaximumNetworkQualityCacheSize);
+  cached_network_quality_.insert(
+      std::make_pair(current_network_id_,
+                     make_scoped_ptr(new CachedNetworkQuality(
+                         NetworkQuality(median_rtt, 0.0, median_kbps, 0.0)))));
+  DCHECK_LE(cached_network_quality_.size(), kMaximumNetworkQualityCacheSize);
+}
+
+NetworkQualityEstimator::CachedNetworkQuality::CachedNetworkQuality(
+    const NetworkQuality& network_quality)
+    : last_update_time_(base::TimeTicks::Now()),
+      network_quality_(network_quality) {
+}
+
+NetworkQualityEstimator::CachedNetworkQuality::~CachedNetworkQuality() {
+}
+
+void NetworkQualityEstimator::CachedNetworkQuality::UpdateNetworkQuality(
+    uint64_t median_kbps,
+    const base::TimeDelta& median_rtt) {
+  DCHECK_GE(median_kbps, 0U);
+  DCHECK_GE(median_rtt, base::TimeDelta());
+  last_update_time_ = base::TimeTicks::Now();
+
+  network_quality_.fastest_rtt = median_rtt;
+  network_quality_.peak_throughput_kbps = median_kbps;
+}
+
 }  // namespace net