core(lantern): improve LCP tail error

lighthouse-core/computed/metrics/lantern-largest-contentful-paint.js

@@ -17,7 +17,6 @@ class LanternLargestContentfulPaint extends LanternMetric {
    * @return {LH.Gatherer.Simulation.MetricCoefficients}
    */
   static get COEFFICIENTS() {
-    // TODO: Calibrate
     return {
       intercept: 0,
       optimistic: 0.5,
@@ -26,7 +25,21 @@ class LanternLargestContentfulPaint extends LanternMetric {
   }
 
   /**
-   * TODO: Validate.
+   * Low priority image nodes are usually offscreen and very unlikely to be the
+   * resource that is required for LCP. Our LCP graphs include everything except for these images.
+   *
+   * @param {Node} node
+   * @return {boolean}
+   */
+  static isNotLowPriorityImageNode(node) {
+    if (node.type !== 'network') return true;
+
+    const isImage = node.record.resourceType === 'Image';
+    const isLowPriority = node.record.priority === 'Low' || node.record.priority === 'VeryLow';
+    return !isImage || !isLowPriority;
+  }
+
+  /**
    * @param {Node} dependencyGraph
    * @param {LH.Artifacts.TraceOfTab} traceOfTab
    * @return {Node}
@@ -40,12 +53,11 @@ class LanternLargestContentfulPaint extends LanternMetric {
     return LanternFirstContentfulPaint.getFirstPaintBasedGraph(
       dependencyGraph,
       lcp,
-      _ => true
+      LanternLargestContentfulPaint.isNotLowPriorityImageNode
     );
   }
 
   /**
-   * TODO: Validate.
    * @param {Node} dependencyGraph
    * @param {LH.Artifacts.TraceOfTab} traceOfTab
    * @return {Node}
@@ -65,6 +77,21 @@ class LanternLargestContentfulPaint extends LanternMetric {
     );
   }
 
+  /**
+   * @param {LH.Gatherer.Simulation.Result} simulationResult
+   * @return {LH.Gatherer.Simulation.Result}
+   */
+  static getEstimateFromSimulation(simulationResult) {
+    const nodeTimesNotOffscreenImages = Array.from(simulationResult.nodeTimings.entries())
+      .filter(entry => LanternLargestContentfulPaint.isNotLowPriorityImageNode(entry[0]))
+      .map(entry => entry[1].endTime);
+
+    return {
+      timeInMs: Math.max(...nodeTimesNotOffscreenImages),
+      nodeTimings: simulationResult.nodeTimings,
+    };
+  }
+
   /**
    * @param {LH.Artifacts.MetricComputationDataInput} data
    * @param {LH.Audit.Context} context

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

@@ -29,6 +29,15 @@ const NodeState = {
   Complete: 3,
 };
 
+/** @type {Record<NetworkNode['record']['priority'], number>} */
+const PriorityStartTimePenalty = {
+  VeryHigh: 0,
+  High: 0.25,
+  Medium: 0.5,
+  Low: 1,
+  VeryLow: 2,
+};
+
 /** @type {Map<string, LH.Gatherer.Simulation.Result['nodeTimings']>} */
 const ALL_SIMULATION_NODE_TIMINGS = new Map();
 
@@ -60,7 +69,7 @@ class Simulator {
     this._cpuSlowdownMultiplier = this._options.cpuSlowdownMultiplier;
     this._layoutTaskMultiplier = this._cpuSlowdownMultiplier * this._options.layoutTaskMultiplier;
     /** @type {Array<Node>} */
-    this._cachedNodeListByStartTime = [];
+    this._cachedNodeListByStartPosition = [];
 
     // Properties reset on every `.simulate` call but duplicated here for type checking
     this._flexibleOrdering = false;
@@ -103,7 +112,7 @@ class Simulator {
     this._numberInProgressByType = new Map();
 
     this._nodes = {};
-    this._cachedNodeListByStartTime = [];
+    this._cachedNodeListByStartPosition = [];
     // NOTE: We don't actually need *all* of these sets, but the clarity that each node progresses
     // through the system is quite nice.
     for (const state of Object.values(NodeState)) {
@@ -144,11 +153,12 @@ class Simulator {
    * @param {number} queuedTime
    */
   _markNodeAsReadyToStart(node, queuedTime) {
-    const firstNodeIndexWithGreaterStartTime = this._cachedNodeListByStartTime
-      .findIndex(candidate => candidate.startTime > node.startTime);
-    const insertionIndex = firstNodeIndexWithGreaterStartTime === -1 ?
-      this._cachedNodeListByStartTime.length : firstNodeIndexWithGreaterStartTime;
-    this._cachedNodeListByStartTime.splice(insertionIndex, 0, node);
+    const nodeStartPosition = Simulator._computeNodeStartPosition(node);
+    const firstNodeIndexWithGreaterStartPosition = this._cachedNodeListByStartPosition
+      .findIndex(candidate => Simulator._computeNodeStartPosition(candidate) > nodeStartPosition);
+    const insertionIndex = firstNodeIndexWithGreaterStartPosition === -1 ?
+      this._cachedNodeListByStartPosition.length : firstNodeIndexWithGreaterStartPosition;
+    this._cachedNodeListByStartPosition.splice(insertionIndex, 0, node);
 
     this._nodes[NodeState.ReadyToStart].add(node);
     this._nodes[NodeState.NotReadyToStart].delete(node);
@@ -160,8 +170,8 @@ class Simulator {
    * @param {number} startTime
    */
   _markNodeAsInProgress(node, startTime) {
-    const indexOfNodeToStart = this._cachedNodeListByStartTime.indexOf(node);
-    this._cachedNodeListByStartTime.splice(indexOfNodeToStart, 1);
+    const indexOfNodeToStart = this._cachedNodeListByStartPosition.indexOf(node);
+    this._cachedNodeListByStartPosition.splice(indexOfNodeToStart, 1);
 
     this._nodes[NodeState.InProgress].add(node);
     this._nodes[NodeState.ReadyToStart].delete(node);
@@ -203,9 +213,9 @@ class Simulator {
   /**
    * @return {Node[]}
    */
-  _getNodesSortedByStartTime() {
+  _getNodesSortedByStartPosition() {
     // Make a copy so we don't skip nodes due to concurrent modification
-    return Array.from(this._cachedNodeListByStartTime);
+    return Array.from(this._cachedNodeListByStartPosition);
   }
 
   /**
@@ -456,7 +466,7 @@ class Simulator {
     // loop as long as we have nodes in the queue or currently in progress
     while (nodesReadyToStart.size || nodesInProgress.size) {
       // move all possible queued nodes to in progress
-      for (const node of this._getNodesSortedByStartTime()) {
+      for (const node of this._getNodesSortedByStartPosition()) {
         this._startNodeIfPossible(node, totalElapsedTime);
       }
 
@@ -500,6 +510,17 @@ class Simulator {
   static get ALL_NODE_TIMINGS() {
     return ALL_SIMULATION_NODE_TIMINGS;
   }
+
+  /**
+   * We attempt to start nodes by their observed start time using the record priority as a tie breaker.
+   * When simulating, just because a low priority image started 5ms before a high priority image doesn't mean
+   * it would have happened like that when the network was slower.
+   * @param {Node} node
+   */
+  static _computeNodeStartPosition(node) {
+    if (node.type === 'cpu') return node.startTime;
+    return node.startTime + (PriorityStartTimePenalty[node.record.priority] * 1000 * 1000 || 0);
+  }
 }
 
 module.exports = Simulator;

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

@@ -11,9 +11,9 @@ Object {
   "firstContentfulPaintTs": undefined,
   "firstMeaningfulPaint": 2758,
   "firstMeaningfulPaintTs": undefined,
-  "interactive": 4614,
+  "interactive": 4462,
   "interactiveTs": undefined,
-  "largestContentfulPaint": 3419,
+  "largestContentfulPaint": 2758,
   "largestContentfulPaintTs": undefined,
   "maxPotentialFID": 1336,
   "observedCumulativeLayoutShift": undefined,
@@ -40,9 +40,9 @@ Object {
   "observedSpeedIndexTs": 713038416494,
   "observedTraceEnd": 7416,
   "observedTraceEndTs": 713044438781,
-  "speedIndex": 3663,
+  "speedIndex": 3681,
   "speedIndexTs": undefined,
-  "totalBlockingTime": 1191,
+  "totalBlockingTime": 1167,
 }
 `;
 
@@ -55,7 +55,7 @@ Object {
   "firstCPUIdleTs": undefined,
   "firstContentfulPaint": 1337,
   "firstContentfulPaintTs": undefined,
-  "firstMeaningfulPaint": 1541,
+  "firstMeaningfulPaint": 1553,
   "firstMeaningfulPaintTs": undefined,
   "interactive": 3427,
   "interactiveTs": undefined,

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

@@ -5,23 +5,23 @@ Object {
   "optimisticEIL": 455,
   "optimisticFCP": 2289,
   "optimisticFMP": 2289,
-  "optimisticLCP": 3192,
+  "optimisticLCP": 2289,
   "optimisticSI": 1393,
   "optimisticTTFCPUI": 3677,
   "optimisticTTI": 3677,
-  "pessimisticEIL": 903,
+  "pessimisticEIL": 904,
   "pessimisticFCP": 2289,
   "pessimisticFMP": 3228,
-  "pessimisticLCP": 3647,
-  "pessimisticSI": 3018,
-  "pessimisticTTFCPUI": 5551,
-  "pessimisticTTI": 5551,
+  "pessimisticLCP": 3228,
+  "pessimisticSI": 3047,
+  "pessimisticTTFCPUI": 5248,
+  "pessimisticTTI": 5248,
   "roughEstimateOfEIL": 543,
   "roughEstimateOfFCP": 2289,
   "roughEstimateOfFMP": 2758,
-  "roughEstimateOfLCP": 3419,
-  "roughEstimateOfSI": 3663,
+  "roughEstimateOfLCP": 2758,
+  "roughEstimateOfSI": 3681,
   "roughEstimateOfTTFCPUI": 3677,
-  "roughEstimateOfTTI": 4614,
+  "roughEstimateOfTTI": 4462,
 }
 `;

lighthouse-core/test/audits/byte-efficiency/__snapshots__/byte-efficiency-audit-test.js.snap

@@ -9,4 +9,4 @@ Object {
 
 exports[`Byte efficiency base audit should create load simulator with the specified settings 1`] = `1330`;
 
-exports[`Byte efficiency base audit should create load simulator with the specified settings 2`] = `22670`;
+exports[`Byte efficiency base audit should create load simulator with the specified settings 2`] = `22520`;

lighthouse-core/test/audits/metrics/__snapshots__/first-meaningful-paint-test.js.snap

@@ -2,7 +2,7 @@
 
 exports[`Performance: first-meaningful-paint audit computes FMP correctly for simulated 1`] = `
 Object {
-  "numericValue": 1540.6100000208244,
+  "numericValue": 1553.2040000472914,
   "score": 0.99,
 }
 `;

lighthouse-core/test/computed/metrics/__snapshots__/first-meaningful-paint-test.js.snap

@@ -2,8 +2,8 @@
 
 exports[`Metrics: FMP should compute a simulated value 1`] = `
 Object {
-  "optimistic": 1469,
+  "optimistic": 1494,
   "pessimistic": 1613,
-  "timing": 1541,
+  "timing": 1553,
 }
 `;

lighthouse-core/test/computed/metrics/__snapshots__/lantern-first-meaningful-paint-test.js.snap

@@ -2,8 +2,8 @@
 
 exports[`Metrics: Lantern FMP should compute predicted value 1`] = `
 Object {
-  "optimistic": 1469,
+  "optimistic": 1494,
   "pessimistic": 1613,
-  "timing": 1541,
+  "timing": 1553,
 }
 `;

lighthouse-core/test/computed/metrics/lantern-largest-contentful-paint-test.js

@@ -9,8 +9,7 @@ const assert = require('assert');
 
 const trace = require('../../fixtures/traces/lcp-m78.json');
 const devtoolsLog = require('../../fixtures/traces/lcp-m78.devtools.log.json');
-const LanternLargestContentfulPaint =
-  require('../../../computed/metrics/lantern-largest-contentful-paint.js');
+const LanternLargestContentfulPaint = require('../../../computed/metrics/lantern-largest-contentful-paint.js'); // eslint-disable-line max-len
 
 /* eslint-env jest */
 describe('Metrics: Lantern LCP', () => {
@@ -30,13 +29,13 @@ describe('Metrics: Lantern LCP', () => {
       {},
       `
       Object {
-        "optimistic": 3192,
-        "pessimistic": 3647,
-        "timing": 3419,
+        "optimistic": 2289,
+        "pessimistic": 3228,
+        "timing": 2758,
       }
     `
     );
-    assert.equal(result.optimisticEstimate.nodeTimings.size, 18);
+    assert.equal(result.optimisticEstimate.nodeTimings.size, 12);
     assert.equal(result.pessimisticEstimate.nodeTimings.size, 19);
     assert.ok(result.optimisticGraph, 'should have created optimistic graph');
     assert.ok(result.pessimisticGraph, 'should have created pessimistic graph');

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

@@ -23,12 +23,12 @@ describe('Metrics: Lantern Speed Index', () => {
       optimistic: Math.round(result.optimisticEstimate.timeInMs),
       pessimistic: Math.round(result.pessimisticEstimate.timeInMs),
     }).toMatchInlineSnapshot(`
-Object {
-  "optimistic": 605,
-  "pessimistic": 1661,
-  "timing": 1676,
-}
-`);
+      Object {
+        "optimistic": 605,
+        "pessimistic": 1661,
+        "timing": 1676,
+      }
+    `);
   });
 
   it('should compute predicted value for different settings', async () => {
@@ -41,12 +41,12 @@ Object {
       optimistic: Math.round(result.optimisticEstimate.timeInMs),
       pessimistic: Math.round(result.pessimisticEstimate.timeInMs),
     }).toMatchInlineSnapshot(`
-Object {
-  "optimistic": 605,
-  "pessimistic": 2411,
-  "timing": 2983,
-}
-`);
+      Object {
+        "optimistic": 605,
+        "pessimistic": 2439,
+        "timing": 3007,
+      }
+    `);
   });
 
   it('should not scale coefficients at default', async () => {
@@ -62,11 +62,11 @@ Object {
   it('should scale coefficients forward', async () => {
     const result = LanternSpeedIndex.getScaledCoefficients(300);
     expect(result).toMatchInlineSnapshot(`
-Object {
-  "intercept": -562.5,
-  "optimistic": 2.525,
-  "pessimistic": 0.8375,
-}
-`);
+      Object {
+        "intercept": -562.5,
+        "optimistic": 2.525,
+        "pessimistic": 0.8375,
+      }
+    `);
   });
 });

lighthouse-core/test/computed/metrics/largest-contentful-paint-test.js

@@ -27,9 +27,9 @@ describe('Metrics: LCP', () => {
       pessimistic: Math.round(result.pessimisticEstimate.timeInMs),
     }).toMatchInlineSnapshot(`
       Object {
-        "optimistic": 3192,
-        "pessimistic": 3647,
-        "timing": 3419,
+        "optimistic": 2289,
+        "pessimistic": 3228,
+        "timing": 2758,
       }
     `);
   });