Add UMA that periodically logs number of outstanding requests

content/browser/loader/resource_dispatcher_host_impl.cc

Index: content/browser/loader/resource_dispatcher_host_impl.cc
diff --git a/content/browser/loader/resource_dispatcher_host_impl.cc b/content/browser/loader/resource_dispatcher_host_impl.cc
index 39108877eb551027f55ebfd7e6d29a13dc89690d..580324d1eeaaf9901e198a9ac485948301dfec93 100644
--- a/content/browser/loader/resource_dispatcher_host_impl.cc
+++ b/content/browser/loader/resource_dispatcher_host_impl.cc
@@ -168,6 +168,9 @@ static ResourceDispatcherHostImpl* g_resource_dispatcher_host;
 // The interval for calls to ResourceDispatcherHostImpl::UpdateLoadStates
 const int kUpdateLoadStatesIntervalMsec = 250;
 
+// The interval for calls to RecordOutstandingRequestsStats.
+const int kRecordOutstandingRequestsStatsIntervalMsec = 60000;
+
 // Maximum byte "cost" of all the outstanding requests for a renderer.
 // See declaration of |max_outstanding_requests_cost_per_process_| for details.
 // This bound is 25MB, which allows for around 6000 outstanding requests.
@@ -339,6 +342,8 @@ ResourceDispatcherHostImpl::ResourceDispatcherHostImpl(
           kMaxOutstandingRequestsCostPerProcess),
       largest_outstanding_request_count_seen_(0),
       largest_outstanding_request_per_process_count_seen_(0),
+      peak_outstanding_request_count_(0),
+      peak_outstanding_request_count_multitab_(0),
       delegate_(nullptr),
       loader_delegate_(nullptr),
       allow_cross_origin_auth_prompt_(false),
@@ -361,6 +366,7 @@ ResourceDispatcherHostImpl::ResourceDispatcherHostImpl(
       base::Bind(&ResourceDispatcherHostImpl::OnInit, base::Unretained(this)));
 
   update_load_states_timer_.reset(new base::RepeatingTimer());
+  record_outstanding_requests_stats_timer_.reset(new base::RepeatingTimer());
 }
 
 // The default ctor is only used by unittests. It is reasonable to assume that
@@ -589,13 +595,20 @@ bool ResourceDispatcherHostImpl::HandleExternalProtocol(ResourceLoader* loader,
 }
 
 void ResourceDispatcherHostImpl::DidStartRequest(ResourceLoader* loader) {
-  // Make sure we have the load state monitor running.
+  // Make sure we have the load state monitors running.
   if (!update_load_states_timer_->IsRunning() &&
       scheduler_->HasLoadingClients()) {
     update_load_states_timer_->Start(
         FROM_HERE, TimeDelta::FromMilliseconds(kUpdateLoadStatesIntervalMsec),
         this, &ResourceDispatcherHostImpl::UpdateLoadInfo);
   }
+  if (!record_outstanding_requests_stats_timer_->IsRunning()) {
+    record_outstanding_requests_stats_timer_->Start(
+        FROM_HERE,
+        TimeDelta::FromMilliseconds(
+            kRecordOutstandingRequestsStatsIntervalMsec),
+        this, &ResourceDispatcherHostImpl::RecordOutstandingRequestsStats);
+  }
 }
 
 void ResourceDispatcherHostImpl::DidReceiveRedirect(
@@ -784,10 +797,11 @@ void ResourceDispatcherHostImpl::OnShutdown() {
   is_shutdown_ = true;
   pending_loaders_.clear();
 
-  // Make sure we shutdown the timer now, otherwise by the time our destructor
+  // Make sure we shutdown the timers now, otherwise by the time our destructor
   // runs if the timer is still running the Task is deleted twice (once by
   // the MessageLoop and the second time by RepeatingTimer).
   update_load_states_timer_.reset();
+  record_outstanding_requests_stats_timer_.reset();
 
   // Clear blocked requests if any left.
   // Note that we have to do this in 2 passes as we cannot call
@@ -1969,6 +1983,15 @@ ResourceDispatcherHostImpl::IncrementOutstandingRequestsCount(
         largest_outstanding_request_per_process_count_seen_);
   }
 
+  if (num_in_flight_requests_ > peak_outstanding_request_count_) {
+    peak_outstanding_request_count_ = num_in_flight_requests_;
+  }
+
+  if (HasRequestsFromMultipleActiveProcesses() &&
+      num_in_flight_requests_ > peak_outstanding_request_count_multitab_) {
+    peak_outstanding_request_count_multitab_ = num_in_flight_requests_;
+  }
+
   return stats;
 }
 
@@ -2497,6 +2520,23 @@ void ResourceDispatcherHostImpl::UpdateLoadInfo() {
       base::Bind(UpdateLoadStateOnUI, loader_delegate_, base::Passed(&infos)));
 }
 
+void ResourceDispatcherHostImpl::RecordOutstandingRequestsStats() {
+  if (peak_outstanding_request_count_ != 0) {
+    UMA_HISTOGRAM_COUNTS_1M(
+        "Net.ResourceDispatcherHost.PeakOutstandingRequests",
+        peak_outstanding_request_count_);
+    peak_outstanding_request_count_ = num_in_flight_requests_;

[kinuko, 2017/06/21 04:00:10]

Intuitively I feel this should be reset to 0 because the current
num_in_flight_requests is already rolled into the previous shot?

[Kunihiko Sakamoto, 2017/06/21 05:25:32]

My intuition is that if there are n outstanding requests at the beginning of the
time interval, the peak of the interval shouldn't be less than n.
(Edge case: if there's no inc/dec event in the period, the peak should be n
instead of 0.)

[kinuko, 2017/06/21 07:06:48]

Hm, ok I think that makes sense.

+  }
+
+  if (peak_outstanding_request_count_multitab_ != 0) {
+    UMA_HISTOGRAM_COUNTS_1M(
+        "Net.ResourceDispatcherHost.PeakOutstandingRequests.MultiTabLoading",

[kinuko, 2017/06/21 04:00:10]

The map we rely on groups requests per-process rather than per-tab, so calling
this 'MultiTab' feels a bit misleading...

[Kunihiko Sakamoto, 2017/06/21 05:25:32]

Yeah, you're right. Renamed the suffix to 'MultipleLoadingProcesses'.

[kinuko, 2017/06/21 07:06:48]

We could use routing_id to gather per-tab stats, how much more code would we
need if we want to have that data?  Clicking to open multiple tabs from single
tab could easily end up with having all tabs within the same process (e.g. if
rel=noopener is specified).  I assume we need to add similar map code to the
per-process one then.

(We could wait for csharrison/Helen's response to see their gut feeling as well)

+        peak_outstanding_request_count_multitab_);
+    peak_outstanding_request_count_multitab_ =
+        HasRequestsFromMultipleActiveProcesses() ? num_in_flight_requests_ : 0;
+  }
+}
+
 void ResourceDispatcherHostImpl::BlockRequestsForRoute(
     const GlobalFrameRoutingId& global_routing_id) {
   DCHECK(io_thread_task_runner_->BelongsToCurrentThread());
@@ -2706,4 +2746,20 @@ ResourceDispatcherHostImpl::HandleDownloadStarted(
   return handler;
 }
 
+bool ResourceDispatcherHostImpl::HasRequestsFromMultipleActiveProcesses() {
+  if (outstanding_requests_stats_map_.size() < 2)
+    return false;
+
+  int active_processes = 0;
+  for (auto iter = outstanding_requests_stats_map_.begin();
+       iter != outstanding_requests_stats_map_.end(); ++iter) {
+    if (iter->second.num_requests > 2) {
+      active_processes++;
+      if (active_processes >= 2)
+        return true;
+    }
+  }
+  return false;
+}
+
 }  // namespace content