Skip to content
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.

Sign up for GitHub

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

Jump to bottom

core(lcp-lazy-loaded): add LCP savings estimate #15064

Merged
merged 5 commits into from
Aug 15, 2023
Merged

core(lcp-lazy-loaded): add LCP savings estimate #15064

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9866bd1
core(lcp-lazy-loaded): metric impact estimate
adamraine May 11, 2023
d003fd5
Merge branch 'main' into lcp-lazy-impact
adamraine May 18, 2023
e075624
new
adamraine May 18, 2023
e62f2cc
Merge branch 'main' into lcp-lazy-impact
adamraine Aug 7, 2023
2521057
estimate
adamraine Aug 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions core/audits/audit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,19 @@ class Audit {
details: product.details,
};
}

/**
*
* @param {LH.Artifacts} artifacts
* @param {LH.Audit.Context} context
* @returns {LH.Artifacts.MetricComputationDataInput}
*/
static makeMetricComputationDataInput(artifacts, context) {
const trace = artifacts.traces[Audit.DEFAULT_PASS];
const devtoolsLog = artifacts.devtoolsLogs[Audit.DEFAULT_PASS];
const gatherContext = artifacts.GatherContext;
return {trace, devtoolsLog, gatherContext, settings: context.settings, URL: artifacts.URL};
}
}

export {Audit};
23 changes: 19 additions & 4 deletions core/audits/lcp-lazy-loaded.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import {Audit} from './audit.js';
import * as i18n from '../lib/i18n/i18n.js';
import {LCPBreakdown} from '../computed/metrics/lcp-breakdown.js';

const UIStrings = {
/** Title of a Lighthouse audit that provides detail on whether the largest above-the-fold image was loaded with sufficient priority. This descriptive title is shown to users when the image was loaded properly. */
Expand All @@ -29,7 +30,8 @@ class LargestContentfulPaintLazyLoaded extends Audit {
failureTitle: str_(UIStrings.failureTitle),
description: str_(UIStrings.description),
supportedModes: ['navigation'],
requiredArtifacts: ['TraceElements', 'ViewportDimensions', 'ImageElements'],
requiredArtifacts: ['TraceElements', 'ViewportDimensions', 'ImageElements',
'traces', 'devtoolsLogs', 'GatherContext', 'URL'],
};
}

Expand All @@ -46,9 +48,10 @@ class LargestContentfulPaintLazyLoaded extends Audit {

/**
* @param {LH.Artifacts} artifacts
* @return {LH.Audit.Product}
* @param {LH.Audit.Context} context
* @return {Promise<LH.Audit.Product>}
*/
static audit(artifacts) {
static async audit(artifacts, context) {
const lcpElement = artifacts.TraceElements.find(element => {
return element.traceEventType === 'largest-contentful-paint' && element.type === 'image';
});
Expand All @@ -73,8 +76,20 @@ class LargestContentfulPaintLazyLoaded extends Audit {
},
]);

const wasLazyLoaded = lcpElementImage.loading === 'lazy';

const metricComputationData = Audit.makeMetricComputationDataInput(artifacts, context);
const lcpBreakdown = await LCPBreakdown.request(metricComputationData, context);
let lcpSavings = 0;
if (wasLazyLoaded && lcpBreakdown.loadStart !== undefined) {
// We don't know when the LCP resource was first "seen" before being lazy loaded.
// Our best estimate for LCP savings is the entire LCP load delay.
lcpSavings = lcpBreakdown.loadStart - lcpBreakdown.ttfb;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe lcpImageRequest.networkRequestTime - lcpImageRequest.rendererStartTime is this time (though still not entirely fair to blame all of it on the lazy loading)? Unless there's some optimizations like e.g. the preload scanner skipping loading="lazy" resources and letting the full parser handle it

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe lcpImageRequest.networkRequestTime - lcpImageRequest.rendererStartTime is this time

From my testing this isn't the case (although would be nice if it was), rendererStartTime is not when the request was "discovered" but when the renderer decided to start the lazy request.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And turns out

Unless there's some optimizations like e.g. the preload scanner skipping loading="lazy" resources and letting the full parser handle it

is indeed a thing:

if (type == ResourceType::kImage && is_img &&
    IsLazyLoadImageDeferable(document_parameters)) {
  return nullptr;
}

:)

It actually makes sense that the renderer would be the owner of this because it has to know about layout, current viewport etc, but I didn't think about that.

I guess we know it's bounded by at least ttfb and rendererStartTime, then? Still not great though. My main issue with this wide estimate is that it makes it as big an opportunity as using fetchpriority=high, when it almost never will be.

Including loading=lazy is just a little bit worse than having no fetchpriority/loading attributes on the image at all, because the browser does eventually figure out what's going on once layout occurs. See the testing in https://web.dev/lcp-lazy-loading/#testing-a-fix, for instance

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Longe term, if we fold this advice into prioritize-lcp-image this savings estimate will make more sense.

One thing we could do is take a statistical guess at the savings. Apply a 13%/15% (https://web.dev/lcp-lazy-loading/#causal-performance) savings to LCP and bound by ttfb and rendererStartTime.

}
details.metricSavings = {LCP: lcpSavings};

return {
score: lcpElementImage.loading === 'lazy' ? 0 : 1,
score: wasLazyLoaded ? 0 : 1,
details,
};
}
Expand Down