misc: simplifications in simulator/connection-pool (#7894)

lighthouse-core/computed/network-analysis.js

@@ -57,7 +57,7 @@ class NetworkAnalysis {
     const records = await NetworkRecords.request(devtoolsLog, context);
     const throughput = NetworkAnalyzer.estimateThroughput(records);
     const rttAndServerResponseTime = NetworkAnalysis.computeRTTAndServerResponseTime(records);
-    return {records, throughput, ...rttAndServerResponseTime};
+    return {throughput, ...rttAndServerResponseTime};
   }
 }
 

lighthouse-core/computed/page-dependency-graph.js

@@ -133,9 +133,9 @@ class PageDependencyGraph {
         rootNode.addDependent(node);
       }
 
-      const redirects = Array.from(node.record.redirects || []);
-      redirects.push(node.record);
+      if (!node.record.redirects) return;
 
+      const redirects = [...node.record.redirects, node.record];
       for (let i = 1; i < redirects.length; i++) {
         const redirectNode = networkNodeOutput.idToNodeMap.get(redirects[i - 1].requestId);
         const actualNode = networkNodeOutput.idToNodeMap.get(redirects[i].requestId);

lighthouse-core/lib/dependency-graph/simulator/connection-pool.js

@@ -139,14 +139,10 @@ module.exports = class ConnectionPool {
    * @return {?TcpConnection}
    */
   acquire(record, options = {}) {
-    if (this._connectionsByRecord.has(record)) {
-      // @ts-ignore
-      return this._connectionsByRecord.get(record);
-    }
+    if (this._connectionsByRecord.has(record)) throw new Error('Record already has a connection');
 
-    const origin = String(record.parsedURL.securityOrigin);
+    const origin = record.parsedURL.securityOrigin;
     const observedConnectionWasReused = !!this._connectionReusedByRequestId.get(record.requestId);
-    /** @type {TcpConnection[]} */
     const connections = this._connectionsByOrigin.get(origin) || [];
     const connectionToUse = this._findAvailableConnectionWithLargestCongestionWindow(connections, {
       ignoreConnectionReused: options.ignoreConnectionReused,
@@ -160,6 +156,20 @@ module.exports = class ConnectionPool {
     return connectionToUse;
   }
 
+  /**
+   * Return the connection currently being used to fetch a record. If no connection
+   * currently being used for this record, an error will be thrown.
+   *
+   * @param {LH.Artifacts.NetworkRequest} record
+   * @return {TcpConnection}
+   */
+  acquireActiveConnectionFromRecord(record) {
+    const activeConnection = this._connectionsByRecord.get(record);
+    if (!activeConnection) throw new Error('Could not find an active connection for record');
+
+    return activeConnection;
+  }
+
   /**
    * @param {LH.Artifacts.NetworkRequest} record
    */

lighthouse-core/lib/dependency-graph/simulator/simulator.js

@@ -300,8 +300,7 @@ class Simulator {
       const sizeInMb = (record.resourceSize || 0) / 1024 / 1024;
       timeElapsed = 8 + 20 * sizeInMb - timingData.timeElapsed;
     } else {
-      // If we're estimating time remaining, we already acquired a connection for this record, definitely non-null
-      const connection = /** @type {TcpConnection} */ (this._acquireConnection(record));
+      const connection = this._connectionPool.acquireActiveConnectionFromRecord(record);
       const dnsResolutionTime = this._dns.getTimeUntilResolution(record, {
         requestedAt: timingData.startTime,
         shouldUpdateCache: true,
@@ -352,8 +351,7 @@ class Simulator {
     if (node.type !== BaseNode.TYPES.NETWORK) throw new Error('Unsupported');
 
     const record = node.record;
-    // If we're updating the progress, we already acquired a connection for this record, definitely non-null
-    const connection = /** @type {TcpConnection} */ (this._acquireConnection(record));
+    const connection = this._connectionPool.acquireActiveConnectionFromRecord(record);
     const dnsResolutionTime = this._dns.getTimeUntilResolution(record, {
       requestedAt: timingData.startTime,
       shouldUpdateCache: true,
@@ -503,7 +501,7 @@ module.exports = Simulator;
  * @typedef NodeTimingIntermediate
  * @property {number} [startTime]
  * @property {number} [endTime]
- * @property {number} [queuedTime]
+ * @property {number} [queuedTime] Helpful for debugging.
  * @property {number} [estimatedTimeElapsed]
  * @property {number} [timeElapsed]
  * @property {number} [timeElapsedOvershoot]

lighthouse-core/test/lib/dependency-graph/simulator/connection-pool-test.js

@@ -85,8 +85,8 @@ describe('DependencyGraph/Simulator/ConnectionPool', () => {
       const connectionForA = pool.acquire(recordA);
       const connectionForB = pool.acquire(recordB);
       for (let i = 0; i < 10; i++) {
-        assert.equal(pool.acquire(recordA), connectionForA);
-        assert.equal(pool.acquire(recordB), connectionForB);
+        assert.equal(pool.acquireActiveConnectionFromRecord(recordA), connectionForA);
+        assert.equal(pool.acquireActiveConnectionFromRecord(recordB), connectionForB);
       }
 
       assert.deepStrictEqual(pool.connectionsInUse(), [connectionForA, connectionForB]);
@@ -152,7 +152,8 @@ describe('DependencyGraph/Simulator/ConnectionPool', () => {
       }
 
       assert.ok(pool.acquire(coldRecord, opts), 'should have acquired connection');
-      assert.ok(pool.acquire(warmRecord, opts), 'should have acquired connection');
+      assert.ok(pool.acquireActiveConnectionFromRecord(warmRecord, opts),
+        'should have acquired connection');
     });
 
     it('should acquire in order of warmness', () => {

types/artifacts.d.ts

@@ -376,7 +376,6 @@ declare global {
       }
 
       export interface NetworkAnalysis {
-        records: Array<NetworkRequest>;
         rtt: number;
         additionalRttByOrigin: Map<string, number>;
         serverResponseTimeByOrigin: Map<string, number>;