-
Notifications
You must be signed in to change notification settings - Fork 9.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
new_audit: lcp-lazy-loaded #12838
new_audit: lcp-lazy-loaded #12838
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @milutin! you pieced together the right info, nicely done :)
we'll probably discuss the exact policy a bit more in our eng sync, so a few more comments might be coming
Co-authored-by: Patrick Hulce <[email protected]>
Sorry for break. I am ready to finish this. I am working on tests to pass checks and on strings. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
taking great shape @milutin thank you! :)
this would also be a great candidate for a smoketest if you'd like to add loading lazy to
lighthouse/lighthouse-cli/test/fixtures/perf/delayed-element.js
Lines 17 to 19 in 47c2bc5
setTimeout(() => { | |
const imgEl = document.createElement('img'); | |
imgEl.src = '../dobetterweb/lighthouse-480x318.jpg'; |
lighthouse/lighthouse-cli/test/smokehouse/test-definitions/perf/expectations.js
Lines 283 to 297 in 47c2bc5
'largest-contentful-paint-element': { | |
score: null, | |
displayValue: '1 element found', | |
details: { | |
items: [ | |
{ | |
node: { | |
type: 'node', | |
nodeLabel: 'section > img', | |
path: '0,HTML,1,BODY,1,DIV,a,#document-fragment,0,SECTION,0,IMG', | |
}, | |
}, | |
], | |
}, | |
}, |
Co-authored-by: Patrick Hulce <[email protected]>
I was looking on smoke tests, it will take me awhile to add one.
Smoke tests failed with difference at TraceElement artifacts. I will look later how to change expected traceElement artifacts to pass smoke test and how to add expectations of failing lcp-lazy-loaded audit. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
excellent job @milutin!
lighthouse-cli/test/smokehouse/test-definitions/perf/expectations.js
Outdated
Show resolved
Hide resolved
Great, thanks a lot for help and guidance! |
Hi folks, we've seen a couple folks ask about this, specifically when it looks like it IS an error it says this: *To be clear, I get what the article and the recommendation are trying to tell us--that above-the-fold images should not be lazy loaded, but I think the verbiage is busted (and probably confusing to non-developers, even if it were corrected). |
Agreed with @nosilver4u here. Maybe verbiage like "Avoid lazy-loading the LCP image" may be more direct |
I just assumed this was just a mistake. I think they meant to say "Largest Contentful Paint image was lazily loaded". They really need to correct this issue it is confusing. |
We have been getting conflicting results with this specific check. On every site we have tested using the Trellis WordPress theme, we have found that not lazy loading the LCP has caused our scores to go down, both on mobile and desktop. We typically see a 50% to 200% increase in performance for the LCP time when we lazy load that image. This makes it difficult when our publishers get the "Largest Contentful Paint image was lazily loaded" diagnostic, with the large red error triangle next to it, but we have not found a single example of this recommendation speeding up the site. Even the Google Developers documentation states the following:
Our interpretation of this is that there is too much emphasis placed on this audit. The potential regression is extremely minimal, and with every site we have tested this showing the exact opposite, we are worried that this perceived requirement will cause more harm than good for website owners using Trellis. |
@sethta what tool are you using to test and are you able to share the results? Also note that we reassessed the performance impact of lazy-loading the LCP image and found clear evidence that it's harmful: https://web.dev/lcp-lazy-loading/ |
We are typically using the Lighthouse audit in the Chrome dev tools, but because most of our publishers on Trellis don't know how to access that, we also use PageSpeed Insights. I've replicated one of our testing sites so it's publicly available, and the results still show that lazy-loading the LCP image is faster on sites using Trellis. These two pages are completely identical with the exception that the primary image is lazy loaded: Lazy Loadedhttps://pagespeed.web.dev/report?url=https%3A%2F%2Ftrellis.alling.dev%2Flcp-lazy-loaded%2F When we run the tests on the page with the lazy loaded image, we consistently get Performance scores of 100 for both Mobile and Desktop results. Other Images: NOT Lazy Loadedhttps://pagespeed.web.dev/report?url=https%3A%2F%2Ftrellis.alling.dev%2Flcp-not-lazy-loaded%2F When we run the tests on the page where we have removed lazy loading, we have never been able to achieve a Performance higher than 96, directly related to slow LCP, for Mobile and the LCP time was consistently slower on Desktop (0.5-0.8s compared to 0.3s). Other Images: |
Thanks for the additional info @sethta. It looks to me like your server might be too inconsistent for these results to be meaningful. Here's a test of 9 runs of the lcp-not-lazy-loaded page and you can see that the backend time to first byte (TTFB) metric varies between about 500ms to 4000ms. That will greatly affect the LCP time, much more so than lazy loading. Interestingly, the test of the lcp-lazy-loaded page shows that TTFB is more consistent. Is it possible that you're doing some kind of slow rewrite on the server to generate the not-lazy-loaded page? Even when we pick tests with similar TTFB times to compare, the eager version still appears to render the image more quickly: The lazy page has a TTFB of 572ms and the eager page has a TTFB of 620ms. However, the lazy page is reported to have an LCP of 653ms while the eager page has an LCP of 1114ms. What's happening is that your lazy version is actually fooling the LCP calculation by drawing a gray box where the image should be: src="data:image/svg+xml,%3Csvg%20xmlns='http://www.w3.org/2000/svg'%20viewBox='0%200%20683%201024'%3E%3Crect%20width='683'%20height='1024'%20style='fill:%23e3e3e3'/%3E%3C/svg%3E" Unbeknownst to the browser, it thinks this inline SVG image is "contentful" so it considers this very early paint time as the LCP. But to a real user, the image doesn't appear loaded until about 1300ms. This is really bad because you're optimizing for something that actually makes the UX worse, despite the metrics seeming to improve. I'd strongly recommend removing the lazy loading behavior from your LCP images. These lab tests confirm that the image will actually render more quickly by loading the images eagerly. Even if your measured LCP seems to get slightly worse, you're making the UX better, which is what matters most. You should also assume that at some point browsers will patch the buggy behavior, so the sooner you move away from relying on it the better. |
@rviscomi Thank you so much for this! We have been banging our heads against the wall for a couple of weeks trying to figure out why we were getting different results. The SVG application was developed a few years ago as a way to handle lazy-loaded images through a Javascript solution before the native browser solution existed, and it has run as a fallback ever since, as it has helped with both speeds and CLS. One of our plans is to remove that application now that Safari supports lazy loaded images, so I think you just saved us from a real headache when we get that feature adjusted. Thanks again! |
Fixes #12785