core(speedindex): scale coefficients according to throttling #7007
Conversation
patrickhulce
force-pushed
the
speed_index_coefficients
branch
from
7ec56a0
to
e5ce2e3
Compare
| @@ -42,6 +42,19 @@ class LanternMetricArtifact { | |||
| throw new Error('COEFFICIENTS unimplemented!'); | |||
| } | |||
|
|
|||
| /** | |||
| * Scale the coefficients according to the throttling settings. | |||
There was a problem hiding this comment.
maybe "Returns the coefficients, scaled by the throttling settings if needed by the metric. Some lantern metrics..." (or something like that) since this is more or less the proper way to retrieve this.COEFFICIENTS now?
| "pessimistic": 1631, | ||
| "timing": 1657, | ||
| } | ||
| `); |
| 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). |
There was a problem hiding this comment.
this description is a little confusing. One attempt at rewording: 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 the baseline points of 30 ms RTT (where Speed Index should be a ~50/50 blend of optimistic/pessimistic) and the default throttled RTT (where Speed Index should be the default-coefficient blend of optimistic/pessimistic).
There was a problem hiding this comment.
Also can you add a note about how this scale was derived/can be re-derived if defaults change in the future? It's not clear why you'd expect 50/50 mix at 30ms and then be what you'd expect between there and the default point and beyond :)
Summary
This PR makes it possible to scale the lantern coefficients based on the throttling settings. Previously the desktop LR preset was giving too pessimistic estimates on Speed Index because of the hardcoded
1.4xmultiplier on the observed speed index.Key Example Fix (facebook.com)
Before
After
Related Issues/PRs
fixes #6971