Skip to content

WIP!!! - Fix app start trace outliers from network delays (#10733) #15409

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

WIP!!! - Fix app start trace outliers from network delays (#10733) #15409

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 45 additions & 23 deletions FirebasePerformance/Sources/AppActivity/FPRAppActivityTracker.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
static NSDate *doubleDispatchTime = nil;
static NSDate *applicationDidFinishLaunchTime = nil;
static NSTimeInterval gAppStartMaxValidDuration = 60 * 60; // 60 minutes.
static NSTimeInterval gAppStartReasonableValidDuration = 30.0; // 30 seconds, reasonable app start time???
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

low

The ??? in the comment suggests uncertainty. Please remove it for a more polished and professional code appearance.

Suggested change
static NSTimeInterval gAppStartReasonableValidDuration = 30.0; // 30 seconds, reasonable app start time???
static NSTimeInterval gAppStartReasonableValidDuration = 30.0; // 30 seconds, reasonable app start time.

Copy link
Contributor Author

@JesusRojass JesusRojass Oct 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will Fix, I may need input from @visumickey to see if this is ideal or an acceptable time (Hence the ???)

static FPRCPUGaugeData *gAppStartCPUGaugeData = nil;
static FPRMemoryGaugeData *gAppStartMemoryGaugeData = nil;
static BOOL isActivePrewarm = NO;
Expand Down Expand Up @@ -71,6 +72,9 @@ @interface FPRAppActivityTracker ()
/** Tracks if the gauge metrics are dispatched. */
@property(nonatomic) BOOL appStartGaugeMetricDispatched;

/** Tracks if TTI stage has been started for this instance. */
@property(nonatomic) BOOL ttiStageStarted;

/** Firebase Performance Configuration object */
@property(nonatomic) FPRConfigurations *configurations;

Expand Down Expand Up @@ -135,6 +139,7 @@ - (instancetype)initAppActivityTracker {
if (self != nil) {
_applicationState = FPRApplicationStateUnknown;
_appStartGaugeMetricDispatched = NO;
_ttiStageStarted = NO;
_configurations = [FPRConfigurations sharedInstance];
[self startTrackingNetwork];
}
Expand Down Expand Up @@ -250,7 +255,7 @@ - (void)appDidBecomeActiveNotification:(NSNotification *)notification {
[self.appStartTrace startStageNamed:kFPRAppStartStageNameTimeToFirstDraw];
});

// If ever the app start trace had it life in background stage, do not send the trace.
// If ever the app start trace had its life in background stage, do not send the trace.
if (self.appStartTrace.backgroundTraceState != FPRTraceStateForegroundOnly) {
self.appStartTrace = nil;
}
Expand All @@ -266,29 +271,46 @@ - (void)appDidBecomeActiveNotification:(NSNotification *)notification {
self.foregroundSessionTrace = appTrace;

// Start measuring time to make the app interactive on the App start trace.
static BOOL TTIStageStarted = NO;
if (!TTIStageStarted) {
if (!self.ttiStageStarted) {
[self.appStartTrace startStageNamed:kFPRAppStartStageNameTimeToUserInteraction];
TTIStageStarted = YES;

// Assumption here is that - the app becomes interactive in the next runloop cycle.
// It is possible that the app does more things later, but for now we are not measuring that.
dispatch_async(dispatch_get_main_queue(), ^{
NSTimeInterval startTimeSinceEpoch = [self.appStartTrace startTimeSinceEpoch];
NSTimeInterval currentTimeSinceEpoch = [[NSDate date] timeIntervalSince1970];

// The below check is to account for 2 scenarios.
// 1. The app gets started in the background and might come to foreground a lot later.
// 2. The app is launched, but immediately backgrounded for some reason and the actual launch
// happens a lot later.
// Dropping the app start trace in such situations where the launch time is taking more than
// 60 minutes. This is an approximation, but a more agreeable timelimit for app start.
if ((currentTimeSinceEpoch - startTimeSinceEpoch < gAppStartMaxValidDuration) &&
[self isAppStartEnabled] && ![self isApplicationPreWarmed]) {
[self.appStartTrace stop];
} else {
[self.appStartTrace cancel];
}
self.ttiStageStarted = YES;

// Use dispatch_async with a higher priority queue to reduce interference from network
// operations This ensures trace completion isn't delayed by main queue congestion from network
// calls
__weak typeof(self) weakSelf = self;
dispatch_async(dispatch_get_global_queue(QOS_CLASS_USER_INITIATED, 0), ^{
dispatch_async(dispatch_get_main_queue(), ^{
__strong typeof(weakSelf) strongSelf = weakSelf;
if (!strongSelf || !strongSelf.appStartTrace) {
return;
}

NSTimeInterval startTimeSinceEpoch = [strongSelf.appStartTrace startTimeSinceEpoch];
NSTimeInterval currentTimeSinceEpoch = [[NSDate date] timeIntervalSince1970];
NSTimeInterval elapsed = currentTimeSinceEpoch - startTimeSinceEpoch;

// The below check accounts for multiple scenarios:
// 1. App started in background and comes to foreground later
// 2. App launched but immediately backgrounded
// 3. Network delays during startup inflating metrics
BOOL shouldCompleteTrace = (elapsed < gAppStartMaxValidDuration) &&
[strongSelf isAppStartEnabled] &&
![strongSelf isApplicationPreWarmed];

// Additional safety: cancel if elapsed time is unreasonably long for app start
if (shouldCompleteTrace && elapsed < gAppStartReasonableValidDuration) {
[strongSelf.appStartTrace stop];
} else {
[strongSelf.appStartTrace cancel];
if (elapsed >= gAppStartReasonableValidDuration) {
// Log for debugging network related delays
NSLog(
@"Firebase Performance: App start trace cancelled due to excessive duration: %.2fs",
elapsed);
}
}
});
});
Comment on lines +278 to 314
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The nested dispatch_async calls seem overly complex for deferring execution. The outer dispatch to a global user-initiated queue, only to immediately dispatch back to the main queue, doesn't prevent delays if the main queue is congested. The block will still have to wait for its turn on the main thread.

A single dispatch_async to the main queue is sufficient to defer the execution to the next run loop cycle. This simplifies the code and removes the misleading comment about avoiding main queue congestion.

The use of weakSelf/strongSelf is a good improvement for safety over the original implementation.

    // Defer stopping the trace to the next run loop cycle. This is to ensure that the app is
    // fully interactive.
    __weak typeof(self) weakSelf = self;
    dispatch_async(dispatch_get_main_queue(), ^{
      __strong typeof(weakSelf) strongSelf = weakSelf;
      if (!strongSelf || !strongSelf.appStartTrace) {
        return;
      }

      NSTimeInterval startTimeSinceEpoch = [strongSelf.appStartTrace startTimeSinceEpoch];
      NSTimeInterval currentTimeSinceEpoch = [[NSDate date] timeIntervalSince1970];
      NSTimeInterval elapsed = currentTimeSinceEpoch - startTimeSinceEpoch;

      // The below check accounts for multiple scenarios:
      // 1. App started in background and comes to foreground later
      // 2. App launched but immediately backgrounded
      // 3. Network delays during startup inflating metrics
      BOOL shouldCompleteTrace = (elapsed < gAppStartMaxValidDuration) &&
                                 [strongSelf isAppStartEnabled] &&
                                 ![strongSelf isApplicationPreWarmed];

      // Additional safety: cancel if elapsed time is unreasonably long for app start
      if (shouldCompleteTrace && elapsed < gAppStartReasonableValidDuration) {
        [strongSelf.appStartTrace stop];
      } else {
        [strongSelf.appStartTrace cancel];
        if (elapsed >= gAppStartReasonableValidDuration) {
          // Log for debugging network related delays
          NSLog(
              @"Firebase Performance: App start trace cancelled due to excessive duration: %.2fs",
              elapsed);
        }
      }
    });

}
}
Expand Down