Commit 3fcbcc19 authored by xunjieli's avatar xunjieli Committed by Commit bot

Add UMA to track the number of live URLRequests.

UMA data shows that there can be a large number of active
HttpStreamRequests at a given time. The number reported by
ResourceDispatcherHostImpl doesn't account for the large number of
HttpStreamRequests. This CL adds a histogram to track the number of
active URLRequests, so we can know whether the cause for the large
number of HttpStreamRequests is internal or external to the network
stack.

BUG=711721

Review-Url: https://codereview.chromium.org/2837313002
Cr-Commit-Position: refs/heads/master@{#467332}
parent d434dda5
...@@ -515,7 +515,7 @@ NET_EXPORT void CreateNetLogEntriesForActiveObjects( ...@@ -515,7 +515,7 @@ NET_EXPORT void CreateNetLogEntriesForActiveObjects(
DCHECK(context->CalledOnValidThread()); DCHECK(context->CalledOnValidThread());
// Contexts should all be using the same NetLog. // Contexts should all be using the same NetLog.
DCHECK_EQ((*contexts.begin())->net_log(), context->net_log()); DCHECK_EQ((*contexts.begin())->net_log(), context->net_log());
for (auto* request : *context->url_requests()) { for (auto* request : context->url_requests()) {
requests.push_back(request); requests.push_back(request);
} }
} }
......
...@@ -186,8 +186,7 @@ URLRequest::~URLRequest() { ...@@ -186,8 +186,7 @@ URLRequest::~URLRequest() {
// on UserData associated with |this| and poke at it during teardown. // on UserData associated with |this| and poke at it during teardown.
job_.reset(); job_.reset();
DCHECK_EQ(1u, context_->url_requests()->count(this)); context_->RemoveURLRequest(this);
context_->url_requests()->erase(this);
int net_error = OK; int net_error = OK;
// Log error only on failure, not cancellation, as even successful requests // Log error only on failure, not cancellation, as even successful requests
...@@ -585,7 +584,7 @@ URLRequest::URLRequest(const GURL& url, ...@@ -585,7 +584,7 @@ URLRequest::URLRequest(const GURL& url,
// Sanity check out environment. // Sanity check out environment.
DCHECK(base::ThreadTaskRunnerHandle::IsSet()); DCHECK(base::ThreadTaskRunnerHandle::IsSet());
context->url_requests()->insert(this); context->InsertURLRequest(this);
net_log_.BeginEvent( net_log_.BeginEvent(
NetLogEventType::REQUEST_ALIVE, NetLogEventType::REQUEST_ALIVE,
base::Bind(&NetLogURLRequestConstructorCallback, &url, priority_)); base::Bind(&NetLogURLRequestConstructorCallback, &url, priority_));
......
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
#include "base/compiler_specific.h" #include "base/compiler_specific.h"
#include "base/debug/alias.h" #include "base/debug/alias.h"
#include "base/memory/ptr_util.h" #include "base/memory/ptr_util.h"
#include "base/metrics/histogram_macros.h"
#include "base/strings/string_util.h" #include "base/strings/string_util.h"
#include "base/strings/stringprintf.h" #include "base/strings/stringprintf.h"
#include "base/threading/thread_task_runner_handle.h" #include "base/threading/thread_task_runner_handle.h"
...@@ -48,10 +49,10 @@ URLRequestContext::URLRequestContext() ...@@ -48,10 +49,10 @@ URLRequestContext::URLRequestContext()
sdch_manager_(nullptr), sdch_manager_(nullptr),
network_quality_estimator_(nullptr), network_quality_estimator_(nullptr),
reporting_service_(nullptr), reporting_service_(nullptr),
url_requests_(new std::set<const URLRequest*>),
enable_brotli_(false), enable_brotli_(false),
check_cleartext_permitted_(false), check_cleartext_permitted_(false),
name_(nullptr) { name_(nullptr),
largest_outstanding_requests_count_seen_(0) {
base::trace_event::MemoryDumpManager::GetInstance()->RegisterDumpProvider( base::trace_event::MemoryDumpManager::GetInstance()->RegisterDumpProvider(
this, "URLRequestContext", base::ThreadTaskRunnerHandle::Get()); this, "URLRequestContext", base::ThreadTaskRunnerHandle::Get());
} }
...@@ -122,13 +123,27 @@ void URLRequestContext::set_cookie_store(CookieStore* cookie_store) { ...@@ -122,13 +123,27 @@ void URLRequestContext::set_cookie_store(CookieStore* cookie_store) {
cookie_store_ = cookie_store; cookie_store_ = cookie_store;
} }
void URLRequestContext::InsertURLRequest(const URLRequest* request) const {
url_requests_.insert(request);
if (url_requests_.size() > largest_outstanding_requests_count_seen_) {
largest_outstanding_requests_count_seen_ = url_requests_.size();
UMA_HISTOGRAM_COUNTS_1M("Net.URLRequestContext.OutstandingRequests",
largest_outstanding_requests_count_seen_);
}
}
void URLRequestContext::RemoveURLRequest(const URLRequest* request) const {
DCHECK_EQ(1u, url_requests_.count(request));
url_requests_.erase(request);
}
void URLRequestContext::AssertNoURLRequests() const { void URLRequestContext::AssertNoURLRequests() const {
int num_requests = url_requests_->size(); int num_requests = url_requests_.size();
if (num_requests != 0) { if (num_requests != 0) {
// We're leaking URLRequests :( Dump the URL of the first one and record how // We're leaking URLRequests :( Dump the URL of the first one and record how
// many we leaked so we have an idea of how bad it is. // many we leaked so we have an idea of how bad it is.
char url_buf[128]; char url_buf[128];
const URLRequest* request = *url_requests_->begin(); const URLRequest* request = *url_requests_.begin();
base::strlcpy(url_buf, request->url().spec().c_str(), arraysize(url_buf)); base::strlcpy(url_buf, request->url().spec().c_str(), arraysize(url_buf));
int load_flags = request->load_flags(); int load_flags = request->load_flags();
base::debug::Alias(url_buf); base::debug::Alias(url_buf);
...@@ -154,7 +169,7 @@ bool URLRequestContext::OnMemoryDump( ...@@ -154,7 +169,7 @@ bool URLRequestContext::OnMemoryDump(
pmd->CreateAllocatorDump(dump_name); pmd->CreateAllocatorDump(dump_name);
dump->AddScalar(base::trace_event::MemoryAllocatorDump::kNameObjectCount, dump->AddScalar(base::trace_event::MemoryAllocatorDump::kNameObjectCount,
base::trace_event::MemoryAllocatorDump::kUnitsObjects, base::trace_event::MemoryAllocatorDump::kUnitsObjects,
url_requests_->size()); url_requests_.size());
HttpTransactionFactory* transaction_factory = http_transaction_factory(); HttpTransactionFactory* transaction_factory = http_transaction_factory();
if (transaction_factory) { if (transaction_factory) {
HttpNetworkSession* network_session = transaction_factory->GetSession(); HttpNetworkSession* network_session = transaction_factory->GetSession();
......
...@@ -221,10 +221,14 @@ class NET_EXPORT URLRequestContext ...@@ -221,10 +221,14 @@ class NET_EXPORT URLRequestContext
// Gets the URLRequest objects that hold a reference to this // Gets the URLRequest objects that hold a reference to this
// URLRequestContext. // URLRequestContext.
std::set<const URLRequest*>* url_requests() const { const std::set<const URLRequest*>& url_requests() const {
return url_requests_.get(); return url_requests_;
} }
void InsertURLRequest(const URLRequest* request) const;
void RemoveURLRequest(const URLRequest* request) const;
// CHECKs that no URLRequests using this context remain. Subclasses should // CHECKs that no URLRequests using this context remain. Subclasses should
// additionally call AssertNoURLRequests() within their own destructor, // additionally call AssertNoURLRequests() within their own destructor,
// prior to implicit destruction of subclass-owned state. // prior to implicit destruction of subclass-owned state.
...@@ -312,7 +316,7 @@ class NET_EXPORT URLRequestContext ...@@ -312,7 +316,7 @@ class NET_EXPORT URLRequestContext
// be added to CopyFrom. // be added to CopyFrom.
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------
std::unique_ptr<std::set<const URLRequest*>> url_requests_; mutable std::set<const URLRequest*> url_requests_;
// Enables Brotli Content-Encoding support. // Enables Brotli Content-Encoding support.
bool enable_brotli_; bool enable_brotli_;
...@@ -325,6 +329,10 @@ class NET_EXPORT URLRequestContext ...@@ -325,6 +329,10 @@ class NET_EXPORT URLRequestContext
// to be unique. // to be unique.
const char* name_; const char* name_;
// The largest number of outstanding URLRequests that have been created by
// |this| and are not yet destroyed. This doesn't need to be in CopyFrom.
mutable size_t largest_outstanding_requests_count_seen_;
DISALLOW_COPY_AND_ASSIGN(URLRequestContext); DISALLOW_COPY_AND_ASSIGN(URLRequestContext);
}; };
......
...@@ -39229,6 +39229,14 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries. ...@@ -39229,6 +39229,14 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
<summary>True if a URLRequest's referrer is empty or valid when set.</summary> <summary>True if a URLRequest's referrer is empty or valid when set.</summary>
</histogram> </histogram>
<histogram name="Net.URLRequestContext.OutstandingRequests" units="count">
<owner>xunjieli@chromium.org</owner>
<summary>
Indicates the number of URLRequests that are handed out by a
URLRequestContext and are not yet destroyed.
</summary>
</histogram>
<histogram name="Net.ValidDNSName" enum="Boolean"> <histogram name="Net.ValidDNSName" enum="Boolean">
<owner>palmer@chromium.org</owner> <owner>palmer@chromium.org</owner>
<summary> <summary>
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment