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

Fix missing LoAF attribution when entries are dispatched before event entries #487

Merged
merged 3 commits into from
Jun 6, 2024
Merged
Changes from 2 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
35 changes: 30 additions & 5 deletions src/attribution/onINP.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,12 @@ interface pendingEntriesGroup {
entries: PerformanceEventTiming[];
}

/**
* The maximum number of LoAFs to retain on cleanup that don't have any
* overlapping events.
philipwalton marked this conversation as resolved.
Show resolved Hide resolved
*/
const MAX_PENDING_ENTRIES = 50;

// A PerformanceObserver, observing new `long-animation-frame` entries.
// If this variable is defined it means the browser supports LoAF.
let loafObserver: PerformanceObserver | undefined;
Expand All @@ -59,6 +65,9 @@ const pendingEntriesGroupMap: Map<number, pendingEntriesGroup> = new Map();
// are removed.
let previousRenderTimes: number[] = [];

// The `processingEnd` time of most recently-processed event, chronologically.
let latestProcessingEnd: number;

// A WeakMap so you can look up the `renderTime` of a given entry and the
// value returned will be the same value used by `pendingEntriesGroupMap`.
const entryToRenderTimeMap: WeakMap<
Expand All @@ -78,7 +87,8 @@ let idleHandle: number = -1;
* Adds new LoAF entries to the `pendingLoAFs` list.
*/
const handleLoAFEntries = (entries: PerformanceLongAnimationFrameTiming[]) => {
entries.forEach((entry) => pendingLoAFs.push(entry));
pendingLoAFs = pendingLoAFs.concat(entries);
queueCleanup();
};

// Get a reference to the interaction target element in case it's removed
Expand All @@ -105,6 +115,8 @@ const groupEntriesByRenderTime = (entry: PerformanceEventTiming) => {
let renderTime = entry.startTime + entry.duration;
let previousRenderTime;

latestProcessingEnd = Math.max(latestProcessingEnd, entry.processingEnd);

// Iterate of all previous render times in reverse order to find a match.
// Go in reverse since the most likely match will be at the end.
for (let i = previousRenderTimes.length - 1; i >= 0; i--) {
Expand Down Expand Up @@ -143,6 +155,8 @@ const groupEntriesByRenderTime = (entry: PerformanceEventTiming) => {
if (entry.interactionId || entry.entryType === 'first-input') {
entryToRenderTimeMap.set(entry, renderTime);
}

queueCleanup();
};

const queueCleanup = () => {
Expand All @@ -167,7 +181,7 @@ const cleanupEntries = () => {
// events are dispatched out of order. When this happens they're generally
// only off by a frame or two, so keeping the most recent 50 should be
// more than sufficient.
philipwalton marked this conversation as resolved.
Show resolved Hide resolved
previousRenderTimes = previousRenderTimes.slice(-50);
previousRenderTimes = previousRenderTimes.slice(-1 * MAX_PENDING_ENTRIES);

// Keep all render times that are part of a pending INP candidate or
// that occurred within the 50 most recently-dispatched animation frames.
Expand All @@ -181,8 +195,9 @@ const cleanupEntries = () => {
if (!renderTimesToKeep.has(key)) pendingEntriesGroupMap.delete(key);
});

// Remove all pending LoAF entries that don't intersect with entries in
// the newly cleaned up `pendingEntriesGroupMap`.
// Keep all pending LoAF entries that either:
// 1) intersect with entries in the newly cleaned up `pendingEntriesGroupMap`
// 2) occur after the most recently-processed event entry.
const loafsToKeep: Set<PerformanceLongAnimationFrameTiming> = new Set();
pendingEntriesGroupMap.forEach((group) => {
getIntersectingLoAFs(group.startTime, group.processingEnd).forEach(
Expand All @@ -191,6 +206,17 @@ const cleanupEntries = () => {
},
);
});
for (let i = 0; i < MAX_PENDING_ENTRIES; i++) {
// Look at pending LoAF in reverse order so the most recent are first.
const loaf = pendingLoAFs[pendingLoAFs.length - 1];

// If we reach LoAFs that overlap with event processing,
// we can assume all previous ones have already been handled.
if (!loaf || loaf.startTime < latestProcessingEnd) break;

loafsToKeep.add(loaf);
}

pendingLoAFs = Array.from(loafsToKeep);

// Reset the idle callback handle so it can be queued again.
Expand All @@ -200,7 +226,6 @@ const cleanupEntries = () => {
entryPreProcessingCallbacks.push(
saveInteractionTarget,
groupEntriesByRenderTime,
queueCleanup,
);

const getIntersectingLoAFs = (
Expand Down
Loading