From e9781e625f183632c0ed95b180fea9a6766052c6 Mon Sep 17 00:00:00 2001 From: Zim Date: Wed, 22 Feb 2023 13:55:35 +0000 Subject: [PATCH] Skip computing proc states during app initialization Now that app initialization covers class loading, there can be hundreds of milliseconds delay before a top app activity gets scheduled to start. During this delay, computing proc states overrides the top app special case set at attachApplication to boost the sched group. This is fixed with the following changes: 1. Skip computing proc states when the application hasn't finished attach application 2. Trigger a proc state update immediately after scheduling bindApplication. This ensures that the special cased proc state is applied as soon as possible Test: atest MockingOomAdjusterTests Bug: 253908737 Change-Id: Iae30a508b8b079dc88d6d5ffa027b7d805fec9e4 --- .../server/am/ActivityManagerService.java | 16 ++++----- .../com/android/server/am/OomAdjuster.java | 33 +++++++++++++++++-- .../server/am/MockingOomAdjusterTests.java | 30 +++++++++++++++++ 3 files changed, 68 insertions(+), 11 deletions(-) diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index a1f7445d41424..af9b2beb998aa 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -4925,14 +4925,8 @@ public class ActivityManagerService extends IActivityManager.Stub EventLogTags.writeAmProcBound(app.userId, pid, app.processName); synchronized (mProcLock) { - app.mState.setCurAdj(ProcessList.INVALID_ADJ); - app.mState.setSetAdj(ProcessList.INVALID_ADJ); - app.mState.setVerifiedAdj(ProcessList.INVALID_ADJ); - mOomAdjuster.setAttachingSchedGroupLSP(app); - app.mState.setForcingToImportant(null); + mOomAdjuster.setAttachingProcessStatesLSP(app); clearProcessForegroundLocked(app); - app.mState.setHasShownUi(false); - app.mState.setCached(false); app.setDebugging(false); app.setKilledByAm(false); app.setKilled(false); @@ -5100,8 +5094,14 @@ public class ActivityManagerService extends IActivityManager.Stub app.makeActive(thread, mProcessStats); checkTime(startTime, "attachApplicationLocked: immediately after bindApplication"); } + app.setPendingFinishAttach(true); + updateLruProcessLocked(app, false, null); checkTime(startTime, "attachApplicationLocked: after updateLruProcessLocked"); + + updateOomAdjLocked(app, OomAdjuster.OOM_ADJ_REASON_PROCESS_BEGIN); + checkTime(startTime, "attachApplicationLocked: after updateOomAdjLocked"); + final long now = SystemClock.uptimeMillis(); synchronized (mAppProfiler.mProfilerLock) { app.mProfile.setLastRequestedGc(now); @@ -5117,8 +5117,6 @@ public class ActivityManagerService extends IActivityManager.Stub if (!mConstants.mEnableWaitForFinishAttachApplication) { finishAttachApplicationInner(startSeq, callingUid, pid); - } else { - app.setPendingFinishAttach(true); } } catch (Exception e) { // We need kill the process group here. (b/148588589) diff --git a/services/core/java/com/android/server/am/OomAdjuster.java b/services/core/java/com/android/server/am/OomAdjuster.java index 679cf0a5694b1..086b77b03f57c 100644 --- a/services/core/java/com/android/server/am/OomAdjuster.java +++ b/services/core/java/com/android/server/am/OomAdjuster.java @@ -1259,12 +1259,19 @@ public class OomAdjuster { for (int i = numLru - 1; i >= 0; i--) { ProcessRecord app = lruList.get(i); final ProcessStateRecord state = app.mState; - if (!app.isKilledByAm() && app.getThread() != null && !app.isPendingFinishAttach()) { + if (!app.isKilledByAm() && app.getThread() != null) { // We don't need to apply the update for the process which didn't get computed if (state.getCompletedAdjSeq() == mAdjSeq) { applyOomAdjLSP(app, true, now, nowElapsed, oomAdjReason); } + if (app.isPendingFinishAttach()) { + // Avoid trimming processes that are still initializing. If they aren't + // hosting any components yet because they may be unfairly killed. + // We however apply any computed previously computed oom scores before skipping. + continue; + } + final ProcessServiceRecord psr = app.mServices; // Count the number of process types. switch (state.getCurProcState()) { @@ -1698,6 +1705,19 @@ public class OomAdjuster { return false; } + if (app.isPendingFinishAttach()) { + state.setAdjSeq(mAdjSeq); + state.setCompletedAdjSeq(mAdjSeq); + // If the process is still initializing, we skip computing any states because we + // don't want to override the special states that have been set at + // AMS#attachApplication with OomAdjuster#setAttachingProcessStates. + // In this limbo state, the app has |PROC_START_TIMEOUT| to finish attach application + // and receive updated proc_state based on its importance. + // Note that in this state, the oom_score is INVALID_ADJ which is outside the standard + // oom score range and the app is safe from lmkd kills. + return false; + } + state.setAdjTypeCode(ActivityManager.RunningAppProcessInfo.REASON_UNKNOWN); state.setAdjSource(null); state.setAdjTarget(null); @@ -3211,7 +3231,7 @@ public class OomAdjuster { } @GuardedBy({"mService", "mProcLock"}) - void setAttachingSchedGroupLSP(ProcessRecord app) { + void setAttachingProcessStatesLSP(ProcessRecord app) { int initialSchedGroup = SCHED_GROUP_DEFAULT; final ProcessStateRecord state = app.mState; // If the process has been marked as foreground, it is starting as the top app (with @@ -3231,6 +3251,15 @@ public class OomAdjuster { state.setSetSchedGroup(initialSchedGroup); state.setCurrentSchedulingGroup(initialSchedGroup); + state.setCurProcState(PROCESS_STATE_CACHED_EMPTY); + state.setCurCapability(PROCESS_CAPABILITY_NONE); + + state.setCurAdj(ProcessList.FOREGROUND_APP_ADJ); + state.setSetAdj(ProcessList.FOREGROUND_APP_ADJ); + state.setVerifiedAdj(ProcessList.FOREGROUND_APP_ADJ); + state.setForcingToImportant(null); + state.setHasShownUi(false); + state.setCached(true); } // ONLY used for unit testing in OomAdjusterTests.java diff --git a/services/tests/mockingservicestests/src/com/android/server/am/MockingOomAdjusterTests.java b/services/tests/mockingservicestests/src/com/android/server/am/MockingOomAdjusterTests.java index 485ce33dfb7d5..0b4c70c285c1f 100644 --- a/services/tests/mockingservicestests/src/com/android/server/am/MockingOomAdjusterTests.java +++ b/services/tests/mockingservicestests/src/com/android/server/am/MockingOomAdjusterTests.java @@ -1966,6 +1966,36 @@ public class MockingOomAdjusterTests { assertBfsl(app1); } + @SuppressWarnings("GuardedBy") + @Test + public void testUpdateOomAdj_DoOne_PendingFinishAttach() { + ProcessRecord app = spy(makeDefaultProcessRecord(MOCKAPP_PID, MOCKAPP_UID, + MOCKAPP_PROCESSNAME, MOCKAPP_PACKAGENAME, false)); + app.setPendingFinishAttach(true); + app.mState.setHasForegroundActivities(false); + + sService.mOomAdjuster.setAttachingProcessStatesLSP(app); + updateOomAdj(app); + + assertProcStates(app, PROCESS_STATE_CACHED_EMPTY, FOREGROUND_APP_ADJ, + SCHED_GROUP_DEFAULT); + } + + @SuppressWarnings("GuardedBy") + @Test + public void testUpdateOomAdj_DoOne_TopApp_PendingFinishAttach() { + ProcessRecord app = spy(makeDefaultProcessRecord(MOCKAPP_PID, MOCKAPP_UID, + MOCKAPP_PROCESSNAME, MOCKAPP_PACKAGENAME, false)); + app.setPendingFinishAttach(true); + app.mState.setHasForegroundActivities(true); + + sService.mOomAdjuster.setAttachingProcessStatesLSP(app); + updateOomAdj(app); + + assertProcStates(app, PROCESS_STATE_CACHED_EMPTY, FOREGROUND_APP_ADJ, + SCHED_GROUP_TOP_APP); + } + @SuppressWarnings("GuardedBy") @Test public void testUpdateOomAdj_UidIdle_StopService() {