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.

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(mpfid): add list of loaf durations to debugdata #15685

Merged
merged 3 commits into from
Dec 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
44 changes: 30 additions & 14 deletions core/audits/metrics/max-potential-fid.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,14 @@
import {ProcessedNavigation} from '../../computed/processed-navigation.js';
import * as i18n from '../../lib/i18n/i18n.js';

/**
* @typedef LoafDebugDetails
* @property {'debugdata'} type
* @property {LH.TraceEvent=} observedMaxDurationLoaf
* @property {LH.TraceEvent=} observedMaxBlockingLoaf
* @property {Array<{startTime: number, duration: number, blockingDuration: number}>} observedLoafs
*/

const UIStrings = {
/** Description of the Maximum Potential First Input Delay metric that marks the maximum estimated time between the page receiving input (a user clicking, tapping, or typing) and the page responding. This description is displayed within a tooltip when the user hovers on the metric name to see more. No character length limits. The last sentence starting with 'Learn' becomes link text to additional documentation. */
description: 'The maximum potential First Input Delay that your users could experience is the ' +
Expand Down Expand Up @@ -55,43 +63,51 @@
* debugdata details.
* @param {LH.Artifacts.ProcessedTrace} processedTrace
* @param {LH.Artifacts.ProcessedNavigation} processedNavigation
* @return {{type: 'debugdata', observedMaxDurationLoaf: LH.TraceEvent, observedMaxBlockingLoaf: LH.TraceEvent}|undefined}
* @return {LoafDebugDetails|undefined}
*/
static getLongAnimationFrameDetails(processedTrace, processedNavigation) {
const {firstContentfulPaint} = processedNavigation.timestamps;
const loafs = processedTrace.mainThreadEvents.filter(evt => {
return evt.name === 'LongAnimationFrame' &&
evt.ph === 'b' &&
evt.ts >= firstContentfulPaint;
const {firstContentfulPaint, timeOrigin} = processedNavigation.timestamps;

const loafEvents = processedTrace.mainThreadEvents.filter(evt => {
return evt.name === 'LongAnimationFrame' && evt.ph === 'b';
});
if (loafEvents.length === 0) return;
Copy link
Member Author

Choose a reason for hiding this comment

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

This case won't really hurt anything without this check, so is mostly for aesthetics (prevent empty {type: 'debugdata', observedLoafs: []} hanging around in LHRs)


let currentMaxDuration = -Infinity;
let currentMaxDurationLoaf;
let currentMaxBlocking = -Infinity;
let currentMaxBlockingLoaf;

for (const loaf of loafs) {
const loafDuration = loaf.args?.data?.duration;
const loafBlocking = loaf.args?.data?.blockingDuration;
const observedLoafs = [];
for (const loafEvent of loafEvents) {
const loafDuration = loafEvent.args?.data?.duration;
const loafBlocking = loafEvent.args?.data?.blockingDuration;
// Should never happen, so mostly keeping the type checker happy.
if (loafDuration === undefined || loafBlocking === undefined) continue;

observedLoafs.push({
startTime: (loafEvent.ts - timeOrigin) / 1000,
duration: loafDuration,
blockingDuration: loafBlocking,
});

// Max LoAFs are only considered after FCP.
if (loafEvent.ts < firstContentfulPaint) continue;

Check warning on line 95 in core/audits/metrics/max-potential-fid.js

View check run for this annotation

Codecov / codecov/patch

core/audits/metrics/max-potential-fid.js#L95

Added line #L95 was not covered by tests
if (loafDuration > currentMaxDuration) {
currentMaxDuration = loafDuration;
currentMaxDurationLoaf = loaf;
currentMaxDurationLoaf = loafEvent;

Check warning on line 98 in core/audits/metrics/max-potential-fid.js

View check run for this annotation

Codecov / codecov/patch

core/audits/metrics/max-potential-fid.js#L98

Added line #L98 was not covered by tests
}
if (loafBlocking > currentMaxBlocking) {
currentMaxBlocking = loafBlocking;
currentMaxBlockingLoaf = loaf;
currentMaxBlockingLoaf = loafEvent;

Check warning on line 102 in core/audits/metrics/max-potential-fid.js

View check run for this annotation

Codecov / codecov/patch

core/audits/metrics/max-potential-fid.js#L102

Added line #L102 was not covered by tests
}
}

if (!currentMaxDurationLoaf || !currentMaxBlockingLoaf) return;

return {
type: 'debugdata',
Copy link
Member

Choose a reason for hiding this comment

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

We won't see the observedLoafs if they were all below FCP in this case. The function will still return undefined because there is no max duration/blocking events after FCP.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks! Good catch

observedMaxDurationLoaf: currentMaxDurationLoaf,
observedMaxBlockingLoaf: currentMaxBlockingLoaf,
observedLoafs,
};
}

Expand Down
158 changes: 158 additions & 0 deletions core/test/audits/metrics/max-potential-fid-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
/**
* @license
* Copyright 2023 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/

import MaxPotentialFid from '../../../audits/metrics/max-potential-fid.js';
import {createTestTrace} from '../../create-test-trace.js';
import {networkRecordsToDevtoolsLog} from '../../network-records-to-devtools-log.js';

/**
* @typedef LoafDef
* @property {number} ts LoAF trace event timestamp in milliseconds relative to timeOrigin.
* @property {number} duration Duration of LoAF in milliseconds.
* @property {number} blockingDuration Blocking duration of LoAF in milliseconds.
*/

/**
* @param {LH.TraceEvent} navStartEvt
* @param {LoafDef} loafDef
*/
function createLoafEvents(navStartEvt, {ts, duration, blockingDuration}) {
const {pid, tid} = navStartEvt;
ts *= 1000;
const endTs = ts + duration * 1000;

return [{
name: 'LongAnimationFrame',
ph: 'b',
cat: 'devtools.timeline',
pid,
tid,
ts,
args: {
data: {
duration,
blockingDuration,
numScripts: 1,
renderDuration: 14,
styleAndLayoutDuration: 13,
},
},
}, {
name: 'LongAnimationFrame',
ph: 'e',
cat: 'devtools.timeline',
pid,
tid,
ts: endTs,
args: {},
}];
}

describe('Max Potential FID', () => {
it('evaluates MPFID and includes LoAF debug data', async () => {
const frameUrl = 'https://example.com/';
const topLevelTasks = [
{ts: 2000, duration: 2999, blockingDuration: 1500}, // Right up to FCP.
{ts: 5500, duration: 1000, blockingDuration: 500}, // Longest `blockingDuration` after FCP.
{ts: 8000, duration: 2000, blockingDuration: 10}, // Longest `duration` after FCP.
];
const trace = createTestTrace({firstContentfulPaint: 5000, frameUrl, topLevelTasks});

// Add LoAF events (reusing long task timings).
const navStart = trace.traceEvents.find(evt => evt.name === 'navigationStart');
for (const task of topLevelTasks) {
trace.traceEvents.push(...createLoafEvents(navStart, task));
}

const artifacts = {
traces: {defaultPass: trace},
devtoolsLogs: {defaultPass: networkRecordsToDevtoolsLog([{url: frameUrl}])},
GatherContext: {gatherMode: 'navigation'},
};
const context = {
settings: {throttlingMethod: 'devtools'},
computedCache: new Map(),
options: MaxPotentialFid.defaultOptions,
};

const result = await MaxPotentialFid.audit(artifacts, context);
expect(result).toMatchObject({
score: 0,
numericValue: 2000,
numericUnit: 'millisecond',
displayValue: expect.toBeDisplayString('2,000 ms'),
details: {
type: 'debugdata',
observedMaxDurationLoaf: {
name: 'LongAnimationFrame',
args: {
data: {
duration: 2000,
blockingDuration: 10,
},
},
},
observedMaxBlockingLoaf: {
name: 'LongAnimationFrame',
args: {
data: {
duration: 1000,
blockingDuration: 500,
},
},
},
observedLoafs: [
{startTime: 2000, duration: 2999, blockingDuration: 1500},
{startTime: 5500, duration: 1000, blockingDuration: 500},
{startTime: 8000, duration: 2000, blockingDuration: 10},
],
},
});
});

it('includes LoAFs before FCP in observedLoafs', async () => {
const frameUrl = 'https://example.com/';
const topLevelTasks = [
{ts: 1000, duration: 1000, blockingDuration: 1000},
{ts: 2000, duration: 1000, blockingDuration: 1000},
{ts: 3000, duration: 1000, blockingDuration: 1000},
];
const trace = createTestTrace({firstContentfulPaint: 5000, frameUrl, topLevelTasks});

// Add LoAF events (reusing long task timings).
const navStart = trace.traceEvents.find(evt => evt.name === 'navigationStart');
for (const task of topLevelTasks) {
trace.traceEvents.push(...createLoafEvents(navStart, task));
}

const artifacts = {
traces: {defaultPass: trace},
devtoolsLogs: {defaultPass: networkRecordsToDevtoolsLog([{url: frameUrl}])},
GatherContext: {gatherMode: 'navigation'},
};
const context = {
settings: {throttlingMethod: 'devtools'},
computedCache: new Map(),
options: MaxPotentialFid.defaultOptions,
};

const result = await MaxPotentialFid.audit(artifacts, context);
expect(result).toMatchObject({
score: 1,
numericValue: 16,
details: {
type: 'debugdata',
observedMaxDurationLoaf: undefined,
observedMaxBlockingLoaf: undefined,
observedLoafs: [
{startTime: 1000, duration: 1000, blockingDuration: 1000},
{startTime: 2000, duration: 1000, blockingDuration: 1000},
{startTime: 3000, duration: 1000, blockingDuration: 1000},
],
},
});
});
});
9 changes: 8 additions & 1 deletion core/test/results/sample_v2.json
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,14 @@
"scope": "devtools.timeline",
"tid": 259,
"ts": 8703546175
}
},
"observedLoafs": [
{
"startTime": 6845.353,
"duration": 1199.518,
"blockingDuration": 1139.261
}
]
}
},
"cumulative-layout-shift": {
Expand Down
Loading