Add JobController/Job count to UMA histograms when the count hits limit

net/http/http_stream_factory_impl.cc

 // Copyright (c) 2012 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/http/http_stream_factory_impl.h"
 
 #include <string>
 #include <tuple>
 
 #include "base/logging.h"
@@ skipping 89 matching lines @@
 }  // anonymous namespace
 
 HttpStreamFactoryImpl::HttpStreamFactoryImpl(HttpNetworkSession* session,
                                              bool for_websockets)
     : session_(session),
       job_factory_(new DefaultJobFactory()),
       for_websockets_(for_websockets) {}
 
 HttpStreamFactoryImpl::~HttpStreamFactoryImpl() {
   DCHECK(spdy_session_request_map_.empty());
-  int alt_job_count = 0;
-  int main_job_count = 0;
-  int preconnect_controller_count = 0;
-  for (const auto& it : job_controller_set_) {
-    DCHECK(it->HasPendingAltJob() || it->HasPendingMainJob());
-    // For a preconnect controller, it should have exactly the main job.
-    if (it->is_preconnect()) {
-      preconnect_controller_count++;
-      continue;
-    }
-    // For non-preconnects.
-    if (it->HasPendingAltJob())
-      alt_job_count++;
-    if (it->HasPendingMainJob())
-      main_job_count++;
-  }
-  UMA_HISTOGRAM_COUNTS_1M("Net.JobControllerSet.CountOfPreconnect",
-                          preconnect_controller_count);
-  UMA_HISTOGRAM_COUNTS_1M("Net.JobControllerSet.CountOfNonPreconnectAltJob",
-                          alt_job_count);
-  UMA_HISTOGRAM_COUNTS_1M("Net.JobControllerSet.CountOfNonPreconnectMainJob",
-                          main_job_count);
+  AddJobControllerCountToHistograms();
 }
 
 HttpStreamRequest* HttpStreamFactoryImpl::RequestStream(
     const HttpRequestInfo& request_info,
     RequestPriority priority,
     const SSLConfig& server_ssl_config,
     const SSLConfig& proxy_ssl_config,
     HttpStreamRequest::Delegate* delegate,
     bool enable_ip_based_pooling,
     bool enable_alternative_services,
@@ skipping 46 matching lines @@
     RequestPriority priority,
     const SSLConfig& server_ssl_config,
     const SSLConfig& proxy_ssl_config,
     HttpStreamRequest::Delegate* delegate,
     WebSocketHandshakeStreamBase::CreateHelper*
         websocket_handshake_stream_create_helper,
     HttpStreamRequest::StreamType stream_type,
     bool enable_ip_based_pooling,
     bool enable_alternative_services,
     const NetLogWithSource& net_log) {
+  AddJobControllerCountToHistograms();
+
   auto job_controller = base::MakeUnique<JobController>(
       this, delegate, session_, job_factory_.get(), request_info,
       /* is_preconnect = */ false, enable_ip_based_pooling,
       enable_alternative_services);
   JobController* job_controller_raw_ptr = job_controller.get();
   job_controller_set_.insert(std::move(job_controller));
   Request* request = job_controller_raw_ptr->Start(
       request_info, delegate, websocket_handshake_stream_create_helper, net_log,
       stream_type, priority, server_ssl_config, proxy_ssl_config);
 
   return request;
 }
 
 void HttpStreamFactoryImpl::PreconnectStreams(
     int num_streams,
     const HttpRequestInfo& request_info) {
+  AddJobControllerCountToHistograms();
+
   SSLConfig server_ssl_config;
   SSLConfig proxy_ssl_config;
   session_->GetSSLConfig(request_info, &server_ssl_config, &proxy_ssl_config);
   // All preconnects should perform EV certificate verification.
   server_ssl_config.verify_ev_cert = true;
   proxy_ssl_config.verify_ev_cert = true;
 
   DCHECK(!for_websockets_);
 
   auto job_controller = base::MakeUnique<JobController>(
@@ skipping 141 matching lines @@
   HostPortPair host_port_pair = proxy_info.proxy_server().host_port_pair();
   DCHECK(!host_port_pair.IsEmpty());
 
   url::SchemeHostPort scheme_host_port("https", host_port_pair.host(),
                                        host_port_pair.port());
 
   return session_->http_server_properties()->SupportsRequestPriority(
       scheme_host_port);
 }
 
+void HttpStreamFactoryImpl::AddJobControllerCountToHistograms() const {
+  int alt_job_count = 0;
+  int main_job_count = 0;
+  int preconnect_controller_count = 0;
+  for (const auto& it : job_controller_set_) {

[Ryan Hamilton, 2017/04/07 22:51:05]

nit: I think "it" is not an iterator, but rather a JobController, right? perhaps
controller would be a better name?

+    DCHECK(it->HasPendingAltJob() || it->HasPendingMainJob());
+    // For a preconnect controller, it should have exactly the main job.
+    if (it->is_preconnect()) {
+      preconnect_controller_count++;
+      continue;
+    }
+    // For non-preconnects.
+    if (it->HasPendingAltJob())
+      alt_job_count++;
+    if (it->HasPendingMainJob())
+      main_job_count++;
+  }
+  UMA_HISTOGRAM_COUNTS_1M("Net.JobControllerSet.CountOfPreconnect",
+                          preconnect_controller_count);
+  UMA_HISTOGRAM_COUNTS_1M("Net.JobControllerSet.CountOfNonPreconnectAltJob",
+                          alt_job_count);
+  UMA_HISTOGRAM_COUNTS_1M("Net.JobControllerSet.CountOfNonPreconnectMainJob",
+                          main_job_count);

[Ryan Hamilton, 2017/04/07 22:51:05]

it's a bit of a bummer that we have to compute these numbers each time. I don't
suppose there's any obvious way to cache/precomute this?

We could easily simply log the size of job_controller_set_. I guess the
controller could also call some method on the factory when it creates/deletes a
job, but that's probably not terribly fool proof.

Well, this definitely seems like a step forward, but its worth thinking about
this.

[Zhongyi Shi, 2017/04/07 23:43:02]

Well, if we want to improve: I guess one way we could do is to use maps to hold
job controllers and differentiate job controllers by its status.

We could separate the HttpStreamFactoryImpl::job_controller_set_: 
1. normal_job_controllers: JobController has main and alt job, and JobController
that only no alt job from the beginning.
2. preconnect_job_controllers: JobController that is used for preconnect
3. job_controllers_with_pending_main_job: JobController that used to have 2
jobs, but now only main job
4. job_controllers_with_pending_alt_job: JobController that used to have 2 jobs,
but now only alt job

When alt job or main job finishes, we need to move the JobController around to
the correct map. 3 and 4 are what we care more I think. WDYT?

+}
+
 void HttpStreamFactoryImpl::DumpMemoryStats(
     base::trace_event::ProcessMemoryDump* pmd,
     const std::string& parent_absolute_name) const {
   if (job_controller_set_.empty())
     return;
   std::string name =
       base::StringPrintf("%s/stream_factory", parent_absolute_name.c_str());
   base::trace_event::MemoryAllocatorDump* factory_dump =
       pmd->CreateAllocatorDump(name);
   size_t alt_job_count = 0;
@@ skipping 27 matching lines @@
   factory_dump->AddScalar("main_job_count",
                           base::trace_event::MemoryAllocatorDump::kUnitsObjects,
                           main_job_count);
   // The number of preconnect controllers.
   factory_dump->AddScalar("preconnect_count",
                           base::trace_event::MemoryAllocatorDump::kUnitsObjects,
                           preconnect_controller_count);
 }
 
 }  // namespace net