amppackager
amppackager copied to clipboard
ampproject
Resource hints are misprioritised in the reorderhead transformer
The reorderhead transformer puts the resource hints at the back of the head (see entry (8)):
// ReorderHead reorders the children of <head>. Specifically, it
// orders the <head> like so:
// (0) <meta charset> tag
// (1) <style amp-runtime> (inserted by ampruntimecss.go)
// (2) remaining <meta> tags (those other than <meta charset>)
// (3) AMP runtime .js <script> tag
// (4) AMP viewer runtime .js <script> tag
// (5) <script> tags that are render delaying
// (6) <script> tags for remaining extensions
// (7) <link> tag for favicons
// (8) <link> tag for resource hints
// (9) <link rel=stylesheet> tags before <style amp-custom>
// (10) <style amp-custom>
// (11) any other tags allowed in <head>
// (12) AMP boilerplate (first style amp-boilerplate, then noscript)
This means that resource hints that are provided to meet the performance guidelines in the Amp documentation will reordered to only appear after the actual download triggers they are meant to optimize.
I think that either the entry (8) should be brought up to appear after (0) in general, or resource hints should be split into two groups where the Amp-related ones are brought higher while the rest stay in (8).
cc @sebastianbenz
Good catch. I think this is not a problem for SXG, because amppkg adds preload headers, which should serve two functions:
- hinting priority to the browser
- hinting recursive subresource prefetch to the browser, when a site prefetches an SXG
Please correct me if that's wrong.
I guess it would a blocker for Optimizer adopting the amppkg transform library. Is that the goal? If so, I suspect that the splitting idea you suggest should solve the problem while being neutral for SXG. That said, I'd like to loop in @jridgewell for his expertise, given related work on http://b/129565737.
(A fallback solution is to add a new transformer and a new OPTIMIZER config that runs it after reorderhead.)
(Background for the SXG header decision at http://b/111845688.)
@twifkak is right, all the resource hints are extracted into link headers.
If I understand correctly, the link headers would only apply in the case of an SXG prefetch?
So if the transformers are used outside of the SXG context, this would still be sub-optimal, and fixing it for the transformers would not have any negative effect on the SXG.
Are you open for a PR for splitting the resource hints into two groups with two different priorities, based on the host domain?
Yes, I'm open to such a PR. It shouldn't just be domain -- we'll want to limit by path, too.
Note that using the transformers for on-origin will likely require other changes that are not SXG-neutral (e.g. ability to disable subresource URL rewriting). If that's the goal, then I have more questions:
- Should we start a separate OPTIMIZE config now, to begin encapsulating these differences?
- Last I heard, the Go transforms can't be compiled into JS that well, and folks didn't want AMP optimizer to require Go be installed, so we'll need both ports. Is it okay if they drift a bit, or do we have a plan to maintain parity?
Just to dig into SXG a bit more, there are three cases:
- User visits
https://foo.example/-- per our frontend recommendation and backend checks, this page should never run through the transforms or be signed -- the original AMP would be served, and this may contain the appropriate link tags or link headers, as they are considered valid AMP. - User visits
https://foo-example.ampcache.example/wp/s/foo.example/with an SXG-accepting browser -- it will include theLink: rel=preloadheaders. - User visits
https://foo-example.ampcache.example/c/s/foo.example/with an SXG-non-accepting browser -- the AMP cache will serve unsigned HTML (either extracted from the SXG it fetched from origin, or from a non-SXG fetch), and thus must reapply AMP transforms (in this case, adding theLinkheaders).
So, all of these cases involve link-rel-preloads, I think.
Should we start a separate OPTIMIZE config now, to begin encapsulating these differences?
+1
Last I heard, the Go transforms can't be compiled into JS that well, and folks didn't want AMP optimizer to require Go be installed, so we'll need both ports. Is it okay if they drift a bit, or do we have a plan to maintain parity?
I think we should try to maintain parity (in particular feature wise), but some drift is OK imo. For example, I've just added auto extension import and missing AMP tag injection to Optimizer which I'm not sure is something we'd need for the Go implementation (as these are mainly aimed towards easing CMS / framework integration).
Correct me if I'm misunderstanding, but I think this is working as intended already, other than the amp.dev example documentation.
In the amp.dev example, we see:
<link rel="preload" as="script" href="https://cdn.ampproject.org/v0.js">
<link rel="preload" as="script" href="https://cdn.ampproject.org/v0/amp-experiment-0.1.js">
<script async src="https://cdn.ampproject.org/v0.js"></script>
<script async custom-element="amp-experiment" src="https://cdn.ampproject.org/v0/amp-experiment-0.1.js"></script>
Why is there a preload tag for a file that is then immediately loaded? What purpose does the link preload tag serve that the script tag does not? This is maybe the crux of my misunderstanding.
IIUC, the only reason we insert these preload tags for Signed Exchanges is as a hint to later code to hoist the preloads into HTTP headers on the unsigned portion of the signed exchange. They aren't for browser consumption, because once the browser is parsing the link tags, it's also parsing the script tags.
There are also potentially preloads for less critical resources, such as preloading an image or a font. Those have value even if not hoisted into headers, but they should be prioritized below the critical resources like v0.js, not ahead of them.
