-
Notifications
You must be signed in to change notification settings - Fork 430
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
LoAF Summary attribution information #574
base: v5
Are you sure you want to change the base?
Changes from all commits
f195e55
0cb43d6
95237c6
194cc08
1b49c03
d8cb2a1
0b6b12f
aad54d8
0cdc93f
918e068
4b16fac
add385a
320eaa6
94abf79
dcd95df
9513369
e0c2f08
80d272a
807c85b
7f4c7d9
f8fa543
9c61110
19a2f42
934b803
3fa8ae0
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 | ||||
---|---|---|---|---|---|---|
|
@@ -927,6 +927,125 @@ interface INPAttribution { | |||||
* are detect, this array will be empty. | ||||||
*/ | ||||||
longAnimationFrameEntries: PerformanceLongAnimationFrameTiming[]; | ||||||
/** | ||||||
* If the browser supports the Long Animation Frame API, this object | ||||||
* summarises information relevant to INP across the long animation frames | ||||||
* intersecting the INP interaction. See the LongAnimationFrameSummary | ||||||
* definition for an explanation of what is included. | ||||||
*/ | ||||||
longAnimationFrameSummary?: LongAnimationFrameSummary; | ||||||
{ | ||||||
/** | ||||||
* The number of Long Animation Frame scripts that intersect the INP | ||||||
* interaction. | ||||||
* NOTE: This may be be less than the total count of scripts in the Long | ||||||
* Animation Frames as some scripts may occur before the interaction. | ||||||
*/ | ||||||
numIntersectingScripts: number; | ||||||
/** | ||||||
* The number of Long Animation Frames intersecting the INP interaction. | ||||||
*/ | ||||||
numLongAnimationFrames: number; | ||||||
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. This seems unnecessary because someone could easily do |
||||||
/** | ||||||
* The slowest Long Animation Frame script that intersects the INP | ||||||
* interaction. | ||||||
*/ | ||||||
slowestScript: SlowestScriptSummary; | ||||||
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. Right now the What if instead we just had three top-level attribution properties?
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. Also, I wonder if |
||||||
{ | ||||||
/** | ||||||
* The slowest Long Animation Frame script that intersects the INP | ||||||
* interaction. | ||||||
*/ | ||||||
entry: PerformanceScriptTiming; | ||||||
/** | ||||||
* The INP sub-part where the longest script ran. | ||||||
*/ | ||||||
subpart: INPSubpart; //'inputDelay' | 'processingDuration' | 'presentationDelay'; | ||||||
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. On web.dev these are currently referred to as "phases". Are we planning to change that? 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. @brendankenny I think it was you that told me were were moving towards subparts to mirror LCP, is that right? 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. Yes, we should definitely unify. I suggested it specifically because we didn't want to simultaneously cement "phases" here in web-vitals and "subparts" over in CrUX, and everyone in the LCP discussions agreed on "subpart". 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.
Suggested change
For consistency with other enum-type options. 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. This is interesting. Yes as an enum they should use that format. However this is then exposed in output—as both property names and values: "slowestScript": {
...
"subpart": "processingDuration",
"intersectingDuration": 100,
...
},
"totalDurationsPerSubpart": {
"processingDuration": {
"event-listener": 100
}
}, And so they should really follow camelCase? Though the invoker type is hyphenated (and that's part of the spec) so it's a little inconsistent anyway in that regards The other consideration is that camelCase is also consistent with what we use for LCP sub parts, but there they are strict property names (not values) and we don't have them defined as an enum. |
||||||
/** | ||||||
* The amount of time the slowest script intersected the INP duration. | ||||||
*/ | ||||||
intersectingDuration: number; | ||||||
/** | ||||||
* The total duration time of the slowest script (compile and execution, | ||||||
* including forced style and layout). Note this may be longer than the | ||||||
* intersectingScriptDuration if the INP interaction happened mid-script. | ||||||
*/ | ||||||
totalDuration: number; | ||||||
/** | ||||||
* The compile duration of the slowest script. Note this may be longer | ||||||
* than the intersectingScriptDuration if the INP interaction happened | ||||||
* mid-script. | ||||||
*/ | ||||||
compileDuration: number; | ||||||
/** | ||||||
* The execution duration of the slowest script. Note this may be longer | ||||||
* than the intersectingScriptDuration if the INP interaction happened | ||||||
* mid-script. | ||||||
*/ | ||||||
executionDuration: number; | ||||||
/** | ||||||
/** | ||||||
* The forced style and layoult duration of the slowest script. Note this | ||||||
* may be longer than the intersectingScriptDuration if the INP interaction | ||||||
* happened mid-script. | ||||||
*/ | ||||||
forcedStyleAndLayoutDuration: number; | ||||||
/** | ||||||
* The pause duration of the slowest script. Note this may be longer | ||||||
* than the intersectingScriptDuration if the INP interaction happened | ||||||
* mid-script. | ||||||
*/ | ||||||
pauseDuration: number; | ||||||
/** | ||||||
* The invokerType of the slowest script. | ||||||
*/ | ||||||
invokerType: ScriptInvokerType; | ||||||
/** | ||||||
* The invoker of the slowest script. | ||||||
*/ | ||||||
invoker?: string; | ||||||
/** | ||||||
* The sourceURL of the slowest script. | ||||||
*/ | ||||||
sourceURL?: string; | ||||||
/** | ||||||
* The sourceFunctionName of the slowest script. | ||||||
*/ | ||||||
sourceFunctionName?: string; | ||||||
/** | ||||||
* The sourceCharPosition of the slowest script. | ||||||
*/ | ||||||
sourceCharPosition?: number; | ||||||
} | ||||||
/** | ||||||
* The total intersecting durations in each sub-part by invoker for | ||||||
* scripts that intersect the INP interaction. | ||||||
* For example: | ||||||
* { | ||||||
* 'inputDelay': { 'event-listener': 185, 'user-callback': 28}, | ||||||
* 'processingDuration': { 'event-listener': 144}, | ||||||
* } | ||||||
*/ | ||||||
totalDurationsPerSubpart: Partial< | ||||||
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. Nit: I'd rather we name this something like |
||||||
Record<INPSubpart, Partial<Record<ScriptInvokerType, number>>> | ||||||
>; | ||||||
/** | ||||||
* The total forced style and layout durations as provided by Long Animation | ||||||
* Frame scripts intersecting the INP interaction. | ||||||
*/ | ||||||
totalForcedStyleAndLayoutDuration: number; | ||||||
/** | ||||||
* The total non-force (i.e. end-of-frame) style and layout duration from any | ||||||
* Long Animation Frames intersecting INP interaction. | ||||||
*/ | ||||||
totalNonForcedStyleAndLayoutDuration: number; | ||||||
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. I'd rather have 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. Personally I'd definitely rather have the "nonForced" time (i.e. the end of frame style and layout time) separately and maybe total time and then the forced time can we (somewhat) calculated from that. But agree The problem is the "total forced" time is not really know since it's only made up on scripts > 5ms (see w3c/long-animation-frames#23) so I don't think it's accurate to say the "total forced time". |
||||||
/** | ||||||
* The total duration of Long Animation Frame scripts that intersect the INP | ||||||
* duration. Note, this includes forced style and layout within those | ||||||
* scripts and is limited to scripts > 5 milliseconds. | ||||||
*/ | ||||||
totalIntersectingScriptsDuration: number; | ||||||
} | ||||||
/** | ||||||
* The time from when the user interacted with the page until when the | ||||||
* browser was first able to start processing event listeners for that | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,7 +25,9 @@ import { | |
INPAttribution, | ||
INPMetric, | ||
INPMetricWithAttribution, | ||
INPSubpart, | ||
INPAttributionReportOpts, | ||
LongAnimationFrameSummary, | ||
} from '../types.js'; | ||
|
||
interface pendingEntriesGroup { | ||
|
@@ -260,6 +262,99 @@ export const onINP = ( | |
return intersectingLoAFs; | ||
}; | ||
|
||
const getLoAFSummary = (attribution: INPAttribution) => { | ||
// Stats across all LoAF entries and scripts. | ||
const interactionTime = attribution.interactionTime; | ||
const inputDelay = attribution.inputDelay; | ||
const processingDuration = attribution.processingDuration; | ||
let totalNonForcedStyleAndLayoutDuration = 0; | ||
let totalForcedStyleAndLayout = 0; | ||
let totalIntersectingScriptsDuration = 0; | ||
let numScripts = 0; | ||
let slowestScriptDuration = 0; | ||
let slowestScriptEntry!: PerformanceScriptTiming; | ||
let slowestScriptSubpart!: INPSubpart; | ||
const subparts: Partial< | ||
Record<INPSubpart, Partial<Record<ScriptInvokerType, number>>> | ||
> = {}; | ||
|
||
for (const loafEntry of attribution.longAnimationFrameEntries) { | ||
totalNonForcedStyleAndLayoutDuration += | ||
loafEntry.startTime + | ||
loafEntry.duration - | ||
loafEntry.styleAndLayoutStart; | ||
|
||
for (const script of loafEntry.scripts) { | ||
const scriptEndTime = script.startTime + script.duration; | ||
if (scriptEndTime < interactionTime) { | ||
return; | ||
} | ||
const intersectingScriptDuration = | ||
scriptEndTime - Math.max(interactionTime, script.startTime); | ||
totalIntersectingScriptsDuration += intersectingScriptDuration; | ||
numScripts++; | ||
totalForcedStyleAndLayout += script.forcedStyleAndLayoutDuration; | ||
const invokerType = script.invokerType; | ||
let subpart: INPSubpart = 'processingDuration'; | ||
if (script.startTime < interactionTime + inputDelay) { | ||
subpart = 'inputDelay'; | ||
} else if ( | ||
script.startTime >= | ||
interactionTime + inputDelay + processingDuration | ||
) { | ||
subpart = 'presentationDelay'; | ||
} | ||
|
||
// Define the record if necessary. Annoyingly TypeScript doesn't yet | ||
// recognize this so need a few `!`s on the next two lines to convinced | ||
// it is typed. | ||
subparts[subpart] ??= {}; | ||
subparts[subpart]![invokerType] ??= 0; | ||
// Increment it with this value | ||
subparts[subpart]![invokerType]! += intersectingScriptDuration; | ||
|
||
if (intersectingScriptDuration > slowestScriptDuration) { | ||
slowestScriptEntry = script; | ||
slowestScriptSubpart = subpart; | ||
slowestScriptDuration = intersectingScriptDuration; | ||
} | ||
} | ||
} | ||
|
||
// Gather the loaf summary information into the loafAttribution object | ||
const loafSummary: LongAnimationFrameSummary = { | ||
numLongAnimationFrames: attribution.longAnimationFrameEntries.length, | ||
numIntersectingScripts: numScripts, | ||
slowestScript: { | ||
entry: slowestScriptEntry, | ||
subpart: slowestScriptSubpart, | ||
intersectingDuration: slowestScriptDuration, | ||
totalDuration: slowestScriptEntry?.duration, | ||
compileDuration: | ||
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. Using the I know there's been some discussion of removing these properties anyway, but if we don't then it's probably better to just omit these (or set them to 0) if there's no slowest script. |
||
slowestScriptEntry?.executionStart - slowestScriptEntry?.startTime, | ||
executionDuration: | ||
slowestScriptEntry?.startTime + | ||
slowestScriptEntry?.duration - | ||
slowestScriptEntry?.executionStart, | ||
forcedStyleAndLayoutDuration: | ||
slowestScriptEntry?.forcedStyleAndLayoutDuration, | ||
pauseDuration: slowestScriptEntry?.pauseDuration, | ||
invokerType: slowestScriptEntry?.invokerType, | ||
invoker: slowestScriptEntry?.invoker, | ||
sourceURL: slowestScriptEntry?.sourceURL, | ||
sourceFunctionName: slowestScriptEntry?.sourceFunctionName, | ||
sourceCharPosition: slowestScriptEntry?.sourceCharPosition, | ||
}, | ||
totalDurationsPerSubpart: subparts, | ||
totalForcedStyleAndLayoutDuration: totalForcedStyleAndLayout, | ||
totalNonForcedStyleAndLayoutDuration: | ||
totalNonForcedStyleAndLayoutDuration, | ||
totalIntersectingScriptsDuration: totalIntersectingScriptsDuration, | ||
}; | ||
|
||
return loafSummary; | ||
}; | ||
|
||
const attributeINP = (metric: INPMetric): INPMetricWithAttribution => { | ||
const firstEntry = metric.entries[0]; | ||
const group = entryToEntriesGroupMap.get(firstEntry)!; | ||
|
@@ -311,6 +406,8 @@ export const onINP = ( | |
loadState: getLoadState(firstEntry.startTime), | ||
}; | ||
|
||
attribution.longAnimationFrameSummary = getLoAFSummary(attribution); | ||
|
||
// Use `Object.assign()` to ensure the original metric object is returned. | ||
const metricWithAttribution: INPMetricWithAttribution = Object.assign( | ||
metric, | ||
|
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.
Rather that just listing the number of intersecting scripts, I wonder if it would be more useful to create an array of intersecting scripts (which then someone could easily call
.length
on).Related, how common is it for scripts from multiple different LoAF entries to all be intersecting with INP?