diff --git a/core/lib/dependency-graph/simulator/network-analyzer.js b/core/lib/dependency-graph/simulator/network-analyzer.js index 7018af991727..f64ea557e524 100644 --- a/core/lib/dependency-graph/simulator/network-analyzer.js +++ b/core/lib/dependency-graph/simulator/network-analyzer.js @@ -114,19 +114,25 @@ class NetworkAnalyzer { } /** - * Estimates the observed RTT to each origin based on how long the TCP handshake took. + * Estimates the observed RTT to each origin based on how long the connection setup. + * For h1 and h2, this could includes two estimates - one for the TCP handshake, another for + * SSL negotiation. + * For h3, we get only one estimate since QUIC establishes a secure connection in a + * single handshake. * This is the most accurate and preferred method of measurement when the data is available. * * @param {LH.Artifacts.NetworkRequest[]} records * @return {Map} */ - static _estimateRTTByOriginViaTCPTiming(records) { - return NetworkAnalyzer._estimateValueByOrigin(records, ({timing, connectionReused}) => { + static _estimateRTTByOriginViaConnectionTiming(records) { + return NetworkAnalyzer._estimateValueByOrigin(records, ({timing, connectionReused, record}) => { if (connectionReused) return; - // If the request was SSL we get two estimates, one for the SSL negotiation and another for the - // regular handshake. SSL can also be more than 1 RT but assume False Start was used. - if (timing.sslStart > 0 && timing.sslEnd > 0) { + if (timing.connectEnd > 0 && timing.connectStart > 0 && record.protocol.startsWith('h3')) { + // These values are equal to sslStart and sslEnd for h3. + return timing.connectEnd - timing.connectStart; + } else if (timing.sslStart > 0 && timing.sslEnd > 0) { + // SSL can also be more than 1 RT but assume False Start was used. return [timing.connectEnd - timing.sslStart, timing.sslStart - timing.connectStart]; } else if (timing.connectStart > 0 && timing.connectEnd > 0) { return timing.connectEnd - timing.connectStart; @@ -318,7 +324,7 @@ class NetworkAnalyzer { useHeadersEndEstimates = true, } = options || {}; - let estimatesByOrigin = NetworkAnalyzer._estimateRTTByOriginViaTCPTiming(records); + let estimatesByOrigin = NetworkAnalyzer._estimateRTTByOriginViaConnectionTiming(records); if (!estimatesByOrigin.size || forceCoarseEstimates) { estimatesByOrigin = new Map(); const estimatesViaDownload = NetworkAnalyzer._estimateRTTByOriginViaDownloadTiming(records); diff --git a/core/test/audits/__snapshots__/metrics-test.js.snap b/core/test/audits/__snapshots__/metrics-test.js.snap index c54c1568072d..10479d75be5c 100644 --- a/core/test/audits/__snapshots__/metrics-test.js.snap +++ b/core/test/audits/__snapshots__/metrics-test.js.snap @@ -4,20 +4,20 @@ exports[`Performance: metrics evaluates valid input (with image lcp) correctly 1 Object { "cumulativeLayoutShift": 0, "cumulativeLayoutShiftMainFrame": 0, - "firstContentfulPaint": 3364, + "firstContentfulPaint": 3313, "firstContentfulPaintAllFrames": undefined, "firstContentfulPaintAllFramesTs": undefined, "firstContentfulPaintTs": undefined, - "firstMeaningfulPaint": 3364, + "firstMeaningfulPaint": 3313, "firstMeaningfulPaintTs": undefined, - "interactive": 6412, + "interactive": 6380, "interactiveTs": undefined, - "largestContentfulPaint": 5578, + "largestContentfulPaint": 5621, "largestContentfulPaintAllFrames": undefined, "largestContentfulPaintAllFramesTs": undefined, "largestContentfulPaintTs": undefined, - "lcpLoadEnd": 5529, - "lcpLoadStart": 5407, + "lcpLoadEnd": 5572, + "lcpLoadStart": 5449, "maxPotentialFID": 160, "observedCumulativeLayoutShift": 0, "observedCumulativeLayoutShiftMainFrame": 0, @@ -50,7 +50,7 @@ Object { "observedTotalCumulativeLayoutShift": 0, "observedTraceEnd": 4778, "observedTraceEndTs": 760625421283, - "speedIndex": 6339, + "speedIndex": 6313, "speedIndexTs": undefined, "timeToFirstByte": 2394, "timeToFirstByteTs": undefined, diff --git a/core/test/audits/byte-efficiency/render-blocking-resources-test.js b/core/test/audits/byte-efficiency/render-blocking-resources-test.js index 19970413fe37..1ea43804c454 100644 --- a/core/test/audits/byte-efficiency/render-blocking-resources-test.js +++ b/core/test/audits/byte-efficiency/render-blocking-resources-test.js @@ -81,7 +81,7 @@ describe('Render blocking resources audit', () => { { 'totalBytes': 621, 'url': 'https://fonts.googleapis.com/css?family=Fira+Sans+Condensed%3A400%2C400i%2C600%2C600i&subset=latin%2Clatin-ext&display=swap', - 'wastedMs': 440, + 'wastedMs': 465, }, // Due to internal H2 simulation details, parallel HTTP/2 requests are pipelined which makes // it look like Montserrat starts after Fira Sans finishes. It would be preferred diff --git a/core/test/computed/metrics/lcp-breakdown-test.js b/core/test/computed/metrics/lcp-breakdown-test.js index 86d04d4346dc..ebcd6ee1caa0 100644 --- a/core/test/computed/metrics/lcp-breakdown-test.js +++ b/core/test/computed/metrics/lcp-breakdown-test.js @@ -102,8 +102,8 @@ describe('LCPBreakdown', () => { const result = await LCPBreakdown.request(data, {computedCache: new Map()}); expect(result.ttfb).toBeCloseTo(2393.7, 0.1); - expect(result.loadStart).toBeCloseTo(5407.1, 0.1); - expect(result.loadEnd).toBeCloseTo(5529.3, 0.1); + expect(result.loadStart).toBeCloseTo(5449.0, 0.1); + expect(result.loadEnd).toBeCloseTo(5572.1, 0.1); }); it('returns breakdown for a real trace with text LCP', async () => { diff --git a/core/test/lib/dependency-graph/simulator/network-analyzer-test.js b/core/test/lib/dependency-graph/simulator/network-analyzer-test.js index 266b128c8a09..ad726b9b22ef 100644 --- a/core/test/lib/dependency-graph/simulator/network-analyzer-test.js +++ b/core/test/lib/dependency-graph/simulator/network-analyzer-test.js @@ -29,7 +29,7 @@ describe('DependencyGraph/Simulator/NetworkAnalyzer', () => { networkRequestTime: 10, networkEndTime: 10, transferSize: 0, - protocol: 'http/1.1', + protocol: opts.protocol || 'http/1.1', parsedURL: {scheme: url.match(/https?/)[0], securityOrigin: url.match(/.*\.com/)[0]}, timing: opts.timing || null, }, @@ -143,6 +143,23 @@ describe('DependencyGraph/Simulator/NetworkAnalyzer', () => { assert.deepStrictEqual(result.get('https://example.com'), expected); }); + it('should infer from tcp and ssl timing when available', () => { + const timing = {connectStart: 1, connectEnd: 100, sslStart: 50, sslEnd: 100}; + const record = createRecord({networkRequestTime: 0, networkEndTime: 1, timing}); + const result = NetworkAnalyzer.estimateRTTByOrigin([record]); + const expected = {min: 49, max: 50, avg: 49.5, median: 49}; + assert.deepStrictEqual(result.get('https://example.com'), expected); + }); + + it('should infer from connection timing when available for h3 (one estimate)', () => { + const timing = {connectStart: 1, connectEnd: 100, sslStart: 1, sslEnd: 100}; + const record = + createRecord({networkRequestTime: 0, networkEndTime: 1, timing, protocol: 'h3'}); + const result = NetworkAnalyzer.estimateRTTByOrigin([record]); + const expected = {min: 99, max: 99, avg: 99, median: 99}; + assert.deepStrictEqual(result.get('https://example.com'), expected); + }); + it('should infer from sendStart when available', () => { const timing = {sendStart: 150}; // this record took 150ms before Chrome could send the request