Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core(network-analyzer): infer single rtt estimate for h3 #15095

Merged
merged 2 commits into from
May 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 13 additions & 7 deletions core/lib/dependency-graph/simulator/network-analyzer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, number[]>}
*/
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;
Expand Down Expand Up @@ -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);
Expand Down
14 changes: 7 additions & 7 deletions core/test/audits/__snapshots__/metrics-test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -50,7 +50,7 @@ Object {
"observedTotalCumulativeLayoutShift": 0,
"observedTraceEnd": 4778,
"observedTraceEndTs": 760625421283,
"speedIndex": 6339,
"speedIndex": 6313,
"speedIndexTs": undefined,
"timeToFirstByte": 2394,
"timeToFirstByteTs": undefined,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions core/test/computed/metrics/lcp-breakdown-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down Expand Up @@ -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
Expand Down