-
-
Notifications
You must be signed in to change notification settings - Fork 337
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(profiling): Add Hermes JS Profiling (experimental) #3057
Conversation
|
iOS (legacy) Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
d197b5c+dirty | 1217.61 ms | 1242.66 ms | 25.05 ms |
e73f4ed+dirty | 1243.27 ms | 1244.52 ms | 1.25 ms |
e2b64fe+dirty | 1232.22 ms | 1255.20 ms | 22.98 ms |
8900e1a+dirty | 1210.27 ms | 1218.66 ms | 8.39 ms |
9a3ca65+dirty | 1247.06 ms | 1274.58 ms | 27.52 ms |
34aba08+dirty | 1276.78 ms | 1308.52 ms | 31.74 ms |
52a8031+dirty | 1280.88 ms | 1289.78 ms | 8.90 ms |
15c80ab+dirty | 1223.74 ms | 1228.96 ms | 5.22 ms |
70caa60+dirty | 1218.27 ms | 1230.30 ms | 12.03 ms |
86d6d2c+dirty | 1267.55 ms | 1286.21 ms | 18.66 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
d197b5c+dirty | 2.36 MiB | 2.82 MiB | 462.86 KiB |
e73f4ed+dirty | 2.36 MiB | 2.82 MiB | 469.44 KiB |
e2b64fe+dirty | 2.36 MiB | 2.85 MiB | 495.80 KiB |
8900e1a+dirty | 2.36 MiB | 2.83 MiB | 479.25 KiB |
9a3ca65+dirty | 2.36 MiB | 2.82 MiB | 462.89 KiB |
34aba08+dirty | 2.36 MiB | 2.85 MiB | 495.32 KiB |
52a8031+dirty | 2.36 MiB | 2.82 MiB | 469.44 KiB |
15c80ab+dirty | 2.36 MiB | 2.83 MiB | 474.49 KiB |
70caa60+dirty | 2.36 MiB | 2.83 MiB | 479.27 KiB |
86d6d2c+dirty | 2.36 MiB | 2.82 MiB | 462.82 KiB |
iOS (new) Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
d197b5c+dirty | 1234.80 ms | 1249.20 ms | 14.40 ms |
e73f4ed+dirty | 1282.90 ms | 1309.30 ms | 26.40 ms |
e2b64fe+dirty | 1285.78 ms | 1297.56 ms | 11.78 ms |
8900e1a+dirty | 1268.36 ms | 1273.04 ms | 4.68 ms |
9a3ca65+dirty | 1276.40 ms | 1279.14 ms | 2.74 ms |
34aba08+dirty | 1268.58 ms | 1276.80 ms | 8.22 ms |
52a8031+dirty | 1255.96 ms | 1273.00 ms | 17.04 ms |
15c80ab+dirty | 1248.41 ms | 1251.24 ms | 2.83 ms |
70caa60+dirty | 1279.08 ms | 1281.54 ms | 2.46 ms |
86d6d2c+dirty | 1291.62 ms | 1296.80 ms | 5.18 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
d197b5c+dirty | 2.92 MiB | 3.37 MiB | 464.41 KiB |
e73f4ed+dirty | 2.92 MiB | 3.38 MiB | 475.71 KiB |
e2b64fe+dirty | 2.92 MiB | 3.41 MiB | 499.97 KiB |
8900e1a+dirty | 2.92 MiB | 3.39 MiB | 485.96 KiB |
9a3ca65+dirty | 2.92 MiB | 3.37 MiB | 464.32 KiB |
34aba08+dirty | 2.92 MiB | 3.41 MiB | 499.03 KiB |
52a8031+dirty | 2.92 MiB | 3.38 MiB | 475.71 KiB |
15c80ab+dirty | 2.92 MiB | 3.39 MiB | 481.56 KiB |
70caa60+dirty | 2.92 MiB | 3.39 MiB | 486.04 KiB |
86d6d2c+dirty | 2.92 MiB | 3.37 MiB | 464.31 KiB |
Android (new) Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
d197b5c+dirty | 258.75 ms | 313.61 ms | 54.86 ms |
e73f4ed+dirty | 262.98 ms | 311.02 ms | 48.04 ms |
e2b64fe+dirty | 258.82 ms | 304.26 ms | 45.44 ms |
8900e1a+dirty | 371.40 ms | 377.70 ms | 6.31 ms |
9a3ca65+dirty | 344.96 ms | 358.92 ms | 13.96 ms |
34aba08+dirty | 331.79 ms | 376.69 ms | 44.91 ms |
52a8031+dirty | 330.72 ms | 358.76 ms | 28.03 ms |
15c80ab+dirty | 276.38 ms | 327.54 ms | 51.17 ms |
70caa60+dirty | 308.83 ms | 393.06 ms | 84.23 ms |
86d6d2c+dirty | 267.21 ms | 325.24 ms | 58.04 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
d197b5c+dirty | 7.15 MiB | 8.09 MiB | 962.72 KiB |
e73f4ed+dirty | 7.15 MiB | 8.09 MiB | 965.94 KiB |
e2b64fe+dirty | 7.15 MiB | 8.07 MiB | 947.16 KiB |
8900e1a+dirty | 7.15 MiB | 8.03 MiB | 901.79 KiB |
9a3ca65+dirty | 7.15 MiB | 8.09 MiB | 962.83 KiB |
34aba08+dirty | 7.15 MiB | 8.07 MiB | 946.13 KiB |
52a8031+dirty | 7.15 MiB | 8.09 MiB | 965.95 KiB |
15c80ab+dirty | 7.15 MiB | 8.09 MiB | 966.13 KiB |
70caa60+dirty | 7.15 MiB | 8.03 MiB | 901.79 KiB |
86d6d2c+dirty | 7.15 MiB | 8.09 MiB | 962.69 KiB |
Android (legacy) Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
d197b5c+dirty | 338.94 ms | 354.87 ms | 15.93 ms |
e73f4ed+dirty | 332.96 ms | 354.33 ms | 21.37 ms |
34aba08 | 328.10 ms | 342.84 ms | 14.74 ms |
8900e1a+dirty | 430.68 ms | 456.13 ms | 25.44 ms |
3853f43 | 329.68 ms | 346.32 ms | 16.64 ms |
0db0c72 | 372.12 ms | 386.00 ms | 13.88 ms |
9a3ca65+dirty | 326.93 ms | 330.14 ms | 3.21 ms |
52a8031+dirty | 311.55 ms | 321.37 ms | 9.82 ms |
15c80ab+dirty | 336.27 ms | 350.58 ms | 14.31 ms |
70caa60+dirty | 299.00 ms | 321.02 ms | 22.02 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
d197b5c+dirty | 17.73 MiB | 20.04 MiB | 2.31 MiB |
e73f4ed+dirty | 17.73 MiB | 20.04 MiB | 2.31 MiB |
34aba08 | 17.73 MiB | 19.80 MiB | 2.07 MiB |
8900e1a+dirty | 17.73 MiB | 19.75 MiB | 2.01 MiB |
3853f43 | 17.73 MiB | 19.81 MiB | 2.08 MiB |
0db0c72 | 17.73 MiB | 19.75 MiB | 2.02 MiB |
9a3ca65+dirty | 17.73 MiB | 20.04 MiB | 2.31 MiB |
52a8031+dirty | 17.73 MiB | 20.04 MiB | 2.31 MiB |
15c80ab+dirty | 17.73 MiB | 20.04 MiB | 2.31 MiB |
70caa60+dirty | 17.73 MiB | 19.75 MiB | 2.01 MiB |
...le-new-architecture/android/app/src/main/java/com/samplenewarchitecture/MainApplication.java
Outdated
Show resolved
Hide resolved
...le-new-architecture/android/app/src/main/java/com/samplenewarchitecture/MainApplication.java
Outdated
Show resolved
Hide resolved
sample-new-architecture/ios/sampleNewArchitecture.xcodeproj/project.pbxproj
Outdated
Show resolved
Hide resolved
@krystofwoldrich could some of the TS types live in/share with the JS SDK? it seels like some of them are duplicate/can be reused. |
@marandaneto @JonasBa I've addressed the comments. The PR is ready for a final review. |
@krystofwoldrich the PR looks good to me, only CI seems to be failing. I'l defer for the final approval to @marandaneto for the more native parts |
@JonasBa Thanks. The CI just timed out, re-run should fix 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.
Left 2 more comments otherwise LGTM.
Sentry backend stopped accepting Profile and Transaction in separate envelopes on 16th June. Because of the current implementation of sending envelopes on Android, we can send Transaction and Profile in one envelope. Android opens the envelope and repacks the envelope items. This will be solved by the new way of enriching events as then Android will blindly send the envelope without unpacking it. |
📢 Type of change
📜 Description
💡 Motivation and Context
Related to: #2668
Doesn't close the issue as this is only the first step. Native profiling will follow.
💚 How did you test it?
📝 Checklist
sendDefaultPII
is enabled🔮 Next steps