Skip to content

Commit

Permalink
Merge branch 'main' into patch-1
Browse files Browse the repository at this point in the history
  • Loading branch information
adamraine authored Aug 7, 2024
2 parents 26166dc + 5b33eb0 commit dab3360
Show file tree
Hide file tree
Showing 18 changed files with 193 additions and 108 deletions.
11 changes: 7 additions & 4 deletions core/audits/third-party-summary.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,10 +101,13 @@ class ThirdPartySummary extends Audit {
const taskDuration = task.selfTime * cpuMultiplier;
// The amount of time spent on main thread is the sum of all durations.
urlSummary.mainThreadTime += taskDuration;
// The amount of time spent *blocking* on main thread is the sum of all time longer than 50ms.
// Note that this is not totally equivalent to the TBT definition since it fails to account for FCP,
// but a majority of third-party work occurs after FCP and should yield largely similar numbers.
urlSummary.blockingTime += Math.max(taskDuration - 50, 0);
// Blocking time is the amount of time spent on the main thread *over* 50ms.
// This value is interpolated because not all tasks attributed to this URL are at the top level.
//
// Note that this is not totally equivalent to the TBT definition since it fails to account for
// the FCP&TTI bounds of TBT.
urlSummary.blockingTime += task.selfBlockingTime;
// TBT impact is similar to blocking time, but it accounts for the FCP&TTI bounds of TBT.
urlSummary.tbtImpact += task.selfTbtImpact;
byURL.set(attributableURL, urlSummary);
}
Expand Down
4 changes: 2 additions & 2 deletions core/computed/metrics/lantern-first-contentful-paint.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ class LanternFirstContentfulPaint extends Lantern.Metrics.FirstContentfulPaint {
* @return {Promise<LH.Artifacts.LanternMetric>}
*/
static async computeMetricWithGraphs(data, context, extras) {
return this.compute(await getComputationDataParams(data, context), extras)
.catch(lanternErrorAdapter);
const params = await getComputationDataParams(data, context);
return Promise.resolve(this.compute(params, extras)).catch(lanternErrorAdapter);
}

/**
Expand Down
4 changes: 2 additions & 2 deletions core/computed/metrics/lantern-interactive.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ class LanternInteractive extends Lantern.Metrics.Interactive {
* @return {Promise<LH.Artifacts.LanternMetric>}
*/
static async computeMetricWithGraphs(data, context, extras) {
return this.compute(await getComputationDataParams(data, context), extras)
.catch(lanternErrorAdapter);
const params = await getComputationDataParams(data, context);
return Promise.resolve(this.compute(params, extras)).catch(lanternErrorAdapter);
}

/**
Expand Down
4 changes: 2 additions & 2 deletions core/computed/metrics/lantern-largest-contentful-paint.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ class LanternLargestContentfulPaint extends Lantern.Metrics.LargestContentfulPai
* @return {Promise<LH.Artifacts.LanternMetric>}
*/
static async computeMetricWithGraphs(data, context, extras) {
return this.compute(await getComputationDataParams(data, context), extras)
.catch(lanternErrorAdapter);
const params = await getComputationDataParams(data, context);
return Promise.resolve(this.compute(params, extras)).catch(lanternErrorAdapter);
}

/**
Expand Down
4 changes: 2 additions & 2 deletions core/computed/metrics/lantern-max-potential-fid.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ class LanternMaxPotentialFID extends Lantern.Metrics.MaxPotentialFID {
* @return {Promise<LH.Artifacts.LanternMetric>}
*/
static async computeMetricWithGraphs(data, context, extras) {
return this.compute(await getComputationDataParams(data, context), extras)
.catch(lanternErrorAdapter);
const params = await getComputationDataParams(data, context);
return Promise.resolve(this.compute(params, extras)).catch(lanternErrorAdapter);
}

/**
Expand Down
4 changes: 2 additions & 2 deletions core/computed/metrics/lantern-speed-index.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ class LanternSpeedIndex extends Lantern.Metrics.SpeedIndex {
* @return {Promise<LH.Artifacts.LanternMetric>}
*/
static async computeMetricWithGraphs(data, context, extras) {
return this.compute(await getComputationDataParams(data, context), extras)
.catch(lanternErrorAdapter);
const params = await getComputationDataParams(data, context);
return Promise.resolve(this.compute(params, extras)).catch(lanternErrorAdapter);
}

/**
Expand Down
35 changes: 29 additions & 6 deletions core/computed/tbt-impact-tasks.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,24 +64,35 @@ class TBTImpactTasks {
/**
* @param {LH.Artifacts.TaskNode[]} tasks
* @param {Map<LH.Artifacts.TaskNode, number>} taskToImpact
* @param {Map<LH.Artifacts.TaskNode, number>} taskToBlockingTime
*/
static createImpactTasks(tasks, taskToImpact) {
static createImpactTasks(tasks, taskToImpact, taskToBlockingTime) {
/** @type {LH.Artifacts.TBTImpactTask[]} */
const tbtImpactTasks = [];

for (const task of tasks) {
const tbtImpact = taskToImpact.get(task) || 0;
let selfTbtImpact = tbtImpact;

const blockingTime = taskToBlockingTime.get(task) || 0;
let selfBlockingTime = blockingTime;

for (const child of task.children) {
const childTbtImpact = taskToImpact.get(child) || 0;
selfTbtImpact -= childTbtImpact;

const childBlockingTime = taskToBlockingTime.get(child) || 0;
selfBlockingTime -= childBlockingTime;
}

tbtImpactTasks.push({
...task,
tbtImpact,
selfTbtImpact,
// Floating point numbers are not perfectly precise, so the subtraction operations above
// can sometimes output negative numbers close to 0 here. To prevent potentially confusing
// output we should bump those values to 0.
tbtImpact: Math.max(tbtImpact, 0),
selfTbtImpact: Math.max(selfTbtImpact, 0),
selfBlockingTime: Math.max(selfBlockingTime, 0),
});
}

Expand All @@ -98,6 +109,9 @@ class TBTImpactTasks {
/** @type {Map<LH.Artifacts.TaskNode, number>} */
const taskToImpact = new Map();

/** @type {Map<LH.Artifacts.TaskNode, number>} */
const taskToBlockingTime = new Map();

for (const task of tasks) {
const event = {
start: task.startTime,
Expand All @@ -113,11 +127,13 @@ class TBTImpactTasks {
};

const tbtImpact = calculateTbtImpactForEvent(event, startTimeMs, endTimeMs, topLevelEvent);
const blockingTime = calculateTbtImpactForEvent(event, -Infinity, Infinity, topLevelEvent);

taskToImpact.set(task, tbtImpact);
taskToBlockingTime.set(task, blockingTime);
}

return this.createImpactTasks(tasks, taskToImpact);
return this.createImpactTasks(tasks, taskToImpact, taskToBlockingTime);
}

/**
Expand All @@ -131,6 +147,9 @@ class TBTImpactTasks {
/** @type {Map<LH.Artifacts.TaskNode, number>} */
const taskToImpact = new Map();

/** @type {Map<LH.Artifacts.TaskNode, number>} */
const taskToBlockingTime = new Map();

/** @type {Map<LH.Artifacts.TaskNode, {start: number, end: number, duration: number}>} */
const topLevelTaskToEvent = new Map();

Expand All @@ -151,19 +170,21 @@ class TBTImpactTasks {
};

const tbtImpact = calculateTbtImpactForEvent(event, startTimeMs, endTimeMs);
const blockingTime = calculateTbtImpactForEvent(event, -Infinity, Infinity);

const task = traceEventToTask.get(node.event);
if (!task) continue;

topLevelTaskToEvent.set(task, event);
taskToImpact.set(task, tbtImpact);
taskToBlockingTime.set(task, blockingTime);
}

// Interpolate the TBT impact of remaining tasks using the top level ancestor tasks.
// We don't have any lantern estimates for tasks that are not top level, so we need to estimate
// the lantern timing based on the task's observed timing relative to it's top level task's observed timing.
for (const task of tasks) {
if (taskToImpact.has(task)) continue;
if (taskToImpact.has(task) || taskToBlockingTime.has(task)) continue;

const topLevelTask = this.getTopLevelTask(task);

Expand All @@ -183,11 +204,13 @@ class TBTImpactTasks {
};

const tbtImpact = calculateTbtImpactForEvent(event, startTimeMs, endTimeMs, topLevelEvent);
const blockingTime = calculateTbtImpactForEvent(event, -Infinity, Infinity, topLevelEvent);

taskToImpact.set(task, tbtImpact);
taskToBlockingTime.set(task, blockingTime);
}

return this.createImpactTasks(tasks, taskToImpact);
return this.createImpactTasks(tasks, taskToImpact, taskToBlockingTime);
}

/**
Expand Down
11 changes: 10 additions & 1 deletion core/computed/trace-engine-result.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,16 @@ class TraceEngineResult {
* @return {Promise<LH.Artifacts.TraceEngineResult>}
*/
static async runTraceEngine(traceEvents) {
const processor = TraceEngine.TraceProcessor.createWithAllHandlers();
const traceHandlers = {...TraceEngine.TraceHandlers};

// @ts-expect-error Temporarily disable this handler
// It's not currently used anywhere in trace engine insights or Lighthouse.
// TODO: Re-enable this when its memory usage is improved in the trace engine
// https://github.com/GoogleChrome/lighthouse/issues/16111
delete traceHandlers.Invalidations;

const processor = new TraceEngine.TraceProcessor(traceHandlers);

// eslint-disable-next-line max-len
await processor.parse(/** @type {import('@paulirish/trace_engine').Types.TraceEvents.TraceEventData[]} */ (
traceEvents
Expand Down
14 changes: 14 additions & 0 deletions core/scripts/roll-to-devtools.sh
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,20 @@ lh_e2e_res_dir="third-party/devtools-tests/e2e/resources/lighthouse/"
fe_e2e_res_dir="$dt_dir/test/e2e/resources/lighthouse"
rsync -avh "$lh_e2e_res_dir" "$fe_e2e_res_dir" --exclude="OWNERS" --delete

PKG_VERSION=$(node -e "console.log(require('./package.json').version)")
REVISION=$(git rev-parse HEAD)
echo "Name: Lighthouse
Short Name: lighthouse
Version: $PKG_VERSION
Revision: $REVISION
URL: https://github.com/GoogleChrome/lighthouse
License: Apache License 2.0
License File: LICENSE
Security Critical: no
Shipped: yes
This directory contains Chromium's version of the lighthouse report assets, including renderer." > "$fe_lh_dir/README.chromium"

echo ""
echo "Done. To run the e2e tests: "
echo " DEVTOOLS_PATH=\"$dt_dir\" yarn test-devtools"
46 changes: 23 additions & 23 deletions core/test/audits/__snapshots__/third-party-summary-test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@
exports[`Third party summary surface the discovered third parties 1`] = `
Array [
Object {
"blockingTime": 223.56,
"blockingTime": 225.30612021679366,
"entity": "Wunderkind",
"mainThreadTime": 879.9769999999949,
"subItems": Object {
"items": Array [
Object {
"blockingTime": 223.56,
"blockingTime": 225.30612021679366,
"mainThreadTime": 287.06299999999925,
"tbtImpact": 225.30612021679366,
"transferSize": 16051,
Expand Down Expand Up @@ -547,13 +547,13 @@ Array [
"transferSize": 450838,
},
Object {
"blockingTime": 21.476,
"blockingTime": 64.66899999999367,
"entity": "Google/Doubleclick Ads",
"mainThreadTime": 347.9769999999987,
"subItems": Object {
"items": Array [
Object {
"blockingTime": 21.476,
"blockingTime": 64.66899999999367,
"mainThreadTime": 339.7179999999987,
"tbtImpact": 10.898000000000135,
"transferSize": 145049,
Expand Down Expand Up @@ -1118,6 +1118,25 @@ Array [
"tbtImpact": 10.898000000000135,
"transferSize": 571412,
},
Object {
"blockingTime": 8.445999999999897,
"entity": "script.ac",
"mainThreadTime": 462.9409999999911,
"subItems": Object {
"items": Array [
Object {
"blockingTime": 8.445999999999897,
"mainThreadTime": 462.9409999999911,
"tbtImpact": 8.445999999999897,
"transferSize": 50609,
"url": "https://cadmus.script.ac/d2uap9jskdzp2/script.js",
},
],
"type": "subitems",
},
"tbtImpact": 8.445999999999897,
"transferSize": 50609,
},
Object {
"blockingTime": 0,
"entity": "gobankingrates.com",
Expand Down Expand Up @@ -3343,25 +3362,6 @@ Array [
"tbtImpact": 0,
"transferSize": 68466,
},
Object {
"blockingTime": 0,
"entity": "script.ac",
"mainThreadTime": 462.9409999999911,
"subItems": Object {
"items": Array [
Object {
"blockingTime": 0,
"mainThreadTime": 462.9409999999911,
"tbtImpact": 8.445999999999897,
"transferSize": 50609,
"url": "https://cadmus.script.ac/d2uap9jskdzp2/script.js",
},
],
"type": "subitems",
},
"tbtImpact": 8.445999999999897,
"transferSize": 50609,
},
Object {
"blockingTime": 0,
"entity": "myfinance.com",
Expand Down
2 changes: 1 addition & 1 deletion core/test/audits/third-party-facades-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,7 @@ Array [
expect(results.score).toBe(0);
expect(results.metricSavings).toEqual({TBT: 145});
expect(results.displayValue).toBeDisplayString('1 facade alternative available');
expect(results.details.items[0].blockingTime).toEqual(134.076); // TBT impact is not equal to the blocking time
expect(results.details.items[0].blockingTime).toBeCloseTo(145);
expect(results.details.items[0].product)
.toBeDisplayString('Intercom Widget (Customer Success)');
});
Expand Down
8 changes: 4 additions & 4 deletions core/test/audits/third-party-summary-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@ describe('Third party summary', () => {
settings.throttlingMethod = 'devtools';
const results = await ThirdPartySummary.audit(artifacts, {computedCache: new Map(), settings});

expect(results.score).toBe(1);
expect(results.score).toBe(0);
expect(results.metricSavings).toEqual({TBT: 245});
expect(results.displayValue).toBeDisplayString(
'Third-party code blocked the main thread for 250 ms'
'Third-party code blocked the main thread for 300 ms'
);
expect(results.details.items).toMatchSnapshot();
});
Expand All @@ -49,9 +49,9 @@ describe('Third party summary', () => {
expect(results.metricSavings).toEqual({TBT: 2570});
expect(results.details.items).toHaveLength(145);
expect(Math.round(results.details.items[0].mainThreadTime)).toEqual(3520);
expect(Math.round(results.details.items[0].blockingTime)).toEqual(1103);
expect(Math.round(results.details.items[0].blockingTime)).toEqual(1182);
expect(Math.round(results.details.items[1].mainThreadTime)).toEqual(1392);
expect(Math.round(results.details.items[1].blockingTime)).toEqual(659);
expect(Math.round(results.details.items[1].blockingTime)).toEqual(508);
});

it('be not applicable when no third parties are present', async () => {
Expand Down
Loading

0 comments on commit dab3360

Please sign in to comment.