-
-
Notifications
You must be signed in to change notification settings - Fork 361
feat(V9): Make performance V2 the default in V9 #6008
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?
Conversation
|
7dc7a8e
to
6a4d52d
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6008 +/- ##
========================================
Coverage ? 86.751%
========================================
Files ? 424
Lines ? 36722
Branches ? 17366
========================================
Hits ? 31857
Misses ? 4820
Partials ? 45
Continue to review full report in Codecov by Sentry.
|
Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
c5bedc0 | 1203.83 ms | 1236.17 ms | 32.33 ms |
aadbffe | 1228.73 ms | 1251.59 ms | 22.86 ms |
2de3f92 | 1207.56 ms | 1234.96 ms | 27.40 ms |
ed85746 | 1231.79 ms | 1248.55 ms | 16.75 ms |
21efa18 | 1227.73 ms | 1250.04 ms | 22.31 ms |
7148f97 | 1235.09 ms | 1258.07 ms | 22.98 ms |
d0f70ce | 1226.54 ms | 1247.04 ms | 20.50 ms |
5bf2b17 | 1213.53 ms | 1238.54 ms | 25.01 ms |
1339919 | 1214.82 ms | 1238.98 ms | 24.16 ms |
2a36c3f | 1227.27 ms | 1246.89 ms | 19.63 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
c5bedc0 | 23.75 KiB | 920.63 KiB | 896.89 KiB |
aadbffe | 23.75 KiB | 912.77 KiB | 889.02 KiB |
2de3f92 | 23.75 KiB | 919.69 KiB | 895.94 KiB |
ed85746 | 23.75 KiB | 920.83 KiB | 897.08 KiB |
21efa18 | 23.75 KiB | 919.70 KiB | 895.95 KiB |
7148f97 | 23.75 KiB | 854.78 KiB | 831.03 KiB |
d0f70ce | 23.75 KiB | 913.09 KiB | 889.34 KiB |
5bf2b17 | 23.75 KiB | 913.27 KiB | 889.52 KiB |
1339919 | 23.75 KiB | 919.70 KiB | 895.95 KiB |
2a36c3f | 23.75 KiB | 874.45 KiB | 850.71 KiB |
@@ -287,6 +287,7 @@ NS_SWIFT_NAME(Options) | |||
*/ | |||
@property (nonatomic, assign) BOOL enableAutoPerformanceTracing; | |||
|
|||
#if !SDK_V9 |
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.
h
: Not sure if I understand this PR's intention. Do you want to make enablePerformanceV2
be enabled by default or do you want to remove the options? If the latter one why would we want to remove the option?
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.
@philprime I assumed if we had a V2 of something we'd eventually want to remove the V1 since V2 is better/newer, but maybe the intent here was to have two definitions of app start, and then I'd recommend having something like an enum for "AppStartType" so it's clear what is changing rather than a v1/v2
As the comment on this property already stated, it should be the default in V9
#skip-changelog