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 211 matching lines @@
 // Each client represents a tab.
 class ResourceScheduler::Client {
  public:
   explicit Client(ResourceScheduler* scheduler,
                   bool is_visible,
                   bool is_audible)
       : 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) {
+  }
 
   ~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 127 matching lines @@
     request->set_classification(classification);
     DCHECK_EQ(
         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) {
+    // classification across redirects.
+    if (request->classification() == LAYOUT_BLOCKING_REQUEST) {
       return LAYOUT_BLOCKING_REQUEST;
     }

[mmenke, 2015/08/12 17:30:08]

nit:  Remove braces, to be more consistent with the rest of this file

[Pat Meenan, 2015/08/13 12:11:03]

Done.

 
-    if (!has_body_ && request->url_request()->priority() > net::LOW)
+    if (!has_html_body_ &&
+        request->url_request()->priority() >
+        scheduler_->layout_blocking_priority_threshold()) {
       return LAYOUT_BLOCKING_REQUEST;
+    }
 
-    if (request->url_request()->priority() < net::LOW) {
+    if (request->url_request()->priority() <
+        scheduler_->layout_blocking_priority_threshold()) {
       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;
       }

[mmenke, 2015/08/12 17:30:08]

Can we add two more classifications here?

DELAYABLE_REQUEST and HIGH_PRIORITY_REQUEST?  Then we can just look at the
classifications in ShouldStartRequest.

I keep on getting confused about what a HIGH_PRIORITY request is (Didn't realize
it included requests with a priority of the
layout_blocking_priority_threshold(), and not above it).  I think having all the
classifications explicitly here would make the code much clearer.

[Pat Meenan, 2015/08/13 12:11:03]

Would you want them broken out into two flags (in-flight or not and the
categorization)?  Otherwise we'll have both DELAYABLE and IN_FLIGHT_DELAYABLE
but not IN_FLIGHT versions of the others.  It's also possible to go ahead and
create IN_FLIGHT versions of all of them since ShouldStartRequest will never be
checking an in-flight 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() >=
+        scheduler_->layout_blocking_priority_threshold()) {
       return START_REQUEST;
     }
 
-    if (in_flight_delayable_count_ >= kMaxNumDelayableRequestsPerClient) {
+    if (in_flight_delayable_count_ >=
+        scheduler_->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_;
+      if (scheduler_->enable_layout_blocking_threshold()) {
+        if (non_delayable_requests_in_flight_count >
+            scheduler_->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 &&

[mmenke, 2015/08/12 17:30:08]

I think we can remove this line - if in_flight_requests_.size() is 0, then
total_layout_blocking_count_ will also be 0, so we won't reach this point.  Or
should we reconsider the conditions of the first if?  It seems kinda weird that
if we have a layout blocking request, high priority requests count against low
priority requests, but if we don't, they don't.

[mmenke, 2015/08/12 17:33:59]

Oh, right...the no body yet case.  Think I forgot about that before.

[Pat Meenan, 2015/08/13 12:11:03]

Not sure if it helps any but I beefed up the comment for this chunk of logic to
hopefully be clearer.  It might also help if I factor out the check for the
layout-blocking loading pahse into a separate function to make it clearer and
bury the actual logic (though this is the only place it has to make that check).

+            (scheduler_->limit_outstanding_requests() ||
+             in_flight_delayable_count_ >=
+             scheduler_->max_num_delayable_while_layout_blocking())) {
+          // Block the request if at least one request is in flight and the
+          // number of in-flight delayable requests has hit the configured
+          // limit.
+          return DO_NOT_START_REQUEST_AND_STOP_SEARCHING;
+        }
+      } else if (non_delayable_requests_in_flight_count > 0 &&
+          (scheduler_->limit_outstanding_requests() ||
+           in_flight_delayable_count_ >=
+           scheduler_->max_num_delayable_while_layout_blocking())) {
+        // 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).
+        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_;
 };
 
 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 305 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