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

ref: pass options object into SentryTimeToDisplayTracker #3989

Closed
wants to merge 2 commits into from

Conversation

armcknight
Copy link
Member

This will help us use the enableContinuousProfiling option in the implementation without adding too many new parameters to the initializer.

#skip-changelog; for #3555

Copy link

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1227.67 ms 1244.38 ms 16.72 ms
Size 21.58 KiB 630.28 KiB 608.70 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
a176fc4 1250.29 ms 1257.88 ms 7.59 ms
8f212af 1246.41 ms 1270.98 ms 24.57 ms
621ba9b 1190.66 ms 1230.84 ms 40.18 ms
d61b939 1238.61 ms 1240.08 ms 1.47 ms
50bb751 1296.52 ms 1323.73 ms 27.21 ms
16450b8 1196.33 ms 1217.26 ms 20.93 ms
6943de0 1237.67 ms 1247.12 ms 9.45 ms
1c96132 1229.29 ms 1248.14 ms 18.86 ms
caa37b6 1197.43 ms 1211.52 ms 14.09 ms
c0b4b71 1218.16 ms 1251.28 ms 33.12 ms

App size

Revision Plain With Sentry Diff
a176fc4 22.84 KiB 403.24 KiB 380.39 KiB
8f212af 22.84 KiB 403.14 KiB 380.29 KiB
621ba9b 20.76 KiB 414.45 KiB 393.69 KiB
d61b939 22.85 KiB 407.63 KiB 384.78 KiB
50bb751 21.58 KiB 417.85 KiB 396.27 KiB
16450b8 22.85 KiB 410.98 KiB 388.13 KiB
6943de0 20.76 KiB 393.33 KiB 372.57 KiB
1c96132 21.58 KiB 418.51 KiB 396.93 KiB
caa37b6 21.58 KiB 424.34 KiB 402.76 KiB
c0b4b71 20.76 KiB 430.98 KiB 410.22 KiB

@armcknight
Copy link
Member Author

This actually isn't good enough, because SentryUIViewControllerPerformanceTracker is a singleton 😭 we need to thread its initialization through to somewhere instead of having it init itself, so the options can be passed in.

@philipphofmann
Copy link
Member

As mentioned in #3938 (comment), I think we don't need this PR.

@armcknight armcknight closed this May 30, 2024
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.

None yet

2 participants