core(trace-processor): correct overlapping tasks (#15921)

GoogleChrome · Apr 5, 2024 · ffdbc63 · ffdbc63
1 parent 04d916d
commit ffdbc63
Show file tree

Hide file tree

Showing 4 changed files with 58 additions and 4 deletions.
diff --git a/core/lib/tracehouse/trace-processor.js b/core/lib/tracehouse/trace-processor.js
@@ -403,6 +403,9 @@ class TraceProcessor {
    */
   static getMainThreadTopLevelEvents(trace, startTime = 0, endTime = Infinity) {
     const topLevelEvents = [];
+    /** @type {ToplevelEvent|undefined} */
+    let prevToplevel = undefined;
+
     // note: mainThreadEvents is already sorted by event start
     for (const event of trace.mainThreadEvents) {
       if (!this.isScheduleableTask(event) || !event.dur) continue;
@@ -411,11 +414,21 @@ class TraceProcessor {
       const end = (event.ts + event.dur - trace.timeOriginEvt.ts) / 1000;
       if (start > endTime || end < startTime) continue;
 
-      topLevelEvents.push({
+      // 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 (prevToplevel && start < prevToplevel.end) {
+        prevToplevel.end = start - 0.001;
+      }
+
+      prevToplevel = {
         start,
         end,
         duration: event.dur / 1000,
-      });
+      };
+
+      topLevelEvents.push(prevToplevel);
     }
 
     return topLevelEvents;

diff --git a/core/test/audits/long-tasks-test.js b/core/test/audits/long-tasks-test.js
@@ -304,7 +304,7 @@ describe('Long tasks audit', () => {
         }],
       },
     });
-    expect(result.metricSavings.TBT).toBeApproximately(353.53);
+    expect(result.metricSavings.TBT).toBeApproximately(171.95);
 
     const debugData = result.details.debugData;
     expect(debugData).toStrictEqual({

diff --git a/core/test/audits/mainthread-work-breakdown-test.js b/core/test/audits/mainthread-work-breakdown-test.js
@@ -120,7 +120,7 @@ Object {
     expect(Math.round(output.numericValue)).toMatchInlineSnapshot(`979`);
     assert.equal(output.details.items.length, 7);
     assert.equal(output.score, 1);
-    expect(output.metricSavings.TBT).toBeCloseTo(353.5, 0.1);
+    expect(output.metricSavings.TBT).toBeCloseTo(171.95, 0.1);
   });
 
   it('should compute the correct values for the load trace (legacy)', async () => {

diff --git a/core/test/lib/tracehouse/trace-processor-test.js b/core/test/lib/tracehouse/trace-processor-test.js
@@ -331,6 +331,47 @@ describe('TraceProcessor', () => {
       assert.equal(ret[1].end, 2000);
       assert.equal(ret[1].duration, 1000);
     });
+
+    it('corrects overlapping tasks', () => {
+      const baseTime = 20_000_000;
+      const name = 'RunTask';
+      const processedTrace = {
+        timeOriginEvt: {ts: baseTime},
+        mainThreadEvents: [
+          // 10ms to 100ms
+          {ts: baseTime + 10_000, dur: 90_000, name},
+          // 40ms to 60ms
+          {ts: baseTime + 40_000, dur: 20_000, name},
+          // 70ms to 90ms
+          {ts: baseTime + 70_000, dur: 20_000, name},
+          // 100ms to 120ms
+          {ts: baseTime + 100_000, dur: 20_000, name},
+        ],
+      };
+
+      const ret = TraceProcessor.getMainThreadTopLevelEvents(
+        processedTrace,
+        0,
+        2000
+      );
+      assert.equal(ret.length, 4);
+
+      assert.equal(ret[0].start, 10);
+      assert.equal(ret[0].end, 39.999);
+      assert.equal(ret[0].duration, 90);
+
+      assert.equal(ret[1].start, 40);
+      assert.equal(ret[1].end, 60);
+      assert.equal(ret[1].duration, 20);
+
+      assert.equal(ret[2].start, 70);
+      assert.equal(ret[2].end, 90);
+      assert.equal(ret[2].duration, 20);
+
+      assert.equal(ret[3].start, 100);
+      assert.equal(ret[3].end, 120);
+      assert.equal(ret[3].duration, 20);
+    });
   });
 
   describe('getMainThreadTopLevelEventDurations', () => {