Skip to content

core(scoring): apply v6 score weightings #9949

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Mar 11, 2020
Merged

core(scoring): apply v6 score weightings #9949

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
a1126fc
core(scoring): apply v6 score weightings
paulirish Nov 7, 2019
033a06b
drop LCP because connor did a beautiful job in 9905 already
paulirish Nov 9, 2019
516ccf2
Merge branch 'master' into v6scoring
paulirish Nov 10, 2019
2d3e8f6
Merge branch 'master' into v6scoring
paulirish Mar 10, 2020
a5097a5
updated weightings.
paulirish Mar 10, 2020
b85f9e2
calibrate CLS for 0.1 as passing
paulirish Mar 10, 2020
f9fc5b3
remove duplicate LCP in config.audits. upgrade test for better diff
paulirish Mar 10, 2020
17ba25b
unit cli snapshot
paulirish Mar 10, 2020
5694740
CLS calibration comment
paulirish Mar 10, 2020
035e4eb
recalib cls in roundtrip
paulirish Mar 10, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions lighthouse-core/audits/metrics/cumulative-layout-shift.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,9 @@ class CumulativeLayoutShift extends Audit {
*/
static get defaultOptions() {
return {
// TODO(paulirish): Calibrate these
scorePODR: 0.05,
// Calibrated to assure 0.1 gets a score of 0.9.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe a note in here or in the PR description or in the scoring doc on where these came from? As we've discovered it can be very difficult to figure out our motivation for some of these curves later on :)

// see https://www.desmos.com/calculator/se7wby6vgc
scorePODR: 0.02,
scoreMedian: 0.4,
};
}
Expand Down
23 changes: 13 additions & 10 deletions lighthouse-core/config/default-config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,7 @@ const defaultConfig = {
'viewport',
'without-javascript',
'metrics/first-contentful-paint',
'metrics/largest-contentful-paint',
'metrics/first-meaningful-paint',
'metrics/largest-contentful-paint',
'load-fast-enough-for-pwa',
Expand Down Expand Up @@ -388,16 +389,18 @@ const defaultConfig = {
'performance': {
title: str_(UIStrings.performanceCategoryTitle),
auditRefs: [
{id: 'first-contentful-paint', weight: 3, group: 'metrics'},
{id: 'first-meaningful-paint', weight: 1, group: 'metrics'},
{id: 'largest-contentful-paint', weight: 0, group: 'metrics'},
{id: 'speed-index', weight: 4, group: 'metrics'},
{id: 'interactive', weight: 5, group: 'metrics'},
{id: 'first-cpu-idle', weight: 2, group: 'metrics'},
{id: 'max-potential-fid', weight: 0, group: 'metrics'},
{id: 'cumulative-layout-shift', weight: 0}, // intentionally left out of metrics so it won't be displayed
{id: 'estimated-input-latency', weight: 0}, // intentionally left out of metrics so it won't be displayed
{id: 'total-blocking-time', weight: 0}, // intentionally left out of metrics so it won't be displayed
{id: 'first-contentful-paint', weight: 15, group: 'metrics'},
{id: 'speed-index', weight: 15, group: 'metrics'},
{id: 'largest-contentful-paint', weight: 25, group: 'metrics'},
{id: 'interactive', weight: 15, group: 'metrics'},
{id: 'total-blocking-time', weight: 25, group: 'metrics'},
{id: 'cumulative-layout-shift', weight: 5, group: 'metrics'},
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are we generally still going to try to stick to the "ship in a release without weight" before we can ship in a breaking change with weight in the future/do we still plan to do that here with cherry-picking? or is this our new policy moving forward?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally, the ideal flow for introducing a new metric is:

  1. add metric (invisible or visible)
  2. cut a minor release
  3. Do these ~simultaneously:
    • gather HA/LR data to inform calibration
    • if not already: flip metric to visible & cut a minor release, then....
      • ship minor release to increase user familiarity with the metric.
  4. if necessary, recalibrate score curve
  5. add weighting
  6. optionally, ship a beta
  7. ship a major rls

while we didnt get to follow this pattern here, i'd like to use that plan going forward. hows that sound?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great! yeah agreed.

I still think we can follow part of that pattern here if we wanted to cherry-pick those metrics into a 5.7.0 or something, so folks can play with them without jumping to a new major. I guess that was the root of my question that just was leading in an unintended direction :)

// intentionally left out of metrics group so they won't be displayed
{id: 'first-cpu-idle', weight: 0},
{id: 'max-potential-fid', weight: 0},
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so this is being killed afterall? I thought we brought up in a recent LH meeting that this was going to stay even if it wasn't weighted but the discussion in the doc suggests this is gone gone.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's still in the JSON but not visible anymore.

We've long wanted to add Top Long Tasks as a diagnostic and I think that will end up being more helpful than keeping the mpFid metric.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's still in the JSON but not visible anymore.

right, but with the intention of removing from the JSON in the future?

I know the support was overwhelming for killing in the doc discussion but I still think it's going to be really unfortunate to have no FID-like equivalent for lab when it's like half our performance story in field.

Copy link
Collaborator

@connorjclark connorjclark Mar 10, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aside: i wouldn't want to remove this from json. but further I'd prefer a average FID metric (instead of just a max one - actually this is just EIL)

{id: 'first-meaningful-paint', weight: 0},
{id: 'estimated-input-latency', weight: 0},

{id: 'render-blocking-resources', weight: 0, group: 'load-opportunities'},
{id: 'uses-responsive-images', weight: 0, group: 'load-opportunities'},
{id: 'offscreen-images', weight: 0, group: 'load-opportunities'},
Expand Down
5 changes: 3 additions & 2 deletions lighthouse-core/report/html/renderer/performance-category-renderer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,9 @@ class PerformanceCategoryRenderer extends CategoryRenderer {
metricAuditsEl.append(..._toggleEl.childNodes);

const metricAudits = category.auditRefs.filter(audit => audit.group === 'metrics');
const keyMetrics = metricAudits.filter(a => a.weight >= 3);
const otherMetrics = metricAudits.filter(a => a.weight < 3);

const keyMetrics = metricAudits.slice(0, 3);
const otherMetrics = metricAudits.slice(3);

const metricsBoxesEl = this.dom.createChildOf(metricAuditsEl, 'div', 'lh-columns');
const metricsColumn1El = this.dom.createChildOf(metricsBoxesEl, 'div', 'lh-column');
Expand Down
83 changes: 41 additions & 42 deletions lighthouse-core/test/results/sample_v2.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,16 +81,6 @@
"numericUnit": "millisecond",
"displayValue": "4.0 s"
},
"first-meaningful-paint": {
"id": "first-meaningful-paint",
"title": "First Meaningful Paint",
"description": "First Meaningful Paint measures when the primary content of a page is visible. [Learn more](https://web.dev/first-meaningful-paint).",
"score": 0.51,
"scoreDisplayMode": "numeric",
"numericValue": 3969.136,
"numericUnit": "millisecond",
"displayValue": "4.0 s"
},
"largest-contentful-paint": {
"id": "largest-contentful-paint",
"title": "Largest Contentful Paint",
Expand All @@ -101,6 +91,16 @@
"numericUnit": "millisecond",
"displayValue": "4.9 s"
},
"first-meaningful-paint": {
"id": "first-meaningful-paint",
"title": "First Meaningful Paint",
"description": "First Meaningful Paint measures when the primary content of a page is visible. [Learn more](https://web.dev/first-meaningful-paint).",
"score": 0.51,
"scoreDisplayMode": "numeric",
"numericValue": 3969.136,
"numericUnit": "millisecond",
"displayValue": "4.0 s"
},
"load-fast-enough-for-pwa": {
"id": "load-fast-enough-for-pwa",
"title": "Page load is fast enough on mobile networks",
Expand Down Expand Up @@ -3639,49 +3639,48 @@
"auditRefs": [
{
"id": "first-contentful-paint",
"weight": 3,
"weight": 15,
"group": "metrics"
},
{
"id": "first-meaningful-paint",
"weight": 1,
"id": "speed-index",
"weight": 15,
"group": "metrics"
},
{
"id": "largest-contentful-paint",
"weight": 0,
"weight": 25,
"group": "metrics"
},
{
"id": "speed-index",
"weight": 4,
"id": "interactive",
"weight": 15,
"group": "metrics"
},
{
"id": "interactive",
"weight": 5,
"id": "total-blocking-time",
"weight": 25,
"group": "metrics"
},
{
"id": "first-cpu-idle",
"weight": 2,
"id": "cumulative-layout-shift",
"weight": 5,
"group": "metrics"
},
{
"id": "max-potential-fid",
"weight": 0,
"group": "metrics"
"id": "first-cpu-idle",
"weight": 0
},
{
"id": "cumulative-layout-shift",
"id": "max-potential-fid",
"weight": 0
},
{
"id": "estimated-input-latency",
"id": "first-meaningful-paint",
"weight": 0
},
{
"id": "total-blocking-time",
"id": "estimated-input-latency",
"weight": 0
},
{
Expand Down Expand Up @@ -3853,7 +3852,7 @@
}
],
"id": "performance",
"score": 0.69
"score": 0.66
},
"accessibility": {
"title": "Accessibility",
Expand Down Expand Up @@ -4525,25 +4524,25 @@
},
{
"startTime": 0,
"name": "lh:audit:first-meaningful-paint",
"name": "lh:audit:largest-contentful-paint",
"duration": 100,
"entryType": "measure"
},
{
"startTime": 0,
"name": "lh:computed:FirstMeaningfulPaint",
"name": "lh:computed:LargestContentfulPaint",
"duration": 100,
"entryType": "measure"
},
{
"startTime": 0,
"name": "lh:audit:largest-contentful-paint",
"name": "lh:audit:first-meaningful-paint",
"duration": 100,
"entryType": "measure"
},
{
"startTime": 0,
"name": "lh:computed:LargestContentfulPaint",
"name": "lh:computed:FirstMeaningfulPaint",
"duration": 100,
"entryType": "measure"
},
Expand Down Expand Up @@ -5632,15 +5631,15 @@
},
{
"values": {
"timeInMs": 3969.136
"timeInMs": 4927.278
},
"path": "audits[first-meaningful-paint].displayValue"
"path": "audits[largest-contentful-paint].displayValue"
},
{
"values": {
"timeInMs": 4927.278
"timeInMs": 3969.136
},
"path": "audits[largest-contentful-paint].displayValue"
"path": "audits[first-meaningful-paint].displayValue"
},
{
"values": {
Expand Down Expand Up @@ -5673,19 +5672,19 @@
"path": "audits[bootup-time].displayValue"
}
],
"lighthouse-core/audits/metrics/largest-contentful-paint.js | title": [
"audits[largest-contentful-paint].title"
],
"lighthouse-core/audits/metrics/largest-contentful-paint.js | description": [
"audits[largest-contentful-paint].description"
],
"lighthouse-core/lib/i18n/i18n.js | firstMeaningfulPaintMetric": [
"audits[first-meaningful-paint].title",
"audits[timing-budget].details.items[2].label"
],
"lighthouse-core/audits/metrics/first-meaningful-paint.js | description": [
"audits[first-meaningful-paint].description"
],
"lighthouse-core/audits/metrics/largest-contentful-paint.js | title": [
"audits[largest-contentful-paint].title"
],
"lighthouse-core/audits/metrics/largest-contentful-paint.js | description": [
"audits[largest-contentful-paint].description"
],
"lighthouse-core/audits/load-fast-enough-for-pwa.js | title": [
"audits[load-fast-enough-for-pwa].title"
],
Expand Down Expand Up @@ -6970,4 +6969,4 @@
}
}
]
}
}
41 changes: 20 additions & 21 deletions proto/sample_v2_round_trip.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3798,48 +3798,47 @@
{
"group": "metrics",
"id": "first-contentful-paint",
"weight": 3.0
"weight": 15.0
},
{
"group": "metrics",
"id": "first-meaningful-paint",
"weight": 1.0
"id": "speed-index",
"weight": 15.0
},
{
"group": "metrics",
"id": "largest-contentful-paint",
"weight": 0.0
"weight": 25.0
},
{
"group": "metrics",
"id": "speed-index",
"weight": 4.0
"id": "interactive",
"weight": 15.0
},
{
"group": "metrics",
"id": "interactive",
"weight": 5.0
"id": "total-blocking-time",
"weight": 25.0
},
{
"group": "metrics",
"id": "first-cpu-idle",
"weight": 2.0
"id": "cumulative-layout-shift",
"weight": 5.0
},
{
"group": "metrics",
"id": "max-potential-fid",
"id": "first-cpu-idle",
"weight": 0.0
},
{
"id": "cumulative-layout-shift",
"id": "max-potential-fid",
"weight": 0.0
},
{
"id": "estimated-input-latency",
"id": "first-meaningful-paint",
"weight": 0.0
},
{
"id": "total-blocking-time",
"id": "estimated-input-latency",
"weight": 0.0
},
{
Expand Down Expand Up @@ -4011,7 +4010,7 @@
}
],
"id": "performance",
"score": 0.69,
"score": 0.66,
"title": "Performance"
},
"pwa": {
Expand Down Expand Up @@ -4441,25 +4440,25 @@
{
"duration": 100.0,
"entryType": "measure",
"name": "lh:audit:first-meaningful-paint",
"name": "lh:audit:largest-contentful-paint",
"startTime": 0.0
},
{
"duration": 100.0,
"entryType": "measure",
"name": "lh:computed:FirstMeaningfulPaint",
"name": "lh:computed:LargestContentfulPaint",
"startTime": 0.0
},
{
"duration": 100.0,
"entryType": "measure",
"name": "lh:audit:largest-contentful-paint",
"name": "lh:audit:first-meaningful-paint",
"startTime": 0.0
},
{
"duration": 100.0,
"entryType": "measure",
"name": "lh:computed:LargestContentfulPaint",
"name": "lh:computed:FirstMeaningfulPaint",
"startTime": 0.0
},
{
Expand Down Expand Up @@ -5438,4 +5437,4 @@
"total": 12345.6789
},
"userAgent": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_3) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/66.0.3358.0 Safari/537.36"
}
}