Cache net::HostPortPair in ResourceScheduler::ScheduledResourceRequest

content/browser/loader/resource_scheduler.cc

Index: content/browser/loader/resource_scheduler.cc
diff --git a/content/browser/loader/resource_scheduler.cc b/content/browser/loader/resource_scheduler.cc
index c977bd5cc93c0e6df64dc6664c3ff28c294f5bbc..84bc054349faf8196d4977ca9a4c9f01f4345036 100644
--- a/content/browser/loader/resource_scheduler.cc
+++ b/content/browser/loader/resource_scheduler.cc
@@ -178,6 +178,7 @@ class ResourceScheduler::ScheduledResourceRequest : public ResourceThrottle {
         scheduler_(scheduler),
         priority_(priority),
         fifo_ordering_(0),
+        host_port_pair_(net::HostPortPair::FromURL(request->url())),
         weak_ptr_factory_(this) {
     DCHECK(!request_->GetUserData(kUserDataKey));
     request_->SetUserData(kUserDataKey, new UnownedPointer(this));
@@ -243,6 +244,7 @@ class ResourceScheduler::ScheduledResourceRequest : public ResourceThrottle {
   void set_attributes(RequestAttributes attributes) {
     attributes_ = attributes;
   }
+  const net::HostPortPair host_port_pair() { return host_port_pair_; }

[mmenke, 2016/11/10 19:11:37]

return by reference?

[mmenke, 2016/11/10 19:15:26]

Also +1 const:

const net::HostPortPair& host_port_pair() const {

[Charlie Harrison, 2016/11/10 19:15:41]

Oops, typo. Done :)

[Charlie Harrison, 2016/11/10 19:17:10]

Done.

 
  private:
   class UnownedPointer : public base::SupportsUserData::Data {
@@ -276,6 +278,8 @@ class ResourceScheduler::ScheduledResourceRequest : public ResourceThrottle {
   ResourceScheduler* scheduler_;
   RequestPriorityParams priority_;
   uint32_t fifo_ordering_;
+  // Cached to excessive recomputation in ShouldKeepSearching.
+  const net::HostPortPair host_port_pair_;
 
   base::WeakPtrFactory<ResourceScheduler::ScheduledResourceRequest>
       weak_ptr_factory_;
@@ -546,8 +550,7 @@ class ResourceScheduler::Client {
     size_t same_host_count = 0;
     for (RequestSet::const_iterator it = in_flight_requests_.begin();
          it != in_flight_requests_.end(); ++it) {
-      net::HostPortPair host_port_pair =
-          net::HostPortPair::FromURL((*it)->url_request()->url());
+      net::HostPortPair host_port_pair = (*it)->host_port_pair();

[mmenke, 2016/11/10 19:11:37]

Store as a const ref, or just inline?  Not sure how smart compilers are, but
best to play it safe.

[Charlie Harrison, 2016/11/10 19:15:41]

Done.

       if (active_request_host.Equals(host_port_pair)) {
         same_host_count++;
         if (same_host_count >= kMaxNumDelayableRequestsPerHostPerClient)
@@ -610,8 +613,7 @@ class ResourceScheduler::Client {
     if (!url_request.url().SchemeIsHTTPOrHTTPS())
       return START_REQUEST;
 
-    net::HostPortPair host_port_pair =
-        net::HostPortPair::FromURL(url_request.url());
+    net::HostPortPair host_port_pair = request->host_port_pair();

[Charlie Harrison, 2016/11/10 19:15:41]

I also made this const &.

 
     if (!priority_requests_delayable_) {
       if (using_spdy_proxy_ && url_request.url().SchemeIs(url::kHttpScheme))