Open philipwalton opened 2 years ago
The only concern I have with this proposal is the aspect of removing the preload advice entirely, because Priority Hints is a Chromium-only feature, and as your HA analysis shows, "preload the lcp element" is measurably beneficial (I am assuming it's useful to derive something about non-Chromium browsers from HA analysis)*. When possible, we like to keep the performance advice in LH browser-agnostic; or at the least not harmful for non-Chromium browsers, which changing our check from "use preload" to "nevermind, use fetchpriority instead" may be.
As a stopgap, could we recommend both, or does preloading the image at the same time result in unwanted behavior?
And for the case where the LCP image is (for some reason) dynamically added to the page, I think the preload audit still makes sense. If doing both is potentially OK, we may want to keep the preload lcp audit (which is only ever triggered when the preload scanner wouldn't discover it) and introduce a new fetchpriority
audit (not a full replacement).
* In isolation, would you agree that "preload the LCP element" results in better performance than not doing that?
Are you aware of any preference for where to set fetchpriority
? https://wicg.github.io/priority-hints/#:~:text=Providing%20Priority%20Hints%20through%20HTTP%20Headers
My take is that the link
header would be best, but not as necessary if the element is in the initial markup. If the image is created dynamically, doing it via the link
would have no affect unless accompanied with a preload
directive (right?).
and setting it via JS el.fetchpriority = ...
is just the worst but we don't have a good way in Lighthouse to distinguish that from it existing in the original markup, so we'll probably have to ignore the distinction.
Maybe it boils down to:
1) if the image is in the initial markup: just add fetchpriority
to the img element
2) otherwise, you should really have a link header that preloads and sets the fetchpriority
The only concern I have with this proposal is the aspect of removing the preload advice entirely, because Priority Hints is a Chromium-only feature
Agreed. Re-reading my original issue I realize that it wasn't at all clear that I was suggesting Lighthouse recommend Priority Hints in addition to preload.
As a stopgap, could we recommend both, or does preloading the image at the same time result in unwanted behavior?
I think it's OK to recommend the use of both (I've tested and did not see any negative results), though I think the Lighthouse audit shouldn't require the use of both in order to pass.
Also depending upon how a site is structured, it may not need either in order to pass, which is why I recommended basing the status and opportunity on whether or not the LCP element resource started loading at the same time as the first resource.
Here's what I was thinking:
fetchpriority
on either the image element itself or a preload element referencing the resource:
a. If fetchpriority
is found, pass the audit regardless
b. If fetchpriority
is not found (regardless of whether a preload
tag is found), recommend the use of either fetchpriority
or an early <preload>
tag/Link
header.The reason for 2.b. above is I've seen many example where a site uses preload
but the resource is still not prioritized because the preload tag is placed below other links tags in the head (e.g. stylesheets, fonts, etc), for example this trace:
Moving the preload tag to the first position in the <head>
solves when the resource starts loading, but I'm not sure this is a desirable outcome because I'm not sure we want people to preload an LCP resource ahead of a blocking stylesheet.
In this screenshot the LCP image is loaded first but with a "low" priority. I'm not sure how that affects the loading of the stylesheets (if at all) which are loaded after but with a "highest" priority. (@pmeenan any opinion on whether this is good/bad?)
Either way, for many sites or platforms it may not be possible to set a link header or tag (e.g. cases where all pages on a site share a common <head>
template), and in these cases fetchpriority
makes a lot more sense.
Lastly, if a site is UA detecting on the server-side, they may be setting just fetchpriority
for Chrome (and thus also Lighthouse), but that doesn't necessarily mean they're not supporting other browsers.
Are you aware of any preference for where to set
fetchpriority
? https://wicg.github.io/priority-hints/#:~:text=Providing%20Priority%20Hints%20through%20HTTP%20Headers
In my testing, using a Link
header vs using a <link>
tag didn't make a difference in any real-world scenarios. I was able to see a difference if I intentionally set a timeout inside a streaming server response (e.g. flushed only part of the <head>
before flushing the parts with the preload tags), but in my normal tests (even on a throttled network) I didn't see any difference if the <link>
tags were near the beginning of the <head>
.
and setting it via JS
el.fetchpriority = ...
is just the worst but we don't have a good way in Lighthouse to distinguish that from it existing in the original markup, so we'll probably have to ignore the distinction.
Ahh, I didn't realize that distinction wasn't possible. However, I think looking at the delta between the first resource start time and the LCP resource start time could help here, so potentially ignore my suggestion in 2.a. above?
FWIW, preload helping may also be specific to Chromium (at least in most cases). Chromium is the only engine with the 2-phase loading where it holds back requests for low-priority resources before the parser gets to the body (as long as there are any other high-priority requests in flight, low priority won't be requested).
Webkit and Gecko both fetch resources as soon as they are discovered, assuming the HTTP/1 connection limit hasn't been reached.
In our meeting today we reach consensus on
1) keeping preload-lcp-image, because it is still important for cases where the image resource (which will become the LCP element) is dynamically added to the page 2) adding priortize-lcp-image, because... well, read this issue.
Paul mentions that in order to accurately estimate the opportunity savings of fetchpriority
, we will need to add to Lantern a concept of queuing time and/or connection setup time.
(oops, hit the wrong button)
@philipwalton should we update https://web.dev/optimize-lcp/ too?
Started researching the lantern side of this. Lantern does hold back starting [1] new network requests based on how many requests are currently in-flight for a given connection [2]. Unstarted network nodes are started by selecting the first node in a maintained array of unstarted network nodes sorted by "start position", which is basically the node's startTime
but uses network priority as a kludge for tiebreakers [3].
Given all that, I think Lantern may already support all we need to simulate priority hints. If we modify node.record.priority
then the tiebreaker/penalty logic kicks in for that node, making Lantern prioritize starting a network node earlier (if the priority is raised from "low" to "high"). Whether the penalties [4] are accurate shouldn't be assumed, although they were selected to minimize errors in LCP estimations [5].
[5] https://github.com/GoogleChrome/lighthouse/pull/10529/files#r409600098
BTW for anyone using the URL provided in OP as test case, it only has an image for LCP in desktop mode. for mobile LCP is the header text:
@philipwalton should we update https://web.dev/optimize-lcp/ too?
Yeah, I plan to update that post ahead of I/O (to coordinate with some other recommendations).
From latest discussions:
Seems optimal to move back to the prioritize-lcp-image
proposal in @philipwalton's original comment and focus on
fetchpriority="high"
on LCP@brendankenny to publish updated version of preload/fetchpriority analysis from HTTP Archive data now that we have explicit priority information in the LHR
Also a couple of related correctness issues: https://github.com/GoogleChrome/lighthouse/issues/14350 and https://github.com/GoogleChrome/lighthouse/issues/14300
FWIW, preload helping may also be specific to Chromium (at least in most cases). Chromium is the only engine with the 2-phase loading where it holds back requests for low-priority resources before the parser gets to the body (as long as there are any other high-priority requests in flight, low priority won't be requested).
Webkit and Gecko both fetch resources as soon as they are discovered, assuming the HTTP/1 connection limit hasn't been reached.
Are you sure about this @pmeenan ? Here's a test Safari where you can see the hero image being held back.
Adding a preload in this case (not shown but I tested locally), doesn't help (nor hinder), and they don't support fetch priority so can't get it started earlier in anyway.
(though there's an edge case where adding preload as the first resource in the <head>
would help even without fetchpriority
- as it would with Chrome - but don't think that's recommended.)
And here's Firefox:
Preloading the image also doesn't help. Though interestingly preloading it as the first asset also doesn't help (even though it does with Chrome and Safari) - Firefox still holds it back.
So anyway, suggesting preload does not help any browser if the asset is discoverable in the HTML. With two small edge cases:
<header>
). fetchpriority="high"
in Chromium is better to force this rather than depend on HTML.In both cases I think preloading (and maintaining two references in the HTML doc, and ensuring they don't fall out of sync) is more hassle than it's worth to solve them, so would not suggest actively promoting them unless we find this happens a lot.
- was LCP discoverable in initial document? If not, either make it discoverable or preload it
- was LCP fetched with high priority? If not (or if suggesting preload), suggest fetchpriority="high" on LCP
@brendankenny an image will never be fetched as high priority (unless using fetchpriority="high"
), unless it's in the viewport which is not known until after the initial render (at which point the priority will change, so not sure if there will be race conditions?).
So might be simpler to say:
was LCP discoverable in initial document?
- If not, either make it discoverable, or preload it with
fetchpriority="high"
- If it was discoverable, ensure
fetchpriority="high"
is set on it and suggest that if not.
As I say I don't see that having any downside to Safari or Firefox.
Seems optimal to move back to the
prioritize-lcp-image
proposal in @philipwalton's original comment and focus on
- was LCP discoverable in initial document? If not, either make it discoverable or preload it
- was LCP fetched with high priority? If not (or if suggesting preload), suggest
fetchpriority="high"
on LCP
@brendankenny I wonder if there's value in reporting these as separate audits. Could it be useful to differentiate each opportunity and provide more targeted guidance?
Also, would it be possible to get more diagnostic information about why the resource wasn't discoverable? (ie CSS background image vs CSR)
Tactically:
"HTMLDocumentParser::FetchQueuedPreloads"
trace event we have enough to know that a req was discoverable by preload scanner blink
cat, but we should add devtools.timeline
(hasFetchPriority && (networkRequestInitatedDuringPreloadScanner || hasAPreload))
and we suggest both fetchpriority and html-img-tag or preloadCurrently unanswered:
fix something about the simulation that brendan mentioned
@brendankenny can you expand? Sorry we didn't capture it during one of our many meetings on this :(
or is this just the whole "queuing is non-existent in lantern" thing
edit: brendan thinks it may have to do with orphaned network requests being reparented to the main doc.. but he's not sure.
Is there an update on this audit?
Is there an update on this audit?
Yeah good question. Reviewing the list from above and commit history...
- renaming preload-lcp-image.js to prioritize-lcp-image
done in #14761
- fix the initiator length assertion, as its wrong -- already fixed in core(preload-lcp-image): fix faulty request chain exclusion logic #14404
done in #14807
- fix something about the simulation that brendan mentioned. (might have to do with orphaned network requests being reparented to the main doc.. but he's not sure.)
- add fetchpriority gathering
done in #14925
- should we also merge lcp-lazy-loaded.js into this?
- incorporate fetchpriority analysis in the audit
- ultimately, we validate that the image
(hasFetchPriority && (networkRequestInitatedDuringPreloadScanner || hasAPreload))
and we suggest both fetchpriority and html-img-tag or preload
these don't appear to be landed, but maybe we deliberately went a different direction?
- figure out how we're approaching estimatedSavings for this audit. We could.. [...]
looks like we simulate the LCP image getting preload, but not fetchpriority.
@brendankenny does this summary look right to you?
reviewed these with brendan. quick notes:
should we also merge lcp-lazy-loaded.js into this?
we could. we didn't think much about it. still open if we care.
incorporate fetchpriority analysis in the audit
it's not, no. lantern needs to be taught about fetchpriority and its hard so we didn't do that (yet)
figure out how we're approaching estimatedSavings for this audit. We could.. [...]
this is "can we work around the fact that lantern's analysis could be better?". adam added the metricSavingsScoreMode that gets us most of the way there. "even if there are no savings.. we can still mark/score it so that it shows up in the non-passing list (with orange)"
ultimately, we validate that the image (hasFetchPriority && (networkRequestInitatedDuringPreloadScanner || hasAPreload)) and we suggest both fetchpriority and html-img-tag or preload
Yeah we discussed a more tailored advice audit.. (related impl) Instead of the audit being "please preload the lcp image" it would be more dynamic and recommend fetchpriority or inline or preload depending on the circumstances. Still open.
it's not, no. lantern needs to be taught about fetchpriority and its hard so we didn't do that (yet)
had no success in that area when I attempted https://github.com/GoogleChrome/lighthouse/pull/13811
figure out how we're approaching estimatedSavings for this audit. We could.. [...]
this is "can we work around the fact that lantern's analysis could be better?". adam added the metricSavingsScoreMode that gets us most of the way there. "even if there are no savings.. we can still mark/score it so that it shows up in the non-passing list (with orange)"
https://github.com/GoogleChrome/lighthouse/issues/15636#issuecomment-1881968879 is a good example of this improvement. Even with the simulation failing to show any improvement, it still gets flagged for the user to look at because there were multiple dependent network loads to get the LCP.
https://github.com/GoogleChrome/lighthouse/issues/15737 can serve to track (non-fetchpriority) simulator improvements
ultimately, we validate that the image (hasFetchPriority && (networkRequestInitatedDuringPreloadScanner || hasAPreload)) and we suggest both fetchpriority and html-img-tag or preload
I believe hasFetchPriority
is the last remaining check from this to add. A simple approach would be to add a fetchpriority check to the score
based on the initiator path length
The results are somewhat opaque and could benefit from:
Yeah we discussed a more tailored advice audit.. Instead of the audit being "please preload the lcp image" it would be more dynamic and recommend fetchpriority or inline or preload depending on the circumstances. Still open.
Notably the current advice is to always preload. What it should be (basically):
here is how you loaded your LCP:
index.html
➡️ script.js
➡️ ➡️ image.png
or
here's how you should have loaded it:
The case in #15636 is tricky though because it already is inline, just with a homegrown lazy loading library. Is detecting and handling the 80% of custom lazy loading cases ok and the audit could give custom advice knowing that the image is already inline and should just stop lazy loading (rather than suggesting inlining the image or preloading it and potentially confusing the developer?)
Feature request summary
tl;dr rename the
preload-lcp-image
audit toprioritize-lcp-image
, and update it to recommend the use of Priority Hints (shipping in Chrome 101) in cases where the LCP image is found within the main document but queued after other resources.What is the motivation or use case for changing this?
Currently, the
preload-lcp-image
does not apply to cases where the LCP image was discoverable from within the main document (see relevant audit logic).This potentially misses a big opportunity for sites to improve LCP because in many cases—even though those elements are discoverable from the main document—they're fetched with a low priority and often end up queued up waiting for other resources to finish.
Here's an example of that happening on httparchive.org
In this screenshot you can see that the LCP image (the red outline) is discovered immediately, but it's assigned a "low" priority, so it still gets queued after other resources (e.g. font/stylesheets). In other words, there is clear opportunity to start loading the LCP image sooner, but Lighthouse currently does not highlight this opportunity.
If this site were updated to use Priority Hints and added
fetchpriority="high"
to the LCP image tag, the trace would look like this:Notice how now the image is fetched in parallel with the fonts and stylesheets, and LCP happens at the same time as FCP.
As for what the opportunity value should be: my suggestion is to take the delta between the
startTime
of the first resource and therequestStart
of the LCP resource. Note that I recommend looking atrequestStart
rather thanstartTime
for the LCP resource becausestartTime
can be the point when the resource was queued (as in the first screenshot above), and the time that's important is the time the network request started.How is this beneficial to Lighthouse?
The current
preload-lcp-image
audit misses lots of opportunity to improve. I recently analyzed all mobile page loads in HTTP Archive where LCP was an image (5M pages) to look at the time delta between when the first resource started loading and when the LCP resource started loading, and those times were surprisingly high.preload-lcp-audit
?This table shows that pages that pass the audit are doing better than pages that do not pass it, but even for pages that pass the audit, a substantial amount of total LCP time is spent waiting for the LCP resource to start loading.
cc: @paulirish since we discussed this yesterday.