-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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.
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
new_audit: add cumulative layout shift metric #9037
Changes from 30 commits
0fe7bbb
c4b153d
eab48c8
a825fc4
9c7c148
14a6228
8897105
8e5eceb
8ccafa1
54adb9f
8c4bbf1
09cb0f0
2f71438
396ad39
54bf68c
eac986c
17ec87a
6b9a996
1374765
aaab275
79b2c4f
66687e9
25f56f6
f234753
12f4197
4b4387c
c35ed5a
efb69e6
d09fbb3
9542b6c
b525650
8be4c07
d625e4c
8e8782f
30f77ac
7b7ae5e
2dad8a8
1e52204
61de864
7b0865d
811f170
d9ea769
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,81 @@ | ||
/** | ||
* @license Copyright 2019 Google Inc. All Rights Reserved. | ||
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 | ||
* Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. | ||
*/ | ||
'use strict'; | ||
|
||
const Audit = require('../audit.js'); | ||
const ComputedCLS = require('../../computed/metrics/cumulative-layout-shift.js'); | ||
const i18n = require('../../lib/i18n/i18n.js'); | ||
|
||
const UIStrings = { | ||
/** The name of the metric "Cumulative Layout Shift" that indicates how much the page changes its layout while it loads. If big segments of the page shift their location during load, the Cumulative Layout Shift will be higher. Shown to users as the label for the numeric metric value. Ideally fits within a ~40 character limit. */ | ||
title: 'Cumulative Layout Shift', | ||
// TODO(paulirish): improve this description. | ||
/** Description of the Cumulative Layout Shift metric that indicates how much the page changes its layout while it loads. If big segments of the page shift their location during load, the Cumulative Layout Shift will be higher. This description is displayed within a tooltip when the user hovers on the metric name to see more. No character length limits. 'Learn More' becomes link text to additional documentation. */ | ||
description: 'The more the page\'s layout changes during its load, the higher the ' + | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The pattern we use is " ." So I took a stab at it. "Cumulative Layout Shift is the sum of all layout shifts that occurred during a page's load. A layout shift is any movement an element makes once it is visible to the user. All layout shift is recorded, scored, and then aggregated into a cumulative score between 0 and 1; 0 being a perfectly stable page, and >=0.5 being a highly shifting page." |
||
'Cumulative Layout Shift. ' + | ||
'Perfectly solid == 0. Unpleasant experience >= 0.50.', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it feels like this should be scaled somehow... is it possible for there to ever be some human interpretable understanding of this? like a scale such that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe we need to leave a TODO in here for improving this description, too? :) |
||
}; | ||
|
||
const str_ = i18n.createMessageInstanceIdFn(__filename, UIStrings); | ||
|
||
/** | ||
* @fileoverview This metric represents the amount of visual shifting of DOM elements during page load. | ||
*/ | ||
class CumulativeLayoutShift extends Audit { | ||
/** | ||
* @return {LH.Audit.Meta} | ||
*/ | ||
static get meta() { | ||
return { | ||
id: 'cumulative-layout-shift', | ||
title: str_(UIStrings.title), | ||
description: str_(UIStrings.description), | ||
scoreDisplayMode: Audit.SCORING_MODES.NUMERIC, | ||
requiredArtifacts: ['traces'], | ||
}; | ||
} | ||
|
||
/** | ||
* @return {LH.Audit.ScoreOptions} | ||
*/ | ||
static get defaultOptions() { | ||
return { | ||
// TODO(paulirish): Calibrate these | ||
scorePODR: 0.1, | ||
scoreMedian: 0.5, | ||
}; | ||
} | ||
|
||
/** | ||
* @param {LH.Artifacts} artifacts | ||
* @param {LH.Audit.Context} context | ||
* @return {Promise<LH.Audit.Product>} | ||
*/ | ||
static async audit(artifacts, context) { | ||
const trace = artifacts.traces[Audit.DEFAULT_PASS]; | ||
const metricResult = await ComputedCLS.request(trace, context); | ||
|
||
/** @type {LH.Audit.Details.DebugData} */ | ||
const details = { | ||
type: 'debugdata', | ||
items: [metricResult.debugInfo], | ||
}; | ||
|
||
return { | ||
score: Audit.computeLogNormalScore( | ||
metricResult.value, | ||
context.options.scorePODR, | ||
context.options.scoreMedian | ||
), | ||
numericValue: metricResult.value, | ||
displayValue: metricResult.value.toLocaleString(context.settings.locale), | ||
details, | ||
}; | ||
} | ||
} | ||
|
||
module.exports = CumulativeLayoutShift; | ||
module.exports.UIStrings = UIStrings; |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,61 @@ | ||||||
/** | ||||||
* @license Copyright 2019 Google Inc. All Rights Reserved. | ||||||
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 | ||||||
* Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. | ||||||
*/ | ||||||
'use strict'; | ||||||
|
||||||
const makeComputedArtifact = require('../computed-artifact.js'); | ||||||
const TraceOfTab = require('../trace-of-tab.js'); | ||||||
const LHError = require('../../lib/lh-error.js'); | ||||||
|
||||||
class CumulativeLayoutShift { | ||||||
/** | ||||||
* @param {LH.Trace} trace | ||||||
* @param {LH.Audit.Context} context | ||||||
* @return {Promise<{value: number, debugInfo: Record<string,boolean> | null}>} | ||||||
*/ | ||||||
static async compute_(trace, context) { | ||||||
const traceOfTab = await TraceOfTab.request(trace, context); | ||||||
|
||||||
// Find the last LayoutShift event, if any. | ||||||
let finalLayoutShift; | ||||||
for (let i = traceOfTab.mainThreadEvents.length - 1; i >= 0; i--) { | ||||||
const evt = traceOfTab.mainThreadEvents[i]; | ||||||
if (evt.name === 'LayoutShift' && evt.args && evt.args.data && evt.args.data.is_main_frame) { | ||||||
finalLayoutShift = evt; | ||||||
break; | ||||||
} | ||||||
} | ||||||
|
||||||
const finalLayoutShiftTraceEventFound = !!finalLayoutShift; | ||||||
// tdresser sez: In about 10% of cases, layout instability is 0, and there will be no trace events. | ||||||
// TODO: Validate that. http://crbug.com/1003459 | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This bug was fixed, is this still valid?
Suggested change
|
||||||
if (!finalLayoutShift) { | ||||||
return { | ||||||
value: 0, | ||||||
debugInfo: { | ||||||
finalLayoutShiftTraceEventFound, | ||||||
}, | ||||||
}; | ||||||
} | ||||||
|
||||||
const cumulativeLayoutShift = | ||||||
finalLayoutShift.args && | ||||||
finalLayoutShift.args.data && | ||||||
finalLayoutShift.args.data.cumulative_score; | ||||||
|
||||||
if (cumulativeLayoutShift === undefined) { | ||||||
throw new LHError(LHError.errors.LAYOUT_SHIFT_MISSING_DATA); | ||||||
} | ||||||
|
||||||
return { | ||||||
value: cumulativeLayoutShift, | ||||||
debugInfo: { | ||||||
finalLayoutShiftTraceEventFound, | ||||||
}, | ||||||
}; | ||||||
} | ||||||
} | ||||||
|
||||||
module.exports = makeComputedArtifact(CumulativeLayoutShift); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -100,6 +100,9 @@ class Driver { | |
// Used instead of 'toplevel' in Chrome 71+ | ||
'disabled-by-default-lighthouse', | ||
|
||
// Used for Cumulative Layout Shift metric | ||
'loading', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. how beefy is this category? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why you hoarding this script paul There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. /* eslint-disable */
'use strict';
const trace = require('../latest-run/defaultPass.trace.json');
const {traceCategories} = require('../lighthouse-core/gather/driver');
const traceCats = {};
// aggregate
let totalBytes = 0;
let totalEvents = 0;
trace.traceEvents.forEach(e => {
let eventCats = e.cat;
for (let eventCat of eventCats.split(',')) {
// don't process cats we dont trace
// if (!traceCategories.includes(eventCat)) return;
if (e.name === 'ThreadControllerImpl::RunTask') eventCat += '::::::::RunTask';
const cat = traceCats[eventCat] || {bytes: 0, events: 0};
const bytes = JSON.stringify(e).length;
cat.bytes += bytes;
totalBytes += bytes;
cat.events += 1;
totalEvents += 1;
traceCats[eventCat] = cat;
}
});
// obj to array
const traceTotals = [];
Object.keys(traceCats).forEach(catname => {
const cat = traceCats[catname];
traceTotals.push({name: catname, bytes: cat.bytes, events: cat.events});
});
// sort and log
console.log('Bytes'.padStart(16), '\t', 'Count'.padStart(7), '\t', 'Event Name'.padStart(18))
traceTotals.sort((a, b) => b.bytes - a.bytes).forEach((tot, i) => {
console.log(
tot.bytes.toLocaleString().padStart(15),
`${(tot.bytes * 100/ totalBytes).toLocaleString(undefined, {maximumFractionDigits: 1})}%`.padStart(6),
'\t',
tot.events.toString().padStart(9),
`${(tot.events * 100/ totalEvents).toLocaleString(undefined, {maximumFractionDigits: 1})}%`.padStart(6),
'\t',
tot.name
);
})
|
||
|
||
// All compile/execute events are captured by parent events in devtools.timeline.. | ||
// But the v8 category provides some nice context for only <0.5% of the trace size | ||
'v8', | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -254,6 +254,11 @@ const ERRORS = { | |
code: 'NO_LCP', | ||
message: UIStrings.badTraceRecording, | ||
}, | ||
/** Layout Shift trace events are found but without data */ | ||
LAYOUT_SHIFT_MISSING_DATA: { | ||
code: 'LAYOUT_SHIFT_MISSING_DATA', | ||
message: UIStrings.badTraceRecording, | ||
}, | ||
|
||
// TTI calculation failures | ||
FMP_TOO_LATE_FOR_FCPUI: {code: 'FMP_TOO_LATE_FOR_FCPUI', message: UIStrings.pageLoadTookTooLong}, | ||
|
@@ -376,6 +381,7 @@ const ERRORS = { | |
}, | ||
|
||
// Hey! When adding a new error type, update lighthouse-result.proto too. | ||
// Only necessary for runtime errors, which come from artifacts or pageLoadErrors. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So this error will never be a runtime error, correct? Or it won't while it is a silent audit and not part of performance? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i wrote this comment idk how it got here in pauls pr :P There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "this" error? This comment is a continuation of the previous line, and is referring to when proto must be edited. All of these errors are runtime errors. |
||
}; | ||
|
||
/** @type {Record<keyof typeof ERRORS, LighthouseErrorDefinition>} */ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,6 +18,7 @@ describe('Timing summary', () => { | |
|
||
expect(result.metrics).toMatchInlineSnapshot(` | ||
Object { | ||
"cumulativeLayoutShift": undefined, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. would be nice to have at least one trace with this event in there as a test that isn't sample-json There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good call. i got this pending in a followup to simplify. |
||
"estimatedInputLatency": 77.79999999999995, | ||
"estimatedInputLatencyTs": undefined, | ||
"firstCPUIdle": 3351.3320000492968, | ||
|
@@ -31,6 +32,8 @@ Object { | |
"largestContentfulPaint": undefined, | ||
"largestContentfulPaintTs": undefined, | ||
"maxPotentialFID": 396.0000000000001, | ||
"observedCumulativeLayoutShift": undefined, | ||
"observedCumulativeLayoutShiftTs": undefined, | ||
"observedDomContentLoaded": 560.294, | ||
"observedDomContentLoadedTs": 225414732309, | ||
"observedFirstContentfulPaint": 498.87, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
poke
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we dont have any additional data right now in order to be more concrete than this.
the TODO is to be addressed before shipping 6.0