Skip to content

Commit

Permalink
core: add cap to amp stylesheet links for simulated throttling (#11069)
Browse files Browse the repository at this point in the history
* add basic cap to amp stylesheet

* Switch to computeStackSpecificTiming

* Use new function for simulated time estimate

* Added comment

* lint

* Use node timings returned directly

* Add basic test case

* Switch cap from 1s to 2.1s

* Nits

* Switch to parrallel friendly override

* Add comments

* Add parrallel stylesheet to test case

* Lint

* nits

* Change logic to cap end time not duration

* Update test comments

* Remove Stacks undefined check

* Fix dns comments

* Fix dependent node bug

* nit

* Add dependent to other test case

* add real amp example

* Start to test case

* Add test case details.items

* nits

* lint

* nits

Co-authored-by: Patrick Hulce <patrick.hulce@gmail.com>
  • Loading branch information
adamraine and patrickhulce committed Jul 13, 2020
1 parent 6fe138e commit 259e222
Show file tree
Hide file tree
Showing 4 changed files with 16,781 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,50 @@ function getNodesAndTimingByUrl(nodeTimings) {
return urlMap;
}

/**
* Adjust the timing of a node and its dependencies to account for stack specific overrides.
* @param {Map<Node, LH.Gatherer.Simulation.NodeTiming>} adjustedNodeTimings
* @param {Node} node
* @param {LH.Artifacts.DetectedStack[]} Stacks
*/
function adjustNodeTimings(adjustedNodeTimings, node, Stacks) {
const nodeTiming = adjustedNodeTimings.get(node);
if (!nodeTiming) return;
const stackSpecificTiming = computeStackSpecificTiming(node, nodeTiming, Stacks);
const difference = nodeTiming.duration - stackSpecificTiming.duration;
if (!difference) return;

// AMP's method of removal of stylesheets effectively removes all dependent nodes from the FCP graph
node.traverse(childNode => {
adjustedNodeTimings.delete(childNode);
});
adjustedNodeTimings.set(node, stackSpecificTiming);
}

/**
* Any stack specific timing overrides should go in this function.
* @see https://github.com/GoogleChrome/lighthouse/issues/2832#issuecomment-591066081
*
* @param {Node} node
* @param {LH.Gatherer.Simulation.NodeTiming} nodeTiming
* @param {LH.Artifacts.DetectedStack[]} Stacks
*/
function computeStackSpecificTiming(node, nodeTiming, Stacks) {
const stackSpecificTiming = {...nodeTiming};
if (Stacks.some(stack => stack.id === 'amp')) {
// AMP will load a linked stylesheet asynchronously if it has not been loaded after 2.1 seconds:
// 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 &&
nodeTiming.endTime > 2100) {
stackSpecificTiming.endTime = Math.max(nodeTiming.startTime, 2100);
stackSpecificTiming.duration = stackSpecificTiming.endTime - stackSpecificTiming.startTime;
}
}
return stackSpecificTiming;
}

class RenderBlockingResources extends Audit {
/**
* @return {LH.Audit.Meta}
Expand All @@ -72,7 +116,8 @@ class RenderBlockingResources extends Audit {
description: str_(UIStrings.description),
// TODO: look into adding an `optionalArtifacts` property that captures the non-required nature
// of CSSUsage
requiredArtifacts: ['URL', 'TagsBlockingFirstPaint', 'traces', 'devtoolsLogs', 'CSSUsage'],
requiredArtifacts: ['URL', 'TagsBlockingFirstPaint', 'traces', 'devtoolsLogs', 'CSSUsage',
'Stacks'],
};
}

Expand Down Expand Up @@ -107,11 +152,13 @@ class RenderBlockingResources extends Audit {

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

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

// Mark this node and all its dependents as deferrable
node.traverse(node => deferredNodeIds.add(node.id));

// "wastedMs" is the download time of the network request, responseReceived - requestSent
const wastedMs = Math.round(nodeTiming.duration);
const wastedMs = Math.round(stackSpecificTiming.duration);
if (wastedMs < MINIMUM_WASTED_MS) continue;

results.push({
Expand All @@ -129,7 +176,8 @@ class RenderBlockingResources extends Audit {
simulator,
fcpSimulation.optimisticGraph,
deferredNodeIds,
wastedCssBytes
wastedCssBytes,
artifacts.Stacks
);

return {results, wastedMs};
Expand All @@ -149,13 +197,17 @@ class RenderBlockingResources extends Audit {
* @param {Node} fcpGraph
* @param {Set<string>} deferredIds
* @param {Map<string, number>} wastedCssBytesByUrl
* @param {LH.Artifacts.DetectedStack[]} Stacks
* @return {number}
*/
static estimateSavingsWithGraphs(simulator, fcpGraph, deferredIds, wastedCssBytesByUrl) {
const originalEstimate = simulator.simulate(fcpGraph).timeInMs;
static estimateSavingsWithGraphs(simulator, fcpGraph, deferredIds, wastedCssBytesByUrl, Stacks) {
const {nodeTimings} = simulator.simulate(fcpGraph);
const adjustedNodeTimings = new Map(nodeTimings);

let totalChildNetworkBytes = 0;
const minimalFCPGraph = /** @type {NetworkNode} */ (fcpGraph.cloneWithRelationships(node => {
adjustNodeTimings(adjustedNodeTimings, node, Stacks);

// If a node can be deferred, exclude it from the new FCP graph
const canDeferRequest = deferredIds.has(node.id);
if (node.type !== BaseNode.TYPES.NETWORK) return !canDeferRequest;
Expand All @@ -170,13 +222,18 @@ class RenderBlockingResources extends Audit {
return !canDeferRequest;
}));

// Recalculate the "before" time based on our adjusted node timings.
const estimateBeforeInline = Math.max(...Array.from(
Array.from(adjustedNodeTimings).map(timing => timing[1].endTime)
));

// Add the inlined bytes to the HTML response
const originalTransferSize = minimalFCPGraph.record.transferSize;
const safeTransferSize = originalTransferSize || 0;
minimalFCPGraph.record.transferSize = safeTransferSize + totalChildNetworkBytes;
const estimateAfterInline = simulator.simulate(minimalFCPGraph).timeInMs;
minimalFCPGraph.record.transferSize = originalTransferSize;
return Math.round(Math.max(originalEstimate - estimateAfterInline, 0));
return Math.round(Math.max(estimateBeforeInline - estimateAfterInline, 0));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ const assert = require('assert').strict;
const trace = require('../../fixtures/traces/progressive-app-m60.json');
const devtoolsLog = require('../../fixtures/traces/progressive-app-m60.devtools.log.json');

const ampTrace = require('../../fixtures/traces/amp-m86.trace.json');
const ampDevtoolsLog = require('../../fixtures/traces/amp-m86.devtoolslog.json');

/* eslint-env jest */

describe('Render blocking resources audit', () => {
Expand All @@ -39,18 +42,69 @@ describe('Render blocking resources audit', () => {
assert.equal(result.numericValue, 0);
});

it('evaluates amp page correctly', async () => {
const artifacts = {
traces: {defaultPass: ampTrace},
devtoolsLogs: {defaultPass: ampDevtoolsLog},
TagsBlockingFirstPaint: [
{
tag: {
url:
'https://fonts.googleapis.com/css?family=Fira+Sans+Condensed%3A400%2C400i%2C600%2C600i&subset=latin%2Clatin-ext&display=swap',
},
transferSize: 621,
},
{
tag: {url: 'https://fonts.googleapis.com/css?family=Montserrat'},
transferSize: 621,
},
],
Stacks: [
{
detector: 'js',
id: 'amp',
name: 'AMP',
version: '2006180239003',
npm: 'https://www.npmjs.com/org/ampproject',
},
],
};

const settings = {throttlingMethod: 'simulate', throttling: mobileSlow4G};
const computedCache = new Map();
const result = await RenderBlockingResourcesAudit.audit(artifacts, {settings, computedCache});
expect(result.numericValue).toMatchInlineSnapshot(`450`);
expect(result.details.items).toMatchObject([
{
'totalBytes': 621,
'url': 'https://fonts.googleapis.com/css?family=Fira+Sans+Condensed%3A400%2C400i%2C600%2C600i&subset=latin%2Clatin-ext&display=swap',
'wastedMs': 440,
},
{
'totalBytes': 621,
'url': 'https://fonts.googleapis.com/css?family=Montserrat',
'wastedMs': 440,
},
]);
});

describe('#estimateSavingsWithGraphs', () => {
const estimate = RenderBlockingResourcesAudit.estimateSavingsWithGraphs;

let requestId;
let record;
let recordSlow;

beforeEach(() => {
requestId = 1;
record = props => {
const parsedURL = {host: 'example.com', securityOrigin: 'http://example.com'};
return Object.assign({parsedURL, requestId: requestId++}, props);
};
recordSlow = props => {
const parsedURL = {host: 'slow.com', securityOrigin: 'http://slow.com'};
return Object.assign({parsedURL, requestId: requestId++}, props);
};
});

it('computes savings from deferring', () => {
Expand All @@ -62,11 +116,12 @@ describe('Render blocking resources audit', () => {
const scriptExecution = new CPUNode({tid: 1, ts: 1, dur: 50 * 1000}, []);
const deferredIds = new Set([2, 3]);
const wastedBytesMap = new Map();
const Stacks = [];

documentNode.addDependent(scriptNode);
documentNode.addDependent(styleNode);
documentNode.addDependent(scriptExecution);
const result = estimate(simulator, documentNode, deferredIds, wastedBytesMap);
const result = estimate(simulator, documentNode, deferredIds, wastedBytesMap, Stacks);
// Saving 1000 + 1000 + 100ms for TCP handshake + request/response + server response time
// -200 ms for the CPU task that becomes new bottleneck
assert.equal(result, 1900);
Expand All @@ -81,11 +136,104 @@ describe('Render blocking resources audit', () => {
); // pushes document over 14KB
const deferredIds = new Set([2]);
const wastedBytesMap = new Map([[undefined, 18 * 1000]]);
const Stacks = [];
documentNode.addDependent(styleNode);

const result = estimate(simulator, documentNode, deferredIds, wastedBytesMap);
const result = estimate(simulator, documentNode, deferredIds, wastedBytesMap, Stacks);
// Saving 1000 + 1000 + 100ms for TCP handshake + 1 RT savings + server response time
assert.equal(result, 2100);
});

it('does not report savings from AMP-stack when document already exceeds 2.1s', () => {
const serverResponseTimeByOrigin = new Map([
['http://example.com', 100],
['http://slow.com', 4000],
]);
const Stacks = [
{
detector: 'js',
id: 'amp',
name: 'AMP',
version: '2006180239003',
npm: 'https://www.npmjs.com/org/ampproject',
},
];
const simulator = new Simulator({rtt: 1000, serverResponseTimeByOrigin});
const documentNode = new NetworkNode(record({transferSize: 4000}));
const styleNode = new NetworkNode(
recordSlow({
transferSize: 3000,
resourceType: NetworkRequest.TYPES.Stylesheet,
})
);
const styleNode2 = new NetworkNode(
recordSlow({
transferSize: 3000,
resourceType: NetworkRequest.TYPES.Stylesheet,
})
);
const styleNode3 = new NetworkNode(
recordSlow({
transferSize: 3000,
resourceType: NetworkRequest.TYPES.Stylesheet,
})
);
const deferredIds = new Set([2, 3, 4]);
const wastedBytesMap = new Map();

documentNode.addDependent(styleNode);
styleNode.addDependent(styleNode2);
documentNode.addDependent(styleNode3);
const result = estimate(simulator, documentNode, deferredIds, wastedBytesMap, Stacks);
// Document node: 2000 + 1000 + 100 + 1000 = 4100 for dns + TCP handshake + server response + requests
// The style nodes are loaded async after 2100 so the potential savings are 0
assert.equal(result, 0);
});

it('computes savings for AMP stylesheets loaded partially before 2.1s', () => {
const serverResponseTimeByOrigin = new Map([
['http://example.com', 100],
['http://slow.com', 4000],
]);
const Stacks = [
{
detector: 'js',
id: 'amp',
name: 'AMP',
version: '2006180239003',
npm: 'https://www.npmjs.com/org/ampproject',
},
];
const simulator = new Simulator({rtt: 100, serverResponseTimeByOrigin});
const documentNode = new NetworkNode(record({transferSize: 4000}));
const styleNode = new NetworkNode(
recordSlow({
transferSize: 3000,
resourceType: NetworkRequest.TYPES.Stylesheet,
})
);
const styleNode2 = new NetworkNode(
recordSlow({
transferSize: 3000,
resourceType: NetworkRequest.TYPES.Stylesheet,
})
);
const styleNode3 = new NetworkNode(
recordSlow({
transferSize: 3000,
resourceType: NetworkRequest.TYPES.Stylesheet,
})
);
const deferredIds = new Set([2, 3, 4]);
const wastedBytesMap = new Map();

documentNode.addDependent(styleNode);
styleNode.addDependent(styleNode2);
documentNode.addDependent(styleNode3);
const result = estimate(simulator, documentNode, deferredIds, wastedBytesMap, Stacks);
// Document node: 200 + 100 + 100 + 100 = 500 for dns + TCP handshake + server response + request
// Remaining 1600 ms can be saved before 2100 AMP stylesheet deadline
assert.equal(result, 1600);
});
});
});

Large diffs are not rendered by default.

Loading

0 comments on commit 259e222

Please sign in to comment.