-
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
core(mpfid): add list of loaf durations to debugdata #15685
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 ' + | ||
|
@@ -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; | ||
|
||
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; | ||
|
||
if (loafDuration > currentMaxDuration) { | ||
currentMaxDuration = loafDuration; | ||
currentMaxDurationLoaf = loaf; | ||
currentMaxDurationLoaf = loafEvent; | ||
} | ||
if (loafBlocking > currentMaxBlocking) { | ||
currentMaxBlocking = loafBlocking; | ||
currentMaxBlockingLoaf = loaf; | ||
currentMaxBlockingLoaf = loafEvent; | ||
} | ||
} | ||
|
||
if (!currentMaxDurationLoaf || !currentMaxBlockingLoaf) return; | ||
|
||
return { | ||
type: 'debugdata', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We won't see the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks! Good catch |
||
observedMaxDurationLoaf: currentMaxDurationLoaf, | ||
observedMaxBlockingLoaf: currentMaxBlockingLoaf, | ||
observedLoafs, | ||
}; | ||
} | ||
|
||
|
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}, | ||
], | ||
}, | ||
}); | ||
}); | ||
}); |
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.
This case won't really hurt anything without this check, so is mostly for aesthetics (prevent empty
{type: 'debugdata', observedLoafs: []}
hanging around in LHRs)