impr: Speed up SentryNetworkTracker (#4366)

Speed up SentryNetworkTracker for multiple HTTP requests in parallel by removing unnecessary locks.
getsentry · Sep 24, 2024 · 3138592 · 3138592
1 parent 2233968
commit 3138592
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 35 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,11 @@
 # Changelog
 
+## Unreleased
+
+### Improvements
+
+- Speed up HTTP tracking for multiple requests in parallel (#4366)
+
 ## 8.37.0-beta.1
 
 ### Features

diff --git a/Sources/Resources/ThreadSanitizer.sup b/Sources/Resources/ThreadSanitizer.sup
@@ -3,7 +3,6 @@
 
 # Races to fix
 race:returnResponse
-race:enableNetworkTracking
 race:enableNetworkBreadcrumbs
 race:disable
 race:URLSessionDataTaskMock

diff --git a/Sources/Sentry/SentryNetworkTracker.m b/Sources/Sentry/SentryNetworkTracker.m
@@ -8,6 +8,7 @@
 #import "SentryHttpStatusCodeRange+Private.h"
 #import "SentryHttpStatusCodeRange.h"
 #import "SentryHub+Private.h"
+#import "SentryInternalCDefines.h"
 #import "SentryLog.h"
 #import "SentryMechanism.h"
 #import "SentryNoOpSpan.h"
@@ -28,6 +29,10 @@
 #import "SentryUser.h"
 #import <objc/runtime.h>
 
+static NSString *const SentryNetworkTrackerThreadSanitizerMessage
+    = @"We accept race conditions for turning flags on and off to avoid acquiring locks, which can "
+      @"significantly slow down many HTTP requests running in parallel.";
+
 /**
  * WARNING: We had issues in the past with this code on older iOS versions. We don't run unit tests
  * on all the iOS versions our SDK supports. When adding this comment on April 12th, 2023, we
@@ -67,42 +72,37 @@ - (instancetype)init
     return self;
 }
 
-- (void)enableNetworkTracking
+- (void)enableNetworkTracking SENTRY_DISABLE_THREAD_SANITIZER(
+    SentryNetworkTrackerThreadSanitizerMessage)
 {
-    @synchronized(self) {
-        _isNetworkTrackingEnabled = YES;
-    }
+    _isNetworkTrackingEnabled = YES;
 }
 
-- (void)enableNetworkBreadcrumbs
+- (void)enableNetworkBreadcrumbs SENTRY_DISABLE_THREAD_SANITIZER(
+    SentryNetworkTrackerThreadSanitizerMessage)
 {
-    @synchronized(self) {
-        _isNetworkBreadcrumbEnabled = YES;
-    }
+    _isNetworkBreadcrumbEnabled = YES;
 }
 
-- (void)enableCaptureFailedRequests
+- (void)enableCaptureFailedRequests SENTRY_DISABLE_THREAD_SANITIZER(
+    SentryNetworkTrackerThreadSanitizerMessage)
 {
-    @synchronized(self) {
-        _isCaptureFailedRequestsEnabled = YES;
-    }
+    _isCaptureFailedRequestsEnabled = YES;
 }
 
-- (void)enableGraphQLOperationTracking
+- (void)enableGraphQLOperationTracking SENTRY_DISABLE_THREAD_SANITIZER(
+    SentryNetworkTrackerThreadSanitizerMessage)
 {
-    @synchronized(self) {
-        _isGraphQLOperationTrackingEnabled = YES;
-    }
+
+    _isGraphQLOperationTrackingEnabled = YES;
 }
 
-- (void)disable
+- (void)disable SENTRY_DISABLE_THREAD_SANITIZER(SentryNetworkTrackerThreadSanitizerMessage)
 {
-    @synchronized(self) {
-        _isNetworkBreadcrumbEnabled = NO;
-        _isNetworkTrackingEnabled = NO;
-        _isCaptureFailedRequestsEnabled = NO;
-        _isGraphQLOperationTrackingEnabled = NO;
-    }
+    _isNetworkBreadcrumbEnabled = NO;
+    _isNetworkTrackingEnabled = NO;
+    _isCaptureFailedRequestsEnabled = NO;
+    _isGraphQLOperationTrackingEnabled = NO;
 }
 
 - (BOOL)isTargetMatch:(NSURL *)URL withTargets:(NSArray *)targets
@@ -168,11 +168,9 @@ - (void)urlSessionTaskResume:(NSURLSessionTask *)sessionTask
             OBJC_ASSOCIATION_RETAIN_NONATOMIC);
     }
 
-    @synchronized(self) {
-        if (!self.isNetworkTrackingEnabled) {
-            [self addTraceWithoutTransactionToTask:sessionTask];
-            return;
-        }
+    if (!self.isNetworkTrackingEnabled) {
+        [self addTraceWithoutTransactionToTask:sessionTask];
+        return;
     }
 
     UrlSanitized *safeUrl = [[UrlSanitized alloc] initWithURL:url];
@@ -365,12 +363,10 @@ - (void)urlSessionTask:(NSURLSessionTask *)sessionTask setState:(NSURLSessionTas
 
 - (void)captureFailedRequests:(NSURLSessionTask *)sessionTask
 {
-    @synchronized(self) {
-        if (!self.isCaptureFailedRequestsEnabled) {
-            SENTRY_LOG_DEBUG(
-                @"captureFailedRequestsEnabled is disabled, not capturing HTTP Client errors.");
-            return;
-        }
+    if (!self.isCaptureFailedRequestsEnabled) {
+        SENTRY_LOG_DEBUG(
+            @"captureFailedRequestsEnabled is disabled, not capturing HTTP Client errors.");
+        return;
     }
 
     // if request or response are null, we can't raise the event