From 02e0e05d85665747b18cfcdb5bfc83781cb1afff Mon Sep 17 00:00:00 2001 From: Patrick Hulce Date: Mon, 23 Apr 2018 14:22:11 -0700 Subject: [PATCH 1/3] core(tti): update ignorable network requests and start point --- .../metrics/consistently-interactive.js | 20 ++++++--- lighthouse-core/lib/errors.js | 2 +- lighthouse-core/lib/network-recorder.js | 10 ++++- .../metrics/consistently-interactive-test.js | 44 +++++++++++-------- typings/web-inspector.d.ts | 1 + 5 files changed, 49 insertions(+), 28 deletions(-) diff --git a/lighthouse-core/gather/computed/metrics/consistently-interactive.js b/lighthouse-core/gather/computed/metrics/consistently-interactive.js index a8c455f3a308..14b77fd7a9dd 100644 --- a/lighthouse-core/gather/computed/metrics/consistently-interactive.js +++ b/lighthouse-core/gather/computed/metrics/consistently-interactive.js @@ -14,6 +14,11 @@ const LHError = require('../../../lib/errors'); const REQUIRED_QUIET_WINDOW = 5000; const ALLOWED_CONCURRENT_REQUESTS = 2; +/** + * @fileoverview Computes "Time To Interactive", the time at which the page has loaded critical + * resources and is mostly idle. + * @see https://docs.google.com/document/d/1yE4YWsusi5wVXrnwhR61j-QyjK9tzENIzfxrCjA1NAk/edit#heading=h.yozfsuqcgpc4 + */ class ConsistentlyInteractive extends MetricArtifact { get name() { return 'ConsistentlyInteractive'; @@ -28,8 +33,11 @@ class ConsistentlyInteractive extends MetricArtifact { */ static _findNetworkQuietPeriods(networkRecords, traceOfTab) { const traceEndTsInMs = traceOfTab.timestamps.traceEnd / 1000; + /** @param {LH.WebInspector.NetworkRequest} record */ + const filter = record => record.finished && record.requestMethod === 'GET' && !record.failed && + Math.floor(record.statusCode / 100) < 4; return NetworkRecorder.findNetworkQuietPeriods(networkRecords, - ALLOWED_CONCURRENT_REQUESTS, traceEndTsInMs); + ALLOWED_CONCURRENT_REQUESTS, traceEndTsInMs, filter); } /** @@ -79,11 +87,11 @@ class ConsistentlyInteractive extends MetricArtifact { * @return {{cpuQuietPeriod: TimePeriod, networkQuietPeriod: TimePeriod, cpuQuietPeriods: Array, networkQuietPeriods: Array}} */ static findOverlappingQuietPeriods(longTasks, networkRecords, traceOfTab) { - const FMPTsInMs = traceOfTab.timestamps.firstMeaningfulPaint / 1000; + const FCPTsInMs = traceOfTab.timestamps.firstContentfulPaint / 1000; /** @type {function(TimePeriod):boolean} */ const isLongEnoughQuietPeriod = period => - period.end > FMPTsInMs + REQUIRED_QUIET_WINDOW && + period.end > FCPTsInMs + REQUIRED_QUIET_WINDOW && period.end - period.start >= REQUIRED_QUIET_WINDOW; const networkQuietPeriods = this._findNetworkQuietPeriods(networkRecords, traceOfTab) .filter(isLongEnoughQuietPeriod); @@ -137,8 +145,8 @@ class ConsistentlyInteractive extends MetricArtifact { */ computeObservedMetric(data) { const {traceOfTab, networkRecords} = data; - if (!traceOfTab.timestamps.firstMeaningfulPaint) { - throw new LHError(LHError.errors.NO_FMP); + if (!traceOfTab.timestamps.firstContentfulPaint) { + throw new LHError(LHError.errors.NO_FCP); } if (!traceOfTab.timestamps.domContentLoaded) { @@ -157,7 +165,7 @@ class ConsistentlyInteractive extends MetricArtifact { const timestamp = Math.max( cpuQuietPeriod.start, - traceOfTab.timestamps.firstMeaningfulPaint / 1000, + traceOfTab.timestamps.firstContentfulPaint / 1000, traceOfTab.timestamps.domContentLoaded / 1000 ) * 1000; const timing = (timestamp - traceOfTab.timestamps.navigationStart) / 1000; diff --git a/lighthouse-core/lib/errors.js b/lighthouse-core/lib/errors.js index f78b29e3fad4..72f79e13a222 100644 --- a/lighthouse-core/lib/errors.js +++ b/lighthouse-core/lib/errors.js @@ -97,7 +97,7 @@ const ERRORS = { Object.keys(ERRORS).forEach(code => ERRORS[code].code = code); -/** @type {Object} */ +/** @type {Object} */ LighthouseError.errors = ERRORS; module.exports = LighthouseError; diff --git a/lighthouse-core/lib/network-recorder.js b/lighthouse-core/lib/network-recorder.js index 5df4dab88056..d20944a207be 100644 --- a/lighthouse-core/lib/network-recorder.js +++ b/lighthouse-core/lib/network-recorder.js @@ -95,15 +95,21 @@ class NetworkRecorder extends EventEmitter { * @param {Array} networkRecords * @param {number} allowedConcurrentRequests * @param {number=} endTime + * @param {(function(LH.WebInspector.NetworkRequest):boolean)=} filter * @return {Array<{start: number, end: number}>} */ - static findNetworkQuietPeriods(networkRecords, allowedConcurrentRequests, endTime = Infinity) { + static findNetworkQuietPeriods( + networkRecords, + allowedConcurrentRequests, + endTime = Infinity, + filter + ) { // First collect the timestamps of when requests start and end /** @type {Array<{time: number, isStart: boolean}>} */ let timeBoundaries = []; networkRecords.forEach(record => { const scheme = record.parsedURL && record.parsedURL.scheme; - if (IGNORED_NETWORK_SCHEMES.includes(scheme)) { + if (IGNORED_NETWORK_SCHEMES.includes(scheme) || (filter && !filter(record))) { return; } diff --git a/lighthouse-core/test/gather/computed/metrics/consistently-interactive-test.js b/lighthouse-core/test/gather/computed/metrics/consistently-interactive-test.js index d3a58335d1a6..0127fcc301d8 100644 --- a/lighthouse-core/test/gather/computed/metrics/consistently-interactive-test.js +++ b/lighthouse-core/test/gather/computed/metrics/consistently-interactive-test.js @@ -17,6 +17,9 @@ function generateNetworkRecords(records, navStart) { const navStartInMs = navStart / 1000; return records.map(item => { return { + failed: item.failed || false, + statusCode: item.statusCode || 200, + requestMethod: item.requestMethod || 'GET', finished: typeof item.finished === 'undefined' ? true : item.finished, startTime: (item.start + navStartInMs) / 1000, endTime: item.end === -1 ? -1 : (item.end + navStartInMs) / 1000, @@ -52,9 +55,9 @@ describe('Metrics: TTCI', () => { describe('#findOverlappingQuietPeriods', () => { it('should return entire range when no activity is present', () => { const navigationStart = 220023532; - const firstMeaningfulPaint = 2500 * 1000 + navigationStart; + const firstContentfulPaint = 2500 * 1000 + navigationStart; const traceEnd = 10000 * 1000 + navigationStart; - const traceOfTab = {timestamps: {navigationStart, firstMeaningfulPaint, traceEnd}}; + const traceOfTab = {timestamps: {navigationStart, firstContentfulPaint, traceEnd}}; const cpu = []; const network = generateNetworkRecords([], navigationStart); @@ -66,9 +69,9 @@ describe('Metrics: TTCI', () => { it('should throw when trace ended too soon after FMP', () => { const navigationStart = 220023532; - const firstMeaningfulPaint = 2500 * 1000 + navigationStart; + const firstContentfulPaint = 2500 * 1000 + navigationStart; const traceEnd = 5000 * 1000 + navigationStart; - const traceOfTab = {timestamps: {navigationStart, firstMeaningfulPaint, traceEnd}}; + const traceOfTab = {timestamps: {navigationStart, firstContentfulPaint, traceEnd}}; const cpu = []; const network = generateNetworkRecords([], navigationStart); @@ -80,9 +83,9 @@ describe('Metrics: TTCI', () => { it('should throw when CPU is quiet but network is not', () => { const navigationStart = 220023532; - const firstMeaningfulPaint = 2500 * 1000 + navigationStart; + const firstContentfulPaint = 2500 * 1000 + navigationStart; const traceEnd = 10000 * 1000 + navigationStart; - const traceOfTab = {timestamps: {navigationStart, firstMeaningfulPaint, traceEnd}}; + const traceOfTab = {timestamps: {navigationStart, firstContentfulPaint, traceEnd}}; const cpu = []; const network = generateNetworkRecords([ @@ -99,9 +102,9 @@ describe('Metrics: TTCI', () => { it('should throw when network is quiet but CPU is not', () => { const navigationStart = 220023532; - const firstMeaningfulPaint = 2500 * 1000 + navigationStart; + const firstContentfulPaint = 2500 * 1000 + navigationStart; const traceEnd = 10000 * 1000 + navigationStart; - const traceOfTab = {timestamps: {navigationStart, firstMeaningfulPaint, traceEnd}}; + const traceOfTab = {timestamps: {navigationStart, firstContentfulPaint, traceEnd}}; const cpu = [ {start: 3000, end: 8000}, @@ -115,29 +118,32 @@ describe('Metrics: TTCI', () => { }, /NO.*CPU_IDLE_PERIOD/); }); - it('should handle unfinished network requests', () => { + it('should ignore unnecessary network requests', () => { const navigationStart = 220023532; - const firstMeaningfulPaint = 2500 * 1000 + navigationStart; + const firstContentfulPaint = 2500 * 1000 + navigationStart; const traceEnd = 10000 * 1000 + navigationStart; - const traceOfTab = {timestamps: {navigationStart, firstMeaningfulPaint, traceEnd}}; + const traceOfTab = {timestamps: {navigationStart, firstContentfulPaint, traceEnd}}; const cpu = []; - const network = generateNetworkRecords([ - {start: 0, end: -1, finished: false}, - {start: 0, end: -1, finished: false}, + let network = generateNetworkRecords([ {start: 0, end: -1, finished: false}, + {start: 0, end: 11000, failed: true}, + {start: 0, end: 11000, requestMethod: 'POST'}, + {start: 0, end: 11000, statusCode: 500}, ], navigationStart); + // Triple the requests to ensure it's not just the 2-quiet kicking in + network = network.concat(network).concat(network); - assert.throws(() => { - ConsistentlyInteractive.findOverlappingQuietPeriods(cpu, network, traceOfTab); - }, /NO.*NETWORK_IDLE_PERIOD/); + const result = ConsistentlyInteractive.findOverlappingQuietPeriods(cpu, network, traceOfTab); + assert.deepEqual(result.cpuQuietPeriod, {start: 0, end: traceEnd / 1000}); + assert.deepEqual(result.networkQuietPeriod, {start: 0, end: traceEnd / 1000}); }); it('should find first overlapping quiet period', () => { const navigationStart = 220023532; - const firstMeaningfulPaint = 10000 * 1000 + navigationStart; + const firstContentfulPaint = 10000 * 1000 + navigationStart; const traceEnd = 45000 * 1000 + navigationStart; - const traceOfTab = {timestamps: {navigationStart, firstMeaningfulPaint, traceEnd}}; + const traceOfTab = {timestamps: {navigationStart, firstContentfulPaint, traceEnd}}; const cpu = [ // quiet period before FMP diff --git a/typings/web-inspector.d.ts b/typings/web-inspector.d.ts index 0c8f514c1d19..f93ab625b479 100644 --- a/typings/web-inspector.d.ts +++ b/typings/web-inspector.d.ts @@ -28,6 +28,7 @@ declare global { _resourceSize?: number; finished: boolean; + requestMethod: string; statusCode: number; redirectSource?: { url: string; From d8596ae99faba9ccd249aa025e91bdbdb36d9ade Mon Sep 17 00:00:00 2001 From: Patrick Hulce Date: Wed, 25 Apr 2018 12:22:40 -0700 Subject: [PATCH 2/3] feedback --- .../metrics/consistently-interactive.js | 17 ++++++++++------- lighthouse-core/lib/network-recorder.js | 10 ++-------- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/lighthouse-core/gather/computed/metrics/consistently-interactive.js b/lighthouse-core/gather/computed/metrics/consistently-interactive.js index 14b77fd7a9dd..87e11d0886c3 100644 --- a/lighthouse-core/gather/computed/metrics/consistently-interactive.js +++ b/lighthouse-core/gather/computed/metrics/consistently-interactive.js @@ -33,11 +33,14 @@ class ConsistentlyInteractive extends MetricArtifact { */ static _findNetworkQuietPeriods(networkRecords, traceOfTab) { const traceEndTsInMs = traceOfTab.timestamps.traceEnd / 1000; - /** @param {LH.WebInspector.NetworkRequest} record */ - const filter = record => record.finished && record.requestMethod === 'GET' && !record.failed && - Math.floor(record.statusCode / 100) < 4; - return NetworkRecorder.findNetworkQuietPeriods(networkRecords, - ALLOWED_CONCURRENT_REQUESTS, traceEndTsInMs, filter); + // Ignore records that failed, never finished, or were POST/PUT/etc. + const filteredNetworkRecords = networkRecords.filter(record => { + return record.finished && record.requestMethod === 'GET' && !record.failed && + // Consider network records that had 4xx/5xx status code as "failed" + Math.floor(record.statusCode / 100) < 4; + }); + return NetworkRecorder.findNetworkQuietPeriods(filteredNetworkRecords, + ALLOWED_CONCURRENT_REQUESTS, traceEndTsInMs); } /** @@ -87,11 +90,11 @@ class ConsistentlyInteractive extends MetricArtifact { * @return {{cpuQuietPeriod: TimePeriod, networkQuietPeriod: TimePeriod, cpuQuietPeriods: Array, networkQuietPeriods: Array}} */ static findOverlappingQuietPeriods(longTasks, networkRecords, traceOfTab) { - const FCPTsInMs = traceOfTab.timestamps.firstContentfulPaint / 1000; + const FcpTsInMs = traceOfTab.timestamps.firstContentfulPaint / 1000; /** @type {function(TimePeriod):boolean} */ const isLongEnoughQuietPeriod = period => - period.end > FCPTsInMs + REQUIRED_QUIET_WINDOW && + period.end > FcpTsInMs + REQUIRED_QUIET_WINDOW && period.end - period.start >= REQUIRED_QUIET_WINDOW; const networkQuietPeriods = this._findNetworkQuietPeriods(networkRecords, traceOfTab) .filter(isLongEnoughQuietPeriod); diff --git a/lighthouse-core/lib/network-recorder.js b/lighthouse-core/lib/network-recorder.js index d20944a207be..5df4dab88056 100644 --- a/lighthouse-core/lib/network-recorder.js +++ b/lighthouse-core/lib/network-recorder.js @@ -95,21 +95,15 @@ class NetworkRecorder extends EventEmitter { * @param {Array} networkRecords * @param {number} allowedConcurrentRequests * @param {number=} endTime - * @param {(function(LH.WebInspector.NetworkRequest):boolean)=} filter * @return {Array<{start: number, end: number}>} */ - static findNetworkQuietPeriods( - networkRecords, - allowedConcurrentRequests, - endTime = Infinity, - filter - ) { + static findNetworkQuietPeriods(networkRecords, allowedConcurrentRequests, endTime = Infinity) { // First collect the timestamps of when requests start and end /** @type {Array<{time: number, isStart: boolean}>} */ let timeBoundaries = []; networkRecords.forEach(record => { const scheme = record.parsedURL && record.parsedURL.scheme; - if (IGNORED_NETWORK_SCHEMES.includes(scheme) || (filter && !filter(record))) { + if (IGNORED_NETWORK_SCHEMES.includes(scheme)) { return; } From 302a1cc653629f3ab3484a235d8ba4ee6669c151 Mon Sep 17 00:00:00 2001 From: Patrick Hulce Date: Wed, 25 Apr 2018 12:31:27 -0700 Subject: [PATCH 3/3] simpler --- .../gather/computed/metrics/consistently-interactive.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lighthouse-core/gather/computed/metrics/consistently-interactive.js b/lighthouse-core/gather/computed/metrics/consistently-interactive.js index 87e11d0886c3..b51836c2b9b6 100644 --- a/lighthouse-core/gather/computed/metrics/consistently-interactive.js +++ b/lighthouse-core/gather/computed/metrics/consistently-interactive.js @@ -37,7 +37,7 @@ class ConsistentlyInteractive extends MetricArtifact { const filteredNetworkRecords = networkRecords.filter(record => { return record.finished && record.requestMethod === 'GET' && !record.failed && // Consider network records that had 4xx/5xx status code as "failed" - Math.floor(record.statusCode / 100) < 4; + record.statusCode < 400; }); return NetworkRecorder.findNetworkQuietPeriods(filteredNetworkRecords, ALLOWED_CONCURRENT_REQUESTS, traceEndTsInMs);