From e22197994a034dbceae96fbd2bfe21addaad7d99 Mon Sep 17 00:00:00 2001 From: Tim Murray Date: Tue, 13 Dec 2022 13:13:59 -0800 Subject: [PATCH 1/3] Reland OomAdjuster: demote previous process to cached after configurable timeout Previous processes shouldn't stay out of cached indefinitely; they should be eventually moved to cached and allowed to be restricted and frozen. Add a configurable timeout to decide when the previous app should move to cached. The default timeout is 60s. Reverts change bb195e7af6b6d4c69def28cfd5c9225ff08a819d. Bug: 261065790 Test: atest MockingOomAdjusterTests Change-Id: I0494059976275cab309a5353bfef608da796bd2f Merged-in: I0494059976275cab309a5353bfef608da796bd2f --- .../server/am/ActivityManagerConstants.java | 16 +++++++ .../com/android/server/am/OomAdjuster.java | 44 ++++++++++++------- 2 files changed, 44 insertions(+), 16 deletions(-) diff --git a/services/core/java/com/android/server/am/ActivityManagerConstants.java b/services/core/java/com/android/server/am/ActivityManagerConstants.java index 44f475f925174..9d4174dfcc16f 100644 --- a/services/core/java/com/android/server/am/ActivityManagerConstants.java +++ b/services/core/java/com/android/server/am/ActivityManagerConstants.java @@ -129,6 +129,7 @@ final class ActivityManagerConstants extends ContentObserver { static final String KEY_KILL_BG_RESTRICTED_CACHED_IDLE = "kill_bg_restricted_cached_idle"; static final String KEY_KILL_BG_RESTRICTED_CACHED_IDLE_SETTLE_TIME = "kill_bg_restricted_cached_idle_settle_time"; + static final String KEY_MAX_PREVIOUS_TIME = "max_previous_time"; /** * Note this key is on {@link DeviceConfig#NAMESPACE_ACTIVITY_MANAGER_COMPONENT_ALIAS}. * @see #mEnableComponentAlias @@ -192,6 +193,7 @@ final class ActivityManagerConstants extends ContentObserver { private static final float DEFAULT_FGS_START_DENIED_LOG_SAMPLE_RATE = 1; // 100% private static final long DEFAULT_PROCESS_KILL_TIMEOUT_MS = 10 * 1000; private static final long DEFAULT_NETWORK_ACCESS_TIMEOUT_MS = 200; // 0.2 sec + private static final long DEFAULT_MAX_PREVIOUS_TIME = 60 * 1000; // 60s static final long DEFAULT_BACKGROUND_SETTLE_TIME = 60 * 1000; static final long DEFAULT_KILL_BG_RESTRICTED_CACHED_IDLE_SETTLE_TIME_MS = 60 * 1000; @@ -534,6 +536,9 @@ final class ActivityManagerConstants extends ContentObserver { public long TOP_TO_ALMOST_PERCEPTIBLE_GRACE_DURATION = DEFAULT_TOP_TO_ALMOST_PERCEPTIBLE_GRACE_DURATION; + // How long a process can remain at previous oom_adj before dropping to cached + public static long MAX_PREVIOUS_TIME = DEFAULT_MAX_PREVIOUS_TIME; + /** * The minimum time we allow between crashes, for us to consider this * application to be bad and stop its services and reject broadcasts. @@ -1171,6 +1176,9 @@ final class ActivityManagerConstants extends ContentObserver { case KEY_ENABLE_WAIT_FOR_FINISH_ATTACH_APPLICATION: updateEnableWaitForFinishAttachApplication(); break; + case KEY_MAX_PREVIOUS_TIME: + updateMaxPreviousTime(); + break; default: break; } @@ -1825,6 +1833,7 @@ final class ActivityManagerConstants extends ContentObserver { DEFAULT_LOW_SWAP_THRESHOLD_PERCENT); } + private void updateTopToFgsGraceDuration() { TOP_TO_FGS_GRACE_DURATION = DeviceConfig.getLong( DeviceConfig.NAMESPACE_ACTIVITY_MANAGER, @@ -1832,6 +1841,13 @@ final class ActivityManagerConstants extends ContentObserver { DEFAULT_TOP_TO_FGS_GRACE_DURATION); } + private void updateMaxPreviousTime() { + MAX_PREVIOUS_TIME = DeviceConfig.getLong( + DeviceConfig.NAMESPACE_ACTIVITY_MANAGER, + KEY_MAX_PREVIOUS_TIME, + DEFAULT_MAX_PREVIOUS_TIME); + } + private void updateMinAssocLogDuration() { MIN_ASSOC_LOG_DURATION = DeviceConfig.getLong( DeviceConfig.NAMESPACE_ACTIVITY_MANAGER, KEY_MIN_ASSOC_LOG_DURATION, diff --git a/services/core/java/com/android/server/am/OomAdjuster.java b/services/core/java/com/android/server/am/OomAdjuster.java index 32db33d3047d2..baa2f8e46dd89 100644 --- a/services/core/java/com/android/server/am/OomAdjuster.java +++ b/services/core/java/com/android/server/am/OomAdjuster.java @@ -2018,25 +2018,37 @@ public class OomAdjuster { } } } - if (state.getCachedIsPreviousProcess() && state.getCachedHasActivities()) { - if (adj > PREVIOUS_APP_ADJ) { - // This was the previous process that showed UI to the user. - // We want to try to keep it around more aggressively, to give - // a good experience around switching between two apps. - adj = PREVIOUS_APP_ADJ; - schedGroup = SCHED_GROUP_BACKGROUND; - state.setCached(false); - state.setAdjType("previous"); - if (DEBUG_OOM_ADJ_REASON || logUid == appUid) { - reportOomAdjMessageLocked(TAG_OOM_ADJ, "Raise adj to prev: " + app); - } - } - if (procState > PROCESS_STATE_LAST_ACTIVITY) { + // This was the previous process that showed UI to the user. We want to + // try to keep it around more aggressively, to give a good experience + // around switching between two apps. However, we don't want to keep the + // process in this privileged state indefinitely. Eventually, allow the + // app to be demoted to cached. + if ((state.getSetProcState() == PROCESS_STATE_LAST_ACTIVITY + && (state.getLastStateTime() + mConstants.MAX_PREVIOUS_TIME) < now)) { procState = PROCESS_STATE_LAST_ACTIVITY; - state.setAdjType("previous"); + schedGroup = SCHED_GROUP_BACKGROUND; + state.setAdjType("previous-expired"); + adj = CACHED_APP_MIN_ADJ; if (DEBUG_OOM_ADJ_REASON || logUid == appUid) { - reportOomAdjMessageLocked(TAG_OOM_ADJ, "Raise procstate to prev: " + app); + reportOomAdjMessageLocked(TAG_OOM_ADJ, "Expire prev adj: " + app); + } + } else { + if (adj > PREVIOUS_APP_ADJ) { + adj = PREVIOUS_APP_ADJ; + schedGroup = SCHED_GROUP_BACKGROUND; + state.setCached(false); + state.setAdjType("previous"); + if (DEBUG_OOM_ADJ_REASON || logUid == appUid) { + reportOomAdjMessageLocked(TAG_OOM_ADJ, "Raise adj to prev: " + app); + } + } + if (procState > PROCESS_STATE_LAST_ACTIVITY) { + procState = PROCESS_STATE_LAST_ACTIVITY; + state.setAdjType("previous"); + if (DEBUG_OOM_ADJ_REASON || logUid == appUid) { + reportOomAdjMessageLocked(TAG_OOM_ADJ, "Raise procstate to prev: " + app); + } } } } From 326c51d79bd7a5ed0fd23842b30dee99f965260a Mon Sep 17 00:00:00 2001 From: Tim Murray Date: Tue, 29 Nov 2022 14:49:04 -0800 Subject: [PATCH 2/3] OomAdjuster: use oom_adj_score tiers for cached apps Metrics suggest that cached apps do not get promoted out of cached in anything resembling LRU ordering. Maintaining the LRU order is expensive for both ActivityManager and LMKD. Instead of a strict LRU, use tiers of oom_adj_scores for cached apps: - 900 for cached apps bound with BIND_WAIVE_PRIORITY or similar - 910 for freezer-eligible apps that entered cached in the last 60s - 950 for freezer-eligible apps that entered cached more than 60s ago Tiered oom_adj_scores for cached apps can be enabled with the "use_tiered_cached_adj" DeviceConfig flag, and the decay time (in ms) can be modified with the "tiered_cached_adj_decay_time" DeviceConfig flag. oom_adj_score tiers currently default to off. Bug: 253908413 Test: atest MockingOomAdjusterTests Change-Id: Ie386672c321227eb689902863df0b0022fa09e49 Merged-in: Ie386672c321227eb689902863df0b0022fa09e49 --- .../server/am/ActivityManagerConstants.java | 32 +++ .../com/android/server/am/OomAdjuster.java | 252 ++++++++++-------- .../server/am/MockingOomAdjusterTests.java | 23 +- 3 files changed, 188 insertions(+), 119 deletions(-) diff --git a/services/core/java/com/android/server/am/ActivityManagerConstants.java b/services/core/java/com/android/server/am/ActivityManagerConstants.java index 9d4174dfcc16f..9e95e5fb0a40e 100644 --- a/services/core/java/com/android/server/am/ActivityManagerConstants.java +++ b/services/core/java/com/android/server/am/ActivityManagerConstants.java @@ -146,6 +146,9 @@ final class ActivityManagerConstants extends ContentObserver { */ static final String KEY_NETWORK_ACCESS_TIMEOUT_MS = "network_access_timeout_ms"; + static final String KEY_USE_TIERED_CACHED_ADJ = "use_tiered_cached_adj"; + static final String KEY_TIERED_CACHED_ADJ_DECAY_TIME = "tiered_cached_adj_decay_time"; + private static final int DEFAULT_MAX_CACHED_PROCESSES = 32; private static final boolean DEFAULT_PRIORITIZE_ALARM_BROADCASTS = true; private static final long DEFAULT_FGSERVICE_MIN_SHOWN_TIME = 2*1000; @@ -201,6 +204,9 @@ final class ActivityManagerConstants extends ContentObserver { static final int DEFAULT_MAX_SERVICE_CONNECTIONS_PER_PROCESS = 3000; + private static final boolean DEFAULT_USE_TIERED_CACHED_ADJ = false; + private static final long DEFAULT_TIERED_CACHED_ADJ_DECAY_TIME = 60 * 1000; + /** * Same as {@link TEMPORARY_ALLOW_LIST_TYPE_FOREGROUND_SERVICE_NOT_ALLOWED} */ @@ -1011,6 +1017,12 @@ final class ActivityManagerConstants extends ContentObserver { public volatile long mShortFgsAnrExtraWaitDuration = DEFAULT_SHORT_FGS_ANR_EXTRA_WAIT_DURATION; + /** @see #KEY_USE_TIERED_CACHED_ADJ */ + public boolean USE_TIERED_CACHED_ADJ = DEFAULT_USE_TIERED_CACHED_ADJ; + + /** @see #KEY_TIERED_CACHED_ADJ_DECAY_TIME */ + public long TIERED_CACHED_ADJ_DECAY_TIME = DEFAULT_TIERED_CACHED_ADJ_DECAY_TIME; + private final OnPropertiesChangedListener mOnDeviceConfigChangedListener = new OnPropertiesChangedListener() { @Override @@ -1179,6 +1191,10 @@ final class ActivityManagerConstants extends ContentObserver { case KEY_MAX_PREVIOUS_TIME: updateMaxPreviousTime(); break; + case KEY_USE_TIERED_CACHED_ADJ: + case KEY_TIERED_CACHED_ADJ_DECAY_TIME: + updateUseTieredCachedAdj(); + break; default: break; } @@ -1924,6 +1940,17 @@ final class ActivityManagerConstants extends ContentObserver { DEFAULT_ENABLE_WAIT_FOR_FINISH_ATTACH_APPLICATION); } + private void updateUseTieredCachedAdj() { + USE_TIERED_CACHED_ADJ = DeviceConfig.getBoolean( + DeviceConfig.NAMESPACE_ACTIVITY_MANAGER, + KEY_USE_TIERED_CACHED_ADJ, + DEFAULT_USE_TIERED_CACHED_ADJ); + TIERED_CACHED_ADJ_DECAY_TIME = DeviceConfig.getLong( + DeviceConfig.NAMESPACE_ACTIVITY_MANAGER, + KEY_TIERED_CACHED_ADJ_DECAY_TIME, + DEFAULT_TIERED_CACHED_ADJ_DECAY_TIME); + } + @NeverCompile // Avoid size overhead of debugging code. void dump(PrintWriter pw) { pw.println("ACTIVITY MANAGER SETTINGS (dumpsys activity settings) " @@ -2108,6 +2135,11 @@ final class ActivityManagerConstants extends ContentObserver { pw.print(" "); pw.print(KEY_SHORT_FGS_ANR_EXTRA_WAIT_DURATION); pw.print("="); pw.println(mShortFgsAnrExtraWaitDuration); + pw.print(" "); pw.print(KEY_USE_TIERED_CACHED_ADJ); + pw.print("="); pw.println(USE_TIERED_CACHED_ADJ); + pw.print(" "); pw.print(KEY_TIERED_CACHED_ADJ_DECAY_TIME); + pw.print("="); pw.println(TIERED_CACHED_ADJ_DECAY_TIME); + pw.println(); if (mOverrideMaxCachedProcesses >= 0) { pw.print(" mOverrideMaxCachedProcesses="); pw.println(mOverrideMaxCachedProcesses); diff --git a/services/core/java/com/android/server/am/OomAdjuster.java b/services/core/java/com/android/server/am/OomAdjuster.java index baa2f8e46dd89..b68d993f2b701 100644 --- a/services/core/java/com/android/server/am/OomAdjuster.java +++ b/services/core/java/com/android/server/am/OomAdjuster.java @@ -1073,135 +1073,165 @@ public class OomAdjuster { @GuardedBy({"mService", "mProcLock"}) private void assignCachedAdjIfNecessary(ArrayList lruList) { final int numLru = lruList.size(); + if (mConstants.USE_TIERED_CACHED_ADJ) { + final long now = SystemClock.uptimeMillis(); + for (int i = numLru - 1; i >= 0; i--) { + ProcessRecord app = lruList.get(i); + final ProcessStateRecord state = app.mState; + final ProcessCachedOptimizerRecord opt = app.mOptRecord; + if (!app.isKilledByAm() && app.getThread() != null && state.getCurAdj() + >= UNKNOWN_ADJ) { + final ProcessServiceRecord psr = app.mServices; + int targetAdj = CACHED_APP_MIN_ADJ; - // First update the OOM adjustment for each of the - // application processes based on their current state. - int curCachedAdj = CACHED_APP_MIN_ADJ; - int nextCachedAdj = curCachedAdj + (CACHED_APP_IMPORTANCE_LEVELS * 2); - int curCachedImpAdj = 0; - int curEmptyAdj = CACHED_APP_MIN_ADJ + CACHED_APP_IMPORTANCE_LEVELS; - int nextEmptyAdj = curEmptyAdj + (CACHED_APP_IMPORTANCE_LEVELS * 2); + if (opt != null && opt.isFreezeExempt()) { + // BIND_WAIVE_PRIORITY and the like get oom_adj 900 + targetAdj += 0; + } else if ((state.getSetAdj() >= CACHED_APP_MIN_ADJ) + && (state.getLastStateTime() + + mConstants.TIERED_CACHED_ADJ_DECAY_TIME) < now) { + // Older cached apps get 950 + targetAdj += 50; + } else { + // Newer cached apps get 910 + targetAdj += 10; + } + state.setCurRawAdj(targetAdj); + state.setCurAdj(psr.modifyRawOomAdj(targetAdj)); + } + } + } else { + // First update the OOM adjustment for each of the + // application processes based on their current state. + int curCachedAdj = CACHED_APP_MIN_ADJ; + int nextCachedAdj = curCachedAdj + (CACHED_APP_IMPORTANCE_LEVELS * 2); + int curCachedImpAdj = 0; + int curEmptyAdj = CACHED_APP_MIN_ADJ + CACHED_APP_IMPORTANCE_LEVELS; + int nextEmptyAdj = curEmptyAdj + (CACHED_APP_IMPORTANCE_LEVELS * 2); - final int emptyProcessLimit = mConstants.CUR_MAX_EMPTY_PROCESSES; - final int cachedProcessLimit = mConstants.CUR_MAX_CACHED_PROCESSES - - emptyProcessLimit; - // Let's determine how many processes we have running vs. - // how many slots we have for background processes; we may want - // to put multiple processes in a slot of there are enough of - // them. - int numEmptyProcs = numLru - mNumNonCachedProcs - mNumCachedHiddenProcs; - if (numEmptyProcs > cachedProcessLimit) { - // If there are more empty processes than our limit on cached - // processes, then use the cached process limit for the factor. - // This ensures that the really old empty processes get pushed - // down to the bottom, so if we are running low on memory we will - // have a better chance at keeping around more cached processes - // instead of a gazillion empty processes. - numEmptyProcs = cachedProcessLimit; - } - int cachedFactor = (mNumCachedHiddenProcs > 0 ? (mNumCachedHiddenProcs + mNumSlots - 1) : 1) - / mNumSlots; - if (cachedFactor < 1) cachedFactor = 1; + final int emptyProcessLimit = mConstants.CUR_MAX_EMPTY_PROCESSES; + final int cachedProcessLimit = mConstants.CUR_MAX_CACHED_PROCESSES + - emptyProcessLimit; + // Let's determine how many processes we have running vs. + // how many slots we have for background processes; we may want + // to put multiple processes in a slot of there are enough of + // them. + int numEmptyProcs = numLru - mNumNonCachedProcs - mNumCachedHiddenProcs; + if (numEmptyProcs > cachedProcessLimit) { + // If there are more empty processes than our limit on cached + // processes, then use the cached process limit for the factor. + // This ensures that the really old empty processes get pushed + // down to the bottom, so if we are running low on memory we will + // have a better chance at keeping around more cached processes + // instead of a gazillion empty processes. + numEmptyProcs = cachedProcessLimit; + } + int cachedFactor = (mNumCachedHiddenProcs > 0 + ? (mNumCachedHiddenProcs + mNumSlots - 1) : 1) + / mNumSlots; + if (cachedFactor < 1) cachedFactor = 1; - int emptyFactor = (numEmptyProcs + mNumSlots - 1) / mNumSlots; - if (emptyFactor < 1) emptyFactor = 1; + int emptyFactor = (numEmptyProcs + mNumSlots - 1) / mNumSlots; + if (emptyFactor < 1) emptyFactor = 1; - int stepCached = -1; - int stepEmpty = -1; - int lastCachedGroup = 0; - int lastCachedGroupImportance = 0; - int lastCachedGroupUid = 0; + int stepCached = -1; + int stepEmpty = -1; + int lastCachedGroup = 0; + int lastCachedGroupImportance = 0; + int lastCachedGroupUid = 0; - for (int i = numLru - 1; i >= 0; i--) { - ProcessRecord app = lruList.get(i); - final ProcessStateRecord state = app.mState; - // If we haven't yet assigned the final cached adj - // to the process, do that now. - if (!app.isKilledByAm() && app.getThread() != null && state.getCurAdj() + + for (int i = numLru - 1; i >= 0; i--) { + ProcessRecord app = lruList.get(i); + final ProcessStateRecord state = app.mState; + // If we haven't yet assigned the final cached adj + // to the process, do that now. + if (!app.isKilledByAm() && app.getThread() != null && state.getCurAdj() >= UNKNOWN_ADJ) { - final ProcessServiceRecord psr = app.mServices; - switch (state.getCurProcState()) { - case PROCESS_STATE_CACHED_ACTIVITY: - case ActivityManager.PROCESS_STATE_CACHED_ACTIVITY_CLIENT: - case ActivityManager.PROCESS_STATE_CACHED_RECENT: - // Figure out the next cached level, taking into account groups. - boolean inGroup = false; - final int connectionGroup = psr.getConnectionGroup(); - if (connectionGroup != 0) { - final int connectionImportance = psr.getConnectionImportance(); - if (lastCachedGroupUid == app.uid + final ProcessServiceRecord psr = app.mServices; + switch (state.getCurProcState()) { + case PROCESS_STATE_CACHED_ACTIVITY: + case ActivityManager.PROCESS_STATE_CACHED_ACTIVITY_CLIENT: + case ActivityManager.PROCESS_STATE_CACHED_RECENT: + // Figure out the next cached level, taking into account groups. + boolean inGroup = false; + final int connectionGroup = psr.getConnectionGroup(); + if (connectionGroup != 0) { + final int connectionImportance = psr.getConnectionImportance(); + if (lastCachedGroupUid == app.uid && lastCachedGroup == connectionGroup) { - // This is in the same group as the last process, just tweak - // adjustment by importance. - if (connectionImportance > lastCachedGroupImportance) { - lastCachedGroupImportance = connectionImportance; - if (curCachedAdj < nextCachedAdj + // This is in the same group as the last process, just tweak + // adjustment by importance. + if (connectionImportance > lastCachedGroupImportance) { + lastCachedGroupImportance = connectionImportance; + if (curCachedAdj < nextCachedAdj && curCachedAdj < CACHED_APP_MAX_ADJ) { - curCachedImpAdj++; + curCachedImpAdj++; + } + } + inGroup = true; + } else { + lastCachedGroupUid = app.uid; + lastCachedGroup = connectionGroup; + lastCachedGroupImportance = connectionImportance; + } + } + if (!inGroup && curCachedAdj != nextCachedAdj) { + stepCached++; + curCachedImpAdj = 0; + if (stepCached >= cachedFactor) { + stepCached = 0; + curCachedAdj = nextCachedAdj; + nextCachedAdj += CACHED_APP_IMPORTANCE_LEVELS * 2; + if (nextCachedAdj > CACHED_APP_MAX_ADJ) { + nextCachedAdj = CACHED_APP_MAX_ADJ; } } - inGroup = true; - } else { - lastCachedGroupUid = app.uid; - lastCachedGroup = connectionGroup; - lastCachedGroupImportance = connectionImportance; } - } - if (!inGroup && curCachedAdj != nextCachedAdj) { - stepCached++; - curCachedImpAdj = 0; - if (stepCached >= cachedFactor) { - stepCached = 0; - curCachedAdj = nextCachedAdj; - nextCachedAdj += CACHED_APP_IMPORTANCE_LEVELS * 2; - if (nextCachedAdj > CACHED_APP_MAX_ADJ) { - nextCachedAdj = CACHED_APP_MAX_ADJ; + // This process is a cached process holding activities... + // assign it the next cached value for that type, and then + // step that cached level. + state.setCurRawAdj(curCachedAdj + curCachedImpAdj); + state.setCurAdj(psr.modifyRawOomAdj(curCachedAdj + curCachedImpAdj)); + if (DEBUG_LRU) { + Slog.d(TAG_LRU, "Assigning activity LRU #" + i + + " adj: " + state.getCurAdj() + + " (curCachedAdj=" + curCachedAdj + + " curCachedImpAdj=" + curCachedImpAdj + ")"); + } + break; + default: + // Figure out the next cached level. + if (curEmptyAdj != nextEmptyAdj) { + stepEmpty++; + if (stepEmpty >= emptyFactor) { + stepEmpty = 0; + curEmptyAdj = nextEmptyAdj; + nextEmptyAdj += CACHED_APP_IMPORTANCE_LEVELS * 2; + if (nextEmptyAdj > CACHED_APP_MAX_ADJ) { + nextEmptyAdj = CACHED_APP_MAX_ADJ; + } } } - } - // This process is a cached process holding activities... - // assign it the next cached value for that type, and then - // step that cached level. - state.setCurRawAdj(curCachedAdj + curCachedImpAdj); - state.setCurAdj(psr.modifyRawOomAdj(curCachedAdj + curCachedImpAdj)); - if (DEBUG_LRU) { - Slog.d(TAG_LRU, "Assigning activity LRU #" + i - + " adj: " + state.getCurAdj() - + " (curCachedAdj=" + curCachedAdj - + " curCachedImpAdj=" + curCachedImpAdj + ")"); - } - break; - default: - // Figure out the next cached level. - if (curEmptyAdj != nextEmptyAdj) { - stepEmpty++; - if (stepEmpty >= emptyFactor) { - stepEmpty = 0; - curEmptyAdj = nextEmptyAdj; - nextEmptyAdj += CACHED_APP_IMPORTANCE_LEVELS * 2; - if (nextEmptyAdj > CACHED_APP_MAX_ADJ) { - nextEmptyAdj = CACHED_APP_MAX_ADJ; - } + // For everything else, assign next empty cached process + // level and bump that up. Note that this means that + // long-running services that have dropped down to the + // cached level will be treated as empty (since their process + // state is still as a service), which is what we want. + state.setCurRawAdj(curEmptyAdj); + state.setCurAdj(psr.modifyRawOomAdj(curEmptyAdj)); + if (DEBUG_LRU) { + Slog.d(TAG_LRU, "Assigning empty LRU #" + i + + " adj: " + state.getCurAdj() + + " (curEmptyAdj=" + curEmptyAdj + + ")"); } - } - // For everything else, assign next empty cached process - // level and bump that up. Note that this means that - // long-running services that have dropped down to the - // cached level will be treated as empty (since their process - // state is still as a service), which is what we want. - state.setCurRawAdj(curEmptyAdj); - state.setCurAdj(psr.modifyRawOomAdj(curEmptyAdj)); - if (DEBUG_LRU) { - Slog.d(TAG_LRU, "Assigning empty LRU #" + i - + " adj: " + state.getCurAdj() + " (curEmptyAdj=" + curEmptyAdj - + ")"); - } - break; + break; + } } } } } - private long mNextNoKillDebugMessageTime; private double mLastFreeSwapPercent = 1.00; 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 dc7f3cdc1f7f5..485ce33dfb7d5 100644 --- a/services/tests/mockingservicestests/src/com/android/server/am/MockingOomAdjusterTests.java +++ b/services/tests/mockingservicestests/src/com/android/server/am/MockingOomAdjusterTests.java @@ -143,8 +143,8 @@ public class MockingOomAdjusterTests { private static final String MOCKAPP5_PROCESSNAME = "test #5"; private static final String MOCKAPP5_PACKAGENAME = "com.android.test.test5"; private static final int MOCKAPP2_UID_OTHER = MOCKAPP2_UID + UserHandle.PER_USER_RANGE; - private static final int FIRST_CACHED_ADJ = ProcessList.CACHED_APP_MIN_ADJ - + ProcessList.CACHED_APP_IMPORTANCE_LEVELS; + private static int sFirstCachedAdj = ProcessList.CACHED_APP_MIN_ADJ + + ProcessList.CACHED_APP_IMPORTANCE_LEVELS; private static Context sContext; private static PackageManagerInternal sPackageManagerInternal; private static ActivityManagerService sService; @@ -208,6 +208,9 @@ public class MockingOomAdjusterTests { new ActiveUids(sService, false)); sService.mOomAdjuster.mAdjSeq = 10000; sService.mWakefulness = new AtomicInteger(PowerManagerInternal.WAKEFULNESS_AWAKE); + if (sService.mConstants.USE_TIERED_CACHED_ADJ) { + sFirstCachedAdj = ProcessList.CACHED_APP_MIN_ADJ + 10; + } } @AfterClass @@ -834,7 +837,7 @@ public class MockingOomAdjusterTests { updateOomAdj(client, app); doReturn(null).when(sService).getTopApp(); - assertProcStates(app, PROCESS_STATE_SERVICE, FIRST_CACHED_ADJ, SCHED_GROUP_BACKGROUND); + assertProcStates(app, PROCESS_STATE_SERVICE, sFirstCachedAdj, SCHED_GROUP_BACKGROUND); } @SuppressWarnings("GuardedBy") @@ -882,7 +885,7 @@ public class MockingOomAdjusterTests { sService.mWakefulness.set(PowerManagerInternal.WAKEFULNESS_AWAKE); updateOomAdj(app); - assertProcStates(app, PROCESS_STATE_CACHED_EMPTY, FIRST_CACHED_ADJ, SCHED_GROUP_BACKGROUND); + assertProcStates(app, PROCESS_STATE_CACHED_EMPTY, sFirstCachedAdj, SCHED_GROUP_BACKGROUND); } @SuppressWarnings("GuardedBy") @@ -1286,7 +1289,7 @@ public class MockingOomAdjusterTests { bindProvider(app, app, null, null, false); updateOomAdj(app); - assertProcStates(app, PROCESS_STATE_CACHED_EMPTY, FIRST_CACHED_ADJ, SCHED_GROUP_BACKGROUND); + assertProcStates(app, PROCESS_STATE_CACHED_EMPTY, sFirstCachedAdj, SCHED_GROUP_BACKGROUND); } @SuppressWarnings("GuardedBy") @@ -1301,7 +1304,7 @@ public class MockingOomAdjusterTests { sService.mWakefulness.set(PowerManagerInternal.WAKEFULNESS_AWAKE); updateOomAdj(app, client); - assertProcStates(app, PROCESS_STATE_CACHED_EMPTY, FIRST_CACHED_ADJ, SCHED_GROUP_BACKGROUND); + assertProcStates(app, PROCESS_STATE_CACHED_EMPTY, sFirstCachedAdj, SCHED_GROUP_BACKGROUND); } @SuppressWarnings("GuardedBy") @@ -2378,8 +2381,12 @@ public class MockingOomAdjusterTests { MOCKAPP2_PROCESSNAME, MOCKAPP2_PACKAGENAME, false)); final int userOwner = 0; final int userOther = 1; - final int cachedAdj1 = CACHED_APP_MIN_ADJ + ProcessList.CACHED_APP_IMPORTANCE_LEVELS; - final int cachedAdj2 = cachedAdj1 + ProcessList.CACHED_APP_IMPORTANCE_LEVELS * 2; + final int cachedAdj1 = sService.mConstants.USE_TIERED_CACHED_ADJ + ? CACHED_APP_MIN_ADJ + 10 + : CACHED_APP_MIN_ADJ + ProcessList.CACHED_APP_IMPORTANCE_LEVELS; + final int cachedAdj2 = sService.mConstants.USE_TIERED_CACHED_ADJ + ? CACHED_APP_MIN_ADJ + 10 + : cachedAdj1 + ProcessList.CACHED_APP_IMPORTANCE_LEVELS * 2; doReturn(userOwner).when(sService.mUserController).getCurrentUserId(); final ArrayList lru = sService.mProcessList.getLruProcessesLOSP(); From f760f7f10d26588e52c38fcdf7cb8b68c723c5ad Mon Sep 17 00:00:00 2001 From: Tim Murray Date: Fri, 10 Feb 2023 17:08:04 -0800 Subject: [PATCH 3/3] Freezer: release AM lock while checking /proc/locks Checking /proc/locks can take a long time if new file locks are being added. Hoist that outside of the AM lock to make sure that system_server doesn't stall unnecessarily. Test: device functions with short freezer timeout Bug: 269207437 Change-Id: I5bab77281992d0e826427e458025103500fc0869 Merged-in: I5bab77281992d0e826427e458025103500fc0869 --- .../android/server/am/CachedAppOptimizer.java | 64 +++++++++++-------- 1 file changed, 37 insertions(+), 27 deletions(-) diff --git a/services/core/java/com/android/server/am/CachedAppOptimizer.java b/services/core/java/com/android/server/am/CachedAppOptimizer.java index 913f151496467..f4685f0f6125f 100644 --- a/services/core/java/com/android/server/am/CachedAppOptimizer.java +++ b/services/core/java/com/android/server/am/CachedAppOptimizer.java @@ -1921,9 +1921,26 @@ public final class CachedAppOptimizer { public void handleMessage(Message msg) { switch (msg.what) { case SET_FROZEN_PROCESS_MSG: + { + ProcessRecord proc = (ProcessRecord) msg.obj; + int pid = proc.getPid(); + final String name = proc.processName; synchronized (mAm) { - freezeProcess((ProcessRecord) msg.obj); + freezeProcess(proc); } + try { + // post-check to prevent deadlock + mProcLocksReader.handleBlockingFileLocks(this); + } catch (Exception e) { + Slog.e(TAG_AM, "Unable to check file locks for " + + name + "(" + pid + "): " + e); + synchronized (mAm) { + synchronized (mProcLock) { + unfreezeAppLSP(proc, OomAdjuster.OOM_ADJ_REASON_NONE); + } + } + } + } break; case REPORT_UNFREEZE_MSG: int pid = msg.arg1; @@ -2057,16 +2074,6 @@ public final class CachedAppOptimizer { } }); } - - try { - // post-check to prevent deadlock - mProcLocksReader.handleBlockingFileLocks(this); - } catch (Exception e) { - Slog.e(TAG_AM, "Unable to check file locks for " + name + "(" + pid + "): " + e); - synchronized (mProcLock) { - unfreezeAppLSP(proc, OomAdjuster.OOM_ADJ_REASON_NONE); - } - } } private void reportUnfreeze(int pid, int frozenDuration, String processName, @@ -2123,22 +2130,25 @@ public final class CachedAppOptimizer { if (DEBUG_FREEZER) { Slog.d(TAG_AM, "Blocking file lock found: " + pids); } - synchronized (mProcLock) { - int pid = pids.get(0); - ProcessRecord app = mFrozenProcesses.get(pid); - ProcessRecord pr; - if (app != null) { - for (int i = 1; i < pids.size(); i++) { - int blocked = pids.get(i); - synchronized (mAm.mPidsSelfLocked) { - pr = mAm.mPidsSelfLocked.get(blocked); - } - if (pr != null && pr.mState.getCurAdj() < ProcessList.CACHED_APP_MIN_ADJ) { - Slog.d(TAG_AM, app.processName + " (" + pid + ") blocks " - + pr.processName + " (" + blocked + ")"); - // Found at least one blocked non-cached process - unfreezeAppLSP(app, OomAdjuster.OOM_ADJ_REASON_NONE); - break; + synchronized (mAm) { + synchronized (mProcLock) { + int pid = pids.get(0); + ProcessRecord app = mFrozenProcesses.get(pid); + ProcessRecord pr; + if (app != null) { + for (int i = 1; i < pids.size(); i++) { + int blocked = pids.get(i); + synchronized (mAm.mPidsSelfLocked) { + pr = mAm.mPidsSelfLocked.get(blocked); + } + if (pr != null + && pr.mState.getCurAdj() < ProcessList.CACHED_APP_MIN_ADJ) { + Slog.d(TAG_AM, app.processName + " (" + pid + ") blocks " + + pr.processName + " (" + blocked + ")"); + // Found at least one blocked non-cached process + unfreezeAppLSP(app, OomAdjuster.OOM_ADJ_REASON_NONE); + break; + } } } }