core(lantern): always use flexible network ordering

core/audits/dobetterweb/uses-http2.js

@@ -78,11 +78,9 @@ class UsesHTTP2Audit extends Audit {
     options = Object.assign({label: ''}, options);
     const beforeLabel = `${this.meta.id}-${options.label}-before`;
     const afterLabel = `${this.meta.id}-${options.label}-after`;
-    const flexibleOrdering = true;
 
     const urlsToChange = new Set(results.map(result => result.url));
-    const simulationBefore =
-      simulator.simulate(graph, {label: beforeLabel, flexibleOrdering});
+    const simulationBefore = simulator.simulate(graph, {label: beforeLabel});
 
     // Update all the protocols to reflect implementing our recommendations
     /** @type {Map<string, string>} */
@@ -95,7 +93,7 @@ class UsesHTTP2Audit extends Audit {
       node.request.protocol = 'h2';
     });
 
-    const simulationAfter = simulator.simulate(graph, {label: afterLabel, flexibleOrdering});
+    const simulationAfter = simulator.simulate(graph, {label: afterLabel});
 
     // Restore the original protocol after we've done our simulation
     graph.traverse(node => {

core/audits/prioritize-lcp-image.js

@@ -194,8 +194,8 @@ class PrioritizeLcpImage extends Audit {
     modifiedLCPNode.removeAllDependencies();
     modifiedLCPNode.addDependency(mainDocumentNode);
 
-    const simulationBeforeChanges = simulator.simulate(graph, {flexibleOrdering: true});
-    const simulationAfterChanges = simulator.simulate(modifiedGraph, {flexibleOrdering: true});
+    const simulationBeforeChanges = simulator.simulate(graph);
+    const simulationAfterChanges = simulator.simulate(modifiedGraph);
     const lcpTimingsBefore = simulationBeforeChanges.nodeTimings.get(lcpNode);
     if (!lcpTimingsBefore) throw new Error('Impossible - node timings should never be undefined');
     const lcpTimingsAfter = simulationAfterChanges.nodeTimings.get(modifiedLCPNode);

core/audits/uses-rel-preload.js

@@ -143,9 +143,9 @@ class UsesRelPreloadAudit extends Audit {
       return {wastedMs: 0, results: []};
     }
 
-    // Preload changes the ordering of requests, simulate the original graph with flexible ordering
+    // Preload changes the ordering of requests, simulate the original graph
     // to have a reasonable baseline for comparison.
-    const simulationBeforeChanges = simulator.simulate(graph, {flexibleOrdering: true});
+    const simulationBeforeChanges = simulator.simulate(graph);
     const modifiedGraph = graph.cloneWithRelationships();
 
     /** @type {Array<LH.Gatherer.Simulation.GraphNetworkNode>} */
@@ -174,8 +174,8 @@ class UsesRelPreloadAudit extends Audit {
       node.addDependency(mainDocumentNode);
     }
 
-    // Once we've modified the dependencies, simulate the new graph with flexible ordering.
-    const simulationAfterChanges = simulator.simulate(modifiedGraph, {flexibleOrdering: true});
+    // Once we've modified the dependencies, simulate the new graph.
+    const simulationAfterChanges = simulator.simulate(modifiedGraph);
     const originalNodesByRecord = Array.from(simulationBeforeChanges.nodeTimings.keys())
         // @ts-expect-error we don't care if all nodes without a record collect on `undefined`
         .reduce((map, node) => map.set(node.record, node), new Map());

core/lib/lantern/metric.js

@@ -101,11 +101,11 @@ class Metric {
     const optimisticGraph = this.getOptimisticGraph(graph, processedNavigation);
     const pessimisticGraph = this.getPessimisticGraph(graph, processedNavigation);
 
-    /** @type {{flexibleOrdering?: boolean, label?: string}} */
     let simulateOptions = {label: `optimistic${metricName}`};
     const optimisticSimulation = simulator.simulate(optimisticGraph, simulateOptions);
 
-    simulateOptions = {label: `optimisticFlex${metricName}`, flexibleOrdering: true};
+    // TODO ! remove this.
+    simulateOptions = {label: `optimisticFlex${metricName}`};
     const optimisticFlexSimulation = simulator.simulate(optimisticGraph, simulateOptions);
 
     simulateOptions = {label: `pessimistic${metricName}`};

core/lib/lantern/metrics/interactive.js

@@ -21,8 +21,8 @@ class Interactive extends Metric {
   static get COEFFICIENTS() {
     return {
       intercept: 0,
-      optimistic: 0.5,
-      pessimistic: 0.5,
+      optimistic: 0.45,
+      pessimistic: 0.45,
     };
   }
 

core/lib/lantern/metrics/speed-index.js

@@ -18,12 +18,9 @@ class SpeedIndex extends Metric {
    */
   static get COEFFICIENTS() {
     return {
-      // Negative intercept is OK because estimate is Math.max(FCP, Speed Index) and
-      // the optimistic estimate is based on the real observed speed index rather than a real
-      // lantern graph.
-      intercept: -250,
+      intercept: 0,
       optimistic: 1.4,
-      pessimistic: 0.65,
+      pessimistic: 0.4,
     };
   }
 

core/lib/lantern/simulator/connection-pool.js

@@ -85,24 +85,13 @@ export class ConnectionPool {
 
   /**
    * @param {Array<TcpConnection>} connections
-   * @param {{ignoreConnectionReused?: boolean, observedConnectionWasReused: boolean}} options
    */
-  _findAvailableConnectionWithLargestCongestionWindow(connections, options) {
-    const {ignoreConnectionReused, observedConnectionWasReused} = options;
-
+  _findAvailableConnectionWithLargestCongestionWindow(connections) {
     /** @type {TcpConnection|null} */
     let maxConnection = null;
     for (let i = 0; i < connections.length; i++) {
       const connection = connections[i];
 
-      // Normally, we want to make sure the connection warmth matches the state of the record
-      // we're acquiring for. Do this check first since it's the common case and cheaper than our
-      // "in use" check below.
-      // Use the _warmed property instead of the getter because this is a surprisingly hot code path.
-      if (!ignoreConnectionReused && connection._warmed !== observedConnectionWasReused) {
-        continue;
-      }
-
       // Connections that are in use are never available.
       if (this._connectionsInUse.has(connection)) {
         continue;
@@ -121,23 +110,15 @@ export class ConnectionPool {
    * if no connection was available. If returned, connection will not be available for other network
    * records until release is called.
    *
-   * If ignoreConnectionReused is true, acquire will consider all connections not in use as available.
-   * Otherwise, only connections that have matching "warmth" are considered available.
-   *
    * @param {Lantern.NetworkRequest} record
-   * @param {{ignoreConnectionReused?: boolean}} options
    * @return {?TcpConnection}
    */
-  acquire(record, options = {}) {
+  acquire(record) {
     if (this._connectionsByRecord.has(record)) throw new Error('Record already has a connection');
 
     const origin = record.parsedURL.securityOrigin;
-    const observedConnectionWasReused = !!this._connectionReusedByRequestId.get(record.requestId);
     const connections = this._connectionsByOrigin.get(origin) || [];
-    const connectionToUse = this._findAvailableConnectionWithLargestCongestionWindow(connections, {
-      ignoreConnectionReused: options.ignoreConnectionReused,
-      observedConnectionWasReused,
-    });
+    const connectionToUse = this._findAvailableConnectionWithLargestCongestionWindow(connections);
 
     if (!connectionToUse) return null;
 

core/lib/lantern/simulator/simulator.js

@@ -80,7 +80,6 @@ class Simulator {
     this._cachedNodeListByStartPosition = [];
 
     // Properties reset on every `.simulate` call but duplicated here for type checking
-    this._flexibleOrdering = false;
     this._nodeTimings = new SimulatorTimingMap();
     /** @type {Map<string, number>} */
     this._numberInProgressByType = new Map();
@@ -197,9 +196,7 @@ class Simulator {
    * @return {?TcpConnection}
    */
   _acquireConnection(record) {
-    return this._connectionPool.acquire(record, {
-      ignoreConnectionReused: this._flexibleOrdering,
-    });
+    return this._connectionPool.acquire(record);
   }
 
   /**
@@ -425,13 +422,13 @@ class Simulator {
    * Estimates the time taken to process all of the graph's nodes, returns the overall time along with
    * each node annotated by start/end times.
    *
-   * If flexibleOrdering is set, simulator/connection pool are allowed to deviate from what was
+   * Simulator/connection pool are allowed to deviate from what was
    * observed in the trace/devtoolsLog and start requests as soon as they are queued (i.e. do not
    * wait around for a warm connection to be available if the original record was fetched on a warm
    * connection).
    *
    * @param {Node} graph
-   * @param {{flexibleOrdering?: boolean, label?: string}=} options
+   * @param {{label?: string}=} options
    * @return {Lantern.Simulation.Result<T>}
    */
   simulate(graph, options) {
@@ -441,11 +438,9 @@ class Simulator {
 
     options = Object.assign({
       label: undefined,
-      flexibleOrdering: false,
     }, options);
 
     // initialize the necessary data containers
-    this._flexibleOrdering = !!options.flexibleOrdering;
     this._dns = new DNSCache({rtt: this._rtt});
     this._initializeConnectionPool(graph);
     this._initializeAuxiliaryData();
@@ -470,11 +465,9 @@ class Simulator {
       }
 
       if (!nodesInProgress.size) {
-        // interplay between fromDiskCache and connectionReused can be incorrect
-        // proceed with flexibleOrdering if we can, otherwise give up
-        if (this._flexibleOrdering) throw new Error('Failed to start a node');
-        this._flexibleOrdering = true;
-        continue;
+        // Interplay between fromDiskCache and connectionReused can be incorrect,
+        // have to give up.
+        throw new Error('Failed to start a node');
       }
 
       // set the available throughput for all connections based on # inflight

core/test/audits/__snapshots__/metrics-test.js.snap

@@ -10,7 +10,7 @@ Object {
   "firstContentfulPaintTs": undefined,
   "firstMeaningfulPaint": 3364,
   "firstMeaningfulPaintTs": undefined,
-  "interactive": 6354,
+  "interactive": 5648,
   "interactiveTs": undefined,
   "largestContentfulPaint": 5567,
   "largestContentfulPaintAllFrames": undefined,
@@ -49,7 +49,7 @@ Object {
   "observedTimeOriginTs": 760620643599,
   "observedTraceEnd": 4778,
   "observedTraceEndTs": 760625421283,
-  "speedIndex": 6330,
+  "speedIndex": 5546,
   "speedIndexTs": undefined,
   "timeToFirstByte": 2394,
   "timeToFirstByteTs": undefined,
@@ -124,7 +124,7 @@ Object {
   "firstContentfulPaintTs": undefined,
   "firstMeaningfulPaint": 2764,
   "firstMeaningfulPaintTs": undefined,
-  "interactive": 4457,
+  "interactive": 4080,
   "interactiveTs": undefined,
   "largestContentfulPaint": 2764,
   "largestContentfulPaintAllFrames": undefined,
@@ -163,7 +163,7 @@ Object {
   "observedTimeOriginTs": 713037023064,
   "observedTraceEnd": 7416,
   "observedTraceEndTs": 713044439102,
-  "speedIndex": 3684,
+  "speedIndex": 3172,
   "speedIndexTs": undefined,
   "timeToFirstByte": 611,
   "timeToFirstByteTs": undefined,
@@ -238,7 +238,7 @@ Object {
   "firstContentfulPaintTs": undefined,
   "firstMeaningfulPaint": 1541,
   "firstMeaningfulPaintTs": undefined,
-  "interactive": 4206,
+  "interactive": 3578,
   "interactiveTs": undefined,
   "largestContentfulPaint": undefined,
   "largestContentfulPaintAllFrames": undefined,
@@ -277,7 +277,7 @@ Object {
   "observedTimeOriginTs": 225414172015,
   "observedTraceEnd": 12540,
   "observedTraceEndTs": 225426711887,
-  "speedIndex": 1676,
+  "speedIndex": 1511,
   "speedIndexTs": undefined,
   "timeToFirstByte": 760,
   "timeToFirstByteTs": undefined,

core/test/audits/__snapshots__/predictive-perf-test.js.snap

@@ -11,14 +11,14 @@ Object {
   "pessimisticFMP": 3233,
   "pessimisticLCP": 3233,
   "pessimisticSI": 3052,
-  "pessimisticTTI": 5119,
+  "pessimisticTTI": 5272,
   "roughEstimateOfFCP": 2294,
   "roughEstimateOfFMP": 2764,
   "roughEstimateOfLCP": 2764,
   "roughEstimateOfLCPLoadEnd": undefined,
   "roughEstimateOfLCPLoadStart": undefined,
-  "roughEstimateOfSI": 3684,
+  "roughEstimateOfSI": 3172,
   "roughEstimateOfTTFB": 611,
-  "roughEstimateOfTTI": 4457,
+  "roughEstimateOfTTI": 4080,
 }
 `;

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

@@ -172,8 +172,8 @@ describe('Render blocking resources audit', () => {
       documentNode.addDependent(styleNode);
 
       const result = estimate(simulator, documentNode, deferredIds, wastedBytesMap, Stacks);
-      // Saving 1000 + 1000 + 100ms for TCP handshake + 1 RT savings + server response time
-      assert.equal(result, 2100);
+      // Saving 1000 + 100ms for 1 RT savings + server response time
+      assert.equal(result, 1100);
     });
 
     it('does not report savings from AMP-stack when document already exceeds 2.1s', () => {

core/test/audits/predictive-perf-test.js

@@ -25,7 +25,7 @@ describe('Performance: predictive performance audit', () => {
     const context = {computedCache: new Map(), settings: {locale: 'en'}};
 
     const output = await PredictivePerf.audit(artifacts, context);
-    expect(output.displayValue).toBeDisplayString('4,460 ms');
+    expect(output.displayValue).toBeDisplayString('4,080 ms');
     const metrics = output.details.items[0];
     for (const [key, value] of Object.entries(metrics)) {
       metrics[key] = value === undefined ? value : Math.round(value);

core/test/computed/metrics/__snapshots__/interactive-test.js.snap

@@ -3,7 +3,7 @@
 exports[`Metrics: TTI should compute a simulated value 1`] = `
 Object {
   "optimistic": 4178,
-  "pessimistic": 4234,
-  "timing": 4206,
+  "pessimistic": 3773,
+  "timing": 3578,
 }
 `;

core/test/computed/metrics/lantern-speed-index-test.js

@@ -0,0 +1,79 @@
+/**
+ * @license Copyright 2018 The Lighthouse Authors. All Rights Reserved.
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License.
+ */
+
+import * as constants from '../../../config/constants.js';
+import {LanternSpeedIndex} from '../../../computed/metrics/lantern-speed-index.js';
+import {getURLArtifactFromDevtoolsLog, readJson} from '../../test-utils.js';
+
+const trace = readJson('../../fixtures/traces/progressive-app-m60.json', import.meta);
+const devtoolsLog = readJson('../../fixtures/traces/progressive-app-m60.devtools.log.json', import.meta);
+
+const defaultThrottling = constants.throttling.mobileSlow4G;
+const URL = getURLArtifactFromDevtoolsLog(devtoolsLog);
+
+describe('Metrics: Lantern Speed Index', () => {
+  const gatherContext = {gatherMode: 'navigation'};
+  it('should compute predicted value', async () => {
+    const settings = {throttlingMethod: 'simulate', throttling: defaultThrottling};
+    const context = {settings, computedCache: new Map()};
+    const result = await LanternSpeedIndex.request(
+      {trace, devtoolsLog, gatherContext, settings, URL},
+      context);
+
+    expect({
+      timing: Math.round(result.timing),
+      optimistic: Math.round(result.optimisticEstimate.timeInMs),
+      pessimistic: Math.round(result.pessimisticEstimate.timeInMs),
+    }).toMatchInlineSnapshot(`
+      Object {
+        "optimistic": 605,
+        "pessimistic": 1661,
+        "timing": 1511,
+      }
+    `);
+  });
+
+  it('should compute predicted value for different settings', async () => {
+    const settings = {throttlingMethod: 'simulate', throttling: {...defaultThrottling, rttMs: 300}};
+    const context = {settings, computedCache: new Map()};
+    const result = await LanternSpeedIndex.request(
+      {trace, devtoolsLog, gatherContext, settings, URL},
+      context);
+
+    expect({
+  timing: Math.round(result.timing),
+  optimistic: Math.round(result.optimisticEstimate.timeInMs),
+  pessimistic: Math.round(result.pessimisticEstimate.timeInMs) }).
+toMatchInlineSnapshot(`
+Object {
+  "optimistic": 605,
+  "pessimistic": 2440,
+  "timing": 2198,
+}
+`);
+  });
+
+  it('should not scale coefficients at default', async () => {
+    const result = LanternSpeedIndex.getScaledCoefficients(defaultThrottling.rttMs);
+    expect(result).toEqual(LanternSpeedIndex.COEFFICIENTS);
+  });
+
+  it('should scale coefficients back', async () => {
+    const result = LanternSpeedIndex.getScaledCoefficients(5);
+    expect(result).toEqual({intercept: 0, pessimistic: 0.5, optimistic: 0.5});
+  });
+
+  it('should scale coefficients forward', async () => {
+    const result = LanternSpeedIndex.getScaledCoefficients(300);
+    expect(result).toMatchInlineSnapshot(`
+      Object {
+        "intercept": 0,
+        "optimistic": 2.525,
+        "pessimistic": 0.275,
+      }
+    `);
+  });
+});