-
-
Notifications
You must be signed in to change notification settings - Fork 245
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
feat(android): use debug image loading by address api #2706
feat(android): use debug image loading by address api #2706
Conversation
|
🚨 Detected changes in high risk code 🚨High-risk code has higher potential to break the SDK and may be hard to test. To prevent severe bugs, apply the rollout process for releasing such changes and be extra careful when changing and reviewing these files:
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2706 +/- ##
===========================================
+ Coverage 78.37% 92.09% +13.71%
===========================================
Files 35 91 +56
Lines 1489 2946 +1457
===========================================
+ Hits 1167 2713 +1546
+ Misses 322 233 -89 ☔ View full report in Codecov by Sentry. |
Android Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
e893df5 | 310.60 ms | 380.58 ms | 69.98 ms |
8de999e | 475.70 ms | 531.38 ms | 55.68 ms |
636cb61 | 366.59 ms | 448.14 ms | 81.55 ms |
1596141 | 300.92 ms | 368.94 ms | 68.02 ms |
8bac8ba | 467.65 ms | 520.27 ms | 52.63 ms |
e66e71e | 296.84 ms | 345.43 ms | 48.59 ms |
9555112 | 448.81 ms | 488.89 ms | 40.08 ms |
95c69e3 | 379.93 ms | 441.96 ms | 62.02 ms |
30c1193 | 349.00 ms | 438.20 ms | 89.20 ms |
6d317ea | 303.46 ms | 356.06 ms | 52.60 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
e893df5 | 6.06 MiB | 7.09 MiB | 1.03 MiB |
8de999e | 6.46 MiB | 7.48 MiB | 1.01 MiB |
636cb61 | 6.27 MiB | 7.20 MiB | 959.08 KiB |
1596141 | 6.16 MiB | 7.14 MiB | 1003.98 KiB |
8bac8ba | 6.46 MiB | 7.48 MiB | 1.03 MiB |
e66e71e | 6.06 MiB | 7.09 MiB | 1.03 MiB |
9555112 | 6.52 MiB | 7.59 MiB | 1.06 MiB |
95c69e3 | 6.35 MiB | 7.35 MiB | 1021.71 KiB |
30c1193 | 6.27 MiB | 7.20 MiB | 958.74 KiB |
6d317ea | 5.94 MiB | 6.92 MiB | 1001.74 KiB |
Previous results on branch: feat/enh-debug-image-loading-by-address-android
Startup times
Revision | Plain | With Sentry | Diff |
---|---|---|---|
2c2fb86 | 449.77 ms | 547.95 ms | 98.19 ms |
1e742da | 450.84 ms | 519.37 ms | 68.53 ms |
59d6df4 | 547.96 ms | 553.21 ms | 5.26 ms |
d5d7eeb | 504.00 ms | 603.67 ms | 99.67 ms |
3981290 | 419.54 ms | 509.96 ms | 90.41 ms |
8244c3f | 512.27 ms | 587.32 ms | 75.05 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
2c2fb86 | 6.46 MiB | 7.48 MiB | 1.02 MiB |
1e742da | 6.46 MiB | 7.48 MiB | 1.02 MiB |
59d6df4 | 6.46 MiB | 7.48 MiB | 1.02 MiB |
d5d7eeb | 6.46 MiB | 7.48 MiB | 1.02 MiB |
3981290 | 6.46 MiB | 7.48 MiB | 1.02 MiB |
8244c3f | 6.46 MiB | 7.48 MiB | 1.02 MiB |
iOS Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
e82709a | 1249.22 ms | 1271.35 ms | 22.12 ms |
1cc9547 | 1230.81 ms | 1260.79 ms | 29.98 ms |
99704d2 | 1241.74 ms | 1258.00 ms | 16.26 ms |
934f10e | 1245.18 ms | 1263.77 ms | 18.59 ms |
6aab859 | 1245.14 ms | 1247.59 ms | 2.45 ms |
0118295 | 1211.31 ms | 1227.02 ms | 15.71 ms |
8fe0817 | 1235.41 ms | 1245.51 ms | 10.10 ms |
d7dc4e5 | 1249.15 ms | 1275.63 ms | 26.49 ms |
f4cc744 | 1274.57 ms | 1290.79 ms | 16.22 ms |
3a43905 | 1254.31 ms | 1266.35 ms | 12.04 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
e82709a | 8.33 MiB | 9.40 MiB | 1.07 MiB |
1cc9547 | 8.42 MiB | 9.86 MiB | 1.44 MiB |
99704d2 | 8.42 MiB | 9.87 MiB | 1.44 MiB |
934f10e | 8.38 MiB | 9.77 MiB | 1.39 MiB |
6aab859 | 8.29 MiB | 9.36 MiB | 1.07 MiB |
0118295 | 8.32 MiB | 9.38 MiB | 1.05 MiB |
8fe0817 | 8.29 MiB | 9.39 MiB | 1.10 MiB |
d7dc4e5 | 8.42 MiB | 9.91 MiB | 1.48 MiB |
f4cc744 | 8.16 MiB | 9.16 MiB | 1.01 MiB |
3a43905 | 8.10 MiB | 9.18 MiB | 1.08 MiB |
Previous results on branch: feat/enh-debug-image-loading-by-address-android
Startup times
Revision | Plain | With Sentry | Diff |
---|---|---|---|
2c2fb86 | 1233.27 ms | 1254.02 ms | 20.75 ms |
1e742da | 1257.29 ms | 1266.13 ms | 8.84 ms |
59d6df4 | 1248.85 ms | 1261.83 ms | 12.98 ms |
8244c3f | 1249.88 ms | 1264.52 ms | 14.65 ms |
3981290 | 1247.02 ms | 1260.45 ms | 13.43 ms |
d5d7eeb | 1251.60 ms | 1262.70 ms | 11.09 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
2c2fb86 | 8.42 MiB | 9.91 MiB | 1.49 MiB |
1e742da | 8.42 MiB | 9.91 MiB | 1.49 MiB |
59d6df4 | 8.42 MiB | 9.91 MiB | 1.48 MiB |
8244c3f | 8.42 MiB | 9.91 MiB | 1.49 MiB |
3981290 | 8.42 MiB | 9.91 MiB | 1.49 MiB |
d5d7eeb | 8.42 MiB | 9.91 MiB | 1.49 MiB |
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.
Code looks good, can we test this somehow? On iOS we do some conversion of the addresses, but i imagine that is just because they are a non-string type? -> Ah, seen that manual testing was done.
@denrase I'd like to add some automated tests for this but not sure how to get an obfuscated build into the test suite |
I'll take a look if we can do this, it should be possible with maestro but maybe with the flutter integration tests as well update: didnt add a full obfuscated integration test yet, but in the test I'm grabbing all debug images, get one the images and create a synthetic stack frame with an instruction address that should be within the range of the image and then use the new API to grab only the relevant image which should be 1 in the end. imo this is good enough for now |
📜 Description
💡 Motivation and Context
Closes #2440
Uses api that only fetches relevant debug images and not everything that's available. This reduces the data transfer between the bridge immensely and reduces the final envelope size
💚 How did you test it?
Manual, couldn't figure out how to have automated tests for features requiring obfuscated builds
📝 Checklist
sendDefaultPii
is enabled🔮 Next steps