From 3f8b59e282e1d85397dbf2b65987b98d14dd821d Mon Sep 17 00:00:00 2001 From: Patrick Hulce Date: Wed, 2 May 2018 16:44:52 -0700 Subject: [PATCH] core(opportunities): take max of savings on TTI, load (#5084) --- .../dobetterweb/dbw-expectations.js | 2 ++ .../byte-efficiency/byte-efficiency-audit.js | 26 +++++++++------ .../byte-efficiency/offscreen-images.js | 29 +++++++++++++---- .../byte-efficiency/uses-optimized-images.js | 2 +- lighthouse-core/audits/font-display.js | 2 +- lighthouse-core/audits/redirects.js | 3 +- lighthouse-core/lib/dependency-graph/node.js | 2 +- .../byte-efficiency-audit-test.js | 32 ++++++++++++++++++- lighthouse-core/test/results/sample_v2.json | 8 ++--- typings/gatherer.d.ts | 2 ++ 10 files changed, 82 insertions(+), 26 deletions(-) diff --git a/lighthouse-cli/test/smokehouse/dobetterweb/dbw-expectations.js b/lighthouse-cli/test/smokehouse/dobetterweb/dbw-expectations.js index c63e504f2751..eeaece514a9b 100644 --- a/lighthouse-cli/test/smokehouse/dobetterweb/dbw-expectations.js +++ b/lighthouse-cli/test/smokehouse/dobetterweb/dbw-expectations.js @@ -164,6 +164,8 @@ module.exports = [ }, }, 'efficient-animated-content': { + score: '<1', + rawValue: '>2000', extendedInfo: { value: { wastedKb: 666, diff --git a/lighthouse-core/audits/byte-efficiency/byte-efficiency-audit.js b/lighthouse-core/audits/byte-efficiency/byte-efficiency-audit.js index e7c32eba4a42..9c1987bea5ca 100644 --- a/lighthouse-core/audits/byte-efficiency/byte-efficiency-audit.js +++ b/lighthouse-core/audits/byte-efficiency/byte-efficiency-audit.js @@ -99,15 +99,20 @@ class UnusedBytes extends Audit { } /** - * Computes the estimated effect of all the byte savings on the last long task - * in the provided graph. + * 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} results The array of byte savings results per resource * @param {Node} graph * @param {Simulator} simulator + * @param {{includeLoad?: boolean}=} options * @return {number} */ - static computeWasteWithTTIGraph(results, graph, simulator) { + static computeWasteWithTTIGraph(results, graph, simulator, options) { + options = Object.assign({includeLoad: true}, options); + const simulationBeforeChanges = simulator.simulate(graph); /** @type {Map} */ const resultsByUrl = new Map(); @@ -142,14 +147,15 @@ class UnusedBytes extends Audit { networkNode.record._transferSize = originalTransferSize; }); - const savingsOnTTI = Math.max( - Interactive.getLastLongTaskEndTime(simulationBeforeChanges.nodeTimings) - - Interactive.getLastLongTaskEndTime(simulationAfterChanges.nodeTimings), - 0 - ); + const savingsOnOverallLoad = simulationBeforeChanges.timeInMs - simulationAfterChanges.timeInMs; + const savingsOnTTI = Interactive.getLastLongTaskEndTime(simulationBeforeChanges.nodeTimings) - + Interactive.getLastLongTaskEndTime(simulationAfterChanges.nodeTimings); + + let savings = savingsOnTTI; + if (options.includeLoad) savings = Math.max(savings, savingsOnOverallLoad); // Round waste to nearest 10ms - return Math.round(savingsOnTTI / 10) * 10; + return Math.round(Math.max(savings, 0) / 10) * 10; } /** @@ -164,7 +170,7 @@ class UnusedBytes extends Audit { const wastedBytes = results.reduce((sum, item) => sum + item.wastedBytes, 0); const wastedKb = Math.round(wastedBytes / KB_IN_BYTES); - const wastedMs = UnusedBytes.computeWasteWithTTIGraph(results, graph, simulator); + const wastedMs = this.computeWasteWithTTIGraph(results, graph, simulator); let displayValue = result.displayValue || ''; if (typeof result.displayValue === 'undefined' && wastedBytes) { diff --git a/lighthouse-core/audits/byte-efficiency/offscreen-images.js b/lighthouse-core/audits/byte-efficiency/offscreen-images.js index ce3140a35e6e..bbf2d3f64f71 100644 --- a/lighthouse-core/audits/byte-efficiency/offscreen-images.js +++ b/lighthouse-core/audits/byte-efficiency/offscreen-images.js @@ -26,12 +26,12 @@ class OffscreenImages extends ByteEfficiencyAudit { static get meta() { return { name: 'offscreen-images', - description: 'Offscreen images', + description: 'Defer offscreen images', informative: true, scoreDisplayMode: ByteEfficiencyAudit.SCORING_MODES.NUMERIC, helpText: - 'Consider lazy-loading offscreen and hidden images to improve page load speed ' + - 'and time to interactive. ' + + 'Consider lazy-loading offscreen and hidden images after all critical resources have ' + + 'finished loading to lower time to interactive. ' + '[Learn more](https://developers.google.com/web/tools/lighthouse/audits/offscreen-images).', requiredArtifacts: ['ImageUsage', 'ViewportDimensions', 'traces', 'devtoolsLogs'], }; @@ -81,6 +81,21 @@ 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} results + * @param {LH.Gatherer.Simulation.GraphNode} graph + * @param {LH.Gatherer.Simulation.Simulator} simulator + * @return {number} + */ + static computeWasteWithTTIGraph(results, graph, simulator) { + return ByteEfficiencyAudit.computeWasteWithTTIGraph(results, graph, simulator, + {includeLoad: false}); + } + /** * @param {LH.Artifacts} artifacts * @param {Array} networkRecords @@ -117,11 +132,13 @@ class OffscreenImages extends ByteEfficiencyAudit { return results; }, new Map()); - // TODO(phulce): move this to always use lantern const settings = context.settings; return artifacts.requestFirstCPUIdle({trace, devtoolsLog, settings}).then(firstInteractive => { - // @ts-ignore - see above TODO. - const ttiTimestamp = firstInteractive.timestamp / 1000000; + // The filter below is just to be extra safe that we exclude images that were loaded post-TTI. + // If we're in the Lantern case and `timestamp` isn't available, we just have to rely on the + // graph simulation doing the right thing. + const ttiTimestamp = firstInteractive.timestamp ? firstInteractive.timestamp / 1e6 : Infinity; + const results = Array.from(resultsMap.values()).filter(item => { const isWasteful = item.wastedBytes > IGNORE_THRESHOLD_IN_BYTES && diff --git a/lighthouse-core/audits/byte-efficiency/uses-optimized-images.js b/lighthouse-core/audits/byte-efficiency/uses-optimized-images.js index bcc5fe7d606b..491862fbdcf8 100644 --- a/lighthouse-core/audits/byte-efficiency/uses-optimized-images.js +++ b/lighthouse-core/audits/byte-efficiency/uses-optimized-images.js @@ -22,7 +22,7 @@ class UsesOptimizedImages extends ByteEfficiencyAudit { static get meta() { return { name: 'uses-optimized-images', - description: 'Optimize images', + description: 'Efficiently encode images', informative: true, scoreDisplayMode: ByteEfficiencyAudit.SCORING_MODES.NUMERIC, helpText: 'Optimized images load faster and consume less cellular data. ' + diff --git a/lighthouse-core/audits/font-display.js b/lighthouse-core/audits/font-display.js index 76d48bff9e4e..5434011faf8e 100644 --- a/lighthouse-core/audits/font-display.js +++ b/lighthouse-core/audits/font-display.js @@ -17,7 +17,7 @@ class FontDisplay extends Audit { return { name: 'font-display', description: 'All text remains visible during webfont loads', - failureDescription: 'Avoid invisible text while webfonts are loading', + failureDescription: 'Text is invisible while webfonts are loading', helpText: 'Leverage the font-display CSS feature to ensure text is user-visible while ' + 'webfonts are loading. ' + '[Learn more](https://developers.google.com/web/updates/2016/02/font-display).', diff --git a/lighthouse-core/audits/redirects.js b/lighthouse-core/audits/redirects.js index 249796e016b6..a9c7762ce162 100644 --- a/lighthouse-core/audits/redirects.js +++ b/lighthouse-core/audits/redirects.js @@ -16,8 +16,7 @@ class Redirects extends Audit { static get meta() { return { name: 'redirects', - description: 'Avoids page redirects', - failureDescription: 'Has multiple page redirects', + description: 'Avoid multiple page redirects', scoreDisplayMode: Audit.SCORING_MODES.NUMERIC, helpText: 'Redirects introduce additional delays before the page can be loaded. [Learn more](https://developers.google.com/speed/docs/insights/AvoidRedirects).', requiredArtifacts: ['URL', 'devtoolsLogs', 'traces'], diff --git a/lighthouse-core/lib/dependency-graph/node.js b/lighthouse-core/lib/dependency-graph/node.js index ed2f444402ef..af1208d8393d 100644 --- a/lighthouse-core/lib/dependency-graph/node.js +++ b/lighthouse-core/lib/dependency-graph/node.js @@ -122,7 +122,7 @@ class Node { * Clones the entire graph connected to this node filtered by the optional predicate. If a node is * included by the predicate, all nodes along the paths between the two will be included. If the * node that was called clone is not included in the resulting filtered graph, the method will throw. - * @param {function(Node):boolean=} predicate + * @param {function(Node):boolean} [predicate] * @return {Node} */ cloneWithRelationships(predicate) { diff --git a/lighthouse-core/test/audits/byte-efficiency/byte-efficiency-audit-test.js b/lighthouse-core/test/audits/byte-efficiency/byte-efficiency-audit-test.js index a1c1fba64d3b..addba84b3b31 100644 --- a/lighthouse-core/test/audits/byte-efficiency/byte-efficiency-audit-test.js +++ b/lighthouse-core/test/audits/byte-efficiency/byte-efficiency-audit-test.js @@ -218,11 +218,41 @@ describe('Byte efficiency base audit', () => { let settings = {throttlingMethod: 'simulate', throttling: modestThrottling}; let result = await MockAudit.audit(artifacts, {settings}); // expect modest savings - assert.equal(result.rawValue, 800); + assert.equal(result.rawValue, 1480); settings = {throttlingMethod: 'simulate', throttling: ultraSlowThrottling}; result = await MockAudit.audit(artifacts, {settings}); // expect lots of savings assert.equal(result.rawValue, 22350); }); + + it('should allow overriding of computeWasteWithTTIGraph', async () => { + class MockAudit extends ByteEfficiencyAudit { + static audit_(artifacts, records) { + return { + results: records.map(record => ({url: record.url, wastedBytes: record.transferSize})), + headings: [], + }; + } + } + + class MockJustTTIAudit extends MockAudit { + static computeWasteWithTTIGraph(results, graph, simulator) { + return ByteEfficiencyAudit.computeWasteWithTTIGraph(results, graph, simulator, + {includeLoad: false}); + } + } + + const artifacts = Runner.instantiateComputedArtifacts(); + artifacts.traces = {defaultPass: trace}; + artifacts.devtoolsLogs = {defaultPass: devtoolsLog}; + + const modestThrottling = {rttMs: 150, throughputKbps: 1000, cpuSlowdownMultiplier: 2}; + const settings = {throttlingMethod: 'simulate', throttling: modestThrottling}; + const result = await MockAudit.audit(artifacts, {settings}); + const resultTti = await MockJustTTIAudit.audit(artifacts, {settings}); + // expect more savings from default + assert.equal(result.rawValue, 1480); + assert.equal(resultTti.rawValue, 800); + }); }); diff --git a/lighthouse-core/test/results/sample_v2.json b/lighthouse-core/test/results/sample_v2.json index e8fa36ec42f5..063967abb8d8 100644 --- a/lighthouse-core/test/results/sample_v2.json +++ b/lighthouse-core/test/results/sample_v2.json @@ -636,7 +636,7 @@ }, "scoreDisplayMode": "numeric", "name": "redirects", - "description": "Avoids page redirects", + "description": "Avoid multiple page redirects", "helpText": "Redirects introduce additional delays before the page can be loaded. [Learn more](https://developers.google.com/speed/docs/insights/AvoidRedirects).", "details": { "type": "table", @@ -2843,8 +2843,8 @@ "scoreDisplayMode": "numeric", "informative": true, "name": "offscreen-images", - "description": "Offscreen images", - "helpText": "Consider lazy-loading offscreen and hidden images to improve page load speed and time to interactive. [Learn more](https://developers.google.com/web/tools/lighthouse/audits/offscreen-images).", + "description": "Defer offscreen images", + "helpText": "Consider lazy-loading offscreen and hidden images after all critical resources have finished loading to lower time to interactive. [Learn more](https://developers.google.com/web/tools/lighthouse/audits/offscreen-images).", "details": { "type": "table", "headings": [], @@ -3111,7 +3111,7 @@ "scoreDisplayMode": "numeric", "informative": true, "name": "uses-optimized-images", - "description": "Optimize images", + "description": "Efficiently encode images", "helpText": "Optimized images load faster and consume less cellular data. [Learn more](https://developers.google.com/web/tools/lighthouse/audits/optimize-images).", "details": { "type": "table", diff --git a/typings/gatherer.d.ts b/typings/gatherer.d.ts index ceede7ef15d0..ff9827cc37d7 100644 --- a/typings/gatherer.d.ts +++ b/typings/gatherer.d.ts @@ -7,6 +7,7 @@ import * as _Node from '../lighthouse-core/lib/dependency-graph/node'; import * as _NetworkNode from '../lighthouse-core/lib/dependency-graph/network-node'; import * as _CPUNode from '../lighthouse-core/lib/dependency-graph/cpu-node'; +import * as _Simulator from '../lighthouse-core/lib/dependency-graph/simulator/simulator'; import * as Driver from '../lighthouse-core/gather/driver'; declare global { @@ -31,6 +32,7 @@ declare global { export type GraphNode = InstanceType; export type GraphNetworkNode = InstanceType; export type GraphCPUNode = InstanceType; + export type Simulator = InstanceType; export interface MetricCoefficients { intercept: number;