perf(android): Move start-reason binder off main thread (JAVA-616) #5866
+97
−41
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| package io.sentry.android.core.performance; | ||
|
|
||
| import android.annotation.SuppressLint; | ||
| import android.app.Activity; | ||
| import android.app.ActivityManager; | ||
| import android.app.Application; | ||
|
|
@@ -35,6 +36,7 @@ | |
| import java.util.HashMap; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import java.util.concurrent.Executor; | ||
| import java.util.concurrent.TimeUnit; | ||
| import java.util.concurrent.atomic.AtomicBoolean; | ||
| import java.util.concurrent.atomic.AtomicInteger; | ||
|
|
@@ -99,7 +101,17 @@ public enum AppStartType { | |
| private @Nullable String appStartSentryTraceHeader; | ||
| private @Nullable String appStartBaggageHeader; | ||
| private @Nullable SentryDate appStartEndTime; | ||
| // Volatile: written from the background lookup thread, read from the main thread. Null until the | ||
| // deferred getHistoricalProcessStartReasons lookup resolves. | ||
| private volatile @Nullable ApplicationStartInfo cachedStartInfo; | ||
| // Runs the getHistoricalProcessStartReasons binder call off the main thread. The Sentry executor | ||
| // does not exist this early (ContentProvider time), so a plain daemon thread is used by default. | ||
| private @NotNull Executor startInfoExecutor = | ||
|
Member
There was a problem hiding this comment. We could have the executor as a local variable and immediately call shut down after enqueuing like here, to avoid any dangling executors. |
||
| command -> { | ||
| final Thread thread = new Thread(command, "SentryAppStartInfo"); | ||
| thread.setDaemon(true); | ||
| thread.start(); | ||
| }; | ||
| private final @NotNull AppStartExtension appStartExtension = new AppStartExtension(this); | ||
|
|
||
| public static @NotNull AppStartMetrics getInstance() { | ||
|
|
@@ -487,37 +499,59 @@ public void registerLifecycleCallbacks(final @NotNull Application application) { | |
| final @Nullable ActivityManager activityManager = | ||
| (ActivityManager) application.getSystemService(Context.ACTIVITY_SERVICE); | ||
| if (activityManager != null) { | ||
| // getHistoricalProcessStartReasons blocks on a system_server binder round-trip. Run it off | ||
| // the main thread so it doesn't stall the coldest part of app start. The result is consumed | ||
| // best-effort: appStartType (see onActivityCreated) falls back to the pre-API-35 heuristic | ||
| // and getAppStartReason returns null if the lookup hasn't resolved yet. | ||
| startInfoExecutor.execute(() -> loadStartInfo(activityManager)); | ||
| } | ||
| } | ||
|
|
||
| scheduleHeadlessAppStartCheckOnMain(); | ||
| } | ||
|
|
||
| @SuppressLint("NewApi") // reached only from the API 35+ guarded branch above | ||
| private void loadStartInfo(final @NotNull ActivityManager activityManager) { | ||
| try { | ||
| final List<ApplicationStartInfo> historicalProcessStartReasons = | ||
| activityManager.getHistoricalProcessStartReasons(1); | ||
| if (!historicalProcessStartReasons.isEmpty()) { | ||
| cachedStartInfo = historicalProcessStartReasons.get(0); | ||
| } | ||
| } catch (RuntimeException ignored) { | ||
| // getHistoricalProcessStartReasons may throw different kinds of exceptions, namely: | ||
| // - SecurityException when called from an isolated process | ||
| // - IllegalArgumentException when called with a wrong userId | ||
| // - others | ||
| // See impl: | ||
| // https://cs.android.com/android/platform/superproject/+/android-latest-release:frameworks/base/services/core/java/com/android/server/am/ActivityManagerService.java;l=10866-10893 | ||
| Log.w("AppStartMetrics", ignored); // no logger instance here, so we just Log | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Maps the deferred {@link ApplicationStartInfo} to an app start type. Returns {@link | ||
| * AppStartType#UNKNOWN} when the lookup hasn't resolved yet or the OS hadn't finished the start, | ||
| * so callers fall back to the pre-API-35 heuristic. | ||
| */ | ||
| private @NotNull AppStartType getStartInfoAppStartType() { | ||
| final @Nullable ApplicationStartInfo info = cachedStartInfo; | ||
| if (info == null || Build.VERSION.SDK_INT < Build.VERSION_CODES.VANILLA_ICE_CREAM) { | ||
| return AppStartType.UNKNOWN; | ||
| } | ||
| if (info.getStartupState() != ApplicationStartInfo.STARTUP_STATE_STARTED) { | ||
| return AppStartType.UNKNOWN; | ||
| } | ||
| return info.getStartType() == ApplicationStartInfo.START_TYPE_COLD | ||
| ? AppStartType.COLD | ||
| : AppStartType.WARM; | ||
| } | ||
|
|
||
| @TestOnly | ||
| void setStartInfoExecutor(final @NotNull Executor startInfoExecutor) { | ||
| this.startInfoExecutor = startInfoExecutor; | ||
| } | ||
|
|
||
| private void scheduleHeadlessAppStartCheckOnMain() { | ||
| if (!headlessAppStartCheckPending.compareAndSet(false, true)) { | ||
| return; | ||
|
|
@@ -562,10 +596,12 @@ private void handleHeadlessAppStartIfNeededOnMain() { | |
|
|
||
| appLaunchedInForeground.setValue(false); | ||
|
|
||
| // Headless starts have no Activity signal for the pre-API 35 warm/cold heuristic. Prefer the | ||
| // deferred ApplicationStartInfo type if its lookup has resolved, otherwise classify the | ||
| // process start as cold. | ||
| if (appStartType == AppStartType.UNKNOWN) { | ||
| final @NotNull AppStartType startInfoType = getStartInfoAppStartType(); | ||
| appStartType = startInfoType != AppStartType.UNKNOWN ? startInfoType : AppStartType.COLD; | ||
| } | ||
|
|
||
| // we stop the app start profilers, as they are useless and likely to timeout | ||
|
|
@@ -657,8 +693,12 @@ public void onActivityCreated(@NonNull Activity activity, @Nullable Bundle saved | |
| contentProviderOnCreates.clear(); | ||
| applicationOnCreate.reset(); | ||
| } else if (appStartType == AppStartType.UNKNOWN) { | ||
| // Prefer the authoritative API 35+ ApplicationStartInfo if its deferred lookup has | ||
| // resolved, otherwise fall back to the pre-API-35 heuristic. | ||
| final @NotNull AppStartType startInfoType = getStartInfoAppStartType(); | ||
| if (startInfoType != AppStartType.UNKNOWN) { | ||
| appStartType = startInfoType; | ||
| } else if (savedInstanceState != null) { | ||
| appStartType = AppStartType.WARM; | ||
| } else if (firstIdle != -1 && activityCreatedUptimeMillis > firstIdle) { | ||
| appStartType = AppStartType.WARM; | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
How about turning this into a
LazyEvaluator<ApplicationStartInfo> cachedStartInfo? Then the executor can simple call get(), to trigger evaluation off the main thread.