NQE: Change GetEffectiveConnectionType to return last ECT

net/nqe/network_quality_estimator.cc

 // Copyright 2015 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 "net/nqe/network_quality_estimator.h"
 
 #include <algorithm>
 #include <cmath>
 #include <limits>
 #include <utility>
@@ skipping 355 matching lines @@
       last_connection_change_(tick_clock_->NowTicks()),
       current_network_id_(nqe::internal::NetworkID(
           NetworkChangeNotifier::ConnectionType::CONNECTION_UNKNOWN,
           std::string())),
       downstream_throughput_kbps_observations_(weight_multiplier_per_second_),
       rtt_observations_(weight_multiplier_per_second_),
       effective_connection_type_at_last_main_frame_(
           EFFECTIVE_CONNECTION_TYPE_UNKNOWN),
       external_estimate_provider_(std::move(external_estimates_provider)),
       effective_connection_type_recomputation_interval_(
-          base::TimeDelta::FromSeconds(15)),
+          base::TimeDelta::FromSeconds(10)),
       rtt_observations_size_at_last_ect_computation_(0),
       throughput_observations_size_at_last_ect_computation_(0),
       effective_connection_type_(EFFECTIVE_CONNECTION_TYPE_UNKNOWN),
       min_signal_strength_since_connection_change_(INT32_MAX),
       max_signal_strength_since_connection_change_(INT32_MIN),
       correlation_uma_logging_probability_(
           GetDoubleValueForVariationParamWithDefaultValue(
               variation_params,
               "correlation_logging_probability",
               0.0)),
@@ skipping 233 matching lines @@
     if (!GetTransportRTTEstimate(&estimated_transport_rtt))
       estimated_transport_rtt = nqe::internal::InvalidRTT();
 
     int32_t downstream_throughput_kbps;
     if (!GetDownlinkThroughputKbpsEstimate(&downstream_throughput_kbps))
       downstream_throughput_kbps = nqe::internal::kInvalidThroughput;
 
     estimated_quality_at_last_main_frame_ = nqe::internal::NetworkQuality(
         estimated_http_rtt, estimated_transport_rtt,
         downstream_throughput_kbps);
+
+    RecomputeEffectiveConnectionType();
     effective_connection_type_at_last_main_frame_ =
         GetEffectiveConnectionType();
 
     RecordMetricsOnMainFrameRequest();
     MaybeQueryExternalEstimateProvider();
 
     // Post the tasks which will run in the future and record the estimation
     // accuracy based on the observations received between now and the time of
     // task execution. Posting the task at different intervals makes it
     // possible to measure the accuracy by comparing the estimate with the
@@ skipping 511 matching lines @@
       base::Histogram::FactoryGet(
           std::string("NQE.MainFrame.EffectiveConnectionType.") +
               GetNameForConnectionType(current_network_id_.type),
           0, EFFECTIVE_CONNECTION_TYPE_LAST,
           EFFECTIVE_CONNECTION_TYPE_LAST /* Number of buckets */,
           base::HistogramBase::kUmaTargetedHistogramFlag);
 
   effective_connection_type_histogram->Add(effective_connection_type);
 }
 
+void NetworkQualityEstimator::RecomputeEffectiveConnectionType() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  const base::TimeTicks now = tick_clock_->NowTicks();
+
+  const EffectiveConnectionType past_type = effective_connection_type_;
+  last_effective_connection_type_computation_ = now;
+
+  effective_connection_type_ =
+      GetRecentEffectiveConnectionType(base::TimeTicks());
+
+  if (past_type != effective_connection_type_)
+    NotifyObserversOfEffectiveConnectionTypeChanged();
+
+  rtt_observations_size_at_last_ect_computation_ = rtt_observations_.Size();
+  throughput_observations_size_at_last_ect_computation_ =
+      downstream_throughput_kbps_observations_.Size();
+}
+
 EffectiveConnectionType NetworkQualityEstimator::GetEffectiveConnectionType()
     const {
   DCHECK(thread_checker_.CalledOnValidThread());
-  return GetRecentEffectiveConnectionType(base::TimeTicks());
+  return effective_connection_type_;
 }
 
 EffectiveConnectionType
 NetworkQualityEstimator::GetRecentEffectiveConnectionType(
     const base::TimeTicks& start_time) const {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   if (effective_connection_type_algorithm_ ==
       EffectiveConnectionTypeAlgorithm::HTTP_RTT_AND_DOWNSTREAM_THROUGHOUT) {
     return GetRecentEffectiveConnectionTypeUsingMetrics(
@@ skipping 443 matching lines @@
   // computation. Strict inequalities are used to ensure that effective
   // connection type is recomputed on connection change events even if the clock
   // has not updated.
   if (now - last_effective_connection_type_computation_ <
           effective_connection_type_recomputation_interval_ &&
       last_connection_change_ < last_effective_connection_type_computation_ &&
       // Recompute the effective connection type if the previously computed
       // effective connection type was unknown.
       effective_connection_type_ != EFFECTIVE_CONNECTION_TYPE_UNKNOWN &&
       // Recompute the effective connection type if the number of samples
-      // available now are more than twice in count than the number of
-      // samples that were available when the effective connection type was
-      // last computed.
-      rtt_observations_size_at_last_ect_computation_ * 2 >=
+      // available now are 50% more in count than the number of samples that

[bengr, 2016/08/25 22:51:24]

remove "in count"

[tbansal1, 2016/09/07 18:33:26]

Done.

+      // were available when the effective connection type was last computed.
+      rtt_observations_size_at_last_ect_computation_ * 1.5 >=

[bengr, 2016/08/25 22:51:24]

Does this mean that there's an exponential backoff on recomputation? If so, I
don't think that's a great idea, since we'll want to be responsive to sudden
changes. 

[tbansal1, 2016/09/07 18:33:26]

There is exponential backoff. Additionally, ECT is recomputed every 10 seconds
(provided there is network traffic), and on every main frame request. And, also
(in future) we may recompute if there is a significant change in the signal
strength.

The goal here is that NQE should detect "sudden changes" and respond to them
instead of callers detecting these changes, and forcing NQE to recompute ECT.

           rtt_observations_.Size() &&
-      throughput_observations_size_at_last_ect_computation_ * 2 >=
+      throughput_observations_size_at_last_ect_computation_ * 1.5 >=
           downstream_throughput_kbps_observations_.Size()) {
     return;
   }
 
-  const EffectiveConnectionType past_type = effective_connection_type_;
-  last_effective_connection_type_computation_ = now;
-  effective_connection_type_ = GetEffectiveConnectionType();
-
-  if (past_type != effective_connection_type_)
-    NotifyObserversOfEffectiveConnectionTypeChanged();
-
-  rtt_observations_size_at_last_ect_computation_ = rtt_observations_.Size();
-  throughput_observations_size_at_last_ect_computation_ =
-      downstream_throughput_kbps_observations_.Size();
+  RecomputeEffectiveConnectionType();

[bengr, 2016/08/25 22:51:24]

Can you just call this "ComputeEffectiveConnectionType()?"

[tbansal1, 2016/09/07 18:33:26]

Done.

 }
 
 void NetworkQualityEstimator::
     NotifyObserversOfEffectiveConnectionTypeChanged() {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK_NE(EFFECTIVE_CONNECTION_TYPE_LAST, effective_connection_type_);
 
   // TODO(tbansal): Add hysteresis in the notification.
   FOR_EACH_OBSERVER(
       EffectiveConnectionTypeObserver, effective_connection_type_observer_list_,
       OnEffectiveConnectionTypeChanged(effective_connection_type_));
 
   // Add the estimates of the current network to the cache store.
   if (effective_connection_type_ != EFFECTIVE_CONNECTION_TYPE_UNKNOWN) {
     network_quality_store_.Add(
         current_network_id_,
         nqe::internal::CachedNetworkQuality(
             tick_clock_->NowTicks(), estimated_quality_at_last_main_frame_,
             effective_connection_type_));
   }
 }
 
 }  // namespace net