From e5ce2e35d29aa31d1d4805bab2bc352bd0699384 Mon Sep 17 00:00:00 2001 From: Patrick Hulce Date: Mon, 14 Jan 2019 15:10:56 -0600 Subject: [PATCH] core(speedindex): scale coefficients according to throttling --- .../computed/metrics/lantern-metric.js | 22 ++++++-- .../computed/metrics/lantern-speed-index.js | 20 ++++++++ .../dependency-graph/simulator/simulator.js | 5 ++ .../lantern-speed-index-test.js.snap | 9 ---- .../metrics/lantern-speed-index-test.js | 50 ++++++++++++++++++- 5 files changed, 91 insertions(+), 15 deletions(-) delete mode 100644 lighthouse-core/test/computed/metrics/__snapshots__/lantern-speed-index-test.js.snap diff --git a/lighthouse-core/computed/metrics/lantern-metric.js b/lighthouse-core/computed/metrics/lantern-metric.js index 9362d589b7ff..383f5a5add8c 100644 --- a/lighthouse-core/computed/metrics/lantern-metric.js +++ b/lighthouse-core/computed/metrics/lantern-metric.js @@ -42,6 +42,19 @@ class LanternMetricArtifact { throw new Error('COEFFICIENTS unimplemented!'); } + /** + * Scale the coefficients according to the throttling settings. + * Some lantern metrics (speed-index) use components in their estimate that are not + * from the simulator. In this case, we need to adjust the coefficients as the target throttling + * settings change. + * + * @param {number} rttMs + * @return {LH.Gatherer.Simulation.MetricCoefficients} + */ + static scaleCoefficients(rttMs) { // eslint-disable-line no-unused-vars + return this.COEFFICIENTS; + } + /** * @param {Node} dependencyGraph * @param {LH.Artifacts.TraceOfTab} traceOfTab @@ -107,13 +120,14 @@ class LanternMetricArtifact { Object.assign({}, extras, {optimistic: false}) ); + const coefficients = this.scaleCoefficients(simulator.rtt); // Estimates under 1s don't really follow the normal curve fit, minimize the impact of the intercept - const interceptMultiplier = this.COEFFICIENTS.intercept > 0 ? + const interceptMultiplier = coefficients.intercept > 0 ? Math.min(1, optimisticEstimate.timeInMs / 1000) : 1; const timing = - this.COEFFICIENTS.intercept * interceptMultiplier + - this.COEFFICIENTS.optimistic * optimisticEstimate.timeInMs + - this.COEFFICIENTS.pessimistic * pessimisticEstimate.timeInMs; + coefficients.intercept * interceptMultiplier + + coefficients.optimistic * optimisticEstimate.timeInMs + + coefficients.pessimistic * pessimisticEstimate.timeInMs; return { timing, diff --git a/lighthouse-core/computed/metrics/lantern-speed-index.js b/lighthouse-core/computed/metrics/lantern-speed-index.js index 7b6c5d53dafa..039767540961 100644 --- a/lighthouse-core/computed/metrics/lantern-speed-index.js +++ b/lighthouse-core/computed/metrics/lantern-speed-index.js @@ -10,6 +10,7 @@ const LanternMetric = require('./lantern-metric.js'); const BaseNode = require('../../lib/dependency-graph/base-node.js'); const Speedline = require('../speedline.js'); const LanternFirstContentfulPaint = require('./lantern-first-contentful-paint.js'); +const defaultThrottling = require('../../config/constants.js').throttling; /** @typedef {BaseNode.Node} Node */ @@ -28,6 +29,25 @@ class LanternSpeedIndex extends LanternMetric { }; } + /** + * @param {number} rttMs + * @return {LH.Gatherer.Simulation.MetricCoefficients} + */ + static scaleCoefficients(rttMs) { // eslint-disable-line no-unused-vars + // We want to multiply our default coefficients based on how much farther from baseline our + // new throttling settings are compared to the defaults. + // We will use a baseline of 30 ms RTT (where Speed Index should be a ~50/50 blend of optimistic/pessimistic). + const defaultCoefficients = this.COEFFICIENTS; + const defaultRttExcess = defaultThrottling.mobileSlow4G.rttMs - 30; + const multiplier = Math.max((rttMs - 30) / defaultRttExcess, 0); + + return { + intercept: defaultCoefficients.intercept * multiplier, + optimistic: 0.5 + (defaultCoefficients.optimistic - 0.5) * multiplier, + pessimistic: 0.5 + (defaultCoefficients.pessimistic - 0.5) * multiplier, + }; + } + /** * @param {Node} dependencyGraph * @return {Node} diff --git a/lighthouse-core/lib/dependency-graph/simulator/simulator.js b/lighthouse-core/lib/dependency-graph/simulator/simulator.js index d3b92b5dddaf..fe46d21c207e 100644 --- a/lighthouse-core/lib/dependency-graph/simulator/simulator.js +++ b/lighthouse-core/lib/dependency-graph/simulator/simulator.js @@ -75,6 +75,11 @@ class Simulator { this._connectionPool = /** @type {ConnectionPool} */ (null); } + /** @return {number} */ + get rtt() { + return this._rtt; + } + /** * @param {Node} graph */ diff --git a/lighthouse-core/test/computed/metrics/__snapshots__/lantern-speed-index-test.js.snap b/lighthouse-core/test/computed/metrics/__snapshots__/lantern-speed-index-test.js.snap deleted file mode 100644 index 8854004ffa01..000000000000 --- a/lighthouse-core/test/computed/metrics/__snapshots__/lantern-speed-index-test.js.snap +++ /dev/null @@ -1,9 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`Metrics: Lantern Speed Index should compute predicted value 1`] = ` -Object { - "optimistic": 605, - "pessimistic": 1631, - "timing": 1657, -} -`; diff --git a/lighthouse-core/test/computed/metrics/lantern-speed-index-test.js b/lighthouse-core/test/computed/metrics/lantern-speed-index-test.js index f5cc91656895..92051dc47a27 100644 --- a/lighthouse-core/test/computed/metrics/lantern-speed-index-test.js +++ b/lighthouse-core/test/computed/metrics/lantern-speed-index-test.js @@ -5,6 +5,7 @@ */ 'use strict'; +const defaultThrottling = require('../../../config/constants.js').throttling.mobileSlow4G; const LanternSpeedIndex = require('../../../computed/metrics/lantern-speed-index.js'); const trace = require('../../fixtures/traces/progressive-app-m60.json'); @@ -13,7 +14,7 @@ const devtoolsLog = require('../../fixtures/traces/progressive-app-m60.devtools. /* eslint-env jest */ describe('Metrics: Lantern Speed Index', () => { it('should compute predicted value', async () => { - const settings = {}; + const settings = {throttlingMethod: 'simulate', throttling: defaultThrottling}; const context = {settings, computedCache: new Map()}; const result = await LanternSpeedIndex.request({trace, devtoolsLog, settings}, context); @@ -21,6 +22,51 @@ describe('Metrics: Lantern Speed Index', () => { timing: Math.round(result.timing), optimistic: Math.round(result.optimisticEstimate.timeInMs), pessimistic: Math.round(result.pessimisticEstimate.timeInMs), - }).toMatchSnapshot(); + }).toMatchInlineSnapshot(` +Object { + "optimistic": 605, + "pessimistic": 1631, + "timing": 1657, +} +`); + }); + + it('should compute predicted value for different settings', async () => { + const settings = {throttlingMethod: 'simulate', throttling: {...defaultThrottling, rttMs: 300}}; + const context = {settings, computedCache: new Map()}; + const result = await LanternSpeedIndex.request({trace, devtoolsLog, settings}, context); + + expect({ + timing: Math.round(result.timing), + optimistic: Math.round(result.optimisticEstimate.timeInMs), + pessimistic: Math.round(result.pessimisticEstimate.timeInMs), + }).toMatchInlineSnapshot(` +Object { + "optimistic": 605, + "pessimistic": 2381, + "timing": 2958, +} +`); + }); + + it('should not scale coefficients at default', async () => { + const result = LanternSpeedIndex.scaleCoefficients(defaultThrottling.rttMs); + expect(result).toEqual(LanternSpeedIndex.COEFFICIENTS); + }); + + it('should scale coefficients back', async () => { + const result = LanternSpeedIndex.scaleCoefficients(5); + expect(result).toEqual({intercept: -0, pessimistic: 0.5, optimistic: 0.5}); + }); + + it('should scale coefficients forward', async () => { + const result = LanternSpeedIndex.scaleCoefficients(300); + expect(result).toMatchInlineSnapshot(` +Object { + "intercept": -562.5, + "optimistic": 2.525, + "pessimistic": 0.8375, +} +`); }); });