-
Notifications
You must be signed in to change notification settings - Fork 708
firebase-perf: fix API 34+ background-start heuristic in AppStartTrac… #8202
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.
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
592ef91
ff71dfa
5595887
f639984
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -74,11 +74,6 @@ public class AppStartTrace implements ActivityLifecycleCallbacks, LifecycleObser | |
| private static final @NonNull Timer PERF_CLASS_LOAD_TIME = new Clock().getTime(); | ||
| private static final long MAX_LATENCY_BEFORE_UI_INIT = TimeUnit.MINUTES.toMicros(1); | ||
|
|
||
| // If the `mainThreadRunnableTime` was set within this duration, the assumption | ||
| // is that it was called immediately before `onActivityCreated` in foreground starts on API 34+. | ||
| // See b/339891952. | ||
| private static final long MAX_BACKGROUND_RUNNABLE_DELAY = TimeUnit.MILLISECONDS.toMicros(50); | ||
|
|
||
| // Core pool size 0 allows threads to shut down if they're idle | ||
| private static final int CORE_POOL_SIZE = 0; | ||
| private static final int MAX_POOL_SIZE = 1; // Only need single thread | ||
|
|
@@ -134,6 +129,11 @@ public class AppStartTrace implements ActivityLifecycleCallbacks, LifecycleObser | |
| private final DrawCounter onDrawCounterListener = new DrawCounter(); | ||
| private boolean systemForegroundCheck = false; | ||
|
|
||
| // OS-reported reason this process was forked. Captured once during | ||
| // registerActivityLifecycleCallbacks; consulted by resolveIsStartedFromBackground on | ||
| // API 34+. | ||
| private @Nullable ProcessStartCause processStartCause = null; | ||
|
|
||
| /** | ||
| * Called from onCreate() method of an activity by instrumented byte code. | ||
| * | ||
|
|
@@ -224,6 +224,9 @@ public synchronized void registerActivityLifecycleCallbacks(@NonNull Context con | |
| if (appContext instanceof Application) { | ||
| ((Application) appContext).registerActivityLifecycleCallbacks(this); | ||
| systemForegroundCheck = systemForegroundCheck || isAnyAppProcessInForeground(appContext); | ||
| // Capture the OS-reported start cause as early as possible (this method runs from | ||
| // FirebasePerfEarly during the ContentProvider init chain). | ||
| processStartCause = ProcessStartCause.capture(appContext); | ||
| isRegisteredForLifecycleCallbacks = true; | ||
| this.appContext = appContext; | ||
| } | ||
|
|
@@ -327,37 +330,30 @@ private void recordOnDrawFrontOfQueue() { | |
| } | ||
|
|
||
| /** | ||
| * Sets the `isStartedFromBackground` flag to `true` if the `mainThreadRunnableTime` was set | ||
| * from the `StartFromBackgroundRunnable`. | ||
| * <p> | ||
| * If it's prior to API 34, it's always set to true if `mainThreadRunnableTime` was set. | ||
| * <p> | ||
| * If it's on or after API 34, and it was called less than `MAX_BACKGROUND_RUNNABLE_DELAY` | ||
| * before `onActivityCreated`, the | ||
| * assumption is that it was called immediately before the activity lifecycle callbacks in a | ||
| * foreground start. | ||
| * See b/339891952. | ||
| * Decide whether this process was background-only and, if so, set | ||
| * {@link #isStartedFromBackground} so the activity-lifecycle callbacks suppress the | ||
| * {@code _app_start} trace. | ||
| * | ||
| * API < 34: legacy pre-bug ordering. If {@link StartFromBackgroundRunnable} fired | ||
| * before the first {@code onActivityCreated}, suppress. | ||
| * | ||
| * API 34+: {@link ProcessStartCause} owns the decision. {@code FOREGROUND} lets the | ||
| * trace through; {@code UNKNOWN} or null suppresses. | ||
| * | ||
| * See b/339891952 and https://github.com/firebase/firebase-android-sdk/issues/8103. | ||
| */ | ||
| private void resolveIsStartedFromBackground() { | ||
| // If the mainThreadRunnableTime is null, either the runnable hasn't run, or this check has | ||
| // already been made. | ||
| if (mainThreadRunnableTime == null) { | ||
| if (Build.VERSION.SDK_INT < 34) { | ||
| if (mainThreadRunnableTime != null) { | ||
| isStartedFromBackground = true; | ||
| mainThreadRunnableTime = null; | ||
| } | ||
| return; | ||
| } | ||
|
|
||
| // If the `mainThreadRunnableTime` was set prior to API 34, it's always assumed that's it's | ||
| // a background start. | ||
| // Otherwise it's assumed to be a background start if the runnable was set more than | ||
| // `MAX_BACKGROUND_RUNNABLE_DELAY` | ||
| // before the first `onActivityCreated` call. | ||
| // TODO(b/339891952): Investigate removing the API check. | ||
| if ((Build.VERSION.SDK_INT < 34) | ||
| || (mainThreadRunnableTime.getDurationMicros() > MAX_BACKGROUND_RUNNABLE_DELAY)) { | ||
| if (processStartCause == null | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you add a comment here about the API 34+ behaviour? |
||
| || processStartCause.cause != ProcessStartCause.Cause.FOREGROUND) { | ||
| isStartedFromBackground = true; | ||
| } | ||
|
|
||
| // Set this to null to prevent additional checks. | ||
| mainThreadRunnableTime = null; | ||
| } | ||
|
|
||
| @Override | ||
|
|
@@ -633,4 +629,15 @@ Timer getOnResumeTime() { | |
| void setMainThreadRunnableTime(Timer timer) { | ||
| mainThreadRunnableTime = timer; | ||
| } | ||
|
|
||
| @VisibleForTesting | ||
| void setProcessStartCauseForTest(@Nullable ProcessStartCause cause) { | ||
| this.processStartCause = cause; | ||
| } | ||
|
|
||
| @VisibleForTesting | ||
| @Nullable | ||
| ProcessStartCause getProcessStartCauseForTest() { | ||
| return processStartCause; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,102 @@ | ||
| // Copyright 2026 Google LLC | ||
| // | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // | ||
| // You may obtain a copy of the License at | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| package com.google.firebase.perf.metrics; | ||
|
|
||
| import android.app.ActivityManager; | ||
| import android.content.Context; | ||
| import android.os.Build; | ||
| import androidx.annotation.NonNull; | ||
| import androidx.annotation.Nullable; | ||
| import androidx.annotation.VisibleForTesting; | ||
|
|
||
| /** | ||
| * OS-reported reason this process was forked, used by {@link AppStartTrace} to decide | ||
| * whether to emit the {@code _app_start} trace. | ||
| * | ||
| * API 34+: {@link ActivityManager#getMyMemoryState} importance. | ||
| * {@code IMPORTANCE_FOREGROUND} at first capture indicates an activity-driven start. | ||
| * | ||
| * API < 34: returns {@link Cause#UNKNOWN}; legacy logic in {@link AppStartTrace} owns | ||
| * the decision on these versions. | ||
| * | ||
| * @hide | ||
| */ | ||
| final class ProcessStartCause { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you rename this to
|
||
|
|
||
| /** Classification of why the process was forked. */ | ||
| enum Cause { | ||
| /** Process forked to satisfy an activity launch. */ | ||
| FOREGROUND, | ||
| /** Couldn't decide — caller falls back to its own heuristic. */ | ||
| UNKNOWN | ||
| } | ||
|
|
||
| /** OS classification. Never null. */ | ||
| final @NonNull Cause cause; | ||
|
|
||
| /** {@code RunningAppProcessInfo.importance} at capture, or {@code -1} if unread. */ | ||
| final int importance; | ||
|
|
||
| /** {@link Build.VERSION#SDK_INT} at capture. */ | ||
| final int apiLevel; | ||
|
|
||
| @VisibleForTesting | ||
| ProcessStartCause(@NonNull Cause cause, int importance, int apiLevel) { | ||
| this.cause = cause; | ||
| this.importance = importance; | ||
| this.apiLevel = apiLevel; | ||
| } | ||
|
|
||
| /** | ||
| * Capture the cause for the current process. Call as early as possible (during | ||
| * {@code AppStartTrace.registerActivityLifecycleCallbacks}) so the OS-set values still | ||
| * reflect the original fork reason rather than transient state mid-init. | ||
| */ | ||
| static @NonNull ProcessStartCause capture(@Nullable Context appContext) { | ||
| final int apiLevel = Build.VERSION.SDK_INT; | ||
| if (appContext == null) { | ||
| return new ProcessStartCause(Cause.UNKNOWN, -1, apiLevel); | ||
| } | ||
|
|
||
| final ActivityManager activityManager = | ||
| (ActivityManager) appContext.getSystemService(Context.ACTIVITY_SERVICE); | ||
| if (activityManager == null) { | ||
| return new ProcessStartCause(Cause.UNKNOWN, -1, apiLevel); | ||
| } | ||
|
|
||
| final int importance = readImportance(); | ||
|
|
||
| if (apiLevel >= 34) { | ||
| Cause cause = | ||
| importance == ActivityManager.RunningAppProcessInfo.IMPORTANCE_FOREGROUND | ||
| ? Cause.FOREGROUND | ||
| : Cause.UNKNOWN; | ||
| return new ProcessStartCause(cause, importance, apiLevel); | ||
| } | ||
|
|
||
| // API < 34: legacy AppStartTrace logic owns the decision. | ||
| return new ProcessStartCause(Cause.UNKNOWN, importance, apiLevel); | ||
| } | ||
|
|
||
| private static int readImportance() { | ||
| try { | ||
| ActivityManager.RunningAppProcessInfo info = new ActivityManager.RunningAppProcessInfo(); | ||
| ActivityManager.getMyMemoryState(info); | ||
| return info.importance; | ||
| } catch (Throwable t) { | ||
| return -1; | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a comment here summarizing that only if it's < API 34 it checks / updates the mainThreadRunnable.