ref: Reduce complexity for SentryDependencyContainer #3294
Conversation
| @@ -44,24 +44,30 @@ + (void)initialize | |||
| { | |||
| if (self == [SentryDependencyContainer class]) { | |||
| sentryDependencyContainerLock = [[NSObject alloc] init]; | |||
| instance = [[SentryDependencyContainer alloc] init]; | |||
There was a problem hiding this comment.
The next step would be to initialize the instance when SentrySDK is started. This approach allows us to pass the options and eliminate any dependencies on SentryDependencyContainer from both the hub and client, resolving the unusual cyclic dependency.
There was a problem hiding this comment.
Yes, I agree with that approach 👍🏻
| _dispatchQueueWrapper = [[SentryDispatchQueueWrapper alloc] init]; | ||
| _random = [[SentryRandom alloc] init]; | ||
| _threadWrapper = [[SentryThreadWrapper alloc] init]; | ||
| _binaryImageCache = [[SentryBinaryImageCache alloc] init]; | ||
| _debugImageProvider = [[SentryDebugImageProvider alloc] init]; | ||
| _dateProvider = [[SentryCurrentDateProvider alloc] init]; |
There was a problem hiding this comment.
These objects are either always required or too small to justify having a "double check/lock" path for them.
There was a problem hiding this comment.
Thanks for the improvement, @brustolin.
|
Hey @armcknight. I will leave it as draft so we can discuss about it. |
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 881a955 | 1230.98 ms | 1246.22 ms | 15.24 ms |
| 3e7aa41 | 1214.02 ms | 1238.84 ms | 24.82 ms |
| 98752f3 | 1240.61 ms | 1259.80 ms | 19.18 ms |
| 289c0b8 | 1245.63 ms | 1253.76 ms | 8.13 ms |
| b066509 | 1246.14 ms | 1272.42 ms | 26.28 ms |
| fdfe96b | 1227.90 ms | 1242.56 ms | 14.66 ms |
| dbc67d2 | 1239.49 ms | 1248.88 ms | 9.39 ms |
| 4259afd | 1222.12 ms | 1249.74 ms | 27.62 ms |
| dacf894 | 1223.96 ms | 1250.41 ms | 26.45 ms |
| 24b4df1 | 1240.98 ms | 1253.98 ms | 13.00 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 881a955 | 22.85 KiB | 407.63 KiB | 384.79 KiB |
| 3e7aa41 | 20.76 KiB | 432.31 KiB | 411.55 KiB |
| 98752f3 | 20.76 KiB | 435.09 KiB | 414.33 KiB |
| 289c0b8 | 22.85 KiB | 407.67 KiB | 384.82 KiB |
| b066509 | 22.84 KiB | 403.13 KiB | 380.29 KiB |
| fdfe96b | 20.76 KiB | 419.70 KiB | 398.95 KiB |
| dbc67d2 | 20.76 KiB | 427.74 KiB | 406.98 KiB |
| 4259afd | 20.76 KiB | 419.70 KiB | 398.94 KiB |
| dacf894 | 20.76 KiB | 426.34 KiB | 405.58 KiB |
| 24b4df1 | 20.76 KiB | 425.77 KiB | 405.01 KiB |
|
I believe we should iterate over this ideas slowly. I suggest that we merge this first changes, it will reduce complexity on some main objects, and later we improve to initialize all objects during |
Making
SentryDependencyContainerless complex and more predictable by removing some unnecessary lazy loads.The goal would be to have all necessary objects initialized according to SentryOptions and remove all the @syncronized locks.
#skip-changelog