Link prefetch fetches deferred until onload

Link prefetch fetches deferred until onload

Currently link prefetched resources are competing on bandwidth with the
current page's resources.

This CL makes sure that such resources are deferred until the window's
onload event, in order to prevent such contention.

BUG=651160

[Yoav Weiss, 2017-03-16 14:00:01]

The CQ bit was checked by yoav@yoav.ws to run a CQ dry run

[commit-bot: I haz the power, 2017-03-16 14:00:12]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-16 15:15:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-16 15:15:46]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Yoav Weiss, 2017-03-16 16:43:35]

Description was changed from

==========
Link prefetch fetches deferred until onload

Currently link prefetched resources are competing on bandwidth with the
current page's resources.

This CL makes sure that such resources are deferred until the window's
onload event, in order to prevent such contention.

BUG=651160
==========

to

==========
Link prefetch fetches deferred until onload

Currently link prefetched resources are competing on bandwidth with the
current page's resources.

This CL makes sure that such resources are deferred until the window's
onload event, in order to prevent such contention.

BUG=651160
==========

[Yoav Weiss, 2017-03-16 16:43:36]

yoav@yoav.ws changed reviewers:
+ csharrison@chromium.org, mkwst@chromium.org

[Yoav Weiss, 2017-03-16 16:46:05]

Hey Mike and Charlie,

This implements deferring of prefetch resources until the window's onload event,
so that they won't stall current page resources.

However, this seems to break a bunch of WPT tests, which don't count on these
resources being deferred.

Mike - are you familiar with the mixed content tests? How should they be changed
to support the possibility (not obligation) of these resources being deferred?

[Charlie Harrison, 2017-03-16 16:52:25]

I am a little scared of this change. I wonder if we need to do a broader
analysis before landing.

[kinuko, 2017-03-17 04:52:33]

kinuko@chromium.org changed reviewers:
+ kouhei@chromium.org

[kinuko, 2017-03-17 05:01:20]

+kouhei so that this is on his radar too.

Throttling prefetches in renderer until DCL sounds pretty reasonable to me
(while adding more ad-hoc throttling makes me sad but we can hopefully
rearchitect too), but I prefer testing this with a trial flag to see perf impact
(which probably needs internal help), do we feel it's too much?

[kinuko, 2017-03-17 05:03:54]

On 2017/03/17 05:01:20, kinuko wrote:
> +kouhei so that this is on his radar too.
> 
> Throttling prefetches in renderer until DCL sounds pretty reasonable to me

s/DCL/onload/

> (while adding more ad-hoc throttling makes me sad but we can hopefully
> rearchitect too), but I prefer testing this with a trial flag to see perf
impact
> (which probably needs internal help), do we feel it's too much?

[kouhei (in TOK), 2017-03-17 05:13:51]

On 2017/03/17 05:03:54, kinuko wrote:
> On 2017/03/17 05:01:20, kinuko wrote:
> > +kouhei so that this is on his radar too.
> > 
> > Throttling prefetches in renderer until DCL sounds pretty reasonable to me
> 
> s/DCL/onload/
> 
> > (while adding more ad-hoc throttling makes me sad but we can hopefully
> > rearchitect too), but I prefer testing this with a trial flag to see perf
> impact
> > (which probably needs internal help), do we feel it's too much?

+1

[Yoav Weiss, 2017-03-17 09:28:52]

On 2017/03/16 16:52:25, Charlie Harrison-slow til 3-20 wrote:
> I am a little scared of this change. I wonder if we need to do a broader
> analysis before landing.

Can you elaborate on your concerns?

[Yoav Weiss, 2017-03-17 09:30:58]

On 2017/03/17 05:03:54, kinuko wrote:
> On 2017/03/17 05:01:20, kinuko wrote:
> > +kouhei so that this is on his radar too.
> > 
> > Throttling prefetches in renderer until DCL sounds pretty reasonable to me
> 
> s/DCL/onload/
> 
> > (while adding more ad-hoc throttling makes me sad but we can hopefully
> > rearchitect too), but I prefer testing this with a trial flag to see perf
> impact
> > (which probably needs internal help), do we feel it's too much?

I think it makes sense to put this behind a trial flag

[Yoav Weiss, 2017-03-17 09:41:38]

The CQ bit was checked by yoav@yoav.ws to run a CQ dry run

[commit-bot: I haz the power, 2017-03-17 09:41:48]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Charlie Harrison, 2017-03-17 09:53:50]

Sorry for being unclear. I am mainly concerned about performance regressions for
pages which use prefetch in slightly unusual ways.

For instance:
1. A site which uses a client side redirect, which uses link rel prefetch for
the final leg of the redirect. Delaying the fetch until onload could delay that
fetch.

2. A site which wants to prefetch the main resource of an iframe without loading
the entire iframe. This could be used to delay loading/rendering of an entire
iframe, but optimizing the final load by prefetching its main resource in
parallel with the main page.

[commit-bot: I haz the power, 2017-03-17 11:34:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-17 11:34:20]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Mike West, 2017-03-17 12:30:20]

On 2017/03/16 at 16:46:05, yoav wrote:
> Mike - are you familiar with the mixed content tests? How should they be
changed to support the possibility (not obligation) of these resources being
deferred?

Those tests are probably using WPTServe's stash mechanism to store the fact that
a request was or wasn't made. If we're changing the timing of the requests until
after `onload`, we're probably breaking the test's expectation that the things
it's testing have all finished. :/

[Mike West, 2017-03-17 12:30:55]

On 2017/03/17 at 12:30:20, Mike West (Slow.) wrote:
> On 2017/03/16 at 16:46:05, yoav wrote:
> > Mike - are you familiar with the mixed content tests? How should they be
changed to support the possibility (not obligation) of these resources being
deferred?
> 
> Those tests are probably using WPTServe's stash mechanism to store the fact
that a request was or wasn't made. If we're changing the timing of the requests
until after `onload`, we're probably breaking the test's expectation that the
things it's testing have all finished. :/

(That's not a reason not to make this change; I'll defer to folks who know
things about performance for that question. If this is a good idea, we'll update
the tests. *shrug*)

[kinuko, 2017-03-23 10:02:44]

kinuko@chromium.org changed reviewers:
+ kinuko@chromium.org