Skip to content

Commit

Permalink
ref: Add FramesDelayResult (#4279)
Browse files Browse the repository at this point in the history
Add SentryFramesDelayResult containing framesContributingToDelayCount,
which is the count for the frames that contributed to the frames delay
count. This is required for GH-3492.
  • Loading branch information
philipphofmann committed Aug 14, 2024
1 parent a4ffa76 commit df2835d
Show file tree
Hide file tree
Showing 9 changed files with 78 additions and 40 deletions.
12 changes: 12 additions & 0 deletions Sentry.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@
62B0C30D2BA9D39600648D59 /* CounterMetricTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 62B0C30C2BA9D39600648D59 /* CounterMetricTests.swift */; };
62B0C30F2BA9D74800648D59 /* DistributionMetricTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 62B0C30E2BA9D74800648D59 /* DistributionMetricTests.swift */; };
62B0C3112BA9D85C00648D59 /* SetMetricTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 62B0C3102BA9D85C00648D59 /* SetMetricTests.swift */; };
62B558B02C6B9C3C00C34FEC /* SentryFramesDelayResult.swift in Sources */ = {isa = PBXBuildFile; fileRef = 62B558AF2C6B9C3C00C34FEC /* SentryFramesDelayResult.swift */; };
62B86CFC29F052BB008F3947 /* SentryTestLogConfig.m in Sources */ = {isa = PBXBuildFile; fileRef = 62B86CFB29F052BB008F3947 /* SentryTestLogConfig.m */; };
62BAD74E2BA1C58D00EBAAFC /* EncodeMetricTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 62262B952BA1C564004DA3DD /* EncodeMetricTests.swift */; };
62BAD7502BA1C5AF00EBAAFC /* SentryMetricsClientTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 62BAD74F2BA1C5AF00EBAAFC /* SentryMetricsClientTests.swift */; };
Expand Down Expand Up @@ -1106,6 +1107,7 @@
62B0C30C2BA9D39600648D59 /* CounterMetricTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CounterMetricTests.swift; sourceTree = "<group>"; };
62B0C30E2BA9D74800648D59 /* DistributionMetricTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DistributionMetricTests.swift; sourceTree = "<group>"; };
62B0C3102BA9D85C00648D59 /* SetMetricTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SetMetricTests.swift; sourceTree = "<group>"; };
62B558AF2C6B9C3C00C34FEC /* SentryFramesDelayResult.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryFramesDelayResult.swift; sourceTree = "<group>"; };
62B86CFB29F052BB008F3947 /* SentryTestLogConfig.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = SentryTestLogConfig.m; sourceTree = "<group>"; };
62BAD74F2BA1C5AF00EBAAFC /* SentryMetricsClientTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryMetricsClientTests.swift; sourceTree = "<group>"; };
62BAD7552BA202C300EBAAFC /* SentryMetricsClient.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryMetricsClient.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -2152,6 +2154,14 @@
path = Extensions;
sourceTree = "<group>";
};
62B558AE2C6B9C3000C34FEC /* FramesTracking */ = {
isa = PBXGroup;
children = (
62B558AF2C6B9C3C00C34FEC /* SentryFramesDelayResult.swift */,
);
path = FramesTracking;
sourceTree = "<group>";
};
630436001EBCB87500C4D3FA /* Networking */ = {
isa = PBXGroup;
children = (
Expand Down Expand Up @@ -3883,6 +3893,7 @@
D8CAC02D2BA0663E00E38F34 /* Integrations */ = {
isa = PBXGroup;
children = (
62B558AE2C6B9C3000C34FEC /* FramesTracking */,
D8739CF72BECFF92007D2F66 /* Performance */,
D8CAC02C2BA0663E00E38F34 /* SessionReplay */,
);
Expand Down Expand Up @@ -4717,6 +4728,7 @@
9286059729A5098900F96038 /* SentryGeo.m in Sources */,
7B42C48227E08F4B009B58C2 /* SentryDependencyContainer.m in Sources */,
639FCFAD1EBC811400778193 /* SentryUser.m in Sources */,
62B558B02C6B9C3C00C34FEC /* SentryFramesDelayResult.swift in Sources */,
7DAC589123D8B2E0001CF26B /* SentryGlobalEventProcessor.m in Sources */,
7BBD189E244EC8D200427C76 /* SentryRetryAfterHeaderParser.m in Sources */,
63FE711920DA4C1000CDBAE8 /* SentryCrashMachineContext.c in Sources */,
Expand Down
18 changes: 12 additions & 6 deletions Sources/Sentry/SentryDelayedFramesTracker.m
Original file line number Diff line number Diff line change
Expand Up @@ -112,12 +112,13 @@ - (void)removeOldDelayedFrames
[self.delayedFrames removeObjectsInRange:NSMakeRange(0, left)];
}

- (CFTimeInterval)getFramesDelay:(uint64_t)startSystemTimestamp
endSystemTimestamp:(uint64_t)endSystemTimestamp
isRunning:(BOOL)isRunning
slowFrameThreshold:(CFTimeInterval)slowFrameThreshold
- (SentryFramesDelayResult *)getFramesDelay:(uint64_t)startSystemTimestamp
endSystemTimestamp:(uint64_t)endSystemTimestamp
isRunning:(BOOL)isRunning
slowFrameThreshold:(CFTimeInterval)slowFrameThreshold
{
CFTimeInterval cantCalculateFrameDelayReturnValue = -1.0;
SentryFramesDelayResult *cantCalculateFrameDelayReturnValue =
[[SentryFramesDelayResult alloc] initWithDelayDuration:-1.0 framesCount:0];

if (isRunning == NO) {
SENTRY_LOG_DEBUG(@"Not calculating frames delay because frames tracker isn't running.");
Expand Down Expand Up @@ -189,6 +190,7 @@ - (CFTimeInterval)getFramesDelay:(uint64_t)startSystemTimestamp
endDate:endDate];

CFTimeInterval delay = 0.0;
NSUInteger framesCount = 0;

// Iterate in reverse order, as younger frame delays are more likely to match the queried
// period.
Expand All @@ -201,9 +203,13 @@ - (CFTimeInterval)getFramesDelay:(uint64_t)startSystemTimestamp
}

delay = delay + [self calculateDelay:frame queryDateInterval:queryDateInterval];
framesCount++;
}

return delay;
SentryFramesDelayResult *data =
[[SentryFramesDelayResult alloc] initWithDelayDuration:delay framesCount:framesCount];

return data;
}

- (CFTimeInterval)calculateDelay:(SentryDelayedFrame *)delayedFrame
Expand Down
5 changes: 3 additions & 2 deletions Sources/Sentry/SentryFramesTracker.m
Original file line number Diff line number Diff line change
Expand Up @@ -299,8 +299,9 @@ - (SentryScreenFrames *)currentFrames SENTRY_DISABLE_THREAD_SANITIZER()
# endif // SENTRY_TARGET_PROFILING_SUPPORTED
}

- (CFTimeInterval)getFramesDelay:(uint64_t)startSystemTimestamp
endSystemTimestamp:(uint64_t)endSystemTimestamp SENTRY_DISABLE_THREAD_SANITIZER()
- (SentryFramesDelayResult *)getFramesDelay:(uint64_t)startSystemTimestamp
endSystemTimestamp:(uint64_t)endSystemTimestamp
SENTRY_DISABLE_THREAD_SANITIZER()
{
return [self.delayedFramesTracker getFramesDelay:startSystemTimestamp
endSystemTimestamp:endSystemTimestamp
Expand Down
3 changes: 2 additions & 1 deletion Sources/Sentry/SentrySpan.m
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,8 @@ - (void)finishWithStatus:(SentrySpanStatus)status

CFTimeInterval framesDelay = [_framesTracker
getFramesDelay:_startSystemTime
endSystemTimestamp:SentryDependencyContainer.sharedInstance.dateProvider.systemTime];
endSystemTimestamp:SentryDependencyContainer.sharedInstance.dateProvider.systemTime]
.delayDuration;

if (framesDelay >= 0) {
[self setDataValue:@(framesDelay) forKey:@"frames.delay"];
Expand Down
3 changes: 2 additions & 1 deletion Sources/Sentry/SentryTracer.m
Original file line number Diff line number Diff line change
Expand Up @@ -845,7 +845,8 @@ - (void)addFrameStatistics
if (framesTracker.isRunning) {
CFTimeInterval framesDelay = [framesTracker
getFramesDelay:self.startSystemTime
endSystemTimestamp:SentryDependencyContainer.sharedInstance.dateProvider.systemTime];
endSystemTimestamp:SentryDependencyContainer.sharedInstance.dateProvider.systemTime]
.delayDuration;

if (framesDelay >= 0) {
[self setDataValue:@(framesDelay) forKey:@"frames.delay"];
Expand Down
9 changes: 3 additions & 6 deletions Sources/Sentry/include/HybridPublic/SentryFramesTracker.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
@class SentryDispatchQueueWrapper;
@class SentryNSNotificationCenterWrapper;
@class SentryScreenFrames;
@class SentryFramesDelayResult;

NS_ASSUME_NONNULL_BEGIN

Expand Down Expand Up @@ -48,12 +49,8 @@ NS_ASSUME_NONNULL_BEGIN
- (void)start;
- (void)stop;

/*
* Returns the frames delay for the passed time period. If the method can't calculate the frames
* delay, it returns -1.
*/
- (CFTimeInterval)getFramesDelay:(uint64_t)startSystemTimestamp
endSystemTimestamp:(uint64_t)endSystemTimestamp;
- (SentryFramesDelayResult *)getFramesDelay:(uint64_t)startSystemTimestamp
endSystemTimestamp:(uint64_t)endSystemTimestamp;

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

Expand Down
11 changes: 5 additions & 6 deletions Sources/Sentry/include/SentryDelayedFramesTracker.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#if SENTRY_HAS_UIKIT

@class SentryCurrentDateProvider;
@class SentryFramesDelayResult;

NS_ASSUME_NONNULL_BEGIN

Expand Down Expand Up @@ -48,13 +49,11 @@ SENTRY_NO_INIT
* @param endSystemTimestamp The end system time stamp for the time interval to query frames delay.
* @param isRunning Wether the frames tracker is running or not.
* @param slowFrameThreshold The threshold for a slow frame. For 60 fps this is roughly 16.67 ms.
*
* @return the frames delay duration or -1 if it can't calculate the frames delay.
*/
- (CFTimeInterval)getFramesDelay:(uint64_t)startSystemTimestamp
endSystemTimestamp:(uint64_t)endSystemTimestamp
isRunning:(BOOL)isRunning
slowFrameThreshold:(CFTimeInterval)slowFrameThreshold;
- (SentryFramesDelayResult *)getFramesDelay:(uint64_t)startSystemTimestamp
endSystemTimestamp:(uint64_t)endSystemTimestamp
isRunning:(BOOL)isRunning
slowFrameThreshold:(CFTimeInterval)slowFrameThreshold;

@end

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import Foundation

@objcMembers
class SentryFramesDelayResult: NSObject {
/// The frames delay for the passed time period. If frame delay can't be calculated this is -1.
let delayDuration: CFTimeInterval
let framesContributingToDelayCount: UInt

init(delayDuration: CFTimeInterval, framesCount: UInt) {
self.delayDuration = delayDuration
self.framesContributingToDelayCount = framesCount
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import _SentryPrivate
@testable import _SentryPrivate
@testable import Sentry
import SentryTestUtils
import XCTest

Expand Down Expand Up @@ -198,7 +199,8 @@ class SentryFramesTrackerTests: XCTestCase {
let expectedDelay = displayLink.timeEpsilon + displayLink.slowestSlowFrameDuration - slowFrameThreshold(displayLink.currentFrameRate.rawValue)

let actualFrameDelay = sut.getFramesDelay(startSystemTime, endSystemTimestamp: endSystemTime)
XCTAssertEqual(actualFrameDelay, expectedDelay, accuracy: 0.0001)
XCTAssertEqual(actualFrameDelay.delayDuration, expectedDelay, accuracy: 0.0001)
XCTAssertEqual(actualFrameDelay.framesContributingToDelayCount, 4)
}

/**
Expand All @@ -225,7 +227,8 @@ class SentryFramesTrackerTests: XCTestCase {
let expectedDelay = delayWithoutFrameRecord - slowFrameThreshold(displayLink.currentFrameRate.rawValue)

let actualFrameDelay = sut.getFramesDelay(startSystemTime, endSystemTimestamp: endSystemTime)
XCTAssertEqual(actualFrameDelay, expectedDelay, accuracy: 0.0001)
XCTAssertEqual(actualFrameDelay.delayDuration, expectedDelay, accuracy: 0.0001)
XCTAssertEqual(actualFrameDelay.framesContributingToDelayCount, 2)
}

/**
Expand Down Expand Up @@ -293,7 +296,8 @@ class SentryFramesTrackerTests: XCTestCase {
let endSystemTime = fixture.dateProvider.systemTime()

let actualFrameDelay = sut.getFramesDelay(startSystemTime, endSystemTimestamp: endSystemTime)
XCTAssertEqual(actualFrameDelay, -1)
XCTAssertEqual(actualFrameDelay.delayDuration, -1)
XCTAssertEqual(actualFrameDelay.framesContributingToDelayCount, 0)
}

func testDelayedFrames_NoRecordedDelayedFrames_ReturnsZero() {
Expand All @@ -312,7 +316,8 @@ class SentryFramesTrackerTests: XCTestCase {
let endSystemTime = fixture.dateProvider.systemTime()

let actualFrameDelay = sut.getFramesDelay(startSystemTime, endSystemTimestamp: endSystemTime)
XCTAssertEqual(actualFrameDelay, 0.0, accuracy: 0.0001)
XCTAssertEqual(actualFrameDelay.delayDuration, 0.0, accuracy: 0.0001)
XCTAssertEqual(actualFrameDelay.framesContributingToDelayCount, 2)
}

func testDelayedFrames_NoRecordedDelayedFrames_ButFrameIsDelayed_ReturnsDelay() {
Expand All @@ -334,7 +339,8 @@ class SentryFramesTrackerTests: XCTestCase {
let expectedDelay = delay - slowFrameThreshold(fixture.displayLinkWrapper.currentFrameRate.rawValue)

let actualFrameDelay = sut.getFramesDelay(startSystemTime, endSystemTimestamp: endSystemTime)
XCTAssertEqual(actualFrameDelay, expectedDelay, accuracy: 0.0001)
XCTAssertEqual(actualFrameDelay.delayDuration, expectedDelay, accuracy: 0.0001)
XCTAssertEqual(actualFrameDelay.framesContributingToDelayCount, 2)
}

func testDelayedFrames_FrameIsDelayedSmallerThanSlowFrameThreshold_ReturnsDelay() {
Expand All @@ -359,7 +365,9 @@ class SentryFramesTrackerTests: XCTestCase {
let expectedDelay = delay

let actualFrameDelay = sut.getFramesDelay(startSystemTime, endSystemTimestamp: endSystemTime)
XCTAssertEqual(actualFrameDelay, expectedDelay, accuracy: 0.0001)
XCTAssertEqual(
actualFrameDelay.delayDuration, expectedDelay, accuracy: 0.0001)
XCTAssertEqual(actualFrameDelay.framesContributingToDelayCount, 1)
}

private func testFrameDelay(timeIntervalAfterFrameStart: TimeInterval = 0.0, timeIntervalBeforeFrameEnd: TimeInterval = 0.0, expectedDelay: TimeInterval) {
Expand All @@ -377,7 +385,7 @@ class SentryFramesTrackerTests: XCTestCase {
let startSystemTime = slowFrameStartSystemTime + timeIntervalToNanoseconds(timeIntervalAfterFrameStart)

let actualFrameDelay = sut.getFramesDelay(startSystemTime, endSystemTimestamp: endSystemTime)
XCTAssertEqual(actualFrameDelay, expectedDelay, accuracy: 0.0001)
XCTAssertEqual(actualFrameDelay.delayDuration, expectedDelay, accuracy: 0.0001)
}

/**
Expand Down Expand Up @@ -405,7 +413,7 @@ class SentryFramesTrackerTests: XCTestCase {
let expectedDelay = displayLink.slowestSlowFrameDuration - slowFrameThreshold(displayLink.currentFrameRate.rawValue)

let actualFrameDelay = sut.getFramesDelay(startSystemTime, endSystemTimestamp: endSystemTime)
XCTAssertEqual(actualFrameDelay, expectedDelay, accuracy: 0.0001)
XCTAssertEqual(actualFrameDelay.delayDuration, expectedDelay, accuracy: 0.0001)
}

func testFrameDelay_WithStartBeforeEnd_ReturnsMinusOne() {
Expand All @@ -417,7 +425,7 @@ class SentryFramesTrackerTests: XCTestCase {
_ = displayLink.slowestSlowFrame()

let actualFrameDelay = sut.getFramesDelay(1, endSystemTimestamp: 0)
XCTAssertEqual(actualFrameDelay, -1.0)
XCTAssertEqual(actualFrameDelay.delayDuration, -1.0)
}

func testFrameDelay_LongestTimeStamp_ReturnsMinusOne() {
Expand All @@ -429,7 +437,7 @@ class SentryFramesTrackerTests: XCTestCase {
_ = displayLink.slowestSlowFrame()

let actualFrameDelay = sut.getFramesDelay(0, endSystemTimestamp: UInt64.max)
XCTAssertEqual(actualFrameDelay, -1.0)
XCTAssertEqual(actualFrameDelay.delayDuration, -1.0)
}

func testFrameDelay_KeepAddingSlowFrames_OnlyTheMaxDurationFramesReturned() {
Expand All @@ -441,7 +449,7 @@ class SentryFramesTrackerTests: XCTestCase {
let endSystemTime = fixture.dateProvider.systemTime()

let actualFrameDelay = sut.getFramesDelay(startSystemTime, endSystemTimestamp: endSystemTime)
XCTAssertEqual(actualFrameDelay, expectedDelay, accuracy: 0.0001)
XCTAssertEqual(actualFrameDelay.delayDuration, expectedDelay, accuracy: 0.0001)
}

func testFrameDelay_MoreThanMaxDuration_FrameInformationMissing_DelayReturned() {
Expand All @@ -459,7 +467,7 @@ class SentryFramesTrackerTests: XCTestCase {
let expectedDelay = slowFramesDelay + delayNotRecorded

let actualFrameDelay = sut.getFramesDelay(startSystemTime, endSystemTimestamp: endSystemTime)
XCTAssertEqual(actualFrameDelay, expectedDelay, accuracy: 0.0001)
XCTAssertEqual(actualFrameDelay.delayDuration, expectedDelay, accuracy: 0.0001)
}

func testFrameDelay_MoreThanMaxDuration_StartTimeTooEarly_ReturnsMinusOne() {
Expand All @@ -471,7 +479,7 @@ class SentryFramesTrackerTests: XCTestCase {
let endSystemTime = fixture.dateProvider.systemTime()

let actualFrameDelay = sut.getFramesDelay(startSystemTime - 1, endSystemTimestamp: endSystemTime)
XCTAssertEqual(actualFrameDelay, -1, accuracy: 0.0001, "startSystemTimeStamp starts one nanosecond before the oldest slow frame. Therefore the frame delay can't be calculated and should me 0.")
XCTAssertEqual(actualFrameDelay.delayDuration, -1, accuracy: 0.0001, "startSystemTimeStamp starts one nanosecond before the oldest slow frame. Therefore the frame delay can't be calculated and should me 0.")
}

func testFrameDelay_FramesTrackerNotRunning_ReturnsMinusOne() {
Expand All @@ -489,7 +497,7 @@ class SentryFramesTrackerTests: XCTestCase {
sut.stop()

let actualFrameDelay = sut.getFramesDelay(startSystemTime, endSystemTimestamp: endSystemTime)
XCTAssertEqual(actualFrameDelay, -1.0)
XCTAssertEqual(actualFrameDelay.delayDuration, -1.0)
}

func testFrameDelay_RestartTracker_ReturnsMinusOne() {
Expand All @@ -504,7 +512,7 @@ class SentryFramesTrackerTests: XCTestCase {
let endSystemTime = fixture.dateProvider.systemTime()

let actualFrameDelay = sut.getFramesDelay(startSystemTime, endSystemTimestamp: endSystemTime)
XCTAssertEqual(actualFrameDelay, -1.0)
XCTAssertEqual(actualFrameDelay.delayDuration, -1.0)
}

func testFrameDelay_GetInfoFromBackgroundThreadWhileAdding() {
Expand All @@ -523,7 +531,7 @@ class SentryFramesTrackerTests: XCTestCase {

let actualFrameDelay = sut.getFramesDelay(startSystemTime, endSystemTimestamp: endSystemTime)

XCTAssertGreaterThanOrEqual(actualFrameDelay, -1)
XCTAssertGreaterThanOrEqual(actualFrameDelay.delayDuration, -1)

expectation.fulfill()
}
Expand Down Expand Up @@ -556,7 +564,7 @@ class SentryFramesTrackerTests: XCTestCase {

let frameDelay = sut.getFramesDelay(startSystemTimestamp, endSystemTimestamp: endSystemTimestamp)

XCTAssertLessThanOrEqual(frameDelay, 1.0)
XCTAssertLessThanOrEqual(frameDelay.delayDuration, 1.0)
}

expectation.fulfill()
Expand Down

0 comments on commit df2835d

Please sign in to comment.