From 4dfae250e76f4db02647ce84c2b78a583dd80d8e Mon Sep 17 00:00:00 2001 From: Deepanjan Roy Date: Thu, 18 Jul 2019 14:22:46 -0400 Subject: [PATCH 1/5] Clean up total blocking time code --- .../test/cli/__snapshots__/index-test.js.snap | 4 +- lighthouse-core/audits/metrics.js | 8 +- ...ueuing-delay.js => total-blocking-time.js} | 42 +++++----- ...elay.js => lantern-total-blocking-time.js} | 22 +++--- ...ueuing-delay.js => total-blocking-time.js} | 64 ++++++++------- lighthouse-core/config/default-config.js | 4 +- lighthouse-core/lib/i18n/en-US.json | 8 ++ lighthouse-core/lib/i18n/locales/en-XL.json | 6 ++ .../audits/__snapshots__/metrics-test.js.snap | 2 +- ...ay-test.js => total-blocking-time-test.js} | 18 ++--- ...ay-test.js => total-blocking-time-test.js} | 77 +++++++------------ lighthouse-core/test/results/sample_v2.json | 28 +++++-- proto/sample_v2_round_trip.json | 28 +++---- 13 files changed, 160 insertions(+), 151 deletions(-) rename lighthouse-core/audits/metrics/{cumulative-long-queuing-delay.js => total-blocking-time.js} (53%) rename lighthouse-core/computed/metrics/{lantern-cumulative-long-queuing-delay.js => lantern-total-blocking-time.js} (82%) rename lighthouse-core/computed/metrics/{cumulative-long-queuing-delay.js => total-blocking-time.js} (60%) rename lighthouse-core/test/audits/metrics/{cumulative-long-queuing-delay-test.js => total-blocking-time-test.js} (64%) rename lighthouse-core/test/computed/metrics/{cumulative-long-queuing-delay-test.js => total-blocking-time-test.js} (58%) diff --git a/lighthouse-cli/test/cli/__snapshots__/index-test.js.snap b/lighthouse-cli/test/cli/__snapshots__/index-test.js.snap index 06c58615e47c..1478da3da54a 100644 --- a/lighthouse-cli/test/cli/__snapshots__/index-test.js.snap +++ b/lighthouse-cli/test/cli/__snapshots__/index-test.js.snap @@ -43,7 +43,7 @@ Object { "path": "metrics/estimated-input-latency", }, Object { - "path": "metrics/cumulative-long-queuing-delay", + "path": "metrics/total-blocking-time", }, Object { "path": "metrics/max-potential-fid", @@ -717,7 +717,7 @@ Object { "weight": 0, }, Object { - "id": "cumulative-long-queuing-delay", + "id": "total-blocking-time", "weight": 0, }, Object { diff --git a/lighthouse-core/audits/metrics.js b/lighthouse-core/audits/metrics.js index 374517c2137a..98a50d46e7c5 100644 --- a/lighthouse-core/audits/metrics.js +++ b/lighthouse-core/audits/metrics.js @@ -14,7 +14,7 @@ const FirstCPUIdle = require('../computed/metrics/first-cpu-idle.js'); const Interactive = require('../computed/metrics/interactive.js'); const SpeedIndex = require('../computed/metrics/speed-index.js'); const EstimatedInputLatency = require('../computed/metrics/estimated-input-latency.js'); -const CumulativeLongQueuingDelay = require('../computed/metrics/cumulative-long-queuing-delay.js'); +const TotalBlockingTime = require('../computed/metrics/total-blocking-time.js'); class Metrics extends Audit { /** @@ -60,7 +60,7 @@ class Metrics extends Audit { const interactive = await requestOrUndefined(Interactive, metricComputationData); const speedIndex = await requestOrUndefined(SpeedIndex, metricComputationData); const estimatedInputLatency = await EstimatedInputLatency.request(metricComputationData, context); // eslint-disable-line max-len - const cumulativeLongQueuingDelay = await CumulativeLongQueuingDelay.request(metricComputationData, context); // eslint-disable-line max-len + const totalBlockingTime = await TotalBlockingTime.request(metricComputationData, context); // eslint-disable-line max-len /** @type {UberMetricsItem} */ const metrics = { @@ -77,7 +77,7 @@ class Metrics extends Audit { speedIndexTs: speedIndex && speedIndex.timestamp, estimatedInputLatency: estimatedInputLatency.timing, estimatedInputLatencyTs: estimatedInputLatency.timestamp, - cumulativeLongQueuingDelay: cumulativeLongQueuingDelay.timing, + totalBlockingTime: totalBlockingTime.timing, // Include all timestamps of interest from trace of tab observedNavigationStart: traceOfTab.timings.navigationStart, @@ -140,7 +140,7 @@ class Metrics extends Audit { * @property {number=} speedIndexTs * @property {number} estimatedInputLatency * @property {number=} estimatedInputLatencyTs - * @property {number} cumulativeLongQueuingDelay + * @property {number} totalBlockingTime * @property {number} observedNavigationStart * @property {number} observedNavigationStartTs * @property {number=} observedFirstPaint diff --git a/lighthouse-core/audits/metrics/cumulative-long-queuing-delay.js b/lighthouse-core/audits/metrics/total-blocking-time.js similarity index 53% rename from lighthouse-core/audits/metrics/cumulative-long-queuing-delay.js rename to lighthouse-core/audits/metrics/total-blocking-time.js index dd0297ad8da6..a741264c9618 100644 --- a/lighthouse-core/audits/metrics/cumulative-long-queuing-delay.js +++ b/lighthouse-core/audits/metrics/total-blocking-time.js @@ -6,24 +6,28 @@ 'use strict'; const Audit = require('../audit.js'); -const CumulativeLQD = require('../../computed/metrics/cumulative-long-queuing-delay.js'); +const ComputedTBT = require('../../computed/metrics/total-blocking-time.js'); +const i18n = require('../../lib/i18n/i18n.js'); -// TODO(deepanjanroy): i18n strings once metric is final. -const UIStringsNotExported = { - title: 'Cumulative Long Queuing Delay', - description: '[Experimental metric] Total time period between FCP and Time to Interactive ' + - 'during which queuing time for any input event would be higher than 50ms.', +const UIStrings = { + /** The name of the metric that calculates the total duration of blocking time for a page. Blocking times are time periods when the page would be slow at responding to user input (clicks, taps, and keypresses will feel janky). Shown to users as the label for the numeric metric value. Ideally fits within a ~40 character limit. */ + title: 'Total Blocking Time', + /** Description of the Total Blocking Time (TBT) metric, which calculates the total duration of blocking time for a page. Blocking times are times when if a page received any input (clicks, taps, and keypresses), the page would be slow at responding. This is displayed within a tooltip when the user hovers on the metric name to see more. No character length limits.*/ + description: 'Sum of all time periods between FCP and Time to Interactive, ' + + 'when task length exceeded by more than 50ms, expressed in milliseconds.', }; -class CumulativeLongQueuingDelay extends Audit { +const str_ = i18n.createMessageInstanceIdFn(__filename, UIStrings); + +class TotalBlockingTime extends Audit { /** * @return {LH.Audit.Meta} */ static get meta() { return { - id: 'cumulative-long-queuing-delay', - title: UIStringsNotExported.title, - description: UIStringsNotExported.description, + id: 'total-blocking-time', + title: str_(UIStrings.title), + description: str_(UIStrings.description), scoreDisplayMode: Audit.SCORING_MODES.NUMERIC, requiredArtifacts: ['traces', 'devtoolsLogs'], }; @@ -46,13 +50,12 @@ class CumulativeLongQueuingDelay extends Audit { } /** - * Audits the page to calculate Cumulative Long Queuing Delay. + * Audits the page to calculate Total Blocking Time. * - * We define Long Queuing Delay Region as any time interval in the loading timeline where queuing - * time for an input event would be longer than 50ms. For example, if there is a 110ms main thread - * task, the first 60ms of it is Long Queuing Delay Region, because any input event occuring in - * that region has to wait more than 50ms. Cumulative Long Queuing Delay is the sum of all Long - * Queuing Delay Regions between First Contentful Paint and Interactive Time (TTI). + * We define Blocking Time as any time interval in the loading timeline where task length exceeds + * 50ms. For example, if there is a 110ms main thread task, the last 60ms of it is blocking time. + * Total Blocking Time is the sum of all Blocking Time between First Contentful Paint and + * Interactive Time (TTI). * * @param {LH.Artifacts} artifacts * @param {LH.Audit.Context} context @@ -62,7 +65,7 @@ class CumulativeLongQueuingDelay extends Audit { const trace = artifacts.traces[Audit.DEFAULT_PASS]; const devtoolsLog = artifacts.devtoolsLogs[Audit.DEFAULT_PASS]; const metricComputationData = {trace, devtoolsLog, settings: context.settings}; - const metricResult = await CumulativeLQD.request(metricComputationData, context); + const metricResult = await ComputedTBT.request(metricComputationData, context); return { score: Audit.computeLogNormalScore( @@ -71,9 +74,10 @@ class CumulativeLongQueuingDelay extends Audit { context.options.scoreMedian ), numericValue: metricResult.timing, - displayValue: 10 * Math.round(metricResult.timing / 10) + '\xa0ms', + displayValue: str_(i18n.UIStrings.ms, {timeInMs: metricResult.timing}), }; } } -module.exports = CumulativeLongQueuingDelay; +module.exports = TotalBlockingTime; +module.exports.UIStrings = UIStrings; diff --git a/lighthouse-core/computed/metrics/lantern-cumulative-long-queuing-delay.js b/lighthouse-core/computed/metrics/lantern-total-blocking-time.js similarity index 82% rename from lighthouse-core/computed/metrics/lantern-cumulative-long-queuing-delay.js rename to lighthouse-core/computed/metrics/lantern-total-blocking-time.js index 112b3539fb57..214f5c4259d5 100644 --- a/lighthouse-core/computed/metrics/lantern-cumulative-long-queuing-delay.js +++ b/lighthouse-core/computed/metrics/lantern-total-blocking-time.js @@ -13,7 +13,7 @@ const LanternInteractive = require('./lantern-interactive.js'); /** @typedef {BaseNode.Node} Node */ -class LanternCumulativeLongQueuingDelay extends LanternMetric { +class LanternTotalBlockingTime extends LanternMetric { /** * @return {LH.Gatherer.Simulation.MetricCoefficients} */ @@ -48,32 +48,32 @@ class LanternCumulativeLongQueuingDelay extends LanternMetric { */ static getEstimateFromSimulation(simulation, extras) { // Intentionally use the opposite FCP estimate. A pessimistic FCP is higher than equal to an - // optimistic FCP, which means potentially more tasks are excluded from the - // CumulativeLongQueuingDelay computation. So a more pessimistic FCP gives a more optimistic - // CumulativeLongQueuingDelay for the same work. + // optimistic FCP, which means potentially more tasks are excluded from the Total Blocking Time + // computation. So a more pessimistic FCP gives a more optimistic Total Blocking Time for the + // same work. const fcpTimeInMs = extras.optimistic ? extras.fcpResult.pessimisticEstimate.timeInMs : extras.fcpResult.optimisticEstimate.timeInMs; // Similarly, we always have pessimistic TTI >= optimistic TTI. Therefore, picking optimistic // TTI means our window of interest is smaller and thus potentially more tasks are excluded from - // CumulativeLongQueuingDelay computation, yielding a lower (more optimistic) - // CumulativeLongQueuingDelay value for the same work. + // Total Blocking Time computation, yielding a lower (more optimistic) Total Blocking Time value + // for the same work. const interactiveTimeMs = extras.optimistic ? extras.interactiveResult.optimisticEstimate.timeInMs : extras.interactiveResult.pessimisticEstimate.timeInMs; // Require here to resolve circular dependency. - const CumulativeLongQueuingDelay = require('./cumulative-long-queuing-delay.js'); - const minDurationMs = CumulativeLongQueuingDelay.LONG_QUEUING_DELAY_THRESHOLD; + const TotalBlockingTime = require('./total-blocking-time.js'); + const minDurationMs = TotalBlockingTime.BLOCKING_TIME_THRESHOLD; - const events = LanternCumulativeLongQueuingDelay.getTopLevelEvents( + const events = LanternTotalBlockingTime.getTopLevelEvents( simulation.nodeTimings, minDurationMs ); return { - timeInMs: CumulativeLongQueuingDelay.calculateSumOfLongQueuingDelay( + timeInMs: TotalBlockingTime.calculateSumOfBlockingTime( events, fcpTimeInMs, interactiveTimeMs @@ -118,4 +118,4 @@ class LanternCumulativeLongQueuingDelay extends LanternMetric { } } -module.exports = makeComputedArtifact(LanternCumulativeLongQueuingDelay); +module.exports = makeComputedArtifact(LanternTotalBlockingTime); diff --git a/lighthouse-core/computed/metrics/cumulative-long-queuing-delay.js b/lighthouse-core/computed/metrics/total-blocking-time.js similarity index 60% rename from lighthouse-core/computed/metrics/cumulative-long-queuing-delay.js rename to lighthouse-core/computed/metrics/total-blocking-time.js index 18aa99d1561a..1364ec513cda 100644 --- a/lighthouse-core/computed/metrics/cumulative-long-queuing-delay.js +++ b/lighthouse-core/computed/metrics/total-blocking-time.js @@ -9,28 +9,27 @@ const makeComputedArtifact = require('../computed-artifact.js'); const ComputedMetric = require('./metric.js'); const LHError = require('../../lib/lh-error.js'); const TracingProcessor = require('../../lib/tracehouse/trace-processor.js'); -const LanternCumulativeLongQueuingDelay = require('./lantern-cumulative-long-queuing-delay.js'); +const LanternTotalBlockingTime = require('./lantern-total-blocking-time.js'); const TimetoInteractive = require('./interactive.js'); /** - * @fileoverview This audit determines Cumulative Long Queuing Delay between FCP and TTI. + * @fileoverview This audit determines Total Blocking Time. - * We define Long Queuing Delay Region as any time interval in the loading timeline where queuing - * time for an input event would be longer than 50ms. For example, if there is a 110ms main thread - * task, the first 60ms of it is Long Queuing Delay Region, because any input event occuring in - * that region has to wait more than 50ms. Cumulative Long Queuing Delay is the sum of all Long - * Queuing Delay Regions between First Contentful Paint and Interactive Time (TTI). + * We define Blocking Time as any time interval in the loading timeline where task length exceeds + * 50ms. For example, if there is a 110ms main thread task, the last 60ms of it is blocking time. + * Total Blocking Time is the sum of all Blocking Time between First Contentful Paint and + * Interactive Time (TTI). * * This is a new metric designed to accompany Time to Interactive. TTI is strict and does not * reflect incremental improvements to the site performance unless the improvement concerns the last - * long task. Cumulative Long Queuing Delay on the other hand is designed to be much more responsive + * long task. Total Blocking Time on the other hand is designed to be much more responsive * to smaller improvements to main thread responsiveness. */ -class CumulativeLongQueuingDelay extends ComputedMetric { +class TotalBlockingTime extends ComputedMetric { /** * @return {number} */ - static get LONG_QUEUING_DELAY_THRESHOLD() { + static get BLOCKING_TIME_THRESHOLD() { return 50; } /** @@ -39,30 +38,29 @@ class CumulativeLongQueuingDelay extends ComputedMetric { * @param {number} interactiveTimeMs * @return {number} */ - static calculateSumOfLongQueuingDelay(topLevelEvents, fcpTimeInMs, interactiveTimeMs) { + static calculateSumOfBlockingTime(topLevelEvents, fcpTimeInMs, interactiveTimeMs) { if (interactiveTimeMs <= fcpTimeInMs) return 0; - const threshold = CumulativeLongQueuingDelay.LONG_QUEUING_DELAY_THRESHOLD; - const longQueuingDelayRegions = []; - // First identifying the long queuing delay regions. + const threshold = TotalBlockingTime.BLOCKING_TIME_THRESHOLD; + const blockingRegions = []; for (const event of topLevelEvents) { - // If the task is less than the delay threshold, it contains no Long Queuing Delay Region. + // If the task is less than the delay threshold, it contains no Blocking Region. if (event.duration < threshold) continue; - // Otherwise, the duration of the task before the delay-threshold-sized interval at the end is - // considered Long Queuing Delay Region. Example assuming the threshold is 50ms: + // Otherwise, the duration of the task beyond blocking time threshold at the beginning is + // considered Blocking Region. Example assuming the threshold is 50ms: // [ 250ms Task ] - // | Long Queuing Delay Region | Last 50ms | - // 200 ms - longQueuingDelayRegions.push({ - start: event.start, - end: event.end - threshold, + // | First 50ms | Blocking Time Region | + // 200 ms + blockingRegions.push({ + start: event.start + threshold, + end: event.end, duration: event.duration - threshold, }); } - let sumLongQueuingDelay = 0; - for (const region of longQueuingDelayRegions) { - // We only want to add up the Long Queuing Delay regions that fall between FCP and TTI. + let sumBlockingTime = 0; + for (const region of blockingRegions) { + // We only want to add up the Blocking Time Regions that fall between FCP and TTI. // // FCP is picked as the lower bound because there is little risk of user input happening // before FCP so Long Queuing Qelay regions do not harm user experience. Developers should be @@ -73,16 +71,16 @@ class CumulativeLongQueuingDelay extends ComputedMetric { if (region.end < fcpTimeInMs) continue; if (region.start > interactiveTimeMs) continue; - // If a Long Queuing Delay Region spans the edges of our region of interest, we clip it to - // only include the part of the region that falls inside. + // If a Blocking Time Region spans the edges of our region of interest, we clip it to only + // include the part of the region that falls inside. const clippedStart = Math.max(region.start, fcpTimeInMs); const clippedEnd = Math.min(region.end, interactiveTimeMs); - const queuingDelayAfterClipping = clippedEnd - clippedStart; + const blockingTimeAfterClipping = clippedEnd - clippedStart; - sumLongQueuingDelay += queuingDelayAfterClipping; + sumBlockingTime += blockingTimeAfterClipping; } - return sumLongQueuingDelay; + return sumBlockingTime; } /** @@ -91,7 +89,7 @@ class CumulativeLongQueuingDelay extends ComputedMetric { * @return {Promise} */ static computeSimulatedMetric(data, context) { - return LanternCumulativeLongQueuingDelay.request(data, context); + return LanternTotalBlockingTime.request(data, context); } /** @@ -112,7 +110,7 @@ class CumulativeLongQueuingDelay extends ComputedMetric { const events = TracingProcessor.getMainThreadTopLevelEvents(data.traceOfTab); return { - timing: CumulativeLongQueuingDelay.calculateSumOfLongQueuingDelay( + timing: TotalBlockingTime.calculateSumOfBlockingTime( events, firstContentfulPaint, interactiveTimeMs @@ -121,4 +119,4 @@ class CumulativeLongQueuingDelay extends ComputedMetric { } } -module.exports = makeComputedArtifact(CumulativeLongQueuingDelay); +module.exports = makeComputedArtifact(TotalBlockingTime); diff --git a/lighthouse-core/config/default-config.js b/lighthouse-core/config/default-config.js index 93ab96ddf671..77e94fdf54bf 100644 --- a/lighthouse-core/config/default-config.js +++ b/lighthouse-core/config/default-config.js @@ -171,7 +171,7 @@ const defaultConfig = { 'screenshot-thumbnails', 'final-screenshot', 'metrics/estimated-input-latency', - 'metrics/cumulative-long-queuing-delay', + 'metrics/total-blocking-time', 'metrics/max-potential-fid', 'errors-in-console', 'time-to-first-byte', @@ -369,7 +369,7 @@ const defaultConfig = { {id: 'first-cpu-idle', weight: 2, group: 'metrics'}, {id: 'max-potential-fid', weight: 0, group: 'metrics'}, {id: 'estimated-input-latency', weight: 0}, // intentionally left out of metrics so it won't be displayed - {id: 'cumulative-long-queuing-delay', 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: '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'}, diff --git a/lighthouse-core/lib/i18n/en-US.json b/lighthouse-core/lib/i18n/en-US.json index b0dd8ba42de2..e22ef2a1a191 100644 --- a/lighthouse-core/lib/i18n/en-US.json +++ b/lighthouse-core/lib/i18n/en-US.json @@ -1039,6 +1039,14 @@ "message": "Speed Index", "description": "The name of the metric that summarizes how quickly the page looked visually complete. The name of this metric is largely abstract and can be loosely translated. Shown to users as the label for the numeric metric value. Ideally fits within a ~40 character limit." }, + "lighthouse-core/audits/metrics/total-blocking-time.js | description": { + "message": "Sum of all time periods between FCP and Time to Interactive, when task length exceeded by more than 50ms, expressed in milliseconds.", + "description": "Description of the Total Blocking Time (TBT) metric, which calculates the total duration of blocking time for a page. Blocking times are times when if a page received any input (clicks, taps, and keypresses), the page would be slow at responding. This is displayed within a tooltip when the user hovers on the metric name to see more. No character length limits." + }, + "lighthouse-core/audits/metrics/total-blocking-time.js | title": { + "message": "Total Blocking Time", + "description": "The name of the metric that calculates the total duration of blocking time for a page. Blocking times are time periods when the page would be slow at responding to user input (clicks, taps, and keypresses will feel janky). Shown to users as the label for the numeric metric value. Ideally fits within a ~40 character limit." + }, "lighthouse-core/audits/network-rtt.js | description": { "message": "Network round trip times (RTT) have a large impact on performance. If the RTT to an origin is high, it's an indication that servers closer to the user could improve performance. [Learn more](https://hpbn.co/primer-on-latency-and-bandwidth/).", "description": "Description of a Lighthouse audit that tells the user that a high network round trip time (RTT) can effect their website's performance because the server is physically far away from them thus making the RTT high. This is displayed after a user expands the section to see more. No character length limits. 'Learn More' becomes link text to additional documentation." diff --git a/lighthouse-core/lib/i18n/locales/en-XL.json b/lighthouse-core/lib/i18n/locales/en-XL.json index 7759bc4cae61..df6e78095310 100644 --- a/lighthouse-core/lib/i18n/locales/en-XL.json +++ b/lighthouse-core/lib/i18n/locales/en-XL.json @@ -779,6 +779,12 @@ "lighthouse-core/audits/metrics/speed-index.js | title": { "message": "Ŝṕêéd̂ Ín̂d́êx́" }, + "lighthouse-core/audits/metrics/total-blocking-time.js | description": { + "message": "Ŝúm̂ óf̂ ál̂ĺ t̂ím̂é p̂ér̂íôd́ŝ b́êt́ŵéêń F̂ĆP̂ án̂d́ T̂ím̂é t̂ó Îńt̂ér̂áĉt́îv́ê, ẃĥén̂ t́âśk̂ ĺêńĝt́ĥ éx̂ćêéd̂éd̂ b́ŷ ḿôŕê t́ĥán̂ 50ḿŝ, éx̂ṕr̂éŝśêd́ îń m̂íl̂ĺîśêćôńd̂ś." + }, + "lighthouse-core/audits/metrics/total-blocking-time.js | title": { + "message": "T̂ót̂ál̂ B́l̂óĉḱîńĝ T́îḿê" + }, "lighthouse-core/audits/network-rtt.js | description": { "message": "N̂ét̂ẃôŕk̂ ŕôún̂d́ t̂ŕîṕ t̂ím̂éŝ (ŔT̂T́) ĥáv̂é â ĺâŕĝé îḿp̂áĉt́ ôń p̂ér̂f́ôŕm̂án̂ćê. Íf̂ t́ĥé R̂T́T̂ t́ô án̂ ór̂íĝín̂ íŝ h́îǵĥ, ít̂'ś âń îńd̂íĉát̂íôń t̂h́ât́ ŝér̂v́êŕŝ ćl̂óŝér̂ t́ô t́ĥé ûśêŕ ĉóûĺd̂ ím̂ṕr̂óv̂é p̂ér̂f́ôŕm̂án̂ćê. [Ĺêár̂ń m̂ór̂é](ĥt́t̂ṕŝ://h́p̂b́n̂.ćô/ṕr̂ím̂ér̂-ón̂-ĺât́êńĉý-âńd̂-b́âńd̂ẃîd́t̂h́/)." }, diff --git a/lighthouse-core/test/audits/__snapshots__/metrics-test.js.snap b/lighthouse-core/test/audits/__snapshots__/metrics-test.js.snap index 685d98388384..934dc6fa079d 100644 --- a/lighthouse-core/test/audits/__snapshots__/metrics-test.js.snap +++ b/lighthouse-core/test/audits/__snapshots__/metrics-test.js.snap @@ -2,7 +2,6 @@ exports[`Performance: metrics evaluates valid input correctly 1`] = ` Object { - "cumulativeLongQueuingDelay": 748, "estimatedInputLatency": 78, "estimatedInputLatencyTs": undefined, "firstCPUIdle": 3351, @@ -35,5 +34,6 @@ Object { "observedTraceEndTs": 225426711887, "speedIndex": 1657, "speedIndexTs": undefined, + "totalBlockingTime": 776, } `; diff --git a/lighthouse-core/test/audits/metrics/cumulative-long-queuing-delay-test.js b/lighthouse-core/test/audits/metrics/total-blocking-time-test.js similarity index 64% rename from lighthouse-core/test/audits/metrics/cumulative-long-queuing-delay-test.js rename to lighthouse-core/test/audits/metrics/total-blocking-time-test.js index 160322069358..220683bc4a26 100644 --- a/lighthouse-core/test/audits/metrics/cumulative-long-queuing-delay-test.js +++ b/lighthouse-core/test/audits/metrics/total-blocking-time-test.js @@ -5,28 +5,28 @@ */ 'use strict'; -const cLQDAudit = require('../../../audits/metrics/cumulative-long-queuing-delay.js'); -const options = cLQDAudit.defaultOptions; +const TBTAudit = require('../../../audits/metrics/total-blocking-time.js'); +const options = TBTAudit.defaultOptions; const pwaTrace = require('../../fixtures/traces/progressive-app-m60.json'); function generateArtifactsWithTrace(trace) { return { - traces: {[cLQDAudit.DEFAULT_PASS]: trace}, - devtoolsLogs: {[cLQDAudit.DEFAULT_PASS]: []}, + traces: {[TBTAudit.DEFAULT_PASS]: trace}, + devtoolsLogs: {[TBTAudit.DEFAULT_PASS]: []}, }; } /* eslint-env jest */ -describe('Performance: cumulative-long-queuing-delay audit', () => { - it('evaluates cumulative long queuing delay metric properly', async () => { +describe('Performance: total-blocking-time audit', () => { + it('evaluates Total Blocking Time metric properly', async () => { const artifacts = generateArtifactsWithTrace(pwaTrace); const settings = {throttlingMethod: 'provided'}; const context = {options, settings, computedCache: new Map()}; - const output = await cLQDAudit.audit(artifacts, context); + const output = await TBTAudit.audit(artifacts, context); - expect(output.numericValue).toBeCloseTo(48.3, 1); + expect(output.numericValue).toBeCloseTo(57.5, 1); expect(output.score).toBe(1); - expect(output.displayValue).toBeDisplayString('50\xa0ms'); + expect(output.displayValue).toBeDisplayString('60\xa0ms'); }); }); diff --git a/lighthouse-core/test/computed/metrics/cumulative-long-queuing-delay-test.js b/lighthouse-core/test/computed/metrics/total-blocking-time-test.js similarity index 58% rename from lighthouse-core/test/computed/metrics/cumulative-long-queuing-delay-test.js rename to lighthouse-core/test/computed/metrics/total-blocking-time-test.js index 8b02ada00c13..d2a544acf6ec 100644 --- a/lighthouse-core/test/computed/metrics/cumulative-long-queuing-delay-test.js +++ b/lighthouse-core/test/computed/metrics/total-blocking-time-test.js @@ -5,21 +5,17 @@ */ 'use strict'; -const CumulativeLongQueuingDelay = - require('../../../computed/metrics/cumulative-long-queuing-delay.js'); +const TotalBlockingTime = require('../../../computed/metrics/total-blocking-time.js'); const trace = require('../../fixtures/traces/progressive-app-m60.json'); const devtoolsLog = require('../../fixtures/traces/progressive-app-m60.devtools.log.json'); /* eslint-env jest */ -describe('Metrics: CumulativeLongQueuingDelay', () => { +describe('Metrics: TotalBlockingTime', () => { it('should compute a simulated value', async () => { const settings = {throttlingMethod: 'simulate'}; const context = {settings, computedCache: new Map()}; - const result = await CumulativeLongQueuingDelay.request( - {trace, devtoolsLog, settings}, - context - ); + const result = await TotalBlockingTime.request({trace, devtoolsLog, settings}, context); expect({ timing: Math.round(result.timing), @@ -27,9 +23,9 @@ describe('Metrics: CumulativeLongQueuingDelay', () => { pessimistic: Math.round(result.pessimisticEstimate.timeInMs), }).toMatchInlineSnapshot(` Object { - "optimistic": 719, - "pessimistic": 777, - "timing": 748, + "optimistic": 726, + "pessimistic": 827, + "timing": 776, } `); }); @@ -37,14 +33,11 @@ Object { it('should compute an observed value', async () => { const settings = {throttlingMethod: 'provided'}; const context = {settings, computedCache: new Map()}; - const result = await CumulativeLongQueuingDelay.request( - {trace, devtoolsLog, settings}, - context - ); - expect(result.timing).toBeCloseTo(48.3, 1); + const result = await TotalBlockingTime.request({trace, devtoolsLog, settings}, context); + expect(result.timing).toBeCloseTo(57.5, 1); }); - describe('#calculateSumOfLongQueuingDelay', () => { + describe('#calculateSumOfBlockingTime', () => { it('reports 0 when no task is longer than 50ms', () => { const events = [ {start: 1000, end: 1050, duration: 50}, @@ -54,14 +47,12 @@ Object { const fcpTimeMs = 500; const interactiveTimeMs = 4000; - expect(CumulativeLongQueuingDelay.calculateSumOfLongQueuingDelay( - events, - fcpTimeMs, - interactiveTimeMs - )).toBe(0); + expect( + TotalBlockingTime.calculateSumOfBlockingTime(events, fcpTimeMs, interactiveTimeMs) + ).toBe(0); }); - it('only looks at tasks within FMP and TTI', () => { + it('only looks at tasks within FCP and TTI', () => { const events = [ {start: 1000, end: 1060, duration: 60}, {start: 2000, end: 2100, duration: 100}, @@ -72,27 +63,21 @@ Object { const fcpTimeMs = 1500; const interactiveTimeMs = 2500; - expect(CumulativeLongQueuingDelay.calculateSumOfLongQueuingDelay( - events, - fcpTimeMs, - interactiveTimeMs - )).toBe(150); + expect( + TotalBlockingTime.calculateSumOfBlockingTime(events, fcpTimeMs, interactiveTimeMs) + ).toBe(150); }); - it('clips queuing delay regions properly', () => { - const fcpTimeMs = 1050; - const interactiveTimeMs = 2050; + it('treats the end of task as blocking time', () => { + const fcpTimeMs = 100; + const interactiveTimeMs = 200; + // The last 50ms of this 100ms task is blocking time, and it happens after FCP, so it's + // counted towards TBT. + const events = [{start: 50, end: 150, duration: 100}]; - const events = [ - {start: 1000, end: 1110, duration: 110}, // Contributes 10ms. - {start: 2000, end: 2200, duration: 200}, // Contributes 50ms. - ]; - - expect(CumulativeLongQueuingDelay.calculateSumOfLongQueuingDelay( - events, - fcpTimeMs, - interactiveTimeMs - )).toBe(60); + expect( + TotalBlockingTime.calculateSumOfBlockingTime(events, fcpTimeMs, interactiveTimeMs) + ).toBe(50); }); // This can happen in the lantern metric case, where we use the optimistic @@ -101,15 +86,11 @@ Object { const fcpTimeMs = 2050; const interactiveTimeMs = 1050; - const events = [ - {start: 500, end: 3000, duration: 2500}, - ]; + const events = [{start: 500, end: 3000, duration: 2500}]; - expect(CumulativeLongQueuingDelay.calculateSumOfLongQueuingDelay( - events, - fcpTimeMs, - interactiveTimeMs - )).toBe(0); + expect( + TotalBlockingTime.calculateSumOfBlockingTime(events, fcpTimeMs, interactiveTimeMs) + ).toBe(0); }); }); }); diff --git a/lighthouse-core/test/results/sample_v2.json b/lighthouse-core/test/results/sample_v2.json index 1ec3c304731d..15d4f7af1ce9 100644 --- a/lighthouse-core/test/results/sample_v2.json +++ b/lighthouse-core/test/results/sample_v2.json @@ -191,10 +191,10 @@ "numericValue": 16, "displayValue": "20 ms" }, - "cumulative-long-queuing-delay": { - "id": "cumulative-long-queuing-delay", - "title": "Cumulative Long Queuing Delay", - "description": "[Experimental metric] Total time period between FCP and Time to Interactive during which queuing time for any input event would be higher than 50ms.", + "total-blocking-time": { + "id": "total-blocking-time", + "title": "Total Blocking Time", + "description": "Sum of all time periods between FCP and Time to Interactive, when task length exceeded by more than 50ms, expressed in milliseconds.", "score": 1, "scoreDisplayMode": "numeric", "numericValue": 116.79800000000023, @@ -1258,7 +1258,7 @@ "speedIndex": 4417, "speedIndexTs": 185607736912, "estimatedInputLatency": 16, - "cumulativeLongQueuingDelay": 117, + "totalBlockingTime": 117, "observedNavigationStart": 0, "observedNavigationStartTs": 185603319912, "observedFirstPaint": 3969, @@ -3417,7 +3417,7 @@ "weight": 0 }, { - "id": "cumulative-long-queuing-delay", + "id": "total-blocking-time", "weight": 0 }, { @@ -4283,13 +4283,13 @@ }, { "startTime": 0, - "name": "lh:audit:cumulative-long-queuing-delay", + "name": "lh:audit:total-blocking-time", "duration": 100, "entryType": "measure" }, { "startTime": 0, - "name": "lh:computed:CumulativeLongQueuingDelay", + "name": "lh:computed:TotalBlockingTime", "duration": 100, "entryType": "measure" }, @@ -5215,6 +5215,12 @@ }, "path": "audits[estimated-input-latency].displayValue" }, + { + "values": { + "timeInMs": 116.79800000000023 + }, + "path": "audits[total-blocking-time].displayValue" + }, { "values": { "timeInMs": 122.537 @@ -5234,6 +5240,12 @@ "path": "audits[network-server-latency].displayValue" } ], + "lighthouse-core/audits/metrics/total-blocking-time.js | title": [ + "audits[total-blocking-time].title" + ], + "lighthouse-core/audits/metrics/total-blocking-time.js | description": [ + "audits[total-blocking-time].description" + ], "lighthouse-core/audits/metrics/max-potential-fid.js | title": [ "audits[max-potential-fid].title" ], diff --git a/proto/sample_v2_round_trip.json b/proto/sample_v2_round_trip.json index 8e2f7b1af859..692304a8600a 100644 --- a/proto/sample_v2_round_trip.json +++ b/proto/sample_v2_round_trip.json @@ -377,15 +377,6 @@ "scoreDisplayMode": "informative", "title": "Minimize Critical Requests Depth" }, - "cumulative-long-queuing-delay": { - "description": "[Experimental metric] Total time period between FCP and Time to Interactive during which queuing time for any input event would be higher than 50ms.", - "displayValue": "120\u00a0ms", - "id": "cumulative-long-queuing-delay", - "numericValue": 116.79800000000023, - "score": 1.0, - "scoreDisplayMode": "numeric", - "title": "Cumulative Long Queuing Delay" - }, "custom-controls-labels": { "description": "Custom interactive controls have associated labels, provided by aria-label or aria-labelledby. [Learn more](https://developers.google.com/web/fundamentals/accessibility/how-to-review#try_it_with_a_screen_reader).", "id": "custom-controls-labels", @@ -1494,7 +1485,6 @@ "details": { "items": [ { - "cumulativeLongQueuingDelay": 117.0, "estimatedInputLatency": 16.0, "firstCPUIdle": 4927.0, "firstCPUIdleTs": 185608247190.0, @@ -1525,7 +1515,8 @@ "observedTraceEnd": 10281.0, "observedTraceEndTs": 185613601189.0, "speedIndex": 4417.0, - "speedIndexTs": 185607736912.0 + "speedIndexTs": 185607736912.0, + "totalBlockingTime": 117.0 } ], "type": "debugdata" @@ -2629,6 +2620,15 @@ "scoreDisplayMode": "binary", "title": "Server response times are low (TTFB)" }, + "total-blocking-time": { + "description": "Sum of all time periods between FCP and Time to Interactive, when task length exceeded by more than 50ms, expressed in milliseconds.", + "displayValue": "120\u00a0ms", + "id": "total-blocking-time", + "numericValue": 116.79800000000023, + "score": 1.0, + "scoreDisplayMode": "numeric", + "title": "Total Blocking Time" + }, "total-byte-weight": { "description": "Large network payloads cost users real money and are highly correlated with long load times. [Learn more](https://developers.google.com/web/tools/lighthouse/audits/network-payloads).", "details": { @@ -3546,7 +3546,7 @@ "weight": 0.0 }, { - "id": "cumulative-long-queuing-delay", + "id": "total-blocking-time", "weight": 0.0 }, { @@ -4180,13 +4180,13 @@ { "duration": 100.0, "entryType": "measure", - "name": "lh:audit:cumulative-long-queuing-delay", + "name": "lh:audit:total-blocking-time", "startTime": 0.0 }, { "duration": 100.0, "entryType": "measure", - "name": "lh:computed:CumulativeLongQueuingDelay", + "name": "lh:computed:TotalBlockingTime", "startTime": 0.0 }, { From 6094cd3518a6df9892a5fdbb5bc1b918642a0930 Mon Sep 17 00:00:00 2001 From: Deepanjan Roy Date: Fri, 19 Jul 2019 18:38:39 -0400 Subject: [PATCH 2/5] Address review comments --- .../audits/metrics/total-blocking-time.js | 2 +- .../computed/metrics/total-blocking-time.js | 48 ++++++++----------- lighthouse-core/lib/i18n/en-US.json | 2 +- lighthouse-core/lib/i18n/locales/en-XL.json | 2 +- .../audits/__snapshots__/metrics-test.js.snap | 2 +- .../metrics/total-blocking-time-test.js | 4 +- .../metrics/total-blocking-time-test.js | 29 ++++++----- lighthouse-core/test/results/sample_v2.json | 2 +- proto/sample_v2_round_trip.json | 2 +- 9 files changed, 46 insertions(+), 47 deletions(-) diff --git a/lighthouse-core/audits/metrics/total-blocking-time.js b/lighthouse-core/audits/metrics/total-blocking-time.js index a741264c9618..596cc317aca4 100644 --- a/lighthouse-core/audits/metrics/total-blocking-time.js +++ b/lighthouse-core/audits/metrics/total-blocking-time.js @@ -14,7 +14,7 @@ const UIStrings = { title: 'Total Blocking Time', /** Description of the Total Blocking Time (TBT) metric, which calculates the total duration of blocking time for a page. Blocking times are times when if a page received any input (clicks, taps, and keypresses), the page would be slow at responding. This is displayed within a tooltip when the user hovers on the metric name to see more. No character length limits.*/ description: 'Sum of all time periods between FCP and Time to Interactive, ' + - 'when task length exceeded by more than 50ms, expressed in milliseconds.', + 'when task length exceeded 50ms, expressed in milliseconds.', }; const str_ = i18n.createMessageInstanceIdFn(__filename, UIStrings); diff --git a/lighthouse-core/computed/metrics/total-blocking-time.js b/lighthouse-core/computed/metrics/total-blocking-time.js index 1364ec513cda..c2089310928a 100644 --- a/lighthouse-core/computed/metrics/total-blocking-time.js +++ b/lighthouse-core/computed/metrics/total-blocking-time.js @@ -42,42 +42,36 @@ class TotalBlockingTime extends ComputedMetric { if (interactiveTimeMs <= fcpTimeInMs) return 0; const threshold = TotalBlockingTime.BLOCKING_TIME_THRESHOLD; - const blockingRegions = []; + let sumBlockingTime = 0; for (const event of topLevelEvents) { - // If the task is less than the delay threshold, it contains no Blocking Region. + // Early exit for small tasks, which should far outnumber long tasks. if (event.duration < threshold) continue; - // Otherwise, the duration of the task beyond blocking time threshold at the beginning is - // considered Blocking Region. Example assuming the threshold is 50ms: - // [ 250ms Task ] - // | First 50ms | Blocking Time Region | - // 200 ms - blockingRegions.push({ - start: event.start + threshold, - end: event.end, - duration: event.duration - threshold, - }); - } - let sumBlockingTime = 0; - for (const region of blockingRegions) { - // We only want to add up the Blocking Time Regions that fall between FCP and TTI. - // + // We only want to consider tasks that fall between FCP and TTI. // FCP is picked as the lower bound because there is little risk of user input happening // before FCP so Long Queuing Qelay regions do not harm user experience. Developers should be // optimizing to reach FCP as fast as possible without having to worry about task lengths. - // + if (event.end < fcpTimeInMs) continue; + // TTI is picked as the upper bound because we want a well defined end point so that the // metric does not rely on how long we trace. - if (region.end < fcpTimeInMs) continue; - if (region.start > interactiveTimeMs) continue; + if (event.start > interactiveTimeMs) continue; - // If a Blocking Time Region spans the edges of our region of interest, we clip it to only - // include the part of the region that falls inside. - const clippedStart = Math.max(region.start, fcpTimeInMs); - const clippedEnd = Math.min(region.end, interactiveTimeMs); - const blockingTimeAfterClipping = clippedEnd - clippedStart; + // We first perform the clipping, and then calculate Blocking Region. So if we have a 150ms + // task [0, 150] and FCP happens midway at 50ms, we first clip the task to [50, 150], and then + // calculate the Blocking Region to be [100, 150]. There rational here is that tasks before + // FCP is unimportant, so we care whether the main thread is busy more than 50ms at a time + // only after FCP. + const clippedStart = Math.max(event.start, fcpTimeInMs); + const clippedEnd = Math.min(event.end, interactiveTimeMs); + const clippedDuration = clippedEnd - clippedStart; + if (clippedDuration < threshold) continue; - sumBlockingTime += blockingTimeAfterClipping; + // The duration of the task beyond 50ms at the beginning is considered the Blocking Region. + // Example: + // [ 250ms Task ] + // | First 50ms | Blocking Region (200ms) | + sumBlockingTime += (clippedDuration - threshold); } return sumBlockingTime; @@ -105,8 +99,6 @@ class TotalBlockingTime extends ComputedMetric { const interactiveTimeMs = (await TimetoInteractive.request(data, context)).timing; - // Not using the start time argument of getMainThreadTopLevelEvents, because - // we need to clip the part of the task before the last 50ms properly. const events = TracingProcessor.getMainThreadTopLevelEvents(data.traceOfTab); return { diff --git a/lighthouse-core/lib/i18n/en-US.json b/lighthouse-core/lib/i18n/en-US.json index e22ef2a1a191..2306f58249d3 100644 --- a/lighthouse-core/lib/i18n/en-US.json +++ b/lighthouse-core/lib/i18n/en-US.json @@ -1040,7 +1040,7 @@ "description": "The name of the metric that summarizes how quickly the page looked visually complete. The name of this metric is largely abstract and can be loosely translated. Shown to users as the label for the numeric metric value. Ideally fits within a ~40 character limit." }, "lighthouse-core/audits/metrics/total-blocking-time.js | description": { - "message": "Sum of all time periods between FCP and Time to Interactive, when task length exceeded by more than 50ms, expressed in milliseconds.", + "message": "Sum of all time periods between FCP and Time to Interactive, when task length exceeded 50ms, expressed in milliseconds.", "description": "Description of the Total Blocking Time (TBT) metric, which calculates the total duration of blocking time for a page. Blocking times are times when if a page received any input (clicks, taps, and keypresses), the page would be slow at responding. This is displayed within a tooltip when the user hovers on the metric name to see more. No character length limits." }, "lighthouse-core/audits/metrics/total-blocking-time.js | title": { diff --git a/lighthouse-core/lib/i18n/locales/en-XL.json b/lighthouse-core/lib/i18n/locales/en-XL.json index df6e78095310..aa487633106a 100644 --- a/lighthouse-core/lib/i18n/locales/en-XL.json +++ b/lighthouse-core/lib/i18n/locales/en-XL.json @@ -780,7 +780,7 @@ "message": "Ŝṕêéd̂ Ín̂d́êx́" }, "lighthouse-core/audits/metrics/total-blocking-time.js | description": { - "message": "Ŝúm̂ óf̂ ál̂ĺ t̂ím̂é p̂ér̂íôd́ŝ b́êt́ŵéêń F̂ĆP̂ án̂d́ T̂ím̂é t̂ó Îńt̂ér̂áĉt́îv́ê, ẃĥén̂ t́âśk̂ ĺêńĝt́ĥ éx̂ćêéd̂éd̂ b́ŷ ḿôŕê t́ĥán̂ 50ḿŝ, éx̂ṕr̂éŝśêd́ îń m̂íl̂ĺîśêćôńd̂ś." + "message": "Ŝúm̂ óf̂ ál̂ĺ t̂ím̂é p̂ér̂íôd́ŝ b́êt́ŵéêń F̂ĆP̂ án̂d́ T̂ím̂é t̂ó Îńt̂ér̂áĉt́îv́ê, ẃĥén̂ t́âśk̂ ĺêńĝt́ĥ éx̂ćêéd̂éd̂ 50ḿŝ, éx̂ṕr̂éŝśêd́ îń m̂íl̂ĺîśêćôńd̂ś." }, "lighthouse-core/audits/metrics/total-blocking-time.js | title": { "message": "T̂ót̂ál̂ B́l̂óĉḱîńĝ T́îḿê" diff --git a/lighthouse-core/test/audits/__snapshots__/metrics-test.js.snap b/lighthouse-core/test/audits/__snapshots__/metrics-test.js.snap index 934dc6fa079d..f79601e53938 100644 --- a/lighthouse-core/test/audits/__snapshots__/metrics-test.js.snap +++ b/lighthouse-core/test/audits/__snapshots__/metrics-test.js.snap @@ -34,6 +34,6 @@ Object { "observedTraceEndTs": 225426711887, "speedIndex": 1657, "speedIndexTs": undefined, - "totalBlockingTime": 776, + "totalBlockingTime": 726, } `; diff --git a/lighthouse-core/test/audits/metrics/total-blocking-time-test.js b/lighthouse-core/test/audits/metrics/total-blocking-time-test.js index 220683bc4a26..1720753ee20a 100644 --- a/lighthouse-core/test/audits/metrics/total-blocking-time-test.js +++ b/lighthouse-core/test/audits/metrics/total-blocking-time-test.js @@ -25,8 +25,8 @@ describe('Performance: total-blocking-time audit', () => { const context = {options, settings, computedCache: new Map()}; const output = await TBTAudit.audit(artifacts, context); - expect(output.numericValue).toBeCloseTo(57.5, 1); + expect(output.numericValue).toBeCloseTo(48.3, 1); expect(output.score).toBe(1); - expect(output.displayValue).toBeDisplayString('60\xa0ms'); + expect(output.displayValue).toBeDisplayString('50\xa0ms'); }); }); diff --git a/lighthouse-core/test/computed/metrics/total-blocking-time-test.js b/lighthouse-core/test/computed/metrics/total-blocking-time-test.js index d2a544acf6ec..0c816b8595af 100644 --- a/lighthouse-core/test/computed/metrics/total-blocking-time-test.js +++ b/lighthouse-core/test/computed/metrics/total-blocking-time-test.js @@ -23,9 +23,9 @@ describe('Metrics: TotalBlockingTime', () => { pessimistic: Math.round(result.pessimisticEstimate.timeInMs), }).toMatchInlineSnapshot(` Object { - "optimistic": 726, - "pessimistic": 827, - "timing": 776, + "optimistic": 676, + "pessimistic": 777, + "timing": 726, } `); }); @@ -34,7 +34,7 @@ Object { const settings = {throttlingMethod: 'provided'}; const context = {settings, computedCache: new Map()}; const result = await TotalBlockingTime.request({trace, devtoolsLog, settings}, context); - expect(result.timing).toBeCloseTo(57.5, 1); + expect(result.timing).toBeCloseTo(48.3, 1); }); describe('#calculateSumOfBlockingTime', () => { @@ -68,16 +68,23 @@ Object { ).toBe(150); }); - it('treats the end of task as blocking time', () => { - const fcpTimeMs = 100; - const interactiveTimeMs = 200; - // The last 50ms of this 100ms task is blocking time, and it happens after FCP, so it's - // counted towards TBT. - const events = [{start: 50, end: 150, duration: 100}]; + it('clips before finding blocking regions', () => { + const fcpTimeMs = 150; + const interactiveTimeMs = 300; + + const events = [ + // The clipping is done first, so the task becomes [150, 200] after clipping and contributes + // 0ms of blocking time. This is in contrast to first calculating the blocking region ([100, + // 200]) and then clipping at FCP (150ms), which yields 50ms blocking time. + {start: 50, end: 200, duration: 150}, + // Similarly, the task is first clipped above to be [240, 300], and then contributes 10ms + // blocking time. + {start: 240, end: 460, duration: 120}, + ]; expect( TotalBlockingTime.calculateSumOfBlockingTime(events, fcpTimeMs, interactiveTimeMs) - ).toBe(50); + ).toBe(10); // 0ms + 10ms. }); // This can happen in the lantern metric case, where we use the optimistic diff --git a/lighthouse-core/test/results/sample_v2.json b/lighthouse-core/test/results/sample_v2.json index 15d4f7af1ce9..d17c0fb0e6d8 100644 --- a/lighthouse-core/test/results/sample_v2.json +++ b/lighthouse-core/test/results/sample_v2.json @@ -194,7 +194,7 @@ "total-blocking-time": { "id": "total-blocking-time", "title": "Total Blocking Time", - "description": "Sum of all time periods between FCP and Time to Interactive, when task length exceeded by more than 50ms, expressed in milliseconds.", + "description": "Sum of all time periods between FCP and Time to Interactive, when task length exceeded 50ms, expressed in milliseconds.", "score": 1, "scoreDisplayMode": "numeric", "numericValue": 116.79800000000023, diff --git a/proto/sample_v2_round_trip.json b/proto/sample_v2_round_trip.json index 692304a8600a..071e6c091b51 100644 --- a/proto/sample_v2_round_trip.json +++ b/proto/sample_v2_round_trip.json @@ -2621,7 +2621,7 @@ "title": "Server response times are low (TTFB)" }, "total-blocking-time": { - "description": "Sum of all time periods between FCP and Time to Interactive, when task length exceeded by more than 50ms, expressed in milliseconds.", + "description": "Sum of all time periods between FCP and Time to Interactive, when task length exceeded 50ms, expressed in milliseconds.", "displayValue": "120\u00a0ms", "id": "total-blocking-time", "numericValue": 116.79800000000023, From ba227b749c9d20623569b1d81a571555a36c221c Mon Sep 17 00:00:00 2001 From: Deepanjan Roy Date: Tue, 23 Jul 2019 15:16:08 -0400 Subject: [PATCH 3/5] Fix comments --- lighthouse-core/audits/metrics/total-blocking-time.js | 4 ++-- lighthouse-core/computed/metrics/total-blocking-time.js | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lighthouse-core/audits/metrics/total-blocking-time.js b/lighthouse-core/audits/metrics/total-blocking-time.js index 596cc317aca4..174e5d5d53b7 100644 --- a/lighthouse-core/audits/metrics/total-blocking-time.js +++ b/lighthouse-core/audits/metrics/total-blocking-time.js @@ -10,9 +10,9 @@ const ComputedTBT = require('../../computed/metrics/total-blocking-time.js'); const i18n = require('../../lib/i18n/i18n.js'); const UIStrings = { - /** The name of the metric that calculates the total duration of blocking time for a page. Blocking times are time periods when the page would be slow at responding to user input (clicks, taps, and keypresses will feel janky). Shown to users as the label for the numeric metric value. Ideally fits within a ~40 character limit. */ + /** The name of a metric that calculates the total duration of blocking time for a web page. Blocking times are time periods when the page would be blocked (prevented) from responding to user input (clicks, taps, and keypresses will feel slow to respond). Shown to users as the label for the numeric metric value. Ideally fits within a ~40 character limit. */ title: 'Total Blocking Time', - /** Description of the Total Blocking Time (TBT) metric, which calculates the total duration of blocking time for a page. Blocking times are times when if a page received any input (clicks, taps, and keypresses), the page would be slow at responding. This is displayed within a tooltip when the user hovers on the metric name to see more. No character length limits.*/ + /** Description of the Total Blocking Time (TBT) metric, which calculates the total duration of blocking time for a web page. Blocking times are time periods when the page would be blocked (prevented) from responding to user input (clicks, taps, and keypresses will feel slow to respond). This is displayed within a tooltip when the user hovers on the metric name to see more. No character length limits.*/ description: 'Sum of all time periods between FCP and Time to Interactive, ' + 'when task length exceeded 50ms, expressed in milliseconds.', }; diff --git a/lighthouse-core/computed/metrics/total-blocking-time.js b/lighthouse-core/computed/metrics/total-blocking-time.js index c2089310928a..d5419203d39d 100644 --- a/lighthouse-core/computed/metrics/total-blocking-time.js +++ b/lighthouse-core/computed/metrics/total-blocking-time.js @@ -59,9 +59,9 @@ class TotalBlockingTime extends ComputedMetric { // We first perform the clipping, and then calculate Blocking Region. So if we have a 150ms // task [0, 150] and FCP happens midway at 50ms, we first clip the task to [50, 150], and then - // calculate the Blocking Region to be [100, 150]. There rational here is that tasks before - // FCP is unimportant, so we care whether the main thread is busy more than 50ms at a time - // only after FCP. + // calculate the Blocking Region to be [100, 150]. The rational here is that tasks before FCP + // are unimportant, so we care whether the main thread is busy more than 50ms at a time only + // after FCP. const clippedStart = Math.max(event.start, fcpTimeInMs); const clippedEnd = Math.min(event.end, interactiveTimeMs); const clippedDuration = clippedEnd - clippedStart; From 480731c3757b97910b0ac606fb06bf12f266fb79 Mon Sep 17 00:00:00 2001 From: Deepanjan Roy Date: Tue, 23 Jul 2019 16:26:18 -0400 Subject: [PATCH 4/5] Add more test cases --- .../metrics/total-blocking-time-test.js | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/lighthouse-core/test/computed/metrics/total-blocking-time-test.js b/lighthouse-core/test/computed/metrics/total-blocking-time-test.js index 0c816b8595af..1ede35e1f6f8 100644 --- a/lighthouse-core/test/computed/metrics/total-blocking-time-test.js +++ b/lighthouse-core/test/computed/metrics/total-blocking-time-test.js @@ -87,6 +87,51 @@ Object { ).toBe(10); // 0ms + 10ms. }); + // TTI can happen in the middle of a task, for example, if TTI is at FMP which occurs as part + // of a larger task, or in the lantern case where we use estimate TTI using a different graph + // from the one used to estimate TBT. + it('clips properly if TTI falls in the middle of a task', () => { + const fcpTimeMs = 1000; + const interactiveTimeMs = 2000; + + expect(TotalBlockingTime.calculateSumOfBlockingTime( + [{start: 1951, end: 2100, duration: 149}], + fcpTimeMs, + interactiveTimeMs + )).toBe(0); // Duration after clipping is 49, which is < 50. + expect(TotalBlockingTime.calculateSumOfBlockingTime( + [{start: 1950, end: 2100, duration: 150}], + fcpTimeMs, + interactiveTimeMs + )).toBe(0); // Duration after clipping is 50, so time after 50ms is 0ms. + expect(TotalBlockingTime.calculateSumOfBlockingTime( + [{start: 1949, end: 2100, duration: 151}], + fcpTimeMs, + interactiveTimeMs + )).toBe(1); // Duration after clipping is 51, so time after 50ms is 1ms. + }); + + it('clips properly if FCP falls in the middle of a task', () => { + const fcpTimeMs = 1000; + const interactiveTimeMs = 2000; + + expect(TotalBlockingTime.calculateSumOfBlockingTime( + [{start: 900, end: 1049, duration: 149}], + fcpTimeMs, + interactiveTimeMs + )).toBe(0); // Duration after clipping is 49, which is < 50. + expect(TotalBlockingTime.calculateSumOfBlockingTime( + [{start: 900, end: 1050, duration: 150}], + fcpTimeMs, + interactiveTimeMs + )).toBe(0); // Duration after clipping is 50, so time after 50ms is 0ms. + expect(TotalBlockingTime.calculateSumOfBlockingTime( + [{start: 900, end: 1051, duration: 151}], + fcpTimeMs, + interactiveTimeMs + )).toBe(1); // Duration after clipping is 51, so time after 50ms is 1ms. + }); + // This can happen in the lantern metric case, where we use the optimistic // TTI and pessimistic FCP. it('returns 0 if interactiveTime is earlier than FCP', () => { From 03567d5e222b291036e29da3d65e401e3e29fe69 Mon Sep 17 00:00:00 2001 From: Deepanjan Roy Date: Tue, 23 Jul 2019 16:31:49 -0400 Subject: [PATCH 5/5] Fix lint errors and update sample json --- lighthouse-core/lib/i18n/en-US.json | 4 ++-- .../computed/metrics/total-blocking-time-test.js | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/lighthouse-core/lib/i18n/en-US.json b/lighthouse-core/lib/i18n/en-US.json index 2306f58249d3..a2f378022fa0 100644 --- a/lighthouse-core/lib/i18n/en-US.json +++ b/lighthouse-core/lib/i18n/en-US.json @@ -1041,11 +1041,11 @@ }, "lighthouse-core/audits/metrics/total-blocking-time.js | description": { "message": "Sum of all time periods between FCP and Time to Interactive, when task length exceeded 50ms, expressed in milliseconds.", - "description": "Description of the Total Blocking Time (TBT) metric, which calculates the total duration of blocking time for a page. Blocking times are times when if a page received any input (clicks, taps, and keypresses), the page would be slow at responding. This is displayed within a tooltip when the user hovers on the metric name to see more. No character length limits." + "description": "Description of the Total Blocking Time (TBT) metric, which calculates the total duration of blocking time for a web page. Blocking times are time periods when the page would be blocked (prevented) from responding to user input (clicks, taps, and keypresses will feel slow to respond). This is displayed within a tooltip when the user hovers on the metric name to see more. No character length limits." }, "lighthouse-core/audits/metrics/total-blocking-time.js | title": { "message": "Total Blocking Time", - "description": "The name of the metric that calculates the total duration of blocking time for a page. Blocking times are time periods when the page would be slow at responding to user input (clicks, taps, and keypresses will feel janky). Shown to users as the label for the numeric metric value. Ideally fits within a ~40 character limit." + "description": "The name of a metric that calculates the total duration of blocking time for a web page. Blocking times are time periods when the page would be blocked (prevented) from responding to user input (clicks, taps, and keypresses will feel slow to respond). Shown to users as the label for the numeric metric value. Ideally fits within a ~40 character limit." }, "lighthouse-core/audits/network-rtt.js | description": { "message": "Network round trip times (RTT) have a large impact on performance. If the RTT to an origin is high, it's an indication that servers closer to the user could improve performance. [Learn more](https://hpbn.co/primer-on-latency-and-bandwidth/).", diff --git a/lighthouse-core/test/computed/metrics/total-blocking-time-test.js b/lighthouse-core/test/computed/metrics/total-blocking-time-test.js index 1ede35e1f6f8..15e9fcff59e2 100644 --- a/lighthouse-core/test/computed/metrics/total-blocking-time-test.js +++ b/lighthouse-core/test/computed/metrics/total-blocking-time-test.js @@ -98,17 +98,17 @@ Object { [{start: 1951, end: 2100, duration: 149}], fcpTimeMs, interactiveTimeMs - )).toBe(0); // Duration after clipping is 49, which is < 50. + )).toBe(0); // Duration after clipping is 49, which is < 50. expect(TotalBlockingTime.calculateSumOfBlockingTime( [{start: 1950, end: 2100, duration: 150}], fcpTimeMs, interactiveTimeMs - )).toBe(0); // Duration after clipping is 50, so time after 50ms is 0ms. + )).toBe(0); // Duration after clipping is 50, so time after 50ms is 0ms. expect(TotalBlockingTime.calculateSumOfBlockingTime( [{start: 1949, end: 2100, duration: 151}], fcpTimeMs, interactiveTimeMs - )).toBe(1); // Duration after clipping is 51, so time after 50ms is 1ms. + )).toBe(1); // Duration after clipping is 51, so time after 50ms is 1ms. }); it('clips properly if FCP falls in the middle of a task', () => { @@ -119,17 +119,17 @@ Object { [{start: 900, end: 1049, duration: 149}], fcpTimeMs, interactiveTimeMs - )).toBe(0); // Duration after clipping is 49, which is < 50. + )).toBe(0); // Duration after clipping is 49, which is < 50. expect(TotalBlockingTime.calculateSumOfBlockingTime( [{start: 900, end: 1050, duration: 150}], fcpTimeMs, interactiveTimeMs - )).toBe(0); // Duration after clipping is 50, so time after 50ms is 0ms. + )).toBe(0); // Duration after clipping is 50, so time after 50ms is 0ms. expect(TotalBlockingTime.calculateSumOfBlockingTime( [{start: 900, end: 1051, duration: 151}], fcpTimeMs, interactiveTimeMs - )).toBe(1); // Duration after clipping is 51, so time after 50ms is 1ms. + )).toBe(1); // Duration after clipping is 51, so time after 50ms is 1ms. }); // This can happen in the lantern metric case, where we use the optimistic