Skip to content

Commit

Permalink
core(speedindex): scale scoring coefficients based on throttling (#7007)
Browse files Browse the repository at this point in the history
  • Loading branch information
patrickhulce authored and brendankenny committed Jan 15, 2019
1 parent 534621c commit 82819a0
Show file tree
Hide file tree
Showing 5 changed files with 99 additions and 15 deletions.
22 changes: 18 additions & 4 deletions lighthouse-core/computed/metrics/lantern-metric.js
Expand Up @@ -42,6 +42,19 @@ class LanternMetricArtifact {
throw new Error('COEFFICIENTS unimplemented!');
}

/**
* Returns the coefficients, scaled by the throttling settings if needed by the metric.
* 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 getScaledCoefficients(rttMs) { // eslint-disable-line no-unused-vars
return this.COEFFICIENTS;
}

/**
* @param {Node} dependencyGraph
* @param {LH.Artifacts.TraceOfTab} traceOfTab
Expand Down Expand Up @@ -107,13 +120,14 @@ class LanternMetricArtifact {
Object.assign({}, extras, {optimistic: false})
);

const coefficients = this.getScaledCoefficients(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,
Expand Down
28 changes: 28 additions & 0 deletions lighthouse-core/computed/metrics/lantern-speed-index.js
Expand Up @@ -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 */

Expand All @@ -28,6 +29,33 @@ class LanternSpeedIndex extends LanternMetric {
};
}

/**
* @param {number} rttMs
* @return {LH.Gatherer.Simulation.MetricCoefficients}
*/
static getScaledCoefficients(rttMs) { // eslint-disable-line no-unused-vars
// We want to scale our default coefficients based on the speed of the connection.
// We will linearly interpolate coefficients for the passed-in rttMs based on two pre-determined points:
// 1. Baseline point of 30 ms RTT where Speed Index should be a ~50/50 blend of optimistic/pessimistic.
// 30 ms was based on a typical home WiFi connection's actual RTT.
// Coefficients here follow from the fact that the optimistic estimate should be very close
// to reality at this connection speed and the pessimistic estimate compensates for minor
// connection speed differences.
// 2. Default throttled point of 150 ms RTT where the default coefficients have been determined to be most accurate.
// Coefficients here were determined through thorough analysis and linear regression on the
// lantern test data set. See lighthouse-core/scripts/test-lantern.sh for more detail.
// While the coefficients haven't been analyzed at the interpolated points, it's our current best effort.
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}
Expand Down
5 changes: 5 additions & 0 deletions lighthouse-core/lib/dependency-graph/simulator/simulator.js
Expand Up @@ -75,6 +75,11 @@ class Simulator {
this._connectionPool = /** @type {ConnectionPool} */ (null);
}

/** @return {number} */
get rtt() {
return this._rtt;
}

/**
* @param {Node} graph
*/
Expand Down

This file was deleted.

50 changes: 48 additions & 2 deletions lighthouse-core/test/computed/metrics/lantern-speed-index-test.js
Expand Up @@ -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');
Expand All @@ -13,14 +14,59 @@ 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);

expect({
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.getScaledCoefficients(defaultThrottling.rttMs);
expect(result).toEqual(LanternSpeedIndex.COEFFICIENTS);
});

it('should scale coefficients back', async () => {
const result = LanternSpeedIndex.getScaledCoefficients(5);
expect(result).toEqual({intercept: -0, pessimistic: 0.5, optimistic: 0.5});
});

it('should scale coefficients forward', async () => {
const result = LanternSpeedIndex.getScaledCoefficients(300);
expect(result).toMatchInlineSnapshot(`
Object {
"intercept": -562.5,
"optimistic": 2.525,
"pessimistic": 0.8375,
}
`);
});
});

0 comments on commit 82819a0

Please sign in to comment.