Skip to content

ci: Add new metrics overhead app #7300

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

Merged
merged 4 commits into from
Mar 1, 2023
Merged

ci: Add new metrics overhead app #7300

merged 4 commits into from
Mar 1, 2023

Conversation

mydea
Copy link
Member

@mydea mydea commented Feb 28, 2023

To hopefully be able to debug replay issues a bit better, and have a bit more representative checking.

This is a simple plain JS test app that looks something like this:

image

I tried to incorporate some not so efficient stuff to be able to test stuff.

It seems to be working decently even with replay enabled. I built it in a way that you can pass ?count=xxx in order to test it with varying amounts of DOM elements.

@mydea mydea requested review from billyvg and Lms24 February 28, 2023 14:13
@mydea mydea self-assigned this Feb 28, 2023
To hopefully be able to debug replay issues a bit better, and have a bit more representative checking.
@mydea mydea force-pushed the fn/better-metrics-app branch from 99b3687 to c800954 Compare February 28, 2023 14:34
@github-actions
Copy link
Contributor

github-actions bot commented Feb 28, 2023

Replay SDK metrics 🚀

    Plain +Sentry +Replay
Revision Value Value Diff Ratio Value Diff Ratio
LCP This PR fd7126c 74.58 ms 66.86 ms -7.72 ms -10.35 % 68.43 ms -6.15 ms -8.25 %
Baseline dbd7a81 116.00 ms 148.94 ms +32.94 ms +28.40 % 214.37 ms +98.37 ms +84.80 %
CLS This PR fd7126c 0.00 ms 0.00 ms 0.00 ms 0.00 % 0.26 ms +0.25 ms +5471.87 %
Baseline dbd7a81 0.06 ms 0.06 ms -0.00 ms -0.69 % 0.06 ms -0.00 ms -0.79 %
CPU This PR fd7126c 20.95 % 21.49 % +0.54 pp +2.60 % 37.39 % +16.44 pp +78.50 %
Baseline dbd7a81 27.65 % 26.67 % -0.98 pp -3.55 % 34.33 % +6.68 pp +24.16 %
JS heap avg This PR fd7126c 3.52 MB 6.87 MB +3.36 MB +95.32 % 11.05 MB +7.53 MB +213.82 %
Baseline dbd7a81 1.88 MB 2.31 MB +428.27 kB +22.80 % 3.52 MB +1.64 MB +87.53 %
JS heap max This PR fd7126c 3.87 MB 8.44 MB +4.57 MB +117.96 % 14.01 MB +10.14 MB +262.00 %
Baseline dbd7a81 2.28 MB 2.68 MB +399.08 kB +17.47 % 4.1 MB +1.82 MB +79.65 %
netTx This PR fd7126c 0 B 360.46 kB +360.46 kB n/a 106 kB +106 kB n/a
Baseline dbd7a81 0 B 0 B 0 B n/a 2.23 kB +2.23 kB n/a
netRx This PR fd7126c 20.49 kB 18.44 kB -2.05 kB -10.01 % 19.18 kB -1.31 kB -6.39 %
Baseline dbd7a81 0 B 0 B 0 B n/a 41 B +41 B n/a
netCount This PR fd7126c 1 2 +1 +100.00 % 4.4 +3.4000000000000004 +340.00 %
Baseline dbd7a81 0 0 0 n/a 1 +1 n/a
netTime This PR fd7126c 493.45 ms 562.27 ms +68.81 ms +13.95 % 623.67 ms +130.22 ms +26.39 %
Baseline dbd7a81 0.00 ms 0.00 ms 0.00 ms n/a 124.71 ms +124.71 ms n/a

Baseline results on branch: develop

RevisionLCPCLSCPUJS heap avgJS heap maxnetTxnetRxnetCountnetTime
dbd7a81+86.19 ms-0.00 ms+6.77 pp+1.58 MB+1.67 MB+2.23 kB+41 B+1+87.99 ms
67b0684+84.74 ms-0.00 ms+7.38 pp+1.7 MB+1.81 MB+2.23 kB+41 B+1+85.52 ms
4b95c04+57.56 ms-0.00 ms+7.94 pp+920.88 kB+1.05 MB+2.21 kB+41 B+1+90.32 ms
e60cd02+56.25 ms-0.00 ms+6.32 pp+927.44 kB+1.06 MB+2.21 kB+41 B+1+117.55 ms
e25c067+48.34 ms+0.00 ms+5.59 pp+926.37 kB+1.05 MB+2.22 kB+41 B+1+65.23 ms
b1b249b+43.88 ms+0.00 ms+4.80 pp+937.99 kB+1.05 MB+2.22 kB+41 B+1+111.56 ms
12e34d4+28.57 ms+0.00 ms+5.77 pp+930.12 kB+1.04 MB+2.26 kB+41 B+1+109.67 ms
c46c56c+65.45 ms-0.00 ms+5.38 pp+930.26 kB+1.07 MB+2.21 kB+41 B+1+91.29 ms
7f4c4ec+56.64 ms-0.00 ms+5.57 pp+927.42 kB+1.06 MB+2.21 kB+41 B+1+110.83 ms

Previous results on branch: fn/better-metrics-app

RevisionLCPCLSCPUJS heap avgJS heap maxnetTxnetRxnetCountnetTime
541c510-14.41 ms+0.25 ms+18.48 pp+6.95 MB+9.86 MB+103.42 kB-1.29 kB+3+66.84 ms

*) pp - percentage points - an absolute difference between two percentages.
Last updated: Wed, 01 Mar 2023 09:30:36 GMT

Copy link
Member

@Lms24 Lms24 left a comment

Choose a reason for hiding this comment

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

LGTM, just had a couple of questions

Comment on lines 77 to 78
// Wait for flushing, which we set to 2000ms
await new Promise(resolve => setTimeout(resolve, 2000));
Copy link
Member

Choose a reason for hiding this comment

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

Should we wait a little longer than 2000ms considering the timing inconsistencies we observed in #7266?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, true I guess! I'll extend it to 4000ms to be safe.

} else {
window.cumulativeLayoutShiftScore += entry.value;
}
if (window.cumulativeLayoutShiftScore === undefined) {
Copy link
Member

@Lms24 Lms24 Feb 28, 2023

Choose a reason for hiding this comment

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

q: That's the reason for removing this guard here?

Copy link
Member

Choose a reason for hiding this comment

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

Nevermind, I missed that you just rewrote this.

@mydea
Copy link
Member Author

mydea commented Mar 1, 2023

I added a yarn dev:run:replay command to be able to easily run the with-replay app locally for debugging etc.

I also inlined the styles into the html page, as otherwise when debugging replays the styles will not be correct etc.

@mydea mydea merged commit 70abc37 into develop Mar 1, 2023
@mydea mydea deleted the fn/better-metrics-app branch March 1, 2023 10:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants