From e965cfd6f2a340af20bbfa3bc01e8c55bb37f08f Mon Sep 17 00:00:00 2001 From: Patrick Hulce Date: Fri, 11 May 2018 16:15:06 -0700 Subject: [PATCH 1/6] core(lantern): handle disk cache simulation --- .../lib/dependency-graph/network-node.js | 7 +++ .../dependency-graph/simulator/simulator.js | 47 +++++++++++++------ .../simulator/simulator-test.js | 13 +++++ 3 files changed, 52 insertions(+), 15 deletions(-) diff --git a/lighthouse-core/lib/dependency-graph/network-node.js b/lighthouse-core/lib/dependency-graph/network-node.js index c5f9bdf1ea7f..9b7aa8830392 100644 --- a/lighthouse-core/lib/dependency-graph/network-node.js +++ b/lighthouse-core/lib/dependency-graph/network-node.js @@ -52,6 +52,13 @@ class NetworkNode extends Node { return this._record._initiator && this._record._initiator.type; } + /** + * @return {boolean} + */ + get fromDiskCache() { + return this._record._fromDiskCache; + } + /** * @return {boolean} */ diff --git a/lighthouse-core/lib/dependency-graph/simulator/simulator.js b/lighthouse-core/lib/dependency-graph/simulator/simulator.js index c4fea57d50ef..14891dd619b6 100644 --- a/lighthouse-core/lib/dependency-graph/simulator/simulator.js +++ b/lighthouse-core/lib/dependency-graph/simulator/simulator.js @@ -178,11 +178,15 @@ class Simulator { if (node.type !== Node.TYPES.NETWORK) throw new Error('Unsupported'); - // 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._acquireConnection(/** @type {NetworkNode} */ (node).record); - if (!connection) return; + const networkNode = /** @type {NetworkNode} */ (node); + // If a network request is cached, we can always start it, so skip the connection checks + if (!networkNode.fromDiskCache) { + // 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._acquireConnection(networkNode.record); + if (!connection) return; + } this._markNodeAsInProgress(node, totalElapsedTime); this._setTimingData(node, { @@ -224,16 +228,27 @@ class Simulator { if (node.type !== Node.TYPES.NETWORK) throw new Error('Unsupported'); - const record = /** @type {NetworkNode} */ (node).record; + const networkNode = /** @type {NetworkNode} */ (node); const timingData = this._nodeTimings.get(node); - // 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} - ); - const estimatedTimeElapsed = calculation.timeElapsed + timingData.timeElapsedOvershoot; + let timeElapsed = 0; + if (networkNode.fromDiskCache) { + // Rough access time for seeking to location on disk and reading sequentially + // @see http://norvig.com/21-days.html#answers + const sizeInMb = (networkNode.record._resourceSize || 0) / 1024 / 1024; + timeElapsed = 8 + 20 * sizeInMb - timingData.timeElapsed; + } else { + // If we're estimating time remaining, we already acquired a connection for this record, definitely non-null + const connection = /** @type {TcpConnection} */ (this._acquireConnection(networkNode.record)); + const calculation = connection.simulateDownloadUntil( + networkNode.record.transferSize - timingData.bytesDownloaded, + {timeAlreadyElapsed: timingData.timeElapsed, maximumTimeToElapse: Infinity} + ); + + timeElapsed = calculation.timeElapsed; + } + + const estimatedTimeElapsed = timeElapsed + timingData.timeElapsedOvershoot; this._setTimingData(node, {estimatedTimeElapsed}); return estimatedTimeElapsed; } @@ -261,7 +276,9 @@ class Simulator { const timingData = this._nodeTimings.get(node); const isFinished = timingData.estimatedTimeElapsed === timePeriodLength; - if (node.type === Node.TYPES.CPU) { + const networkNode = /** @type {NetworkNode} */ (node); + + if (node.type === Node.TYPES.CPU || networkNode.fromDiskCache) { return isFinished ? this._markNodeAsComplete(node, totalElapsedTime) : (timingData.timeElapsed += timePeriodLength); @@ -269,7 +286,7 @@ class Simulator { if (node.type !== Node.TYPES.NETWORK) throw new Error('Unsupported'); - const record = /** @type {NetworkNode} */ (node).record; + const record = networkNode.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( diff --git a/lighthouse-core/test/lib/dependency-graph/simulator/simulator-test.js b/lighthouse-core/test/lib/dependency-graph/simulator/simulator-test.js index 20ecb5a29fcf..d8dbd1991c1d 100644 --- a/lighthouse-core/test/lib/dependency-graph/simulator/simulator-test.js +++ b/lighthouse-core/test/lib/dependency-graph/simulator/simulator-test.js @@ -91,6 +91,19 @@ describe('DependencyGraph/Simulator', () => { assertNodeTiming(result, nodeD, {startTime: 2400, endTime: 3200}); }); + it('should simulate cached network graphs', () => { + const nodeA = new NetworkNode(request({startTime: 0, endTime: 1, _fromDiskCache: true})); + const nodeB = new NetworkNode(request({startTime: 0, endTime: 3, _fromDiskCache: true})); + nodeA.addDependent(nodeB); + + const simulator = new Simulator({serverResponseTimeByOrigin}); + const result = simulator.simulate(nodeA); + // should be ~8ms each for A, B + assert.equal(result.timeInMs, 16); + assertNodeTiming(result, nodeA, {startTime: 0, endTime: 8}); + assertNodeTiming(result, nodeB, {startTime: 8, endTime: 16}); + }); + it('should simulate basic CPU queue graphs', () => { const nodeA = new NetworkNode(request({})); const nodeB = new CpuNode(cpuTask({duration: 100})); From 6cd532e01645b0a6eaa62392e05809e0e22c8bff Mon Sep 17 00:00:00 2001 From: Patrick Hulce Date: Tue, 15 May 2018 14:33:39 -0700 Subject: [PATCH 2/6] scale intercept --- lighthouse-core/gather/computed/metrics/lantern-metric.js | 5 ++++- lighthouse-core/test/audits/first-meaningful-paint-test.js | 6 +++--- lighthouse-core/test/audits/metrics-test.js | 4 ++-- lighthouse-core/test/audits/predictive-perf-test.js | 4 ++-- .../gather/computed/metrics/first-contentful-paint-test.js | 2 +- .../gather/computed/metrics/first-meaningful-paint-test.js | 2 +- .../computed/metrics/lantern-first-contentful-paint-test.js | 2 +- .../computed/metrics/lantern-first-meaningful-paint-test.js | 2 +- typings/web-inspector.d.ts | 1 + 9 files changed, 16 insertions(+), 12 deletions(-) diff --git a/lighthouse-core/gather/computed/metrics/lantern-metric.js b/lighthouse-core/gather/computed/metrics/lantern-metric.js index b6e575e4d1eb..7585f68175cf 100644 --- a/lighthouse-core/gather/computed/metrics/lantern-metric.js +++ b/lighthouse-core/gather/computed/metrics/lantern-metric.js @@ -97,8 +97,11 @@ class LanternMetricArtifact extends ComputedArtifact { Object.assign({}, extras, {optimistic: false}) ); + // Estimates under 1s don't really follow the normal curve fit, minimize the impact of the intercept + const interceptMultiplier = this.COEFFICIENTS.intercept > 0 ? + Math.min(1, optimisticEstimate.timeInMs / 1000) : 1; const timing = - this.COEFFICIENTS.intercept + + this.COEFFICIENTS.intercept * interceptMultiplier + this.COEFFICIENTS.optimistic * optimisticEstimate.timeInMs + this.COEFFICIENTS.pessimistic * pessimisticEstimate.timeInMs; diff --git a/lighthouse-core/test/audits/first-meaningful-paint-test.js b/lighthouse-core/test/audits/first-meaningful-paint-test.js index b261c390aee0..2b266e28d8bd 100644 --- a/lighthouse-core/test/audits/first-meaningful-paint-test.js +++ b/lighthouse-core/test/audits/first-meaningful-paint-test.js @@ -39,8 +39,8 @@ describe('Performance: first-meaningful-paint audit', () => { const context = {options, settings: {throttlingMethod: 'simulate'}}; const fmpResult = await FMPAudit.audit(artifacts, context); - assert.equal(fmpResult.score, 0.95); - assert.equal(Util.formatDisplayValue(fmpResult.displayValue), '2,030\xa0ms'); - assert.equal(Math.round(fmpResult.rawValue), 2029); + assert.equal(fmpResult.score, 0.96); + assert.equal(Util.formatDisplayValue(fmpResult.displayValue), '1,950\xa0ms'); + assert.equal(Math.round(fmpResult.rawValue), 1949); }); }); diff --git a/lighthouse-core/test/audits/metrics-test.js b/lighthouse-core/test/audits/metrics-test.js index 2596f238d814..f915b304f3f3 100644 --- a/lighthouse-core/test/audits/metrics-test.js +++ b/lighthouse-core/test/audits/metrics-test.js @@ -29,9 +29,9 @@ describe('Performance: metrics', () => { const result = await Audit.audit(artifacts, {settings}); assert.deepStrictEqual(result.details.items[0], { - firstContentfulPaint: 1272, + firstContentfulPaint: 1038, firstContentfulPaintTs: undefined, - firstMeaningfulPaint: 2029, + firstMeaningfulPaint: 1949, firstMeaningfulPaintTs: undefined, firstCPUIdle: 4309, firstCPUIdleTs: undefined, diff --git a/lighthouse-core/test/audits/predictive-perf-test.js b/lighthouse-core/test/audits/predictive-perf-test.js index 9896a17258c3..d735dccbc211 100644 --- a/lighthouse-core/test/audits/predictive-perf-test.js +++ b/lighthouse-core/test/audits/predictive-perf-test.js @@ -30,10 +30,10 @@ describe('Performance: predictive performance audit', () => { assert.equal(output.displayValue, '4,310\xa0ms'); const valueOf = name => Math.round(output.extendedInfo.value[name]); - assert.equal(valueOf('roughEstimateOfFCP'), 1272); + assert.equal(valueOf('roughEstimateOfFCP'), 1038); assert.equal(valueOf('optimisticFCP'), 611); assert.equal(valueOf('pessimisticFCP'), 611); - assert.equal(valueOf('roughEstimateOfFMP'), 2029); + assert.equal(valueOf('roughEstimateOfFMP'), 1949); assert.equal(valueOf('optimisticFMP'), 911); assert.equal(valueOf('pessimisticFMP'), 1198); assert.equal(valueOf('roughEstimateOfTTI'), 4309); diff --git a/lighthouse-core/test/gather/computed/metrics/first-contentful-paint-test.js b/lighthouse-core/test/gather/computed/metrics/first-contentful-paint-test.js index 6d9d05ff2b84..dfc880f0f27d 100644 --- a/lighthouse-core/test/gather/computed/metrics/first-contentful-paint-test.js +++ b/lighthouse-core/test/gather/computed/metrics/first-contentful-paint-test.js @@ -19,7 +19,7 @@ describe('Metrics: FCP', () => { const settings = {throttlingMethod: 'simulate'}; const result = await artifacts.requestFirstContentfulPaint({trace, devtoolsLog, settings}); - assert.equal(Math.round(result.timing), 1272); + assert.equal(Math.round(result.timing), 1038); assert.equal(Math.round(result.optimisticEstimate.timeInMs), 611); assert.equal(Math.round(result.pessimisticEstimate.timeInMs), 611); assert.equal(result.optimisticEstimate.nodeTimings.size, 2); diff --git a/lighthouse-core/test/gather/computed/metrics/first-meaningful-paint-test.js b/lighthouse-core/test/gather/computed/metrics/first-meaningful-paint-test.js index 3fbdde59f72b..52bfad40442d 100644 --- a/lighthouse-core/test/gather/computed/metrics/first-meaningful-paint-test.js +++ b/lighthouse-core/test/gather/computed/metrics/first-meaningful-paint-test.js @@ -39,7 +39,7 @@ describe('Metrics: FMP', () => { const result = await artifacts.requestFirstMeaningfulPaint({trace, devtoolsLog, settings}); - assert.equal(Math.round(result.timing), 2029); + assert.equal(Math.round(result.timing), 1949); assert.equal(Math.round(result.optimisticEstimate.timeInMs), 911); assert.equal(Math.round(result.pessimisticEstimate.timeInMs), 1198); assert.equal(result.optimisticEstimate.nodeTimings.size, 4); diff --git a/lighthouse-core/test/gather/computed/metrics/lantern-first-contentful-paint-test.js b/lighthouse-core/test/gather/computed/metrics/lantern-first-contentful-paint-test.js index 6911800366e6..e7728a2ed542 100644 --- a/lighthouse-core/test/gather/computed/metrics/lantern-first-contentful-paint-test.js +++ b/lighthouse-core/test/gather/computed/metrics/lantern-first-contentful-paint-test.js @@ -18,7 +18,7 @@ describe('Metrics: Lantern FCP', () => { const result = await artifacts.requestLanternFirstContentfulPaint({trace, devtoolsLog, settings: {}}); - assert.equal(Math.round(result.timing), 1272); + assert.equal(Math.round(result.timing), 1038); assert.equal(Math.round(result.optimisticEstimate.timeInMs), 611); assert.equal(Math.round(result.pessimisticEstimate.timeInMs), 611); assert.equal(result.optimisticEstimate.nodeTimings.size, 2); diff --git a/lighthouse-core/test/gather/computed/metrics/lantern-first-meaningful-paint-test.js b/lighthouse-core/test/gather/computed/metrics/lantern-first-meaningful-paint-test.js index fbd55770f761..c56dd7cb8c3f 100644 --- a/lighthouse-core/test/gather/computed/metrics/lantern-first-meaningful-paint-test.js +++ b/lighthouse-core/test/gather/computed/metrics/lantern-first-meaningful-paint-test.js @@ -18,7 +18,7 @@ describe('Metrics: Lantern FMP', () => { const result = await artifacts.requestLanternFirstMeaningfulPaint({trace, devtoolsLog, settings: {}}); - assert.equal(Math.round(result.timing), 2029); + assert.equal(Math.round(result.timing), 1949); assert.equal(Math.round(result.optimisticEstimate.timeInMs), 911); assert.equal(Math.round(result.pessimisticEstimate.timeInMs), 1198); assert.equal(result.optimisticEstimate.nodeTimings.size, 4); diff --git a/typings/web-inspector.d.ts b/typings/web-inspector.d.ts index 7a7ccc64bdd3..ce845066a879 100644 --- a/typings/web-inspector.d.ts +++ b/typings/web-inspector.d.ts @@ -30,6 +30,7 @@ declare global { _transferSize?: number; /** Should use a default of 0 if not defined */ _resourceSize?: number; + _fromDiskCache?: boolean; finished: boolean; requestMethod: string; From 2c67268986a138245bc96fa87680005901a4ec8b Mon Sep 17 00:00:00 2001 From: Patrick Hulce Date: Tue, 15 May 2018 14:53:23 -0700 Subject: [PATCH 3/6] fix SI --- .../gather/computed/metrics/lantern-speed-index.js | 2 +- lighthouse-core/test/audits/metrics-test.js | 2 +- .../test/gather/computed/metrics/lantern-speed-index-test.js | 4 ++-- .../test/gather/computed/metrics/speed-index-test.js | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lighthouse-core/gather/computed/metrics/lantern-speed-index.js b/lighthouse-core/gather/computed/metrics/lantern-speed-index.js index 2cd48edb7414..5da7b99fdc9b 100644 --- a/lighthouse-core/gather/computed/metrics/lantern-speed-index.js +++ b/lighthouse-core/gather/computed/metrics/lantern-speed-index.js @@ -50,7 +50,7 @@ class SpeedIndex extends MetricArtifact { * @return {LH.Gatherer.Simulation.Result} */ getEstimateFromSimulation(simulationResult, extras) { - const fcpTimeInMs = extras.fcpResult.timing; + const fcpTimeInMs = extras.fcpResult.pessimisticEstimate.timeInMs; const estimate = extras.optimistic ? extras.speedline.speedIndex : SpeedIndex.computeLayoutBasedSpeedIndex(simulationResult.nodeTimings, fcpTimeInMs); diff --git a/lighthouse-core/test/audits/metrics-test.js b/lighthouse-core/test/audits/metrics-test.js index f915b304f3f3..f5b4fea1c270 100644 --- a/lighthouse-core/test/audits/metrics-test.js +++ b/lighthouse-core/test/audits/metrics-test.js @@ -37,7 +37,7 @@ describe('Performance: metrics', () => { firstCPUIdleTs: undefined, interactive: 4309, interactiveTs: undefined, - speedIndex: 1501, + speedIndex: 1461, speedIndexTs: undefined, estimatedInputLatency: 104, estimatedInputLatencyTs: undefined, diff --git a/lighthouse-core/test/gather/computed/metrics/lantern-speed-index-test.js b/lighthouse-core/test/gather/computed/metrics/lantern-speed-index-test.js index 5b8f1e554f65..e06691e7bdf4 100644 --- a/lighthouse-core/test/gather/computed/metrics/lantern-speed-index-test.js +++ b/lighthouse-core/test/gather/computed/metrics/lantern-speed-index-test.js @@ -17,8 +17,8 @@ describe('Metrics: Lantern Speed Index', () => { const artifacts = Runner.instantiateComputedArtifacts(); const result = await artifacts.requestLanternSpeedIndex({trace, devtoolsLog, settings: {}}); - assert.equal(Math.round(result.timing), 1501); + assert.equal(Math.round(result.timing), 1461); assert.equal(Math.round(result.optimisticEstimate.timeInMs), 605); - assert.equal(Math.round(result.pessimisticEstimate.timeInMs), 1392); + assert.equal(Math.round(result.pessimisticEstimate.timeInMs), 1330); }); }); diff --git a/lighthouse-core/test/gather/computed/metrics/speed-index-test.js b/lighthouse-core/test/gather/computed/metrics/speed-index-test.js index eb0aefa90da3..b2b6e2e499e3 100644 --- a/lighthouse-core/test/gather/computed/metrics/speed-index-test.js +++ b/lighthouse-core/test/gather/computed/metrics/speed-index-test.js @@ -19,9 +19,9 @@ describe('Metrics: Speed Index', () => { const settings = {throttlingMethod: 'simulate'}; const result = await artifacts.requestSpeedIndex({trace, devtoolsLog, settings}); - assert.equal(Math.round(result.timing), 1501); + assert.equal(Math.round(result.timing), 1461); assert.equal(Math.round(result.optimisticEstimate.timeInMs), 605); - assert.equal(Math.round(result.pessimisticEstimate.timeInMs), 1392); + assert.equal(Math.round(result.pessimisticEstimate.timeInMs), 1330); }); it('should compute an observed value', async () => { From 4a635b1e99a8c747b118aadf80d75eefed05395c Mon Sep 17 00:00:00 2001 From: Patrick Hulce Date: Tue, 15 May 2018 16:16:18 -0700 Subject: [PATCH 4/6] type check --- lighthouse-core/lib/dependency-graph/network-node.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lighthouse-core/lib/dependency-graph/network-node.js b/lighthouse-core/lib/dependency-graph/network-node.js index 9b7aa8830392..1a21f2e6d57a 100644 --- a/lighthouse-core/lib/dependency-graph/network-node.js +++ b/lighthouse-core/lib/dependency-graph/network-node.js @@ -56,7 +56,7 @@ class NetworkNode extends Node { * @return {boolean} */ get fromDiskCache() { - return this._record._fromDiskCache; + return !!this._record._fromDiskCache; } /** From 0ff873e6550ed87e38538525e41ad7912eee17e2 Mon Sep 17 00:00:00 2001 From: Patrick Hulce Date: Wed, 16 May 2018 09:13:14 -0700 Subject: [PATCH 5/6] split estimateTimeRemaining --- .../dependency-graph/simulator/simulator.js | 45 ++++++++++++------- 1 file changed, 30 insertions(+), 15 deletions(-) diff --git a/lighthouse-core/lib/dependency-graph/simulator/simulator.js b/lighthouse-core/lib/dependency-graph/simulator/simulator.js index 14891dd619b6..adbc90496386 100644 --- a/lighthouse-core/lib/dependency-graph/simulator/simulator.js +++ b/lighthouse-core/lib/dependency-graph/simulator/simulator.js @@ -213,23 +213,38 @@ class Simulator { */ _estimateTimeRemaining(node) { if (node.type === Node.TYPES.CPU) { - const timingData = this._nodeTimings.get(node); - const multiplier = /** @type {CpuNode} */ (node).didPerformLayout() - ? this._layoutTaskMultiplier - : this._cpuSlowdownMultiplier; - const totalDuration = Math.min( - Math.round(/** @type {CpuNode} */ (node).event.dur / 1000 * multiplier), - DEFAULT_MAXIMUM_CPU_TASK_DURATION - ); - const estimatedTimeElapsed = totalDuration - timingData.timeElapsed; - this._setTimingData(node, {estimatedTimeElapsed}); - return estimatedTimeElapsed; + return this._estimateCPUTimeRemaining(/** @type {CpuNode} */ (node)); + } else if (node.type === Node.TYPES.NETWORK) { + return this._estimateNetworkTimeRemaining(/** @type {NetworkNode} */ (node)); + } else { + throw new Error('Unsupported'); } + } - if (node.type !== Node.TYPES.NETWORK) throw new Error('Unsupported'); + /** + * @param {CpuNode} cpuNode + * @return {number} + */ + _estimateCPUTimeRemaining(cpuNode) { + const timingData = this._nodeTimings.get(cpuNode); + const multiplier = cpuNode.didPerformLayout() + ? this._layoutTaskMultiplier + : this._cpuSlowdownMultiplier; + const totalDuration = Math.min( + Math.round(cpuNode.event.dur / 1000 * multiplier), + DEFAULT_MAXIMUM_CPU_TASK_DURATION + ); + const estimatedTimeElapsed = totalDuration - timingData.timeElapsed; + this._setTimingData(cpuNode, {estimatedTimeElapsed}); + return estimatedTimeElapsed; + } - const networkNode = /** @type {NetworkNode} */ (node); - const timingData = this._nodeTimings.get(node); + /** + * @param {NetworkNode} networkNode + * @return {number} + */ + _estimateNetworkTimeRemaining(networkNode) { + const timingData = this._nodeTimings.get(networkNode); let timeElapsed = 0; if (networkNode.fromDiskCache) { @@ -249,7 +264,7 @@ class Simulator { } const estimatedTimeElapsed = timeElapsed + timingData.timeElapsedOvershoot; - this._setTimingData(node, {estimatedTimeElapsed}); + this._setTimingData(networkNode, {estimatedTimeElapsed}); return estimatedTimeElapsed; } From 37daa01c1f0bb755954d408c2bb674fb5dac2545 Mon Sep 17 00:00:00 2001 From: Patrick Hulce Date: Fri, 18 May 2018 10:59:27 -0700 Subject: [PATCH 6/6] fix simulator --- lighthouse-core/lib/dependency-graph/simulator/simulator.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lighthouse-core/lib/dependency-graph/simulator/simulator.js b/lighthouse-core/lib/dependency-graph/simulator/simulator.js index adbc90496386..260c26e69dc3 100644 --- a/lighthouse-core/lib/dependency-graph/simulator/simulator.js +++ b/lighthouse-core/lib/dependency-graph/simulator/simulator.js @@ -377,7 +377,11 @@ class Simulator { } if (!nodesInProgress.size) { - throw new Error('Failed to start a node, potential mismatch in original execution'); + // interplay between fromDiskCache and connectionReused can be incorrect + // proceed with flexibleOrdering if we can, otherwise give up + if (this._flexibleOrdering) throw new Error('Failed to start a node'); + this._flexibleOrdering = true; + continue; } // set the available throughput for all connections based on # inflight