-
-
Notifications
You must be signed in to change notification settings - Fork 341
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): new launch profiling option #4981
base: armcknight/git-chain/profiling/new-continuous-apis/0-topic-branch
Are you sure you want to change the base?
Conversation
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## armcknight/git-chain/profiling/new-continuous-apis/0-topic-branch #4981 +/- ##
=======================================================================================================
- Coverage 92.743% 92.731% -0.012%
=======================================================================================================
Files 673 674 +1
Lines 82805 83510 +705
Branches 30122 30434 +312
=======================================================================================================
+ Hits 76796 77440 +644
- Misses 5910 5973 +63
+ Partials 99 97 -2
... and 8 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
11c45fa | 1232.76 ms | 1252.08 ms | 19.32 ms |
9667f56 | 1219.00 ms | 1235.13 ms | 16.13 ms |
781bb09 | 1230.79 ms | 1250.00 ms | 19.21 ms |
28931ab | 1227.59 ms | 1245.56 ms | 17.97 ms |
b87a934 | 1208.32 ms | 1237.02 ms | 28.70 ms |
b81c36b | 1229.65 ms | 1252.16 ms | 22.51 ms |
346d3a3 | 1215.51 ms | 1250.65 ms | 35.14 ms |
28250fc | 1229.90 ms | 1249.08 ms | 19.18 ms |
99dbffb | 1229.43 ms | 1250.16 ms | 20.73 ms |
d9789ac | 1224.06 ms | 1248.47 ms | 24.41 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
11c45fa | 22.30 KiB | 839.07 KiB | 816.77 KiB |
9667f56 | 22.30 KiB | 829.36 KiB | 807.06 KiB |
781bb09 | 22.30 KiB | 833.45 KiB | 811.14 KiB |
28931ab | 22.30 KiB | 840.17 KiB | 817.86 KiB |
b87a934 | 22.30 KiB | 840.16 KiB | 817.85 KiB |
b81c36b | 22.30 KiB | 823.44 KiB | 801.13 KiB |
346d3a3 | 22.30 KiB | 833.56 KiB | 811.26 KiB |
28250fc | 22.30 KiB | 829.69 KiB | 807.39 KiB |
99dbffb | 22.30 KiB | 840.17 KiB | 817.86 KiB |
d9789ac | 22.30 KiB | 823.68 KiB | 801.38 KiB |
Previous results on branch: armcknight/git-chain/profiling/new-continuous-apis/6-launch-profiling
Startup times
Revision | Plain | With Sentry | Diff |
---|---|---|---|
9839a68 | 1228.24 ms | 1247.98 ms | 19.73 ms |
cf7f7c2 | 1215.34 ms | 1239.89 ms | 24.55 ms |
2fd0f0e | 1225.94 ms | 1247.47 ms | 21.53 ms |
0a0a2bf | 1213.19 ms | 1239.20 ms | 26.01 ms |
284b1c6 | 1245.83 ms | 1264.37 ms | 18.53 ms |
4ab7fff | 1200.92 ms | 1220.73 ms | 19.82 ms |
4f73298 | 1211.70 ms | 1234.02 ms | 22.32 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
9839a68 | 22.30 KiB | 843.83 KiB | 821.52 KiB |
cf7f7c2 | 22.30 KiB | 842.86 KiB | 820.56 KiB |
2fd0f0e | 22.30 KiB | 842.05 KiB | 819.75 KiB |
0a0a2bf | 22.30 KiB | 843.82 KiB | 821.52 KiB |
284b1c6 | 22.30 KiB | 843.83 KiB | 821.52 KiB |
4ab7fff | 22.30 KiB | 843.09 KiB | 820.78 KiB |
4f73298 | 22.30 KiB | 843.23 KiB | 820.92 KiB |
12b6b16
to
c3c3cfd
Compare
…naging a continuous launch profile v2
7b6e093
to
a65fcde
Compare
…ns for supported platforms
…ple-rate-on-fg' into armcknight/profiling/new-continuous-apis/fix-tests
…o armcknight/git-chain/profiling/new-continuous-apis/6-launch-profiling
…s-apis/0-topic-branch
…pic-branch' into armcknight/git-chain/profiling/new-continuous-apis/6-launch-profiling
…pic-branch' into armcknight/git-chain/profiling/new-continuous-apis/6-launch-profiling
🚨 Detected changes in high risk code 🚨High-risk code can easily blow up and is hard to test. We had severe bugs in the past. Be extra careful when changing these files, and have an extra careful look at these:
|
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 some comments, and also it seems like compilation fails for some platform, but I couldn't directly comment it. Please see the test annotations
🚨 Detected changes in high risk code 🚨High-risk code can easily blow up and is hard to test. We had severe bugs in the past. Be extra careful when changing these files, and have an extra careful look at these:
|
…e out how to deal with the deprecated usage in SentryEnabledFeaturesBuilder" This reverts commit dc0686f.
🚨 Detected changes in high risk code 🚨High-risk code can easily blow up and is hard to test. We had severe bugs in the past. Be extra careful when changing these files, and have an extra careful look at these:
|
thanks @philprime for the review! addressed all your comments. as for the CI issues, i just merged in #5008 which fixed the remaining issue for it. |
…after visionOS becamse a thing
🚨 Detected changes in high risk code 🚨High-risk code can easily blow up and is hard to test. We had severe bugs in the past. Be extra careful when changing these files, and have an extra careful look at these:
|
Introduce the new
SentryProfileOptions.profileAppStarts
option.I realized I would need to be able to distinguish between continuous profiling V1 with launch profiling enabled and this V2 launch profiling, so I had to rework the way
SentryProfileOptions
are configured in SentryOptions, so I used a configuration block pattern similar to user feedback and replay, so that I could look for the presence of the configuration by nilchecking it.Note that while the diff size on this is large, almost all of it is test code.
#skip-changelog; for #4853