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

core(render-blocking): use trace engine as the source of truth #15839

Merged
merged 15 commits into from
Apr 11, 2024
51 changes: 0 additions & 51 deletions cli/test/smokehouse/test-definitions/dobetterweb.js
Original file line number Diff line number Diff line change
Expand Up @@ -156,57 +156,6 @@ const expectations = {
property: 'og:description',
},
],
TagsBlockingFirstPaint: [
{
tag: {
tagName: 'LINK',
url: 'http://localhost:10200/dobetterweb/dbw_tester.css?delay=100',
},
},
{
tag: {
tagName: 'LINK',
url: 'http://localhost:10200/dobetterweb/unknown404.css?delay=200',
},
},
{
tag: {
tagName: 'LINK',
url: 'http://localhost:10200/dobetterweb/dbw_tester.css?delay=2200',
},

},
{
tag: {
tagName: 'LINK',
url: 'http://localhost:10200/dobetterweb/dbw_tester.css?delay=3000&capped',
mediaChanges: [
{
href: 'http://localhost:10200/dobetterweb/dbw_tester.css?delay=3000&capped',
media: 'not-matching',
matches: false,
},
{
href: 'http://localhost:10200/dobetterweb/dbw_tester.css?delay=3000&capped',
media: 'screen',
matches: true,
},
],
},
},
{
tag: {
tagName: 'SCRIPT',
url: 'http://localhost:10200/dobetterweb/dbw_tester.js',
},
},
{
tag: {
tagName: 'SCRIPT',
url: 'http://localhost:10200/dobetterweb/fcp-delayer.js?delay=5000',
},
},
],
DevtoolsLog: {
_includes: [
// Ensure we are getting async call stacks.
Expand Down
52 changes: 28 additions & 24 deletions core/audits/byte-efficiency/render-blocking-resources.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {ProcessedNavigation} from '../../computed/processed-navigation.js';
import {LoadSimulator} from '../../computed/load-simulator.js';
import {FirstContentfulPaint} from '../../computed/metrics/first-contentful-paint.js';
import {LCPImageRecord} from '../../computed/lcp-image-record.js';
import {TraceEngineResult} from '../../computed/trace-engine-result.js';


/** @typedef {import('../../lib/lantern/simulator/simulator.js').Simulator} Simulator */
Expand All @@ -44,21 +45,19 @@ const str_ = i18n.createIcuMessageFn(import.meta.url, UIStrings);
/**
* Given a simulation's nodeTimings, return an object with the nodes/timing keyed by network URL
* @param {LH.Gatherer.Simulation.Result['nodeTimings']} nodeTimings
* @return {Object<string, {node: Node, nodeTiming: LH.Gatherer.Simulation.NodeTiming}>}
* @return {Map<string, {node: Node, nodeTiming: LH.Gatherer.Simulation.NodeTiming}>}
*/
function getNodesAndTimingByUrl(nodeTimings) {
/** @type {Object<string, {node: Node, nodeTiming: LH.Gatherer.Simulation.NodeTiming}>} */
const urlMap = {};
const nodes = Array.from(nodeTimings.keys());
nodes.forEach(node => {
if (node.type !== 'network') return;
const nodeTiming = nodeTimings.get(node);
if (!nodeTiming) return;

urlMap[node.record.url] = {node, nodeTiming};
});
function getNodesAndTimingByRequestId(nodeTimings) {
/** @type {Map<string, {node: Node, nodeTiming: LH.Gatherer.Simulation.NodeTiming}>} */
const requestIdToNode = new Map();

for (const [node, nodeTiming] of nodeTimings) {
if (node.type !== 'network') continue;

requestIdToNode.set(node.record.requestId, {node, nodeTiming});
}

return urlMap;
return requestIdToNode;
}

/**
Expand Down Expand Up @@ -119,8 +118,7 @@ class RenderBlockingResources extends Audit {
guidanceLevel: 2,
// TODO: look into adding an `optionalArtifacts` property that captures the non-required nature
// of CSSUsage
requiredArtifacts: ['URL', 'TagsBlockingFirstPaint', 'traces', 'devtoolsLogs', 'CSSUsage',
'GatherContext', 'Stacks'],
requiredArtifacts: ['URL', 'traces', 'devtoolsLogs', 'CSSUsage', 'GatherContext', 'Stacks'],
};
}

Expand All @@ -137,6 +135,13 @@ class RenderBlockingResources extends Audit {
const processedNavigation = await ProcessedNavigation.request(trace, context);
const simulator = await LoadSimulator.request(simulatorData, context);
const wastedCssBytes = await RenderBlockingResources.computeWastedCSSBytes(artifacts, context);
const traceEngineResult = await TraceEngineResult.request({trace}, context);

const navInsights = traceEngineResult.insights.get(processedNavigation.navigationId);
if (!navInsights) throw new Error('No insights for navigation');

const renderBlocking = navInsights.RenderBlocking;
if (renderBlocking instanceof Error) throw renderBlocking;

/** @type {LH.Audit.Context['settings']} */
const metricSettings = {
Expand All @@ -150,19 +155,18 @@ class RenderBlockingResources extends Audit {
// Cast to just `LanternMetric` since we explicitly set `throttlingMethod: 'simulate'`.
const fcpSimulation = /** @type {LH.Artifacts.LanternMetric} */
(await FirstContentfulPaint.request(metricComputationData, context));
const fcpTsInMs = processedNavigation.timestamps.firstContentfulPaint / 1000;

const nodesByUrl = getNodesAndTimingByUrl(fcpSimulation.optimisticEstimate.nodeTimings);
const nodesAndTimingsByRequestId =
getNodesAndTimingByRequestId(fcpSimulation.optimisticEstimate.nodeTimings);

const results = [];
const deferredNodeIds = new Set();
for (const resource of artifacts.TagsBlockingFirstPaint) {
// Ignore any resources that finished after observed FCP (they're clearly not render-blocking)
if (resource.endTime > fcpTsInMs) continue;
for (const resource of renderBlocking.renderBlockingRequests) {
const nodeAndTiming = nodesAndTimingsByRequestId.get(resource.args.data.requestId);
// TODO: beacon to Sentry, https://github.com/GoogleChrome/lighthouse/issues/7041
if (!nodesByUrl[resource.tag.url]) continue;
if (!nodeAndTiming) continue;

const {node, nodeTiming} = nodesByUrl[resource.tag.url];
const {node, nodeTiming} = nodeAndTiming;

const stackSpecificTiming = computeStackSpecificTiming(node, nodeTiming, artifacts.Stacks);

Expand All @@ -174,8 +178,8 @@ class RenderBlockingResources extends Audit {
if (wastedMs < MINIMUM_WASTED_MS) continue;

results.push({
url: resource.tag.url,
totalBytes: resource.transferSize,
url: resource.args.data.url,
totalBytes: resource.args.data.encodedDataLength,
wastedMs,
});
}
Expand Down
2 changes: 1 addition & 1 deletion core/audits/layout-shifts.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ class LayoutShifts extends Audit {
static async audit(artifacts, context) {
const trace = artifacts.traces[Audit.DEFAULT_PASS];
const traceEngineResult = await TraceEngineResult.request({trace}, context);
const clusters = traceEngineResult.LayoutShifts.clusters ?? [];
const clusters = traceEngineResult.data.LayoutShifts.clusters ?? [];
const {cumulativeLayoutShift: clsSavings, impactByNodeId} =
await CumulativeLayoutShiftComputed.request(trace, context);
const traceElements = artifacts.TraceElements
Expand Down
33 changes: 19 additions & 14 deletions core/computed/trace-engine-result.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,31 +8,39 @@ import * as TraceEngine from '../lib/trace-engine.js';
import {makeComputedArtifact} from './computed-artifact.js';
import {CumulativeLayoutShift} from './metrics/cumulative-layout-shift.js';
import {ProcessedTrace} from './processed-trace.js';
import * as LH from '../../types/lh.js';

/** @typedef {typeof ENABLED_HANDLERS} EnabledHandlers */

const ENABLED_HANDLERS = {
AuctionWorklets: TraceEngine.TraceHandlers.AuctionWorklets,
Initiators: TraceEngine.TraceHandlers.Initiators,
LayoutShifts: TraceEngine.TraceHandlers.LayoutShifts,
NetworkRequests: TraceEngine.TraceHandlers.NetworkRequests,
Renderer: TraceEngine.TraceHandlers.Renderer,
Samples: TraceEngine.TraceHandlers.Samples,
Screenshots: TraceEngine.TraceHandlers.Screenshots,
PageLoadMetrics: TraceEngine.TraceHandlers.PageLoadMetrics,
};

/**
* @fileoverview Processes trace with the shared trace engine.
*/
class TraceEngineResult {
/**
* @param {LH.TraceEvent[]} traceEvents
* @return {Promise<LH.Artifacts.TraceEngineResult>}
*/
static async runTraceEngine(traceEvents) {
const engine = new TraceEngine.TraceProcessor({
AuctionWorklets: TraceEngine.TraceHandlers.AuctionWorklets,
Initiators: TraceEngine.TraceHandlers.Initiators,
LayoutShifts: TraceEngine.TraceHandlers.LayoutShifts,
NetworkRequests: TraceEngine.TraceHandlers.NetworkRequests,
Renderer: TraceEngine.TraceHandlers.Renderer,
Samples: TraceEngine.TraceHandlers.Samples,
Screenshots: TraceEngine.TraceHandlers.Screenshots,
});
const engine = new TraceEngine.TraceProcessor(ENABLED_HANDLERS);
// eslint-disable-next-line max-len
await engine.parse(/** @type {import('@paulirish/trace_engine').Types.TraceEvents.TraceEventData[]} */ (
traceEvents
));
// TODO: use TraceEngine.TraceProcessor.createWithAllHandlers above.
return /** @type {import('@paulirish/trace_engine').Handlers.Types.TraceParseData} */(
engine.traceParsedData);
if (!engine.traceParsedData) throw new Error('No data');
if (!engine.insights) throw new Error('No insights');
return {data: engine.traceParsedData, insights: engine.insights};
}

/**
Expand Down Expand Up @@ -67,9 +75,6 @@ class TraceEngineResult {
}

const result = await TraceEngineResult.runTraceEngine(traceEvents);
if (!result) {
throw new Error('null trace engine result');
}
return result;
}
}
Expand Down
11 changes: 6 additions & 5 deletions core/gather/gatherers/root-causes.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@ class RootCauses extends BaseGatherer {

/**
* @param {LH.Gatherer.Driver} driver
* @param {LH.Artifacts.TraceEngineResult} traceEngineResult
* @param {LH.Artifacts.TraceEngineResult['data']} traceParsedData
* @return {Promise<LH.Artifacts.TraceEngineRootCauses>}
*/
static async runRootCauseAnalysis(driver, traceEngineResult) {
static async runRootCauseAnalysis(driver, traceParsedData) {
await driver.defaultSession.sendCommand('DOM.enable');
await driver.defaultSession.sendCommand('CSS.enable');

Expand Down Expand Up @@ -112,9 +112,10 @@ class RootCauses extends BaseGatherer {
layoutShifts: {},
};
const rootCausesEngine = new TraceEngine.RootCauses(protocolInterface);
const layoutShiftEvents = traceEngineResult.LayoutShifts.clusters.flatMap(c => c.events);
const layoutShiftEvents = traceParsedData.LayoutShifts.clusters.flatMap(c => c.events);
for (const event of layoutShiftEvents) {
const r = await rootCausesEngine.layoutShifts.rootCausesForEvent(traceEngineResult, event);
// @ts-expect-error The data we do get in the trace processor is still enough here
const r = await rootCausesEngine.layoutShifts.rootCausesForEvent(traceParsedData, event);
if (!r) continue;

for (const cause of r.fontChanges) {
Expand All @@ -137,7 +138,7 @@ class RootCauses extends BaseGatherer {
async getArtifact(context) {
const trace = context.dependencies.Trace;
const traceEngineResult = await TraceEngineResult.request({trace}, context);
return RootCauses.runRootCauseAnalysis(context.driver, traceEngineResult);
return RootCauses.runRootCauseAnalysis(context.driver, traceEngineResult.data);
}
}

Expand Down
4 changes: 2 additions & 2 deletions core/gather/gatherers/trace-elements.js
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ class TraceElements extends BaseGatherer {
* that may have caused the shift.
*
* @param {LH.Trace} trace
* @param {LH.Artifacts.TraceEngineResult} traceEngineResult
* @param {LH.Artifacts.TraceEngineResult['data']} traceEngineResult
* @param {LH.Artifacts.TraceEngineRootCauses} rootCauses
* @param {LH.Gatherer.Context} context
* @return {Promise<Array<{nodeId: number}>>}
Expand Down Expand Up @@ -322,7 +322,7 @@ class TraceElements extends BaseGatherer {

const lcpNodeData = await TraceElements.getLcpElement(trace, context);
const shiftsData = await TraceElements.getTopLayoutShifts(
trace, traceEngineResult, rootCauses, context);
trace, traceEngineResult.data, rootCauses, context);
const animatedElementData = await this.getAnimatedElements(mainThreadEvents);
const responsivenessElementData = await TraceElements.getResponsivenessElement(trace, context);

Expand Down
4 changes: 4 additions & 0 deletions core/lib/tracehouse/trace-processor.js
Original file line number Diff line number Diff line change
Expand Up @@ -829,6 +829,9 @@ class TraceProcessor {
/** @param {number=} ts */
const maybeGetTiming = (ts) => ts === undefined ? undefined : getTiming(ts);

const navigationId = timeOriginEvt.args.data?.navigationId;
if (!navigationId) throw new Error('No navigation ID');
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Several old traces don't have this event...so may need to update them or do a workaround


return {
timings: {
timeOrigin: timings.timeOrigin,
Expand Down Expand Up @@ -864,6 +867,7 @@ class TraceProcessor {
domContentLoadedEvt: frameTimings.domContentLoadedEvt,
fmpFellBack: frameTimings.fmpFellBack,
lcpInvalidated: frameTimings.lcpInvalidated,
navigationId,
};
}

Expand Down
Loading
Loading