core(metrics): switch to speedIndex from perceptualSpeedIndex #4980

patrickhulce · 2018-04-17T20:07:27Z

Rename speed-index-metric audit to speed-index
This was inconsistent with the names of all our other metric audits
Use speedIndex instead of perceptualSpeedIndex
While perceptual speed index was a nice experiment that had a few nice properties, it's not worth it.
- Perceptual speed index has some glaring issues that speed index does not (Use SSIM as progress without normalization paulirish/speedline#52 (comment))
- Speed Index is relatively well known compared to perceptual speed index and we frequently have ended up saying "it's basically the same" so the cognitive complexity of it being different is unnecessary
- Most importantly, speed index is actually better at predicting which pages people feel load faster. Creator of perceptual speed index did research and while pSI when thrown into a fancy ML model with other parameters can outperform SI, regular SI matches people's perception 10% more than pSI. source.
- Bonus (not real reason): it's super confusing with PSI (PageSpeed Insights) and we can start just using SI :D
Adds the computed artifact version of speed index
Regular change that was coming anyhow, but consequences are the extendedInfo has moved to the metrics mega-audit.

patrickhulce · 2018-04-17T20:08:27Z

lighthouse-core/gather/computed/speedline.js

      });
    }).catch(err => {
      if (/No screenshots found in trace/.test(err.message)) {
        throw new LHError(LHError.errors.NO_SCREENSHOTS);
      }

      throw err;
+    }).then(speedline => {
+      if (speedline.frames.length === 0) {


this was done just in the audit, but it makes sense to be doing this everywhere speedline is needed, moved the test as well

paulirish · 2018-04-18T17:14:46Z

I'd like to add that the mentioned paper considers suggests speed index up to onload to be a potential improvement. While we can see the value in using that to weed out the carousel/modal problem, it would face some challenges with edge cases like the polymer shop (which fires onload very early, way. before the page has finished)

patrickhulce · 2018-04-18T17:20:37Z

I'd like to add that the mentioned paper considers suggests speed index up to onload to be a potential improvement. While we can see the value in using that to weed out the carousel/modal problem, it would face some challenges with edge cases like the polymer shop (which fires onload very early, way. before the page has finished)

yeah definitely would love to explore this in the future, but agreed there are too many thorny issues to jump to without thorough investigation

paulirish · 2018-04-18T17:15:48Z

lighthouse-core/audits/metrics.js

@@ -50,6 +56,23 @@ class Metrics extends Audit {
      metrics.push({metricName, timing, timestamp});
    }

+    // Include some visual metrics from speedline
+    metrics.push({
+      metricName: 'traceFirstVisualChange',


I feel like these aren't from the trace so i think we want a diff prefix.

open to ideas, I'd like to be consistent here on the fact that these are the raw observed values compared to the (potentially) simulated metric values

WDYT about observedMetricName

hmm okay. maybe we use 'observed' prefix?
(don't tell me i suggested we use 'trace' instead of 'observed' 😨 )

no no you're 😎 I originally had raw haha which was just worse, I didn't think to suggest observed until now :)

paulirish · 2018-04-18T17:16:23Z

lighthouse-core/gather/computed/metrics/speed-index.js

+   * @return {Promise<LH.Artifacts.Metric>}
+   */
+  async computeObservedMetric(data, artifacts) {
+    const speedline = await artifacts.requestSpeedline(data.trace);


should we just collapse these 2 into 1?

not sure if we use speedline for non-speedindex purposes.
i guess if we'd want firstVC or lastVC but no speedindex computed?

I'd like to keep them separate for exactly that reason, also if we want to do other stuff with the frames

aiight groovy. 🕺

paulirish · 2018-04-18T18:04:34Z

lighthouse-core/audits/metrics.js

-      metrics.push(Object.assign({metricName}, values));
+      metrics.push({
+        metricName,
+        timing: Math.round(values.timing),


paulirish · 2018-04-18T18:18:58Z

lighthouse-core/test/results/sample_v2.json

+            "timestamp": 185608110912
+          },
+          {
+            "metricName": "traceSpeedIndex",


this duplicates "metricName": "speedIndex",

it won't in simulated cases (just like FCP/FMP)

patrickhulce · 2018-04-19T16:16:16Z

@paulirish was that all you had? I know blinkon is messing up the works

brendankenny

can wait for @paulirish, but LGTM2

brendankenny · 2018-04-19T16:36:28Z

except whatever happened to the tests :) I won't miss safeGet if that part was on purpose.

patrickhulce · 2018-04-19T16:53:20Z

except whatever happened to the tests :) I won't miss safeGet if that part was on purpose.

not sure what you mean, which tests what about them? haha I have so many ❓s :)

brendankenny · 2018-04-19T16:54:53Z

I meant "LGTM except for whatever made the tests start failing" :) And one of the failures is eslint complaining about safeGet no longer being used, which would be 🎉

patrickhulce · 2018-04-19T17:04:31Z

ooooh, yes I probably need to update some stuff from rebase

…Chrome#4980)

core(metrics): switch to speedIndex from perceptualSpeedIndex

2a29d61

patrickhulce requested review from brendankenny, paulirish and vinamratasingal-zz as code owners April 17, 2018 20:07

patrickhulce added 3.0 labels Apr 17, 2018

patrickhulce commented Apr 17, 2018

View reviewed changes

devtools-bot added the waiting4reviewer label Apr 17, 2018

update smoke expectations

4161dce

paulirish requested changes Apr 18, 2018

View reviewed changes

patrickhulce added 2 commits April 18, 2018 11:34

trace -> observed

ef527da

Merge branch 'master' into speed_index

ec36daf

brendankenny approved these changes Apr 19, 2018

View reviewed changes

paulirish approved these changes Apr 19, 2018

View reviewed changes

fix pwmetrics after rebase

f75c225

paulirish approved these changes Apr 19, 2018

View reviewed changes

paulirish merged commit cc56516 into master Apr 19, 2018

paulirish deleted the speed_index branch April 19, 2018 20:10

paulirish removed the waiting4reviewer label Apr 30, 2018

kdzwinel pushed a commit to kdzwinel/lighthouse that referenced this pull request Aug 16, 2018

core(metrics): switch to speedIndex from perceptualSpeedIndex (Google…

1e0c324

…Chrome#4980)

paulirish removed 3.0 labels Dec 18, 2018

patrickhulce mentioned this pull request Jan 29, 2019

Is PSI documented anywhere? #7106

Closed

fengzilong mentioned this pull request Jun 12, 2020

PSI 和 SI 的区别 fengzilong/pso#19

Open

brendankenny mentioned this pull request Jun 8, 2021

update metric thresholds for Lighthouse 8 GoogleChrome/web.dev#5544

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

core(metrics): switch to speedIndex from perceptualSpeedIndex #4980

core(metrics): switch to speedIndex from perceptualSpeedIndex #4980

patrickhulce commented Apr 17, 2018 •

edited

Loading

patrickhulce Apr 17, 2018

paulirish commented Apr 18, 2018

patrickhulce commented Apr 18, 2018

paulirish Apr 18, 2018

patrickhulce Apr 18, 2018 •

edited

Loading

paulirish Apr 18, 2018

patrickhulce Apr 18, 2018

paulirish Apr 18, 2018

patrickhulce Apr 18, 2018

paulirish Apr 18, 2018 •

edited

Loading

paulirish Apr 18, 2018

paulirish Apr 18, 2018

patrickhulce Apr 18, 2018

patrickhulce commented Apr 19, 2018

brendankenny left a comment

brendankenny commented Apr 19, 2018

patrickhulce commented Apr 19, 2018

brendankenny commented Apr 19, 2018 •

edited

Loading

patrickhulce commented Apr 19, 2018

core(metrics): switch to speedIndex from perceptualSpeedIndex #4980

core(metrics): switch to speedIndex from perceptualSpeedIndex #4980

Conversation

patrickhulce commented Apr 17, 2018 • edited Loading

Choose a reason for hiding this comment

paulirish commented Apr 18, 2018

patrickhulce commented Apr 18, 2018

Choose a reason for hiding this comment

patrickhulce Apr 18, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paulirish Apr 18, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

patrickhulce commented Apr 19, 2018

brendankenny left a comment

Choose a reason for hiding this comment

brendankenny commented Apr 19, 2018

patrickhulce commented Apr 19, 2018

brendankenny commented Apr 19, 2018 • edited Loading

patrickhulce commented Apr 19, 2018

patrickhulce commented Apr 17, 2018 •

edited

Loading

patrickhulce Apr 18, 2018 •

edited

Loading

paulirish Apr 18, 2018 •

edited

Loading

brendankenny commented Apr 19, 2018 •

edited

Loading