Skip to content

Commit

Permalink
core(cumulative-layout-shift): experiment with new shared trace engine (
Browse files Browse the repository at this point in the history
  • Loading branch information
connorjclark committed Dec 19, 2023
1 parent f05700e commit f7ea338
Show file tree
Hide file tree
Showing 9 changed files with 272 additions and 13 deletions.
18 changes: 14 additions & 4 deletions cli/test/smokehouse/__snapshots__/report-assert-test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
exports[`getAssertionReport works (multiple failing) 1`] = `
"X difference at cumulative-layout-shift audit.details.items.length
expected: []
found: [{\\"cumulativeLayoutShiftMainFrame\\":0.13570762803819444}]
found: [{\\"cumulativeLayoutShiftMainFrame\\":0.13570762803819444,\\"newEngineResult\\":{\\"cumulativeLayoutShift\\":0.13570762803819444,\\"cumulativeLayoutShiftMainFrame\\":0.13570762803819444},\\"newEngineResultDiffered\\":false}]
X difference at cumulative-layout-shift audit.details.blah
Expand All @@ -28,7 +28,12 @@ exports[`getAssertionReport works (multiple failing) 1`] = `
\\"type\\": \\"debugdata\\",
\\"items\\": [
{
\\"cumulativeLayoutShiftMainFrame\\": 0.13570762803819444
\\"cumulativeLayoutShiftMainFrame\\": 0.13570762803819444,
\\"newEngineResult\\": {
\\"cumulativeLayoutShift\\": 0.13570762803819444,
\\"cumulativeLayoutShiftMainFrame\\": 0.13570762803819444
},
\\"newEngineResultDiffered\\": false
}
]
}
Expand All @@ -38,7 +43,7 @@ exports[`getAssertionReport works (multiple failing) 1`] = `
exports[`getAssertionReport works (trivial failing) 1`] = `
"X difference at cumulative-layout-shift audit.details.items.length
expected: []
found: [{\\"cumulativeLayoutShiftMainFrame\\":0.13570762803819444}]
found: [{\\"cumulativeLayoutShiftMainFrame\\":0.13570762803819444,\\"newEngineResult\\":{\\"cumulativeLayoutShift\\":0.13570762803819444,\\"cumulativeLayoutShiftMainFrame\\":0.13570762803819444},\\"newEngineResultDiffered\\":false}]
found result:
{
Expand All @@ -58,7 +63,12 @@ exports[`getAssertionReport works (trivial failing) 1`] = `
\\"type\\": \\"debugdata\\",
\\"items\\": [
{
\\"cumulativeLayoutShiftMainFrame\\": 0.13570762803819444
\\"cumulativeLayoutShiftMainFrame\\": 0.13570762803819444,
\\"newEngineResult\\": {
\\"cumulativeLayoutShift\\": 0.13570762803819444,
\\"cumulativeLayoutShiftMainFrame\\": 0.13570762803819444
},
\\"newEngineResultDiffered\\": false
}
]
}
Expand Down
78 changes: 74 additions & 4 deletions core/computed/metrics/cumulative-layout-shift.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@
import {makeComputedArtifact} from '../computed-artifact.js';
import {ProcessedTrace} from '../processed-trace.js';
import * as RectHelpers from '../../lib/rect-helpers.js';
import * as TraceEngine from '../../lib/trace-engine.js';
import {Sentry} from '../../lib/sentry.js';

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

const RECENT_INPUT_WINDOW = 500;

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

Expand Down Expand Up @@ -145,10 +148,29 @@ class CumulativeLayoutShift {
return maxScore;
}

/**
* @param {LayoutShiftEvent[]} allFrameShiftEvents
* @param {LayoutShiftEvent[]} mainFrameShiftEvents
*/
static async computeWithSharedTraceEngine(allFrameShiftEvents, mainFrameShiftEvents) {
/** @param {LH.TraceEvent[]} events */
const run = async (events) => {
const processor = new TraceEngine.TraceProcessor({
LayoutShifts: TraceEngine.TraceHandlers.LayoutShifts,
Screenshots: TraceEngine.TraceHandlers.Screenshots,
});
await processor.parse(events);
return processor.data.LayoutShifts.sessionMaxScore;
};
const cumulativeLayoutShift = await run(allFrameShiftEvents.map(e => e.event));
const cumulativeLayoutShiftMainFrame = await run(mainFrameShiftEvents.map(e => e.event));
return {cumulativeLayoutShift, cumulativeLayoutShiftMainFrame};
}

/**
* @param {LH.Trace} trace
* @param {LH.Artifacts.ComputedContext} context
* @return {Promise<{cumulativeLayoutShift: number, cumulativeLayoutShiftMainFrame: number, impactByNodeId: Map<number, number>}>}
* @return {Promise<{cumulativeLayoutShift: number, cumulativeLayoutShiftMainFrame: number, impactByNodeId: Map<number, number>, newEngineResult?: {cumulativeLayoutShift: number, cumulativeLayoutShiftMainFrame: number}, newEngineResultDiffered: boolean}>}
*/
static async compute_(trace, context) {
const processedTrace = await ProcessedTrace.request(trace, context);
Expand All @@ -157,11 +179,59 @@ class CumulativeLayoutShift {
CumulativeLayoutShift.getLayoutShiftEvents(processedTrace);
const impactByNodeId = CumulativeLayoutShift.getImpactByNodeId(allFrameShiftEvents);
const mainFrameShiftEvents = allFrameShiftEvents.filter(e => e.isMainFrame);
const cumulativeLayoutShift = CumulativeLayoutShift.calculate(allFrameShiftEvents);
const cumulativeLayoutShiftMainFrame = CumulativeLayoutShift.calculate(mainFrameShiftEvents);

// Run with the new trace engine, and only throw an error if we are running our unit tests.
// Otherwise, simply report any differences or errors to Sentry.
// TODO: TraceEngine always drops `had_recent_input` events, but Lighthouse is more lenient.
// See comment in `getLayoutShiftEvents`. We need to upstream this.
let newEngineResult;
let newEngineResultDiffered = false;
let tryNewTraceEngine = true;
if (allFrameShiftEvents.some(e => e.event.args.data?.had_recent_input)) {
tryNewTraceEngine = false;
}
if (tryNewTraceEngine) {
try {
newEngineResult =
await this.computeWithSharedTraceEngine(allFrameShiftEvents, mainFrameShiftEvents);
newEngineResultDiffered =
newEngineResult.cumulativeLayoutShift !== cumulativeLayoutShift ||
newEngineResult.cumulativeLayoutShiftMainFrame !== cumulativeLayoutShiftMainFrame;
if (newEngineResultDiffered) {
newEngineResultDiffered = true;
const expected = JSON.stringify({cumulativeLayoutShift, cumulativeLayoutShiftMainFrame});
const got = JSON.stringify(newEngineResult);
throw new Error(`new trace engine differed. expected: ${expected}, got: ${got}`);
}
} catch (err) {
console.error(err);
newEngineResultDiffered = true;

const error = new Error('Error when using new trace engine', {cause: err});
// @ts-expect-error Check for running from tests.
if (global.expect) {
throw error;
} else {
Sentry.captureException(error, {
tags: {computed: 'new-trace-engine'},
level: 'error',
extra: {
// Not sure if Sentry handles `cause`, so just in case add the info in a second place.
errorMsg: err.toString(),
},
});
}
}
}

return {
cumulativeLayoutShift: CumulativeLayoutShift.calculate(allFrameShiftEvents),
cumulativeLayoutShiftMainFrame: CumulativeLayoutShift.calculate(mainFrameShiftEvents),
cumulativeLayoutShift,
cumulativeLayoutShiftMainFrame,
impactByNodeId,
newEngineResult,
newEngineResultDiffered,
};
}
}
Expand Down
111 changes: 111 additions & 0 deletions core/lib/polyfill-dom-rect.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
// @ts-nocheck
/* eslint-disable */

function polyfillDOMRect() {
// devtools assumes clientside :(

// Everything else in here is the DOMRect polyfill
// https://raw.githubusercontent.com/JakeChampion/polyfill-library/master/polyfills/DOMRect/polyfill.js

(function (global) {
function number(v) {
return v === undefined ? 0 : Number(v);
}

function different(u, v) {
return u !== v && !(isNaN(u) && isNaN(v));
}

function DOMRect(xArg, yArg, wArg, hArg) {
let x, y, width, height, left, right, top, bottom;

x = number(xArg);
y = number(yArg);
width = number(wArg);
height = number(hArg);

Object.defineProperties(this, {
x: {
get: function () { return x; },
set: function (newX) {
if (different(x, newX)) {
x = newX;
left = right = undefined;
}
},
enumerable: true
},
y: {
get: function () { return y; },
set: function (newY) {
if (different(y, newY)) {
y = newY;
top = bottom = undefined;
}
},
enumerable: true
},
width: {
get: function () { return width; },
set: function (newWidth) {
if (different(width, newWidth)) {
width = newWidth;
left = right = undefined;
}
},
enumerable: true
},
height: {
get: function () { return height; },
set: function (newHeight) {
if (different(height, newHeight)) {
height = newHeight;
top = bottom = undefined;
}
},
enumerable: true
},
left: {
get: function () {
if (left === undefined) {
left = x + Math.min(0, width);
}
return left;
},
enumerable: true
},
right: {
get: function () {
if (right === undefined) {
right = x + Math.max(0, width);
}
return right;
},
enumerable: true
},
top: {
get: function () {
if (top === undefined) {
top = y + Math.min(0, height);
}
return top;
},
enumerable: true
},
bottom: {
get: function () {
if (bottom === undefined) {
bottom = y + Math.max(0, height);
}
return bottom;
},
enumerable: true
}
});
}

globalThis.DOMRect = DOMRect;
})(globalThis);
}

export {polyfillDOMRect};
16 changes: 16 additions & 0 deletions core/lib/trace-engine.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// @ts-expect-error missing types
import * as TraceEngine from '@paulirish/trace_engine';

import {polyfillDOMRect} from './polyfill-dom-rect.js';

polyfillDOMRect();

const TraceProcessor = TraceEngine.Processor.TraceProcessor;
const TraceHandlers = TraceEngine.Handlers.ModelHandlers;
const RootCauses = TraceEngine.RootCauses.RootCauses.RootCauses;

export {
TraceProcessor,
TraceHandlers,
RootCauses,
};
Loading

0 comments on commit f7ea338

Please sign in to comment.