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.

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

core(preload): use lantern to compute savings #5062

Merged
merged 9 commits into from
May 3, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
133 changes: 109 additions & 24 deletions lighthouse-core/audits/uses-rel-preload.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
* 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.
*/
// @ts-nocheck
'use strict';

const Audit = require('./audit');
Expand All @@ -27,8 +26,19 @@ class UsesRelPreloadAudit extends Audit {
};
}

/**
* @param {LH.Artifacts.CriticalRequestNode} chains
* @param {number} maxLevel
* @param {number=} minLevel
*/
static _flattenRequests(chains, maxLevel, minLevel = 0) {
/** @type {Array<LH.WebInspector.NetworkRequest>} */
const requests = [];

/**
* @param {LH.Artifacts.CriticalRequestNode} chains
* @param {number} level
*/
const flatten = (chains, level) => {
Object.keys(chains).forEach(chain => {
if (chains[chain]) {
Expand All @@ -49,56 +59,131 @@ class UsesRelPreloadAudit extends Audit {
return requests;
}

/**
* Computes the estimated effect of preloading all the resources.
* @param {Set<string>} urls The array of byte savings results per resource
* @param {LH.Gatherer.Simulation.GraphNode} graph
* @param {LH.Gatherer.Simulation.Simulator} simulator
* @param {LH.WebInspector.NetworkRequest} mainResource
* @return {{wastedMs: number, results: Array<{url: string, wastedMs: number}>}}
*/
static computeWasteWithGraph(urls, graph, simulator, mainResource) {
if (!urls.size) {
return {wastedMs: 0, results: []};
}

// Preload changes the ordering of requests, simulate the original graph with flexible ordering
// to have a reasonable baseline for comparison.
const simulationBeforeChanges = simulator.simulate(graph, {flexibleOrdering: true});

const modifiedGraph = graph.cloneWithRelationships();

/** @type {Array<LH.Gatherer.Simulation.GraphNetworkNode>} */
const nodesToPreload = [];
/** @type {LH.Gatherer.Simulation.GraphNode|null} */
let mainDocumentNode = null;
modifiedGraph.traverse(node => {
if (node.type !== 'network') return;

const networkNode = /** @type {LH.Gatherer.Simulation.GraphNetworkNode} */ (node);
if (networkNode.record && urls.has(networkNode.record.url)) {
nodesToPreload.push(networkNode);
}

if (networkNode.record && networkNode.record.url === mainResource.url) {
mainDocumentNode = networkNode;
}
});

if (!mainDocumentNode) {
// Should always find the main document node
throw new Error('Could not find main document node');
}

// Preload has the effect of moving the resource's only dependency to the main HTML document
// Remove all dependencies of the nodes
for (const node of nodesToPreload) {
node.removeAllDependencies();
node.addDependency(mainDocumentNode);
}

// Once we've modified the dependencies, simulate the new graph with flexible ordering.
const simulationAfterChanges = simulator.simulate(modifiedGraph, {flexibleOrdering: true});
const originalNodesByRecord = Array.from(simulationBeforeChanges.nodeTimings.keys())
// @ts-ignore we don't care if all nodes without a record collect on `undefined`
.reduce((map, node) => map.set(node.record, node), new Map());

const results = [];
for (const node of nodesToPreload) {
const originalNode = originalNodesByRecord.get(node.record);
const timingAfter = simulationAfterChanges.nodeTimings.get(node);
const timingBefore = simulationBeforeChanges.nodeTimings.get(originalNode);
// @ts-ignore TODO(phulce): fix timing typedef
const wastedMs = Math.round(timingBefore.endTime - timingAfter.endTime);
if (wastedMs < THRESHOLD_IN_MS) continue;
results.push({url: node.record.url, wastedMs});
}

if (!results.length) {
return {wastedMs: 0, results};
}

return {
// Preload won't necessarily impact the deepest chain/overall time
// We'll use the maximum endTime improvement for now
wastedMs: Math.max(...results.map(item => item.wastedMs)),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems like it gets most of the way to an opportunity but then stops here? Is measuring impact on TTI or whatever problematic?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah fair, I was mostly retaining parity with the old way, but we can do max(TTI, load) like the others 👍

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

having had the hour to think about it, I've reconvinced myself that this is a bad idea and we should keep it to max(...wastedMs), our selection of preloaded items is fairly narrow and in a busy page it will rarely impact the longest chain

given our super ambiguous no-longer-just-on-a-particular-metric approach, it seems OK for now

results,
};
}

/**
* @param {LH.Artifacts} artifacts
* @param {LH.Audit.Context} context
* @return {Promise<LH.Audit.Product>}
*/
static audit(artifacts) {
static audit(artifacts, context) {
const trace = artifacts.traces[UsesRelPreloadAudit.DEFAULT_PASS];
const devtoolsLog = artifacts.devtoolsLogs[UsesRelPreloadAudit.DEFAULT_PASS];
const URL = artifacts.URL;
const simulatorOptions = {trace, devtoolsLog, settings: context.settings};

return Promise.all([
// TODO(phulce): eliminate dependency on CRC
artifacts.requestCriticalRequestChains({devtoolsLog, URL}),
artifacts.requestMainResource({devtoolsLog, URL}),
]).then(([critChains, mainResource]) => {
const results = [];
let maxWasted = 0;
artifacts.requestPageDependencyGraph({trace, devtoolsLog}),
artifacts.requestLoadSimulator(simulatorOptions),
]).then(([critChains, mainResource, graph, simulator]) => {
// 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);
criticalRequests.forEach(request => {
const networkRecord = request;

/** @type {Set<string>} */
const urls = new Set();
for (const networkRecord of criticalRequests) {
if (!networkRecord._isLinkPreload && networkRecord.protocol !== 'data') {
// calculate time between mainresource.endTime and resource start time
const wastedMs = Math.min(request._startTime - mainResource._endTime,
request._endTime - request._startTime) * 1000;

if (wastedMs >= THRESHOLD_IN_MS) {
maxWasted = Math.max(wastedMs, maxWasted);
results.push({
url: request.url,
wastedMs: Util.formatMilliseconds(wastedMs),
});
}
urls.add(networkRecord._url);
}
});
}

const {results, wastedMs} = UsesRelPreloadAudit.computeWasteWithGraph(urls, graph, simulator,
mainResource);
// sort results by wastedTime DESC
results.sort((a, b) => b.wastedMs - a.wastedMs);

const headings = [
{key: 'url', itemType: 'url', text: 'URL'},
{key: 'wastedMs', itemType: 'text', text: 'Potential Savings'},
{key: 'wastedMs', itemType: 'ms', text: 'Potential Savings', granularity: 10},
];
const summary = {wastedMs: maxWasted};
const summary = {wastedMs};
const details = Audit.makeTableDetails(headings, results, summary);

return {
score: UnusedBytes.scoreForWastedMs(maxWasted),
rawValue: maxWasted,
displayValue: Util.formatMilliseconds(maxWasted),
score: UnusedBytes.scoreForWastedMs(wastedMs),
rawValue: wastedMs,
displayValue: Util.formatMilliseconds(wastedMs),
extendedInfo: {
value: results,
},
Expand Down
25 changes: 25 additions & 0 deletions lighthouse-core/lib/dependency-graph/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,31 @@ class Node {
this._dependencies.push(node);
}

/**
* @param {Node} node
*/
removeDependent(node) {
node.removeDependency(this);
}

/**
* @param {Node} node
*/
removeDependency(node) {
if (!this._dependencies.includes(node)) {
return;
}

node._dependents.splice(node._dependents.indexOf(this), 1);
this._dependencies.splice(this._dependencies.indexOf(node), 1);
}

removeAllDependencies() {
for (const node of this._dependencies.slice()) {
this.removeDependency(node);
}
}

/**
* Clones the node's information without adding any dependencies/dependents.
* @return {Node}
Expand Down
45 changes: 35 additions & 10 deletions lighthouse-core/lib/dependency-graph/simulator/connection-pool.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ const TcpConnection = require('./tcp-connection');
const DEFAULT_SERVER_RESPONSE_TIME = 30;
const TLS_SCHEMES = ['https', 'wss'];

// Each origin can have 6 simulatenous connections open
// https://cs.chromium.org/chromium/src/net/socket/client_socket_pool_manager.cc?type=cs&q="int+g_max_sockets_per_group"
const CONNECTIONS_PER_ORIGIN = 6;

module.exports = class ConnectionPool {
/**
* @param {LH.WebInspector.NetworkRequest[]} records
Expand Down Expand Up @@ -82,15 +86,26 @@ module.exports = class ConnectionPool {
throw new Error(`Could not find a connection for origin: ${origin}`);
}

// Make sure each origin has minimum number of connections available for max throughput
while (connections.length < CONNECTIONS_PER_ORIGIN) connections.push(connections[0].clone());

this._connectionsByOrigin.set(origin, connections);
}
}

/**
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we add a few lines to describe what's happening in here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

* This method finds an available connection to the origin specified by the network record or null
* if no connection was available. If returned, connection will not be available for other network
* records until release is called.
*
* If ignoreConnectionReused is true, acquire will consider all connections not in use as available.
* Otherwise, only connections that have matching "warmth" are considered available.
*
* @param {LH.WebInspector.NetworkRequest} record
* @param {{ignoreConnectionReused?: boolean}} options
* @return {?TcpConnection}
*/
acquire(record) {
acquire(record, options = {}) {
if (this._connectionsByRecord.has(record)) {
// @ts-ignore
return this._connectionsByRecord.get(record);
Expand All @@ -99,16 +114,26 @@ module.exports = class ConnectionPool {
const origin = String(record.parsedURL.securityOrigin());
/** @type {TcpConnection[]} */
const connections = this._connectionsByOrigin.get(origin) || [];
const wasConnectionWarm = !!this._connectionReusedByRequestId.get(record.requestId);
const connection = connections.find(connection => {
const meetsWarmRequirement = wasConnectionWarm === connection.isWarm();
return meetsWarmRequirement && !this._connectionsInUse.has(connection);
});
// Sort connections by decreasing congestion window, i.e. warmest to coldest
const availableConnections = connections
.filter(connection => !this._connectionsInUse.has(connection))
.sort((a, b) => b.congestionWindow - a.congestionWindow);

const observedConnectionWasReused = !!this._connectionReusedByRequestId.get(record.requestId);

/** @type {TcpConnection|undefined} */
let connectionToUse = availableConnections[0];
if (!options.ignoreConnectionReused) {
connectionToUse = availableConnections.find(
connection => connection.isWarm() === observedConnectionWasReused
);
}

if (!connectionToUse) return null;

if (!connection) return null;
this._connectionsInUse.add(connection);
this._connectionsByRecord.set(record, connection);
return connection;
this._connectionsInUse.add(connectionToUse);
this._connectionsByRecord.set(record, connectionToUse);
return connectionToUse;
}

/**
Expand Down
37 changes: 32 additions & 5 deletions lighthouse-core/lib/dependency-graph/simulator/simulator.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ class Simulator {
this._layoutTaskMultiplier = this._cpuSlowdownMultiplier * this._options.layoutTaskMultiplier;

// Properties reset on every `.simulate` call but duplicated here for type checking
this._flexibleOrdering = false;
this._nodeTimings = new Map();
this._numberInProgressByType = new Map();
this._nodes = {};
Expand Down Expand Up @@ -150,6 +151,16 @@ class Simulator {
}
}

/**
* @param {LH.WebInspector.NetworkRequest} record
* @return {?TcpConnection}
*/
_acquireConnection(record) {
return this._connectionPool.acquire(record, {
ignoreConnectionReused: this._flexibleOrdering,
});
}

/**
* @param {Node} node
* @param {number} totalElapsedTime
Expand All @@ -170,7 +181,7 @@ class Simulator {
// Start a network request if we're not at max requests and a connection is available
const numberOfActiveRequests = this._numberInProgress(node.type);
if (numberOfActiveRequests >= this._maximumConcurrentRequests) return;
const connection = this._connectionPool.acquire(/** @type {NetworkNode} */ (node).record);
const connection = this._acquireConnection(/** @type {NetworkNode} */ (node).record);
if (!connection) return;

this._markNodeAsInProgress(node, totalElapsedTime);
Expand Down Expand Up @@ -215,7 +226,8 @@ class Simulator {

const record = /** @type {NetworkNode} */ (node).record;
const timingData = this._nodeTimings.get(node);
const connection = /** @type {TcpConnection} */ (this._connectionPool.acquire(record));
// If we're estimating time remaining, we already acquired a connection for this record, definitely non-null
const connection = /** @type {TcpConnection} */ (this._acquireConnection(record));
const calculation = connection.simulateDownloadUntil(
record.transferSize - timingData.bytesDownloaded,
{timeAlreadyElapsed: timingData.timeElapsed, maximumTimeToElapse: Infinity}
Expand Down Expand Up @@ -258,7 +270,8 @@ class Simulator {
if (node.type !== Node.TYPES.NETWORK) throw new Error('Unsupported');

const record = /** @type {NetworkNode} */ (node).record;
const connection = /** @type {TcpConnection} */ (this._connectionPool.acquire(record));
// If we're updating the progress, we already acquired a connection for this record, definitely non-null
const connection = /** @type {TcpConnection} */ (this._acquireConnection(record));
const calculation = connection.simulateDownloadUntil(
record.transferSize - timingData.bytesDownloaded,
{
Expand Down Expand Up @@ -289,12 +302,22 @@ class Simulator {
}

/**
* Estimates the time taken to process all of the graph's nodes.
* Estimates the time taken to process all of the graph's nodes, returns the overall time along with
* each node annotated by start/end times.
*
* If flexibleOrdering is set, simulator/connection pool are allowed to deviate from what was
* observed in the trace/devtoolsLog and start requests as soon as they are queued (i.e. do not
* wait around for a warm connection to be available if the original record was fetched on a warm
* connection).
*
* @param {Node} graph
* @param {{flexibleOrdering?: boolean}=} options
* @return {LH.Gatherer.Simulation.Result}
*/
simulate(graph) {
simulate(graph, options) {
options = Object.assign({flexibleOrdering: false}, options);
// initialize the necessary data containers
this._flexibleOrdering = options.flexibleOrdering;
this._initializeConnectionPool(graph);
this._initializeAuxiliaryData();

Expand All @@ -318,6 +341,10 @@ class Simulator {
this._startNodeIfPossible(node, totalElapsedTime);
}

if (!nodesInProgress.size) {
throw new Error('Failed to start a node, potential mismatch in original execution');
}

// set the available throughput for all connections based on # inflight
this._updateNetworkCapacity();

Expand Down
Loading