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): coarse rtt estimate on per-origin basis #15103

Merged
merged 10 commits into from
Jul 19, 2023
228 changes: 128 additions & 100 deletions core/lib/dependency-graph/simulator/network-analyzer.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,60 +132,60 @@
* 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[]>}
* @param {RequestInfo} info
* @return {number[]|number|undefined}
*/
static _estimateRTTByOriginViaConnectionTiming(records) {
return NetworkAnalyzer._estimateValueByOrigin(records, ({timing, connectionReused, record}) => {
if (connectionReused) return;

// In LR, network records are missing connection timing, but we've smuggled it in via headers.
if (global.isLightrider && record.lrStatistics) {
if (record.protocol.startsWith('h3')) {
return record.lrStatistics.TCPMs;
} else {
return [record.lrStatistics.TCPMs / 2, record.lrStatistics.TCPMs / 2];
}
static _estimateRTTViaConnectionTiming(info) {
const {timing, connectionReused, record} = info;
if (connectionReused) return;

// In LR, network records are missing connection timing, but we've smuggled it in via headers.
if (global.isLightrider && record.lrStatistics) {
if (record.protocol.startsWith('h3')) {
return record.lrStatistics.TCPMs;
} else {
return [record.lrStatistics.TCPMs / 2, record.lrStatistics.TCPMs / 2];

Check warning on line 147 in core/lib/dependency-graph/simulator/network-analyzer.js

View check run for this annotation

Codecov / codecov/patch

core/lib/dependency-graph/simulator/network-analyzer.js#L144-L147

Added lines #L144 - L147 were not covered by tests
}
}

Check warning on line 149 in core/lib/dependency-graph/simulator/network-analyzer.js

View check run for this annotation

Codecov / codecov/patch

core/lib/dependency-graph/simulator/network-analyzer.js#L149

Added line #L149 was not covered by tests

const {connectStart, sslStart, sslEnd, connectEnd} = timing;
if (connectEnd >= 0 && connectStart >= 0 && record.protocol.startsWith('h3')) {
// These values are equal to sslStart and sslEnd for h3.
return connectEnd - connectStart;
} else if (sslStart >= 0 && sslEnd >= 0 && sslStart !== connectStart) {
// SSL can also be more than 1 RT but assume False Start was used.
return [connectEnd - sslStart, sslStart - connectStart];
} else if (connectStart >= 0 && connectEnd >= 0) {
return connectEnd - connectStart;
}
});
const {connectStart, sslStart, sslEnd, connectEnd} = timing;
if (connectEnd >= 0 && connectStart >= 0 && record.protocol.startsWith('h3')) {
// These values are equal to sslStart and sslEnd for h3.
return connectEnd - connectStart;

Check warning on line 154 in core/lib/dependency-graph/simulator/network-analyzer.js

View check run for this annotation

Codecov / codecov/patch

core/lib/dependency-graph/simulator/network-analyzer.js#L153-L154

Added lines #L153 - L154 were not covered by tests
} else if (sslStart >= 0 && sslEnd >= 0 && sslStart !== connectStart) {
// SSL can also be more than 1 RT but assume False Start was used.
return [connectEnd - sslStart, sslStart - connectStart];
} else if (connectStart >= 0 && connectEnd >= 0) {
return connectEnd - connectStart;
}
}

/**
* Estimates the observed RTT to each origin based on how long a download took on a fresh connection.
* NOTE: this will tend to overestimate the actual RTT quite significantly as the download can be
* slow for other reasons as well such as bandwidth constraints.
*
* @param {LH.Artifacts.NetworkRequest[]} records
* @return {Map<string, number[]>}
* @param {RequestInfo} info
* @return {number|undefined}
*/
static _estimateRTTByOriginViaDownloadTiming(records) {
return NetworkAnalyzer._estimateValueByOrigin(records, ({record, timing, connectionReused}) => {
if (connectionReused) return;
// Only look at downloads that went past the initial congestion window
if (record.transferSize <= INITIAL_CWD) return;
if (!Number.isFinite(timing.receiveHeadersEnd) || timing.receiveHeadersEnd < 0) return;
static _estimateRTTViaDownloadTiming(info) {
const {timing, connectionReused, record} = info;
if (connectionReused) return;

// Compute the amount of time downloading everything after the first congestion window took
const totalTime = record.networkEndTime - record.networkRequestTime;
const downloadTimeAfterFirstByte = totalTime - timing.receiveHeadersEnd;
const numberOfRoundTrips = Math.log2(record.transferSize / INITIAL_CWD);
// Only look at downloads that went past the initial congestion window
if (record.transferSize <= INITIAL_CWD) return;
if (!Number.isFinite(timing.receiveHeadersEnd) || timing.receiveHeadersEnd < 0) return;

// Ignore requests that required a high number of round trips since bandwidth starts to play
// a larger role than latency
if (numberOfRoundTrips > 5) return;
return downloadTimeAfterFirstByte / numberOfRoundTrips;
});
// Compute the amount of time downloading everything after the first congestion window took
const totalTime = record.networkEndTime - record.networkRequestTime;
const downloadTimeAfterFirstByte = totalTime - timing.receiveHeadersEnd;
const numberOfRoundTrips = Math.log2(record.transferSize / INITIAL_CWD);

// Ignore requests that required a high number of round trips since bandwidth starts to play
// a larger role than latency
if (numberOfRoundTrips > 5) return;

return downloadTimeAfterFirstByte / numberOfRoundTrips;
}

/**
Expand All @@ -194,21 +194,21 @@
* NOTE: this will tend to overestimate the actual RTT as the request can be delayed for other
* reasons as well such as more SSL handshakes if TLS False Start is not enabled.
*
* @param {LH.Artifacts.NetworkRequest[]} records
* @return {Map<string, number[]>}
* @param {RequestInfo} info
* @return {number|undefined}
*/
static _estimateRTTByOriginViaSendStartTiming(records) {
return NetworkAnalyzer._estimateValueByOrigin(records, ({record, timing, connectionReused}) => {
if (connectionReused) return;
if (!Number.isFinite(timing.sendStart) || timing.sendStart < 0) return;

// Assume everything before sendStart was just DNS + (SSL)? + TCP handshake
// 1 RT for DNS, 1 RT (maybe) for SSL, 1 RT for TCP
let roundTrips = 1;
if (!record.protocol.startsWith('h3')) roundTrips += 1; // TCP
if (record.parsedURL.scheme === 'https') roundTrips += 1;
return timing.sendStart / roundTrips;
});
static _estimateRTTViaSendStartTiming(info) {
const {timing, connectionReused, record} = info;
if (connectionReused) return;

if (!Number.isFinite(timing.sendStart) || timing.sendStart < 0) return;

// Assume everything before sendStart was just DNS + (SSL)? + TCP handshake
// 1 RT for DNS, 1 RT (maybe) for SSL, 1 RT for TCP
let roundTrips = 1;
if (!record.protocol.startsWith('h3')) roundTrips += 1; // TCP
if (record.parsedURL.scheme === 'https') roundTrips += 1;
return timing.sendStart / roundTrips;
}

/**
Expand All @@ -217,34 +217,33 @@
* NOTE: this is the most inaccurate way to estimate the RTT, but in some environments it's all
* we have access to :(
*
* @param {LH.Artifacts.NetworkRequest[]} records
* @return {Map<string, number[]>}
* @param {RequestInfo} info
* @return {number|undefined}
*/
static _estimateRTTByOriginViaHeadersEndTiming(records) {
return NetworkAnalyzer._estimateValueByOrigin(records, ({record, timing, connectionReused}) => {
if (!Number.isFinite(timing.receiveHeadersEnd) || timing.receiveHeadersEnd < 0) return;
if (!record.resourceType) return;

const serverResponseTimePercentage =
SERVER_RESPONSE_PERCENTAGE_OF_TTFB[record.resourceType] ||
DEFAULT_SERVER_RESPONSE_PERCENTAGE;
const estimatedServerResponseTime = timing.receiveHeadersEnd * serverResponseTimePercentage;

// When connection was reused...
// TTFB = 1 RT for request + server response time
let roundTrips = 1;

// When connection was fresh...
// TTFB = DNS + (SSL)? + TCP handshake + 1 RT for request + server response time
if (!connectionReused) {
roundTrips += 1; // DNS
if (!record.protocol.startsWith('h3')) roundTrips += 1; // TCP
if (record.parsedURL.scheme === 'https') roundTrips += 1; // SSL
}
static _estimateRTTViaHeadersEndTiming(info) {
const {timing, connectionReused, record} = info;
if (!Number.isFinite(timing.receiveHeadersEnd) || timing.receiveHeadersEnd < 0) return;
if (!record.resourceType) return;

const serverResponseTimePercentage =
SERVER_RESPONSE_PERCENTAGE_OF_TTFB[record.resourceType] ||
DEFAULT_SERVER_RESPONSE_PERCENTAGE;
const estimatedServerResponseTime = timing.receiveHeadersEnd * serverResponseTimePercentage;

// When connection was reused...
// TTFB = 1 RT for request + server response time
let roundTrips = 1;

// When connection was fresh...
// TTFB = DNS + (SSL)? + TCP handshake + 1 RT for request + server response time
if (!connectionReused) {
roundTrips += 1; // DNS
if (!record.protocol.startsWith('h3')) roundTrips += 1; // TCP
if (record.parsedURL.scheme === 'https') roundTrips += 1; // SSL
}

// subtract out our estimated server response time
return Math.max((timing.receiveHeadersEnd - estimatedServerResponseTime) / roundTrips, 3);
});
// subtract out our estimated server response time
return Math.max((timing.receiveHeadersEnd - estimatedServerResponseTime) / roundTrips, 3);
}

/**
Expand Down Expand Up @@ -350,32 +349,61 @@
useHeadersEndEstimates = true,
} = options || {};

let estimatesByOrigin = NetworkAnalyzer._estimateRTTByOriginViaConnectionTiming(records);
if (!estimatesByOrigin.size || forceCoarseEstimates) {
estimatesByOrigin = new Map();
const estimatesViaDownload = NetworkAnalyzer._estimateRTTByOriginViaDownloadTiming(records);
const estimatesViaSendStart = NetworkAnalyzer._estimateRTTByOriginViaSendStartTiming(records);
const estimatesViaTTFB = NetworkAnalyzer._estimateRTTByOriginViaHeadersEndTiming(records);
const connectionWasReused = NetworkAnalyzer.estimateIfConnectionWasReused(records);
const groupedByOrigin = NetworkAnalyzer.groupByOrigin(records);

for (const [origin, estimates] of estimatesViaDownload.entries()) {
if (!useDownloadEstimates) continue;
estimatesByOrigin.set(origin, estimates);
const estimatesByOrigin = new Map();
for (const [origin, originRecords] of groupedByOrigin.entries()) {
/** @type {number[]} */
const originEstimates = [];

/**
* @param {(e: RequestInfo) => number[]|number|undefined} estimator
*/
// eslint-disable-next-line no-inner-declarations
function collectEstimates(estimator, multiplier = 1) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_estimateValueByOrigin still has one usage in _estimateResponseTimeByOrigin and shares a lot of the same implementation details as this function. Is it possible to create a common collectEstimates function used here and in _estimateResponseTimeByOrigin? Then we can remove _estimateValueByOrigin entirely.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not without adding indirection which would make the main use case here harder to understand (in other words, closures are useful).

Not that it couldn't be done. I think it should be a separate PR though, and not very high priority.

for (const record of originRecords) {
const timing = record.timing;
if (!timing) continue;

const estimates = estimator({
record,
timing,
connectionReused: connectionWasReused.get(record.requestId),
});
if (estimates === undefined) continue;

if (!Array.isArray(estimates)) {
originEstimates.push(estimates * multiplier);
} else {
originEstimates.push(...estimates.map(e => e * multiplier));
}
}
}

for (const [origin, estimates] of estimatesViaSendStart.entries()) {
if (!useSendStartEstimates) continue;
const existing = estimatesByOrigin.get(origin) || [];
estimatesByOrigin.set(origin, existing.concat(estimates));
if (!forceCoarseEstimates) {
collectEstimates(this._estimateRTTViaConnectionTiming);
}

for (const [origin, estimates] of estimatesViaTTFB.entries()) {
if (!useHeadersEndEstimates) continue;
const existing = estimatesByOrigin.get(origin) || [];
estimatesByOrigin.set(origin, existing.concat(estimates));
// Connection timing can be missing for a few reasons:
// - Origin was preconnected, which we don't have instrumentation for.
// - Trace began recording after a connection has already been established (for example, in timespan mode)
// - Perhaps Chrome established a connection already in the background (service worker? Just guessing here)
// - Not provided in LR netstack.
if (!originEstimates.length) {
if (useDownloadEstimates) {
collectEstimates(this._estimateRTTViaDownloadTiming, coarseEstimateMultiplier);
}
if (useSendStartEstimates) {
collectEstimates(this._estimateRTTViaSendStartTiming, coarseEstimateMultiplier);
}
if (useHeadersEndEstimates) {
collectEstimates(this._estimateRTTViaHeadersEndTiming, coarseEstimateMultiplier);
}
}

for (const estimates of estimatesByOrigin.values()) {
estimates.forEach((x, i) => (estimates[i] = x * coarseEstimateMultiplier));
if (originEstimates.length) {
estimatesByOrigin.set(origin, originEstimates);
}
}

Expand Down
24 changes: 12 additions & 12 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": 3313,
"firstContentfulPaint": 3364,
"firstContentfulPaintAllFrames": undefined,
"firstContentfulPaintAllFramesTs": undefined,
"firstContentfulPaintTs": undefined,
"firstMeaningfulPaint": 3313,
"firstMeaningfulPaint": 3364,
"firstMeaningfulPaintTs": undefined,
"interactive": 6380,
"interactive": 6354,
"interactiveTs": undefined,
"largestContentfulPaint": 5621,
"largestContentfulPaint": 5567,
"largestContentfulPaintAllFrames": undefined,
"largestContentfulPaintAllFramesTs": undefined,
"largestContentfulPaintTs": undefined,
"lcpLoadEnd": 5572,
"lcpLoadStart": 5449,
"lcpLoadEnd": 5519,
"lcpLoadStart": 5397,
"maxPotentialFID": 160,
"observedCumulativeLayoutShift": 0,
"observedCumulativeLayoutShiftMainFrame": 0,
Expand Down Expand Up @@ -49,7 +49,7 @@ Object {
"observedTimeOriginTs": 760620643599,
"observedTraceEnd": 4778,
"observedTraceEndTs": 760625421283,
"speedIndex": 6313,
"speedIndex": 6330,
"speedIndexTs": undefined,
"timeToFirstByte": 2394,
"timeToFirstByteTs": undefined,
Expand Down Expand Up @@ -118,15 +118,15 @@ exports[`Performance: metrics evaluates valid input (with lcp) correctly 1`] = `
Object {
"cumulativeLayoutShift": 0,
"cumulativeLayoutShiftMainFrame": 0,
"firstContentfulPaint": 2291,
"firstContentfulPaint": 2294,
"firstContentfulPaintAllFrames": undefined,
"firstContentfulPaintAllFramesTs": undefined,
"firstContentfulPaintTs": undefined,
"firstMeaningfulPaint": 2761,
"firstMeaningfulPaint": 2764,
"firstMeaningfulPaintTs": undefined,
"interactive": 4446,
"interactive": 4457,
"interactiveTs": undefined,
"largestContentfulPaint": 2761,
"largestContentfulPaint": 2764,
"largestContentfulPaintAllFrames": undefined,
"largestContentfulPaintAllFramesTs": undefined,
"largestContentfulPaintTs": undefined,
Expand Down Expand Up @@ -163,7 +163,7 @@ Object {
"observedTimeOriginTs": 713037023064,
"observedTraceEnd": 7416,
"observedTraceEndTs": 713044439102,
"speedIndex": 3683,
"speedIndex": 3684,
"speedIndexTs": undefined,
"timeToFirstByte": 611,
"timeToFirstByteTs": undefined,
Expand Down
28 changes: 14 additions & 14 deletions core/test/audits/__snapshots__/predictive-perf-test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,23 @@

exports[`Performance: predictive performance audit should compute the predicted values 1`] = `
Object {
"optimisticFCP": 2291,
"optimisticFMP": 2291,
"optimisticLCP": 2291,
"optimisticFCP": 2294,
"optimisticFMP": 2294,
"optimisticLCP": 2294,
"optimisticSI": 1393,
"optimisticTTI": 3792,
"pessimisticFCP": 2291,
"pessimisticFMP": 3230,
"pessimisticLCP": 3230,
"pessimisticSI": 3049,
"pessimisticTTI": 5099,
"roughEstimateOfFCP": 2291,
"roughEstimateOfFMP": 2761,
"roughEstimateOfLCP": 2761,
"optimisticTTI": 3795,
"pessimisticFCP": 2294,
"pessimisticFMP": 3233,
"pessimisticLCP": 3233,
"pessimisticSI": 3052,
"pessimisticTTI": 5119,
"roughEstimateOfFCP": 2294,
"roughEstimateOfFMP": 2764,
"roughEstimateOfLCP": 2764,
"roughEstimateOfLCPLoadEnd": undefined,
"roughEstimateOfLCPLoadStart": undefined,
"roughEstimateOfSI": 3683,
"roughEstimateOfSI": 3684,
"roughEstimateOfTTFB": 611,
"roughEstimateOfTTI": 4446,
"roughEstimateOfTTI": 4457,
}
`;
Loading
Loading