Note: this will be a bigger problem once rdsmiths resource throttling comes after the cache layer. Instead of H2/QUIC hammering the task queue we'll have all requests.

I suspect we will need some simple throttling at the cache layer.

jkarlin, can you post a trace of this in action? How long in ms does it take for the renderer to get the first bytes? Is Blink's main thread idle?

Another option is to add priorities to posted tasks on the IO thread. Then we wouldn't have to throttle. That would be.. interesting.

I made a pretty well fleshed out trace of this in action several months back. See this thread for more detail: https://groups.google.com/a/chromium.org/forum/#!topic/loading-dev/3JJIkq0jt_0

I've attached a desktop and mobile trace of that demo site to this bug. The mobile version shows a 400ms delay.

Deleted: trace_linux_release_spacer_deep_dive (1).json.gz 1.0 MB

trace_linux_release_spacer_deep_dive (1).json.gz 1.0 MB Download

Deleted: trace_mobile_spacers.json 5.4 MB

trace_mobile_spacers.json 5.4 MB View Download

400ms is huge. This is making me think that solving this might be a blocker for Randy's change. 100 requests is more than most sites will load from cache, but not by a lot.

Even if the resource isn't cached, we'll still do the 100 safe browsing checks and the 100 open cache entry tasks before throttling on the sockets.

How much would we gain from finding a way to batch these so we didn't have the task overhead (and maybe lose some of the safe browsing checks overhead)?

The other thing I think is worth exploring is throttling and prioritizing cache requests.  The combination would allow high priority requests to jump ahead of low priority requests, and would allow further processing to continue because all the SB & Cache tasks wouldn't be posted at once.

Sounds like we might want something like a simple scheduler (for throttling and prioritizing) at cache layer?

(/sub for layered throttling/scheduling discussion for scalable loading)

Hi,

Here is another trace, gathered the following way:
- Nexus 5X, Android N
- Loading android.com after a recent visit (so most resources are in the cache, and the OS page cache as well)
- Chromium master as of 10/14

Here we see that the renderer is waiting ~530ms before being able to do anything, even though all the resources are in the cache, and accessing the disk itself is free (as shown by the crazy small amount of time spent in File::Read).

This means that:
- Disk reads are not the bottleneck (processing time on the IO thread is)
- Page loads to visited URLs can be slow (back navigations for instance)
- On low-end devices with few cores, this is probably even worse, as the IO thread is likely to be descheduled at some point (since we are doing work on at least 4 threads here: disk_cache thread pool, Browser IO thread, Renderer IO thread, Browser Main). Here what we see is probably a best-case scenario, since the IO thread is not starved of CPU time, and given the load, it is highly likely to be running on a big core. On mid and low-end devices with only A53s (little cores), we can expect the IO thread CPU time to be at least twice as high.

Deleted: trace_android_com_nexus5x.json.tar.gz 1.0 MB

trace_android_com_nexus5x.json.tar.gz 1.0 MB Download

There seems to be two things that need to be done here. 

1) Allow requests that start earlier to complete earlier (e.g., by capping the max number of started requests)

2) The IO thread is nuts. We need to dig in an optimize this. I'm going to rebase https://codereview.chromium.org/1569673002/ so that we can get a better picture of where the time is being spent. A couple of things we already know that we're spending time on are 1) parsing SSL certificates and 2) creating shared memory buffers for each request.

I think you should land a (subset?) of those tracing so we can all see this in action.

(1) Seems to be a proposal to throttle requests. I'm not entirely convinced throttling is needed over prioritization.

Yes, working on a permanent tracing CL as well. Throttling was just an example.

> (e.g., by capping the max number of started requests)

I see the "e.g." but I just want to call out that where the cap occurs can very much effect the data available to make decisions.  I.e. I'd recommend against throttling requests in the renderer, because (e.g.) that may mean that requests that can be served from the cache are blocked behind requests that can't, and that high priority requests are blocked behind low.  I'm very much in favor of per-subsystem throttles, e.g. keeping the number of outstanding cache requests limited to the cache bandwidth (or maybe slightly more), which will minimize thread contention from those requests and allow later dispatched highpri requests to jump the queue.

Right. I am strongly opposed to solving this with renderer side throttling.

Just to clarify, when we say 'throttling' I'm also assuming that we are talking about throttling possibility in the browser's cache layer or somewhere around that, not in the renderer.

#16 yes exactly.

> I'm very much in favor of per-subsystem throttles, e.g. keeping the number
> of outstanding cache requests limited to the cache bandwidth

I'm looking at traces on Android where most or all of the data comes from the cache, and there is no way to even come close to the cache bandwidth.
The IO thread itself is the bottleneck.

There are no obvious offenders unfortunately.
Some things jump out, such as parsing certificates, the data use measurements and network quality estimators... improving these would certainly help but only marginally.

Re-ordering operations could be promising, or parallelize more (i.e. moving things off the IO thread to another thread).

#18, probably "cache bandwidth" should be observed after all this work is done (not at the disk layer), and the data is ready to be consumed.

Do you mean some kind of throttling where the fastest the component is, the more we throttle it?

That would mean throttling requests coming from cache a lot, since they have the potential to heavily spam the IO thread due to quick response time.

But throttle less the requests coming from the network, since we expect most of their time will be spent out of the IO thread.

This was a bit counter-intuitive to me, but that makes sense.

Possibly something fine-grained like that could work. I was thinking of just tracking the IO thread overhead per request and throttling all requests based on that.

I still think we can solve this without throttling though (and only do task prioritization).

E.g. I think if we prioritized tasks to always prefer ones that moved a request to a later stage, this situation would be far better.

In particular, this scheme would ensure that some request will always have the chance to make meaningful progress, without doing throttling which would potentially leave idle periods.

One option would be priority-based task scheduling on IO thread.

By the way do we need both short-term/stop-gap and mid/long-term solution here?  How soon do we expect that this could become a really huge blocking issue?

This might block rdsmiths work, which will roll out with a finch experiment anyways (killing ResourceScheduler).

I sent an email to scheduler-dev regarding the solution in #22. See the post here:
https://groups.google.com/a/chromium.org/forum/#!topic/scheduler-dev/Qaq7MiYyaEM

In particular, the lucky-luke project adds some notion of task priority but it is very coarse grained. Maybe scheduler-dev has tips for us.

I had looked into lucky luke doc when I found this issue but it didn't look their initial design scope covers this, but let's see the discussion there.

Possibly we could implement our own loading task runner with prioritization on top of what lucky-luke offers too like what blink scheduler does-- I'll follow the discussion there anyways.

(adding the FB tag here because we've generally found that the latency of loading large numbers of resources can be pretty impactful for us)

We can fix this without changing altering the thread's task scheduler:

1) Make the RequestResource IPC handler extremely light weight and just post a task.
2) When the task fires choose the request that's highest priority and start it
3) Place the HttpCache::Transaction callbacks into a priority queue in HttpCache, let the state machine callbacks call HttpCache::RunNextTask() which chooses the highest priority doLoop that's ready to proceed.

That solution sounds good. I have a few questions, though.

How does the priority queue work? Do we move an element to the back of the queue when we run its callback?

How does this perform with n resources with the same priority?

Quick test: a dumb throttling in ResourceScheduler to 1 to 10 requests when loading from cache is improving load times by ~20% on my android.com example.

Even with so few requests, we are still largely IO-thread bound.

Note that a lot of time (300-500 ms) is spent in just receiving the IPCs from the renderer (code like ResourceHostMsg_RequestResource, ResourceLoader::StartRequest...) and is not addressed by this kind of scheduling.

#29: each time a task is posted the real callback is put in the priority queue and the posted task just points to a "RunNextCallback" function that grabs from the front of the queue, runs the callback (e.g., the doLoop), and the callback disappears.

As for how does it perform with n resource of the same priority, that depends on the goals:

1) Tasks from higher priority requests should get to run before lower priority request tasks
2) Tasks from older requests (but are the same priority) should run before tasks of younger requests

If we do prioritization, we clearly want 1. I could see also wanting 2. We could get 2 by doing something like:

low priority = 10

med priority = 20

high priority = 30

And then as the request progresses through the state machine increment its priority by one.

#30 Not surprised by that result, but it's great to have the confirmation.

If we did throttling, we could still reduce the initial cost (e.g., StartRequest) by throttling earlier in the process.

I think the primary case against throttling is that throttling isn't priority sensitive, and some important javascript might get stuck behind an image. I don't have a good feeling for the order that resources appear on pages to know if this is a major concern, but it seems reasonable.

The win for throttling is that it's super simple, might get us most of the way there, and could be a quick hack while we work on fixing prioritization.

Agreed that prioritizing and throttling requests to pump the IO thread in a responsive way is appropriate. However, this should not rule out reducing the amount of work being done (multiply all benefits!), so I think it also makes sense to do more batching in the cached case: instead of 3 tasks to open+readheaders+readbody, we should have openandread all the way from Blink.

This could work as follows with simplecache:

1. openandread behaves the same way as open on the various failure cases

2. [edge case] once openandread hits an entry that is open concurrently by something else, return the headers immediately (with simplecache they are guaranteed to be in RAM) - no thread hops!

3. once openandread hits the disk, it reads the headers first, if those came in fast, as if it came from RAM (thresholds, yay), then we probably used this resource recently (prefetched, doing a back navigation, etc), high chances the rest of the body is in RAM as well, so read that to fill the 32K buffer we have (sorry, forgot again where it came from).

4. on revalidation case we would need to discard the body we have just read => extra work, not good, but I suspect performance-wise it is a small addition (at least on linux kernels), battery-wise - less confident

Implementing this would be easy in simplecache, not so easy in the http cache transaction and not-worth-it in blockfile cache. All the way to Blink - not sure.

Since parsing certificates was mentioned on the thread to loading-dev, adding the tag and some CCs.

I haven't investigated the traces to see where the time is being spent; if someone can point to the best trace (from the thread and this bug it's unclear which is the clearest picture of that), we can look into it and see if it's one of the things that's been on our radar.

I certainly wouldn't turn you down if you were able to grab a profiler trace easily enough, that could save a lot of time for David and I, since I suspect David's device is flashed with an unreleased and I'm away from mine this week :)

#36 look at trace_mobile_spacers.json from comment #3. Look at the IO thread during the 100 HTTPCache::Transaction::DoCacheReadResponseComplete tasks, you'll see that X509Certificate::CreateFromDERCertChain is quite slow.

I plan on implementing throttling the same way that we throttle HTTP/1.1 as a short-term fix. Long term it looks like we're going to need prioritized task schedulers, which is a larger undertaking. 

note: the trace from lizeb@ demonstrates the problem on android.com, which uses HTTP/1.1.

OK, just dumping a little state from offline convo with Josh:

At a high-level, the certificate parsing issue is exactly what my EP intern, Brandon Salmon, had worked on. At the conceptual level, the problem is we store all connection-oriented information in the disk cache (so that we can reproduce it for UI and such), which means that all cache entries from a connection end up storing duplicate info (which itself is often duplicated with future connections). For example, we know that something close to 99% of users' disk cache entries will be loading certificate chains that they'd already loaded (e.g. multiple resources that were previously received over the same connection). Unfortunately, the performance needs of the cache and the privacy needs of being able to clear data made it particularly challenging to solve by the end of the internship.

At a more concrete level, there do seem to be some optimizations we can apply to the X509Certificate OSCertHandle cache. At present, we're looking up entries by their OSCertHandle, but keying them off the hash of the cert data. This API means that the flow of loading a disk cache entry is to parse a cert with OpenSSL, look it up (extracting the DER and computing the hash), seeing there's already an OSCertHandle, and upping that refcount and releasing the one we looked up. This helps significantly reduce memory usage (because we only keep one OSCertHandle), but it means there is more CPU time spent parsing the cert.

Long-term, David and I have been discussing a path to avoiding the need for the OSCertHandle step as part of X509Certificate, which would reduce this problem, as well as internal-to-BoringSSL ref-counting management, which would avoid the parsing problem. Short-term, we can look into some changes for X509Certificate's OSCertHandle cache, so that we could look it up based on DER rather than the parsed handle; this would eliminate the parsing time from the trace, but we'd still have the IO load/lookups that have to happen (because of the aforementioned cache-vs-connection issues)

Filed  Issue 657047  for the short-term fix. I don't think we've finalized or prioritized the long-term plans (but we have a doc and a plan!), but the short-term fix may address the immediate performance issues here for parsing.

Certainly, I encouraged Josh to get a fresh trace; we had some inefficiencies in the cache that were highlighted by the memory team, and we were able to significantly reduce the cache miss rate here, which may correspond to better performance here.

This is the longer term work, for reference (sorry, Google-only):
https://docs.google.com/document/d/16c_O76rjFEc0_gfLn2Ggq-E1qytNAWPoGFo_0vkMhmc/edit#

There is nothing inherently expensive about dealing with certificates once the data's been read from disk. It's just that OpenSSL's X.509 code (used by Android) is awful and made just about the wrong decision at every turn. Once we're rid of that, CreateFromDERCertChain should be very cheap.

@pasko, re #41: android.com is QUIC. (served from a GFE)

As per usual profiling on Android is not working for me right now, sorry. Given that this seems Android specific I don't think Linux profiles will be very useful.

re #45: OK, it was not QUIC on my Chrome instance. Experiments running?

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a9d98bd309fd58129b40ee9fccefc2ef76732f6c

commit a9d98bd309fd58129b40ee9fccefc2ef76732f6c
Author: jkarlin <jkarlin@chromium.org>
Date: Wed Oct 19 18:45:48 2016

[ResourceScheduler] Throttle H2/QUIC requests just like we do for 1.1

This is a short-term fix to alleviate long delays on early requests when there
are many requests in flight. See the bug for more detail.

BUG=655585

Review-Url: https://chromiumcodereview.appspot.com/2435743002
Cr-Commit-Position: refs/heads/master@{#426251}

[modify] https://crrev.com/a9d98bd309fd58129b40ee9fccefc2ef76732f6c/content/browser/loader/resource_scheduler.cc
[modify] https://crrev.com/a9d98bd309fd58129b40ee9fccefc2ef76732f6c/content/browser/loader/resource_scheduler.h
[modify] https://crrev.com/a9d98bd309fd58129b40ee9fccefc2ef76732f6c/content/browser/loader/resource_scheduler_unittest.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f390a5e4ff02ee6a72b42faed7d519213d689896

commit f390a5e4ff02ee6a72b42faed7d519213d689896
Author: jkarlin <jkarlin@chromium.org>
Date: Thu Oct 20 16:58:18 2016

[ResourceScheduler] Make throttling H2+QUIC default disabled

Since we don't know for sure what the outcome will be and need to perform
field trials, default to disabled.

BUG=655585

Review-Url: https://chromiumcodereview.appspot.com/2441633002
Cr-Commit-Position: refs/heads/master@{#426508}

[modify] https://crrev.com/f390a5e4ff02ee6a72b42faed7d519213d689896/content/browser/loader/resource_scheduler.cc
[modify] https://crrev.com/f390a5e4ff02ee6a72b42faed7d519213d689896/content/browser/loader/resource_scheduler_unittest.cc
[modify] https://crrev.com/f390a5e4ff02ee6a72b42faed7d519213d689896/testing/variations/fieldtrial_testing_config.json

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/72fad123cfec3d1b139456fadc091d344ca4f7bc

commit 72fad123cfec3d1b139456fadc091d344ca4f7bc
Author: jkarlin <jkarlin@chromium.org>
Date: Fri Oct 21 16:28:31 2016

Change name of field trial to NetDelayableH2AndQuicRequests

BUG=655585

Review-Url: https://chromiumcodereview.appspot.com/2434313002
Cr-Commit-Position: refs/heads/master@{#426808}

[modify] https://crrev.com/72fad123cfec3d1b139456fadc091d344ca4f7bc/testing/variations/fieldtrial_testing_config.json

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1a8d5f3805dbae5242cf7dce14df3807344dae37

commit 1a8d5f3805dbae5242cf7dce14df3807344dae37
Author: jkarlin <jkarlin@chromium.org>
Date: Sun Feb 12 20:12:24 2017

[ResourceScheduler] Yield after starting several requests

This CL periodically yields the ResourceScheduler after starting several
requests. This gives existing IO tasks (such as active network requests) a
chance to run between network starts. This should result in less jank and more
interleaved network request tasks.

BUG=655585

Review-Url: https://codereview.chromium.org/2682163002
Cr-Commit-Position: refs/heads/master@{#449895}

[modify] https://crrev.com/1a8d5f3805dbae5242cf7dce14df3807344dae37/content/browser/loader/resource_scheduler.cc
[modify] https://crrev.com/1a8d5f3805dbae5242cf7dce14df3807344dae37/content/browser/loader/resource_scheduler.h
[modify] https://crrev.com/1a8d5f3805dbae5242cf7dce14df3807344dae37/content/browser/loader/resource_scheduler_unittest.cc
[modify] https://crrev.com/1a8d5f3805dbae5242cf7dce14df3807344dae37/testing/variations/fieldtrial_testing_config.json

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a9757ebf44c92e009d075322ad35f008b0bfa1ad

commit a9757ebf44c92e009d075322ad35f008b0bfa1ad
Author: mtomasz <mtomasz@chromium.org>
Date: Wed Feb 22 09:09:24 2017

Revert of [ResourceScheduler] Yield after starting several requests (patchset #9 id:240001 of https://codereview.chromium.org/2682163002/ )

Reason for revert:
Regresses blob requests making them delayable.

Original issue's description:
> [ResourceScheduler] Yield after starting several requests
>
> This CL periodically yields the ResourceScheduler after starting several
> requests. This gives existing IO tasks (such as active network requests) a
> chance to run between network starts. This should result in less jank and more
> interleaved network request tasks.
>
> BUG=655585
>
> Review-Url: https://codereview.chromium.org/2682163002
> Cr-Commit-Position: refs/heads/master@{#449895}
> Committed: https://chromium.googlesource.com/chromium/src/+/1a8d5f3805dbae5242cf7dce14df3807344dae37

TBR=csharrison@chromium.org,isherman@chromium.org,jkarlin@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=655585

Review-Url: https://codereview.chromium.org/2708323002
Cr-Commit-Position: refs/heads/master@{#451946}

[modify] https://crrev.com/a9757ebf44c92e009d075322ad35f008b0bfa1ad/content/browser/loader/resource_scheduler.cc
[modify] https://crrev.com/a9757ebf44c92e009d075322ad35f008b0bfa1ad/content/browser/loader/resource_scheduler.h
[modify] https://crrev.com/a9757ebf44c92e009d075322ad35f008b0bfa1ad/content/browser/loader/resource_scheduler_unittest.cc
[modify] https://crrev.com/a9757ebf44c92e009d075322ad35f008b0bfa1ad/testing/variations/fieldtrial_testing_config.json

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bf88dc8ed93ded5866c4c2965803ae24d89050b6

commit bf88dc8ed93ded5866c4c2965803ae24d89050b6
Author: jkarlin <jkarlin@chromium.org>
Date: Wed Feb 22 21:40:03 2017

Reland of [ResourceScheduler] Yield after starting several requests (patchset
#1 id:1 of https://codereview.chromium.org/2708323002/ )

The original patch was reverted because it delayed non-http(s) requests. This reland fixes that issue.

Original issue's description:
> Revert of [ResourceScheduler] Yield after starting several requests (patchset #9 id:240001 of https://codereview.chromium.org/2682163002/ )
>
> Reason for revert:
> Regresses blob requests making them delayable.
>
> Original issue's description:
> > [ResourceScheduler] Yield after starting several requests
> >
> > This CL periodically yields the ResourceScheduler after starting several
> > requests. This gives existing IO tasks (such as active network requests) a
> > chance to run between network starts. This should result in less jank and more
> > interleaved network request tasks.
> >
> > BUG=655585
> >
> > Review-Url: https://codereview.chromium.org/2682163002
> > Cr-Commit-Position: refs/heads/master@{#449895}
> > Committed: https://chromium.googlesource.com/chromium/src/+/1a8d5f3805dbae5242cf7dce14df3807344dae37
>
> TBR=csharrison@chromium.org,isherman@chromium.org,jkarlin@chromium.org
> # Not skipping CQ checks because original CL landed more than 1 days ago.
> BUG=655585
>
> Review-Url: https://codereview.chromium.org/2708323002
> Cr-Commit-Position: refs/heads/master@{#451946}
> Committed: https://chromium.googlesource.com/chromium/src/+/a9757ebf44c92e009d075322ad35f008b0bfa1ad

BUG=655585

Review-Url: https://codereview.chromium.org/2711633003
Cr-Commit-Position: refs/heads/master@{#452216}

[modify] https://crrev.com/bf88dc8ed93ded5866c4c2965803ae24d89050b6/content/browser/loader/resource_scheduler.cc
[modify] https://crrev.com/bf88dc8ed93ded5866c4c2965803ae24d89050b6/content/browser/loader/resource_scheduler.h
[modify] https://crrev.com/bf88dc8ed93ded5866c4c2965803ae24d89050b6/content/browser/loader/resource_scheduler_unittest.cc
[modify] https://crrev.com/bf88dc8ed93ded5866c4c2965803ae24d89050b6/testing/variations/fieldtrial_testing_config.json

 Issue 710042  has been merged into this issue.

+mattm's recent work in  issue #671420  should switch Android and Mac (other platforms on the way) onto a more shallow certificate parser. This is the work I was mentioning to in comment #44. Do you still see certificate-related issues on those two platforms?

We're not quite doing zero parsing of the certificate, but hopefully much lighter parsing now. (If certificate parsing is still showing up as a bottleneck, zero parsing is a thing we can explore once this stage is done, but we need to get the platforms on a common path first.)

Cool, which versions will have this improved behavior?

M59 for Mac and M60 for Android.

I did a brief trace run of M60 Android and can confirm I saw a lot less X509 stuff.

X509Certificate::CreateFromDERCertChain is still showing up, but it doesn't look like a bad actor.

#57: Fantastic! How soon until it's on Linux? Easier for me to investigate there.

Better to test on the platform you're actually looking to measure. The effects will be very different per-platform because the code we're replacing is platform-specific. The version on Android was disproportionately bad, though I don't know how other platforms' cert parsers are.

(mattm can say for certain when it'll be on Linux/CrOS, but I expect it will be hairier than the other platforms due to CrOS things and diverging Linux and CrOS is probably undesirable.)

Yeah, I expect doing  issue 671420  for chromeos and linux will both be harder, but especially chromeos. I think it makes sense to keep linux and chromeos on the same path, so linux will be blocked on chromeos. I was also planning to get the client certs private key refactoring finished before starting that work. So it's not going to be soon, like a month or more.

 Issue 757191  has been merged into this issue.

 Issue 757586  has been merged into this issue.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/40b0fcdb43d352e722159a7eade5ad5fefead017

commit 40b0fcdb43d352e722159a7eade5ad5fefead017
Author: Josh Karlin <jkarlin@chromium.org>
Date: Mon Aug 28 15:33:54 2017

[H2+QUIC Throttling] Experiment with throttling only while parsing head

The max-per-host and max-per-tab limits for H2 requests are problematic. They
prevent pages from opening lots of videos simultaneously. They also add
unnecessary latency. In order to alleviate that, while still providing
priority to critical resources, this patch introduces throttling of H2/QUIC
only while the parser is in head.

Bug: 655585
Change-Id: If3ec20583e4924ed3ef93035121900026f4db5f0
Reviewed-on: https://chromium-review.googlesource.com/635491
Commit-Queue: Josh Karlin <jkarlin@chromium.org>
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#497764}
[modify] https://crrev.com/40b0fcdb43d352e722159a7eade5ad5fefead017/content/browser/loader/resource_scheduler.cc
[modify] https://crrev.com/40b0fcdb43d352e722159a7eade5ad5fefead017/content/browser/loader/resource_scheduler.h
[modify] https://crrev.com/40b0fcdb43d352e722159a7eade5ad5fefead017/content/browser/loader/resource_scheduler_unittest.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/40c8756b9f5588142839e332b585d8ddc9486a55

commit 40c8756b9f5588142839e332b585d8ddc9486a55
Author: Josh Karlin <jkarlin@chromium.org>
Date: Mon Aug 28 19:00:39 2017

[ResourceScheduler] Disable H2 throttling by default

Bug: 655585
Change-Id: Ie97d8c4a93b1e36ecb7e9b15e3a93524d24fd501
Reviewed-on: https://chromium-review.googlesource.com/638536
Commit-Queue: Josh Karlin <jkarlin@chromium.org>
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#497820}
[modify] https://crrev.com/40c8756b9f5588142839e332b585d8ddc9486a55/content/browser/loader/resource_scheduler.cc

I've disabled H2+QUIC throttling everywhere except in canary/dev experiments as it was breaking some pages (videos couldn't play in parallel) and slowing some pages with considerable numbers of low-priority resources down.

re #61: the linux use_byte_certs work is currently in process of being landed, but if you just want to do local testing before the remaining bits land, you can patch in https://chromium-review.googlesource.com/c/chromium/src/+/578568 and its CL chain.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5b2b3d379e138867226516d46e79b5f1c81eb4f5

commit 5b2b3d379e138867226516d46e79b5f1c81eb4f5
Author: Josh Karlin <jkarlin@chromium.org>
Date: Thu Aug 31 13:08:38 2017

[ResourceScheduler] Update Yielding experiment

Change yielding to:

1) Only count low-priority resources as yieldable
2) Only yield for requests whose servers support priorities (e.g., H2 + QUIC)
   since HTTP/1.1 already has throttling.
3) Yield for a (field trial configurable) amount of time

Other changes include:

1. Cleaner tests (I recommend ignoring the old tests in the review)
2. ResourceScheduler now has public getters for parameters
3. Adding all 3 features to fieldtrial_testing_config in separate platforms

Bug: 655585
Change-Id: I285294ab8b26716378a5ed73d63f714bb4ae1453
Reviewed-on: https://chromium-review.googlesource.com/641995
Commit-Queue: Josh Karlin <jkarlin@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#498838}
[modify] https://crrev.com/5b2b3d379e138867226516d46e79b5f1c81eb4f5/content/browser/loader/resource_scheduler.cc
[modify] https://crrev.com/5b2b3d379e138867226516d46e79b5f1c81eb4f5/content/browser/loader/resource_scheduler.h
[modify] https://crrev.com/5b2b3d379e138867226516d46e79b5f1c81eb4f5/content/browser/loader/resource_scheduler_unittest.cc
[modify] https://crrev.com/5b2b3d379e138867226516d46e79b5f1c81eb4f5/testing/variations/fieldtrial_testing_config.json

FYI, use_byte_certs has now landed on all platforms, so you may wish to redo the initial experiments.

Josh: Re #71, would it be quick and easy to resume the experiment?
FB said that they are observing the issue and it seems to have a significant perf impact.

The experiment is still running in canary/dev. The results aren't as good as they used to be. If loading certs is faster now, that could explain why.

I'm taking a different tack on this problem now. Rather than focus on H2/QUIC I've got a CL* to make the disk cache's worker pool process tasks in priority order. In local testing this seems to have pretty significant impact on performance.

* https://chromium-review.googlesource.com/c/chromium/src/+/1064506

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2b5971fb6ad17a56f3dd7206817a8a82f7de6b6f

commit 2b5971fb6ad17a56f3dd7206817a8a82f7de6b6f
Author: Josh Karlin <jkarlin@chromium.org>
Date: Thu Jun 07 12:47:59 2018

Enable field trial testing of SimpleCache task prioritization

Bug: 655585
Change-Id: I672e5229377469ce9c942a7078e8118ff31a7606
Reviewed-on: https://chromium-review.googlesource.com/1088628
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Commit-Queue: Josh Karlin <jkarlin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565243}
[modify] https://crrev.com/2b5971fb6ad17a56f3dd7206817a8a82f7de6b6f/testing/variations/fieldtrial_testing_config.json