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.

Sign up for GitHub

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(network-analyzer): use arithmetic mean for median #15096

Merged
merged 8 commits into from
May 20, 2023
Merged

core(network-analyzer): use arithmetic mean for median #15096

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
d8af3c7
core(network-analyzer): use arithmetic mean for median of even samples
connorjclark May 18, 2023
fb08222
Update core/lib/dependency-graph/simulator/network-analyzer.js
connorjclark May 18, 2023
a5eca34
comments
connorjclark May 18, 2023
77f273e
cool
connorjclark May 18, 2023
1e537ed
update test snaps
connorjclark May 18, 2023
2df98d4
cantdothat
connorjclark May 18, 2023
0f274c6
jest snapshot update format sux
connorjclark May 18, 2023
e6f6c90
Merge remote-tracking branch 'origin/main' into nit-median-avg
connorjclark May 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion core/lib/dependency-graph/simulator/network-analyzer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,22 @@ class NetworkAnalyzer {
static getSummary(values) {
values.sort((a, b) => a - b);

let median;
if (values.length === 0) {
median = values[0];
} else if (values.length % 2 === 0) {
const a = values[Math.floor((values.length - 1) / 2)];
const b = values[Math.floor((values.length - 1) / 2) + 1];
median = (a + b) / 2;
} else {
median = values[Math.floor((values.length - 1) / 2)];
}

return {
min: values[0],
max: values[values.length - 1],
avg: values.reduce((a, b) => a + b, 0) / values.length,
median: values[Math.floor((values.length - 1) / 2)],
median,
};
}

Expand Down
16 changes: 8 additions & 8 deletions core/test/audits/__snapshots__/metrics-test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,15 +122,15 @@ exports[`Performance: metrics evaluates valid input (with lcp) correctly 1`] = `
Object {
"cumulativeLayoutShift": 0,
"cumulativeLayoutShiftMainFrame": 0,
"firstContentfulPaint": 2289,
"firstContentfulPaint": 2291,
"firstContentfulPaintAllFrames": undefined,
"firstContentfulPaintAllFramesTs": undefined,
"firstContentfulPaintTs": undefined,
"firstMeaningfulPaint": 2758,
"firstMeaningfulPaint": 2761,
"firstMeaningfulPaintTs": undefined,
"interactive": 4367,
"interactive": 4446,
"interactiveTs": undefined,
"largestContentfulPaint": 2758,
"largestContentfulPaint": 2761,
"largestContentfulPaintAllFrames": undefined,
"largestContentfulPaintAllFramesTs": undefined,
"largestContentfulPaintTs": undefined,
Expand Down Expand Up @@ -168,9 +168,9 @@ Object {
"observedTotalCumulativeLayoutShift": 0,
"observedTraceEnd": 7416,
"observedTraceEndTs": 713044439102,
"speedIndex": 3681,
"speedIndex": 3683,
"speedIndexTs": undefined,
"timeToFirstByte": 609,
"timeToFirstByte": 611,
"timeToFirstByteTs": undefined,
"totalBlockingTime": 1205,
"totalCumulativeLayoutShift": 0,
Expand Down Expand Up @@ -246,7 +246,7 @@ Object {
"firstContentfulPaintTs": undefined,
"firstMeaningfulPaint": 1541,
"firstMeaningfulPaintTs": undefined,
"interactive": 4205,
"interactive": 4206,
"interactiveTs": undefined,
"largestContentfulPaint": undefined,
"largestContentfulPaintAllFrames": undefined,
Expand Down Expand Up @@ -288,7 +288,7 @@ Object {
"observedTraceEndTs": 225426711887,
"speedIndex": 1676,
"speedIndexTs": undefined,
"timeToFirstByte": 759,
"timeToFirstByte": 760,
"timeToFirstByteTs": undefined,
"totalBlockingTime": 777,
"totalCumulativeLayoutShift": 0,
Expand Down
30 changes: 15 additions & 15 deletions core/test/audits/__snapshots__/predictive-perf-test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,23 @@

exports[`Performance: predictive performance audit should compute the predicted values 1`] = `
Object {
"optimisticFCP": 2289,
"optimisticFMP": 2289,
"optimisticLCP": 2289,
"optimisticFCP": 2291,
"optimisticFMP": 2291,
"optimisticLCP": 2291,
"optimisticSI": 1393,
"optimisticTTI": 3790,
"pessimisticFCP": 2289,
"pessimisticFMP": 3228,
"pessimisticLCP": 3228,
"pessimisticSI": 3047,
"pessimisticTTI": 4944,
"roughEstimateOfFCP": 2289,
"roughEstimateOfFMP": 2758,
"roughEstimateOfLCP": 2758,
"optimisticTTI": 3792,
"pessimisticFCP": 2291,
"pessimisticFMP": 3230,
"pessimisticLCP": 3230,
"pessimisticSI": 3049,
"pessimisticTTI": 5099,
"roughEstimateOfFCP": 2291,
"roughEstimateOfFMP": 2761,
"roughEstimateOfLCP": 2761,
"roughEstimateOfLCPLoadEnd": undefined,
"roughEstimateOfLCPLoadStart": undefined,
"roughEstimateOfSI": 3681,
"roughEstimateOfTTFB": 609,
"roughEstimateOfTTI": 4367,
"roughEstimateOfSI": 3683,
"roughEstimateOfTTFB": 611,
"roughEstimateOfTTI": 4446,
}
`;
2 changes: 1 addition & 1 deletion core/test/audits/diagnostics-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ describe('Diagnostics audit', () => {
const result = await Diagnostics.audit(artifacts, {computedCache: new Map()});
expect(result.details.items[0]).toEqual({
maxRtt: 3.6660000041592014,
maxServerLatency: 159.5549999910874,
maxServerLatency: 159.70249997917608,
numFonts: 1,
numRequests: 66,
numScripts: 6,
Expand Down
2 changes: 1 addition & 1 deletion core/test/audits/metrics/__snapshots__/first-meaningful-paint-test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

exports[`Performance: first-meaningful-paint audit computes FMP correctly for simulated 1`] = `
Object {
"numericValue": 1540.6100000208241,
"numericValue": 1540.8905000321101,
"score": 0.99,
}
`;
8 changes: 4 additions & 4 deletions core/test/audits/network-server-latency-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@ describe('Network Server Latency audit', () => {
// artifical response time = Math.max(real response time, 150ms)
expect(result.details.items).toEqual([
{
origin: 'https://www.google-analytics.com',
serverResponseTime: 159.55,
origin: 'https://pwa.rocks',
serverResponseTime: 159.7,
},
{
origin: 'https://pwa.rocks',
serverResponseTime: 159.42,
origin: 'https://www.google-analytics.com',
serverResponseTime: 159.55,
},
{
origin: 'https://www.googletagmanager.com',
Expand Down
2 changes: 1 addition & 1 deletion core/test/audits/predictive-perf-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ describe('Performance: predictive performance audit', () => {
const context = {computedCache: new Map(), settings: {locale: 'en'}};

const output = await PredictivePerf.audit(artifacts, context);
expect(output.displayValue).toBeDisplayString('4,370 ms');
expect(output.displayValue).toBeDisplayString('4,450 ms');
const metrics = output.details.items[0];
for (const [key, value] of Object.entries(metrics)) {
metrics[key] = value === undefined ? value : Math.round(value);
Expand Down
4 changes: 1 addition & 3 deletions core/test/computed/load-simulator-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,7 @@ describe('Simulator artifact', () => {
expect(additionalRttByOrigin.get('https://pwa.rocks')).toMatchInlineSnapshot(
`0.3960000176447025`
);
expect(serverResponseTimeByOrigin.get('https://pwa.rocks')).toMatchInlineSnapshot(
`159.42199996789026`
);
expect(serverResponseTimeByOrigin.get('https://pwa.rocks')).toMatchInlineSnapshot(`159.70249997917608`);
});

it('returns a simulator with precomputed lantern data', async () => {
Expand Down
6 changes: 3 additions & 3 deletions core/test/computed/metrics/__snapshots__/interactive-test.js.snap
View file
Open in desktop
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": 4177,
"pessimistic": 4233,
"timing": 4205,
"optimistic": 4178,
"pessimistic": 4234,
"timing": 4206,
}
`;
6 changes: 3 additions & 3 deletions core/test/computed/metrics/__snapshots__/lantern-interactive-test.js.snap
View file
Open in desktop
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": 4177,
"pessimistic": 4233,
"timing": 4205,
"optimistic": 4178,
"pessimistic": 4234,
"timing": 4206,
}
`;

Expand Down
18 changes: 8 additions & 10 deletions core/test/computed/metrics/lantern-largest-contentful-paint-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,16 +26,14 @@ describe('Metrics: Lantern LCP', () => {
expect({
timing: Math.round(result.timing),
optimistic: Math.round(result.optimisticEstimate.timeInMs),
pessimistic: Math.round(result.pessimisticEstimate.timeInMs),
}).toMatchInlineSnapshot(
`
Object {
"optimistic": 2289,
"pessimistic": 3228,
"timing": 2758,
}
`
);
pessimistic: Math.round(result.pessimisticEstimate.timeInMs)}).
toMatchInlineSnapshot(`
Object {
"optimistic": 2291,
"pessimistic": 3230,
"timing": 2761,
}
`);
assert.equal(result.optimisticEstimate.nodeTimings.size, 12);
assert.equal(result.pessimisticEstimate.nodeTimings.size, 19);
assert.ok(result.optimisticGraph, 'should have created optimistic graph');
Expand Down
16 changes: 8 additions & 8 deletions core/test/computed/metrics/lantern-speed-index-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,14 @@ describe('Metrics: Lantern Speed Index', () => {
expect({
timing: Math.round(result.timing),
optimistic: Math.round(result.optimisticEstimate.timeInMs),
pessimistic: Math.round(result.pessimisticEstimate.timeInMs),
}).toMatchInlineSnapshot(`
Object {
"optimistic": 605,
"pessimistic": 2439,
"timing": 3007,
}
`);
pessimistic: Math.round(result.pessimisticEstimate.timeInMs)}).
toMatchInlineSnapshot(`
Object {
"optimistic": 605,
"pessimistic": 2440,
"timing": 3008,
}
`);
});

it('should not scale coefficients at default', async () => {
Expand Down
16 changes: 8 additions & 8 deletions core/test/computed/metrics/largest-contentful-paint-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,14 @@ describe('Metrics: LCP', () => {
expect({
timing: Math.round(result.timing),
optimistic: Math.round(result.optimisticEstimate.timeInMs),
pessimistic: Math.round(result.pessimisticEstimate.timeInMs),
}).toMatchInlineSnapshot(`
Object {
"optimistic": 2289,
"pessimistic": 3228,
"timing": 2758,
}
`);
pessimistic: Math.round(result.pessimisticEstimate.timeInMs)}).
toMatchInlineSnapshot(`
Object {
"optimistic": 2291,
"pessimistic": 3230,
"timing": 2761,
}
`);
});

it('should compute an observed value', async () => {
Expand Down
4 changes: 2 additions & 2 deletions core/test/computed/network-analysis-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,10 @@ Map {
`);
expect(result.serverResponseTimeByOrigin).toMatchInlineSnapshot(`
Map {
"https://pwa.rocks" => 159.42199996789026,
"https://pwa.rocks" => 159.70249997917608,
"https://www.googletagmanager.com" => 153.03200000198592,
"https://www.google-analytics.com" => 159.5549999910874,
"__SUMMARY__" => 159.42199996789026,
"__SUMMARY__" => 159.48849997948884,
}
`);
});
Expand Down
6 changes: 3 additions & 3 deletions core/test/fixtures/fraggle-rock/reports/sample-flow-result.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1204,7 +1204,7 @@
},
{
"origin": "https://fonts.gstatic.com",
"serverResponseTime": 16.217999999999993
"serverResponseTime": 17.328999999999994
},
{
"origin": "https://www.mikescerealshack.co",
Expand Down Expand Up @@ -8766,7 +8766,7 @@
},
{
"origin": "https://mnl4bjjsnz-dsn.algolia.net",
"serverResponseTime": 0
"serverResponseTime": 263.2025
}
Copy link
Collaborator Author

@connorjclark connorjclark May 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmmm seems like a lot..

Copy link
Collaborator Author

@connorjclark connorjclark May 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This came from these estimates:
image

in _estimateResponseTimeByOrigin's Math.max(ttfb - rtt, 0), ttfb - rtt is somehow negative so we get a zero as an estimate.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The zero estimate came from this record: https://mnl4bjjsnz-dsn.algolia.net/1/indexes/dev_OFFICE_SCENES/query, ttfb 49 (slightly more than the rtt estimate of 49.56)

The second, higher estimate came from: https://mnl4bjjsnz-dsn.algolia.net/1/indexes/dev_OFFICE_SCENES/query, ttfb 575

it's reasonable for query time to be so variable for such a website, so I think taking the average-ish value here (via the median changes in this PR) is good.

],
"sortedBy": [
Expand Down Expand Up @@ -17696,7 +17696,7 @@
},
{
"origin": "https://fonts.gstatic.com",
"serverResponseTime": 0.874
"serverResponseTime": 0.9365
}
],
"sortedBy": [
Expand Down
18 changes: 9 additions & 9 deletions core/test/fixtures/lantern-baseline-accuracy.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,27 +1,27 @@
{
"roughEstimateOfFCP": {
"p50": 0.292391744233104,
"p90": 0.49063322495742373,
"p50": 0.2880704563650921,
"p90": 0.48970428858956494,
"p95": 0.5191288993525603
},
"roughEstimateOfFMP": {
"p50": 0.3131914893617021,
"p50": 0.3123404255319149,
"p90": 0.5385564466378778,
"p95": 0.6417150302025346
"p95": 0.6414781475778751
},
"roughEstimateOfSI": {
"p50": 0.26498844703879365,
"p50": 0.26511005715675545,
"p90": 0.6462556888705007,
"p95": 0.9006036782254668
},
"roughEstimateOfTTI": {
"p50": 0.26853932584269663,
"p90": 0.674238788460513,
"p95": 0.743897672766033
"p90": 0.6741321388577828,
"p95": 0.7425119914727305
},
"roughEstimateOfLCP": {
"p50": 0.20407433881343817,
"p90": 0.6299877796078848,
"p50": 0.20175125089349535,
"p90": 0.6705274690874942,
"p95": 0.8768898488120951
}
}
Loading