-
-
Notifications
You must be signed in to change notification settings - Fork 358
iOS UI Profiling #5611
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
base: main
Are you sure you want to change the base?
iOS UI Profiling #5611
Conversation
Semver Impact of This PR⚪ None (no version bump detected) 📋 Changelog PreviewThis is how your changes will appear in the changelog.
🤖 This preview updates automatically when you update the PR. |
Android (legacy) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| f3b058c+dirty | 501.18 ms | 536.70 ms | 35.52 ms |
| bfe454a+dirty | 573.44 ms | 579.46 ms | 6.02 ms |
| 294387d+dirty | 424.30 ms | 465.40 ms | 41.10 ms |
| 955f2eb+dirty | 422.74 ms | 410.19 ms | -12.55 ms |
| 3bd3f0d+dirty | 447.21 ms | 472.31 ms | 25.10 ms |
| d081295+dirty | 408.08 ms | 453.62 ms | 45.54 ms |
| ad27f6e+dirty | 471.44 ms | 516.23 ms | 44.79 ms |
| 07808fb+dirty | 419.10 ms | 419.08 ms | -0.02 ms |
| 170d5ea+dirty | 407.92 ms | 422.49 ms | 14.57 ms |
| 8e653ac+dirty | 360.28 ms | 372.04 ms | 11.76 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| f3b058c+dirty | 43.75 MiB | 48.07 MiB | 4.32 MiB |
| bfe454a+dirty | 17.75 MiB | 19.69 MiB | 1.94 MiB |
| 294387d+dirty | 43.75 MiB | 48.04 MiB | 4.29 MiB |
| 955f2eb+dirty | 17.75 MiB | 19.70 MiB | 1.95 MiB |
| 3bd3f0d+dirty | 17.75 MiB | 19.70 MiB | 1.95 MiB |
| d081295+dirty | 43.75 MiB | 48.04 MiB | 4.29 MiB |
| ad27f6e+dirty | 43.75 MiB | 48.07 MiB | 4.32 MiB |
| 07808fb+dirty | 17.75 MiB | 19.70 MiB | 1.95 MiB |
| 170d5ea+dirty | 17.75 MiB | 19.70 MiB | 1.95 MiB |
| 8e653ac+dirty | 17.75 MiB | 19.75 MiB | 2.00 MiB |
iOS (legacy) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 6bd9054+dirty | 1212.20 ms | 1217.89 ms | 5.70 ms |
| d73150f+dirty | 1198.44 ms | 1210.06 ms | 11.62 ms |
| d081295+dirty | 1214.40 ms | 1211.27 ms | -3.13 ms |
| d1fd647+dirty | 1219.35 ms | 1233.18 ms | 13.83 ms |
| ea3e26e+dirty | 1229.13 ms | 1228.46 ms | -0.67 ms |
| 80e4616+dirty | 1221.32 ms | 1225.64 ms | 4.32 ms |
| 955f2eb+dirty | 1235.06 ms | 1253.88 ms | 18.81 ms |
| 5ee3314+dirty | 1215.18 ms | 1207.64 ms | -7.54 ms |
| 70250df+dirty | 1214.51 ms | 1215.04 ms | 0.53 ms |
| 664c66f+dirty | 1215.37 ms | 1221.30 ms | 5.92 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 6bd9054+dirty | 3.41 MiB | 4.67 MiB | 1.25 MiB |
| d73150f+dirty | 3.38 MiB | 4.60 MiB | 1.22 MiB |
| d081295+dirty | 3.41 MiB | 4.59 MiB | 1.18 MiB |
| d1fd647+dirty | 2.63 MiB | 3.99 MiB | 1.36 MiB |
| ea3e26e+dirty | 3.41 MiB | 4.58 MiB | 1.17 MiB |
| 80e4616+dirty | 3.38 MiB | 4.60 MiB | 1.22 MiB |
| 955f2eb+dirty | 2.63 MiB | 3.98 MiB | 1.35 MiB |
| 5ee3314+dirty | 2.63 MiB | 3.99 MiB | 1.35 MiB |
| 70250df+dirty | 3.44 MiB | 4.59 MiB | 1.15 MiB |
| 664c66f+dirty | 3.38 MiB | 4.60 MiB | 1.22 MiB |
iOS (new) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 6bd9054+dirty | 1207.02 ms | 1199.27 ms | -7.76 ms |
| d73150f+dirty | 1224.52 ms | 1227.17 ms | 2.65 ms |
| d081295+dirty | 1205.24 ms | 1207.52 ms | 2.28 ms |
| d1fd647+dirty | 1218.16 ms | 1225.82 ms | 7.65 ms |
| ea3e26e+dirty | 1216.61 ms | 1214.15 ms | -2.47 ms |
| 80e4616+dirty | 1206.90 ms | 1205.94 ms | -0.96 ms |
| 955f2eb+dirty | 1225.78 ms | 1239.27 ms | 13.49 ms |
| 5ee3314+dirty | 1234.25 ms | 1235.44 ms | 1.19 ms |
| 70250df+dirty | 1211.96 ms | 1222.31 ms | 10.35 ms |
| 664c66f+dirty | 1195.94 ms | 1194.80 ms | -1.14 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 6bd9054+dirty | 3.41 MiB | 4.67 MiB | 1.25 MiB |
| d73150f+dirty | 3.38 MiB | 4.60 MiB | 1.22 MiB |
| d081295+dirty | 3.41 MiB | 4.59 MiB | 1.18 MiB |
| d1fd647+dirty | 3.19 MiB | 4.56 MiB | 1.37 MiB |
| ea3e26e+dirty | 3.41 MiB | 4.58 MiB | 1.17 MiB |
| 80e4616+dirty | 3.38 MiB | 4.60 MiB | 1.22 MiB |
| 955f2eb+dirty | 3.19 MiB | 4.55 MiB | 1.36 MiB |
| 5ee3314+dirty | 3.19 MiB | 4.55 MiB | 1.37 MiB |
| 70250df+dirty | 3.44 MiB | 4.59 MiB | 1.15 MiB |
| 664c66f+dirty | 3.38 MiB | 4.60 MiB | 1.22 MiB |
| * @platform android | ||
| */ | ||
| androidProfilingOptions?: AndroidProfilingOptions; | ||
| profilingOptions?: ProfilingOptions; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we keep androidProfilingOptions and mark it as deprecated?
We can merge both values on the wrapper init and on a later version remove it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Android (new) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| f3b058c+dirty | 422.90 ms | 468.30 ms | 45.40 ms |
| 294387d+dirty | 359.44 ms | 393.40 ms | 33.97 ms |
| 1d62dde+dirty | 366.59 ms | 408.80 ms | 42.21 ms |
| d081295+dirty | 416.95 ms | 461.24 ms | 44.29 ms |
| ad27f6e+dirty | 484.67 ms | 532.79 ms | 48.12 ms |
| 1226664+dirty | 377.65 ms | 453.94 ms | 76.29 ms |
| 083f560+dirty | 383.96 ms | 417.76 ms | 33.80 ms |
| ec14be7+dirty | 401.58 ms | 475.26 ms | 73.68 ms |
| eb07ba3+dirty | 419.49 ms | 482.12 ms | 62.63 ms |
| d1fd647+dirty | 374.46 ms | 409.51 ms | 35.05 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| f3b058c+dirty | 43.94 MiB | 48.90 MiB | 4.96 MiB |
| 294387d+dirty | 43.94 MiB | 48.87 MiB | 4.93 MiB |
| 1d62dde+dirty | 7.15 MiB | 8.46 MiB | 1.31 MiB |
| d081295+dirty | 43.94 MiB | 48.87 MiB | 4.93 MiB |
| ad27f6e+dirty | 43.94 MiB | 48.90 MiB | 4.96 MiB |
| 1226664+dirty | 7.15 MiB | 8.46 MiB | 1.30 MiB |
| 083f560+dirty | 7.15 MiB | 8.43 MiB | 1.28 MiB |
| ec14be7+dirty | 7.15 MiB | 8.42 MiB | 1.26 MiB |
| eb07ba3+dirty | 7.15 MiB | 8.42 MiB | 1.27 MiB |
| d1fd647+dirty | 7.15 MiB | 8.43 MiB | 1.28 MiB |
|
|
||
| ### Features | ||
|
|
||
| - Extends the experimental support of UI profiling to iOS ([#5611](https://github.com/getsentry/sentry-react-native/pull/5611)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we decide to proceed with just renaming the androidProfilingOptions to profilingOptions in the conversation above let's add a note to the changelog
| - Extends the experimental support of UI profiling to iOS ([#5611](https://github.com/getsentry/sentry-react-native/pull/5611)) | |
| - Extends the experimental support of UI profiling to iOS ([#5611](https://github.com/getsentry/sentry-react-native/pull/5611)) | |
| - Note that Android is also now using `profilingOptions` instead of `androidProfilingOptions` |
antonis
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes LGTM 🚀
Let's make that we handle the androidProfilingOptions deprecation/removal consistently before merging and update the docs.
📢 Type of change
📜 Description
Added missing support for UI profiling on iOS.
💡 Motivation and Context
Fixes #5517
📝 Checklist
sendDefaultPIIis enabled🔮 Next steps
Merge it and enjoy!