Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

core(preload): remove CRC dependency #5901

Merged
merged 2 commits into from
Aug 24, 2018
Merged

core(preload): remove CRC dependency #5901

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3273e12
core(preload): remove CRC dependency
patrickhulce Aug 24, 2018
596d8fe
remove flatten too
patrickhulce Aug 24, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 35 additions & 49 deletions lighthouse-core/audits/uses-rel-preload.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
const URL = require('../lib/url-shim');
const Audit = require('./audit');
const UnusedBytes = require('./byte-efficiency/byte-efficiency-audit');
const CriticalRequestChains = require('../gather/computed/critical-request-chains');
const i18n = require('../lib/i18n');

const UIStrings = {
Expand Down Expand Up @@ -37,49 +38,49 @@ class UsesRelPreloadAudit extends Audit {
}

/**
* @param {LH.Artifacts.CriticalRequestNode} chains
* @param {number} maxLevel
* @param {number=} minLevel
* @param {LH.Artifacts.NetworkRequest} mainResource
* @param {LH.Gatherer.Simulation.GraphNode} graph
* @return {Set<string>}
*/
static _flattenRequests(chains, maxLevel, minLevel = 0) {
/** @type {Array<LH.Artifacts.NetworkRequest>} */
const requests = [];

/**
* @param {LH.Artifacts.CriticalRequestNode} chains
* @param {number} level
*/
const flatten = (chains, level) => {
Object.keys(chains).forEach(chain => {
if (chains[chain]) {
const currentChain = chains[chain];
if (level >= minLevel) {
requests.push(currentChain.request);
}

if (level < maxLevel) {
flatten(currentChain.children, level + 1);
}
}
});
};
static getURLsToPreload(mainResource, graph) {
/** @type {Set<string>} */
const urls = new Set();

flatten(chains, 0);
graph.traverse((node, traversalPath) => {
if (node.type !== 'network') return;
// Don't include the node itself or any CPU nodes in the initiatorPath
const path = traversalPath.slice(1).filter(initiator => initiator.type === 'network');
if (!UsesRelPreloadAudit.shouldPreloadRequest(node.record, mainResource, path)) return;
urls.add(node.record.url);
});

return requests;
return urls;
}

/**
* We want to preload all first party critical requests at depth 2.
* Third party requests can be tricky to know the URL ahead of time.
* Critical requests at depth 1 would already be identified by the browser for preloading.
* Critical requests deeper than depth 2 are more likely to be a case-by-case basis such that it
* would be a little risky to recommend blindly.
*
* @param {LH.Artifacts.NetworkRequest} request
* @param {LH.Artifacts.NetworkRequest} mainResource
* @param {Array<LH.Gatherer.Simulation.GraphNode>} initiatorPath
* @return {boolean}
*/
static shouldPreload(request, mainResource) {
if (request.isLinkPreload || URL.NON_NETWORK_PROTOCOLS.includes(request.protocol)) {
return false;
}

static shouldPreloadRequest(request, mainResource, initiatorPath) {
const mainResourceDepth = mainResource.redirects ? mainResource.redirects.length : 0;

// If it's already preloaded, no need to recommend it.
if (request.isLinkPreload) return false;
// It's not critical, don't recommend it.
if (!CriticalRequestChains.isCritical(request, mainResource)) return false;
// It's not a request loaded over the network, don't recommend it.
if (URL.NON_NETWORK_PROTOCOLS.includes(request.protocol)) return false;
// It's not at the right depth, don't recommend it.
if (initiatorPath.length !== mainResourceDepth + 2) return false;
// We survived everything else, just check that it's a first party request.
return URL.rootDomainsMatch(request.url, mainResource.url);
}

Expand Down Expand Up @@ -168,28 +169,13 @@ class UsesRelPreloadAudit extends Audit {
const URL = artifacts.URL;
const simulatorOptions = {trace, devtoolsLog, settings: context.settings};

const [critChains, mainResource, graph, simulator] = await Promise.all([
// TODO(phulce): eliminate dependency on CRC
artifacts.requestCriticalRequestChains({devtoolsLog, URL}),
const [mainResource, graph, simulator] = await Promise.all([
artifacts.requestMainResource({devtoolsLog, URL}),
artifacts.requestPageDependencyGraph({trace, devtoolsLog}),
artifacts.requestLoadSimulator(simulatorOptions),
]);

// get all critical requests 2 + mainResourceIndex levels deep
const mainResourceIndex = mainResource.redirects ? mainResource.redirects.length : 0;

const criticalRequests = UsesRelPreloadAudit._flattenRequests(critChains,
3 + mainResourceIndex, 2 + mainResourceIndex);

/** @type {Set<string>} */
const urls = new Set();
for (const networkRecord of criticalRequests) {
if (UsesRelPreloadAudit.shouldPreload(networkRecord, mainResource)) {
urls.add(networkRecord.url);
}
}

const urls = UsesRelPreloadAudit.getURLsToPreload(mainResource, graph);
const {results, wastedMs} = UsesRelPreloadAudit.computeWasteWithGraph(urls, graph, simulator);
// sort results by wastedTime DESC
results.sort((a, b) => b.wastedMs - a.wastedMs);
Expand Down
186 changes: 63 additions & 123 deletions lighthouse-core/test/audits/uses-rel-preload-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,10 @@ describe('Performance: uses-rel-preload audit', () => {
let mockGraph;
let mockSimulator;

const mockArtifacts = (networkRecords, mockChain, mainResource = defaultMainResource) => {
const mockArtifacts = (networkRecords, mainResource = defaultMainResource) => {
return {
traces: {[UsesRelPreload.DEFAULT_PASS]: {traceEvents: []}},
devtoolsLogs: {[UsesRelPreload.DEFAULT_PASS]: []},
requestCriticalRequestChains: () => {
return Promise.resolve(mockChain);
},
requestLoadSimulator: () => mockSimulator,
requestPageDependencyGraph: () => mockGraph,
requestNetworkRecords: () => networkRecords,
Expand All @@ -40,21 +37,69 @@ describe('Performance: uses-rel-preload audit', () => {
};
};

function buildNode(requestId, url) {
return new NetworkNode({url, requestId});
}

afterEach(() => {
mockSimulator = undefined;
});

it('should suggest preload resource', () => {
const rootNode = buildNode(1, 'http://example.com');
const mainDocumentNode = buildNode(2, 'http://www.example.com:3000');
const scriptNode = buildNode(3, 'http://www.example.com/script.js');
const scriptAddedNode = buildNode(4, 'http://www.example.com/script-added.js');
const scriptSubNode = buildNode(5, 'http://sub.example.com/script-sub.js');
const scriptOtherNode = buildNode(6, 'http://otherdomain.com/script-other.js');
const mainResource = Object.assign({}, defaultMainResource, {
url: 'http://www.example.com:3000',
redirects: [''],
});

const networkRecords = [
{
requestId: '2',
resourceType: 'Document',
priority: 'High',
isLinkPreload: false,
url: 'http://example.com:3000',
redirects: [''],
},
{
requestId: '2:redirect',
resourceType: 'Document',
priority: 'High',
isLinkPreload: false,
url: 'http://www.example.com:3000',
redirects: [''],
},
{
requestId: '3',
resourceType: 'Script',
priority: 'High',
isLinkPreload: false,
url: 'http://www.example.com/script.js',
},
{
requestId: '4',
resourceType: 'Script',
priority: 'High',
isLinkPreload: false,
url: 'http://www.example.com/script-added.js',
},
{
requestId: '5',
resourceType: 'Script',
priority: 'High',
isLinkPreload: false,
url: 'http://sub.example.com/script-sub.js',
},
{
requestId: '6',
resourceType: 'Script',
priority: 'High',
isLinkPreload: false,
url: 'http://otherdomain.com/script-other.js',
},
];

const rootNode = new NetworkNode(networkRecords[0]);
const mainDocumentNode = new NetworkNode(networkRecords[1]);
const scriptNode = new NetworkNode(networkRecords[2]);
const scriptAddedNode = new NetworkNode(networkRecords[3]);
const scriptSubNode = new NetworkNode(networkRecords[4]);
const scriptOtherNode = new NetworkNode(networkRecords[5]);

mainDocumentNode.setIsMainDocument(true);
mainDocumentNode.addDependency(rootNode);
Expand Down Expand Up @@ -101,68 +146,7 @@ describe('Performance: uses-rel-preload audit', () => {
},
};

const mainResource = Object.assign({}, defaultMainResource, {
url: 'http://www.example.com:3000',
redirects: [''],
});
const networkRecords = [
{
requestId: '2',
isLinkPreload: false,
url: 'http://www.example.com:3000',
},
{
requestId: '3',
isLinkPreload: false,
url: 'http://www.example.com/script.js',
},
{
requestId: '4',
isLinkPreload: false,
url: 'http://www.example.com/script-added.js',
},
{
requestId: '5',
isLinkPreload: false,
url: 'http://sub.example.com/script-sub.js',
},
{
requestId: '6',
isLinkPreload: false,
url: 'http://otherdomain.com/script-other.js',
},
];

const chains = {
'1': {
children: {
'2': {
request: networkRecords[0],
children: {
'3': {
request: networkRecords[1],
children: {
'4': {
request: networkRecords[2],
children: {},
},
'5': {
request: networkRecords[3],
children: {},
},
'6': {
request: networkRecords[4],
children: {},
},
},
},
},
},
},
},
};

return UsesRelPreload.audit(mockArtifacts(networkRecords, chains, mainResource), {}).then(
return UsesRelPreload.audit(mockArtifacts(networkRecords, mainResource), {}).then(
output => {
assert.equal(output.rawValue, 1250);
assert.equal(output.details.items.length, 2);
Expand All @@ -181,22 +165,8 @@ describe('Performance: uses-rel-preload audit', () => {
url: 'http://www.example.com/script.js',
},
];
const chains = {
'1': {
children: {
'2': {
children: {
'3': {
request: networkRecords[0],
children: {},
},
},
},
},
},
};

return UsesRelPreload.audit(mockArtifacts(networkRecords, chains), {}).then(output => {
return UsesRelPreload.audit(mockArtifacts(networkRecords), {}).then(output => {
assert.equal(output.rawValue, 0);
assert.equal(output.details.items.length, 0);
});
Expand All @@ -211,22 +181,7 @@ describe('Performance: uses-rel-preload audit', () => {
},
];

const chains = {
'1': {
children: {
'2': {
children: {
'3': {
request: networkRecords[0],
children: {},
},
},
},
},
},
};

return UsesRelPreload.audit(mockArtifacts(networkRecords, chains), {}).then(output => {
return UsesRelPreload.audit(mockArtifacts(networkRecords), {}).then(output => {
assert.equal(output.rawValue, 0);
assert.equal(output.details.items.length, 0);
});
Expand All @@ -241,22 +196,7 @@ describe('Performance: uses-rel-preload audit', () => {
},
];

const chains = {
'1': {
children: {
'2': {
children: {
'3': {
request: networkRecords[0],
children: {},
},
},
},
},
},
};

return UsesRelPreload.audit(mockArtifacts(networkRecords, chains), {}).then(output => {
return UsesRelPreload.audit(mockArtifacts(networkRecords), {}).then(output => {
assert.equal(output.rawValue, 0);
assert.equal(output.details.items.length, 0);
});
Expand Down