From 75a4e3f38c52366d59447cc865b65e4d51622697 Mon Sep 17 00:00:00 2001 From: Jing Ji Date: Fri, 19 Mar 2021 22:20:29 -0700 Subject: [PATCH] DO NOT MERGE: Don't take the AM lock in cached app compaction handler A single oomAdjUpdate pass could trigger quite a few app compactions, each of them would need to acquire the AM lock during the handling, results in lock contentions. Now use its own lock. Also skip the scheduling if the given app is pending with compaction already. Bug: 163297662 Test: atest CachedAppOptimizerTest Test: Manual - Boot & check the logs Change-Id: I4618a3ae78838cd9783a431e7364130270ebb4d6 --- .../server/am/ActivityManagerService.java | 5 +- .../android/server/am/CachedAppOptimizer.java | 83 ++++++++++++------- .../com/android/server/am/OomAdjuster.java | 3 + .../com/android/server/am/ProcessRecord.java | 31 ++++++- .../server/am/CachedAppOptimizerTest.java | 4 +- 5 files changed, 92 insertions(+), 34 deletions(-) diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index 5134f498928b4..d7a9e05d94aad 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -2124,7 +2124,7 @@ public class ActivityManagerService extends IActivityManager.Stub 0, new HostingRecord("system")); app.setPersistent(true); - app.pid = MY_PID; + app.pid = app.mPidForCompact = MY_PID; app.getWindowProcessController().setPid(MY_PID); app.maxAdj = ProcessList.SYSTEM_ADJ; app.makeActive(mSystemThread.getApplicationThread(), mProcessStats); @@ -5105,6 +5105,9 @@ public class ActivityManagerService extends IActivityManager.Stub EventLogTags.writeAmProcBound(app.userId, app.pid, app.processName); app.curAdj = app.setAdj = app.verifiedAdj = ProcessList.INVALID_ADJ; + synchronized (mOomAdjuster.mCachedAppOptimizer) { + app.mSetAdjForCompact = ProcessList.INVALID_ADJ; + } mOomAdjuster.setAttachingSchedGroupLocked(app); app.forcingToImportant = null; updateProcessForegroundLocked(app, false, 0, false); diff --git a/services/core/java/com/android/server/am/CachedAppOptimizer.java b/services/core/java/com/android/server/am/CachedAppOptimizer.java index edd0c5b72c5e8..2f776fc55034f 100644 --- a/services/core/java/com/android/server/am/CachedAppOptimizer.java +++ b/services/core/java/com/android/server/am/CachedAppOptimizer.java @@ -151,6 +151,7 @@ public final class CachedAppOptimizer { */ final ServiceThread mCachedAppOptimizerThread; + @GuardedBy("this") private final ArrayList mPendingCompactionProcesses = new ArrayList(); private final ActivityManagerService mAm; @@ -348,51 +349,74 @@ public final class CachedAppOptimizer { @GuardedBy("mAm") void compactAppSome(ProcessRecord app) { - app.reqCompactAction = COMPACT_PROCESS_SOME; - mPendingCompactionProcesses.add(app); - mCompactionHandler.sendMessage( - mCompactionHandler.obtainMessage( - COMPACT_PROCESS_MSG, app.setAdj, app.setProcState)); + synchronized (this) { + app.reqCompactAction = COMPACT_PROCESS_SOME; + if (!app.mPendingCompact) { + app.mPendingCompact = true; + mPendingCompactionProcesses.add(app); + mCompactionHandler.sendMessage( + mCompactionHandler.obtainMessage( + COMPACT_PROCESS_MSG, app.setAdj, app.setProcState)); + } + } } @GuardedBy("mAm") void compactAppFull(ProcessRecord app) { - app.reqCompactAction = COMPACT_PROCESS_FULL; - mPendingCompactionProcesses.add(app); - mCompactionHandler.sendMessage( - mCompactionHandler.obtainMessage( - COMPACT_PROCESS_MSG, app.setAdj, app.setProcState)); - + synchronized (this) { + app.reqCompactAction = COMPACT_PROCESS_FULL; + if (!app.mPendingCompact) { + app.mPendingCompact = true; + mPendingCompactionProcesses.add(app); + mCompactionHandler.sendMessage( + mCompactionHandler.obtainMessage( + COMPACT_PROCESS_MSG, app.setAdj, app.setProcState)); + } + } } @GuardedBy("mAm") void compactAppPersistent(ProcessRecord app) { - app.reqCompactAction = COMPACT_PROCESS_PERSISTENT; - mPendingCompactionProcesses.add(app); - mCompactionHandler.sendMessage( - mCompactionHandler.obtainMessage( - COMPACT_PROCESS_MSG, app.curAdj, app.setProcState)); + synchronized (this) { + app.reqCompactAction = COMPACT_PROCESS_PERSISTENT; + if (!app.mPendingCompact) { + app.mPendingCompact = true; + mPendingCompactionProcesses.add(app); + mCompactionHandler.sendMessage( + mCompactionHandler.obtainMessage( + COMPACT_PROCESS_MSG, app.curAdj, app.setProcState)); + } + } } @GuardedBy("mAm") boolean shouldCompactPersistent(ProcessRecord app, long now) { - return (app.lastCompactTime == 0 - || (now - app.lastCompactTime) > mCompactThrottlePersistent); + synchronized (this) { + return (app.lastCompactTime == 0 + || (now - app.lastCompactTime) > mCompactThrottlePersistent); + } } @GuardedBy("mAm") void compactAppBfgs(ProcessRecord app) { - app.reqCompactAction = COMPACT_PROCESS_BFGS; - mPendingCompactionProcesses.add(app); - mCompactionHandler.sendMessage( - mCompactionHandler.obtainMessage( - COMPACT_PROCESS_MSG, app.curAdj, app.setProcState)); + synchronized (this) { + app.reqCompactAction = COMPACT_PROCESS_BFGS; + if (!app.mPendingCompact) { + app.mPendingCompact = true; + mPendingCompactionProcesses.add(app); + mCompactionHandler.sendMessage( + mCompactionHandler.obtainMessage( + COMPACT_PROCESS_MSG, app.curAdj, app.setProcState)); + } + } } @GuardedBy("mAm") boolean shouldCompactBFGS(ProcessRecord app, long now) { - return (app.lastCompactTime == 0 - || (now - app.lastCompactTime) > mCompactThrottleBFGS); + synchronized (this) { + return (app.lastCompactTime == 0 + || (now - app.lastCompactTime) > mCompactThrottleBFGS); + } } @GuardedBy("mAm") @@ -854,18 +878,19 @@ public final class CachedAppOptimizer { LastCompactionStats lastCompactionStats; int lastOomAdj = msg.arg1; int procState = msg.arg2; - synchronized (mAm) { + synchronized (CachedAppOptimizer.this) { proc = mPendingCompactionProcesses.remove(0); pendingAction = proc.reqCompactAction; - pid = proc.pid; + pid = proc.mPidForCompact; name = proc.processName; + proc.mPendingCompact = false; // 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.setAdj <= ProcessList.PERCEPTIBLE_APP_ADJ)) { + && (proc.mSetAdjForCompact <= ProcessList.PERCEPTIBLE_APP_ADJ)) { if (DEBUG_COMPACTION) { Slog.d(TAG_AM, "Skipping compaction as process " + name + " is " @@ -1052,7 +1077,7 @@ public final class CachedAppOptimizer { lastOomAdj, ActivityManager.processStateAmToProto(procState), zramFreeKbBefore, zramFreeKbAfter); } - synchronized (mAm) { + synchronized (CachedAppOptimizer.this) { proc.lastCompactTime = end; proc.lastCompactAction = pendingAction; } diff --git a/services/core/java/com/android/server/am/OomAdjuster.java b/services/core/java/com/android/server/am/OomAdjuster.java index f0343e1d807ca..faa7dce316900 100644 --- a/services/core/java/com/android/server/am/OomAdjuster.java +++ b/services/core/java/com/android/server/am/OomAdjuster.java @@ -2198,6 +2198,9 @@ public final class OomAdjuster { } app.setAdj = app.curAdj; app.verifiedAdj = ProcessList.INVALID_ADJ; + synchronized (mCachedAppOptimizer) { + app.mSetAdjForCompact = app.setAdj; + } } final int curSchedGroup = app.getCurrentSchedulingGroup(); diff --git a/services/core/java/com/android/server/am/ProcessRecord.java b/services/core/java/com/android/server/am/ProcessRecord.java index c5152c081e70e..284903d390d46 100644 --- a/services/core/java/com/android/server/am/ProcessRecord.java +++ b/services/core/java/com/android/server/am/ProcessRecord.java @@ -59,6 +59,7 @@ import android.util.SparseArray; import android.util.TimeUtils; import android.util.proto.ProtoOutputStream; +import com.android.internal.annotations.GuardedBy; import com.android.internal.annotations.VisibleForTesting; import com.android.internal.app.procstats.ProcessState; import com.android.internal.app.procstats.ProcessStats; @@ -162,8 +163,11 @@ class ProcessRecord implements WindowProcessListener { int curCapability; // Current capability flags of this process. For example, // PROCESS_CAPABILITY_FOREGROUND_LOCATION is one capability. int setCapability; // Last set capability flags. + @GuardedBy("mService.mOomAdjuster.mCachedAppOptimizer") long lastCompactTime; // The last time that this process was compacted + @GuardedBy("mService.mOomAdjuster.mCachedAppOptimizer") int reqCompactAction; // The most recent compaction action requested for this app. + @GuardedBy("mService.mOomAdjuster.mCachedAppOptimizer") int lastCompactAction; // The most recent compaction action performed for this app. boolean frozen; // True when the process is frozen. long freezeUnfreezeTime; // Last time the app was (un)frozen, 0 for never @@ -352,6 +356,24 @@ class ProcessRecord implements WindowProcessListener { boolean mReachable; // Whether or not this process is reachable from given process + /** + * The snapshot of {@link #setAdj}, meant to be read by {@link CachedAppOptimizer} only. + */ + @GuardedBy("mService.mOomAdjuster.mCachedAppOptimizer") + int mSetAdjForCompact; + + /** + * The snapshot of {@link #pid}, meant to be read by {@link CachedAppOptimizer} only. + */ + @GuardedBy("mService.mOomAdjuster.mCachedAppOptimizer") + int mPidForCompact; + + /** + * This process has been scheduled for a memory compaction. + */ + @GuardedBy("mService.mOomAdjuster.mCachedAppOptimizer") + boolean mPendingCompact; + void setStartParams(int startUid, HostingRecord hostingRecord, String seInfo, long startTime) { this.startUid = startUid; @@ -447,8 +469,10 @@ class ProcessRecord implements WindowProcessListener { pw.print(" setRaw="); pw.print(setRawAdj); pw.print(" cur="); pw.print(curAdj); pw.print(" set="); pw.println(setAdj); - pw.print(prefix); pw.print("lastCompactTime="); pw.print(lastCompactTime); - pw.print(" lastCompactAction="); pw.println(lastCompactAction); + synchronized (mService.mOomAdjuster.mCachedAppOptimizer) { + pw.print(prefix); pw.print("lastCompactTime="); pw.print(lastCompactTime); + pw.print(" lastCompactAction="); pw.println(lastCompactAction); + } pw.print(prefix); pw.print("mCurSchedGroup="); pw.print(mCurSchedGroup); pw.print(" setSchedGroup="); pw.print(setSchedGroup); pw.print(" systemNoUi="); pw.print(systemNoUi); @@ -672,6 +696,9 @@ class ProcessRecord implements WindowProcessListener { public void setPid(int _pid) { pid = _pid; + synchronized (mService.mOomAdjuster.mCachedAppOptimizer) { + mPidForCompact = _pid; + } mWindowProcessController.setPid(pid); procStatFile = null; shortStringName = null; 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 96a44a46bbafb..8d245ce4c6434 100644 --- a/services/tests/mockingservicestests/src/com/android/server/am/CachedAppOptimizerTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/am/CachedAppOptimizerTest.java @@ -134,11 +134,11 @@ public final class CachedAppOptimizerTest { ApplicationInfo ai = new ApplicationInfo(); ai.packageName = packageName; ProcessRecord app = new ProcessRecord(mAms, ai, processName, uid); - app.pid = pid; + app.pid = app.mPidForCompact = pid; app.info.uid = packageUid; // Exact value does not mater, it can be any state for which compaction is allowed. app.setProcState = PROCESS_STATE_BOUND_FOREGROUND_SERVICE; - app.setAdj = 905; + app.setAdj = app.mSetAdjForCompact = 905; return app; }