Throughput: Change the min number of requests in flight required

net/nqe/throughput_analyzer.cc

 // Copyright 2016 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/throughput_analyzer.h"
 
 #include <cmath>
 
 #include "base/location.h"
 #include "base/single_thread_task_runner.h"
 #include "net/base/network_activity_monitor.h"
 #include "net/base/url_util.h"
+#include "net/nqe/network_quality_estimator_params.h"
 #include "net/url_request/url_request.h"
 
 #if defined(OS_ANDROID)
 #include "net/android/traffic_stats.h"
 #endif  // OS_ANDROID
 
 namespace net {
 
 namespace {
 
 // Maximum number of accuracy degrading requests, and requests that do not
 // degrade accuracy held in the memory.
 static const size_t kMaxRequestsSize = 300;
 
 // Tiny transfer sizes may give inaccurate throughput results.
 // Minimum size of the transfer over which the throughput is computed.
 static const int kMinTransferSizeInBits = 32 * 8 * 1000;
 
 }  // namespace
 
 namespace nqe {
 
 namespace internal {
 
 ThroughputAnalyzer::ThroughputAnalyzer(
+    const NetworkQualityEstimatorParams* params,
     scoped_refptr<base::SingleThreadTaskRunner> task_runner,
     ThroughputObservationCallback throughput_observation_callback,
     bool use_local_host_requests_for_tests,
     bool use_smaller_responses_for_tests)
-    : task_runner_(task_runner),
+    : params_(params),

[RyanSturm, 2017/05/22 19:45:04]

Do you plan to use this for other metrics? If not, just pass in
throughput_min_request_in_flight

[tbansal1, 2017/05/22 19:58:01]

Yes, I am going to do it for other metrics too.

[RyanSturm, 2017/05/22 20:14:40]

Acknowledged.

+      task_runner_(task_runner),
       throughput_observation_callback_(throughput_observation_callback),
       last_connection_change_(base::TimeTicks::Now()),
       window_start_time_(base::TimeTicks()),
       bits_received_at_window_start_(0),
       disable_throughput_measurements_(false),
       use_localhost_requests_for_tests_(use_local_host_requests_for_tests),
       use_small_responses_for_tests_(use_smaller_responses_for_tests) {
+  DCHECK(params_);
   DCHECK(task_runner_);
   DCHECK(!IsCurrentlyTrackingThroughput());
 }
 
 ThroughputAnalyzer::~ThroughputAnalyzer() {
   DCHECK(thread_checker_.CalledOnValidThread());
 }
 
 void ThroughputAnalyzer::MaybeStartThroughputObservationWindow() {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   if (disable_throughput_measurements_)
     return;
 
   // Throughput observation window can be started only if no accuracy degrading
   // requests are currently active, the observation window is not already
   // started, and there is at least one active request that does not degrade
   // throughput computation accuracy.
   if (accuracy_degrading_requests_.size() > 0 ||
-      IsCurrentlyTrackingThroughput() || requests_.size() <= 0) {
+      IsCurrentlyTrackingThroughput() ||
+      requests_.size() < params_->throughput_min_requests_in_flight()) {
     return;
   }
   window_start_time_ = base::TimeTicks::Now();
   bits_received_at_window_start_ = GetBitsReceived();
 }
 
 void ThroughputAnalyzer::EndThroughputObservationWindow() {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   // Mark the throughput observation window as stopped by resetting the window
   // parameters.
   window_start_time_ = base::TimeTicks();
   bits_received_at_window_start_ = 0;
 }
 
 bool ThroughputAnalyzer::IsCurrentlyTrackingThroughput() const {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   if (window_start_time_.is_null())
     return false;
 
   // If the throughput observation window is running, then at least one request
   // that does not degrade throughput computation accuracy should be active.
   DCHECK_GT(requests_.size(), 0U);
 
   // If the throughput observation window is running, then no accuracy degrading
   // requests should be currently active.
   DCHECK_EQ(0U, accuracy_degrading_requests_.size());
 
+  DCHECK_LE(params_->throughput_min_requests_in_flight(), requests_.size());
+
   return true;
 }
 
 void ThroughputAnalyzer::NotifyStartTransaction(const URLRequest& request) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   if (disable_throughput_measurements_)
     return;
 
   const bool degrades_accuracy = DegradesAccuracy(request);
@@ skipping 25 matching lines @@
 
   // Return early if the |request| is not present in the collections of
   // requests. This may happen when a completed request is later destroyed.
   if (requests_.find(&request) == requests_.end() &&
       accuracy_degrading_requests_.find(&request) ==
           accuracy_degrading_requests_.end()) {
     return;
   }
 
   int32_t downstream_kbps;
   if (MayBeGetThroughputObservation(&downstream_kbps)) {

[RyanSturm, 2017/05/22 20:14:40]

nit please: Change the name of this to
s/MayBeGetThroughputObservation/MaybeGetThroughputObservation/

Thanks

[tbansal1, 2017/05/22 22:51:55]

Done.

     // Notify the provided callback.
     task_runner_->PostTask(
         FROM_HERE,
         base::Bind(throughput_observation_callback_, downstream_kbps));
   }
 
   // Try to remove the request from either |accuracy_degrading_requests_| or
   // |requests_|, since it is no longer active.
   if (accuracy_degrading_requests_.erase(&request) == 1u) {
     // |request| cannot be in both |accuracy_degrading_requests_| and
     // |requests_| at the same time.
     DCHECK(requests_.end() == requests_.find(&request));
 
     // If a request that degraded the accuracy of throughput computation has
     // completed, then it may be possible to start the tracking window.
     MaybeStartThroughputObservationWindow();
     return;
   }
 
   if (requests_.erase(&request) == 1u) {
     // If there is no network activity, stop tracking throughput to prevent
     // recording of any observations.
-    if (requests_.size() == 0)
+    if (requests_.size() < params_->throughput_min_requests_in_flight())
       EndThroughputObservationWindow();
     return;
   }
   // |request| must be either in |accuracy_degrading_requests_| or |requests_|.
   NOTREACHED();
 }
 
 bool ThroughputAnalyzer::MayBeGetThroughputObservation(
     int32_t* downstream_kbps) {
   DCHECK(thread_checker_.CalledOnValidThread());
@@ skipping 112 matching lines @@
     // TODO(tbansal): crbug.com/609174 Add UMA to record how frequently this
     // happens.
   }
 }
 
 }  // namespace internal
 
 }  // namespace nqe
 
 }  // namespace net