Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Fix critical chains & time to first byte errors
Browse files Browse the repository at this point in the history
wardpeet committed May 11, 2017
1 parent b51b906 commit b1b2161
Showing 7 changed files with 515 additions and 441 deletions.
6 changes: 5 additions & 1 deletion lighthouse-core/audits/time-to-first-byte.js
Original file line number Diff line number Diff line change
@@ -43,6 +43,10 @@ class TTFBMetric extends Audit {
static caclulateTTFB(record) {
const timing = record._timing;

if (!timing) {
console.log(record.url, record._timing, record);
}

return timing.receiveHeadersEnd - timing.sendEnd;
}

@@ -66,7 +70,7 @@ class TTFBMetric extends Audit {
criticalRequests.forEach(request => {
const networkRecord = networkRecords.find(record => record._requestId === request.id);

if (networkRecord) {
if (networkRecord && networkRecord._timing) {
const ttfb = TTFBMetric.caclulateTTFB(networkRecord);
results.push({
url: URL.getDisplayName(networkRecord._url),
3 changes: 3 additions & 0 deletions lighthouse-core/closure/typedefs/ComputedArtifacts.js
Original file line number Diff line number Diff line change
@@ -37,6 +37,9 @@ let TraceOfTabArtifact;
*/
function ComputedArtifacts() {}

/** @type {function(!Array): !Promise<!Object>} */
ComputedArtifacts.prototype.requestCriticalRequests;

/** @type {function(!Array): !Promise<!Object>} */
ComputedArtifacts.prototype.requestCriticalRequestChains;

142 changes: 37 additions & 105 deletions lighthouse-core/gather/computed/critical-request-chains.js
Original file line number Diff line number Diff line change
@@ -18,125 +18,57 @@
'use strict';

const ComputedArtifact = require('./computed-artifact');
const WebInspector = require('../../lib/web-inspector');

class CriticalRequestChains extends ComputedArtifact {

get name() {
return 'CriticalRequestChains';
}

/**
* For now, we use network priorities as a proxy for "render-blocking"/critical-ness.
* It's imperfect, but there is not a higher-fidelity signal available yet.
* @see https://docs.google.com/document/d/1bCDuq9H1ih9iNjgzyAL0gpwNFiEP4TZS-YLRp_RuMlc
* @param {any} request
*/
isCritical(request) {
const resourceTypeCategory = request._resourceType && request._resourceType._category;

// XHRs are fetched at High priority, but we exclude them, as they are unlikely to be critical
// Images are also non-critical.
const nonCriticalResourceTypes = [
WebInspector.resourceTypes.Image._category,
WebInspector.resourceTypes.XHR._category
];
if (nonCriticalResourceTypes.includes(resourceTypeCategory)) {
return false;
}

// Treat favicons as non-critical resources
if (request.mimeType === 'image/x-icon' ||
(request.parsedURL && request.parsedURL.lastPathComponent === 'favicon.ico')) {
return false;
}

return ['VeryHigh', 'High', 'Medium'].includes(request.priority());
}

compute_(networkRecords) {
// Build a map of requestID -> Node.
const requestIdToRequests = new Map();
for (const request of networkRecords) {
requestIdToRequests.set(request.requestId, request);
}

// Get all the critical requests.
/** @type {!Array<NetworkRequest>} */
const criticalRequests = networkRecords.filter(req => this.isCritical(req));

const flattenRequest = request => {
return {
url: request._url,
generateChain(request) {
return {
request: {
id: request.id,
url: request.url,
startTime: request.startTime,
endTime: request.endTime,
responseReceivedTime: request.responseReceivedTime,
transferSize: request.transferSize
};
transferSize: request.transferSize,
},
children: {},
};
}

// Create a tree of critical requests.
const criticalRequestChains = {};
for (const request of criticalRequests) {
// Work back from this request up to the root. If by some weird quirk we are giving request D
// here, which has ancestors C, B and A (where A is the root), we will build array [C, B, A]
// during this phase.
const ancestors = [];
let ancestorRequest = request.initiatorRequest();
let node = criticalRequestChains;
while (ancestorRequest) {
const ancestorIsCritical = this.isCritical(ancestorRequest);

// If the parent request isn't a high priority request it won't be in the
// requestIdToRequests map, and so we can break the chain here. We should also
// break it if we've seen this request before because this is some kind of circular
// reference, and that's bad.
if (!ancestorIsCritical || ancestors.includes(ancestorRequest.requestId)) {
// Set the ancestors to an empty array and unset node so that we don't add
// the request in to the tree.
ancestors.length = 0;
node = undefined;
break;
}
ancestors.push(ancestorRequest.requestId);
ancestorRequest = ancestorRequest.initiatorRequest();
}

// With the above array we can work from back to front, i.e. A, B, C, and during this process
// we can build out the tree for any nodes that have yet to be created.
let ancestor = ancestors.pop();
while (ancestor) {
const parentRequest = requestIdToRequests.get(ancestor);
const parentRequestId = parentRequest.requestId;
if (!node[parentRequestId]) {
node[parentRequestId] = {
request: flattenRequest(parentRequest),
children: {}
};
compute_(networkRecords, artifacts) {
return artifacts.requestCriticalRequests(networkRecords)
.then(criticalRequests => {
// Create a tree of critical requests.
const criticalRequestChains = {};
const mappedRequests = {};

let request = criticalRequests.shift();
while(request) {
if (!mappedRequests[request.id]) {
mappedRequests[request.id] = this.generateChain(request);
}

const node = mappedRequests[request.id];
const parent = request.parent;
if (parent) {
if (!mappedRequests[parent.id]) {
mappedRequests[parent.id] = this.generateChain(parent);
}

mappedRequests[parent.id].children[request.id] = node;
} else {
criticalRequestChains[request.id] = node;
}

request = criticalRequests.shift();
}

// Step to the next iteration.
ancestor = ancestors.pop();
node = node[parentRequestId].children;
}

if (!node) {
continue;
}

// If the node already exists, bail.
if (node[request.requestId]) {
continue;
}

// node should now point to the immediate parent for this request.
node[request.requestId] = {
request: flattenRequest(request),
children: {}
};
}

return criticalRequestChains;
return criticalRequestChains;
});
}
}

74 changes: 61 additions & 13 deletions lighthouse-core/gather/computed/critical-requests.js
Original file line number Diff line number Diff line change
@@ -32,10 +32,16 @@ class CriticalRequests extends ComputedArtifact {
* @see https://docs.google.com/document/d/1bCDuq9H1ih9iNjgzyAL0gpwNFiEP4TZS-YLRp_RuMlc
* @param {any} request
*/
static isCritical(request) {
// XHRs are fetched at High priority, but we exclude them, as they are unlikely to be critical
isCritical(request) {
const resourceTypeCategory = request._resourceType && request._resourceType._category;
if (resourceTypeCategory === WebInspector.resourceTypes.XHR._category) {

// XHRs are fetched at High priority, but we exclude them, as they are unlikely to be critical
// Images are also non-critical.
const nonCriticalResourceTypes = [
WebInspector.resourceTypes.Image._category,
WebInspector.resourceTypes.XHR._category
];
if (nonCriticalResourceTypes.includes(resourceTypeCategory)) {
return false;
}

@@ -48,19 +54,61 @@ class CriticalRequests extends ComputedArtifact {
return ['VeryHigh', 'High', 'Medium'].includes(request.priority());
}

flattenRequest(record) {
const ancestor = record.initiatorRequest();

return {
id: record._requestId,
url: record._url,
startTime: record.startTime,
endTime: record.endTime,
responseReceivedTime: record.responseReceivedTime,
transferSize: record.transferSize,
parent: ancestor ? this.flattenRequest(ancestor) : null,
};
}

compute_(networkRecords) {
// Get all the critical requests.
/** @type {!Array<NetworkRequest>} */
const criticalRequests = networkRecords.filter(req => CriticalRequests.isCritical(req));

return criticalRequests.map(req => ({
id: req._requestId,
url: req._url,
startTime: req.startTime,
endTime: req.endTime,
responseReceivedTime: req.responseReceivedTime,
transferSize: req.transferSize,
}));
const criticalRequests = networkRecords.filter(req => this.isCritical(req));
const requestIds = [];

// Create a tree of critical requests.
const flattenedRequests = [];
for (const request of criticalRequests) {
// Work back from this request up to the root. If by some weird quirk we are giving request D
// here, which has ancestors C, B and A (where A is the root), we will build array [C, B, A]
// during this phase.
const ancestors = [];
let ancestorRequest = request.initiatorRequest();
while (ancestorRequest) {
const ancestorIsCritical = this.isCritical(ancestorRequest);

// If the parent request isn't a high priority request it won't be in the
// requestIdToRequests map, and so we can break the chain here. We should also
// break it if we've seen this request before because this is some kind of circular
// reference, and that's bad.
if (!ancestorIsCritical || ancestors.includes(ancestorRequest._requestId)) {
// Set the ancestors to an empty array and unset node so that we don't add
// the request in to the tree.
ancestors.length = 0;
break;
}

ancestors.push(ancestorRequest._requestId);
ancestorRequest = ancestorRequest.initiatorRequest();
}

const isAlreadyLogged = requestIds.indexOf(request._requestId) === -1;
const isHighPriorityChain = !request.initiatorRequest() || ancestors.length;
if (isAlreadyLogged && isHighPriorityChain) {
flattenedRequests.push(this.flattenRequest(request));
requestIds.push(request._requestId);
}
}

return flattenedRequests;
}
}

93 changes: 40 additions & 53 deletions lighthouse-core/test/audits/critical-request-chains-test.js
Original file line number Diff line number Diff line change
@@ -18,62 +18,49 @@
const Audit = require('../../audits/critical-request-chains.js');
const assert = require('assert');

const FAILING_REQUEST_CHAIN = {
0: {
request: {
endTime: 1,
responseReceivedTime: 5,
startTime: 0,
url: 'https://example.com/'
},
children: {
1: {
request: {
endTime: 16,
responseReceivedTime: 14,
startTime: 11,
url: 'https://example.com/b.js'
},
children: {
}
},
2: {
request: {
endTime: 17,
responseReceivedTime: 15,
startTime: 12,
url: 'https://example.com/c.js'
},
children: {}
}
}
}
const rootElm = {
id: '0',
endTime: 1,
responseReceivedTime: 5,
startTime: 0,
url: 'https://example.com/',
parent: null,
};

const PASSING_REQUEST_CHAIN = {
0: {
request: {
endTime: 1,
responseReceivedTime: 5,
startTime: 0,
url: 'https://example.com/'
},
children: {},
const FAILING_REQUEST_CHAIN = [
rootElm,
{
id: '1',
endTime: 16,
responseReceivedTime: 14,
startTime: 11,
url: 'https://example.com/b.js',
parent: rootElm,
},
};
{
id: '2',
endTime: 17,
responseReceivedTime: 15,
startTime: 12,
url: 'https://example.com/c.js',
parent: rootElm,
},
];

const PASSING_REQUEST_CHAIN_2 = {
13653.1: {
request: {
url: 'http://localhost:10503/offline-ready.html',
startTime: 33552.036878,
endTime: 33552.285438,
responseReceivedTime: 33552.275677,
transferSize: 1849
},
children: {}
const PASSING_REQUEST_CHAIN = [
rootElm,
];

const PASSING_REQUEST_CHAIN_2 = [
{
id: '13653.1',
url: 'http://localhost:10503/offline-ready.html',
startTime: 33552.036878,
endTime: 33552.285438,
responseReceivedTime: 33552.275677,
transferSize: 1849,
parent: null,
}
};
];

const EMPTY_REQUEST_CHAIN = {};

@@ -85,7 +72,7 @@ const mockArtifacts = (mockChain) => {
requestNetworkRecords: () => {
return Promise.resolve([]);
},
requestCriticalRequestChains: function() {
requestCriticalRequest: function() {
return Promise.resolve(mockChain);
}
};
441 changes: 172 additions & 269 deletions lighthouse-core/test/gather/computed/critical-request-chains-test.js

Large diffs are not rendered by default.

197 changes: 197 additions & 0 deletions lighthouse-core/test/gather/computed/critical-requests-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,197 @@
/**
* Copyright 2016 Google Inc. All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
'use strict';

/* eslint-env mocha */

const GathererClass = require('../../../gather/computed/critical-requests');
const assert = require('assert');
const Gatherer = new GathererClass();

const HIGH = 'High';
const VERY_HIGH = 'VeryHigh';
const MEDIUM = 'Medium';
const LOW = 'Low';
const VERY_LOW = 'VeryLow';

function mockTracingData(prioritiesList, edges) {
const networkRecords = prioritiesList.map((priority, index) =>
({_requestId: index.toString(),
_resourceType: {
_category: 'fake'
},
priority: () => priority,
initiatorRequest: () => null
}));

// add mock initiator information
edges.forEach(edge => {
const initiator = networkRecords[edge[0]];
networkRecords[edge[1]].initiatorRequest = () => initiator;
});

return networkRecords;
}

function testGetCriticalRequests(data) {
const networkRecords = mockTracingData(data.priorityList, data.edges);
return Gatherer.request(networkRecords).then(criticalRequests => {
assert.deepEqual(criticalRequests, data.expected);
});
}

let requests;
function constructEmptyRequest(parent = null, id = null) {
const request = {
id,
parent: requests[parent] || null,
endTime: undefined,
responseReceivedTime: undefined,
startTime: undefined,
url: undefined,
transferSize: undefined
};

requests[id] = request;

return request;
}

describe('CriticalRequest gatherer: getCriticalRequests function', () => {
beforeEach(() => {
requests = {};
});

it('returns correct data for four critical requests', () =>
testGetCriticalRequests({
priorityList: [HIGH, MEDIUM, VERY_HIGH, HIGH],
edges: [[0, 1], [1, 2], [2, 3]],
expected: [
constructEmptyRequest(null, '0'),
constructEmptyRequest('0', '1'),
constructEmptyRequest('1', '2'),
constructEmptyRequest('2', '3'),
]
}));

it('returns correct data for chain interleaved with non-critical requests',
() => testGetCriticalRequests({
priorityList: [MEDIUM, HIGH, LOW, MEDIUM, HIGH, VERY_LOW],
edges: [[0, 1], [1, 2], [2, 3], [3, 4]],
expected: [
constructEmptyRequest(null, '0'),
constructEmptyRequest('0', '1'),
]
}));

it('returns correct data for two parallel chains', () =>
testGetCriticalRequests({
priorityList: [HIGH, HIGH, HIGH, HIGH],
edges: [[0, 2], [1, 3]],
expected: [
constructEmptyRequest(null, '0'),
constructEmptyRequest(null, '1'),
constructEmptyRequest('0', '2'),
constructEmptyRequest('1', '3'),
]
}));

it('returns correct data for fork at root', () =>
testGetCriticalRequests({
priorityList: [HIGH, HIGH, HIGH],
edges: [[0, 1], [0, 2]],
expected: [
constructEmptyRequest(null, '0'),
constructEmptyRequest('0', '1'),
constructEmptyRequest('0', '2'),
]
}));

it('returns correct data for fork at non root', () =>
testGetCriticalRequests({
priorityList: [HIGH, HIGH, HIGH, HIGH],
edges: [[0, 1], [1, 2], [1, 3]],
expected: [
constructEmptyRequest(null, '0'),
constructEmptyRequest('0', '1'),
constructEmptyRequest('1', '2'),
constructEmptyRequest('1', '3'),
]
}));

it('returns empty list when no critical request', () =>
testGetCriticalRequests({
priorityList: [LOW, LOW],
edges: [[0, 1]],
expected: {}
}));

it('returns empty list when no request whatsoever', () =>
testGetCriticalRequests({
priorityList: [],
edges: [],
expected: {}
}));

it('returns correct data on a random big graph', () =>
testGetCriticalRequests({
priorityList: Array(9).fill(HIGH),
edges: [[0, 1], [1, 2], [1, 3], [4, 5], [5, 7], [7, 8], [5, 6]],
expected: [
constructEmptyRequest(null, '0'),
constructEmptyRequest('0', '1'),
constructEmptyRequest('1', '2'),
constructEmptyRequest('1', '3'),
constructEmptyRequest(null, '4'),
constructEmptyRequest('4', '5'),
constructEmptyRequest('5', '6'),
constructEmptyRequest('5', '7'),
constructEmptyRequest('7', '8'),
]
}));

it('handles redirects', () => {
const networkRecords = mockTracingData([HIGH, HIGH, HIGH], [[0, 1], [1, 2]]);

// Make a fake redirect
networkRecords[1].requestId = '1:redirected.0';
networkRecords[2].requestId = '1';
return Gatherer.request(networkRecords).then(criticalRequests => {
assert.deepEqual(criticalRequests, [
constructEmptyRequest(null, '0'),
constructEmptyRequest('0', '1'),
constructEmptyRequest('1', '2'),
]);
});
});

it('discards favicons as non-critical', () => {
const networkRecords = mockTracingData([HIGH, HIGH, HIGH], [[0, 1], [0, 2]]);

// 2nd record is a favicon
networkRecords[1].url = 'https://example.com/favicon.ico';
networkRecords[1].parsedURL = {
lastPathComponent: 'favicon.ico'
};
// 3rd record is also a favicon
networkRecords[2].mimeType = 'image/x-icon';
return Gatherer.request(networkRecords).then(criticalRequests => {
assert.deepEqual(criticalRequests, [
constructEmptyRequest(null, '0'),
]);
});
});
});

0 comments on commit b1b2161

Please sign in to comment.