Skip to content

Commit cb1fe5a

Browse files
connorjclarkconnorjclark
authored
core(network-request): use ms instead of seconds (#14567)
1 parent af77ebb commit cb1fe5a

24 files changed

+323
-313
lines changed

‎core/audits/byte-efficiency/offscreen-images.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ class OffscreenImages extends ByteEfficiencyAudit {
151151
return images.filter(image => {
152152
if (image.wastedBytes < IGNORE_THRESHOLD_IN_BYTES) return false;
153153
if (image.wastedPercent < IGNORE_THRESHOLD_IN_PERCENT) return false;
154-
return image.requestStartTime < interactiveTimestamp / 1e6 - IGNORE_THRESHOLD_IN_MS / 1000;
154+
return image.requestStartTime < interactiveTimestamp / 1000 - IGNORE_THRESHOLD_IN_MS;
155155
});
156156
}
157157

‎core/audits/byte-efficiency/render-blocking-resources.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ class RenderBlockingResources extends Audit {
158158
const deferredNodeIds = new Set();
159159
for (const resource of artifacts.TagsBlockingFirstPaint) {
160160
// Ignore any resources that finished after observed FCP (they're clearly not render-blocking)
161-
if (resource.endTime * 1000 > fcpTsInMs) continue;
161+
if (resource.endTime > fcpTsInMs) continue;
162162
// TODO: beacon to Sentry, https://github.com/GoogleChrome/lighthouse/issues/7041
163163
if (!nodesByUrl[resource.tag.url]) continue;
164164

‎core/audits/critical-request-chains.js

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,8 @@ class CriticalRequestChains extends Audit {
7070
depth,
7171
id,
7272
node: child,
73-
chainDuration: (child.request.endTime - startTime) * 1000,
74-
chainTransferSize: (transferSize + child.request.transferSize),
73+
chainDuration: child.request.endTime - startTime,
74+
chainTransferSize: transferSize + child.request.transferSize,
7575
});
7676

7777
// Carry on walking.
@@ -96,7 +96,7 @@ class CriticalRequestChains extends Audit {
9696
transferSize: 0,
9797
};
9898
CriticalRequestChains._traverse(tree, opts => {
99-
const duration = opts.chainDuration;
99+
const duration = opts.chainDuration * 1000;
100100
if (duration > longest.duration) {
101101
longest.duration = duration;
102102
longest.transferSize = opts.chainTransferSize;
@@ -123,9 +123,9 @@ class CriticalRequestChains extends Audit {
123123
const request = opts.node.request;
124124
const simpleRequest = {
125125
url: request.url,
126-
startTime: request.startTime,
127-
endTime: request.endTime,
128-
responseReceivedTime: request.responseReceivedTime,
126+
startTime: request.startTime / 1000,
127+
endTime: request.endTime / 1000,
128+
responseReceivedTime: request.responseReceivedTime / 1000,
129129
transferSize: request.transferSize,
130130
};
131131

‎core/audits/font-display.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ class FontDisplay extends Audit {
165165
.map(record => {
166166
// In reality the end time should be calculated with paint time included
167167
// all browsers wait 3000ms to block text so we make sure 3000 is our max wasted time
168-
const wastedMs = Math.min((record.endTime - record.startTime) * 1000, 3000);
168+
const wastedMs = Math.min(record.endTime - record.startTime, 3000);
169169

170170
return {
171171
url: record.url,

‎core/audits/network-requests.js

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,8 @@ class NetworkRequests extends Audit {
4747
}
4848

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

5353
const results = records.map(record => {
5454
const endTimeDeltaMs = record.lrStatistics?.endTimeDeltaMs;
@@ -64,8 +64,8 @@ class NetworkRequests extends Audit {
6464
return {
6565
url: UrlUtils.elideDataURI(record.url),
6666
protocol: record.protocol,
67-
startTime: timeToMs(record.startTime),
68-
endTime: timeToMs(record.endTime),
67+
startTime: normalizeTime(record.startTime),
68+
endTime: normalizeTime(record.endTime),
6969
finished: record.finished,
7070
transferSize: record.transferSize,
7171
resourceSize: record.resourceSize,
@@ -112,7 +112,7 @@ class NetworkRequests extends Audit {
112112

113113
// Include starting timestamp to allow syncing requests with navStart/metric timestamps.
114114
const networkStartTimeTs = Number.isFinite(earliestStartTime) ?
115-
earliestStartTime * 1_000_000 : undefined;
115+
earliestStartTime * 1000 : undefined;
116116
tableDetails.debugData = {
117117
type: 'debugdata',
118118
networkStartTimeTs,

‎core/audits/redirects.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,9 +125,9 @@ class Redirects extends Audit {
125125
}
126126

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

133133
tableRows.push({

‎core/audits/uses-rel-preconnect.js

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import {LanternLargestContentfulPaint} from '../computed/metrics/lantern-largest
2020
// around for 10s. Meaning, the time delta between processing preconnect a request should be <10s,
2121
// otherwise it's wasted. We add a 5s margin so we are sure to capture all key requests.
2222
// @see https://github.com/GoogleChrome/lighthouse/issues/3106#issuecomment-333653747
23-
const PRECONNECT_SOCKET_MAX_IDLE = 15;
23+
const PRECONNECT_SOCKET_MAX_IDLE_IN_MS = 15_000;
2424

2525
const IGNORE_THRESHOLD_IN_MS = 50;
2626

@@ -96,7 +96,7 @@ class UsesRelPreconnectAudit extends Audit {
9696
* @return {boolean}
9797
*/
9898
static socketStartTimeIsBelowThreshold(record, mainResource) {
99-
return Math.max(0, record.startTime - mainResource.endTime) < PRECONNECT_SOCKET_MAX_IDLE;
99+
return Math.max(0, record.startTime - mainResource.endTime) < PRECONNECT_SOCKET_MAX_IDLE_IN_MS;
100100
}
101101

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

191191
const timeBetweenMainResourceAndDnsStart =
192-
firstRecordOfOrigin.startTime * 1000 -
193-
mainResource.endTime * 1000 +
192+
firstRecordOfOrigin.startTime -
193+
mainResource.endTime +
194194
firstRecordOfOrigin.timing.dnsStart;
195195

196196
const wastedMs = Math.min(connectionTime, timeBetweenMainResourceAndDnsStart);

‎core/lib/dependency-graph/network-node.js

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,17 +22,21 @@ class NetworkNode extends BaseNode {
2222
}
2323

2424
/**
25+
* In seconds.
26+
* TODO: make ms.
2527
* @return {number}
2628
*/
2729
get startTime() {
28-
return this._record.startTime * 1000 * 1000;
30+
return this._record.startTime * 1000;
2931
}
3032

3133
/**
34+
* In seconds.
35+
* TODO: make ms.
3236
* @return {number}
3337
*/
3438
get endTime() {
35-
return this._record.endTime * 1000 * 1000;
39+
return this._record.endTime * 1000;
3640
}
3741

3842
/**

‎core/lib/dependency-graph/simulator/network-analyzer.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ class NetworkAnalyzer {
150150
if (!Number.isFinite(timing.receiveHeadersEnd) || timing.receiveHeadersEnd < 0) return;
151151

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

@@ -399,8 +399,8 @@ class NetworkAnalyzer {
399399

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

‎core/lib/network-request.js

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -124,10 +124,14 @@ class NetworkRequest {
124124
this.parsedURL = /** @type {ParsedURL} */ ({scheme: ''});
125125
this.documentURL = '';
126126

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

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

220-
this.startTime = data.timestamp;
224+
// Expected to be overriden with better value in `_recomputeTimesWithResourceTiming`.
225+
this.startTime = data.timestamp * 1000;
221226

222227
this.requestMethod = data.request.method;
223228

@@ -261,7 +266,7 @@ class NetworkRequest {
261266
if (this.finished) return;
262267

263268
this.finished = true;
264-
this.endTime = data.timestamp;
269+
this.endTime = data.timestamp * 1000;
265270
if (data.encodedDataLength >= 0) {
266271
this.transferSize = data.encodedDataLength;
267272
}
@@ -279,7 +284,7 @@ class NetworkRequest {
279284
if (this.finished) return;
280285

281286
this.finished = true;
282-
this.endTime = data.timestamp;
287+
this.endTime = data.timestamp * 1000;
283288

284289
this.failed = true;
285290
this.resourceType = data.type && RESOURCE_TYPES[data.type];
@@ -305,7 +310,7 @@ class NetworkRequest {
305310
this._onResponse(data.redirectResponse, data.timestamp, data.type);
306311
this.resourceType = undefined;
307312
this.finished = true;
308-
this.endTime = data.timestamp;
313+
this.endTime = data.timestamp * 1000;
309314

310315
this._updateResponseReceivedTimeIfNecessary();
311316
}
@@ -319,7 +324,7 @@ class NetworkRequest {
319324

320325
/**
321326
* @param {LH.Crdp.Network.Response} response
322-
* @param {number} timestamp
327+
* @param {number} timestamp in seconds
323328
* @param {LH.Crdp.Network.ResponseReceivedEvent['type']=} resourceType
324329
*/
325330
_onResponse(response, timestamp, resourceType) {
@@ -330,7 +335,7 @@ class NetworkRequest {
330335

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

333-
this.responseReceivedTime = timestamp;
338+
this.responseReceivedTime = timestamp * 1000;
334339

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

480485
this.lrStatistics = {
481-
endTimeDeltaMs: (this.endTime - (this.startTime + (totalMs / 1000))) * 1000,
486+
endTimeDeltaMs: this.endTime - (this.startTime + totalMs),
482487
TCPMs: TCPMs,
483488
requestMs: requestMs,
484489
responseMs: responseMs,

‎core/test/audits/byte-efficiency/offscreen-images-test.js

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -359,7 +359,7 @@ describe('OffscreenImages audit', () => {
359359

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

379379
it('disregards images loaded after Trace End when interactive throws error', () => {
380-
const networkRecord = generateRecord({resourceSizeInKb: 100, startTime: 3});
380+
const networkRecord = generateRecord({resourceSizeInKb: 100, startTime: 3000});
381381
const artifacts = {
382382
ViewportDimensions: DEFAULT_DIMENSIONS,
383383
GatherContext: {gatherMode: 'navigation'},
@@ -420,7 +420,7 @@ describe('OffscreenImages audit', () => {
420420
resourceSize: wastedSize,
421421
transferSize: wastedSize,
422422
requestId: 'a',
423-
startTime: 1,
423+
startTime: 1000,
424424
priority: 'High',
425425
timing: {receiveHeadersEnd: 1.25},
426426
};
@@ -429,7 +429,7 @@ describe('OffscreenImages audit', () => {
429429
resourceSize: wastedSize,
430430
transferSize: wastedSize,
431431
requestId: 'b',
432-
startTime: 2.25,
432+
startTime: 2_250,
433433
priority: 'High',
434434
timing: {receiveHeadersEnd: 2.5},
435435
};
@@ -481,7 +481,7 @@ describe('OffscreenImages audit', () => {
481481
resourceSize: wastedSize,
482482
transferSize: wastedSize,
483483
requestId: 'a',
484-
startTime: 1,
484+
startTime: 1000,
485485
priority: 'High',
486486
timing: {receiveHeadersEnd: 1.25},
487487
};
@@ -490,7 +490,7 @@ describe('OffscreenImages audit', () => {
490490
resourceSize: wastedSize,
491491
transferSize: wastedSize,
492492
requestId: 'b',
493-
startTime: 1.25,
493+
startTime: 1_250,
494494
priority: 'High',
495495
timing: {receiveHeadersEnd: 1.5},
496496
};
@@ -542,8 +542,8 @@ describe('OffscreenImages audit', () => {
542542
it('rethrow error when interactive throws error in Lantern', async () => {
543543
context = {settings: {throttlingMethod: 'simulate'}, computedCache: new Map()};
544544
const networkRecords = [
545-
generateRecord({url: 'a', resourceSizeInKb: 100, startTime: 3}),
546-
generateRecord({url: 'b', resourceSizeInKb: 100, startTime: 4}),
545+
generateRecord({url: 'a', resourceSizeInKb: 100, startTime: 3000}),
546+
generateRecord({url: 'b', resourceSizeInKb: 100, startTime: 4000}),
547547
];
548548
const artifacts = {
549549
ViewportDimensions: DEFAULT_DIMENSIONS,
@@ -583,7 +583,7 @@ describe('OffscreenImages audit', () => {
583583
resourceSize: wastedSize,
584584
transferSize: 0,
585585
requestId: 'a',
586-
startTime: 1,
586+
startTime: 1000,
587587
priority: 'High',
588588
timing: {receiveHeadersEnd: 1.25},
589589
};

0 commit comments

Comments
 (0)