From 68b7c0c94f415e9193c3b425b796c3dbc1eee493 Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Thu, 22 May 2025 16:07:36 +0300 Subject: [PATCH 1/5] Use low priority queue for dispatch queue wrapper --- Sources/Sentry/SentryDispatchFactory.m | 6 +++--- Sources/Sentry/SentrySessionReplayIntegration.m | 8 ++++---- .../include/SentryDispatchQueueProviderProtocol.h | 6 +++--- .../SentrySessionReplayIntegrationTests.swift | 4 ++-- .../Networking/SentryDispatchFactoryTests.m | 10 +++++----- .../Networking/SentryDispatchQueueWrapperTests.m | 6 +++--- 6 files changed, 20 insertions(+), 20 deletions(-) diff --git a/Sources/Sentry/SentryDispatchFactory.m b/Sources/Sentry/SentryDispatchFactory.m index 7ec41369f8e..8d49f16d55a 100644 --- a/Sources/Sentry/SentryDispatchFactory.m +++ b/Sources/Sentry/SentryDispatchFactory.m @@ -11,13 +11,13 @@ - (SentryDispatchQueueWrapper *)queueWithName:(const char *)name return [[SentryDispatchQueueWrapper alloc] initWithName:name attributes:attributes]; } -- (SentryDispatchQueueWrapper *)createBackgroundQueueWithName:(const char *)name - relativePriority:(int)relativePriority +- (SentryDispatchQueueWrapper *)createLowPriorityQueue:(const char *)name + relativePriority:(int)relativePriority { SENTRY_CASSERT(relativePriority <= 0 && relativePriority >= QOS_MIN_RELATIVE_PRIORITY, @"Relative priority must be between 0 and %d", QOS_MIN_RELATIVE_PRIORITY); dispatch_queue_attr_t attributes = dispatch_queue_attr_make_with_qos_class( - DISPATCH_QUEUE_SERIAL, QOS_CLASS_BACKGROUND, relativePriority); + DISPATCH_QUEUE_SERIAL, QOS_CLASS_UTILITY, relativePriority); return [[SentryDispatchQueueWrapper alloc] initWithName:name attributes:attributes]; } diff --git a/Sources/Sentry/SentrySessionReplayIntegration.m b/Sources/Sentry/SentrySessionReplayIntegration.m index df67ad4cd4c..c8babe8dace 100644 --- a/Sources/Sentry/SentrySessionReplayIntegration.m +++ b/Sources/Sentry/SentrySessionReplayIntegration.m @@ -143,14 +143,14 @@ - (void)setupWith:(SentryReplayOptions *)replayOptions // The asset worker queue is used to work on video and frames data. // Use a relative priority of -1 to make it lower than the default background priority. _replayAssetWorkerQueue = - [dispatchQueueProvider createBackgroundQueueWithName:"io.sentry.session-replay.asset-worker" - relativePriority:-1]; + [dispatchQueueProvider createLowPriorityQueue:"io.sentry.session-replay.asset-worker" + relativePriority:-1]; // The dispatch queue is used to asynchronously wait for the asset worker queue to finish its // work. To avoid a deadlock, the priority of the processing queue must be lower than the asset // worker queue. Use a relative priority of -2 to make it lower than the asset worker queue. _replayProcessingQueue = - [dispatchQueueProvider createBackgroundQueueWithName:"io.sentry.session-replay.processing" - relativePriority:-2]; + [dispatchQueueProvider createLowPriorityQueue:"io.sentry.session-replay.processing" + relativePriority:-2]; // The asset worker queue is used to work on video and frames data. diff --git a/Sources/Sentry/include/SentryDispatchQueueProviderProtocol.h b/Sources/Sentry/include/SentryDispatchQueueProviderProtocol.h index 7b8bf017241..dc6ddfd1ea1 100644 --- a/Sources/Sentry/include/SentryDispatchQueueProviderProtocol.h +++ b/Sources/Sentry/include/SentryDispatchQueueProviderProtocol.h @@ -13,7 +13,7 @@ NS_ASSUME_NONNULL_BEGIN attributes:(dispatch_queue_attr_t)attributes; /** - * Creates a background queue with the given name and relative priority, wrapped in a @c + * Creates a low priority queue with the given name and relative priority, wrapped in a @c * SentryDispatchQueueWrapper. * * @note This method is only a factory method and does not keep a reference to the created queue. @@ -25,8 +25,8 @@ NS_ASSUME_NONNULL_BEGIN * quality-of-service. * @return Unretained reference to the created queue. */ -- (SentryDispatchQueueWrapper *)createBackgroundQueueWithName:(const char *)name - relativePriority:(int)relativePriority; +- (SentryDispatchQueueWrapper *)createLowPriorityQueue:(const char *)name + relativePriority:(int)relativePriority; @end diff --git a/Tests/SentryTests/Integrations/SessionReplay/SentrySessionReplayIntegrationTests.swift b/Tests/SentryTests/Integrations/SessionReplay/SentrySessionReplayIntegrationTests.swift index 89e92f54f4f..bcac44b2ca7 100644 --- a/Tests/SentryTests/Integrations/SessionReplay/SentrySessionReplayIntegrationTests.swift +++ b/Tests/SentryTests/Integrations/SessionReplay/SentrySessionReplayIntegrationTests.swift @@ -650,10 +650,10 @@ class SentrySessionReplayIntegrationTests: XCTestCase { // -- Assert -- XCTAssertEqual(assetWorkerQueue.queue.label, "io.sentry.session-replay.asset-worker") - XCTAssertEqual(assetWorkerQueue.queue.qos.qosClass, .background) + XCTAssertEqual(assetWorkerQueue.queue.qos.qosClass, .utility) XCTAssertEqual(processingQueue.queue.label, "io.sentry.session-replay.processing") - XCTAssertEqual(processingQueue.queue.qos.qosClass, .background) + XCTAssertEqual(processingQueue.queue.qos.qosClass, .utility) // The actual priorities are not relevant, we just need to check that the processing queue has a lower priority // than the asset worker queue and that both are lower than the default priority. diff --git a/Tests/SentryTests/Networking/SentryDispatchFactoryTests.m b/Tests/SentryTests/Networking/SentryDispatchFactoryTests.m index 9f791910c1e..6d7fd7e21bf 100644 --- a/Tests/SentryTests/Networking/SentryDispatchFactoryTests.m +++ b/Tests/SentryTests/Networking/SentryDispatchFactoryTests.m @@ -43,7 +43,7 @@ - (void)testQueueWithNameAndAttributes_shouldReturnQueueWithNameAndAttributesSet } - (void) - testCreateBackgroundQueueWithNameAndRelativePriority_shouldReturnQueueWithNameAndRelativePrioritySet + testCreateLowPriorityQueueWithNameAndRelativePriority_shouldReturnQueueWithNameAndRelativePrioritySet { // Note: We are not testing the functionality of the queue itself, just the creation of it, // making sure the factory sets the name and attributes correctly. @@ -54,7 +54,7 @@ - (void)testQueueWithNameAndAttributes_shouldReturnQueueWithNameAndAttributesSet // -- Act -- SentryDispatchQueueWrapper *wrappedQueue = - [self.sut createBackgroundQueueWithName:queueName relativePriority:relativePriority]; + [self.sut createLowPriorityQueue:queueName relativePriority:relativePriority]; // -- Assert -- const char *actualName = dispatch_queue_get_label(wrappedQueue.queue); @@ -63,7 +63,7 @@ - (void)testQueueWithNameAndAttributes_shouldReturnQueueWithNameAndAttributesSet int actualRelativePriority; dispatch_qos_class_t actualQoSClass = dispatch_queue_get_qos_class(wrappedQueue.queue, &actualRelativePriority); - XCTAssertEqual(actualQoSClass, QOS_CLASS_BACKGROUND); + XCTAssertEqual(actualQoSClass, QOS_CLASS_UTILITY); XCTAssertEqual(actualRelativePriority, relativePriority); } @@ -111,7 +111,7 @@ - (void)testSource_queueUsesCorrectQoSAndPriority uint64_t leeway = 0; const char *queueName = "sentry-dispatch-factory.qos-check"; dispatch_queue_attr_t attributes - = dispatch_queue_attr_make_with_qos_class(DISPATCH_QUEUE_SERIAL, QOS_CLASS_BACKGROUND, -10); + = dispatch_queue_attr_make_with_qos_class(DISPATCH_QUEUE_SERIAL, QOS_CLASS_UTILITY, -10); void (^eventHandler)(void) = ^{ }; // -- Act -- @@ -128,7 +128,7 @@ - (void)testSource_queueUsesCorrectQoSAndPriority int relativePriority; dispatch_qos_class_t qos = dispatch_queue_get_qos_class(queueWrapper.queue, &relativePriority); - XCTAssertEqual(qos, QOS_CLASS_BACKGROUND); + XCTAssertEqual(qos, QOS_CLASS_UTILITY); XCTAssertEqual(relativePriority, -10); } diff --git a/Tests/SentryTests/Networking/SentryDispatchQueueWrapperTests.m b/Tests/SentryTests/Networking/SentryDispatchQueueWrapperTests.m index 085986040a9..6eabbf97c3f 100644 --- a/Tests/SentryTests/Networking/SentryDispatchQueueWrapperTests.m +++ b/Tests/SentryTests/Networking/SentryDispatchQueueWrapperTests.m @@ -29,7 +29,7 @@ - (void)testInitWithNameAndAttributes_customAttributes_shouldCreateQueueWithGive // -- Arrange -- const char *queueName = "sentry-dispatch-factory.test"; int relativePriority = -5; - qos_class_t qosClass = QOS_CLASS_BACKGROUND; + qos_class_t qosClass = QOS_CLASS_UTILITY; dispatch_queue_attr_t _Nullable attr = DISPATCH_QUEUE_SERIAL; dispatch_queue_attr_t _Nullable attributes @@ -43,7 +43,7 @@ - (void)testInitWithNameAndAttributes_customAttributes_shouldCreateQueueWithGive int actualRelativePriority; dispatch_qos_class_t actualQoSClass = dispatch_queue_get_qos_class(wrappedQueue.queue, &actualRelativePriority); - XCTAssertEqual(actualQoSClass, QOS_CLASS_BACKGROUND); + XCTAssertEqual(actualQoSClass, QOS_CLASS_UTILITY); XCTAssertEqual(actualRelativePriority, -5); } @@ -58,7 +58,7 @@ - (void)testInitWithNameAndAttributes_customAttributes_shouldCreateQueueWithGive // -- Arrange -- const char *queueName = "sentry-dispatch-factory.test"; int relativePriority = 5; - qos_class_t qosClass = QOS_CLASS_BACKGROUND; + qos_class_t qosClass = QOS_CLASS_UTILITY; dispatch_queue_attr_t _Nullable attr = DISPATCH_QUEUE_SERIAL; dispatch_queue_attr_t _Nullable attributes From 5b8d09ce08731a1cf26bd599366aea207f33fbf6 Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Thu, 22 May 2025 16:10:51 +0300 Subject: [PATCH 2/5] Fix lint issue --- Tests/SentryTests/Networking/SentryDispatchFactoryTests.m | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Tests/SentryTests/Networking/SentryDispatchFactoryTests.m b/Tests/SentryTests/Networking/SentryDispatchFactoryTests.m index 6d7fd7e21bf..f2ee9f5243b 100644 --- a/Tests/SentryTests/Networking/SentryDispatchFactoryTests.m +++ b/Tests/SentryTests/Networking/SentryDispatchFactoryTests.m @@ -53,8 +53,8 @@ - (void)testQueueWithNameAndAttributes_shouldReturnQueueWithNameAndAttributesSet int relativePriority = -5; // -- Act -- - SentryDispatchQueueWrapper *wrappedQueue = - [self.sut createLowPriorityQueue:queueName relativePriority:relativePriority]; + SentryDispatchQueueWrapper *wrappedQueue = [self.sut createLowPriorityQueue:queueName + relativePriority:relativePriority]; // -- Assert -- const char *actualName = dispatch_queue_get_label(wrappedQueue.queue); From e101b4a8773ab7cf7bd6a7172b202171e9192830 Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Thu, 22 May 2025 16:16:18 +0300 Subject: [PATCH 3/5] Adds changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0430518bd5f..3e40340e56f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Fixes + +- Uses low-priority queues to reduce the chance of session replay internal multi-threading processes being dropped (#5280) + ### Improvements - Threading issues in internal dependency container (#5225) From 3e238f132b87c7c601af340cfb46dffc5efc0bba Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Thu, 22 May 2025 16:40:48 +0300 Subject: [PATCH 4/5] Adds comment to explain why QOS_CLASS_UTILITY is considered a low priority --- Sources/Sentry/SentryDispatchFactory.m | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Sources/Sentry/SentryDispatchFactory.m b/Sources/Sentry/SentryDispatchFactory.m index 8d49f16d55a..a99080e6160 100644 --- a/Sources/Sentry/SentryDispatchFactory.m +++ b/Sources/Sentry/SentryDispatchFactory.m @@ -16,6 +16,8 @@ - (SentryDispatchQueueWrapper *)createLowPriorityQueue:(const char *)name { SENTRY_CASSERT(relativePriority <= 0 && relativePriority >= QOS_MIN_RELATIVE_PRIORITY, @"Relative priority must be between 0 and %d", QOS_MIN_RELATIVE_PRIORITY); + // The QOS_CLASS_UTILITY is defined in `qos.h` and used by the `DISPATCH_QUEUE_PRIORITY_LOW`, + // therefore it can be considered it a low priority queue dispatch_queue_attr_t attributes = dispatch_queue_attr_make_with_qos_class( DISPATCH_QUEUE_SERIAL, QOS_CLASS_UTILITY, relativePriority); return [[SentryDispatchQueueWrapper alloc] initWithName:name attributes:attributes]; From cafb32f897d43c5a87d915d7128eb0b5a8c7589d Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Thu, 22 May 2025 16:50:21 +0300 Subject: [PATCH 5/5] Adds reference in the comment --- Sources/Sentry/SentryDispatchFactory.m | 1 + 1 file changed, 1 insertion(+) diff --git a/Sources/Sentry/SentryDispatchFactory.m b/Sources/Sentry/SentryDispatchFactory.m index a99080e6160..168759397ec 100644 --- a/Sources/Sentry/SentryDispatchFactory.m +++ b/Sources/Sentry/SentryDispatchFactory.m @@ -18,6 +18,7 @@ - (SentryDispatchQueueWrapper *)createLowPriorityQueue:(const char *)name @"Relative priority must be between 0 and %d", QOS_MIN_RELATIVE_PRIORITY); // The QOS_CLASS_UTILITY is defined in `qos.h` and used by the `DISPATCH_QUEUE_PRIORITY_LOW`, // therefore it can be considered it a low priority queue + // Reference: https://developer.apple.com/documentation/dispatch/dispatch_queue_priority_low dispatch_queue_attr_t attributes = dispatch_queue_attr_make_with_qos_class( DISPATCH_QUEUE_SERIAL, QOS_CLASS_UTILITY, relativePriority); return [[SentryDispatchQueueWrapper alloc] initWithName:name attributes:attributes];