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..2d974490d89b35d95e178a87461f279a502ee838 100644
--- a/content/browser/loader/resource_dispatcher_host_impl.cc
+++ b/content/browser/loader/resource_dispatcher_host_impl.cc
@@ -78,6 +78,7 @@
 #include "content/common/resource_request.h"
 #include "content/common/resource_request_body_impl.h"
 #include "content/common/resource_request_completion_status.h"
+#include "content/common/site_isolation_policy.h"
 #include "content/common/view_messages.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/global_request_id.h"
@@ -168,6 +169,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 kRecordOutstandingRequestsStatsIntervalSec = 60;
+
 // 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.
@@ -360,7 +364,16 @@ ResourceDispatcherHostImpl::ResourceDispatcherHostImpl(
       FROM_HERE,
       base::Bind(&ResourceDispatcherHostImpl::OnInit, base::Unretained(this)));
 
-  update_load_states_timer_.reset(new base::RepeatingTimer());
+  update_load_states_timer_ = base::MakeUnique<base::RepeatingTimer>();
+
+  // Monitor per-tab outstanding requests only if OOPIF is not enabled, because
+  // the routing id doesn't represent tabs in OOPIF modes.
+  if (!SiteIsolationPolicy::UseDedicatedProcessesForAllSites() &&
+      !SiteIsolationPolicy::IsTopDocumentIsolationEnabled() &&
+      !SiteIsolationPolicy::AreIsolatedOriginsEnabled()) {

[alexmos, 2017/06/30 17:27:22]

Just saw this CL, and this check seems problematic.  OOPIFs have already been
shipped and enabled by default for Isolate Extensions, so this isn't going to
avoid all OOPIF scenarios.  This also doesn't check for oopif-based webview
(features::kGuestViewCrossProcessFrames), which relies on some OOPIF plumbing
and is an experiment on canary/dev/beta and likely to ship in M61.  This will
also be problematic when we start baking certain OOPIF-enabled isolated origins
into Chrome, which will likely happen within a few weeks.

+    record_outstanding_requests_stats_timer_ =
+        base::MakeUnique<base::RepeatingTimer>();
+  }
 }
 
 // The default ctor is only used by unittests. It is reasonable to assume that
@@ -371,6 +384,7 @@ ResourceDispatcherHostImpl::ResourceDispatcherHostImpl()
 
 ResourceDispatcherHostImpl::~ResourceDispatcherHostImpl() {
   DCHECK(outstanding_requests_stats_map_.empty());
+  DCHECK(outstanding_requests_per_tab_map_.empty());
   DCHECK(g_resource_dispatcher_host);
   DCHECK(main_thread_task_runner_->BelongsToCurrentThread());
   g_resource_dispatcher_host = NULL;
@@ -589,13 +603,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_ &&
+      !record_outstanding_requests_stats_timer_->IsRunning()) {
+    record_outstanding_requests_stats_timer_->Start(
+        FROM_HERE,
+        TimeDelta::FromSeconds(kRecordOutstandingRequestsStatsIntervalSec),
+        this, &ResourceDispatcherHostImpl::RecordOutstandingRequestsStats);
+  }
 }
 
 void ResourceDispatcherHostImpl::DidReceiveRedirect(
@@ -784,10 +805,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
@@ -1919,6 +1941,18 @@ void ResourceDispatcherHostImpl::UpdateOutstandingRequestsStats(
     outstanding_requests_stats_map_[info.GetChildID()] = stats;
 }
 
+void ResourceDispatcherHostImpl::IncrementOutstandingRequestsPerTab(
+    int count,
+    const ResourceRequestInfoImpl& info) {
+  auto key = std::make_pair(info.GetChildID(), info.GetRouteID());
+  OutstandingRequestsPerTabMap::iterator entry =
+      outstanding_requests_per_tab_map_.insert(std::make_pair(key, 0)).first;
+  entry->second += count;
+  DCHECK_GE(entry->second, 0);
+  if (entry->second == 0)
+    outstanding_requests_per_tab_map_.erase(entry);
+}
+
 ResourceDispatcherHostImpl::OustandingRequestsStats
 ResourceDispatcherHostImpl::IncrementOutstandingRequestsMemory(
     int count,
@@ -1954,6 +1988,8 @@ ResourceDispatcherHostImpl::IncrementOutstandingRequestsCount(
   DCHECK_GE(stats.num_requests, 0);
   UpdateOutstandingRequestsStats(*info, stats);
 
+  IncrementOutstandingRequestsPerTab(count, *info);
+
   if (num_in_flight_requests_ > largest_outstanding_request_count_seen_) {
     largest_outstanding_request_count_seen_ = num_in_flight_requests_;
     UMA_HISTOGRAM_COUNTS_1M(
@@ -1969,6 +2005,14 @@ 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 (HasRequestsFromMultipleActiveTabs() &&
+      num_in_flight_requests_ > peak_outstanding_request_count_multitab_) {
+    peak_outstanding_request_count_multitab_ = num_in_flight_requests_;
+  }
+
   return stats;
 }
 
@@ -2497,6 +2541,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_;
+  }
+
+  if (peak_outstanding_request_count_multitab_ != 0) {
+    UMA_HISTOGRAM_COUNTS_1M(
+        "Net.ResourceDispatcherHost.PeakOutstandingRequests.MultiTabLoading",
+        peak_outstanding_request_count_multitab_);
+    peak_outstanding_request_count_multitab_ =
+        HasRequestsFromMultipleActiveTabs() ? num_in_flight_requests_ : 0;
+  }
+}
+
 void ResourceDispatcherHostImpl::BlockRequestsForRoute(
     const GlobalFrameRoutingId& global_routing_id) {
   DCHECK(io_thread_task_runner_->BelongsToCurrentThread());
@@ -2706,4 +2767,20 @@ ResourceDispatcherHostImpl::HandleDownloadStarted(
   return handler;
 }
 
+bool ResourceDispatcherHostImpl::HasRequestsFromMultipleActiveTabs() {
+  if (outstanding_requests_per_tab_map_.size() < 2)
+    return false;
+
+  int active_tabs = 0;
+  for (auto iter = outstanding_requests_per_tab_map_.begin();
+       iter != outstanding_requests_per_tab_map_.end(); ++iter) {
+    if (iter->second > 2) {
+      active_tabs++;
+      if (active_tabs >= 2)
+        return true;
+    }
+  }
+  return false;
+}
+
 }  // namespace content