core(lantern): correct overlapping tasks in CPU nodes (#15938)

GoogleChrome · Apr 11, 2024 · 8b3d357 · 8b3d357
1 parent 7187606
commit 8b3d357
Show file tree

Hide file tree

Showing 5 changed files with 58 additions and 6 deletions.
diff --git a/core/lib/lantern/cpu-node.js b/core/lib/lantern/cpu-node.js
@@ -15,13 +15,15 @@ class CPUNode extends BaseNode {
   /**
    * @param {LH.TraceEvent} parentEvent
    * @param {LH.TraceEvent[]=} childEvents
+   * @param {number=} correctedEndTs
    */
-  constructor(parentEvent, childEvents = []) {
+  constructor(parentEvent, childEvents = [], correctedEndTs) {
     const nodeId = `${parentEvent.tid}.${parentEvent.ts}`;
     super(nodeId);
 
     this._event = parentEvent;
     this._childEvents = childEvents;
+    this._correctedEndTs = correctedEndTs;
   }
 
   get type() {
@@ -39,9 +41,17 @@ class CPUNode extends BaseNode {
    * @return {number}
    */
   get endTime() {
+    if (this._correctedEndTs) return this._correctedEndTs;
     return this._event.ts + this._event.dur;
   }
 
+  /**
+   * @return {number}
+   */
+  get duration() {
+    return this.endTime - this.startTime;
+  }
+
   /**
    * @return {LH.TraceEvent}
    */
@@ -83,7 +93,7 @@ class CPUNode extends BaseNode {
    * @return {CPUNode}
    */
   cloneWithoutRelationships() {
-    return new CPUNode(this._event, this._childEvents);
+    return new CPUNode(this._event, this._childEvents, this._correctedEndTs);
   }
 }
 

diff --git a/core/lib/lantern/metrics/interactive.js b/core/lib/lantern/metrics/interactive.js
@@ -37,7 +37,7 @@ class Interactive extends Metric {
     return dependencyGraph.cloneWithRelationships(node => {
       // Include everything that might be a long task
       if (node.type === BaseNode.TYPES.CPU) {
-        return node.event.dur > minimumCpuTaskDuration;
+        return node.duration > minimumCpuTaskDuration;
       }
 
       // Include all scripts and high priority requests, exclude all images

diff --git a/core/lib/lantern/page-dependency-graph.js b/core/lib/lantern/page-dependency-graph.js
@@ -128,6 +128,9 @@ class PageDependencyGraph {
         continue;
       }
 
+      /** @type {number|undefined} */
+      let correctedEndTs = undefined;
+
       // Capture all events that occurred within the task
       /** @type {Array<LH.TraceEvent>} */
       const children = [];
@@ -136,10 +139,19 @@ class PageDependencyGraph {
         i < mainThreadEvents.length && mainThreadEvents[i].ts < endTime;
         i++
       ) {
+        // Temporary fix for a Chrome bug where some RunTask events can be overlapping.
+        // We correct that here be ensuring each RunTask ends at least 1 microsecond before the next
+        // https://github.com/GoogleChrome/lighthouse/issues/15896
+        // https://issues.chromium.org/issues/329678173
+        if (TraceProcessor.isScheduleableTask(mainThreadEvents[i]) && mainThreadEvents[i].dur) {
+          correctedEndTs = mainThreadEvents[i].ts - 1;
+          break;
+        }
+
         children.push(mainThreadEvents[i]);
       }
 
-      nodes.push(new CPUNode(evt, children));
+      nodes.push(new CPUNode(evt, children, correctedEndTs));
     }
 
     return nodes;
@@ -359,7 +371,7 @@ class PageDependencyGraph {
         isFirst = foundFirstParse = true;
       }
 
-      if (isFirst || node.event.dur >= minimumEvtDur) {
+      if (isFirst || node.duration >= minimumEvtDur) {
         // Don't prune this node. The task is long / important so it will impact simulation.
         continue;
       }

diff --git a/core/lib/lantern/simulator/simulator.js b/core/lib/lantern/simulator/simulator.js
@@ -276,7 +276,7 @@ class Simulator {
       ? this._layoutTaskMultiplier
       : this._cpuSlowdownMultiplier;
     const totalDuration = Math.min(
-      Math.round(cpuNode.event.dur / 1000 * multiplier),
+      Math.round(cpuNode.duration / 1000 * multiplier),
       DEFAULT_MAXIMUM_CPU_TASK_DURATION
     );
     const estimatedTimeElapsed = totalDuration - timingData.timeElapsed;

diff --git a/core/test/lib/lantern/page-dependency-graph-test.js b/core/test/lib/lantern/page-dependency-graph-test.js
@@ -160,6 +160,36 @@ describe('PageDependencyGraph computed artifact:', () => {
       assert.equal(node2.childEvents.length, 1);
       assert.equal(node2.childEvents[0].name, 'LaterEvent');
     });
+
+    it('should correct overlapping tasks', () => {
+      addTaskEvents(0, 500, [
+        {name: 'MyCustomEvent'},
+        {name: 'OtherEvent'},
+      ]);
+
+      addTaskEvents(400, 50, [
+        {name: 'OverlappingEvent'},
+      ]);
+
+      assert.equal(traceEvents.length, 5);
+      const nodes = PageDependencyGraph.getCPUNodes(traceEvents);
+      assert.equal(nodes.length, 2);
+
+      const node1 = nodes[0];
+      assert.equal(node1.id, '1.0');
+      assert.equal(node1.type, 'cpu');
+      assert.equal(node1.event, traceEvents[0]);
+      assert.equal(node1.childEvents.length, 2);
+      assert.equal(node1.childEvents[0].name, 'MyCustomEvent');
+      assert.equal(node1.childEvents[1].name, 'OtherEvent');
+
+      const node2 = nodes[1];
+      assert.equal(node2.id, '1.400000');
+      assert.equal(node2.type, 'cpu');
+      assert.equal(node2.event, traceEvents[3]);
+      assert.equal(node2.childEvents.length, 1);
+      assert.equal(node2.childEvents[0].name, 'OverlappingEvent');
+    });
   });
 
   describe('#createGraph', () => {