Skip to content

Commit

Permalink
core(network-request): use ms instead of seconds (#14567)
Browse files Browse the repository at this point in the history
  • Loading branch information
connorjclark committed Nov 29, 2022
1 parent af77ebb commit cb1fe5a
Show file tree
Hide file tree
Showing 24 changed files with 323 additions and 313 deletions.
2 changes: 1 addition & 1 deletion core/audits/byte-efficiency/offscreen-images.js
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ class OffscreenImages extends ByteEfficiencyAudit {
return images.filter(image => {
if (image.wastedBytes < IGNORE_THRESHOLD_IN_BYTES) return false;
if (image.wastedPercent < IGNORE_THRESHOLD_IN_PERCENT) return false;
return image.requestStartTime < interactiveTimestamp / 1e6 - IGNORE_THRESHOLD_IN_MS / 1000;
return image.requestStartTime < interactiveTimestamp / 1000 - IGNORE_THRESHOLD_IN_MS;
});
}

Expand Down
2 changes: 1 addition & 1 deletion core/audits/byte-efficiency/render-blocking-resources.js
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ class RenderBlockingResources extends Audit {
const deferredNodeIds = new Set();
for (const resource of artifacts.TagsBlockingFirstPaint) {
// Ignore any resources that finished after observed FCP (they're clearly not render-blocking)
if (resource.endTime * 1000 > fcpTsInMs) continue;
if (resource.endTime > fcpTsInMs) continue;
// TODO: beacon to Sentry, https://github.com/GoogleChrome/lighthouse/issues/7041
if (!nodesByUrl[resource.tag.url]) continue;

Expand Down
12 changes: 6 additions & 6 deletions core/audits/critical-request-chains.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,8 @@ class CriticalRequestChains extends Audit {
depth,
id,
node: child,
chainDuration: (child.request.endTime - startTime) * 1000,
chainTransferSize: (transferSize + child.request.transferSize),
chainDuration: child.request.endTime - startTime,
chainTransferSize: transferSize + child.request.transferSize,
});

// Carry on walking.
Expand All @@ -96,7 +96,7 @@ class CriticalRequestChains extends Audit {
transferSize: 0,
};
CriticalRequestChains._traverse(tree, opts => {
const duration = opts.chainDuration;
const duration = opts.chainDuration * 1000;
if (duration > longest.duration) {
longest.duration = duration;
longest.transferSize = opts.chainTransferSize;
Expand All @@ -123,9 +123,9 @@ class CriticalRequestChains extends Audit {
const request = opts.node.request;
const simpleRequest = {
url: request.url,
startTime: request.startTime,
endTime: request.endTime,
responseReceivedTime: request.responseReceivedTime,
startTime: request.startTime / 1000,
endTime: request.endTime / 1000,
responseReceivedTime: request.responseReceivedTime / 1000,
transferSize: request.transferSize,
};

Expand Down
2 changes: 1 addition & 1 deletion core/audits/font-display.js
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ class FontDisplay extends Audit {
.map(record => {
// In reality the end time should be calculated with paint time included
// all browsers wait 3000ms to block text so we make sure 3000 is our max wasted time
const wastedMs = Math.min((record.endTime - record.startTime) * 1000, 3000);
const wastedMs = Math.min(record.endTime - record.startTime, 3000);

return {
url: record.url,
Expand Down
10 changes: 5 additions & 5 deletions core/audits/network-requests.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ class NetworkRequests extends Audit {
}

/** @param {number} time */
const timeToMs = time => time < earliestStartTime || !Number.isFinite(time) ?
undefined : (time - earliestStartTime) * 1000;
const normalizeTime = time => time < earliestStartTime || !Number.isFinite(time) ?
undefined : (time - earliestStartTime);

const results = records.map(record => {
const endTimeDeltaMs = record.lrStatistics?.endTimeDeltaMs;
Expand All @@ -64,8 +64,8 @@ class NetworkRequests extends Audit {
return {
url: UrlUtils.elideDataURI(record.url),
protocol: record.protocol,
startTime: timeToMs(record.startTime),
endTime: timeToMs(record.endTime),
startTime: normalizeTime(record.startTime),
endTime: normalizeTime(record.endTime),
finished: record.finished,
transferSize: record.transferSize,
resourceSize: record.resourceSize,
Expand Down Expand Up @@ -112,7 +112,7 @@ class NetworkRequests extends Audit {

// Include starting timestamp to allow syncing requests with navStart/metric timestamps.
const networkStartTimeTs = Number.isFinite(earliestStartTime) ?
earliestStartTime * 1_000_000 : undefined;
earliestStartTime * 1000 : undefined;
tableDetails.debugData = {
type: 'debugdata',
networkStartTimeTs,
Expand Down
4 changes: 2 additions & 2 deletions core/audits/redirects.js
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,9 @@ class Redirects extends Audit {
}

const lanternTimingDeltaMs = redirectedTiming.startTime - initialTiming.startTime;
const observedTimingDeltaS = redirectedRequest.startTime - initialRequest.startTime;
const observedTimingDeltaMs = redirectedRequest.startTime - initialRequest.startTime;
const wastedMs = settings.throttlingMethod === 'simulate' ?
lanternTimingDeltaMs : observedTimingDeltaS * 1000;
lanternTimingDeltaMs : observedTimingDeltaMs;
totalWastedMs += wastedMs;

tableRows.push({
Expand Down
10 changes: 5 additions & 5 deletions core/audits/uses-rel-preconnect.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import {LanternLargestContentfulPaint} from '../computed/metrics/lantern-largest
// around for 10s. Meaning, the time delta between processing preconnect a request should be <10s,
// otherwise it's wasted. We add a 5s margin so we are sure to capture all key requests.
// @see https://github.com/GoogleChrome/lighthouse/issues/3106#issuecomment-333653747
const PRECONNECT_SOCKET_MAX_IDLE = 15;
const PRECONNECT_SOCKET_MAX_IDLE_IN_MS = 15_000;

const IGNORE_THRESHOLD_IN_MS = 50;

Expand Down Expand Up @@ -96,7 +96,7 @@ class UsesRelPreconnectAudit extends Audit {
* @return {boolean}
*/
static socketStartTimeIsBelowThreshold(record, mainResource) {
return Math.max(0, record.startTime - mainResource.endTime) < PRECONNECT_SOCKET_MAX_IDLE;
return Math.max(0, record.startTime - mainResource.endTime) < PRECONNECT_SOCKET_MAX_IDLE_IN_MS;
}

/**
Expand Down Expand Up @@ -150,7 +150,7 @@ class UsesRelPreconnectAudit extends Audit {
!lcpGraphURLs.has(record.url) ||
// Filter out all resources where origins are already resolved.
UsesRelPreconnectAudit.hasAlreadyConnectedToOrigin(record) ||
// Make sure the requests are below the PRECONNECT_SOCKET_MAX_IDLE (15s) mark.
// Make sure the requests are below the PRECONNECT_SOCKET_MAX_IDLE_IN_MS (15s) mark.
!UsesRelPreconnectAudit.socketStartTimeIsBelowThreshold(record, mainResource)
) {
return;
Expand Down Expand Up @@ -189,8 +189,8 @@ class UsesRelPreconnectAudit extends Audit {
if (firstRecordOfOrigin.parsedURL.scheme === 'https') connectionTime = connectionTime * 2;

const timeBetweenMainResourceAndDnsStart =
firstRecordOfOrigin.startTime * 1000 -
mainResource.endTime * 1000 +
firstRecordOfOrigin.startTime -
mainResource.endTime +
firstRecordOfOrigin.timing.dnsStart;

const wastedMs = Math.min(connectionTime, timeBetweenMainResourceAndDnsStart);
Expand Down
8 changes: 6 additions & 2 deletions core/lib/dependency-graph/network-node.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,21 @@ class NetworkNode extends BaseNode {
}

/**
* In seconds.
* TODO: make ms.
* @return {number}
*/
get startTime() {
return this._record.startTime * 1000 * 1000;
return this._record.startTime * 1000;
}

/**
* In seconds.
* TODO: make ms.
* @return {number}
*/
get endTime() {
return this._record.endTime * 1000 * 1000;
return this._record.endTime * 1000;
}

/**
Expand Down
6 changes: 3 additions & 3 deletions core/lib/dependency-graph/simulator/network-analyzer.js
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ class NetworkAnalyzer {
if (!Number.isFinite(timing.receiveHeadersEnd) || timing.receiveHeadersEnd < 0) return;

// Compute the amount of time downloading everything after the first congestion window took
const totalTime = (record.endTime - record.startTime) * 1000;
const totalTime = record.endTime - record.startTime;
const downloadTimeAfterFirstByte = totalTime - timing.receiveHeadersEnd;
const numberOfRoundTrips = Math.log2(record.transferSize / INITIAL_CWD);

Expand Down Expand Up @@ -399,8 +399,8 @@ class NetworkAnalyzer {

// If we've made it this far, all the times we need should be valid (i.e. not undefined/-1).
totalBytes += record.transferSize;
boundaries.push({time: record.responseReceivedTime, isStart: true});
boundaries.push({time: record.endTime, isStart: false});
boundaries.push({time: record.responseReceivedTime / 1000, isStart: true});
boundaries.push({time: record.endTime / 1000, isStart: false});
return boundaries;
}, /** @type {Array<{time: number, isStart: boolean}>} */([])).sort((a, b) => a.time - b.time);

Expand Down
27 changes: 16 additions & 11 deletions core/lib/network-request.js
Original file line number Diff line number Diff line change
Expand Up @@ -124,10 +124,14 @@ class NetworkRequest {
this.parsedURL = /** @type {ParsedURL} */ ({scheme: ''});
this.documentURL = '';

/**
* When the network service is about to handle a request, ie. just before going to the
* HTTP cache or going to the network for DNS/connection setup, in milliseconds.
*/
this.startTime = -1;
/** @type {number} */
/** When the last byte of the response body is received, in milliseconds. */
this.endTime = -1;
/** @type {number} */
/** When the last byte of the response headers is received, in milliseconds. */
this.responseReceivedTime = -1;

// Go read the comment on _updateTransferSizeForLightrider.
Expand Down Expand Up @@ -217,7 +221,8 @@ class NetworkRequest {
};
this.isSecure = UrlUtils.isSecureScheme(this.parsedURL.scheme);

this.startTime = data.timestamp;
// Expected to be overriden with better value in `_recomputeTimesWithResourceTiming`.
this.startTime = data.timestamp * 1000;

this.requestMethod = data.request.method;

Expand Down Expand Up @@ -261,7 +266,7 @@ class NetworkRequest {
if (this.finished) return;

this.finished = true;
this.endTime = data.timestamp;
this.endTime = data.timestamp * 1000;
if (data.encodedDataLength >= 0) {
this.transferSize = data.encodedDataLength;
}
Expand All @@ -279,7 +284,7 @@ class NetworkRequest {
if (this.finished) return;

this.finished = true;
this.endTime = data.timestamp;
this.endTime = data.timestamp * 1000;

this.failed = true;
this.resourceType = data.type && RESOURCE_TYPES[data.type];
Expand All @@ -305,7 +310,7 @@ class NetworkRequest {
this._onResponse(data.redirectResponse, data.timestamp, data.type);
this.resourceType = undefined;
this.finished = true;
this.endTime = data.timestamp;
this.endTime = data.timestamp * 1000;

this._updateResponseReceivedTimeIfNecessary();
}
Expand All @@ -319,7 +324,7 @@ class NetworkRequest {

/**
* @param {LH.Crdp.Network.Response} response
* @param {number} timestamp
* @param {number} timestamp in seconds
* @param {LH.Crdp.Network.ResponseReceivedEvent['type']=} resourceType
*/
_onResponse(response, timestamp, resourceType) {
Expand All @@ -330,7 +335,7 @@ class NetworkRequest {

if (response.protocol) this.protocol = response.protocol;

this.responseReceivedTime = timestamp;
this.responseReceivedTime = timestamp * 1000;

this.transferSize = response.encodedDataLength;
if (typeof response.fromDiskCache === 'boolean') this.fromDiskCache = response.fromDiskCache;
Expand Down Expand Up @@ -364,8 +369,8 @@ class NetworkRequest {
// Take startTime and responseReceivedTime from timing data for better accuracy.
// Timing's requestTime is a baseline in seconds, rest of the numbers there are ticks in millis.
// TODO: This skips the "queuing time" before the netstack has taken over ... is this a mistake?
this.startTime = timing.requestTime;
const headersReceivedTime = timing.requestTime + timing.receiveHeadersEnd / 1000;
this.startTime = timing.requestTime * 1000;
const headersReceivedTime = this.startTime + timing.receiveHeadersEnd;
if (!this.responseReceivedTime || this.responseReceivedTime < 0) {
this.responseReceivedTime = headersReceivedTime;
}
Expand Down Expand Up @@ -478,7 +483,7 @@ class NetworkRequest {
}

this.lrStatistics = {
endTimeDeltaMs: (this.endTime - (this.startTime + (totalMs / 1000))) * 1000,
endTimeDeltaMs: this.endTime - (this.startTime + totalMs),
TCPMs: TCPMs,
requestMs: requestMs,
responseMs: responseMs,
Expand Down
18 changes: 9 additions & 9 deletions core/test/audits/byte-efficiency/offscreen-images-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@ describe('OffscreenImages audit', () => {

it('disregards images loaded after TTI', () => {
const topLevelTasks = [{ts: 1900, duration: 100}];
const networkRecord = generateRecord({resourceSizeInKb: 100, startTime: 3});
const networkRecord = generateRecord({resourceSizeInKb: 100, startTime: 3000});
const artifacts = {
ViewportDimensions: DEFAULT_DIMENSIONS,
GatherContext: {gatherMode: 'navigation'},
Expand All @@ -377,7 +377,7 @@ describe('OffscreenImages audit', () => {
});

it('disregards images loaded after Trace End when interactive throws error', () => {
const networkRecord = generateRecord({resourceSizeInKb: 100, startTime: 3});
const networkRecord = generateRecord({resourceSizeInKb: 100, startTime: 3000});
const artifacts = {
ViewportDimensions: DEFAULT_DIMENSIONS,
GatherContext: {gatherMode: 'navigation'},
Expand Down Expand Up @@ -420,7 +420,7 @@ describe('OffscreenImages audit', () => {
resourceSize: wastedSize,
transferSize: wastedSize,
requestId: 'a',
startTime: 1,
startTime: 1000,
priority: 'High',
timing: {receiveHeadersEnd: 1.25},
};
Expand All @@ -429,7 +429,7 @@ describe('OffscreenImages audit', () => {
resourceSize: wastedSize,
transferSize: wastedSize,
requestId: 'b',
startTime: 2.25,
startTime: 2_250,
priority: 'High',
timing: {receiveHeadersEnd: 2.5},
};
Expand Down Expand Up @@ -481,7 +481,7 @@ describe('OffscreenImages audit', () => {
resourceSize: wastedSize,
transferSize: wastedSize,
requestId: 'a',
startTime: 1,
startTime: 1000,
priority: 'High',
timing: {receiveHeadersEnd: 1.25},
};
Expand All @@ -490,7 +490,7 @@ describe('OffscreenImages audit', () => {
resourceSize: wastedSize,
transferSize: wastedSize,
requestId: 'b',
startTime: 1.25,
startTime: 1_250,
priority: 'High',
timing: {receiveHeadersEnd: 1.5},
};
Expand Down Expand Up @@ -542,8 +542,8 @@ describe('OffscreenImages audit', () => {
it('rethrow error when interactive throws error in Lantern', async () => {
context = {settings: {throttlingMethod: 'simulate'}, computedCache: new Map()};
const networkRecords = [
generateRecord({url: 'a', resourceSizeInKb: 100, startTime: 3}),
generateRecord({url: 'b', resourceSizeInKb: 100, startTime: 4}),
generateRecord({url: 'a', resourceSizeInKb: 100, startTime: 3000}),
generateRecord({url: 'b', resourceSizeInKb: 100, startTime: 4000}),
];
const artifacts = {
ViewportDimensions: DEFAULT_DIMENSIONS,
Expand Down Expand Up @@ -583,7 +583,7 @@ describe('OffscreenImages audit', () => {
resourceSize: wastedSize,
transferSize: 0,
requestId: 'a',
startTime: 1,
startTime: 1000,
priority: 'High',
timing: {receiveHeadersEnd: 1.25},
};
Expand Down
Loading

0 comments on commit cb1fe5a

Please sign in to comment.