Skip to content

Commit

Permalink
ref: Improve frames tracker performance (#4469)
Browse files Browse the repository at this point in the history
Improves the speed of the frames tracker by not locking it anymore since it always run in the main thread.
  • Loading branch information
brustolin authored Oct 28, 2024
1 parent 4569cc9 commit d802ba2
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 14 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

### Improvements

- Improve frames tracker performance (#4469)
- Log a warning when dropping envelopes due to rate-limiting (#4463)

## 8.39.0-beta.1
Expand Down
23 changes: 9 additions & 14 deletions Sources/Sentry/SentryFramesTracker.m
Original file line number Diff line number Diff line change
Expand Up @@ -256,14 +256,11 @@ - (void)displayLinkCallback

- (void)reportNewFrame
{
NSArray *localListeners;
@synchronized(self.listeners) {
localListeners = [self.listeners allObjects];
}

NSDate *newFrameDate = [self.dateProvider date];

for (id<SentryFramesTrackerListener> listener in localListeners) {
// We need to copy the list because some listeners will remove themselves
// from the list during the callback, causing a crash during iteration.
NSArray<id<SentryFramesTrackerListener>> *listeners = [self.listeners copy];
for (id<SentryFramesTrackerListener> listener in listeners) {
[listener framesTrackerHasNewFrame:newFrameDate];
}
}
Expand Down Expand Up @@ -310,17 +307,15 @@ - (SentryFramesDelayResult *)getFramesDelay:(uint64_t)startSystemTimestamp

- (void)addListener:(id<SentryFramesTrackerListener>)listener
{

@synchronized(self.listeners) {
[self.listeners addObject:listener];
}
// Adding listeners on the main thread to avoid race condition with new frame callback
[self.dispatchQueueWrapper dispatchAsyncOnMainQueue:^{ [self.listeners addObject:listener]; }];
}

- (void)removeListener:(id<SentryFramesTrackerListener>)listener
{
@synchronized(self.listeners) {
[self.listeners removeObject:listener];
}
// Removing listeners on the main thread to avoid race condition with new frame callback
[self.dispatchQueueWrapper
dispatchAsyncOnMainQueue:^{ [self.listeners removeObject:listener]; }];
}

- (void)pause
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ class SentryFramesTrackerTests: XCTestCase {
}

lazy var sut: SentryFramesTracker = SentryFramesTracker(displayLinkWrapper: displayLinkWrapper, dateProvider: dateProvider, dispatchQueueWrapper: SentryDispatchQueueWrapper(), notificationCenter: notificationCenter, keepDelayedFramesDuration: keepDelayedFramesDuration)

}

private var fixture: Fixture!
Expand Down Expand Up @@ -602,6 +603,16 @@ class SentryFramesTrackerTests: XCTestCase {
XCTAssertEqual(listener2.newFrameInvocations.count, 1)
XCTAssertEqual(listener2.newFrameInvocations.first?.timeIntervalSince1970, expectedFrameDate.timeIntervalSince1970)
}

func testListenerAreAddedInMainThread() {
let dispatchQueueWrapper = TestSentryDispatchQueueWrapper()
let sut = SentryFramesTracker(displayLinkWrapper: fixture.displayLinkWrapper, dateProvider: fixture.dateProvider, dispatchQueueWrapper: dispatchQueueWrapper, notificationCenter: fixture.notificationCenter, keepDelayedFramesDuration: fixture.keepDelayedFramesDuration)
let listener = FrameTrackerListener()

sut.add(listener)

XCTAssertEqual(dispatchQueueWrapper.blockOnMainInvocations.count, 1)
}

func testRemoveListener() {
let sut = fixture.sut
Expand Down

0 comments on commit d802ba2

Please sign in to comment.