Fix Resource Priorities and Scheduling (Blink Side)

Source/core/fetch/ResourceFetcher.cpp

Index: Source/core/fetch/ResourceFetcher.cpp
diff --git a/Source/core/fetch/ResourceFetcher.cpp b/Source/core/fetch/ResourceFetcher.cpp
index 24794a10d923f60b411719a5e8c28aefad10d3ee..00332bb6bf291d50fa51c73fd7a1a4ad7970f7b2 100644
--- a/Source/core/fetch/ResourceFetcher.cpp
+++ b/Source/core/fetch/ResourceFetcher.cpp
@@ -63,26 +63,45 @@ ResourceLoadPriority ResourceFetcher::loadPriority(Resource::Type type, const Fe
     if (request.priority() != ResourceLoadPriorityUnresolved)
         return request.priority();
 
+    // An image fetch is used to distinguish between "early" and "late" scripts in a document
+    if (type == Resource::Image)
+        m_imageFetched = true;
+
+    // Runtime experiment that change how we prioritize resources.
+    // Background doc: https://docs.google.com/document/d/1bCDuq9H1ih9iNjgzyAL0gpwNFiEP4TZS-YLRp_RuMlc/edit?usp=sharing
+    bool deferLateScripts = context().fetchDeferLateScripts();
+    bool increaseFontPriority = context().fetchIncreaseFontPriority();
+    bool increaseAsyncScriptPriority = context().fetchIncreaseAsyncScriptPriority();
+    bool increasePriorities = context().fetchIncreasePriorities();

[Bryan McQuade, 2015/08/07 17:30:40]

this is the only one that could probably use a comment, mostly to explain why it
needs to exist. what does it change? does that lead to changes in actual
behavior or does it only enable some of the other boolean experiments above to
work properly? also, if any of these experiments must only be run with other
experiments enabled, it'd be good to add a comment for that and audit that it's
the case here. It's probably also worth adding a comment noting that these are
temporary in order for us to identify the optimal loading strategy, after which
point the code will have less branching and become simpler.

[Pat Meenan, 2015/08/07 21:21:14]

Added some documentation explaining it.  All of the experiments can be toggled
individually (including the "increasePriorities" one though the results are
likely to interact with each other).

+
     switch (type) {
     case Resource::MainResource:
         return context().isLowPriorityIframe() ? ResourceLoadPriorityVeryLow : ResourceLoadPriorityVeryHigh;
     case Resource::CSSStyleSheet:
-        return ResourceLoadPriorityHigh;
+        return increasePriorities ? ResourceLoadPriorityVeryHigh : ResourceLoadPriorityHigh;

[Bryan McQuade, 2015/08/07 17:30:40]

just an idea - perhaps we could add a member method:

ResourceLoadPriority maybeIncreasePriority(ResourceLoadPriority rlp) {
  if (!context().fetchIncreasePriorities()) return;
  switch (rlp) {
    case high: return veryhigh;
    case low: return medium;
    ...
    default: ASSERT();
  }
}

Then the logic that chooses a priority at all of the call sites is simplified:
return maybeIncreasePriority(ResourceLoadPriorityHigh);

[Pat Meenan, 2015/08/07 21:21:14]

It's a little more complicated because not all of the changes are of the same
magnitude.  Hopefully it's only short term until we get the results of the field
trial and all of the logic goes back to single decisions with fixed priorities.

[Bryan McQuade, 2015/08/10 19:27:09]

Ah, interesting, didn't realize not all boosts were of the same priority.

I'm mixed on adding complexity for temporary code. It doesn't seem like the best
thing in the long term if experiments we add all add temporary complexity -
eventually it's going to cause someone to introduce a bug.

If you see ways to simplify this change in the meantime, I think that's
beneficial, but if not I think our longer term goal should be to come up with
ways to run experiments that introduce less risk/complexity, even if that
complexity is temporary.

     case Resource::Raw:
         return request.options().synchronousPolicy == RequestSynchronously ? ResourceLoadPriorityVeryHigh : ResourceLoadPriorityMedium;
     case Resource::Script:
-        // Async scripts do not block the parser so they get the lowest priority and can be
-        // loaded in parser order with images.
+        // Async/Defer scripts.
         if (FetchRequest::LazyLoad == request.defer())
-            return ResourceLoadPriorityLow;
-        return ResourceLoadPriorityMedium;
+            return increaseAsyncScriptPriority ? ResourceLoadPriorityMedium : ResourceLoadPriorityLow;
+        // Reduce the priority of late-body scripts.
+        if (deferLateScripts && request.forPreload() && m_imageFetched)
+            return increasePriorities ? ResourceLoadPriorityMedium : ResourceLoadPriorityLow;
+        // Parser-blocking scripts.
+        if (increasePriorities && !request.forPreload())
+            return increasePriorities ? ResourceLoadPriorityVeryHigh : ResourceLoadPriorityMedium;
+        // Early scripts discovered by the preload scanner.
+        return increasePriorities ? ResourceLoadPriorityHigh : ResourceLoadPriorityMedium;
     case Resource::Font:
+        if (increaseFontPriority)
+            return increasePriorities ? ResourceLoadPriorityVeryHigh : ResourceLoadPriorityHigh;
+        return increasePriorities ? ResourceLoadPriorityHigh : ResourceLoadPriorityMedium;
     case Resource::ImportResource:
         return ResourceLoadPriorityMedium;
     case Resource::Image:
-        // Default images to VeryLow, and promote whatever is visible. This improves
-        // speed-index by ~5% on average, ~14% at the 99th percentile.
-        return ResourceLoadPriorityVeryLow;
+        // Default images to VeryLow, and promote when they become visible.
+        return increasePriorities ? ResourceLoadPriorityLow : ResourceLoadPriorityVeryLow;
     case Resource::Media:
         return ResourceLoadPriorityLow;
     case Resource::XSLStyleSheet:
@@ -98,6 +117,7 @@ ResourceLoadPriority ResourceFetcher::loadPriority(Resource::Type type, const Fe
     case Resource::TextTrack:
         return ResourceLoadPriorityLow;
     }
+
     ASSERT_NOT_REACHED();
     return ResourceLoadPriorityUnresolved;
 }
@@ -162,6 +182,7 @@ ResourceFetcher::ResourceFetcher(FetchContext* context)
     , m_autoLoadImages(true)
     , m_imagesEnabled(true)
     , m_allowStaleResources(false)
+    , m_imageFetched(false)
 {
 }