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

core(lantern): use LCP instead of FMP for TTI simulation bounds #16046

Merged
merged 4 commits into from
Jun 6, 2024
Merged

core(lantern): use LCP instead of FMP for TTI simulation bounds #16046

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
dd01b17
core(lantern): use LCP instead of FMP for TTI simulation bounds
connorjclark Jun 6, 2024
43ed9e5
fix tests
connorjclark Jun 6, 2024
72d6820
fix stale comments
connorjclark Jun 6, 2024
68638c6
fix stale comments
connorjclark Jun 6, 2024
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
2 changes: 1 addition & 1 deletion core/audits/metrics/interactive.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ const str_ = i18n.createIcuMessageFn(import.meta.url, UIStrings);

/**
* @fileoverview This audit identifies the time the page is "consistently interactive".
* Looks for the first period of at least 5 seconds after FMP where both CPU and network were quiet,
* Looks for the first period of at least 5 seconds after FCP where both CPU and network were quiet,
Copy link

Choose a reason for hiding this comment

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

Should this be LCP and not FCP?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yup, fixed over in #16173. thank ya.

* and returns the timestamp of the beginning of the CPU quiet period.
* @see https://docs.google.com/document/d/1GGiI9-7KeY3TPqS3YT271upUVimo-XiL5mwWorDUD4c/edit#
*/
Expand Down
6 changes: 3 additions & 3 deletions core/computed/metrics/lantern-interactive.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/

import {makeComputedArtifact} from '../computed-artifact.js';
import {LanternFirstMeaningfulPaint} from './lantern-first-meaningful-paint.js';
import {LanternLargestContentfulPaint} from './lantern-largest-contentful-paint.js';
import {Interactive} from '../../lib/lantern/metrics/interactive.js';
import {getComputationDataParams, lanternErrorAdapter} from './lantern-metric.js';

Expand All @@ -29,8 +29,8 @@ class LanternInteractive extends Interactive {
* @return {Promise<LH.Artifacts.LanternMetric>}
*/
static async compute_(data, context) {
const fmpResult = await LanternFirstMeaningfulPaint.request(data, context);
return this.computeMetricWithGraphs(data, context, {fmpResult});
const lcpResult = await LanternLargestContentfulPaint.request(data, context);
return this.computeMetricWithGraphs(data, context, {lcpResult});
}
}

Expand Down
2 changes: 1 addition & 1 deletion core/lib/lantern/metric.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import {RESOURCE_TYPES} from '../../lib/network-request.js';
* @typedef Extras
* @property {boolean} optimistic
* @property {LH.Artifacts.LanternMetric=} fcpResult
* @property {LH.Artifacts.LanternMetric=} fmpResult
* @property {LH.Artifacts.LanternMetric=} lcpResult
* @property {LH.Artifacts.LanternMetric=} interactiveResult
* @property {{speedIndex: number}=} speedline
*/
Expand Down
14 changes: 7 additions & 7 deletions core/lib/lantern/metrics/interactive.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,12 +66,12 @@ class Interactive extends Metric {
* @return {LH.Gatherer.Simulation.Result}
*/
static getEstimateFromSimulation(simulationResult, extras) {
if (!extras.fmpResult) throw new Error('missing fmpResult');
if (!extras.lcpResult) throw new Error('missing lcpResult');

const lastTaskAt = Interactive.getLastLongTaskEndTime(simulationResult.nodeTimings);
const minimumTime = extras.optimistic
? extras.fmpResult.optimisticEstimate.timeInMs
: extras.fmpResult.pessimisticEstimate.timeInMs;
? extras.lcpResult.optimisticEstimate.timeInMs
: extras.lcpResult.pessimisticEstimate.timeInMs;
return {
timeInMs: Math.max(minimumTime, lastTaskAt),
nodeTimings: simulationResult.nodeTimings,
Expand All @@ -84,13 +84,13 @@ class Interactive extends Metric {
* @return {Promise<LH.Artifacts.LanternMetric>}
*/
static async compute(data, extras) {
const fmpResult = extras?.fmpResult;
if (!fmpResult) {
throw new Error('FMP is required to calculate the Interactive metric');
const lcpResult = extras?.lcpResult;
if (!lcpResult) {
throw new Error('LCP is required to calculate the Interactive metric');
}

const metricResult = await super.compute(data, extras);
metricResult.timing = Math.max(metricResult.timing, fmpResult.timing);
metricResult.timing = Math.max(metricResult.timing, lcpResult.timing);
return metricResult;
}

Expand Down
12 changes: 10 additions & 2 deletions core/test/audits/byte-efficiency/offscreen-images-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,11 @@ describe('OffscreenImages audit', () => {
src: recordB.url,
}),
],
traces: {defaultPass: createTestTrace({topLevelTasks, networkRecords})},
traces: {defaultPass: createTestTrace({
largestContentfulPaint: 15,
topLevelTasks,
networkRecords,
})},
devtoolsLogs: {defaultPass: devtoolsLog},
URL: {
requestedUrl: recordA.url,
Expand Down Expand Up @@ -523,7 +527,11 @@ describe('OffscreenImages audit', () => {
src: recordB.url,
}),
],
traces: {defaultPass: createTestTrace({topLevelTasks, networkRecords})},
traces: {defaultPass: createTestTrace({
largestContentfulPaint: 15,
topLevelTasks,
networkRecords,
})},
devtoolsLogs: {defaultPass: devtoolsLog},
URL: {
requestedUrl: recordA.url,
Expand Down
1 change: 1 addition & 0 deletions core/test/audits/dobetterweb/dom-size-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ describe('DOMSize audit', () => {
const mainDocumentUrl = 'https://example.com/';
const networkRecords = [{url: mainDocumentUrl, priority: 'High'}];
const trace = createTestTrace({
largestContentfulPaint: 15,
topLevelTasks: [
{ts: 1000, duration: 1000, children: [
{ts: 1100, duration: 200, eventName: 'ScheduleStyleRecalculation'},
Expand Down
1 change: 1 addition & 0 deletions core/test/audits/long-tasks-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ function generateTraceWithLongTasks(args) {
traceTasks.push(task);
}
return createTestTrace({
largestContentfulPaint: BASE_TS + 15,
topLevelTasks: traceTasks,
timeOrigin: BASE_TS,
traceEnd: BASE_TS + 20_000,
Expand Down
7 changes: 6 additions & 1 deletion core/test/audits/redirects-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,12 @@ describe('Performance: Redirects audit', () => {
const devtoolsLog = networkRecordsToDevtoolsLog(networkRecords);
const frameUrl = networkRecords[0].url;

const trace = createTestTrace({frameUrl, traceEnd: 5000, networkRecords});
const trace = createTestTrace({
frameUrl,
largestContentfulPaint: 15,
traceEnd: 5000,
networkRecords,
});
const navStart = trace.traceEvents.find(e => e.name === 'navigationStart');
navStart.args.data.navigationId = '1';

Expand Down
28 changes: 24 additions & 4 deletions core/test/audits/third-party-facades-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,12 @@ describe('Third party facades audit', () => {
devtoolsLogs: {
defaultPass: networkRecordsToDevtoolsLog(networkRecords),
},
traces: {defaultPass: createTestTrace({timeOrigin: 0, traceEnd: 2000, networkRecords})},
traces: {defaultPass: createTestTrace({
timeOrigin: 0,
largestContentfulPaint: 15,
traceEnd: 2000,
networkRecords,
})},
URL: {
requestedUrl: 'https://example.com',
mainDocumentUrl: 'https://example.com',
Expand Down Expand Up @@ -99,7 +104,12 @@ describe('Third party facades audit', () => {
devtoolsLogs: {
defaultPass: networkRecordsToDevtoolsLog(networkRecords),
},
traces: {defaultPass: createTestTrace({timeOrigin: 0, traceEnd: 2000, networkRecords})},
traces: {defaultPass: createTestTrace({
timeOrigin: 0,
largestContentfulPaint: 15,
traceEnd: 2000,
networkRecords,
})},
URL: {
requestedUrl: 'https://example.com',
mainDocumentUrl: 'https://example.com',
Expand Down Expand Up @@ -175,7 +185,12 @@ describe('Third party facades audit', () => {
devtoolsLogs: {
defaultPass: networkRecordsToDevtoolsLog(networkRecords),
},
traces: {defaultPass: createTestTrace({timeOrigin: 0, traceEnd: 2000, networkRecords})},
traces: {defaultPass: createTestTrace({
timeOrigin: 0,
largestContentfulPaint: 15,
traceEnd: 2000,
networkRecords,
})},
URL: {
requestedUrl: 'https://example.com',
mainDocumentUrl: 'https://example.com',
Expand Down Expand Up @@ -226,7 +241,12 @@ describe('Third party facades audit', () => {
devtoolsLogs: {
defaultPass: networkRecordsToDevtoolsLog(networkRecords),
},
traces: {defaultPass: createTestTrace({timeOrigin: 0, traceEnd: 2000, networkRecords})},
traces: {defaultPass: createTestTrace({
timeOrigin: 0,
largestContentfulPaint: 15,
traceEnd: 2000,
networkRecords,
})},
URL: {
requestedUrl: 'https://intercomcdn.com',
mainDocumentUrl: 'https://intercomcdn.com',
Expand Down
4 changes: 2 additions & 2 deletions core/test/computed/metrics/interactive-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ describe('Metrics: TTI', () => {
assert.deepEqual(result.networkQuietPeriod, {start: 0, end: traceEnd / 1000});
});

it('should throw when trace ended too soon after FMP', () => {
it('should throw when trace ended too soon after FCP', () => {
const timeOrigin = 220023532;
const firstContentfulPaint = 2500 * 1000 + timeOrigin;
const traceEnd = 5000 * 1000 + timeOrigin;
Expand Down Expand Up @@ -190,7 +190,7 @@ describe('Metrics: TTI', () => {
);

const cpu = [
// quiet period before FMP
// quiet period before FCP
{start: 9000, end: 9900},
{start: 11000, end: 13000},
// quiet period during network activity
Expand Down
1 change: 1 addition & 0 deletions core/test/computed/tbt-impact-tasks-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ describe('TBTImpactTasks', () => {
beforeEach(() => {
metricComputationData = {
trace: createTestTrace({
largestContentfulPaint: 15,
traceEnd: 10_000,
frameUrl: mainDocumentUrl,
topLevelTasks: [
Expand Down
6 changes: 3 additions & 3 deletions core/test/fixtures/lantern-baseline-accuracy.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@
"p95": 0.6524087591240876
},
"roughEstimateOfTTI": {
"p50": 0.28936170212765955,
"p90": 0.6481503092542887,
"p95": 0.7325279801030379
"p50": 0.27433435181143606,
"p90": 0.583952617757166,
"p95": 0.7259561558741738
},
"roughEstimateOfLCP": {
"p50": 0.20067905646890635,
Expand Down
Loading
Loading