Skip to content

Commit

Permalink
core(lantern): resolve some differences when using trace (#16033)
Browse files Browse the repository at this point in the history
  • Loading branch information
connorjclark committed Jun 4, 2024
1 parent cc62c81 commit fdebcbe
Show file tree
Hide file tree
Showing 20 changed files with 125 additions and 122 deletions.
2 changes: 1 addition & 1 deletion core/audits/byte-efficiency/byte-efficiency-audit.js
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ class ByteEfficiencyAudit extends Audit {
const originalTransferSizes = new Map();
graph.traverse(node => {
if (node.type !== 'network') return;
const wastedBytes = wastedBytesByUrl.get(node.record.url);
const wastedBytes = wastedBytesByUrl.get(node.request.url);
if (!wastedBytes) return;

const original = node.request.transferSize;
Expand Down
2 changes: 1 addition & 1 deletion core/audits/byte-efficiency/offscreen-images.js
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ class OffscreenImages extends ByteEfficiencyAudit {
if (node.type === 'cpu' && timing.duration >= 50) {
lastLongTaskStartTime = Math.max(lastLongTaskStartTime, timing.startTime);
} else if (node.type === 'network') {
startTimesByURL.set(node.record.url, timing.startTime);
startTimesByURL.set(node.request.url, timing.startTime);
}
}

Expand Down
10 changes: 5 additions & 5 deletions core/audits/byte-efficiency/render-blocking-resources.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ function computeStackSpecificTiming(node, nodeTiming, Stacks) {
// https://github.com/ampproject/amphtml/blob/8e03ac2f315774070651584a7e046ff24212c9b1/src/font-stylesheet-timeout.js#L54-L59
// Any potential savings must only include time spent on AMP stylesheet nodes before 2.1 seconds.
if (node.type === BaseNode.TYPES.NETWORK &&
node.record.resourceType === NetworkRequest.TYPES.Stylesheet &&
node.request.resourceType === NetworkRequest.TYPES.Stylesheet &&
nodeTiming.endTime > 2100) {
stackSpecificTiming.endTime = Math.max(nodeTiming.startTime, 2100);
stackSpecificTiming.duration = stackSpecificTiming.endTime - stackSpecificTiming.startTime;
Expand Down Expand Up @@ -228,11 +228,11 @@ class RenderBlockingResources extends Audit {
if (node.type !== BaseNode.TYPES.NETWORK) return !canDeferRequest;

const isStylesheet =
node.record.resourceType === NetworkRequest.TYPES.Stylesheet;
node.request.resourceType === NetworkRequest.TYPES.Stylesheet;
if (canDeferRequest && isStylesheet) {
// We'll inline the used bytes of the stylesheet and assume the rest can be deferred
const wastedBytes = wastedCssBytesByUrl.get(node.record.url) || 0;
totalChildNetworkBytes += (node.record.transferSize || 0) - wastedBytes;
const wastedBytes = wastedCssBytesByUrl.get(node.request.url) || 0;
totalChildNetworkBytes += (node.request.transferSize || 0) - wastedBytes;
}
return !canDeferRequest;
});
Expand All @@ -247,7 +247,7 @@ class RenderBlockingResources extends Audit {
));

// Add the inlined bytes to the HTML response
const originalTransferSize = minimalFCPGraph.record.transferSize;
const originalTransferSize = minimalFCPGraph.request.transferSize;
const safeTransferSize = originalTransferSize || 0;
minimalFCPGraph.request.transferSize = safeTransferSize + totalChildNetworkBytes;
const estimateAfterInline = simulator.simulate(minimalFCPGraph).timeInMs;
Expand Down
4 changes: 2 additions & 2 deletions core/audits/dobetterweb/uses-http2.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,9 @@ class UsesHTTP2Audit extends Audit {
const originalProtocols = new Map();
graph.traverse(node => {
if (node.type !== 'network') return;
if (!urlsToChange.has(node.record.url)) return;
if (!urlsToChange.has(node.request.url)) return;

originalProtocols.set(node.request.requestId, node.record.protocol);
originalProtocols.set(node.request.requestId, node.request.protocol);
node.request.protocol = 'h2';
});

Expand Down
2 changes: 1 addition & 1 deletion core/audits/prioritize-lcp-image.js
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ class PrioritizeLcpImage extends Audit {
wastedMs,
results: [{
node: Audit.makeNodeItem(lcpElement.node),
url: lcpNode.record.url,
url: lcpNode.request.url,
wastedMs,
}],
};
Expand Down
4 changes: 2 additions & 2 deletions core/audits/uses-rel-preconnect.js
Original file line number Diff line number Diff line change
Expand Up @@ -146,14 +146,14 @@ class UsesRelPreconnectAudit extends Audit {
/** @type {Set<string>} */
const lcpGraphURLs = new Set();
lcpGraph.traverse(node => {
if (node.type === 'network') lcpGraphURLs.add(node.record.url);
if (node.type === 'network') lcpGraphURLs.add(node.request.url);
});

const fcpGraph =
await LanternFirstContentfulPaint.getPessimisticGraph(pageGraph, processedNavigation);
const fcpGraphURLs = new Set();
fcpGraph.traverse(node => {
if (node.type === 'network') fcpGraphURLs.add(node.record.url);
if (node.type === 'network') fcpGraphURLs.add(node.request.url);
});

/** @type {Map<string, LH.Artifacts.NetworkRequest[]>} */
Expand Down
1 change: 1 addition & 0 deletions core/computed/metrics/lantern-metric.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ async function getComputationDataParamsFromTrace(data, context) {
}

const {trace, URL} = data;
// TODO(15841): use computed artifact.
const {graph} = await createGraphFromTrace(URL, trace, context);
const processedNavigation = await ProcessedNavigation.request(data.trace, context);
const simulator = data.simulator || (await LoadSimulator.request(data, context));
Expand Down
1 change: 0 additions & 1 deletion core/lib/lantern/network-node.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ const NON_NETWORK_SCHEMES = [
];

/**
* Use `NetworkRequest.isNonNetworkRequest(req)` if working with a request.
* Note: the `protocol` field from CDP can be 'h2', 'http', (not 'https'!) or it'll be url's scheme.
* https://source.chromium.org/chromium/chromium/src/+/main:content/browser/devtools/protocol/network_handler.cc;l=598-611;drc=56d4a9a9deb30be73adcee8737c73bcb2a5ab64f
* However, a `new URL(href).protocol` has a colon suffix.
Expand Down
60 changes: 46 additions & 14 deletions core/lib/lantern/page-dependency-graph.js
Original file line number Diff line number Diff line change
Expand Up @@ -712,6 +712,19 @@ class PageDependencyGraph {
if (request.args.data.initiator?.fetchType === 'xmlhttprequest') {
// @ts-expect-error yes XHR is a valid ResourceType. TypeScript const enums are so unhelpful.
resourceType = 'XHR';
} else if (request.args.data.initiator?.fetchType === 'fetch') {
// @ts-expect-error yes Fetch is a valid ResourceType. TypeScript const enums are so unhelpful.
resourceType = 'Fetch';
}

// TODO: set decodedBodyLength for data urls in Trace Engine.
let resourceSize = request.args.data.decodedBodyLength ?? 0;
if (url.protocol === 'data:' && resourceSize === 0) {
const needle = 'base64,';
const index = url.pathname.indexOf(needle);
if (index !== -1) {
resourceSize = atob(url.pathname.substring(index + needle.length)).length;
}
}

return {
Expand All @@ -727,7 +740,7 @@ class PageDependencyGraph {
responseHeadersEndTime: request.args.data.syntheticData.downloadStart / 1000,
networkEndTime: request.args.data.syntheticData.finishTime / 1000,
transferSize: request.args.data.encodedDataLength,
resourceSize: request.args.data.decodedBodyLength,
resourceSize,
fromDiskCache: request.args.data.syntheticData.isDiskCached,
fromMemoryCache: request.args.data.syntheticData.isMemoryCached,
isLinkPreload: request.args.data.isLinkPreload,
Expand Down Expand Up @@ -800,23 +813,42 @@ class PageDependencyGraph {
const requestChain = [];
for (const redirect of redirects) {
const redirectedRequest = structuredClone(request);
if (redirectedRequest.timing) {
// TODO: These are surely wrong for when there is more than one redirect. Would be
// simpler if each redirect remembered it's `timing` object in this `redirects` array.
redirectedRequest.timing.requestTime = redirect.ts / 1000 / 1000;
redirectedRequest.timing.receiveHeadersStart -= redirect.dur / 1000 / 1000;
redirectedRequest.timing.receiveHeadersEnd -= redirect.dur / 1000 / 1000;
redirectedRequest.rendererStartTime = redirect.ts / 1000;
redirectedRequest.networkRequestTime = redirect.ts / 1000;
redirectedRequest.networkEndTime = (redirect.ts + redirect.dur) / 1000;
redirectedRequest.responseHeadersEndTime =
redirectedRequest.timing.requestTime * 1000 +
redirectedRequest.timing.receiveHeadersEnd;
}

redirectedRequest.networkRequestTime = redirect.ts / 1000;
redirectedRequest.rendererStartTime = redirectedRequest.networkRequestTime;

redirectedRequest.networkEndTime = (redirect.ts + redirect.dur) / 1000;
redirectedRequest.responseHeadersEndTime = redirectedRequest.networkEndTime;

redirectedRequest.timing = {
requestTime: redirectedRequest.networkRequestTime / 1000,
receiveHeadersStart: redirectedRequest.responseHeadersEndTime,
receiveHeadersEnd: redirectedRequest.responseHeadersEndTime,
proxyStart: -1,
proxyEnd: -1,
dnsStart: -1,
dnsEnd: -1,
connectStart: -1,
connectEnd: -1,
sslStart: -1,
sslEnd: -1,
sendStart: -1,
sendEnd: -1,
workerStart: -1,
workerReady: -1,
workerFetchStart: -1,
workerRespondWithSettled: -1,
pushStart: -1,
pushEnd: -1,
};

redirectedRequest.url = redirect.url;
redirectedRequest.parsedURL = this._createParsedUrl(redirect.url);
// TODO: TraceEngine is not retaining the actual status code.
redirectedRequest.statusCode = 302;
redirectedRequest.resourceType = undefined;
// TODO: TraceEngine is not retaining transfer size of redirected request.
redirectedRequest.transferSize = 400;
requestChain.push(redirectedRequest);
lanternRequests.push(redirectedRequest);
}
Expand Down
2 changes: 1 addition & 1 deletion core/lib/lantern/simulator/simulator.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ const NodeState = {
Complete: 3,
};

/** @type {Record<NetworkNode['record']['priority'], number>} */
/** @type {Record<NetworkNode['request']['priority'], number>} */
const PriorityStartTimePenalty = {
VeryHigh: 0,
High: 0.25,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,6 @@ const trace = readJson('../../fixtures/artifacts/paul/trace.json', import.meta);
const devtoolsLog = readJson('../../fixtures/artifacts/paul/devtoolslog.json', import.meta);

describe('Byte efficiency base audit', () => {
// TODO(15841): investigate failures
if (process.env.INTERNAL_LANTERN_USE_TRACE !== undefined) {
return;
}

let simulator;
let metricComputationInput;

Expand Down Expand Up @@ -131,7 +126,7 @@ describe('Byte efficiency base audit', () => {
}, simulator, metricComputationInput, {computedCache: new Map()});

assert.equal(result.metricSavings.FCP, 900);
assert.equal(result.metricSavings.LCP, 1350);
assert.equal(result.metricSavings.LCP, 900);
});

it('should use LCP request savings if larger than LCP graph savings', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -313,11 +313,6 @@ describe('DuplicatedJavascript computed artifact', () => {
});

it('.audit', async () => {
// TODO(15841): investigate failures
if (process.env.INTERNAL_LANTERN_USE_TRACE !== undefined) {
return;
}

const artifacts = await loadArtifacts(`${LH_ROOT}/core/test/fixtures/artifacts/cnn`);
const ultraSlowThrottling = {rttMs: 150, throughputKbps: 100, cpuSlowdownMultiplier: 8};
const settings = {throttlingMethod: 'simulate', throttling: ultraSlowThrottling};
Expand Down
5 changes: 0 additions & 5 deletions core/test/audits/byte-efficiency/offscreen-images-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,6 @@ function generateImage({
}

describe('OffscreenImages audit', () => {
// TODO(15841): investigate test failures
if (process.env.INTERNAL_LANTERN_USE_TRACE !== undefined) {
return;
}

let context;
const DEFAULT_DIMENSIONS = {innerWidth: 1920, innerHeight: 1080};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,6 @@ const devtoolsLog = readJson('../../fixtures/artifacts/render-blocking/devtoolsl
const mobileSlow4G = constants.throttling.mobileSlow4G;

describe('Render blocking resources audit', () => {
// TODO(15841): investigate failures
if (process.env.INTERNAL_LANTERN_USE_TRACE !== undefined) {
return;
}

it('evaluates render blocking input correctly', async () => {
const artifacts = {
URL: getURLArtifactFromDevtoolsLog(devtoolsLog),
Expand Down
13 changes: 3 additions & 10 deletions core/test/audits/dobetterweb/dom-size-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,17 @@ describe('DOMSize audit', () => {

beforeEach(() => {
const mainDocumentUrl = 'https://example.com/';

const networkRecords = [{url: mainDocumentUrl, priority: 'High'}];
const trace = createTestTrace({
topLevelTasks: [
{ts: 1000, duration: 1000, children: [
{ts: 1100, duration: 200, eventName: 'ScheduleStyleRecalculation'},
]},
],
networkRecords,
});

const devtoolsLog = networkRecordsToDevtoolsLog([{
url: mainDocumentUrl,
priority: 'High',
}]);
const devtoolsLog = networkRecordsToDevtoolsLog(networkRecords);

artifacts = {
DOMStats: {
Expand All @@ -55,11 +53,6 @@ describe('DOMSize audit', () => {
});

it('calculates score hitting mid distribution', async () => {
// TODO(15841): investigate failures
if (process.env.INTERNAL_LANTERN_USE_TRACE !== undefined) {
return;
}

const auditResult = await DOMSize.audit(artifacts, context);
assert.equal(auditResult.score, 0.43);
assert.equal(auditResult.numericValue, 1500);
Expand Down
17 changes: 6 additions & 11 deletions core/test/audits/largest-contentful-paint-element-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ function mockNetworkRecords() {
isLinkPreload: false,
networkRequestTime: 0,
networkEndTime: 500,
timing: {sendEnd: 0, receiveHeadersEnd: 500},
responseHeadersEndTime: 500,
responseHeadersTransferSize: 400,
transferSize: 400,
url: requestedUrl,
Expand Down Expand Up @@ -68,11 +68,6 @@ function mockNetworkRecords() {
}

describe('Performance: largest-contentful-paint-element audit', () => {
// TODO(15841): investigate failures
if (process.env.INTERNAL_LANTERN_USE_TRACE !== undefined) {
return;
}

it('correctly surfaces the LCP element', async () => {
const networkRecords = mockNetworkRecords();
const artifacts = {
Expand Down Expand Up @@ -110,8 +105,8 @@ describe('Performance: largest-contentful-paint-element audit', () => {

expect(auditResult.score).toEqual(0);
expect(auditResult.notApplicable).toBeUndefined();
expect(auditResult.displayValue).toBeDisplayString('5,800\xa0ms');
expect(auditResult.metricSavings).toEqual({LCP: 3304}); // 5804 - 2500 (p10 mobile)
expect(auditResult.displayValue).toBeDisplayString('5,340\xa0ms');
expect(auditResult.metricSavings).toEqual({LCP: 2837}); // calculated LCP - 2500 (p10 mobile)
expect(auditResult.details.items).toHaveLength(2);
expect(auditResult.details.items[0].items).toHaveLength(1);
expect(auditResult.details.items[0].items[0].node.path).toEqual('1,HTML,3,BODY,5,DIV,0,HEADER');
Expand All @@ -123,11 +118,11 @@ describe('Performance: largest-contentful-paint-element audit', () => {
expect(auditResult.details.items[1].items[0].phase).toBeDisplayString('TTFB');
expect(auditResult.details.items[1].items[0].timing).toBeCloseTo(800, 0.1);
expect(auditResult.details.items[1].items[1].phase).toBeDisplayString('Load Delay');
expect(auditResult.details.items[1].items[1].timing).toBeCloseTo(651, 0.1);
expect(auditResult.details.items[1].items[1].timing).toBeCloseTo(534.2, 0.1);
expect(auditResult.details.items[1].items[2].phase).toBeDisplayString('Load Time');
expect(auditResult.details.items[1].items[2].timing).toBeCloseTo(1813.7, 0.1);
expect(auditResult.details.items[1].items[2].timing).toBeCloseTo(1667.8, 0.1);
expect(auditResult.details.items[1].items[3].phase).toBeDisplayString('Render Delay');
expect(auditResult.details.items[1].items[3].timing).toBeCloseTo(2539.2, 0.1);
expect(auditResult.details.items[1].items[3].timing).toBeCloseTo(2334.9, 0.1);
});

it('doesn\'t throw an error when there is nothing to show', async () => {
Expand Down
5 changes: 0 additions & 5 deletions core/test/audits/prioritize-lcp-image-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,6 @@ const scriptUrl = 'http://www.example.com/script.js';
const imageUrl = 'http://www.example.com/image.png';

describe('Performance: prioritize-lcp-image audit', () => {
// TODO(15841): fix createTestTrace, cycles
if (process.env.INTERNAL_LANTERN_USE_TRACE !== undefined) {
return;
}

const mockArtifacts = (networkRecords, URL) => {
return {
GatherContext: {gatherMode: 'navigation'},
Expand Down
Loading

0 comments on commit fdebcbe

Please sign in to comment.