Skip to content

Commit

Permalink
core(opportunities): take max of savings on TTI, load (GoogleChrome#5084
Browse files Browse the repository at this point in the history
)
  • Loading branch information
patrickhulce authored and kdzwinel committed Aug 16, 2018
1 parent d1ca7d5 commit 3f8b59e
Show file tree
Hide file tree
Showing 10 changed files with 82 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,8 @@ module.exports = [
},
},
'efficient-animated-content': {
score: '<1',
rawValue: '>2000',
extendedInfo: {
value: {
wastedKb: 666,
Expand Down
26 changes: 16 additions & 10 deletions lighthouse-core/audits/byte-efficiency/byte-efficiency-audit.js
Original file line number Diff line number Diff line change
Expand Up @@ -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<LH.Audit.ByteEfficiencyResult>} 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<LH.Audit.ByteEfficiencyResult['url'], LH.Audit.ByteEfficiencyResult>} */
const resultsByUrl = new Map();
Expand Down Expand Up @@ -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;
}

/**
Expand All @@ -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) {
Expand Down
29 changes: 23 additions & 6 deletions lighthouse-core/audits/byte-efficiency/offscreen-images.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'],
};
Expand Down Expand Up @@ -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<LH.Audit.ByteEfficiencyResult>} 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<LH.WebInspector.NetworkRequest>} networkRecords
Expand Down Expand Up @@ -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 &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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. ' +
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/audits/font-display.js
Original file line number Diff line number Diff line change
Expand Up @@ -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).',
Expand Down
3 changes: 1 addition & 2 deletions lighthouse-core/audits/redirects.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'],
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/lib/dependency-graph/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
8 changes: 4 additions & 4 deletions lighthouse-core/test/results/sample_v2.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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": [],
Expand Down Expand Up @@ -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",
Expand Down
2 changes: 2 additions & 0 deletions typings/gatherer.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -31,6 +32,7 @@ declare global {
export type GraphNode = InstanceType<typeof _Node>;
export type GraphNetworkNode = InstanceType<typeof _NetworkNode>;
export type GraphCPUNode = InstanceType<typeof _CPUNode>;
export type Simulator = InstanceType<typeof _Simulator>;

export interface MetricCoefficients {
intercept: number;
Expand Down

0 comments on commit 3f8b59e

Please sign in to comment.