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

Use cached methods SentryDebugImageProvider of the Cocoa SDK #4194

Merged
merged 12 commits into from
Nov 7, 2024

Conversation

antonis
Copy link
Collaborator

@antonis antonis commented Oct 21, 2024

📢 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring

📜 Description

Replaces getDebugImagesForAddresses:isCrash: with getDebugImagesForImageAddressesFromCache from getsentry/sentry-cocoa#4435 to speed up getBinaryImages for finishing transactions and capturing events

💡 Motivation and Context

Fixes #4169

💚 How did you test it?

Manual tests, CI
Added tests in getsentry/sentry-cocoa#4460

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled
  • All tests passing
  • No breaking changes

🔮 Next steps

Copy link
Contributor

github-actions bot commented Oct 21, 2024

Android (legacy) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 442.96 ms 425.22 ms -17.74 ms
Size 17.74 MiB 20.08 MiB 2.34 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
c2a4e9b 480.42 ms 485.60 ms 5.18 ms
4cc5c27 460.04 ms 496.32 ms 36.28 ms
0d3e677 422.82 ms 411.90 ms -10.92 ms
c639edf 466.48 ms 489.57 ms 23.09 ms
d2c32bb 448.85 ms 450.19 ms 1.34 ms
ac41368 451.47 ms 453.67 ms 2.20 ms
5bb8d5f 431.21 ms 459.40 ms 28.19 ms
5571a20 410.55 ms 441.06 ms 30.51 ms
3853f43 329.68 ms 346.32 ms 16.64 ms
2ec71da 438.14 ms 460.46 ms 22.32 ms

App size

Revision Plain With Sentry Diff
c2a4e9b 17.73 MiB 20.06 MiB 2.33 MiB
4cc5c27 17.73 MiB 19.95 MiB 2.21 MiB
0d3e677 17.74 MiB 20.07 MiB 2.34 MiB
c639edf 17.74 MiB 20.08 MiB 2.34 MiB
d2c32bb 17.74 MiB 20.08 MiB 2.34 MiB
ac41368 17.73 MiB 20.11 MiB 2.38 MiB
5bb8d5f 17.73 MiB 19.93 MiB 2.20 MiB
5571a20 17.73 MiB 19.93 MiB 2.19 MiB
3853f43 17.73 MiB 19.81 MiB 2.08 MiB
2ec71da 17.73 MiB 20.10 MiB 2.37 MiB

Previous results on branch: antonis/4169-cocoa-DebugImagesFromCache

Startup times

Revision Plain With Sentry Diff
ff294e7 491.08 ms 484.57 ms -6.51 ms
16c15ec 472.43 ms 479.40 ms 6.97 ms
2172ba1 474.84 ms 471.14 ms -3.70 ms
addbb57 456.54 ms 452.58 ms -3.96 ms

App size

Revision Plain With Sentry Diff
ff294e7 17.74 MiB 20.08 MiB 2.34 MiB
16c15ec 17.74 MiB 20.07 MiB 2.34 MiB
2172ba1 17.74 MiB 20.08 MiB 2.34 MiB
addbb57 17.74 MiB 20.08 MiB 2.34 MiB

Copy link
Contributor

github-actions bot commented Oct 21, 2024

iOS (legacy) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1238.80 ms 1238.78 ms -0.02 ms
Size 2.36 MiB 3.10 MiB 751.38 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
728164b+dirty 1256.10 ms 1259.08 ms 2.98 ms
dadc233+dirty 1223.20 ms 1236.88 ms 13.68 ms
52a8031+dirty 1280.88 ms 1289.78 ms 8.90 ms
12427f4+dirty 1267.15 ms 1271.30 ms 4.15 ms
d7401ac+dirty 1252.38 ms 1275.04 ms 22.66 ms
148f924+dirty 1214.76 ms 1215.73 ms 0.97 ms
a989877+dirty 1228.56 ms 1227.71 ms -0.85 ms
fe13591+dirty 1208.25 ms 1219.53 ms 11.28 ms
484813b+dirty 1222.45 ms 1220.79 ms -1.66 ms
ad6c299+dirty 1244.76 ms 1260.10 ms 15.34 ms

App size

Revision Plain With Sentry Diff
728164b+dirty 2.36 MiB 2.88 MiB 530.38 KiB
dadc233+dirty 2.36 MiB 2.84 MiB 486.85 KiB
52a8031+dirty 2.36 MiB 2.82 MiB 469.44 KiB
12427f4+dirty 2.36 MiB 2.88 MiB 530.38 KiB
d7401ac+dirty 2.36 MiB 2.83 MiB 481.14 KiB
148f924+dirty 2.36 MiB 3.04 MiB 696.25 KiB
a989877+dirty 2.36 MiB 3.10 MiB 752.40 KiB
fe13591+dirty 2.36 MiB 3.10 MiB 752.40 KiB
484813b+dirty 2.36 MiB 3.08 MiB 734.18 KiB
ad6c299+dirty 2.36 MiB 2.84 MiB 488.85 KiB

Previous results on branch: antonis/4169-cocoa-DebugImagesFromCache

Startup times

Revision Plain With Sentry Diff
16c15ec+dirty 1227.53 ms 1231.49 ms 3.96 ms
2172ba1+dirty 1229.92 ms 1227.08 ms -2.84 ms
addbb57+dirty 1232.02 ms 1239.22 ms 7.20 ms
ff294e7+dirty 1233.35 ms 1235.80 ms 2.45 ms

App size

Revision Plain With Sentry Diff
16c15ec+dirty 2.36 MiB 3.08 MiB 734.95 KiB
2172ba1+dirty 2.36 MiB 3.08 MiB 737.12 KiB
addbb57+dirty 2.36 MiB 3.08 MiB 737.15 KiB
ff294e7+dirty 2.36 MiB 3.08 MiB 735.60 KiB

Copy link
Contributor

github-actions bot commented Oct 21, 2024

iOS (new) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1228.33 ms 1220.47 ms -7.86 ms
Size 2.92 MiB 3.66 MiB 755.55 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
728164b+dirty 1280.06 ms 1285.26 ms 5.20 ms
dadc233+dirty 1266.52 ms 1282.55 ms 16.03 ms
52a8031+dirty 1255.96 ms 1273.00 ms 17.04 ms
12427f4+dirty 1224.90 ms 1231.40 ms 6.50 ms
d7401ac+dirty 1288.10 ms 1289.54 ms 1.44 ms
148f924+dirty 1220.72 ms 1221.30 ms 0.58 ms
a989877+dirty 1222.90 ms 1219.89 ms -3.00 ms
fe13591+dirty 1250.69 ms 1246.27 ms -4.43 ms
484813b+dirty 1225.07 ms 1221.00 ms -4.07 ms
ad6c299+dirty 1248.50 ms 1248.88 ms 0.38 ms

App size

Revision Plain With Sentry Diff
728164b+dirty 2.92 MiB 3.44 MiB 533.26 KiB
dadc233+dirty 2.92 MiB 3.40 MiB 492.53 KiB
52a8031+dirty 2.92 MiB 3.38 MiB 475.71 KiB
12427f4+dirty 2.92 MiB 3.44 MiB 533.29 KiB
d7401ac+dirty 2.92 MiB 3.40 MiB 488.06 KiB
148f924+dirty 2.92 MiB 3.60 MiB 701.88 KiB
a989877+dirty 2.92 MiB 3.66 MiB 757.66 KiB
fe13591+dirty 2.92 MiB 3.66 MiB 757.71 KiB
484813b+dirty 2.92 MiB 3.64 MiB 740.56 KiB
ad6c299+dirty 2.92 MiB 3.40 MiB 494.12 KiB

Previous results on branch: antonis/4169-cocoa-DebugImagesFromCache

Startup times

Revision Plain With Sentry Diff
16c15ec+dirty 1244.35 ms 1249.24 ms 4.89 ms
2172ba1+dirty 1219.27 ms 1214.83 ms -4.44 ms
addbb57+dirty 1239.21 ms 1237.73 ms -1.47 ms
ff294e7+dirty 1242.11 ms 1237.53 ms -4.58 ms

App size

Revision Plain With Sentry Diff
16c15ec+dirty 2.92 MiB 3.64 MiB 740.58 KiB
2172ba1+dirty 2.92 MiB 3.64 MiB 743.02 KiB
addbb57+dirty 2.92 MiB 3.64 MiB 742.91 KiB
ff294e7+dirty 2.92 MiB 3.64 MiB 741.28 KiB

Copy link
Contributor

github-actions bot commented Oct 21, 2024

Android (new) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 398.63 ms 495.71 ms 97.08 ms
Size 7.15 MiB 8.35 MiB 1.20 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
ac41368+dirty 395.91 ms 451.17 ms 55.26 ms
4297324+dirty 385.33 ms 435.68 ms 50.35 ms
d8668ce+dirty 372.43 ms 403.84 ms 31.41 ms
e2b64fe+dirty 258.82 ms 304.26 ms 45.44 ms
5446992+dirty 371.61 ms 390.00 ms 18.39 ms
a989877+dirty 383.04 ms 400.92 ms 17.88 ms
9cab16b+dirty 370.82 ms 416.37 ms 45.55 ms
4a6664f+dirty 357.02 ms 394.91 ms 37.89 ms
e5bc97b+dirty 409.10 ms 471.61 ms 62.51 ms
700cbf4+dirty 411.71 ms 485.52 ms 73.81 ms

App size

Revision Plain With Sentry Diff
ac41368+dirty 7.15 MiB 8.39 MiB 1.24 MiB
4297324+dirty 7.15 MiB 8.35 MiB 1.20 MiB
d8668ce+dirty 7.15 MiB 8.35 MiB 1.20 MiB
e2b64fe+dirty 7.15 MiB 8.07 MiB 947.16 KiB
5446992+dirty 7.15 MiB 8.12 MiB 999.45 KiB
a989877+dirty 7.15 MiB 8.35 MiB 1.20 MiB
9cab16b+dirty 7.15 MiB 8.35 MiB 1.20 MiB
4a6664f+dirty 7.15 MiB 8.22 MiB 1.07 MiB
e5bc97b+dirty 7.15 MiB 8.35 MiB 1.20 MiB
700cbf4+dirty 7.15 MiB 8.34 MiB 1.19 MiB

Previous results on branch: antonis/4169-cocoa-DebugImagesFromCache

Startup times

Revision Plain With Sentry Diff
ff294e7+dirty 454.54 ms 535.50 ms 80.96 ms
2172ba1+dirty 371.50 ms 418.29 ms 46.79 ms
16c15ec+dirty 396.57 ms 445.69 ms 49.13 ms
addbb57+dirty 397.27 ms 465.31 ms 68.04 ms

App size

Revision Plain With Sentry Diff
ff294e7+dirty 7.15 MiB 8.35 MiB 1.20 MiB
2172ba1+dirty 7.15 MiB 8.35 MiB 1.20 MiB
16c15ec+dirty 7.15 MiB 8.35 MiB 1.20 MiB
addbb57+dirty 7.15 MiB 8.35 MiB 1.20 MiB

packages/core/ios/RNSentry.h Outdated Show resolved Hide resolved
@antonis antonis marked this pull request as ready for review October 29, 2024 09:49
@lucas-zimerman
Copy link
Collaborator

lucas-zimerman commented Oct 30, 2024

Does this PR depend on the new cocoa release before being merge? if so I would add an label blocked on it.
Otherwise, the PR looks good to me!

@antonis
Copy link
Collaborator Author

antonis commented Oct 30, 2024

Thank you for reviewing @lucas-zimerman 🙇

Does this PR depend on the new cocoa release before being merge?

At the current state the PR does not depend on a future Cocoa release. I've added a note though to remove the private method declaration when the new hybrid sdk header get's released in Cocoa. I think this can be done in another PR when Cocoa 8.40 is out.

@antonis
Copy link
Collaborator Author

antonis commented Nov 7, 2024

At the current state the PR does not depend on a future Cocoa release. I've #4194 (comment) to remove the private method declaration when the new hybrid sdk header get's released in Cocoa. I think this can be done in another PR when Cocoa 8.40 is out.

With Cocoa 8.40.0 merged I removed the private function declaration (48fdebd).
Ready for another pass.

Copy link
Member

@krystofwoldrich krystofwoldrich left a comment

Choose a reason for hiding this comment

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

LGTM 🚀 Thank you

@antonis antonis merged commit cdf2f33 into main Nov 7, 2024
61 checks passed
@antonis antonis deleted the antonis/4169-cocoa-DebugImagesFromCache branch November 7, 2024 11:05
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.

Use cached methods SentryDebugImageProvider of the Cocoa SDK
3 participants