From b732dc37c2f28cdf469f92ea0111abe4b1e7988c Mon Sep 17 00:00:00 2001 From: Patrick Hulce Date: Thu, 3 May 2018 10:54:57 -0700 Subject: [PATCH 1/5] core(metrics): consumable details audit output --- lighthouse-core/audits/metrics.js | 42 ++---- .../lib/traces/pwmetrics-events.js | 50 +++---- lighthouse-core/test/audits/metrics-test.js | 19 +-- lighthouse-core/test/results/sample_v2.json | 122 +++++------------- 4 files changed, 78 insertions(+), 155 deletions(-) diff --git a/lighthouse-core/audits/metrics.js b/lighthouse-core/audits/metrics.js index 49bc60688752..c9bb86bde946 100644 --- a/lighthouse-core/audits/metrics.js +++ b/lighthouse-core/audits/metrics.js @@ -39,7 +39,7 @@ class Metrics extends Audit { const interactive = await artifacts.requestInteractive(metricComputationData); const speedIndex = await artifacts.requestSpeedIndex(metricComputationData); const estimatedInputLatency = await artifacts.requestEstimatedInputLatency(metricComputationData); // eslint-disable-line max-len - const metrics = []; + const metrics = {}; // Include the simulated/observed performance metrics const metricsMap = { @@ -52,11 +52,8 @@ class Metrics extends Audit { }; for (const [metricName, values] of Object.entries(metricsMap)) { - metrics.push({ - metricName, - timing: Math.round(values.timing), - timestamp: values.timestamp, - }); + metrics[metricName] = Math.round(values.timing); + if (values.timestamp) metrics[`${metricName}Ts`] = values.timestamp; } // Include all timestamps of interest from trace of tab @@ -66,37 +63,22 @@ class Metrics extends Audit { const uppercased = traceEventName.slice(0, 1).toUpperCase() + traceEventName.slice(1); const metricName = `observed${uppercased}`; const timestamp = traceOfTab.timestamps[traceEventName]; - metrics.push({metricName, timing, timestamp}); + metrics[metricName] = Math.round(timing); + metrics[`${metricName}Ts`] = timestamp; } // Include some visual metrics from speedline - metrics.push({ - metricName: 'observedFirstVisualChange', - timing: speedline.first, - timestamp: (speedline.first + speedline.beginning) * 1000, - }); - metrics.push({ - metricName: 'observedLastVisualChange', - timing: speedline.complete, - timestamp: (speedline.complete + speedline.beginning) * 1000, - }); - metrics.push({ - metricName: 'observedSpeedIndex', - timing: speedline.speedIndex, - timestamp: (speedline.speedIndex + speedline.beginning) * 1000, - }); - - const headings = [ - {key: 'metricName', itemType: 'text', text: 'Name'}, - {key: 'timing', itemType: 'ms', granularity: 10, text: 'Value (ms)'}, - ]; - - const tableDetails = Audit.makeTableDetails(headings, metrics); + metrics.observedFirstVisualChange = speedline.first; + metrics.observedFirstVisualChangeTs = (speedline.first + speedline.beginning) * 1000; + metrics.observedLastVisualChange = speedline.complete; + metrics.observedLastVisualChangeTs = (speedline.complete + speedline.beginning) * 1000; + metrics.observedSpeedIndex = Math.round(speedline.speedIndex); + metrics.observedSpeedIndexTs = Math.round((speedline.speedIndex + speedline.beginning) * 1000); return { score: 1, rawValue: interactive.timing, - details: tableDetails, + details: {items: [metrics]}, }; } } diff --git a/lighthouse-core/lib/traces/pwmetrics-events.js b/lighthouse-core/lib/traces/pwmetrics-events.js index fc7dfa3917fe..e596ac441284 100644 --- a/lighthouse-core/lib/traces/pwmetrics-events.js +++ b/lighthouse-core/lib/traces/pwmetrics-events.js @@ -8,13 +8,13 @@ const log = require('lighthouse-logger'); -function findValueInMetricsAuditFn(metricName, timingOrTimestamp) { +function findValueInMetricsAuditFn(metricName) { return auditResults => { const metricsAudit = auditResults.metrics; if (!metricsAudit || !metricsAudit.details || !metricsAudit.details.items) return; - const values = metricsAudit.details.items.find(item => item.metricName === metricName); - return values && values[timingOrTimestamp]; + const values = metricsAudit.details.items[0]; + return values && values[metricName]; }; } @@ -33,68 +33,68 @@ class Metrics { { name: 'Navigation Start', id: 'navstart', - getTs: findValueInMetricsAuditFn('observedNavigationStart', 'timestamp'), - getTiming: findValueInMetricsAuditFn('observedNavigationStart', 'timing'), + getTs: findValueInMetricsAuditFn('observedNavigationStartTs'), + getTiming: findValueInMetricsAuditFn('observedNavigationStart'), }, { name: 'First Contentful Paint', id: 'ttfcp', - getTs: findValueInMetricsAuditFn('observedFirstContentfulPaint', 'timestamp'), - getTiming: findValueInMetricsAuditFn('observedFirstContentfulPaint', 'timing'), + getTs: findValueInMetricsAuditFn('observedFirstContentfulPaintTs'), + getTiming: findValueInMetricsAuditFn('observedFirstContentfulPaint'), }, { name: 'First Meaningful Paint', id: 'ttfmp', - getTs: findValueInMetricsAuditFn('observedFirstMeaningfulPaint', 'timestamp'), - getTiming: findValueInMetricsAuditFn('observedFirstMeaningfulPaint', 'timing'), + getTs: findValueInMetricsAuditFn('observedFirstMeaningfulPaintTs'), + getTiming: findValueInMetricsAuditFn('observedFirstMeaningfulPaint'), }, { name: 'Speed Index', id: 'si', - getTs: findValueInMetricsAuditFn('observedSpeedIndex', 'timestamp'), - getTiming: findValueInMetricsAuditFn('observedSpeedIndex', 'timing'), + getTs: findValueInMetricsAuditFn('observedSpeedIndexTs'), + getTiming: findValueInMetricsAuditFn('observedSpeedIndex'), }, { name: 'First Visual Change', id: 'fv', - getTs: findValueInMetricsAuditFn('observedFirstVisualChange', 'timestamp'), - getTiming: findValueInMetricsAuditFn('observedFirstVisualChange', 'timing'), + getTs: findValueInMetricsAuditFn('observedFirstVisualChangeTs'), + getTiming: findValueInMetricsAuditFn('observedFirstVisualChange'), }, { name: 'Visually Complete 100%', id: 'vc100', - getTs: findValueInMetricsAuditFn('observedLastVisualChange', 'timestamp'), - getTiming: findValueInMetricsAuditFn('observedLastVisualChange', 'timing'), + getTs: findValueInMetricsAuditFn('observedLastVisualChangeTs'), + getTiming: findValueInMetricsAuditFn('observedLastVisualChange'), }, { name: 'First CPU Idle', id: 'ttfi', - getTs: findValueInMetricsAuditFn('firstCPUIdle', 'timestamp'), - getTiming: findValueInMetricsAuditFn('firstCPUIdle', 'timing'), + getTs: findValueInMetricsAuditFn('firstCPUIdleTs'), + getTiming: findValueInMetricsAuditFn('firstCPUIdle'), }, { name: 'Interactive', id: 'tti', - getTs: findValueInMetricsAuditFn('interactive', 'timestamp'), - getTiming: findValueInMetricsAuditFn('interactive', 'timing'), + getTs: findValueInMetricsAuditFn('interactiveTs'), + getTiming: findValueInMetricsAuditFn('interactive'), }, { name: 'End of Trace', id: 'eot', - getTs: findValueInMetricsAuditFn('observedTraceEnd', 'timestamp'), - getTiming: findValueInMetricsAuditFn('observedTraceEnd', 'timing'), + getTs: findValueInMetricsAuditFn('observedTraceEndTs'), + getTiming: findValueInMetricsAuditFn('observedTraceEnd'), }, { name: 'On Load', id: 'onload', - getTs: findValueInMetricsAuditFn('observedLoad', 'timestamp'), - getTiming: findValueInMetricsAuditFn('observedLoad', 'timing'), + getTs: findValueInMetricsAuditFn('observedLoadTs'), + getTiming: findValueInMetricsAuditFn('observedLoad'), }, { name: 'DOM Content Loaded', id: 'dcl', - getTs: findValueInMetricsAuditFn('observedDomContentLoaded', 'timestamp'), - getTiming: findValueInMetricsAuditFn('observedDomContentLoaded', 'timing'), + getTs: findValueInMetricsAuditFn('observedDomContentLoadedTs'), + getTiming: findValueInMetricsAuditFn('observedDomContentLoaded'), }, ]; } diff --git a/lighthouse-core/test/audits/metrics-test.js b/lighthouse-core/test/audits/metrics-test.js index 55744de4ccf4..5894e1e5ec25 100644 --- a/lighthouse-core/test/audits/metrics-test.js +++ b/lighthouse-core/test/audits/metrics-test.js @@ -29,15 +29,6 @@ describe('Performance: metrics', () => { const result = await Audit.audit(artifacts, {settings}); assert.deepStrictEqual(result.details.items[0], { - metricName: 'firstContentfulPaint', - timing: 2038, - timestamp: undefined, - }); - - const metrics = {}; - result.details.items.forEach(item => metrics[item.metricName] = Math.round(item.timing)); - - assert.deepStrictEqual(metrics, { firstContentfulPaint: 2038, firstMeaningfulPaint: 2851, firstCPUIdle: 5308, @@ -46,15 +37,25 @@ describe('Performance: metrics', () => { estimatedInputLatency: 104, observedNavigationStart: 0, + observedNavigationStartTs: 225414172015, observedFirstPaint: 499, + observedFirstPaintTs: 225414670868, observedFirstContentfulPaint: 499, + observedFirstContentfulPaintTs: 225414670885, observedFirstMeaningfulPaint: 783, + observedFirstMeaningfulPaintTs: 225414955343, observedTraceEnd: 12540, + observedTraceEndTs: 225426711887, observedLoad: 2199, + observedLoadTs: 225416370913, observedDomContentLoaded: 560, + observedDomContentLoadedTs: 225414732309, observedFirstVisualChange: 520, + observedFirstVisualChangeTs: 225414692015, observedLastVisualChange: 818, + observedLastVisualChangeTs: 225414990015, observedSpeedIndex: 605, + observedSpeedIndexTs: 225414776724, }); }); }); diff --git a/lighthouse-core/test/results/sample_v2.json b/lighthouse-core/test/results/sample_v2.json index 063967abb8d8..77a4997a0354 100644 --- a/lighthouse-core/test/results/sample_v2.json +++ b/lighthouse-core/test/results/sample_v2.json @@ -1491,99 +1491,39 @@ "description": "Metrics", "helpText": "Collects all available metrics.", "details": { - "type": "table", - "headings": [ - { - "key": "metricName", - "itemType": "text", - "text": "Name" - }, - { - "key": "timing", - "itemType": "ms", - "granularity": 10, - "text": "Value (ms)" - } - ], "items": [ { - "metricName": "firstContentfulPaint", - "timing": 3969, - "timestamp": 185607289047 - }, - { - "metricName": "firstMeaningfulPaint", - "timing": 3969, - "timestamp": 185607289048 - }, - { - "metricName": "firstCPUIdle", - "timing": 4927, - "timestamp": 185608247190 - }, - { - "metricName": "interactive", - "timing": 4927, - "timestamp": 185608247190 - }, - { - "metricName": "speedIndex", - "timing": 4417, - "timestamp": 185607736912 - }, - { - "metricName": "estimatedInputLatency", - "timing": 16 - }, - { - "metricName": "observedNavigationStart", - "timing": 0, - "timestamp": 185603319912 - }, - { - "metricName": "observedFirstPaint", - "timing": 3969.131, - "timestamp": 185607289043 - }, - { - "metricName": "observedFirstContentfulPaint", - "timing": 3969.135, - "timestamp": 185607289047 - }, - { - "metricName": "observedFirstMeaningfulPaint", - "timing": 3969.136, - "timestamp": 185607289048 - }, - { - "metricName": "observedTraceEnd", - "timing": 10281.277, - "timestamp": 185613601189 - }, - { - "metricName": "observedLoad", - "timing": 4924.462, - "timestamp": 185608244374 - }, - { - "metricName": "observedDomContentLoaded", - "timing": 4900.822, - "timestamp": 185608220734 - }, - { - "metricName": "observedFirstVisualChange", - "timing": 3969, - "timestamp": 185607288912 - }, - { - "metricName": "observedLastVisualChange", - "timing": 4791, - "timestamp": 185608110912 - }, - { - "metricName": "observedSpeedIndex", - "timing": 4416.851239995658, - "timestamp": 185607736763.24002 + "firstContentfulPaint": 3969, + "firstContentfulPaintTs": 185607289047, + "firstMeaningfulPaint": 3969, + "firstMeaningfulPaintTs": 185607289048, + "firstCPUIdle": 4927, + "firstCPUIdleTs": 185608247190, + "interactive": 4927, + "interactiveTs": 185608247190, + "speedIndex": 4417, + "speedIndexTs": 185607736912, + "estimatedInputLatency": 16, + "observedNavigationStart": 0, + "observedNavigationStartTs": 185603319912, + "observedFirstPaint": 3969, + "observedFirstPaintTs": 185607289043, + "observedFirstContentfulPaint": 3969, + "observedFirstContentfulPaintTs": 185607289047, + "observedFirstMeaningfulPaint": 3969, + "observedFirstMeaningfulPaintTs": 185607289048, + "observedTraceEnd": 10281, + "observedTraceEndTs": 185613601189, + "observedLoad": 4924, + "observedLoadTs": 185608244374, + "observedDomContentLoaded": 4901, + "observedDomContentLoadedTs": 185608220734, + "observedFirstVisualChange": 3969, + "observedFirstVisualChangeTs": 185607288912, + "observedLastVisualChange": 4791, + "observedLastVisualChangeTs": 185608110912, + "observedSpeedIndex": 4417, + "observedSpeedIndexTs": 185607736763 } ] } From 763ac8be5fec4a37ceab43f389310475eab9a5e7 Mon Sep 17 00:00:00 2001 From: Patrick Hulce Date: Thu, 3 May 2018 11:10:46 -0700 Subject: [PATCH 2/5] typechecking ftw --- lighthouse-core/audits/metrics.js | 118 ++++++++++++++++++++++-------- 1 file changed, 87 insertions(+), 31 deletions(-) diff --git a/lighthouse-core/audits/metrics.js b/lighthouse-core/audits/metrics.js index c9bb86bde946..9bdb8b600825 100644 --- a/lighthouse-core/audits/metrics.js +++ b/lighthouse-core/audits/metrics.js @@ -39,48 +39,104 @@ class Metrics extends Audit { const interactive = await artifacts.requestInteractive(metricComputationData); const speedIndex = await artifacts.requestSpeedIndex(metricComputationData); const estimatedInputLatency = await artifacts.requestEstimatedInputLatency(metricComputationData); // eslint-disable-line max-len - const metrics = {}; - // Include the simulated/observed performance metrics - const metricsMap = { - firstContentfulPaint, - firstMeaningfulPaint, - firstCPUIdle, - interactive, - speedIndex, - estimatedInputLatency, - }; + /** @type {UberMetricsItem} */ + const metrics = { + // Include the simulated/observed performance metrics + firstContentfulPaint: firstContentfulPaint.timing, + firstContentfulPaintTs: firstContentfulPaint.timestamp, + firstMeaningfulPaint: firstMeaningfulPaint.timing, + firstMeaningfulPaintTs: firstMeaningfulPaint.timestamp, + firstCPUIdle: firstCPUIdle.timing, + firstCPUIdleTs: firstCPUIdle.timestamp, + interactive: interactive.timing, + interactiveTs: interactive.timestamp, + speedIndex: speedIndex.timing, + speedIndexTs: speedIndex.timestamp, + estimatedInputLatency: estimatedInputLatency.timing, + estimatedInputLatencyTs: estimatedInputLatency.timestamp, - for (const [metricName, values] of Object.entries(metricsMap)) { - metrics[metricName] = Math.round(values.timing); - if (values.timestamp) metrics[`${metricName}Ts`] = values.timestamp; - } + // Include all timestamps of interest from trace of tab + observedNavigationStart: traceOfTab.timings.navigationStart, + observedNavigationStartTs: traceOfTab.timestamps.navigationStart, + observedFirstPaint: traceOfTab.timings.firstPaint, + observedFirstPaintTs: traceOfTab.timestamps.firstPaint, + observedFirstContentfulPaint: traceOfTab.timings.firstContentfulPaint, + observedFirstContentfulPaintTs: traceOfTab.timestamps.firstContentfulPaint, + observedFirstMeaningfulPaint: traceOfTab.timings.firstMeaningfulPaint, + observedFirstMeaningfulPaintTs: traceOfTab.timestamps.firstMeaningfulPaint, + observedTraceEnd: traceOfTab.timings.traceEnd, + observedTraceEndTs: traceOfTab.timestamps.traceEnd, + observedLoad: traceOfTab.timings.load, + observedLoadTs: traceOfTab.timestamps.load, + observedDomContentLoaded: traceOfTab.timings.domContentLoaded, + observedDomContentLoadedTs: traceOfTab.timestamps.domContentLoaded, + + // Include some visual metrics from speedline + observedFirstVisualChange: speedline.first, + observedFirstVisualChangeTs: (speedline.first + speedline.beginning) * 1000, + observedLastVisualChange: speedline.complete, + observedLastVisualChangeTs: (speedline.complete + speedline.beginning) * 1000, + observedSpeedIndex: speedline.speedIndex, + observedSpeedIndexTs: (speedline.speedIndex + speedline.beginning) * 1000, + }; - // Include all timestamps of interest from trace of tab - const timingsEntries = /** @type {Array<[keyof LH.Artifacts.TraceTimes, number]>} */ - (Object.entries(traceOfTab.timings)); - for (const [traceEventName, timing] of timingsEntries) { - const uppercased = traceEventName.slice(0, 1).toUpperCase() + traceEventName.slice(1); - const metricName = `observed${uppercased}`; - const timestamp = traceOfTab.timestamps[traceEventName]; - metrics[metricName] = Math.round(timing); - metrics[`${metricName}Ts`] = timestamp; + for (const [name, value] of Object.entries(metrics)) { + const key = /** @type {keyof UberMetricsItem} */ (name); + if (typeof value === 'undefined') { + delete metrics[key]; + } else { + metrics[key] = Math.round(value); + } } - // Include some visual metrics from speedline - metrics.observedFirstVisualChange = speedline.first; - metrics.observedFirstVisualChangeTs = (speedline.first + speedline.beginning) * 1000; - metrics.observedLastVisualChange = speedline.complete; - metrics.observedLastVisualChangeTs = (speedline.complete + speedline.beginning) * 1000; - metrics.observedSpeedIndex = Math.round(speedline.speedIndex); - metrics.observedSpeedIndexTs = Math.round((speedline.speedIndex + speedline.beginning) * 1000); + /** @type {MetricsDetails} */ + const details = {items: [metrics]}; return { score: 1, rawValue: interactive.timing, - details: {items: [metrics]}, + details, }; } } +/** + * @typedef UberMetricsItem + * @property {number} firstContentfulPaint + * @property {number=} firstContentfulPaintTs + * @property {number} firstMeaningfulPaint + * @property {number=} firstMeaningfulPaintTs + * @property {number} firstCPUIdle + * @property {number=} firstCPUIdleTs + * @property {number} interactive + * @property {number=} interactiveTs + * @property {number} speedIndex + * @property {number=} speedIndexTs + * @property {number} estimatedInputLatency + * @property {number=} estimatedInputLatencyTs + * @property {number} observedNavigationStart + * @property {number} observedNavigationStartTs + * @property {number} observedFirstPaint + * @property {number} observedFirstPaintTs + * @property {number} observedFirstContentfulPaint + * @property {number} observedFirstContentfulPaintTs + * @property {number} observedFirstMeaningfulPaint + * @property {number} observedFirstMeaningfulPaintTs + * @property {number} observedTraceEnd + * @property {number} observedTraceEndTs + * @property {number} observedLoad + * @property {number} observedLoadTs + * @property {number} observedDomContentLoaded + * @property {number} observedDomContentLoadedTs + * @property {number} observedFirstVisualChange + * @property {number} observedFirstVisualChangeTs + * @property {number} observedLastVisualChange + * @property {number} observedLastVisualChangeTs + * @property {number} observedSpeedIndex + * @property {number} observedSpeedIndexTs + */ + +/** @typedef {{items: [UberMetricsItem]}} MetricsDetails */ + module.exports = Metrics; From ef57740dd4da03e92fa3f9fbc3618bc122d367b6 Mon Sep 17 00:00:00 2001 From: Patrick Hulce Date: Fri, 4 May 2018 11:02:40 -0700 Subject: [PATCH 3/5] TODO --- lighthouse-core/lib/traces/pwmetrics-events.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lighthouse-core/lib/traces/pwmetrics-events.js b/lighthouse-core/lib/traces/pwmetrics-events.js index e596ac441284..4ed61c212c91 100644 --- a/lighthouse-core/lib/traces/pwmetrics-events.js +++ b/lighthouse-core/lib/traces/pwmetrics-events.js @@ -8,6 +8,7 @@ const log = require('lighthouse-logger'); +// TODO: rework this file to not need this function findValueInMetricsAuditFn(metricName) { return auditResults => { const metricsAudit = auditResults.metrics; From 7ea6fb5fc0a40ebdfe49f004b41e42cb1fbe743f Mon Sep 17 00:00:00 2001 From: Patrick Hulce Date: Fri, 4 May 2018 11:03:25 -0700 Subject: [PATCH 4/5] better TODO --- lighthouse-core/lib/traces/pwmetrics-events.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lighthouse-core/lib/traces/pwmetrics-events.js b/lighthouse-core/lib/traces/pwmetrics-events.js index 4ed61c212c91..3fc0aa9facf7 100644 --- a/lighthouse-core/lib/traces/pwmetrics-events.js +++ b/lighthouse-core/lib/traces/pwmetrics-events.js @@ -8,7 +8,8 @@ const log = require('lighthouse-logger'); -// TODO: rework this file to not need this +// TODO: rework this file to not need this function +// see https://github.com/GoogleChrome/lighthouse/pull/5101/files#r186168840 function findValueInMetricsAuditFn(metricName) { return auditResults => { const metricsAudit = auditResults.metrics; From 73ae67d955db1d26c1475a101ddc7803d572593f Mon Sep 17 00:00:00 2001 From: Patrick Hulce Date: Fri, 4 May 2018 14:45:55 -0700 Subject: [PATCH 5/5] do not delete --- lighthouse-core/audits/metrics.js | 4 +--- lighthouse-core/test/audits/metrics-test.js | 6 ++++++ 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/lighthouse-core/audits/metrics.js b/lighthouse-core/audits/metrics.js index 9bdb8b600825..4e69d406bdeb 100644 --- a/lighthouse-core/audits/metrics.js +++ b/lighthouse-core/audits/metrics.js @@ -83,9 +83,7 @@ class Metrics extends Audit { for (const [name, value] of Object.entries(metrics)) { const key = /** @type {keyof UberMetricsItem} */ (name); - if (typeof value === 'undefined') { - delete metrics[key]; - } else { + if (typeof value !== 'undefined') { metrics[key] = Math.round(value); } } diff --git a/lighthouse-core/test/audits/metrics-test.js b/lighthouse-core/test/audits/metrics-test.js index 5894e1e5ec25..494db1b80cc5 100644 --- a/lighthouse-core/test/audits/metrics-test.js +++ b/lighthouse-core/test/audits/metrics-test.js @@ -30,11 +30,17 @@ describe('Performance: metrics', () => { assert.deepStrictEqual(result.details.items[0], { firstContentfulPaint: 2038, + firstContentfulPaintTs: undefined, firstMeaningfulPaint: 2851, + firstMeaningfulPaintTs: undefined, firstCPUIdle: 5308, + firstCPUIdleTs: undefined, interactive: 5308, + interactiveTs: undefined, speedIndex: 2063, + speedIndexTs: undefined, estimatedInputLatency: 104, + estimatedInputLatencyTs: undefined, observedNavigationStart: 0, observedNavigationStartTs: 225414172015,