From c8a02c5afb3412ee766c8f7080b447ab1e7b19ab Mon Sep 17 00:00:00 2001 From: Edgar Arriaga Date: Wed, 23 Mar 2022 17:19:07 -0700 Subject: [PATCH 1/7] Add threshold to downgrade compaction when swap is low This patch introduces a threshold that allows compaction system to downgrade full compactions into file only compactions which do not cause compactions to happen as a way to reduce pressure on swap thus aiming for a reduction in overall cpu usage during high memory pressure scenarios as finding new pages will likely be harder when the swap is low. Bug: 226458732 Test: Manual. Verified that compaction runs artificially using different thresholds and logging. Change-Id: I12b460174c857f6750c9a173997c66381e4a1dc3 --- .../android/server/am/CachedAppOptimizer.java | 22 +++++++++++++++++++ ...m_android_server_am_CachedAppOptimizer.cpp | 13 +++++++++++ 2 files changed, 35 insertions(+) diff --git a/services/core/java/com/android/server/am/CachedAppOptimizer.java b/services/core/java/com/android/server/am/CachedAppOptimizer.java index ff569a681a4e4..d0f9e3d6c11c5 100644 --- a/services/core/java/com/android/server/am/CachedAppOptimizer.java +++ b/services/core/java/com/android/server/am/CachedAppOptimizer.java @@ -152,6 +152,11 @@ public final class CachedAppOptimizer { static final int SET_FROZEN_PROCESS_MSG = 3; static final int REPORT_UNFREEZE_MSG = 4; + // When free swap falls below this percentage threshold any full (file + anon) + // compactions will be downgraded to file only compactions to reduce pressure + // on swap resources as file. + static final double COMPACT_DOWNGRADE_FREE_SWAP_THRESHOLD = 0.2; + static final int DO_FREEZE = 1; static final int REPORT_UNFREEZE = 2; @@ -544,6 +549,11 @@ public final class CachedAppOptimizer { static private native void cancelCompaction(); + /** + * Retrieves the free swap percentage. + */ + static private native double getFreeSwapPercent(); + /** * Reads the flag value from DeviceConfig to determine whether app compaction * should be enabled, and starts the freeze/compaction thread if needed. @@ -1338,6 +1348,18 @@ public final class CachedAppOptimizer { default: break; } + // Downgrade compaction if facing swap memory pressure + if (action.equals(mCompactActionFull)) { + double swapUsagePercent = getFreeSwapPercent(); + if (swapUsagePercent < COMPACT_DOWNGRADE_FREE_SWAP_THRESHOLD) { + Slog.d(TAG_AM, + "Downgraded compaction to file only due to low swap." + + " Swap Free% " + swapUsagePercent); + action = mCompactActionSome; + pendingAction = COMPACT_PROCESS_SOME; + } + } + try { Trace.traceBegin(Trace.TRACE_TAG_ACTIVITY_MANAGER, "Compact " + ((pendingAction == COMPACT_PROCESS_SOME) ? "some" : "full") diff --git a/services/core/jni/com_android_server_am_CachedAppOptimizer.cpp b/services/core/jni/com_android_server_am_CachedAppOptimizer.cpp index 636ca4143a33c..6138e840a507b 100644 --- a/services/core/jni/com_android_server_am_CachedAppOptimizer.cpp +++ b/services/core/jni/com_android_server_am_CachedAppOptimizer.cpp @@ -37,6 +37,7 @@ #include #include #include +#include #include #include @@ -305,6 +306,16 @@ static void com_android_server_am_CachedAppOptimizer_cancelCompaction(JNIEnv*, j } } +static jdouble com_android_server_am_CachedAppOptimizer_getFreeSwapPercent(JNIEnv*, jobject) { + struct sysinfo memoryInfo; + int error = sysinfo(&memoryInfo); + if(error == -1) { + LOG(ERROR) << "Could not check free swap space"; + return 0; + } + return (double)memoryInfo.freeswap / (double)memoryInfo.totalswap; +} + static void com_android_server_am_CachedAppOptimizer_compactProcess(JNIEnv*, jobject, jint pid, jint compactionFlags) { compactProcessOrFallback(pid, compactionFlags); @@ -358,6 +369,8 @@ static const JNINativeMethod sMethods[] = { /* name, signature, funcPtr */ {"cancelCompaction", "()V", (void*)com_android_server_am_CachedAppOptimizer_cancelCompaction}, + {"getFreeSwapPercent", "()D", + (void*)com_android_server_am_CachedAppOptimizer_getFreeSwapPercent}, {"compactSystem", "()V", (void*)com_android_server_am_CachedAppOptimizer_compactSystem}, {"compactProcess", "(II)V", (void*)com_android_server_am_CachedAppOptimizer_compactProcess}, {"freezeBinder", "(IZ)I", (void*)com_android_server_am_CachedAppOptimizer_freezeBinder}, From 720a6a5a2130d6bccdb4951431c43f51e117de26 Mon Sep 17 00:00:00 2001 From: Edgar Arriaga Date: Wed, 23 Mar 2022 17:59:37 -0700 Subject: [PATCH 2/7] Add some trace points to compaction batches and cancellation Adding a few trace points that will help to shed some light into some field traces that show long compaction running times to determine whether cancellation is not properly working in the field or if the batch sizes are too big. Bug: 226463719 Test: Verified trace points in perfetto. Change-Id: I362044a38ffc5e820b7fddf6cb08fa2325e6929b --- .../com/android/server/am/CachedAppOptimizer.java | 12 ++++++++++++ .../jni/com_android_server_am_CachedAppOptimizer.cpp | 8 ++++++++ services/tests/mockingservicestests/jni/Android.bp | 1 + 3 files changed, 21 insertions(+) diff --git a/services/core/java/com/android/server/am/CachedAppOptimizer.java b/services/core/java/com/android/server/am/CachedAppOptimizer.java index d0f9e3d6c11c5..792b697e380e7 100644 --- a/services/core/java/com/android/server/am/CachedAppOptimizer.java +++ b/services/core/java/com/android/server/am/CachedAppOptimizer.java @@ -101,6 +101,8 @@ public final class CachedAppOptimizer { private static final int COMPACT_ACTION_FILE_FLAG = 1; private static final int COMPACT_ACTION_ANON_FLAG = 2; + private static final String ATRACE_COMPACTION_TRACK = "Compaction"; + // Defaults for phenotype flags. @VisibleForTesting static final Boolean DEFAULT_USE_COMPACTION = false; @VisibleForTesting static final Boolean DEFAULT_USE_FREEZER = true; @@ -462,6 +464,8 @@ public final class CachedAppOptimizer { void compactAppSome(ProcessRecord app) { app.mOptRecord.setReqCompactAction(COMPACT_PROCESS_SOME); if (!app.mOptRecord.hasPendingCompact()) { + Trace.instantForTrack(Trace.TRACE_TAG_ACTIVITY_MANAGER, ATRACE_COMPACTION_TRACK, + "compactAppSome " + app.processName != null ? app.processName : ""); app.mOptRecord.setHasPendingCompact(true); mPendingCompactionProcesses.add(app); mCompactionHandler.sendMessage( @@ -479,6 +483,8 @@ public final class CachedAppOptimizer { && app.mState.getCurAdj() <= mCompactThrottleMaxOomAdj) { app.mOptRecord.setReqCompactAction(COMPACT_PROCESS_FULL); if (!app.mOptRecord.hasPendingCompact()) { + Trace.instantForTrack(Trace.TRACE_TAG_ACTIVITY_MANAGER, ATRACE_COMPACTION_TRACK, + "compactAppFull " + app.processName != null ? app.processName : ""); app.mOptRecord.setHasPendingCompact(true); mPendingCompactionProcesses.add(app); mCompactionHandler.sendMessage( @@ -498,6 +504,8 @@ public final class CachedAppOptimizer { void compactAppPersistent(ProcessRecord app) { app.mOptRecord.setReqCompactAction(COMPACT_PROCESS_PERSISTENT); if (!app.mOptRecord.hasPendingCompact()) { + Trace.instantForTrack(Trace.TRACE_TAG_ACTIVITY_MANAGER, ATRACE_COMPACTION_TRACK, + "compactAppPersistent " + app.processName != null ? app.processName : ""); app.mOptRecord.setHasPendingCompact(true); mPendingCompactionProcesses.add(app); mCompactionHandler.sendMessage( @@ -516,6 +524,8 @@ public final class CachedAppOptimizer { void compactAppBfgs(ProcessRecord app) { app.mOptRecord.setReqCompactAction(COMPACT_PROCESS_BFGS); if (!app.mOptRecord.hasPendingCompact()) { + Trace.instantForTrack(Trace.TRACE_TAG_ACTIVITY_MANAGER, ATRACE_COMPACTION_TRACK, + "compactAppBfgs " + app.processName != null ? app.processName : ""); app.mOptRecord.setHasPendingCompact(true); mPendingCompactionProcesses.add(app); mCompactionHandler.sendMessage( @@ -532,6 +542,8 @@ public final class CachedAppOptimizer { void compactAllSystem() { if (useCompaction()) { + Trace.instantForTrack( + Trace.TRACE_TAG_ACTIVITY_MANAGER, ATRACE_COMPACTION_TRACK, "compactAllSystem"); mCompactionHandler.sendMessage(mCompactionHandler.obtainMessage( COMPACT_SYSTEM_MSG)); } diff --git a/services/core/jni/com_android_server_am_CachedAppOptimizer.cpp b/services/core/jni/com_android_server_am_CachedAppOptimizer.cpp index 6138e840a507b..8664c38af816c 100644 --- a/services/core/jni/com_android_server_am_CachedAppOptimizer.cpp +++ b/services/core/jni/com_android_server_am_CachedAppOptimizer.cpp @@ -16,6 +16,8 @@ #define LOG_TAG "CachedAppOptimizer" //#define LOG_NDEBUG 0 +#define ATRACE_TAG ATRACE_TAG_ACTIVITY_MANAGER +#define ATRACE_COMPACTION_TRACK "Compaction" #include #include @@ -40,6 +42,7 @@ #include #include #include +#include #include @@ -115,6 +118,8 @@ static int64_t compactMemory(const std::vector& vmas, int pid, int madviseT // is requested and when it is handled during this time our // OOM adjust could have improved. LOG(DEBUG) << "Cancelled running compaction for " << pid; + ATRACE_INSTANT_FOR_TRACK(ATRACE_COMPACTION_TRACK, + StringPrintf("Cancelled compaction for %d", pid).c_str()); break; } @@ -150,7 +155,9 @@ static int64_t compactMemory(const std::vector& vmas, int pid, int madviseT break; } + ATRACE_BEGIN(StringPrintf("Compact %d VMAs", iVec).c_str()); auto bytesProcessed = process_madvise(pidfd, vmasToKernel, iVec, madviseType, 0); + ATRACE_END(); if (CC_UNLIKELY(bytesProcessed == -1)) { if (errno == EINVAL) { @@ -303,6 +310,7 @@ static void com_android_server_am_CachedAppOptimizer_compactSystem(JNIEnv *, job static void com_android_server_am_CachedAppOptimizer_cancelCompaction(JNIEnv*, jobject) { if (compactionInProgress) { cancelRunningCompaction = true; + ATRACE_INSTANT_FOR_TRACK(ATRACE_COMPACTION_TRACK, "Cancel compaction"); } } diff --git a/services/tests/mockingservicestests/jni/Android.bp b/services/tests/mockingservicestests/jni/Android.bp index 89b204b9c999f..f454ac7e9e4b3 100644 --- a/services/tests/mockingservicestests/jni/Android.bp +++ b/services/tests/mockingservicestests/jni/Android.bp @@ -44,6 +44,7 @@ cc_library_shared { "libnativehelper", "libprocessgroup", "libutils", + "libcutils", "android.hardware.graphics.bufferqueue@1.0", "android.hardware.graphics.bufferqueue@2.0", "android.hardware.graphics.common@1.2", From b0ef1944fb8527ed2366cdeb2f7d7516b1acd8f4 Mon Sep 17 00:00:00 2001 From: Edgar Arriaga Date: Mon, 28 Mar 2022 16:47:48 -0700 Subject: [PATCH 3/7] Improve compaction debugging with extra adb command to force compaction This CL adds am compact command that allows forcing compactions this is specially useful for debugging compaction flows as well and it also allows for instrumentation to manually trigger compactions if needed and it also reduces some technical debt to allow forcing compaction code to be manageable. Bug: 226463719 Test: adb shell am compact some|full Change-Id: I67af76a212611260bb226e74e48eb7f495341f12 --- .../am/ActivityManagerShellCommand.java | 36 ++ .../android/server/am/CachedAppOptimizer.java | 474 +++++++++++------- .../am/ProcessCachedOptimizerRecord.java | 15 + .../server/am/CachedAppOptimizerTest.java | 83 ++- 4 files changed, 414 insertions(+), 194 deletions(-) diff --git a/services/core/java/com/android/server/am/ActivityManagerShellCommand.java b/services/core/java/com/android/server/am/ActivityManagerShellCommand.java index 71ae92aecbcfe..27facccfaf42d 100644 --- a/services/core/java/com/android/server/am/ActivityManagerShellCommand.java +++ b/services/core/java/com/android/server/am/ActivityManagerShellCommand.java @@ -219,6 +219,8 @@ final class ActivityManagerShellCommand extends ShellCommand { return runStopService(pw); case "broadcast": return runSendBroadcast(pw); + case "compact": + return runCompact(pw); case "instrument": getOutPrintWriter().println("Error: must be invoked through 'am instrument'."); return -1; @@ -966,6 +968,36 @@ final class ActivityManagerShellCommand extends ShellCommand { return 0; } + @NeverCompile + int runCompact(PrintWriter pw) { + String processName = getNextArgRequired(); + String uid = getNextArgRequired(); + String op = getNextArgRequired(); + ProcessRecord app; + synchronized (mInternal.mProcLock) { + app = mInternal.getProcessRecordLocked(processName, Integer.parseInt(uid)); + } + pw.println("Process record found pid: " + app.mPid); + if (op.equals("full")) { + pw.println("Executing full compaction for " + app.mPid); + synchronized (mInternal.mProcLock) { + mInternal.mOomAdjuster.mCachedAppOptimizer.compactAppFull(app, true); + } + pw.println("Finished full compaction for " + app.mPid); + } else if (op.equals("some")) { + pw.println("Executing some compaction for " + app.mPid); + synchronized (mInternal.mProcLock) { + mInternal.mOomAdjuster.mCachedAppOptimizer.compactAppSome(app, true); + } + pw.println("Finished some compaction for " + app.mPid); + } else { + getErrPrintWriter().println("Error: unknown compact command '" + op + "'"); + return -1; + } + + return 0; + } + int runDumpHeap(PrintWriter pw) throws RemoteException { final PrintWriter err = getErrPrintWriter(); boolean managed = true; @@ -3435,6 +3467,10 @@ final class ActivityManagerShellCommand extends ShellCommand { pw.println(" --allow-background-activity-starts: The receiver may start activities"); pw.println(" even if in the background."); pw.println(" --async: Send without waiting for the completion of the receiver."); + pw.println(" compact [some|full]"); + pw.println(" Force process compaction."); + pw.println(" some: execute file compaction."); + pw.println(" full: execute anon + file compaction."); pw.println(" instrument [-r] [-e ] [-p ] [-w]"); pw.println(" [--user | current]"); pw.println(" [--no-hidden-api-checks [--no-test-api-access]]"); diff --git a/services/core/java/com/android/server/am/CachedAppOptimizer.java b/services/core/java/com/android/server/am/CachedAppOptimizer.java index 792b697e380e7..e91b2b32f930e 100644 --- a/services/core/java/com/android/server/am/CachedAppOptimizer.java +++ b/services/core/java/com/android/server/am/CachedAppOptimizer.java @@ -88,6 +88,12 @@ public final class CachedAppOptimizer { @VisibleForTesting static final String KEY_FREEZER_DEBOUNCE_TIMEOUT = "freeze_debounce_timeout"; + // RSS Indices + private static final int RSS_TOTAL_INDEX = 0; + private static final int RSS_FILE_INDEX = 1; + private static final int RSS_ANON_INDEX = 2; + private static final int RSS_SWAP_INDEX = 3; + // Phenotype sends int configurations and we map them to the strings we'll use on device, // preventing a weird string value entering the kernel. private static final int COMPACT_ACTION_NONE = 0; @@ -106,8 +112,8 @@ public final class CachedAppOptimizer { // Defaults for phenotype flags. @VisibleForTesting static final Boolean DEFAULT_USE_COMPACTION = false; @VisibleForTesting static final Boolean DEFAULT_USE_FREEZER = true; - @VisibleForTesting static final int DEFAULT_COMPACT_ACTION_1 = COMPACT_ACTION_FILE; @VisibleForTesting static final int DEFAULT_COMPACT_ACTION_2 = COMPACT_ACTION_FULL; + @VisibleForTesting static final int DEFAULT_COMPACT_ACTION_1 = COMPACT_ACTION_FILE; @VisibleForTesting static final long DEFAULT_COMPACT_THROTTLE_1 = 5_000; @VisibleForTesting static final long DEFAULT_COMPACT_THROTTLE_2 = 10_000; @VisibleForTesting static final long DEFAULT_COMPACT_THROTTLE_3 = 500; @@ -447,6 +453,16 @@ public final class CachedAppOptimizer { pw.println(" " + app.mOptRecord.getFreezeUnfreezeTime() + ": " + app.getPid() + " " + app.processName); } + + if (!mPendingCompactionProcesses.isEmpty()) { + pw.println(" Pending compactions:"); + size = mPendingCompactionProcesses.size(); + for (int i = 0; i < size; i++) { + ProcessRecord app = mPendingCompactionProcesses.get(i); + pw.println(" pid: " + app.getPid() + ". name: " + app.processName + + ". hasPendingCompact: " + app.mOptRecord.hasPendingCompact()); + } + } } if (DEBUG_COMPACTION) { for (Map.Entry entry @@ -461,12 +477,16 @@ public final class CachedAppOptimizer { } @GuardedBy("mProcLock") - void compactAppSome(ProcessRecord app) { + void compactAppSome(ProcessRecord app, boolean force) { app.mOptRecord.setReqCompactAction(COMPACT_PROCESS_SOME); - if (!app.mOptRecord.hasPendingCompact()) { + if (DEBUG_COMPACTION) { + Slog.d(TAG_AM, " compactAppSome requested for " + app.processName + " force: " + force); + } + if (force || !app.mOptRecord.hasPendingCompact()) { Trace.instantForTrack(Trace.TRACE_TAG_ACTIVITY_MANAGER, ATRACE_COMPACTION_TRACK, "compactAppSome " + app.processName != null ? app.processName : ""); app.mOptRecord.setHasPendingCompact(true); + app.mOptRecord.setForceCompact(force); mPendingCompactionProcesses.add(app); mCompactionHandler.sendMessage( mCompactionHandler.obtainMessage( @@ -475,21 +495,31 @@ public final class CachedAppOptimizer { } @GuardedBy("mProcLock") - void compactAppFull(ProcessRecord app) { - // Apply OOM adj score throttle for Full App Compaction. - if ((app.mState.getSetAdj() < mCompactThrottleMinOomAdj - || app.mState.getSetAdj() > mCompactThrottleMaxOomAdj) + void compactAppFull(ProcessRecord app, boolean force) { + boolean oomAdjEnteredCached = (app.mState.getSetAdj() < mCompactThrottleMinOomAdj + || app.mState.getSetAdj() > mCompactThrottleMaxOomAdj) && app.mState.getCurAdj() >= mCompactThrottleMinOomAdj - && app.mState.getCurAdj() <= mCompactThrottleMaxOomAdj) { + && app.mState.getCurAdj() <= mCompactThrottleMaxOomAdj; + if (DEBUG_COMPACTION) { + Slog.d(TAG_AM, + " compactAppFull requested for " + app.processName + " force: " + force + + " oomAdjEnteredCached: " + oomAdjEnteredCached); + } + // Apply OOM adj score throttle for Full App Compaction. + if (force || oomAdjEnteredCached) { app.mOptRecord.setReqCompactAction(COMPACT_PROCESS_FULL); if (!app.mOptRecord.hasPendingCompact()) { Trace.instantForTrack(Trace.TRACE_TAG_ACTIVITY_MANAGER, ATRACE_COMPACTION_TRACK, "compactAppFull " + app.processName != null ? app.processName : ""); app.mOptRecord.setHasPendingCompact(true); + app.mOptRecord.setForceCompact(force); mPendingCompactionProcesses.add(app); - mCompactionHandler.sendMessage( - mCompactionHandler.obtainMessage( + mCompactionHandler.sendMessage(mCompactionHandler.obtainMessage( COMPACT_PROCESS_MSG, app.mState.getSetAdj(), app.mState.getSetProcState())); + } else if (DEBUG_COMPACTION) { + Slog.d(TAG_AM, + " compactAppFull Skipped for " + app.processName + + " since it has a pending compact"); } } else { if (DEBUG_COMPACTION) { @@ -1136,13 +1166,48 @@ public final class CachedAppOptimizer { // Perform a major compaction when any app enters cached if (oldAdj <= ProcessList.PERCEPTIBLE_APP_ADJ && (newAdj == ProcessList.PREVIOUS_APP_ADJ || newAdj == ProcessList.HOME_APP_ADJ)) { - compactAppSome(app); + compactAppSome(app, false); } else if (newAdj >= ProcessList.CACHED_APP_MIN_ADJ && newAdj <= ProcessList.CACHED_APP_MAX_ADJ) { - compactAppFull(app); + compactAppFull(app, false); } } + /** + * This method resolves which compaction method we should use for the proposed compaction. + */ + int resolveCompactionAction(int pendingAction) { + int resolvedAction; + + switch (pendingAction) { + case COMPACT_PROCESS_SOME: + resolvedAction = COMPACT_ACTION_FILE; + break; + // For the time being, treat these as equivalent. + case COMPACT_PROCESS_FULL: + case COMPACT_PROCESS_PERSISTENT: + case COMPACT_PROCESS_BFGS: + resolvedAction = COMPACT_ACTION_FULL; + break; + default: + resolvedAction = COMPACT_ACTION_NONE; + break; + } + + // Downgrade compaction if facing swap memory pressure + if (resolvedAction == COMPACT_ACTION_FULL) { + double swapUsagePercent = getFreeSwapPercent(); + if (swapUsagePercent < COMPACT_DOWNGRADE_FREE_SWAP_THRESHOLD) { + Slog.d(TAG_AM, + "Downgraded compaction to file only due to low swap." + + " Swap Free% " + swapUsagePercent); + resolvedAction = COMPACT_ACTION_FILE; + } + } + + return resolvedAction; + } + @VisibleForTesting static final class LastCompactionStats { private final long[] mRssAfterCompaction; @@ -1161,6 +1226,167 @@ public final class CachedAppOptimizer { super(mCachedAppOptimizerThread.getLooper()); } + private boolean shouldOomAdjThrottleCompaction(ProcessRecord proc, int action) { + final String name = proc.processName; + if (mAm.mInternal.isPendingTopUid(proc.uid)) { + // In case the OOM Adjust has not yet been propagated we see if this is + // pending on becoming top app in which case we should not compact. + Slog.e(TAG_AM, "Skip compaction since UID is active for " + name); + return true; + } + + // don't compact if the process has returned to perceptible + // and this is only a cached/home/prev compaction + if ((action == COMPACT_ACTION_FILE || action == COMPACT_ACTION_FULL) + && (proc.mState.getSetAdj() <= ProcessList.PERCEPTIBLE_APP_ADJ)) { + if (DEBUG_COMPACTION) { + Slog.d(TAG_AM, + "Skipping compaction as process " + name + " is " + + "now perceptible."); + } + return true; + } + + return false; + } + + private boolean shouldTimeThrottleCompaction( + ProcessRecord proc, long start, int pendingAction) { + final ProcessCachedOptimizerRecord opt = proc.mOptRecord; + final String name = proc.processName; + + int lastCompactAction = opt.getLastCompactAction(); + long lastCompactTime = opt.getLastCompactTime(); + + // basic throttling + // use the Phenotype flag knobs to determine whether current/prevous + // compaction combo should be throtted or not + + // Note that we explicitly don't take mPhenotypeFlagLock here as the flags + // should very seldom change, and taking the risk of using the wrong action is + // preferable to taking the lock for every single compaction action. + if (lastCompactTime != 0) { + if (pendingAction == COMPACT_PROCESS_SOME) { + if ((lastCompactAction == COMPACT_PROCESS_SOME + && (start - lastCompactTime < mCompactThrottleSomeSome)) + || (lastCompactAction == COMPACT_PROCESS_FULL + && (start - lastCompactTime < mCompactThrottleSomeFull))) { + if (DEBUG_COMPACTION) { + Slog.d(TAG_AM, + "Skipping some compaction for " + name + + ": too soon. throttle=" + mCompactThrottleSomeSome + + "/" + mCompactThrottleSomeFull + + " last=" + (start - lastCompactTime) + "ms ago"); + } + return true; + } + } else if (pendingAction == COMPACT_PROCESS_FULL) { + if ((lastCompactAction == COMPACT_PROCESS_SOME + && (start - lastCompactTime < mCompactThrottleFullSome)) + || (lastCompactAction == COMPACT_PROCESS_FULL + && (start - lastCompactTime < mCompactThrottleFullFull))) { + if (DEBUG_COMPACTION) { + Slog.d(TAG_AM, + "Skipping full compaction for " + name + + ": too soon. throttle=" + mCompactThrottleFullSome + + "/" + mCompactThrottleFullFull + + " last=" + (start - lastCompactTime) + "ms ago"); + } + return true; + } + } else if (pendingAction == COMPACT_PROCESS_PERSISTENT) { + if (start - lastCompactTime < mCompactThrottlePersistent) { + if (DEBUG_COMPACTION) { + Slog.d(TAG_AM, + "Skipping persistent compaction for " + name + + ": too soon. throttle=" + mCompactThrottlePersistent + + " last=" + (start - lastCompactTime) + "ms ago"); + } + return true; + } + } else if (pendingAction == COMPACT_PROCESS_BFGS) { + if (start - lastCompactTime < mCompactThrottleBFGS) { + if (DEBUG_COMPACTION) { + Slog.d(TAG_AM, + "Skipping bfgs compaction for " + name + + ": too soon. throttle=" + mCompactThrottleBFGS + + " last=" + (start - lastCompactTime) + "ms ago"); + } + return true; + } + } + } + + return false; + } + + private boolean shouldThrottleMiscCompaction( + ProcessRecord proc, int procState, int action) { + final String name = proc.processName; + if (mProcStateThrottle.contains(procState)) { + if (DEBUG_COMPACTION) { + Slog.d(TAG_AM, + "Skipping full compaction for process " + name + "; proc state is " + + procState); + } + return true; + } + + if (COMPACT_ACTION_NONE == action) { + if (DEBUG_COMPACTION) { + Slog.d(TAG_AM, + "Skipping compaction for process " + name + "since action is None"); + } + return true; + } + + return false; + } + + private boolean shouldRssThrottleCompaction( + int action, int pid, String name, long[] rssBefore) { + long anonRssBefore = rssBefore[RSS_ANON_INDEX]; + LastCompactionStats lastCompactionStats = mLastCompactionStats.get(pid); + + if (rssBefore[RSS_TOTAL_INDEX] == 0 && rssBefore[RSS_FILE_INDEX] == 0 + && rssBefore[RSS_ANON_INDEX] == 0 && rssBefore[RSS_SWAP_INDEX] == 0) { + if (DEBUG_COMPACTION) { + Slog.d(TAG_AM, + "Skipping compaction for" + + "process " + pid + " with no memory usage. Dead?"); + } + return true; + } + + if (action == COMPACT_ACTION_FULL || action == COMPACT_ACTION_ANON) { + if (mFullAnonRssThrottleKb > 0L && anonRssBefore < mFullAnonRssThrottleKb) { + if (DEBUG_COMPACTION) { + Slog.d(TAG_AM, + "Skipping full compaction for process " + name + + "; anon RSS is too small: " + anonRssBefore + "KB."); + } + return true; + } + + if (lastCompactionStats != null && mFullDeltaRssThrottleKb > 0L) { + long[] lastRss = lastCompactionStats.getRssAfterCompaction(); + long absDelta = Math.abs(rssBefore[RSS_FILE_INDEX] - lastRss[RSS_FILE_INDEX]) + + Math.abs(rssBefore[RSS_ANON_INDEX] - lastRss[RSS_ANON_INDEX]) + + Math.abs(rssBefore[RSS_SWAP_INDEX] - lastRss[RSS_SWAP_INDEX]); + if (absDelta <= mFullDeltaRssThrottleKb) { + if (DEBUG_COMPACTION) { + Slog.d(TAG_AM, + "Skipping full compaction for process " + name + + "; abs delta is too small: " + absDelta + "KB."); + } + return true; + } + } + } + + return false; + } + @Override public void handleMessage(Message msg) { switch (msg.what) { @@ -1171,180 +1397,65 @@ public final class CachedAppOptimizer { int pid; String action; final String name; - int pendingAction, lastCompactAction; + int requestedAction, lastCompactAction; long lastCompactTime; - LastCompactionStats lastCompactionStats; int lastOomAdj = msg.arg1; int procState = msg.arg2; + boolean forceCompaction; synchronized (mProcLock) { - if(mPendingCompactionProcesses.isEmpty()) { + if (mPendingCompactionProcesses.isEmpty()) { + if (DEBUG_COMPACTION) { + Slog.d(TAG_AM, "No processes pending compaction, bail out"); + } return; } proc = mPendingCompactionProcesses.remove(0); opt = proc.mOptRecord; + forceCompaction = opt.isForceCompact(); + opt.setForceCompact(false); // since this is a one-shot operation - pendingAction = opt.getReqCompactAction(); + requestedAction = opt.getReqCompactAction(); pid = proc.getPid(); name = proc.processName; opt.setHasPendingCompact(false); - - if (mAm.mInternal.isPendingTopUid(proc.uid)) { - // In case the OOM Adjust has not yet been propagated we see if this is - // pending on becoming top app in which case we should not compact. - Slog.e(TAG_AM, "Skip compaction since UID is active for " + name); - return; - } - - // don't compact if the process has returned to perceptible - // and this is only a cached/home/prev compaction - if ((pendingAction == COMPACT_PROCESS_SOME - || pendingAction == COMPACT_PROCESS_FULL) - && (proc.mState.getSetAdj() <= ProcessList.PERCEPTIBLE_APP_ADJ)) { - if (DEBUG_COMPACTION) { - Slog.d(TAG_AM, - "Skipping compaction as process " + name + " is " - + "now perceptible."); - } - return; - } - lastCompactAction = opt.getLastCompactAction(); lastCompactTime = opt.getLastCompactTime(); - lastCompactionStats = mLastCompactionStats.get(pid); } + int resolvedAction = resolveCompactionAction(requestedAction); + long[] rssBefore; if (pid == 0) { // not a real process, either one being launched or one being killed - return; - } - - // basic throttling - // use the Phenotype flag knobs to determine whether current/prevous - // compaction combo should be throtted or not - - // Note that we explicitly don't take mPhenotypeFlagLock here as the flags - // should very seldom change, and taking the risk of using the wrong action is - // preferable to taking the lock for every single compaction action. - if (lastCompactTime != 0) { - if (pendingAction == COMPACT_PROCESS_SOME) { - if ((lastCompactAction == COMPACT_PROCESS_SOME - && (start - lastCompactTime < mCompactThrottleSomeSome)) - || (lastCompactAction == COMPACT_PROCESS_FULL - && (start - lastCompactTime - < mCompactThrottleSomeFull))) { - if (DEBUG_COMPACTION) { - Slog.d(TAG_AM, "Skipping some compaction for " + name - + ": too soon. throttle=" + mCompactThrottleSomeSome - + "/" + mCompactThrottleSomeFull + " last=" - + (start - lastCompactTime) + "ms ago"); - } - return; - } - } else if (pendingAction == COMPACT_PROCESS_FULL) { - if ((lastCompactAction == COMPACT_PROCESS_SOME - && (start - lastCompactTime < mCompactThrottleFullSome)) - || (lastCompactAction == COMPACT_PROCESS_FULL - && (start - lastCompactTime - < mCompactThrottleFullFull))) { - if (DEBUG_COMPACTION) { - Slog.d(TAG_AM, "Skipping full compaction for " + name - + ": too soon. throttle=" + mCompactThrottleFullSome - + "/" + mCompactThrottleFullFull + " last=" - + (start - lastCompactTime) + "ms ago"); - } - return; - } - } else if (pendingAction == COMPACT_PROCESS_PERSISTENT) { - if (start - lastCompactTime < mCompactThrottlePersistent) { - if (DEBUG_COMPACTION) { - Slog.d(TAG_AM, "Skipping persistent compaction for " + name - + ": too soon. throttle=" + mCompactThrottlePersistent - + " last=" + (start - lastCompactTime) + "ms ago"); - } - return; - } - } else if (pendingAction == COMPACT_PROCESS_BFGS) { - if (start - lastCompactTime < mCompactThrottleBFGS) { - if (DEBUG_COMPACTION) { - Slog.d(TAG_AM, "Skipping bfgs compaction for " + name - + ": too soon. throttle=" + mCompactThrottleBFGS - + " last=" + (start - lastCompactTime) + "ms ago"); - } - return; - } - } - } - - switch (pendingAction) { - case COMPACT_PROCESS_SOME: - action = mCompactActionSome; - break; - // For the time being, treat these as equivalent. - case COMPACT_PROCESS_FULL: - case COMPACT_PROCESS_PERSISTENT: - case COMPACT_PROCESS_BFGS: - action = mCompactActionFull; - break; - default: - action = COMPACT_ACTION_STRING[COMPACT_ACTION_NONE]; - break; - } - - if (COMPACT_ACTION_STRING[COMPACT_ACTION_NONE].equals(action)) { - return; - } - - if (mProcStateThrottle.contains(procState)) { if (DEBUG_COMPACTION) { - Slog.d(TAG_AM, "Skipping full compaction for process " + name - + "; proc state is " + procState); + Slog.d(TAG_AM, "Compaction failed, pid is 0"); } return; } - long[] rssBefore = mProcessDependencies.getRss(pid); - long anonRssBefore = rssBefore[2]; - - if (rssBefore[0] == 0 && rssBefore[1] == 0 && rssBefore[2] == 0 - && rssBefore[3] == 0) { - if (DEBUG_COMPACTION) { - Slog.d(TAG_AM, "Skipping compaction for" + "process " + pid - + " with no memory usage. Dead?"); - } - return; - } - - if (action.equals(COMPACT_ACTION_STRING[COMPACT_ACTION_FULL]) - || action.equals(COMPACT_ACTION_STRING[COMPACT_ACTION_ANON])) { - if (mFullAnonRssThrottleKb > 0L - && anonRssBefore < mFullAnonRssThrottleKb) { - if (DEBUG_COMPACTION) { - Slog.d(TAG_AM, "Skipping full compaction for process " - + name + "; anon RSS is too small: " + anonRssBefore - + "KB."); - } + if (!forceCompaction) { + if (shouldOomAdjThrottleCompaction(proc, resolvedAction)) { return; } - - if (lastCompactionStats != null && mFullDeltaRssThrottleKb > 0L) { - long[] lastRss = lastCompactionStats.getRssAfterCompaction(); - long absDelta = Math.abs(rssBefore[1] - lastRss[1]) - + Math.abs(rssBefore[2] - lastRss[2]) - + Math.abs(rssBefore[3] - lastRss[3]); - if (absDelta <= mFullDeltaRssThrottleKb) { - if (DEBUG_COMPACTION) { - Slog.d(TAG_AM, "Skipping full compaction for process " - + name + "; abs delta is too small: " + absDelta - + "KB."); - } - return; - } + if (shouldTimeThrottleCompaction(proc, start, requestedAction)) { + return; + } + if (shouldThrottleMiscCompaction(proc, procState, resolvedAction)) { + return; + } + rssBefore = mProcessDependencies.getRss(pid); + if (shouldRssThrottleCompaction(resolvedAction, pid, name, rssBefore)) { + return; + } + } else { + rssBefore = mProcessDependencies.getRss(pid); + if (DEBUG_COMPACTION) { + Slog.d(TAG_AM, "Forcing compaction for " + name); } } // Now we've passed through all the throttles and are going to compact, update // bookkeeping. - switch (pendingAction) { + switch (requestedAction) { case COMPACT_PROCESS_SOME: mSomeCompactionCount++; break; @@ -1360,57 +1471,56 @@ public final class CachedAppOptimizer { default: break; } - // Downgrade compaction if facing swap memory pressure - if (action.equals(mCompactActionFull)) { - double swapUsagePercent = getFreeSwapPercent(); - if (swapUsagePercent < COMPACT_DOWNGRADE_FREE_SWAP_THRESHOLD) { - Slog.d(TAG_AM, - "Downgraded compaction to file only due to low swap." - + " Swap Free% " + swapUsagePercent); - action = mCompactActionSome; - pendingAction = COMPACT_PROCESS_SOME; - } - } + action = compactActionIntToString(resolvedAction); try { - Trace.traceBegin(Trace.TRACE_TAG_ACTIVITY_MANAGER, "Compact " - + ((pendingAction == COMPACT_PROCESS_SOME) ? "some" : "full") - + ": " + name); + Trace.traceBegin(Trace.TRACE_TAG_ACTIVITY_MANAGER, + "Compact " + action + ": " + name); long zramFreeKbBefore = Debug.getZramFreeKb(); mProcessDependencies.performCompaction(action, pid); long[] rssAfter = mProcessDependencies.getRss(pid); long end = SystemClock.uptimeMillis(); long time = end - start; long zramFreeKbAfter = Debug.getZramFreeKb(); + long deltaTotalRss = rssAfter[RSS_TOTAL_INDEX] - rssBefore[RSS_TOTAL_INDEX]; + long deltaFileRss = rssAfter[RSS_FILE_INDEX] - rssBefore[RSS_FILE_INDEX]; + long deltaAnonRss = rssAfter[RSS_ANON_INDEX] - rssBefore[RSS_ANON_INDEX]; + long deltaSwapRss = rssAfter[RSS_SWAP_INDEX] - rssBefore[RSS_SWAP_INDEX]; EventLog.writeEvent(EventLogTags.AM_COMPACT, pid, name, action, - rssBefore[0], rssBefore[1], rssBefore[2], rssBefore[3], - rssAfter[0] - rssBefore[0], rssAfter[1] - rssBefore[1], - rssAfter[2] - rssBefore[2], rssAfter[3] - rssBefore[3], time, - lastCompactAction, lastCompactTime, lastOomAdj, procState, - zramFreeKbBefore, zramFreeKbAfter - zramFreeKbBefore); + rssBefore[RSS_TOTAL_INDEX], rssBefore[RSS_FILE_INDEX], + rssBefore[RSS_ANON_INDEX], rssBefore[RSS_SWAP_INDEX], deltaTotalRss, + deltaFileRss, deltaAnonRss, deltaSwapRss, time, lastCompactAction, + lastCompactTime, lastOomAdj, procState, zramFreeKbBefore, + zramFreeKbAfter - zramFreeKbBefore); // Note that as above not taking mPhenoTypeFlagLock here to avoid locking // on every single compaction for a flag that will seldom change and the // impact of reading the wrong value here is low. if (mRandom.nextFloat() < mCompactStatsdSampleRate) { FrameworkStatsLog.write(FrameworkStatsLog.APP_COMPACTED, pid, name, - pendingAction, rssBefore[0], rssBefore[1], rssBefore[2], - rssBefore[3], rssAfter[0], rssAfter[1], rssAfter[2], - rssAfter[3], time, lastCompactAction, lastCompactTime, - lastOomAdj, ActivityManager.processStateAmToProto(procState), + requestedAction, rssBefore[RSS_TOTAL_INDEX], + rssBefore[RSS_FILE_INDEX], rssBefore[RSS_ANON_INDEX], + rssBefore[RSS_SWAP_INDEX], rssAfter[RSS_TOTAL_INDEX], + rssAfter[RSS_FILE_INDEX], rssAfter[RSS_ANON_INDEX], + rssAfter[RSS_SWAP_INDEX], time, lastCompactAction, + lastCompactTime, lastOomAdj, + ActivityManager.processStateAmToProto(procState), zramFreeKbBefore, zramFreeKbAfter); } synchronized (mProcLock) { opt.setLastCompactTime(end); - opt.setLastCompactAction(pendingAction); + opt.setLastCompactAction(resolvedAction); } - if (action.equals(COMPACT_ACTION_STRING[COMPACT_ACTION_FULL]) - || action.equals(COMPACT_ACTION_STRING[COMPACT_ACTION_ANON])) { + if (resolvedAction == COMPACT_ACTION_FULL + || resolvedAction == COMPACT_ACTION_ANON) { // Remove entry and insert again to update insertion order. mLastCompactionStats.remove(pid); mLastCompactionStats.put(pid, new LastCompactionStats(rssAfter)); } } catch (Exception e) { // nothing to do, presumably the process died + Slog.d(TAG_AM, + "Exception occurred while compacting pid: " + name + + ". Exception:" + e.getMessage()); } finally { Trace.traceEnd(Trace.TRACE_TAG_ACTIVITY_MANAGER); } diff --git a/services/core/java/com/android/server/am/ProcessCachedOptimizerRecord.java b/services/core/java/com/android/server/am/ProcessCachedOptimizerRecord.java index a86ba016eeff7..a613729b441f2 100644 --- a/services/core/java/com/android/server/am/ProcessCachedOptimizerRecord.java +++ b/services/core/java/com/android/server/am/ProcessCachedOptimizerRecord.java @@ -56,6 +56,8 @@ final class ProcessCachedOptimizerRecord { @GuardedBy("mProcLock") private boolean mPendingCompact; + @GuardedBy("mProcLock") private boolean mForceCompact; + /** * True when the process is frozen. */ @@ -132,6 +134,16 @@ final class ProcessCachedOptimizerRecord { mPendingCompact = pendingCompact; } + @GuardedBy("mProcLock") + boolean isForceCompact() { + return mForceCompact; + } + + @GuardedBy("mProcLock") + void setForceCompact(boolean forceCompact) { + mForceCompact = forceCompact; + } + @GuardedBy("mProcLock") boolean isFrozen() { return mFrozen; @@ -205,6 +217,9 @@ final class ProcessCachedOptimizerRecord { void dump(PrintWriter pw, String prefix, long nowUptime) { pw.print(prefix); pw.print("lastCompactTime="); pw.print(mLastCompactTime); pw.print(" lastCompactAction="); pw.println(mLastCompactAction); + pw.print(prefix); + pw.print("hasPendingCompaction="); + pw.print(mPendingCompact); pw.print(prefix); pw.print("isFreezeExempt="); pw.print(mFreezeExempt); pw.print(" isPendingFreeze="); pw.print(mPendingFreeze); pw.print(" " + IS_FROZEN + "="); pw.println(mFrozen); diff --git a/services/tests/mockingservicestests/src/com/android/server/am/CachedAppOptimizerTest.java b/services/tests/mockingservicestests/src/com/android/server/am/CachedAppOptimizerTest.java index bf46f555004c9..2baa1ec6cdc24 100644 --- a/services/tests/mockingservicestests/src/com/android/server/am/CachedAppOptimizerTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/am/CachedAppOptimizerTest.java @@ -857,6 +857,7 @@ public final class CachedAppOptimizerTest { .containsExactlyElementsIn(expected); } + @SuppressWarnings("GuardedBy") @Test public void processWithDeltaRSSTooSmall_notFullCompacted() throws Exception { // Initialize CachedAppOptimizer and set flags to (1) enable compaction, (2) set RSS @@ -892,7 +893,7 @@ public final class CachedAppOptimizerTest { mProcessDependencies.setRss(rssBefore1); mProcessDependencies.setRssAfterCompaction(rssAfter1); // // WHEN we try to run compaction - mCachedAppOptimizerUnderTest.compactAppFull(processRecord); + mCachedAppOptimizerUnderTest.compactAppFull(processRecord, false); waitForHandler(); // THEN process IS compacted. assertThat(mCachedAppOptimizerUnderTest.mLastCompactionStats.get(pid)).isNotNull(); @@ -907,7 +908,7 @@ public final class CachedAppOptimizerTest { processRecord.mOptRecord.setLastCompactTime( processRecord.mOptRecord.getLastCompactTime() - 10_000); // WHEN we try to run compaction. - mCachedAppOptimizerUnderTest.compactAppFull(processRecord); + mCachedAppOptimizerUnderTest.compactAppFull(processRecord, false); waitForHandler(); // THEN process IS NOT compacted - values after compaction for process 1 should remain the // same as from the last compaction. @@ -923,7 +924,7 @@ public final class CachedAppOptimizerTest { processRecord.mOptRecord.setLastCompactTime( processRecord.mOptRecord.getLastCompactTime() - 10_000); // WHEN we try to run compaction - mCachedAppOptimizerUnderTest.compactAppFull(processRecord); + mCachedAppOptimizerUnderTest.compactAppFull(processRecord, false); waitForHandler(); // THEN process IS compacted - values after compaction for process 1 should be updated. assertThat(mCachedAppOptimizerUnderTest.mLastCompactionStats.get(pid)).isNotNull(); @@ -932,6 +933,7 @@ public final class CachedAppOptimizerTest { assertThat(valuesAfter).isEqualTo(rssAfter3); } + @SuppressWarnings("GuardedBy") @Test public void processWithAnonRSSTooSmall_notFullCompacted() throws Exception { // Initialize CachedAppOptimizer and set flags to (1) enable compaction, (2) set RSS @@ -963,7 +965,7 @@ public final class CachedAppOptimizerTest { mProcessDependencies.setRss(rssBelowThreshold); mProcessDependencies.setRssAfterCompaction(rssBelowThresholdAfter); // WHEN we try to run compaction - mCachedAppOptimizerUnderTest.compactAppFull(processRecord); + mCachedAppOptimizerUnderTest.compactAppFull(processRecord, false); waitForHandler(); // THEN process IS NOT compacted. assertThat(mCachedAppOptimizerUnderTest.mLastCompactionStats.get(pid)).isNull(); @@ -972,7 +974,7 @@ public final class CachedAppOptimizerTest { mProcessDependencies.setRss(rssAboveThreshold); mProcessDependencies.setRssAfterCompaction(rssAboveThresholdAfter); // WHEN we try to run compaction - mCachedAppOptimizerUnderTest.compactAppFull(processRecord); + mCachedAppOptimizerUnderTest.compactAppFull(processRecord, false); waitForHandler(); // THEN process IS compacted. assertThat(mCachedAppOptimizerUnderTest.mLastCompactionStats.get(pid)).isNotNull(); @@ -981,6 +983,7 @@ public final class CachedAppOptimizerTest { assertThat(valuesAfter).isEqualTo(rssAboveThresholdAfter); } + @SuppressWarnings("GuardedBy") @Test public void processWithOomAdjTooSmall_notFullCompacted() throws Exception { // Initialize CachedAppOptimizer and set flags to (1) enable compaction, (2) set Min and @@ -993,10 +996,11 @@ public final class CachedAppOptimizerTest { // Simulate RSS memory for which compaction should occur. long[] rssBefore = - new long[]{/*Total RSS*/ 15000, /*File RSS*/ 15000, /*Anon RSS*/ 15000, - /*Swap*/ 10000}; + new long[]{/*Total RSS*/ 15000, /*File RSS*/ 15000, /*Anon RSS*/ 15000, + /*Swap*/ 10000}; long[] rssAfter = - new long[]{/*Total RSS*/ 8000, /*File RSS*/ 9000, /*Anon RSS*/ 6000, /*Swap*/5000}; + new long[]{/*Total RSS*/ 8000, /*File RSS*/ 9000, /*Anon RSS*/ 6000, /*Swap*/ + 5000}; // Process that passes properties. int pid = 1; ProcessRecord processRecord = @@ -1010,7 +1014,7 @@ public final class CachedAppOptimizerTest { processRecord.mState.setSetAdj(899); processRecord.mState.setCurAdj(970); // WHEN we try to run compaction - mCachedAppOptimizerUnderTest.compactAppFull(processRecord); + mCachedAppOptimizerUnderTest.compactAppFull(processRecord, false); waitForHandler(); // THEN process IS NOT compacted. assertThat(mCachedAppOptimizerUnderTest.mLastCompactionStats.get(pid)).isNull(); @@ -1019,16 +1023,71 @@ public final class CachedAppOptimizerTest { processRecord.mState.setSetAdj(910); processRecord.mState.setCurAdj(930); // WHEN we try to run compaction - mCachedAppOptimizerUnderTest.compactAppFull(processRecord); + mCachedAppOptimizerUnderTest.compactAppFull(processRecord, false); waitForHandler(); // THEN process IS compacted. assertThat(mCachedAppOptimizerUnderTest.mLastCompactionStats.get(pid)).isNotNull(); long[] valuesAfter = mCachedAppOptimizerUnderTest.mLastCompactionStats - .get(pid) - .getRssAfterCompaction(); + .get(pid) + .getRssAfterCompaction(); assertThat(valuesAfter).isEqualTo(rssAfter); } + @SuppressWarnings("GuardedBy") + @Test + public void process_forceCompacted() throws Exception { + mCachedAppOptimizerUnderTest.init(); + setFlag(CachedAppOptimizer.KEY_USE_COMPACTION, "true", true); + setFlag(CachedAppOptimizer.KEY_COMPACT_THROTTLE_MIN_OOM_ADJ, Long.toString(920), true); + setFlag(CachedAppOptimizer.KEY_COMPACT_THROTTLE_MAX_OOM_ADJ, Long.toString(950), true); + initActivityManagerService(); + + long[] rssBefore = new long[] {/*Total RSS*/ 15000, /*File RSS*/ 15000, /*Anon RSS*/ 15000, + /*Swap*/ 10000}; + long[] rssAfter = new long[] { + /*Total RSS*/ 8000, /*File RSS*/ 9000, /*Anon RSS*/ 6000, /*Swap*/ 5000}; + // Process that passes properties. + int pid = 1; + ProcessRecord processRecord = makeProcessRecord(pid, 2, 3, "p1", "app1"); + mProcessDependencies.setRss(rssBefore); + mProcessDependencies.setRssAfterCompaction(rssAfter); + + // Use an OOM Adjust value that usually avoids compaction + processRecord.mState.setSetAdj(100); + processRecord.mState.setCurAdj(100); + + // Compact process full + mCachedAppOptimizerUnderTest.compactAppFull(processRecord, false); + waitForHandler(); + // the process is not compacted + assertThat(mCachedAppOptimizerUnderTest.mLastCompactionStats.get(pid)).isNull(); + + // Compact process some + mCachedAppOptimizerUnderTest.compactAppSome(processRecord, false); + waitForHandler(); + // the process is not compacted + assertThat(mCachedAppOptimizerUnderTest.mLastCompactionStats.get(pid)).isNull(); + + processRecord.mState.setSetAdj(100); + processRecord.mState.setCurAdj(100); + + // We force a full compaction + mCachedAppOptimizerUnderTest.compactAppFull(processRecord, true); + waitForHandler(); + // then process is compacted. + assertThat(mCachedAppOptimizerUnderTest.mLastCompactionStats.get(pid)).isNotNull(); + + mCachedAppOptimizerUnderTest.mLastCompactionStats.clear(); + + // We force a some compaction + mCachedAppOptimizerUnderTest.compactAppSome(processRecord, true); + waitForHandler(); + // then process is compacted. + String executedCompactAction = + compactActionIntToString(processRecord.mOptRecord.getLastCompactAction()); + assertThat(executedCompactAction) + .isEqualTo(mCachedAppOptimizerUnderTest.mCompactActionSome); + } private void setFlag(String key, String value, boolean defaultValue) throws Exception { mCountDown = new CountDownLatch(1); From 8a054c32c2dc86a93efe19885fd1deea6f9933f4 Mon Sep 17 00:00:00 2001 From: Edgar Arriaga Date: Tue, 29 Mar 2022 11:53:57 -0700 Subject: [PATCH 4/7] Fix for new compactions skipped after cancelling pending compactions When new compactions are scheduled they are added to the pending compaction process list however, they also maintain a flag that indicates whether a compaction is pending on the record, previously, when cancelling compactions we would remove the process from the list but not clear the flag, leaving the process in an uncompactable state as it would be skipped from being scheduled thinking there was a compaction happening when there wasn't any. Bug: 227502250 Test: Manual. Logging and dumpsys activity Change-Id: Id9d712bd14ba83c4d647d3e9faa88e4ddbd1b697 --- .../android/server/am/CachedAppOptimizer.java | 21 +++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/services/core/java/com/android/server/am/CachedAppOptimizer.java b/services/core/java/com/android/server/am/CachedAppOptimizer.java index e91b2b32f930e..2b16b1589288b 100644 --- a/services/core/java/com/android/server/am/CachedAppOptimizer.java +++ b/services/core/java/com/android/server/am/CachedAppOptimizer.java @@ -1146,13 +1146,26 @@ public final class CachedAppOptimizer { if(wakefulness == PowerManagerInternal.WAKEFULNESS_AWAKE) { // Remove any pending compaction we may have scheduled to happen while screen was off Slog.e(TAG_AM, "Cancel pending or running compactions as system is awake"); - synchronized(mProcLock) { - mPendingCompactionProcesses.clear(); - } - cancelCompaction(); + cancelAllCompactions(); } } + void cancelAllCompactions() { + synchronized (mProcLock) { + int size = mPendingCompactionProcesses.size(); + ProcessRecord record; + for (int i=0; i < size; ++i) { + record = mPendingCompactionProcesses.get(i); + // The process record is kept alive after compactions are cleared, + // so make sure to reset the compaction state to avoid skipping any future + // compactions due to a stale value here. + record.mOptRecord.setHasPendingCompact(false); + } + mPendingCompactionProcesses.clear(); + } + cancelCompaction(); + } + @GuardedBy({"mService", "mProcLock"}) void onOomAdjustChanged(int oldAdj, int newAdj, ProcessRecord app) { // Cancel any currently executing compactions From 463a08b2acc43db3cbc7e05f591e58e9ace768a3 Mon Sep 17 00:00:00 2001 From: Edgar Arriaga Date: Tue, 29 Mar 2022 14:42:59 -0700 Subject: [PATCH 5/7] Fix for compaction partially bailing out compaction upon cancellation When compaction if a cancel signal is received such as when unlocking the phone, if the system was doing an anon compaction it will bail out but then continue with the file compaction since it was not fully bailing out from compaction computation. Test: Manual Bug: 227060330 Change-Id: I46bb784c1170808d9aa15757a108900e54175987 --- ...m_android_server_am_CachedAppOptimizer.cpp | 34 ++++++++----------- 1 file changed, 15 insertions(+), 19 deletions(-) diff --git a/services/core/jni/com_android_server_am_CachedAppOptimizer.cpp b/services/core/jni/com_android_server_am_CachedAppOptimizer.cpp index 8664c38af816c..1ff5002568a36 100644 --- a/services/core/jni/com_android_server_am_CachedAppOptimizer.cpp +++ b/services/core/jni/com_android_server_am_CachedAppOptimizer.cpp @@ -74,10 +74,12 @@ using android::base::unique_fd; // limit, it has to be a page aligned value, otherwise, compaction would fail. #define MAX_BYTES_PER_COMPACTION MAX_RW_COUNT +// Selected a high enough number to avoid clashing with linux errno codes +#define ERROR_COMPACTION_CANCELLED -1000 + namespace android { -static bool cancelRunningCompaction; -static bool compactionInProgress; +static std::atomic cancelRunningCompaction; // Legacy method for compacting processes, any new code should // use compactProcess instead. @@ -103,8 +105,6 @@ static int64_t compactMemory(const std::vector& vmas, int pid, int madviseT // Skip compaction if failed to open pidfd with any error return -errno; } - compactionInProgress = true; - cancelRunningCompaction = false; int64_t totalBytesProcessed = 0; @@ -113,14 +113,14 @@ static int64_t compactMemory(const std::vector& vmas, int pid, int madviseT uint64_t bytesSentToCompact = 0; int iVec = 0; while (iVec < MAX_VMAS_PER_COMPACTION && iVma < vmas.size()) { - if (CC_UNLIKELY(cancelRunningCompaction)) { + if (CC_UNLIKELY(cancelRunningCompaction.load())) { // There could be a significant delay between when a compaction // is requested and when it is handled during this time our // OOM adjust could have improved. LOG(DEBUG) << "Cancelled running compaction for " << pid; ATRACE_INSTANT_FOR_TRACK(ATRACE_COMPACTION_TRACK, StringPrintf("Cancelled compaction for %d", pid).c_str()); - break; + return ERROR_COMPACTION_CANCELLED; } uint64_t vmaStart = vmas[iVma].start + vmaOffset; @@ -150,11 +150,6 @@ static int64_t compactMemory(const std::vector& vmas, int pid, int madviseT ++iVma; } - if (cancelRunningCompaction) { - cancelRunningCompaction = false; - break; - } - ATRACE_BEGIN(StringPrintf("Compact %d VMAs", iVec).c_str()); auto bytesProcessed = process_madvise(pidfd, vmasToKernel, iVec, madviseType, 0); ATRACE_END(); @@ -166,14 +161,12 @@ static int64_t compactMemory(const std::vector& vmas, int pid, int madviseT continue; } else { // Forward irrecoverable errors and bail out compaction - compactionInProgress = false; return -errno; } } totalBytesProcessed += bytesProcessed; } - compactionInProgress = false; return totalBytesProcessed; } @@ -202,9 +195,12 @@ static int getAnyPageAdvice(const Vma& vma) { // // Currently supported behaviors are MADV_COLD and MADV_PAGEOUT. // -// Returns the total number of bytes compacted or forwards an -// process_madvise error. +// Returns the total number of bytes compacted on success. On error +// returns process_madvise errno code or if compaction was cancelled +// it returns ERROR_COMPACTION_CANCELLED. static int64_t compactProcess(int pid, VmaToAdviseFunc vmaToAdviseFunc) { + cancelRunningCompaction.store(false); + ProcMemInfo meminfo(pid); std::vector pageoutVmas, coldVmas; auto vmaCollectorCb = [&coldVmas,&pageoutVmas,&vmaToAdviseFunc](const Vma& vma) { @@ -223,12 +219,14 @@ static int64_t compactProcess(int pid, VmaToAdviseFunc vmaToAdviseFunc) { int64_t pageoutBytes = compactMemory(pageoutVmas, pid, MADV_PAGEOUT); if (pageoutBytes < 0) { // Error, just forward it. + cancelRunningCompaction.store(false); return pageoutBytes; } int64_t coldBytes = compactMemory(coldVmas, pid, MADV_COLD); if (coldBytes < 0) { // Error, just forward it. + cancelRunningCompaction.store(false); return coldBytes; } @@ -308,10 +306,8 @@ static void com_android_server_am_CachedAppOptimizer_compactSystem(JNIEnv *, job } static void com_android_server_am_CachedAppOptimizer_cancelCompaction(JNIEnv*, jobject) { - if (compactionInProgress) { - cancelRunningCompaction = true; - ATRACE_INSTANT_FOR_TRACK(ATRACE_COMPACTION_TRACK, "Cancel compaction"); - } + cancelRunningCompaction.store(true); + ATRACE_INSTANT_FOR_TRACK(ATRACE_COMPACTION_TRACK, "Cancel compaction"); } static jdouble com_android_server_am_CachedAppOptimizer_getFreeSwapPercent(JNIEnv*, jobject) { From ce1bdbe56d26746d1e41e371ea1fde5e5d6ce8a7 Mon Sep 17 00:00:00 2001 From: Edgar Arriaga Date: Tue, 29 Mar 2022 19:53:06 -0700 Subject: [PATCH 6/7] Add volatile to some cancel compaction signals to avoid compiler optimizations The compaction cancel signals are delivered between different threads so add volatile keyword for such variables to make sure the compiler does not try to optimize the values avoiding them to be effective due to potential register caching. Bug: 227060330 Test: Manual Change-Id: I42503e02627c6f76382a58da9320b3f5df15a75d --- .../core/java/com/android/server/am/CachedAppOptimizer.java | 2 +- services/core/jni/com_android_server_am_CachedAppOptimizer.cpp | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/services/core/java/com/android/server/am/CachedAppOptimizer.java b/services/core/java/com/android/server/am/CachedAppOptimizer.java index 2b16b1589288b..a172018ab2910 100644 --- a/services/core/java/com/android/server/am/CachedAppOptimizer.java +++ b/services/core/java/com/android/server/am/CachedAppOptimizer.java @@ -1737,7 +1737,7 @@ public final class CachedAppOptimizer { * Default implementation for ProcessDependencies, public vor visibility to OomAdjuster class. */ private static final class DefaultProcessDependencies implements ProcessDependencies { - public static int mPidCompacting = -1; + public static volatile int mPidCompacting = -1; // Get memory RSS from process. @Override diff --git a/services/core/jni/com_android_server_am_CachedAppOptimizer.cpp b/services/core/jni/com_android_server_am_CachedAppOptimizer.cpp index 1ff5002568a36..93152f2ea1b7e 100644 --- a/services/core/jni/com_android_server_am_CachedAppOptimizer.cpp +++ b/services/core/jni/com_android_server_am_CachedAppOptimizer.cpp @@ -79,6 +79,8 @@ using android::base::unique_fd; namespace android { +// Signal happening in separate thread that would bail out compaction +// before starting next VMA batch static std::atomic cancelRunningCompaction; // Legacy method for compacting processes, any new code should From 33b1957c88dcbf1efb7a5140673dbf0d83dbffea Mon Sep 17 00:00:00 2001 From: Edgar Arriaga Date: Thu, 14 Apr 2022 13:20:18 -0700 Subject: [PATCH 7/7] Resend remaining VMAs on partial compaction due to failure process_madvise syscall returns the total bytes that were processed however, upon failure it will send the partial amount of bytes that were processed in which case this patch aims to skip the failure page then resend any batch of vmas that was not fully processed. This way we would effectively process the whole range of vmas rather than skipping the rest of the vmas in batch when process_madvise does a partial advise. Test: Manual Test: am compact full Bug: 205658049 Signed-off-by: Edgar Arriaga Change-Id: I6445ea85f5eb028cb1d11a4af77c5c4cedf76954 --- ...m_android_server_am_CachedAppOptimizer.cpp | 247 ++++++++++++++---- 1 file changed, 193 insertions(+), 54 deletions(-) diff --git a/services/core/jni/com_android_server_am_CachedAppOptimizer.cpp b/services/core/jni/com_android_server_am_CachedAppOptimizer.cpp index 93152f2ea1b7e..49a4021b1a846 100644 --- a/services/core/jni/com_android_server_am_CachedAppOptimizer.cpp +++ b/services/core/jni/com_android_server_am_CachedAppOptimizer.cpp @@ -66,13 +66,13 @@ using android::base::unique_fd; // Defines the maximum amount of VMAs we can send per process_madvise syscall. // Currently this is set to UIO_MAXIOV which is the maximum segments allowed by // iovec implementation used by process_madvise syscall -#define MAX_VMAS_PER_COMPACTION UIO_MAXIOV +#define MAX_VMAS_PER_BATCH UIO_MAXIOV // Maximum bytes that we can send per process_madvise syscall once this limit // is reached we split the remaining VMAs into another syscall. The MAX_RW_COUNT // limit is imposed by iovec implementation. However, if you want to use a smaller -// limit, it has to be a page aligned value, otherwise, compaction would fail. -#define MAX_BYTES_PER_COMPACTION MAX_RW_COUNT +// limit, it has to be a page aligned value. +#define MAX_BYTES_PER_BATCH MAX_RW_COUNT // Selected a high enough number to avoid clashing with linux errno codes #define ERROR_COMPACTION_CANCELLED -1000 @@ -83,6 +83,181 @@ namespace android { // before starting next VMA batch static std::atomic cancelRunningCompaction; +// A VmaBatch represents a set of VMAs that can be processed +// as VMAs are processed by client code it is expected that the +// VMAs get consumed which means they are discarded as they are +// processed so that the first element always is the next element +// to be sent +struct VmaBatch { + struct iovec* vmas; + // total amount of VMAs to reach the end of iovec + int totalVmas; + // total amount of bytes that are remaining within iovec + uint64_t totalBytes; +}; + +// Advances the iterator by the specified amount of bytes. +// This is used to remove already processed or no longer +// needed parts of the batch. +// Returns total bytes consumed +int consumeBytes(VmaBatch& batch, uint64_t bytesToConsume) { + int index = 0; + if (CC_UNLIKELY(bytesToConsume) < 0) { + LOG(ERROR) << "Cannot consume negative bytes for VMA batch !"; + return 0; + } + + if (bytesToConsume > batch.totalBytes) { + // Avoid consuming more bytes than available + bytesToConsume = batch.totalBytes; + } + + uint64_t bytesConsumed = 0; + while (bytesConsumed < bytesToConsume) { + if (CC_UNLIKELY(index >= batch.totalVmas)) { + // reach the end of the batch + return bytesConsumed; + } + if (CC_UNLIKELY(bytesConsumed + batch.vmas[index].iov_len > bytesToConsume)) { + // this is the whole VMA that will be consumed + break; + } + bytesConsumed += batch.vmas[index].iov_len; + batch.totalBytes -= batch.vmas[index].iov_len; + --batch.totalVmas; + ++index; + } + + // Move pointer to consume all the whole VMAs + batch.vmas = batch.vmas + index; + + // Consume the rest of the bytes partially at last VMA in batch + uint64_t bytesLeftToConsume = bytesToConsume - bytesConsumed; + bytesConsumed += bytesLeftToConsume; + if (batch.totalVmas > 0) { + batch.vmas[0].iov_base = (void*)((uint64_t)batch.vmas[0].iov_base + bytesLeftToConsume); + } + + return bytesConsumed; +} + +// given a source of vmas this class will act as a factory +// of VmaBatch objects and it will allow generating batches +// until there are no more left in the source vector. +// Note: the class does not actually modify the given +// vmas vector, instead it iterates on it until the end. +class VmaBatchCreator { + const std::vector* sourceVmas; + // This is the destination array where batched VMAs will be stored + // it gets encapsulated into a VmaBatch which is the object + // meant to be used by client code. + struct iovec* destVmas; + + // Parameters to keep track of the iterator on the source vmas + int currentIndex_; + uint64_t currentOffset_; + +public: + VmaBatchCreator(const std::vector* vmasToBatch, struct iovec* destVmasVec) + : sourceVmas(vmasToBatch), destVmas(destVmasVec), currentIndex_(0), currentOffset_(0) {} + + int currentIndex() { return currentIndex_; } + uint64_t currentOffset() { return currentOffset_; } + + // Generates a batch and moves the iterator on the source vmas + // past the last VMA in the batch. + // Returns true on success, false on failure + bool createNextBatch(VmaBatch& batch) { + if (currentIndex_ >= MAX_VMAS_PER_BATCH && currentIndex_ >= sourceVmas->size()) { + return false; + } + + const std::vector& vmas = *sourceVmas; + batch.vmas = destVmas; + uint64_t totalBytesInBatch = 0; + int indexInBatch = 0; + + // Add VMAs to the batch up until we consumed all the VMAs or + // reached any imposed limit of VMAs per batch. + while (indexInBatch < MAX_VMAS_PER_BATCH && currentIndex_ < vmas.size()) { + uint64_t vmaStart = vmas[currentIndex_].start + currentOffset_; + uint64_t vmaSize = vmas[currentIndex_].end - vmaStart; + if (CC_UNLIKELY(vmaSize == 0)) { + // No more bytes to batch for this VMA, move to next one + // this only happens if a batch partially consumed bytes + // and offset landed at exactly the end of a vma + continue; + } + batch.vmas[indexInBatch].iov_base = (void*)vmaStart; + uint64_t bytesAvailableInBatch = MAX_BYTES_PER_BATCH - totalBytesInBatch; + + if (vmaSize >= bytesAvailableInBatch) { + // VMA would exceed the max available bytes in batch + // clamp with available bytes and finish batch. + vmaSize = bytesAvailableInBatch; + currentOffset_ += bytesAvailableInBatch; + } + + batch.vmas[indexInBatch].iov_len = vmaSize; + totalBytesInBatch += vmaSize; + + ++indexInBatch; + if (totalBytesInBatch >= MAX_BYTES_PER_BATCH) { + // Reached max bytes quota so this marks + // the end of the batch + break; + } + + // Fully finished current VMA, move to next one + currentOffset_ = 0; + ++currentIndex_; + } + // Vmas where fully filled and we are past the last filled index. + batch.totalVmas = indexInBatch; + batch.totalBytes = totalBytesInBatch; + return true; + } +}; + +// Madvise a set of VMAs given in a batch for a specific process +// The total number of bytes successfully madvised will be set on +// outBytesProcessed. +// Returns 0 on success and standard linux -errno code returned by +// process_madvise on failure +int madviseVmasFromBatch(unique_fd& pidfd, VmaBatch& batch, int madviseType, + uint64_t* outBytesProcessed) { + if (batch.totalVmas == 0) { + // No VMAs in Batch, skip. + *outBytesProcessed = 0; + return 0; + } + + ATRACE_BEGIN(StringPrintf("Madvise %d: %d VMAs", madviseType, batch.totalVmas).c_str()); + uint64_t bytesProcessedInSend = + process_madvise(pidfd, batch.vmas, batch.totalVmas, madviseType, 0); + ATRACE_END(); + + if (CC_UNLIKELY(bytesProcessedInSend == -1)) { + bytesProcessedInSend = 0; + if (errno != EINVAL) { + // Forward irrecoverable errors and bail out compaction + *outBytesProcessed = 0; + return -errno; + } + } + + if (bytesProcessedInSend < batch.totalBytes) { + // Did not process all the bytes requested + // skip last page which likely failed + bytesProcessedInSend += PAGE_SIZE; + } + + bytesProcessedInSend = consumeBytes(batch, bytesProcessedInSend); + + *outBytesProcessed = bytesProcessedInSend; + return 0; +} + // Legacy method for compacting processes, any new code should // use compactProcess instead. static inline void compactProcessProcfs(int pid, const std::string& compactionType) { @@ -96,8 +271,6 @@ static inline void compactProcessProcfs(int pid, const std::string& compactionTy // If any VMA fails compaction due to -EINVAL it will be skipped and continue. // However, if it fails for any other reason, it will bail out and forward the error static int64_t compactMemory(const std::vector& vmas, int pid, int madviseType) { - static struct iovec vmasToKernel[MAX_VMAS_PER_COMPACTION]; - if (vmas.empty()) { return 0; } @@ -108,13 +281,16 @@ static int64_t compactMemory(const std::vector& vmas, int pid, int madviseT return -errno; } - int64_t totalBytesProcessed = 0; + struct iovec destVmas[MAX_VMAS_PER_BATCH]; - int64_t vmaOffset = 0; - for (int iVma = 0; iVma < vmas.size();) { - uint64_t bytesSentToCompact = 0; - int iVec = 0; - while (iVec < MAX_VMAS_PER_COMPACTION && iVma < vmas.size()) { + VmaBatch batch; + VmaBatchCreator batcher(&vmas, destVmas); + + int64_t totalBytesProcessed = 0; + while (batcher.createNextBatch(batch)) { + uint64_t bytesProcessedInSend; + + do { if (CC_UNLIKELY(cancelRunningCompaction.load())) { // There could be a significant delay between when a compaction // is requested and when it is handled during this time our @@ -124,50 +300,13 @@ static int64_t compactMemory(const std::vector& vmas, int pid, int madviseT StringPrintf("Cancelled compaction for %d", pid).c_str()); return ERROR_COMPACTION_CANCELLED; } - - uint64_t vmaStart = vmas[iVma].start + vmaOffset; - uint64_t vmaSize = vmas[iVma].end - vmaStart; - if (vmaSize == 0) { - goto next_vma; + int error = madviseVmasFromBatch(pidfd, batch, madviseType, &bytesProcessedInSend); + if (error < 0) { + // Returns standard linux errno code + return error; } - vmasToKernel[iVec].iov_base = (void*)vmaStart; - if (vmaSize > MAX_BYTES_PER_COMPACTION - bytesSentToCompact) { - // Exceeded the max bytes that could be sent, so clamp - // the end to avoid exceeding limit and issue compaction - vmaSize = MAX_BYTES_PER_COMPACTION - bytesSentToCompact; - } - - vmasToKernel[iVec].iov_len = vmaSize; - bytesSentToCompact += vmaSize; - ++iVec; - if (bytesSentToCompact >= MAX_BYTES_PER_COMPACTION) { - // Ran out of bytes within iovec, dispatch compaction. - vmaOffset += vmaSize; - break; - } - - next_vma: - // Finished current VMA, and have more bytes remaining - vmaOffset = 0; - ++iVma; - } - - ATRACE_BEGIN(StringPrintf("Compact %d VMAs", iVec).c_str()); - auto bytesProcessed = process_madvise(pidfd, vmasToKernel, iVec, madviseType, 0); - ATRACE_END(); - - if (CC_UNLIKELY(bytesProcessed == -1)) { - if (errno == EINVAL) { - // This error is somewhat common due to an unevictable VMA if this is - // the case silently skip the bad VMA and continue compacting the rest. - continue; - } else { - // Forward irrecoverable errors and bail out compaction - return -errno; - } - } - - totalBytesProcessed += bytesProcessed; + totalBytesProcessed += bytesProcessedInSend; + } while (batch.totalBytes > 0); } return totalBytesProcessed;