From b1fde39082047cc39a7c380659dcaa15b03a5040 Mon Sep 17 00:00:00 2001 From: Patrick Hulce Date: Mon, 28 Aug 2017 09:52:39 -0700 Subject: [PATCH] fix(predictive-perf): split CPU and Layout task multipliers --- .../dependency-graph/estimator/estimator.js | 24 +++++++++++++++---- .../gather/computed/page-dependency-graph.js | 6 ++++- .../test/audits/predictive-perf-test.js | 12 +++++----- .../estimator/estimator-test.js | 15 +++++++++--- 4 files changed, 43 insertions(+), 14 deletions(-) diff --git a/lighthouse-core/gather/computed/dependency-graph/estimator/estimator.js b/lighthouse-core/gather/computed/dependency-graph/estimator/estimator.js index d958557f24d1..f6816a2ce37f 100644 --- a/lighthouse-core/gather/computed/dependency-graph/estimator/estimator.js +++ b/lighthouse-core/gather/computed/dependency-graph/estimator/estimator.js @@ -10,10 +10,18 @@ const TcpConnection = require('./tcp-connection'); // see https://cs.chromium.org/search/?q=kDefaultMaxNumDelayableRequestsPerClient&sq=package:chromium&type=cs const DEFAULT_MAXIMUM_CONCURRENT_REQUESTS = 10; + +// Fast 3G emulation target from DevTools, WPT 3G - Fast setting const DEFAULT_RESPONSE_TIME = 30; const DEFAULT_RTT = 150; const DEFAULT_THROUGHPUT = 1600 * 1024; // 1.6 Mbps -const DEFAULT_CPU_MULTIPLIER = 5; + +// same multiplier as Lighthouse uses for CPU emulation +const DEFAULT_CPU_TASK_MULTIPLIER = 4; +// layout tasks tend to be less CPU-bound and do not experience the same increase in duration +const DEFAULT_LAYOUT_TASK_MULTIPLIER = 2; +// if a task takes more than 10 seconds it's usually a sign it isn't actually CPU bound and we're over estimating +const DEFAULT_MAXIMUM_CPU_TASK_DURATION = 10000; function groupBy(items, keyFunc) { const grouped = new Map(); @@ -41,7 +49,8 @@ class Estimator { throughput: DEFAULT_THROUGHPUT, defaultResponseTime: DEFAULT_RESPONSE_TIME, maximumConcurrentRequests: DEFAULT_MAXIMUM_CONCURRENT_REQUESTS, - cpuMultiplier: DEFAULT_CPU_MULTIPLIER, + cpuTaskMultiplier: DEFAULT_CPU_TASK_MULTIPLIER, + layoutTaskMultiplier: DEFAULT_LAYOUT_TASK_MULTIPLIER, }, options ); @@ -53,7 +62,8 @@ class Estimator { TcpConnection.maximumSaturatedConnections(this._rtt, this._throughput), this._options.maximumConcurrentRequests ); - this._cpuMultiplier = this._options.cpuMultiplier; + this._cpuTaskMultiplier = this._options.cpuTaskMultiplier; + this._layoutTaskMultiplier = this._options.layoutTaskMultiplier; } /** @@ -227,7 +237,13 @@ class Estimator { _estimateTimeRemaining(node) { if (node.type === Node.TYPES.CPU) { const auxData = this._nodeAuxiliaryData.get(node); - const totalDuration = Math.round(node.event.dur / 1000 * this._cpuMultiplier); + const multiplier = node.didPerformLayout() + ? this._layoutTaskMultiplier + : this._cpuTaskMultiplier; + const totalDuration = Math.min( + Math.round(node.event.dur / 1000 * multiplier), + DEFAULT_MAXIMUM_CPU_TASK_DURATION + ); const estimatedTimeElapsed = totalDuration - auxData.timeElapsed; this._setAuxData(node, {estimatedTimeElapsed}); return estimatedTimeElapsed; diff --git a/lighthouse-core/gather/computed/page-dependency-graph.js b/lighthouse-core/gather/computed/page-dependency-graph.js index ddd153f4dfa7..5d2cee0a1a3c 100644 --- a/lighthouse-core/gather/computed/page-dependency-graph.js +++ b/lighthouse-core/gather/computed/page-dependency-graph.js @@ -11,7 +11,10 @@ const CPUNode = require('./dependency-graph/cpu-node'); const GraphEstimator = require('./dependency-graph/estimator/estimator'); const TracingProcessor = require('../../lib/traces/tracing-processor'); -const MINIMUM_TASK_DURATION = 10 * 1000; +// tasks shorter than 10 ms are unlikely have a significant impact +const MINIMUM_TASK_DURATION = 10000; +// video files tend to be enormous and throw off all graph traversals +const IGNORED_MIME_TYPES_REGEX = /^video/; class PageDependencyGraphArtifact extends ComputedArtifact { get name() { @@ -47,6 +50,7 @@ class PageDependencyGraphArtifact extends ComputedArtifact { const urlToNodeMap = new Map(); networkRecords.forEach(record => { + if (IGNORED_MIME_TYPES_REGEX.test(record.mimeType)) return; const node = new NetworkNode(record); nodes.push(node); idToNodeMap.set(record.requestId, node); diff --git a/lighthouse-core/test/audits/predictive-perf-test.js b/lighthouse-core/test/audits/predictive-perf-test.js index 24b5e9205a31..802ba92cd613 100644 --- a/lighthouse-core/test/audits/predictive-perf-test.js +++ b/lighthouse-core/test/audits/predictive-perf-test.js @@ -25,15 +25,15 @@ describe('Performance: predictive performance audit', () => { }, Runner.instantiateComputedArtifacts()); return PredictivePerf.audit(artifacts).then(output => { - assert.equal(output.score, 96); - assert.equal(Math.round(output.rawValue), 2453); - assert.equal(output.displayValue, '2,450\xa0ms'); + assert.equal(output.score, 99); + assert.equal(Math.round(output.rawValue), 1696); + assert.equal(output.displayValue, '1,700\xa0ms'); const valueOf = name => Math.round(output.extendedInfo.value[name]); assert.equal(valueOf('optimisticFMP'), 754); - assert.equal(valueOf('pessimisticFMP'), 2070); - assert.equal(valueOf('optimisticTTCI'), 3340); - assert.equal(valueOf('pessimisticTTCI'), 3649); + assert.equal(valueOf('pessimisticFMP'), 1191); + assert.equal(valueOf('optimisticTTCI'), 2438); + assert.equal(valueOf('pessimisticTTCI'), 2399); }); }); }); diff --git a/lighthouse-core/test/gather/computed/dependency-graph/estimator/estimator-test.js b/lighthouse-core/test/gather/computed/dependency-graph/estimator/estimator-test.js index bab442d7403b..ed8869d7ac31 100644 --- a/lighthouse-core/test/gather/computed/dependency-graph/estimator/estimator-test.js +++ b/lighthouse-core/test/gather/computed/dependency-graph/estimator/estimator-test.js @@ -51,7 +51,10 @@ describe('DependencyGraph/Estimator', () => { const cpuNode = new CpuNode(cpuTask({duration: 200})); cpuNode.addDependency(rootNode); - const estimator = new Estimator(rootNode, {defaultResponseTime: 500}); + const estimator = new Estimator(rootNode, { + defaultResponseTime: 500, + cpuTaskMultiplier: 5, + }); const result = estimator.estimate(); // should be 2 RTTs and 500ms for the server response time + 200 CPU assert.equal(result, 300 + 500 + 200); @@ -83,7 +86,10 @@ describe('DependencyGraph/Estimator', () => { nodeA.addDependent(nodeC); nodeA.addDependent(nodeD); - const estimator = new Estimator(nodeA, {defaultResponseTime: 500}); + const estimator = new Estimator(nodeA, { + defaultResponseTime: 500, + cpuTaskMultiplier: 5, + }); const result = estimator.estimate(); // should be 800ms A, then 1000 ms total for B, C, D in serial assert.equal(result, 1800); @@ -103,7 +109,10 @@ describe('DependencyGraph/Estimator', () => { nodeC.addDependent(nodeD); nodeC.addDependent(nodeF); // finishes 400 ms after D - const estimator = new Estimator(nodeA, {defaultResponseTime: 500}); + const estimator = new Estimator(nodeA, { + defaultResponseTime: 500, + cpuTaskMultiplier: 5, + }); const result = estimator.estimate(); // should be 800ms each for A, B, C, D, with F finishing 400 ms after D assert.equal(result, 3600);