Closed Bug 1594449 Opened 5 years ago Closed 5 years ago

Experimentally implement <link rel="preload"> in the content process as a speculative load with higher priority

Categories

(Core :: DOM: Core & HTML, enhancement, P1)

72 Branch
enhancement

Tracking

()

RESOLVED FIXED
mozilla72
Webcompat Priority P2
Tracking Status
firefox72 --- fixed

People

(Reporter: mayhemer, Assigned: mayhemer)

References

(Blocks 2 open bugs)

Details

(Keywords: dev-doc-needed, Whiteboard: [necko-triaged])

Attachments

(2 files)

<link rel="preload"> is intended to deliver the (unprocessed) content data to the requesting content process as soon as possible after encountered and held in both the http cache and in an in-memory (preload) cache on the requesting content process.

Based on that and the following I decided to implement it as a speculative load with a flag to raise its priority.

  • When fission is enabled the link-preload preloaded content will be present in the right content process, as both are scoped by {schema:eTLD+1}
  • With fission disabled, content processes subsume scoping for preload scoping
  • Speculative loads are already coalesced by nsHtml5TreeOpExecutor.mPreloadedURLs hash table
  • Speculative loads already handle CSP, SW, webext, redirects, so no new code for that is needed
  • If decision to scope preloads is to be not just per document (what speculative loads provide) but also, when unused, for the whole document group, we just need a new service on the content process that can be searched for existing unused preloads; let's implement this as a followup, if the decision is made that way
  • We want to handle <link rel=preload> as a speculative load anyway, as was the original motivation for bug 1393540 (which this bug renders as a duplicate)
Depends on: 1594672

Comment on attachment 9106919 [details]
Bug 1594449 - <link rel="preload"> implemented as a speculative load initiated during the prescan phase in the HTML5 parser, disabled by default, only supports "script" and "styles" types, r=ckerschb!

Henri, this is a preliminary patch for implementing rel=preload entirely only as speculative loads with an additional flag to prioritize them differently; the prioritization schema is subject to improve based on thorough testing.

There is definitely some more work to do starting with support for more types and expiration logic for unused resources behind regular garbage collection.

If you find time, please overlook it. This is similar to the patch you gave feedback to 2 years ago at 1393540, Comment 14, some of the concerns are not considered in this patch, and we can do them as followups.

The difference is that this patch is enough to support rel=preload altogether, w/o any other code needed.

Thanks.

Attachment #9106919 - Flags: feedback?(hsivonen)

Comment on attachment 9106919 [details]
Bug 1594449 - <link rel="preload"> implemented as a speculative load initiated during the prescan phase in the HTML5 parser, disabled by default, only supports "script" and "styles" types, r=ckerschb!

Looks good with the same comments about multiple rel tokens and surrounding whitespace in attribute values as before.

Attachment #9106919 - Flags: feedback?(hsivonen) → feedback+
Attachment #9106919 - Attachment description: Bug 1594449 - Draft: rel=preload as a speculative load with priorities, script and style only supported types → Bug 1594449 - <link rel="preload"> implemented as a speculative load initiated during the prescan phase in the HTML5 parser, disabled by default, only supports "script" and "styles" types, r=hsivonen!,ckerschb!,heycam!
Blocks: 1599791
Blocks: 1599793
Blocks: 1598613
Attachment #9106919 - Attachment description: Bug 1594449 - <link rel="preload"> implemented as a speculative load initiated during the prescan phase in the HTML5 parser, disabled by default, only supports "script" and "styles" types, r=hsivonen!,ckerschb!,heycam! → Bug 1594449 - <link rel="preload"> implemented as a speculative load initiated during the prescan phase in the HTML5 parser, disabled by default, only supports "script" and "styles" types, r=ckerschb!

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e5bcacd62570387fd322fa2f470a499a2d290eae for the default network.preload-experimental at false (should not effect anything).

Webcompat Priority: --- → P2
Blocks: 1600117
Blocks: 1600118
Blocks: 1600119
Pushed by honzab.moz@firemni.cz: https://hg.mozilla.org/integration/autoland/rev/ab8d469a79bf <link rel="preload"> implemented as a speculative load initiated during the prescan phase in the HTML5 parser, disabled by default, only supports "script" and "styles" types, r=ckerschb
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla72

Relatedly: We only use this to determine priority. It seems we prioritize
<link rel=preload> over <link rel=stylesheet>, is that intended?

That seems a bit weird, as the preloads from the parser are likely to be used
very soon.

Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/97375be9be30 followup: fix some CSS loader comments. r=mayhemer

Hi there! I was looking at documenting this, but looking through the code updates it seems that we have prefs for this, network.preload/network.preload-experimental, which are currently set to false.

Do we know when this might be enabled in release?

Flags: needinfo?(honzab.moz)

It would be probably good to have a specific bug for enabling the preference in the future. So we could duplicate against it when we meet webcompat issues related to it.

(In reply to Chris Mills (Mozilla, MDN editor) [:cmills] from comment #11)

Hi there! I was looking at documenting this, but looking through the code updates it seems that we have prefs for this, network.preload/network.preload-experimental, which are currently set to false.

Do we know when this might be enabled in release?

We missed 73 and we are very likely about to give this work to the DOM team. So I can't answer, because this is no longer in our hands to prioritize.

Flags: needinfo?(honzab.moz)

We missed 73 and we are very likely about to give this work to the DOM team. So I can't answer, because this is no longer in our hands to prioritize.

No worries! This is useful information — I'll leave it for now and wll pick up documenting this again when we see some more activity.

I cannot find a relevant bug for enabling by default this long awaited feature.
The closest I found is https://bugzilla.mozilla.org/show_bug.cgi?id=1618322 which is about merging network.preload.experimental into network.preload as far as I understand it.
It is not about enabling preload, and it even seems to state the feature is not yet complete:

The experimental pref now controls use of the code in bug 1594449, which as an incomplete implementation of preload as speculative load that cannot be turned on in the field - we announce support for rel=preload, but we don't fully implement it.

Is there any other bug to follow up? Is this actually not yet finished?

(In reply to frederic.delaporte from comment #15)

Is there any other bug to follow up? Is this actually not yet finished?

I've changed title of this bug to more reflect what has been done here. We have landed the code so that performance experiments and other types of evaluations could be made on the code, but it's far from being ready to enable by default. However, preload is right now being worked on.

There is a tracking bug 1583604 with number of dependencies. The plan is to have the feature complete for 77, but more realistically 78.

Summary: Implement <link rel="preload"> in the content process as a speculative load with higher priority → Experimentally implement <link rel="preload"> in the content process as a speculative load with higher priority

So hopefully it should be done for 78, which is also the next ESR. Thanks for the answer and pointer.

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: