Skip to content

Commit

Permalink
core: deprecate overallSavingsMs (#15902)
Browse files Browse the repository at this point in the history
  • Loading branch information
@adamraine
adamraine committed Apr 1, 2024
1 parent 5827da9 commit 79b8907
Show file tree
Hide file tree
Showing 11 changed files with 47 additions and 191 deletions.
4 changes: 2 additions & 2 deletions cli/test/smokehouse/test-definitions/byte-efficiency.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ const expectations = {
details: {
// the specific ms value is not meaningful for this smoketest
// *some largish amount* of savings should be reported
overallSavingsMs: '>500',
overallSavingsMs: '>100',
overallSavingsBytes: '>45000',
items: [
{
Expand Down Expand Up @@ -218,7 +218,7 @@ const expectations = {
details: {
// the specific ms value is not meaningful for this smoketest
// *some largish amount* of savings should be reported
overallSavingsMs: '>700',
overallSavingsMs: '>100',
overallSavingsBytes: '>50000',
items: {
length: 3,
Expand Down
39 changes: 1 addition & 38 deletions core/audits/byte-efficiency/byte-efficiency-audit.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,9 @@
*/

import {Audit} from '../audit.js';
import {LanternInteractive} from '../../computed/metrics/lantern-interactive.js';
import * as i18n from '../../lib/i18n/i18n.js';
import {NetworkRecords} from '../../computed/network-records.js';
import {LoadSimulator} from '../../computed/load-simulator.js';
import {PageDependencyGraph} from '../../computed/page-dependency-graph.js';
import {LanternLargestContentfulPaint} from '../../computed/metrics/lantern-largest-contentful-paint.js';
import {LanternFirstContentfulPaint} from '../../computed/metrics/lantern-first-contentful-paint.js';
import {LCPImageRecord} from '../../computed/lcp-image-record.js';
Expand Down Expand Up @@ -151,37 +149,6 @@ class ByteEfficiencyAudit extends Audit {
};
}

/**
* Computes the estimated effect of all the byte savings on the maximum of the following:
*
* - end time of the last long task in the provided graph
* - (if includeLoad is true or not provided) end time of the last node in the graph
*
* @param {Array<LH.Audit.ByteEfficiencyItem>} results The array of byte savings results per resource
* @param {Node} graph
* @param {Simulator} simulator
* @param {{includeLoad?: boolean, providedWastedBytesByUrl?: Map<string, number>}=} options
* @return {number}
*/
static computeWasteWithTTIGraph(results, graph, simulator, options) {
options = Object.assign({includeLoad: true}, options);
const {savings: savingsOnOverallLoad, simulationBeforeChanges, simulationAfterChanges} =
this.computeWasteWithGraph(results, graph, simulator, {
...options,
label: 'overallLoad',
});

const savingsOnTTI =
LanternInteractive.getLastLongTaskEndTime(simulationBeforeChanges.nodeTimings) -
LanternInteractive.getLastLongTaskEndTime(simulationAfterChanges.nodeTimings);

let savings = savingsOnTTI;
if (options.includeLoad) savings = Math.max(savings, savingsOnOverallLoad);

// Round waste to nearest 10ms
return Math.round(Math.max(savings, 0) / 10) * 10;
}

/**
* @param {ByteEfficiencyProduct} result
* @param {Simulator} simulator
Expand All @@ -204,18 +171,13 @@ class ByteEfficiencyAudit extends Audit {
// This is useful information in the LHR and should be preserved.
let wastedMs;
if (metricComputationInput.gatherContext.gatherMode === 'navigation') {
const graph = await PageDependencyGraph.request(metricComputationInput, context);
const {
optimisticGraph: optimisticFCPGraph,
} = await LanternFirstContentfulPaint.request(metricComputationInput, context);
const {
optimisticGraph: optimisticLCPGraph,
} = await LanternLargestContentfulPaint.request(metricComputationInput, context);

wastedMs = this.computeWasteWithTTIGraph(results, graph, simulator, {
providedWastedBytesByUrl: result.wastedBytesByUrl,
});

const {savings: fcpSavings} = this.computeWasteWithGraph(
results,
optimisticFCPGraph,
Expand Down Expand Up @@ -243,6 +205,7 @@ class ByteEfficiencyAudit extends Audit {

metricSavings.FCP = fcpSavings;
metricSavings.LCP = Math.max(lcpGraphSavings, lcpRecordSavings);
wastedMs = metricSavings.LCP;
} else {
wastedMs = simulator.computeWastedMsFromWastedBytes(wastedBytes);
}
Expand Down
15 changes: 0 additions & 15 deletions core/audits/byte-efficiency/offscreen-images.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,21 +156,6 @@ class OffscreenImages extends ByteEfficiencyAudit {
});
}

/**
* The default byte efficiency audit will report max(TTI, load), since lazy-loading offscreen
* images won't reduce the overall time and the wasted bytes are really only "wasted" for TTI,
* override the function to just look at TTI savings.
*
* @param {Array<LH.Audit.ByteEfficiencyItem>} results
* @param {LH.Gatherer.Simulation.GraphNode} graph
* @param {LH.Gatherer.Simulation.Simulator} simulator
* @return {number}
*/
static computeWasteWithTTIGraph(results, graph, simulator) {
return super.computeWasteWithTTIGraph(results, graph, simulator,
{includeLoad: false});
}

/**
* @param {LH.Artifacts} artifacts
* @param {Array<LH.Artifacts.NetworkRequest>} networkRecords
Expand Down
36 changes: 2 additions & 34 deletions core/audits/dobetterweb/uses-http2.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,9 @@
import {Audit} from '../audit.js';
import {EntityClassification} from '../../computed/entity-classification.js';
import UrlUtils from '../../lib/url-utils.js';
import {LanternInteractive} from '../../computed/metrics/lantern-interactive.js';
import {NetworkRequest} from '../../lib/network-request.js';
import {NetworkRecords} from '../../computed/network-records.js';
import {LoadSimulator} from '../../computed/load-simulator.js';
import {PageDependencyGraph} from '../../computed/page-dependency-graph.js';
import {LanternLargestContentfulPaint} from '../../computed/metrics/lantern-largest-contentful-paint.js';
import {LanternFirstContentfulPaint} from '../../computed/metrics/lantern-first-contentful-paint.js';
import * as i18n from '../../lib/i18n/i18n.js';
Expand Down Expand Up @@ -117,32 +115,6 @@ class UsesHTTP2Audit extends Audit {
};
}

/**
* Computes the estimated effect all results being converted to use http/2, the max of:
*
* - end time of the last long task in the provided graph
* - end time of the last node in the graph
* @param {Array<{url: string}>} results
* @param {Node} graph
* @param {Simulator} simulator
* @return {number}
*/
static computeWasteWithTTIGraph(results, graph, simulator) {
const {savings: savingsOnOverallLoad, simulationBefore, simulationAfter} =
this.computeWasteWithGraph(results, graph, simulator, {
label: 'tti',
});

const savingsOnTTI =
LanternInteractive.getLastLongTaskEndTime(simulationBefore.nodeTimings) -
LanternInteractive.getLastLongTaskEndTime(simulationAfter.nodeTimings);

const savings = Math.max(savingsOnTTI, savingsOnOverallLoad);

// Round waste to nearest 10ms
return Math.round(Math.max(savings, 0) / 10) * 10;
}

/**
* Determines whether a network request is a "static resource" that would benefit from H2 multiplexing.
* XHRs, tracking pixels, etc generally don't benefit as much because they aren't requested en-masse
Expand Down Expand Up @@ -235,7 +207,6 @@ class UsesHTTP2Audit extends Audit {
* @return {Promise<LH.Audit.Product>}
*/
static async audit(artifacts, context) {
const trace = artifacts.traces[Audit.DEFAULT_PASS];
const devtoolsLog = artifacts.devtoolsLogs[Audit.DEFAULT_PASS];
const URL = artifacts.URL;
const networkRecords = await NetworkRecords.request(devtoolsLog, context);
Expand Down Expand Up @@ -269,7 +240,6 @@ class UsesHTTP2Audit extends Audit {
devtoolsLog,
settings,
};
const pageGraph = await PageDependencyGraph.request({trace, devtoolsLog, URL}, context);
const simulator = await LoadSimulator.request(simulatorOptions, context);
const metricComputationInput = Audit.makeMetricComputationDataInput(artifacts, context);

Expand All @@ -280,8 +250,6 @@ class UsesHTTP2Audit extends Audit {
pessimisticGraph: lcpGraph,
} = await LanternLargestContentfulPaint.request(metricComputationInput, context);

const wastedMsTti = UsesHTTP2Audit.computeWasteWithTTIGraph(
resources, pageGraph, simulator);
const wasteFcp =
UsesHTTP2Audit.computeWasteWithGraph(resources,
fcpGraph, simulator, {label: 'fcp'});
Expand All @@ -296,11 +264,11 @@ class UsesHTTP2Audit extends Audit {
];

const details = Audit.makeOpportunityDetails(headings, resources,
{overallSavingsMs: wastedMsTti});
{overallSavingsMs: wasteLcp.savings});

return {
displayValue,
numericValue: wastedMsTti,
numericValue: wasteLcp.savings,
numericUnit: 'millisecond',
score: resources.length ? 0 : 1,
details,
Expand Down
71 changes: 3 additions & 68 deletions core/test/audits/byte-efficiency/byte-efficiency-audit-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ describe('Byte efficiency base audit', () => {
{computedCache: new Map()}
);

assert.equal(result.numericValue, 300);
assert.equal(result.numericValue, 0);
});

it('should create load simulator with the specified settings', async () => {
Expand All @@ -255,78 +255,13 @@ describe('Byte efficiency base audit', () => {
let result = await MockAudit.audit(artifacts, {settings, computedCache});
// expect modest savings
expect(result.numericValue).toBeLessThan(5000);
expect(result.numericValue).toMatchInlineSnapshot(`4640`);
expect(result.numericValue).toMatchInlineSnapshot(`440`);

settings = {throttlingMethod: 'simulate', throttling: ultraSlowThrottling};
result = await MockAudit.audit(artifacts, {settings, computedCache});
// expect lots of savings
expect(result.numericValue).not.toBeLessThan(5000);
expect(result.numericValue).toMatchInlineSnapshot(`55730`);
});

it('should compute TTI savings differently from load savings', async () => {
class MockAudit extends ByteEfficiencyAudit {
static audit_(artifacts, records) {
return {
items: records.map(record => ({url: record.url, wastedBytes: record.transferSize * 0.5})),
headings: [],
};
}
}

class MockTtiAudit extends MockAudit {
static computeWasteWithTTIGraph(results, graph, simulator) {
return ByteEfficiencyAudit.computeWasteWithTTIGraph(results, graph, simulator,
{includeLoad: false});
}
}

const artifacts = {
GatherContext: {gatherMode: 'navigation'},
traces: {defaultPass: trace},
devtoolsLogs: {defaultPass: devtoolsLog},
URL: getURLArtifactFromDevtoolsLog(devtoolsLog),
};
const computedCache = new Map();

const modestThrottling = {rttMs: 150, throughputKbps: 1000, cpuSlowdownMultiplier: 2};
const settings = {throttlingMethod: 'simulate', throttling: modestThrottling};
const result = await MockAudit.audit(artifacts, {settings, computedCache});
const resultTti = await MockTtiAudit.audit(artifacts, {settings, computedCache});
expect(resultTti.numericValue).toBeLessThan(result.numericValue);
expect(result.numericValue).toMatchInlineSnapshot(`2130`);
expect(resultTti.numericValue).toMatchInlineSnapshot(`110`);
});

it('should allow overriding of computeWasteWithTTIGraph', async () => {
class MockAudit extends ByteEfficiencyAudit {
static audit_(artifacts, records) {
return {
items: records.map(record => ({url: record.url, wastedBytes: record.transferSize * 0.5})),
headings: [],
};
}
}

class MockOverrideAudit extends MockAudit {
static computeWasteWithTTIGraph(results, graph, simulator) {
return 0.5 * ByteEfficiencyAudit.computeWasteWithTTIGraph(results, graph, simulator);
}
}

const artifacts = {
GatherContext: {gatherMode: 'navigation'},
traces: {defaultPass: trace},
devtoolsLogs: {defaultPass: devtoolsLog},
URL: getURLArtifactFromDevtoolsLog(devtoolsLog),
};
const computedCache = new Map();

const modestThrottling = {rttMs: 150, throughputKbps: 1000, cpuSlowdownMultiplier: 2};
const settings = {throttlingMethod: 'simulate', throttling: modestThrottling};
const result = await MockAudit.audit(artifacts, {settings, computedCache});
const resultOverride = await MockOverrideAudit.audit(artifacts, {settings, computedCache});
expect(resultOverride.numericValue).toEqual(result.numericValue * 0.5);
expect(result.numericValue).toMatchInlineSnapshot(`5790`);
});

it('should compute savings with throughput in timespan mode', async () => {
Expand Down
2 changes: 1 addition & 1 deletion core/test/audits/byte-efficiency/duplicated-javascript-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,7 @@ describe('DuplicatedJavascript computed artifact', () => {
const results = await DuplicatedJavascript.audit(artifacts, context);

// Without the `wastedBytesByUrl` this would be zero because the items don't define a url.
expect(results.details.overallSavingsMs).toBe(1680);
expect(results.details.overallSavingsMs).toBe(1370);
});

it('_getNodeModuleName', () => {
Expand Down
4 changes: 2 additions & 2 deletions core/test/fixtures/user-flows/reports/sample-flow-result.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20912,7 +20912,7 @@
"description": "Image formats like WebP and AVIF often provide better compression than PNG or JPEG, which means faster downloads and less data consumption. [Learn more about modern image formats](https://developer.chrome.com/docs/lighthouse/performance/uses-webp-images/).",
"score": 0,
"scoreDisplayMode": "metricSavings",
"numericValue": 300,
"numericValue": 330,
"numericUnit": "millisecond",
"displayValue": "Potential savings of 67 KiB",
"warnings": [],
Expand Down Expand Up @@ -20970,7 +20970,7 @@
"wastedWebpBytes": 65184
}
],
"overallSavingsMs": 300,
"overallSavingsMs": 330,
"overallSavingsBytes": 68889.75,
"sortedBy": [
"wastedBytes"
Expand Down
Loading

0 comments on commit 79b8907

Please sign in to comment.