[go: nahoru, domu]
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

Jump to bottom

core(trace-elements): use CLS metric event filtering #15067

Merged
merged 1 commit into from
May 11, 2023
Merged

core(trace-elements): use CLS metric event filtering #15067

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 2 additions & 1 deletion core/computed/metrics/cumulative-layout-shift.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import {makeComputedArtifact} from '../computed-artifact.js';
import {ProcessedTrace} from '../processed-trace.js';

/** @typedef {{ts: number, isMainFrame: boolean, weightedScore: number}} LayoutShiftEvent */
/** @typedef {{ts: number, isMainFrame: boolean, weightedScore: number, impactedNodes?: LH.Artifacts.TraceImpactedNode[]}} LayoutShiftEvent */

const RECENT_INPUT_WINDOW = 500;

Expand Down Expand Up @@ -65,6 +65,7 @@ class CumulativeLayoutShift {
ts: event.ts,
isMainFrame: event.args.data.is_main_frame,
weightedScore: event.args.data.weighted_score_delta,
impactedNodes: event.args.data.impacted_nodes,
});
}

Expand Down
29 changes: 10 additions & 19 deletions core/gather/gatherers/trace-elements.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import {ProcessedNavigation} from '../../computed/processed-navigation.js';
import {LighthouseError} from '../../lib/lh-error.js';
import {Responsiveness} from '../../computed/metrics/responsiveness.js';
import {CumulativeLayoutShift} from '../../computed/metrics/cumulative-layout-shift.js';

/** @typedef {{nodeId: number, score?: number, animations?: {name?: string, failureReasonsMask?: number, unsupportedProperties?: string[]}[], type?: string}} TraceElementData */

Expand Down Expand Up @@ -80,34 +81,24 @@
* We calculate the score per element by taking the 'score' of each layout shift event and
* distributing it between all the nodes that were shifted, proportianal to the impact region of
* each shifted element.
* @param {Array<LH.TraceEvent>} mainThreadEvents
* @param {LH.Artifacts.ProcessedTrace} processedTrace
* @return {Array<TraceElementData>}
*/
static getTopLayoutShiftElements(mainThreadEvents) {
static getTopLayoutShiftElements(processedTrace) {
/** @type {Map<number, number>} */
const clsPerNode = new Map();
const shiftEvents = mainThreadEvents
.filter(e => e.name === 'LayoutShift')
.map(e => e.args?.data);
const indexFirstEventWithoutInput =
shiftEvents.findIndex(event => event && !event.had_recent_input);

shiftEvents.forEach((event, index) => {
if (!event || !event.impacted_nodes || !event.score) {
return;
}
const shiftEvents = CumulativeLayoutShift.getLayoutShiftEvents(processedTrace);

// Ignore events with input, unless it's one of the initial events.
// See comment in computed/metrics/cumulative-layout-shift.js.
if (indexFirstEventWithoutInput !== -1 && index >= indexFirstEventWithoutInput) {
if (event.had_recent_input) return;
shiftEvents.forEach((event) => {
if (!event || !event.impactedNodes) {
return;

Check warning on line 94 in core/gather/gatherers/trace-elements.js

View check run for this annotation

Codecov / codecov/patch

core/gather/gatherers/trace-elements.js#L94 

Added line #L94 was not covered by tests
}

let totalAreaOfImpact = 0;
/** @type {Map<number, number>} */
const pixelsMovedPerNode = new Map();

event.impacted_nodes.forEach(node => {
event.impactedNodes.forEach(node => {
if (!node.node_id || !node.old_rect || !node.new_rect) {
return;
}
Expand All @@ -124,7 +115,7 @@

for (const [nodeId, pixelsMoved] of pixelsMovedPerNode.entries()) {
let clsContribution = clsPerNode.get(nodeId) || 0;
clsContribution += (pixelsMoved / totalAreaOfImpact) * event.score;
clsContribution += (pixelsMoved / totalAreaOfImpact) * event.weightedScore;
clsPerNode.set(nodeId, clsContribution);
}
});
Expand Down Expand Up @@ -272,7 +263,7 @@
const {mainThreadEvents} = processedTrace;

const lcpNodeData = await TraceElements.getLcpElement(trace, context);
const clsNodeData = TraceElements.getTopLayoutShiftElements(mainThreadEvents);
const clsNodeData = TraceElements.getTopLayoutShiftElements(processedTrace);
const animatedElementData = await this.getAnimatedElements(mainThreadEvents);
const responsivenessElementData = await TraceElements.getResponsivenessElement(trace, context);

Expand Down
77 changes: 46 additions & 31 deletions core/test/gather/gatherers/trace-elements-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {Connection} from '../../../legacy/gather/connections/connection.js';
import {createTestTrace, rootFrame} from '../../create-test-trace.js';
import {createMockSendCommandFn, createMockOnFn} from '../mock-commands.js';
import {flushAllTimersAndMicrotasks, fnAny, readJson, timers} from '../../test-utils.js';
import {ProcessedTrace} from '../../../computed/processed-trace.js';

const animationTrace = readJson('../../fixtures/artifacts/animation/trace.json', import.meta);

Expand All @@ -23,11 +24,13 @@ function makeLayoutShiftTraceEvent(score, impactedNodes, had_recent_input = fals
ts: 1200,
args: {
data: {
is_main_frame: true,
had_recent_input, // eslint-disable-line camelcase
impacted_nodes: impactedNodes,
score: score,
weighted_score_delta: score,
},
frame: '3C4CBF06AF1ED5B9EAA59BECA70111F4',
frame: 'ROOT_FRAME',
},
};
}
Expand Down Expand Up @@ -87,8 +90,9 @@ describe('Trace Elements gatherer - GetTopLayoutShiftElements', () => {
expect(diff).toBeLessThanOrEqual(Number.EPSILON);
}

it('returns layout shift data sorted by impact area', () => {
const traceEvents = [
it('returns layout shift data sorted by impact area', async () => {
const trace = createTestTrace({});
trace.traceEvents.push(
makeLayoutShiftTraceEvent(1, [
{
new_rect: [0, 0, 200, 200],
Expand All @@ -100,10 +104,11 @@ describe('Trace Elements gatherer - GetTopLayoutShiftElements', () => {
node_id: 25,
old_rect: [0, 100, 200, 100],
},
]),
];
])
);
const processedTrace = await ProcessedTrace.request(trace, {computedCache: new Map()});

const result = TraceElementsGatherer.getTopLayoutShiftElements(traceEvents);
const result = TraceElementsGatherer.getTopLayoutShiftElements(processedTrace);
expect(result).toEqual([
{nodeId: 25, score: 0.6},
{nodeId: 60, score: 0.4},
Expand All @@ -112,8 +117,9 @@ describe('Trace Elements gatherer - GetTopLayoutShiftElements', () => {
expectEqualFloat(total, 1.0);
});

it('does not ignore initial trace events with input', () => {
const traceEvents = [
it('does not ignore initial trace events with input', async () => {
const trace = createTestTrace({});
trace.traceEvents.push(
makeLayoutShiftTraceEvent(1, [
{
new_rect: [0, 0, 200, 200],
Expand All @@ -127,18 +133,20 @@ describe('Trace Elements gatherer - GetTopLayoutShiftElements', () => {
node_id: 2,
old_rect: [0, 0, 200, 100],
},
], true),
];
], true)
);
const processedTrace = await ProcessedTrace.request(trace, {computedCache: new Map()});

const result = TraceElementsGatherer.getTopLayoutShiftElements(traceEvents);
const result = TraceElementsGatherer.getTopLayoutShiftElements(processedTrace);
expect(result).toEqual([
{nodeId: 1, score: 1},
{nodeId: 2, score: 1},
]);
});

it('does ignore later trace events with input', () => {
const traceEvents = [
it('does ignore later trace events with input', async () => {
const trace = createTestTrace({});
trace.traceEvents.push(
makeLayoutShiftTraceEvent(1, [
{
new_rect: [0, 0, 200, 200],
Expand All @@ -152,17 +160,19 @@ describe('Trace Elements gatherer - GetTopLayoutShiftElements', () => {
node_id: 2,
old_rect: [0, 0, 200, 100],
},
], true),
];
], true)
);
const processedTrace = await ProcessedTrace.request(trace, {computedCache: new Map()});

const result = TraceElementsGatherer.getTopLayoutShiftElements(traceEvents);
const result = TraceElementsGatherer.getTopLayoutShiftElements(processedTrace);
expect(result).toEqual([
{nodeId: 1, score: 1},
]);
});

it('correctly ignores trace events with input (complex)', () => {
const traceEvents = [
it('correctly ignores trace events with input (complex)', async () => {
const trace = createTestTrace({});
trace.traceEvents.push(
makeLayoutShiftTraceEvent(1, [
{
new_rect: [0, 0, 200, 200],
Expand Down Expand Up @@ -211,10 +221,11 @@ describe('Trace Elements gatherer - GetTopLayoutShiftElements', () => {
node_id: 7,
old_rect: [0, 0, 200, 100],
},
]),
];
])
);
const processedTrace = await ProcessedTrace.request(trace, {computedCache: new Map()});

const result = TraceElementsGatherer.getTopLayoutShiftElements(traceEvents);
const result = TraceElementsGatherer.getTopLayoutShiftElements(processedTrace);
expect(result).toEqual([
{nodeId: 1, score: 1},
{nodeId: 2, score: 1},
Expand All @@ -224,8 +235,9 @@ describe('Trace Elements gatherer - GetTopLayoutShiftElements', () => {
]);
});

it('combines scores for the same nodeId accross multiple shift events', () => {
const traceEvents = [
it('combines scores for the same nodeId accross multiple shift events', async () => {
const trace = createTestTrace({});
trace.traceEvents.push(
makeLayoutShiftTraceEvent(1, [
{
new_rect: [0, 0, 200, 200],
Expand All @@ -244,10 +256,11 @@ describe('Trace Elements gatherer - GetTopLayoutShiftElements', () => {
node_id: 60,
old_rect: [0, 0, 200, 200],
},
]),
];
])
);
const processedTrace = await ProcessedTrace.request(trace, {computedCache: new Map()});

const result = TraceElementsGatherer.getTopLayoutShiftElements(traceEvents);
const result = TraceElementsGatherer.getTopLayoutShiftElements(processedTrace);
expect(result).toEqual([
{nodeId: 60, score: 0.7},
{nodeId: 25, score: 0.6},
Expand All @@ -256,8 +269,9 @@ describe('Trace Elements gatherer - GetTopLayoutShiftElements', () => {
expectEqualFloat(total, 1.3);
});

it('returns only the top five values', () => {
const traceEvents = [
it('returns only the top five values', async () => {
const trace = createTestTrace({});
trace.traceEvents.push(
makeLayoutShiftTraceEvent(1, [
{
new_rect: [0, 100, 100, 100],
Expand Down Expand Up @@ -298,10 +312,11 @@ describe('Trace Elements gatherer - GetTopLayoutShiftElements', () => {
node_id: 7,
old_rect: [0, 0, 100, 100],
},
]),
];
])
);
const processedTrace = await ProcessedTrace.request(trace, {computedCache: new Map()});

const result = TraceElementsGatherer.getTopLayoutShiftElements(traceEvents);
const result = TraceElementsGatherer.getTopLayoutShiftElements(processedTrace);
expect(result).toEqual([
{nodeId: 3, score: 1.0},
{nodeId: 1, score: 0.5},
Expand Down
12 changes: 7 additions & 5 deletions types/artifacts.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -948,6 +948,12 @@ declare module Artifacts {
// Convenience methods.
isFirstParty: (url: string) => boolean;
}

interface TraceImpactedNode {
node_id: number;
old_rect?: Array<number>;
new_rect?: Array<number>;
}
}

export interface Trace {
Expand Down Expand Up @@ -1017,11 +1023,7 @@ export interface TraceEvent {
nodeId?: number;
DOMNodeId?: number;
imageUrl?: string;
impacted_nodes?: Array<{
node_id: number,
old_rect?: Array<number>,
new_rect?: Array<number>,
}>;
impacted_nodes?: Artifacts.TraceImpactedNode[];
score?: number;
weighted_score_delta?: number;
had_recent_input?: boolean;
Expand Down
Loading