Fix Resource Priorities and Scheduling (Chrome Side)

content/browser/loader/resource_scheduler.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 "content/browser/loader/resource_scheduler.h"
 #include <set>
 
-#include "content/browser/loader/resource_scheduler.h"
-
 #include "base/metrics/field_trial.h"
 #include "base/metrics/histogram.h"
 #include "base/stl_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_piece.h"
 #include "base/time/time.h"
 #include "content/common/resource_messages.h"
 #include "content/browser/loader/resource_message_delegate.h"
 #include "content/public/browser/resource_controller.h"
 #include "content/public/browser/resource_request_info.h"
@@ skipping 11 matching lines @@
 namespace {
 
 // Field trial constants
 const char kThrottleCoalesceFieldTrial[] = "RequestThrottlingAndCoalescing";
 const char kThrottleCoalesceFieldTrialThrottle[] = "Throttle";
 const char kThrottleCoalesceFieldTrialCoalesce[] = "Coalesce";
 
 const char kRequestLimitFieldTrial[] = "OutstandingRequestLimiting";
 const char kRequestLimitFieldTrialGroupPrefix[] = "Limit";
 
+const char kResourcePrioritiesFieldTrial[] = "ResourcePriorities";
+
 // Post ResourceScheduler histograms of the following forms:
 // If |histogram_suffix| is NULL or the empty string:
 //   ResourceScheduler.base_name.histogram_name
 // Else:
 //   ResourceScheduler.base_name.histogram_name.histogram_suffix
 void PostHistogram(const char* base_name,
                    const char* histogram_name,
                    const char* histogram_suffix,
                    base::TimeDelta time) {
   std::string histogram =
@@ skipping 17 matching lines @@
   else if (num_clients <= 15)
     return "Max15Clients";
   else if (num_clients <= 30)
     return "Max30Clients";
   return "Over30Clients";
 }
 
 }  // namespace
 
 static const size_t kCoalescedTimerPeriod = 5000;
-static const size_t kMaxNumDelayableRequestsPerClient = 10;
+static const size_t kDefaultMaxNumDelayableRequestsPerClient = 10;
 static const size_t kMaxNumDelayableRequestsPerHost = 6;
 static const size_t kMaxNumThrottledRequestsPerClient = 1;
+static const size_t kDefaultMaxNumDelayableWhileLayoutBlocking = 1;
+static const net::RequestPriority
+    kDefaultLayoutBlockingPriorityThreshold = net::LOW;
 
 struct ResourceScheduler::RequestPriorityParams {
   RequestPriorityParams()
     : priority(net::DEFAULT_PRIORITY),
       intra_priority(0) {
   }
 
   RequestPriorityParams(net::RequestPriority priority, int intra_priority)
     : priority(priority),
       intra_priority(intra_priority) {
@@ skipping 206 matching lines @@
   DCHECK(!ContainsKey(pointers_, request));
   request->set_fifo_ordering(MakeFifoOrderingId());
   pointers_[request] = queue_.insert(request);
 }
 
 // Each client represents a tab.
 class ResourceScheduler::Client {
  public:
   explicit Client(ResourceScheduler* scheduler,
                   bool is_visible,
-                  bool is_audible)
+                  bool is_audible,
+                  net::RequestPriority layout_blocking_priority_threshold,
+                  bool enable_layout_blocking_threshold,
+                  size_t in_flight_layout_blocking_threshold,
+                  size_t max_num_delayable_while_layout_blocking,
+                  size_t max_num_delayable_requests)
       : is_audible_(is_audible),
         is_visible_(is_visible),
         is_loaded_(false),
         is_paused_(false),
-        has_body_(false),
+        has_html_body_(false),
         using_spdy_proxy_(false),
         load_started_time_(base::TimeTicks::Now()),
         scheduler_(scheduler),
         in_flight_delayable_count_(0),
         total_layout_blocking_count_(0),
-        throttle_state_(ResourceScheduler::THROTTLED) {}
+        throttle_state_(ResourceScheduler::THROTTLED),
+        max_num_delayable_requests_(max_num_delayable_requests),
+        max_num_delayable_while_layout_blocking_(
+            max_num_delayable_while_layout_blocking),
+        enable_layout_blocking_threshold_(enable_layout_blocking_threshold),
+        in_flight_layout_blocking_threshold_(
+            in_flight_layout_blocking_threshold),
+        layout_blocking_priority_threshold_(
+            layout_blocking_priority_threshold) {
+  }
 
   ~Client() {
     // Update to default state and pause to ensure the scheduler has a
     // correct count of relevant types of clients.
     is_visible_ = false;
     is_audible_ = false;
     is_paused_ = true;
     UpdateThrottleState();
   }
 
@@ skipping 15 matching lines @@
       EraseInFlightRequest(request);
 
       // Removing this request may have freed up another to load.
       LoadAnyStartablePendingRequests();
     }
   }
 
   RequestSet StartAndRemoveAllRequests() {
     // First start any pending requests so that they will be moved into
     // in_flight_requests_. This may exceed the limits
-    // kMaxNumDelayableRequestsPerClient, kMaxNumDelayableRequestsPerHost and
-    // kMaxNumThrottledRequestsPerClient, so this method must not do anything
-    // that depends on those limits before calling ClearInFlightRequests()
-    // below.
+    // kDefaultMaxNumDelayableRequestsPerClient, kMaxNumDelayableRequestsPerHost
+    // and kMaxNumThrottledRequestsPerClient, so this method must not do
+    // anything that depends on those limits before calling
+    // ClearInFlightRequests() below.
     while (!pending_requests_.IsEmpty()) {
       ScheduledResourceRequest* request =
           *pending_requests_.GetNextHighestIterator();
       pending_requests_.Erase(request);
       // StartRequest() may modify pending_requests_. TODO(ricea): Does it?
       StartRequest(request);
     }
     RequestSet unowned_requests;
     for (RequestSet::iterator it = in_flight_requests_.begin();
          it != in_flight_requests_.end(); ++it) {
@@ skipping 96 matching lines @@
       scheduler_->DecrementActiveClientsLoading();
     }
     if (throttle_state_ == COALESCED) {
       scheduler_->IncrementCoalescedClients();
     } else if (old_throttle_state == COALESCED) {
       scheduler_->DecrementCoalescedClients();
     }
   }
 
   void OnNavigate() {
-    has_body_ = false;
+    has_html_body_ = false;
     is_loaded_ = false;
   }
 
   void OnWillInsertBody() {
-    has_body_ = true;
+    has_html_body_ = true;
     LoadAnyStartablePendingRequests();
   }
 
   void OnReceivedSpdyProxiedHttpResponse() {
     if (!using_spdy_proxy_) {
       using_spdy_proxy_ = true;
       LoadAnyStartablePendingRequests();
     }
   }
 
@@ skipping 129 matching lines @@
         CountRequestsWithClassification(IN_FLIGHT_DELAYABLE_REQUEST, false),
         in_flight_delayable_count_);
     DCHECK_EQ(CountRequestsWithClassification(LAYOUT_BLOCKING_REQUEST, true),
               total_layout_blocking_count_);
   }
 
   RequestClassification ClassifyRequest(ScheduledResourceRequest* request) {
     // If a request is already marked as layout-blocking make sure to keep the
     // classification across redirects unless the priority was lowered.
     if (request->classification() == LAYOUT_BLOCKING_REQUEST &&
-        request->url_request()->priority() > net::LOW) {
+        request->url_request()->priority() >
+        layout_blocking_priority_threshold_) {
       return LAYOUT_BLOCKING_REQUEST;
     }
 
-    if (!has_body_ && request->url_request()->priority() > net::LOW)
+    if (!has_html_body_ &&
+        request->url_request()->priority() >
+        layout_blocking_priority_threshold_) {
       return LAYOUT_BLOCKING_REQUEST;
+    }
 
-    if (request->url_request()->priority() < net::LOW) {
-      net::HostPortPair host_port_pair =
-          net::HostPortPair::FromURL(request->url_request()->url());
-      net::HttpServerProperties& http_server_properties =
-          *request->url_request()->context()->http_server_properties();
-      if (!http_server_properties.SupportsRequestPriority(host_port_pair) &&
-          ContainsKey(in_flight_requests_, request)) {
-        return IN_FLIGHT_DELAYABLE_REQUEST;
-      }
+    if (request->url_request()->priority() <
+        layout_blocking_priority_threshold_ &&
+        ContainsKey(in_flight_requests_, request)) {
+      return IN_FLIGHT_DELAYABLE_REQUEST;
     }
     return NORMAL_REQUEST;
   }
 
   bool ShouldKeepSearching(
       const net::HostPortPair& active_request_host) const {
     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 =
@@ skipping 108 matching lines @@
     }
 
     if (throttle_state_ == THROTTLED &&
         in_flight_requests_.size() >= kMaxNumThrottledRequestsPerClient) {
       // There may still be request-priority-capable requests that should be
       // issued.
       return DO_NOT_START_REQUEST_AND_KEEP_SEARCHING;
     }
 
     // High-priority and layout-blocking requests.
-    if (url_request.priority() >= net::LOW) {
+    if (url_request.priority() >= layout_blocking_priority_threshold_) {
       return START_REQUEST;
     }
 
-    if (in_flight_delayable_count_ >= kMaxNumDelayableRequestsPerClient) {
+    if (in_flight_delayable_count_ >= max_num_delayable_requests_) {
       return DO_NOT_START_REQUEST_AND_STOP_SEARCHING;
     }
 
     if (ShouldKeepSearching(host_port_pair)) {
       // There may be other requests for other hosts we'd allow,
       // so keep checking.
       return DO_NOT_START_REQUEST_AND_KEEP_SEARCHING;
     }
 
-    bool have_immediate_requests_in_flight =
-        in_flight_requests_.size() > in_flight_delayable_count_;
-    if (have_immediate_requests_in_flight &&
-        (!has_body_ || total_layout_blocking_count_ != 0) &&
-        // Do not allow a low priority request through in parallel if
-        // we are in a limit field trial.
-        (scheduler_->limit_outstanding_requests() ||
-         in_flight_delayable_count_ != 0)) {
-      return DO_NOT_START_REQUEST_AND_STOP_SEARCHING;
+    // Layout-blocking phase of resource loading.
+    if (!has_html_body_ || total_layout_blocking_count_ != 0) {
+      size_t non_delayable_requests_in_flight_count =
+          in_flight_requests_.size() - in_flight_delayable_count_;

[Bryan McQuade, 2015/08/11 13:25:07]

on first glance this is potentially scary since, if in_flight_delayable_count_
is ever greater than in_flight_requests_.size(), the size_t wraps which could
lead to unexpected behavior. i get the sense that it'd be a bug if this ever
happened, but just to be more explicit about it, can we add
CHECK(in_flight_requests_.size() >= in_flight_delayable_count_); right before
this as a sanity check that this should never happen?

[mmenke, 2015/08/11 13:35:34]

The only place we modify the classification of requests
(SetRequestClassification) already has some sanity DCHECKs.  I'm fine with
adding more, if we want better DCHECKs.  I don't think we want checks for sanity
here - dchecks should be where we actually modify in_flight_delayable_count_
(And possibly in_flight_requests_.size()) make a lot more sense as sanity
checks, since they give more useful information.

If we just want a DCHECK for documentation purposes here (To say "this is true,
we've thought of this), I am actually fine with that, just don't think it makes
sense as a sanity check.

Should not use CHECKs, unless there's some particular concern here, as they
bloat up binary size.

+      if (enable_layout_blocking_threshold_) {
+        if (non_delayable_requests_in_flight_count >
+            in_flight_layout_blocking_threshold_) {
+          // Too many higher priority in-flight requests to allow lower priority
+          // requests through.
+          return DO_NOT_START_REQUEST_AND_STOP_SEARCHING;
+        }
+        if (in_flight_requests_.size() > 0 &&
+            // Allow the request if nothing is in flight or if the limit of
+            // concurrent lower priority requests has not been hit.
+            (scheduler_->limit_outstanding_requests() ||
+             in_flight_delayable_count_ >=
+             max_num_delayable_while_layout_blocking_)) {
+          return DO_NOT_START_REQUEST_AND_STOP_SEARCHING;
+        }
+      } else if (non_delayable_requests_in_flight_count > 0 &&
+          // If there are no high-priority requests in flight the floodgates
+          // open.
+          // If there are high-priority requests in-flight then limit the number
+          // of lower-priority requests (or zero if a limit field trial is
+          // active).
+          (scheduler_->limit_outstanding_requests() ||
+           in_flight_delayable_count_ >=
+           max_num_delayable_while_layout_blocking_)) {
+        return DO_NOT_START_REQUEST_AND_STOP_SEARCHING;
+      }
     }
 
     return START_REQUEST;
   }
 
   void LoadAnyStartablePendingRequests() {
     // We iterate through all the pending requests, starting with the highest
     // priority one. For each entry, one of three things can happen:
     // 1) We start the request, remove it from the list, and keep checking.
     // 2) We do NOT start the request, but ShouldStartRequest() signals us that
@@ skipping 26 matching lines @@
         DCHECK(query_result == DO_NOT_START_REQUEST_AND_STOP_SEARCHING);
         break;
       }
     }
   }
 
   bool is_audible_;
   bool is_visible_;
   bool is_loaded_;
   bool is_paused_;
-  bool has_body_;
+  // Tracks if the main HTML parser has reached the body which marks the end of
+  // layout-blocking resources.
+  bool has_html_body_;
   bool using_spdy_proxy_;
   RequestQueue pending_requests_;
   RequestSet in_flight_requests_;
   base::TimeTicks load_started_time_;
   // The last time the client switched state between active and background.
   base::TimeTicks last_active_switch_time_;
   ResourceScheduler* scheduler_;
   // The number of delayable in-flight requests.
   size_t in_flight_delayable_count_;
   // The number of layout-blocking in-flight requests.
   size_t total_layout_blocking_count_;
   ResourceScheduler::ClientThrottleState throttle_state_;
+  // Maximum number of delayable requests to allow at a time, regardless of
+  // the number of servers (limit to X concurrent requests across all hosts).
+  size_t max_num_delayable_requests_;
+  // Maximum number of delayable requests to allow while in the layout-blocking
+  // phase of resource loading.
+  size_t max_num_delayable_while_layout_blocking_;
+  // Determines if in_flight_layout_blocking_threshold_ should be used.
+  bool enable_layout_blocking_threshold_;
+  // Number number of non-delayable requests above which no delayable requests
+  // should be issued while in the layout-blocking phase of resource loading.
+  size_t in_flight_layout_blocking_threshold_;
+  // Priority level above which resources are considered layout-blocking and
+  // below which they are considered delayable.
+  net::RequestPriority layout_blocking_priority_threshold_;
 };
 
 ResourceScheduler::ResourceScheduler()
     : should_coalesce_(false),
       should_throttle_(false),
       active_clients_loading_(0),
       coalesced_clients_(0),
       limit_outstanding_requests_(false),
       outstanding_request_limit_(0),
+      layout_blocking_priority_threshold_(
+          kDefaultLayoutBlockingPriorityThreshold),
+      enable_layout_blocking_threshold_(false),
+      in_flight_layout_blocking_threshold_(0),
+      max_num_delayable_while_layout_blocking_(
+          kDefaultMaxNumDelayableWhileLayoutBlocking),
+      max_num_delayable_requests_(kDefaultMaxNumDelayableRequestsPerClient),
       coalescing_timer_(new base::Timer(true /* retain_user_task */,
                                         true /* is_repeating */)) {
   std::string throttling_trial_group =
       base::FieldTrialList::FindFullName(kThrottleCoalesceFieldTrial);
   if (throttling_trial_group == kThrottleCoalesceFieldTrialThrottle) {
     should_throttle_ = true;
   } else if (throttling_trial_group == kThrottleCoalesceFieldTrialCoalesce) {
     should_coalesce_ = true;
     should_throttle_ = true;
   }
 
   std::string outstanding_limit_trial_group =
       base::FieldTrialList::FindFullName(kRequestLimitFieldTrial);
   std::vector<std::string> split_group(
       base::SplitString(outstanding_limit_trial_group, "=",
                         base::KEEP_WHITESPACE, base::SPLIT_WANT_ALL));
   int outstanding_limit = 0;
   if (split_group.size() == 2 &&
       split_group[0] == kRequestLimitFieldTrialGroupPrefix &&
       base::StringToInt(split_group[1], &outstanding_limit) &&
       outstanding_limit > 0) {
     limit_outstanding_requests_ = true;
     outstanding_request_limit_ = outstanding_limit;
   }
+
+  // Set up the ResourceScheduling field trial options.
+  // The field trial parameters are also encoded into the group name since
+  // the variations component is not available from here and plumbing the
+  // options through the code is overkill for a short experiment.
+  //
+  // The group name encoding looks like this:
+  //  <descriptiveName>_ABCDE_E2_F_G
+  //    A - fetchDeferLateScripts (1 for true, 0 for false)
+  //    B - fetchIncreaseFontPriority (1 for true, 0 for false)
+  //    C - fetchIncreaseAsyncScriptPriority (1 for true, 0 for false)
+  //    D - fetchIncreasePriorities (1 for true, 0 for false)
+  //    E - fetchEnableLayoutBlockingThreshold (1 for true, 0 for false)
+  //    E2 - fetchLayoutBlockingThreshold (Numeric)
+  //    F - fetchMaxNumDelayableWhileLayoutBlocking (Numeric)
+  //    G - fetchMaxNumDelayableRequests (Numeric)
+  std::string resource_priorities_trial_group =
+      base::FieldTrialList::FindFullName(kResourcePrioritiesFieldTrial);
+  std::vector<std::string> resource_priorities_split_group(
+      base::SplitString(resource_priorities_trial_group, "_",
+                        base::KEEP_WHITESPACE, base::SPLIT_WANT_ALL));
+  if (resource_priorities_split_group.size() == 5 &&
+      resource_priorities_split_group[1].length() == 5) {
+    // fetchIncreasePriorities
+    if (resource_priorities_split_group[1].at(3) == '1')
+      layout_blocking_priority_threshold_ = net::MEDIUM;
+    enable_layout_blocking_threshold_ =
+        resource_priorities_split_group[1].at(4) == '1';
+    size_t numeric_value = 0;
+    if (base::StringToSizeT(resource_priorities_split_group[2], &numeric_value))
+      in_flight_layout_blocking_threshold_ = numeric_value;
+    if (base::StringToSizeT(resource_priorities_split_group[3], &numeric_value))
+      max_num_delayable_while_layout_blocking_ = numeric_value;
+    if (base::StringToSizeT(resource_priorities_split_group[4], &numeric_value))
+      max_num_delayable_requests_ = numeric_value;
+  }
 }
 
 ResourceScheduler::~ResourceScheduler() {
   DCHECK(unowned_requests_.empty());
   DCHECK(client_map_.empty());
 }
 
 void ResourceScheduler::SetThrottleOptionsForTesting(bool should_throttle,
                                                      bool should_coalesce) {
   should_coalesce_ = should_coalesce;
@@ skipping 53 matching lines @@
 }
 
 void ResourceScheduler::OnClientCreated(int child_id,
                                         int route_id,
                                         bool is_visible,
                                         bool is_audible) {
   DCHECK(CalledOnValidThread());
   ClientId client_id = MakeClientId(child_id, route_id);
   DCHECK(!ContainsKey(client_map_, client_id));
 
-  Client* client = new Client(this, is_visible, is_audible);
+  Client* client = new Client(this, is_visible, is_audible,
+      layout_blocking_priority_threshold_, enable_layout_blocking_threshold_,
+      in_flight_layout_blocking_threshold_,
+      max_num_delayable_while_layout_blocking_, max_num_delayable_requests_);
   client_map_[client_id] = client;
 
   client->UpdateThrottleState();
 }
 
 void ResourceScheduler::OnClientDeleted(int child_id, int route_id) {
   DCHECK(CalledOnValidThread());
   ClientId client_id = MakeClientId(child_id, route_id);
   DCHECK(ContainsKey(client_map_, client_id));
   ClientMap::iterator it = client_map_.find(client_id);
@@ skipping 231 matching lines @@
   client->ReprioritizeRequest(
       request, old_priority_params, new_priority_params);
 }
 
 ResourceScheduler::ClientId ResourceScheduler::MakeClientId(
     int child_id, int route_id) {
   return (static_cast<ResourceScheduler::ClientId>(child_id) << 32) | route_id;
 }
 
 }  // namespace content