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

feat(predictive-perf): add shell and base audit #2720

Merged
merged 3 commits into from
Sep 1, 2017
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
57 changes: 57 additions & 0 deletions lighthouse-core/audits/predictive-perf.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
/**
* @license Copyright 2017 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';

const Audit = require('./audit');
const Util = require('../report/v2/renderer/util.js');
const PageDependencyGraph = require('../gather/computed/page-dependency-graph.js');

// Parameters (in ms) for log-normal CDF scoring. To see the curve:
// https://www.desmos.com/calculator/rjp0lbit8y
const SCORING_POINT_OF_DIMINISHING_RETURNS = 1700;
const SCORING_MEDIAN = 10000;

class PredictivePerf extends Audit {
/**
* @return {!AuditMeta}
*/
static get meta() {
return {
category: 'Performance',
name: 'predictive-perf',
description: 'Predicted Performance (beta)',
Copy link
Member

Choose a reason for hiding this comment

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

sorry but you need a failureDescription and non-empty helpText. :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

heh, yeah i'll cherry-pick from the future 🤖

helpText: 'Predicted performance evaluates how your site will perform under ' +
'a 3G connection on a mobile device.',
scoringMode: Audit.SCORING_MODES.NUMERIC,
requiredArtifacts: ['traces', 'devtoolsLogs']
};
}

/**
* @param {!Artifacts} artifacts
* @return {!AuditResult}
*/
static audit(artifacts) {
const trace = artifacts.traces[Audit.DEFAULT_PASS];
const devtoolsLogs = artifacts.devtoolsLogs[Audit.DEFAULT_PASS];
return artifacts.requestPageDependencyGraph(trace, devtoolsLogs).then(graph => {
const rawValue = PageDependencyGraph.computeGraphDuration(graph);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

will eventually be the blending of multiple graph durations

const score = Audit.computeLogNormalScore(
rawValue,
SCORING_POINT_OF_DIMINISHING_RETURNS,
SCORING_MEDIAN
);

return {
score,
rawValue,
displayValue: Util.formatMilliseconds(rawValue),
};
});
}
}

module.exports = PredictivePerf;
10 changes: 10 additions & 0 deletions lighthouse-core/config/fast-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,14 @@ module.exports = {
gatherers: [],
},
],
audits: [
'predictive-perf',
],
categories: {
performance: {
audits: [
{id: 'predictive-perf', weight: 5, group: 'perf-metric'},
],
},
},
};
180 changes: 180 additions & 0 deletions lighthouse-core/gather/computed/dependency-graph/node.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,180 @@
/**
* @license Copyright 2017 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';

/**
* @fileoverview This class encapsulates logic for handling resources and tasks used to model the
* execution dependency graph of the page. A node has a unique identifier and can depend on other
* nodes/be depended on. The construction of the graph maintains some important invariants that are
* inherent to the model:
*
* 1. The graph is a DAG, there are no cycles.
* 2. There is always a root node upon which all other nodes eventually depend.
*
* This allows particular optimizations in this class so that we do no need to check for cycles as
* these methods are called and we can always start traversal at the root node.
*/
class Node {

/**
* @param {string|number} id
*/
constructor(id) {
this._id = id;
this._dependents = [];
this._dependencies = [];
}

/**
* @return {string|number}
*/
get id() {
return this._id;
}

/**
* @return {!Array<!Node>}
*/
getDependents() {
return this._dependents.slice();
}


/**
* @return {!Array<!Node>}
*/
getDependencies() {
return this._dependencies.slice();
}


/**
* @return {!Node}
*/
getRootNode() {
let rootNode = this;
while (rootNode._dependencies.length) {
rootNode = rootNode._dependencies[0];
}

return rootNode;
}

/**
* @param {!Node}
*/
addDependent(node) {
node.addDependency(this);
}

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

node._dependents.push(this);
this._dependencies.push(node);
}

/**
* Clones the node's information without adding any dependencies/dependents.
* @return {!Node}
*/
cloneWithoutRelationships() {
return new Node(this.id);
}

/**
* 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 return will be
* undefined.
* @param {function(!Node):boolean=} predicate
* @return {!Node|undefined}
*/
cloneWithRelationships(predicate) {
const rootNode = this.getRootNode();

let shouldIncludeNode = () => true;
if (predicate) {
const idsToInclude = new Set();
rootNode.traverse(node => {
if (predicate(node)) {
node.traverse(
node => idsToInclude.add(node.id),
node => node._dependencies.filter(parent => !idsToInclude.has(parent))
);
}
});

shouldIncludeNode = node => idsToInclude.has(node.id);
}

const idToNodeMap = new Map();
rootNode.traverse(originalNode => {
if (!shouldIncludeNode(originalNode)) return;
const clonedNode = originalNode.cloneWithoutRelationships();
idToNodeMap.set(clonedNode.id, clonedNode);

for (const dependency of originalNode._dependencies) {
const clonedDependency = idToNodeMap.get(dependency.id);
clonedNode.addDependency(clonedDependency);
}
});

return idToNodeMap.get(this.id);
}

/**
* Traverses all paths in the graph, calling iterator on each node visited. Decides which nodes to
* visit with the getNext function.
* @param {function(!Node,!Array<!Node>)} iterator
* @param {function(!Node):!Array<!Node>} getNext
*/
_traversePaths(iterator, getNext) {
const stack = [[this]];
while (stack.length) {
const path = stack.shift();
const node = path[0];
iterator(node, path);

const nodesToAdd = getNext(node);
for (const nextNode of nodesToAdd) {
stack.push([nextNode].concat(path));
}
}
}

/**
* Traverses all connected nodes exactly once, calling iterator on each. Decides which nodes to
* visit with the getNext function.
* @param {function(!Node,!Array<!Node>)} iterator
* @param {function(!Node):!Array<!Node>=} getNext Defaults to returning the dependents.
*/
traverse(iterator, getNext) {
if (!getNext) {
getNext = node => node.getDependents();
}

const visited = new Set();
const originalGetNext = getNext;

getNext = node => {
visited.add(node.id);
const allNodesToVisit = originalGetNext(node);
const nodesToVisit = allNodesToVisit.filter(nextNode => !visited.has(nextNode.id));
nodesToVisit.forEach(nextNode => visited.add(nextNode.id));
return nodesToVisit;
};

this._traversePaths(iterator, getNext);
}
}

module.exports = Node;
122 changes: 122 additions & 0 deletions lighthouse-core/gather/computed/page-dependency-graph.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
/**
* @license Copyright 2017 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';

const ComputedArtifact = require('./computed-artifact');
const Node = require('./dependency-graph/node');
const Emulation = require('../../lib/emulation');

class PageDependencyGraphArtifact extends ComputedArtifact {
get name() {
return 'PageDependencyGraph';
}

get requiredNumberOfArtifacts() {
return 2;
}

/**
* @param {!WebInspector.NetworkRequest} record
* @return {!Array<string>}
*/
static getNetworkInitiators(record) {
Copy link
Member

Choose a reason for hiding this comment

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

check out https://cs.chromium.org/chromium/src/tools/android/loading/request_dependencies_lens.py?type=cs&q=f:request_dependencies_lens+_GetDependency

The logic for redirect and parser requests seem worthwhile.

The script case handles a lot of edge cases that might be worth using.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh this is great!! wdyt about making these changes on top of #3163 to reflect the incremental improvements?

Copy link
Member

Choose a reason for hiding this comment

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

SGTM

Copy link
Member

Choose a reason for hiding this comment

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

sgtm.

if (!record._initiator) return [];
if (record._initiator.url) return [record._initiator.url];
if (record._initiator.type === 'script') {
const frames = record._initiator.stack.callFrames;
return Array.from(new Set(frames.map(frame => frame.url))).filter(Boolean);
}

return [];
}

/**
* @param {!TraceOfTabArtifact} traceOfTab
* @param {!Array<!WebInspector.NetworkRequest>} networkRecords
* @return {!Node}
*/
static createGraph(traceOfTab, networkRecords) {
const idToNodeMap = new Map();
const urlToNodeMap = new Map();

networkRecords.forEach(record => {
const node = new Node(record.requestId);
idToNodeMap.set(record.requestId, node);

if (urlToNodeMap.has(record.url)) {
// If duplicate requests have been made to this URL we can't be certain which node is being
// referenced, so act like we don't know the URL at all.
urlToNodeMap.set(record.url, undefined);
} else {
urlToNodeMap.set(record.url, node);
}
});

const rootRequest = networkRecords
.reduce((min, next) => min.startTime < next.startTime ? min : next);
const rootNode = idToNodeMap.get(rootRequest.requestId);
networkRecords.forEach(record => {
const initiators = PageDependencyGraphArtifact.getNetworkInitiators(record);
const node = idToNodeMap.get(record.requestId);
if (initiators.length) {
initiators.forEach(initiator => {
const parent = urlToNodeMap.get(initiator) || rootNode;
parent.addDependent(node);
});
} else if (record !== rootRequest) {
rootNode.addDependent(node);
}
});

return rootNode;
}

/**
* @param {!Node} rootNode
* @return {number}
*/
static computeGraphDuration(rootNode) {
const depthByNodeId = new Map();
const getMax = arr => Array.from(arr).reduce((max, next) => Math.max(max, next), 0);

let startingMax = Infinity;
let endingMax = Infinity;
while (endingMax === Infinity || startingMax > endingMax) {
startingMax = endingMax;
endingMax = 0;

rootNode.traverse(node => {
const dependencies = node.getDependencies();
const dependencyDepths = dependencies.map(node => depthByNodeId.get(node.id) || Infinity);
const maxDepth = getMax(dependencyDepths);
endingMax = Math.max(endingMax, maxDepth);
depthByNodeId.set(node.id, maxDepth + 1);
});
}

const maxDepth = getMax(depthByNodeId.values());
return maxDepth * Emulation.settings.TYPICAL_MOBILE_THROTTLING_METRICS.latency;
Copy link
Member

Choose a reason for hiding this comment

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

Structure-wise I feel like it'd be a tad better to have this be a generic graph computeMaxDepth method.

Optionally, I could see this latency work going into the audit rather than here in the graph computed artifact.

You are now telling me verbally that this all disappears in the next commit... so.... 💨 💨 💨 💨

}

/**
* @param {!Trace} trace
* @param {!DevtoolsLog} devtoolsLog
* @param {!ComputedArtifacts} artifacts
* @return {!Promise<!Node>}
*/
compute_(trace, devtoolsLog, artifacts) {
const promises = [
artifacts.requestTraceOfTab(trace),
artifacts.requestNetworkRecords(devtoolsLog),
];

return Promise.all(promises).then(([traceOfTab, networkRecords]) => {
return PageDependencyGraphArtifact.createGraph(traceOfTab, networkRecords);
});
}
}

module.exports = PageDependencyGraphArtifact;
8 changes: 6 additions & 2 deletions lighthouse-core/runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -256,9 +256,13 @@ class Runner {
*/
static instantiateComputedArtifacts() {
const computedArtifacts = {};
const filenamesToSkip = [
'computed-artifact.js', // the base class which other artifacts inherit
'dependency-graph', // a folder containing dependencies, not an artifact
];

require('fs').readdirSync(__dirname + '/gather/computed').forEach(function(filename) {
// Skip base class.
if (filename === 'computed-artifact.js') return;
if (filenamesToSkip.includes(filename)) return;

// Drop `.js` suffix to keep browserify import happy.
filename = filename.replace(/\.js$/, '');
Expand Down
Loading