Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core(lantern): allow non-XHRs to depend on CPU Nodes #11767

Merged
merged 13 commits into from
Dec 11, 2020
15 changes: 13 additions & 2 deletions lighthouse-core/computed/page-dependency-graph.js
Original file line number Diff line number Diff line change
Expand Up @@ -186,15 +186,26 @@ class PageDependencyGraph {
* @param {Array<CPUNode>} cpuNodes
*/
static linkCPUNodes(rootNode, networkNodeOutput, cpuNodes) {
/** @type {Set<LH.Crdp.Network.ResourceType|undefined>} */
const linkableResourceTypes = new Set([
NetworkRequest.TYPES.XHR, NetworkRequest.TYPES.Fetch, NetworkRequest.TYPES.Script,
]);

/** @param {CPUNode} cpuNode @param {string} reqId */
function addDependentNetworkRequest(cpuNode, reqId) {
const networkNode = networkNodeOutput.idToNodeMap.get(reqId);
if (!networkNode ||
// Ignore all non-XHRs
networkNode.record.resourceType !== NetworkRequest.TYPES.XHR ||
// Ignore all network nodes that started before this CPU task started
// A network request that started earlier could not possibly have been started by this task
networkNode.startTime <= cpuNode.startTime) return;
const {record} = networkNode;
const resourceType = record.resourceType ||
record.redirectDestination && record.redirectDestination.resourceType;
if (!linkableResourceTypes.has(resourceType)) {
// We only link some resources to CPU nodes because we observe LCP simulation
// regressions when including images, etc.
return;
}
cpuNode.addDependent(networkNode);
}

Expand Down
3 changes: 1 addition & 2 deletions lighthouse-core/lib/network-request.js
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,7 @@ class NetworkRequest {
this.failed = false;
this.localizedFailDescription = '';

/** @type {LH.Crdp.Network.Initiator} */
this.initiator = {type: 'other'};
this.initiator = /** @type {LH.Crdp.Network.Initiator} */ ({type: 'other'});
warrengm marked this conversation as resolved.
Show resolved Hide resolved
/** @type {LH.Crdp.Network.ResourceTiming|undefined} */
this.timing = undefined;
/** @type {LH.Crdp.Network.ResourceType|undefined} */
Expand Down
12 changes: 6 additions & 6 deletions lighthouse-core/test/audits/__snapshots__/metrics-test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -59,13 +59,13 @@ Object {
"cumulativeLayoutShiftAllFrames": 0,
"estimatedInputLatency": 543,
"estimatedInputLatencyTs": undefined,
"firstCPUIdle": 3677,
"firstCPUIdle": 3790,
"firstCPUIdleTs": undefined,
"firstContentfulPaint": 2289,
"firstContentfulPaintTs": undefined,
"firstMeaningfulPaint": 2758,
"firstMeaningfulPaintTs": undefined,
"interactive": 4462,
"interactive": 4671,
"interactiveTs": undefined,
"largestContentfulPaint": 2758,
"largestContentfulPaintAllFrames": undefined,
Expand Down Expand Up @@ -102,7 +102,7 @@ Object {
"observedTraceEndTs": 713044439102,
"speedIndex": 3681,
"speedIndexTs": undefined,
"totalBlockingTime": 1167,
"totalBlockingTime": 1205,
}
`;

Expand Down Expand Up @@ -163,15 +163,15 @@ exports[`Performance: metrics evaluates valid input correctly 1`] = `
Object {
"cumulativeLayoutShift": 0,
"cumulativeLayoutShiftAllFrames": 0,
"estimatedInputLatency": 81,
"estimatedInputLatency": 78,
"estimatedInputLatencyTs": undefined,
"firstCPUIdle": 3351,
"firstCPUIdle": 4313,
"firstCPUIdleTs": undefined,
"firstContentfulPaint": 1337,
"firstContentfulPaintTs": undefined,
"firstMeaningfulPaint": 1553,
"firstMeaningfulPaintTs": undefined,
"interactive": 3427,
"interactive": 4348,
"interactiveTs": undefined,
"largestContentfulPaint": undefined,
"largestContentfulPaintAllFrames": undefined,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,21 @@ Object {
"optimisticFMP": 2289,
"optimisticLCP": 2289,
"optimisticSI": 1393,
"optimisticTTFCPUI": 3677,
"optimisticTTI": 3677,
"optimisticTTFCPUI": 3790,
"optimisticTTI": 3790,
"pessimisticEIL": 904,
"pessimisticFCP": 2289,
"pessimisticFMP": 3228,
"pessimisticLCP": 3228,
"pessimisticSI": 3047,
"pessimisticTTFCPUI": 5248,
"pessimisticTTI": 5248,
"pessimisticTTFCPUI": 5551,
"pessimisticTTI": 5551,
"roughEstimateOfEIL": 543,
"roughEstimateOfFCP": 2289,
"roughEstimateOfFMP": 2758,
"roughEstimateOfLCP": 2758,
"roughEstimateOfSI": 3681,
"roughEstimateOfTTFCPUI": 3677,
"roughEstimateOfTTI": 4462,
"roughEstimateOfTTFCPUI": 3790,
"roughEstimateOfTTI": 4671,
}
`;
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@

exports[`Byte efficiency base audit should allow overriding of computeWasteWithTTIGraph 1`] = `
Object {
"default": 1330,
"justTTI": 800,
"default": 560,
"justTTI": 504,
}
`;

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 1`] = `960`;

exports[`Byte efficiency base audit should create load simulator with the specified settings 2`] = `22520`;
exports[`Byte efficiency base audit should create load simulator with the specified settings 2`] = `21500`;
Original file line number Diff line number Diff line change
Expand Up @@ -242,15 +242,17 @@ describe('Byte efficiency base audit', () => {
class MockAudit extends ByteEfficiencyAudit {
static audit_(artifacts, records) {
return {
items: records.map(record => ({url: record.url, wastedBytes: record.transferSize})),
items: records.map(record => ({url: record.url, wastedBytes: record.transferSize * 0.5})),
headings: [],
};
}
}

class MockJustTTIAudit extends MockAudit {
static computeWasteWithTTIGraph(results, graph, simulator) {
return ByteEfficiencyAudit.computeWasteWithTTIGraph(results, graph, simulator,
// TODO: Pass in a graph that organically has a lower TTI result rather than forcing it
// to be scaled down.
return 0.9 * ByteEfficiencyAudit.computeWasteWithTTIGraph(results, graph, simulator,
{includeLoad: false});
}
}
Expand Down
8 changes: 4 additions & 4 deletions lighthouse-core/test/audits/dobetterweb/uses-http2-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ describe('Resources are fetched over http/2', () => {
// make sure we flag all the rest
expect(result.details.items).toHaveLength(60);
// make sure we report savings
expect(result.numericValue).toMatchInlineSnapshot(`1310`);
expect(result.details.overallSavingsMs).toMatchInlineSnapshot(`1310`);
expect(result.numericValue).toMatchInlineSnapshot(`1340`);
expect(result.details.overallSavingsMs).toMatchInlineSnapshot(`1340`);
// make sure we have a failing score
expect(result.score).toBeLessThan(0.5);
});
Expand All @@ -70,7 +70,7 @@ describe('Resources are fetched over http/2', () => {
expect(urls).not.toContain(records[30].url);
expect(result.details.items).toHaveLength(30);
// make sure we report less savings
expect(result.numericValue).toMatchInlineSnapshot(`850`);
expect(result.details.overallSavingsMs).toMatchInlineSnapshot(`850`);
expect(result.numericValue).toMatchInlineSnapshot(`500`);
expect(result.details.overallSavingsMs).toMatchInlineSnapshot(`500`);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@

exports[`Metrics: EIL should compute a simulated value 1`] = `
Object {
"optimistic": 101,
"optimistic": 93,
"pessimistic": 101,
"timing": 81,
"timing": 78,
}
`;
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@

exports[`FirstInteractive computed artifact: should simulate when settings specify 1`] = `
Object {
"optimistic": 3351,
"pessimistic": 3502,
"timing": 3351,
"optimistic": 4313,
"pessimistic": 4383,
"timing": 4313,
}
`;
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@

exports[`Metrics: TTI should compute a simulated value 1`] = `
Object {
"optimistic": 3351,
"pessimistic": 3502,
"timing": 3427,
"optimistic": 4313,
"pessimistic": 4383,
"timing": 4348,
}
`;
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@

exports[`Metrics: Lantern EIL should compute a simulated value 1`] = `
Object {
"optimistic": 101,
"optimistic": 93,
"pessimistic": 101,
"timing": 81,
"timing": 78,
}
`;
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@

exports[`Metrics: Lantern TTFCPUI should compute predicted value 1`] = `
Object {
"optimistic": 3351,
"pessimistic": 3502,
"timing": 3351,
"optimistic": 4313,
"pessimistic": 4383,
"timing": 4313,
}
`;
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@

exports[`Metrics: Lantern TTI should compute predicted value 1`] = `
Object {
"optimistic": 3351,
"pessimistic": 3502,
"timing": 3427,
"optimistic": 4313,
"pessimistic": 4383,
"timing": 4348,
}
`;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ describe('FirstInteractive computed artifact:', () => {
optimistic: Math.round(result.optimisticEstimate.timeInMs),
pessimistic: Math.round(result.pessimisticEstimate.timeInMs),
}).toMatchSnapshot();
assert.equal(result.optimisticEstimate.nodeTimings.size, 19);
assert.equal(result.optimisticEstimate.nodeTimings.size, 20);
assert.equal(result.pessimisticEstimate.nodeTimings.size, 80);
assert.ok(result.optimisticGraph, 'should have created optimistic graph');
assert.ok(result.pessimisticGraph, 'should have created pessimistic graph');
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/test/computed/metrics/interactive-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ describe('Metrics: TTI', () => {
optimistic: Math.round(result.optimisticEstimate.timeInMs),
pessimistic: Math.round(result.pessimisticEstimate.timeInMs),
}).toMatchSnapshot();
assert.equal(result.optimisticEstimate.nodeTimings.size, 19);
assert.equal(result.optimisticEstimate.nodeTimings.size, 20);
assert.equal(result.pessimisticEstimate.nodeTimings.size, 80);
assert.ok(result.optimisticGraph, 'should have created optimistic graph');
assert.ok(result.pessimisticGraph, 'should have created pessimistic graph');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ describe('Metrics: Lantern TTFCPUI', () => {
optimistic: Math.round(result.optimisticEstimate.timeInMs),
pessimistic: Math.round(result.pessimisticEstimate.timeInMs),
}).toMatchSnapshot();
assert.equal(result.optimisticEstimate.nodeTimings.size, 19);
assert.equal(result.optimisticEstimate.nodeTimings.size, 20);
assert.equal(result.pessimisticEstimate.nodeTimings.size, 80);
assert.ok(result.optimisticGraph, 'should have created optimistic graph');
assert.ok(result.pessimisticGraph, 'should have created pessimistic graph');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ describe('Metrics: Lantern TTI', () => {
optimistic: Math.round(result.optimisticEstimate.timeInMs),
pessimistic: Math.round(result.pessimisticEstimate.timeInMs),
}).toMatchSnapshot();
assert.equal(result.optimisticEstimate.nodeTimings.size, 19);
assert.equal(result.optimisticEstimate.nodeTimings.size, 20);
assert.equal(result.pessimisticEstimate.nodeTimings.size, 80);
assert.ok(result.optimisticGraph, 'should have created optimistic graph');
assert.ok(result.pessimisticGraph, 'should have created pessimistic graph');
Expand Down
4 changes: 2 additions & 2 deletions lighthouse-core/test/computed/metrics/speed-index-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ describe('Metrics: Speed Index', () => {
}).toMatchInlineSnapshot(`
Object {
"optimistic": 575,
"pessimistic": 563,
"timing": 599,
"pessimistic": 633,
"timing": 635,
}
`);
});
Expand Down
30 changes: 15 additions & 15 deletions lighthouse-core/test/fixtures/lantern-master-accuracy.json
Original file line number Diff line number Diff line change
@@ -1,32 +1,32 @@
{
"roughEstimateOfFCP": {
"p50": 0.292391744233104,
"p90": 0.501397081651661,
"p95": 0.5795269168026101
"p50": 0.28115015974440893,
"p90": 0.49063322495742373,
"p95": 0.5191288993525603
},
"roughEstimateOfFMP": {
"p50": 0.30206175397060275,
"p50": 0.301651376146789,
"p90": 0.5385564466378778,
"p95": 0.6641924269472893
"p95": 0.625285684604197
},
"roughEstimateOfSI": {
"p50": 0.274947094253622,
"p90": 0.701696317749276,
"p95": 1.0080896079651525
"p50": 0.260989010989011,
"p90": 0.6901117087298304,
"p95": 0.9592408214063473
},
"roughEstimateOfTTFCPUI": {
"p50": 0.28465238303454304,
"p90": 0.6509379230322955,
"p95": 0.8098063599218334
"p50": 0.2543563799887577,
"p90": 0.6242117117117117,
"p95": 0.8210511483725518
},
"roughEstimateOfTTI": {
"p50": 0.3055258632709021,
"p90": 0.6577081000373274,
"p50": 0.24779470729751404,
"p90": 0.6419348364220245,
"p95": 0.743897672766033
},
"roughEstimateOfLCP": {
"p50": 0.203757225433526,
"p90": 0.6464365256124721,
"p95": 0.8768898488120951
"p90": 0.6962974535228871,
"p95": 0.9262573279851898
}
}
Loading