Update uid process state synchronously
ActivityTaskManagerService has a mirror ActiveUids. Originally it
may be updated to an intermediate state (e.g. UidRecord.reset) and
the uid state can be accessed before the actual value is set. That
leads to unexpected behavior depends on timing.
Now the mirror ActiveUids will only be updated when the actual uid
state is decided during updating oom-adj. And by acquiring window
manager's lock when updating oom-adj, it also synchronizes other
states with activity task manager. Although an additional lock is
held, it is possible to have benefit that reduces the frequency
of lock and unlock.
Also optimize the usage of priority booster by having an alias lock
with different type declaration, then the overhead of unnecessary
nested boost injections can be eliminated.
Bug: 123502026
Test: atest ActivityManagerServiceTest
Test: Manual measure until the last boot complete receiver is done:
- Invocation of nested priority boost
Reduce ~10000 times (50%, total 0.3s) boost invocation
- Invocation of updateOomAdjLocked
~170 times total reduces ~0.05s (only compare between with and
without global lock)
Change-Id: I160f951e103835401ccaaf7c732ba407e011c39b
diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java
index bec7386..280d825 100644
--- a/services/core/java/com/android/server/am/ActivityManagerService.java
+++ b/services/core/java/com/android/server/am/ActivityManagerService.java
@@ -2217,7 +2217,7 @@
mConstants = hasHandlerThread ? new ActivityManagerConstants(this, mHandler) : null;
final ActiveUids activeUids = new ActiveUids(this, false /* postChangesToAtm */);
mProcessList.init(this, activeUids);
- mOomAdjuster = new OomAdjuster(this, mProcessList, activeUids);
+ mOomAdjuster = new OomAdjuster(this, mProcessList, activeUids, new Object());
mIntentFirewall = hasHandlerThread
? new IntentFirewall(new IntentFirewallInterface(), mHandler) : null;
@@ -2265,7 +2265,7 @@
mConstants = new ActivityManagerConstants(this, mHandler);
final ActiveUids activeUids = new ActiveUids(this, true /* postChangesToAtm */);
mProcessList.init(this, activeUids);
- mOomAdjuster = new OomAdjuster(this, mProcessList, activeUids);
+ mOomAdjuster = new OomAdjuster(this, mProcessList, activeUids, atm.getGlobalLock());
// Broadcast policy parameters
final BroadcastConstants foreConstants = new BroadcastConstants(
@@ -3091,7 +3091,7 @@
} else {
UidRecord validateUid = mValidateUids.get(item.uid);
if (validateUid == null) {
- validateUid = new UidRecord(item.uid, mAtmInternal);
+ validateUid = new UidRecord(item.uid);
mValidateUids.put(item.uid, validateUid);
}
if ((item.change & UidRecord.CHANGE_IDLE) != 0) {
diff --git a/services/core/java/com/android/server/am/OomAdjuster.java b/services/core/java/com/android/server/am/OomAdjuster.java
index 6e8646e..4985c52 100644
--- a/services/core/java/com/android/server/am/OomAdjuster.java
+++ b/services/core/java/com/android/server/am/OomAdjuster.java
@@ -127,8 +127,18 @@
private final ActivityManagerService mService;
private final ProcessList mProcessList;
- OomAdjuster(ActivityManagerService service, ProcessList processList, ActiveUids activeUids) {
+ /**
+ * Used to lock {@link #updateOomAdjImpl} for state consistency. It also reduces frequency lock
+ * and unlock when getting and setting value to {@link ProcessRecord#mWindowProcessController}.
+ * Note it is declared as Object type so the locked-region-code-injection won't wrap the
+ * unnecessary priority booster.
+ */
+ private final Object mAtmGlobalLock;
+
+ OomAdjuster(ActivityManagerService service, ProcessList processList, ActiveUids activeUids,
+ Object atmGlobalLock) {
mService = service;
+ mAtmGlobalLock = atmGlobalLock;
mProcessList = processList;
mActiveUids = activeUids;
@@ -186,6 +196,13 @@
@GuardedBy("mService")
final void updateOomAdjLocked() {
+ synchronized (mAtmGlobalLock) {
+ updateOomAdjImpl();
+ }
+ }
+
+ @GuardedBy({"mService", "mAtmGlobalLock"})
+ private void updateOomAdjImpl() {
Trace.traceBegin(Trace.TRACE_TAG_ACTIVITY_MANAGER, "updateOomAdj");
mService.mOomAdjProfiler.oomAdjStarted();
final ProcessRecord TOP_APP = mService.getTopAppLocked();
@@ -534,6 +551,7 @@
uidRec.setProcState = uidRec.getCurProcState();
uidRec.setWhitelist = uidRec.curWhitelist;
uidRec.setIdle = uidRec.idle;
+ mService.mAtmInternal.onUidProcStateChanged(uidRec.uid, uidRec.setProcState);
mService.enqueueUidChangeLocked(uidRec, -1, uidChange);
mService.noteUidProcessState(uidRec.uid, uidRec.getCurProcState());
if (uidRec.foregroundServices) {
diff --git a/services/core/java/com/android/server/am/ProcessList.java b/services/core/java/com/android/server/am/ProcessList.java
index 4aa71f9..49c4bc4 100644
--- a/services/core/java/com/android/server/am/ProcessList.java
+++ b/services/core/java/com/android/server/am/ProcessList.java
@@ -2197,7 +2197,7 @@
}
UidRecord uidRec = mActiveUids.get(proc.uid);
if (uidRec == null) {
- uidRec = new UidRecord(proc.uid, mService.mAtmInternal);
+ uidRec = new UidRecord(proc.uid);
// This is the first appearance of the uid, report it now!
if (DEBUG_UID_OBSERVERS) Slog.i(TAG_UID_OBSERVERS,
"Creating new process uid: " + uidRec);
diff --git a/services/core/java/com/android/server/am/ProcessRecord.java b/services/core/java/com/android/server/am/ProcessRecord.java
index 580d688..fb3cd40 100644
--- a/services/core/java/com/android/server/am/ProcessRecord.java
+++ b/services/core/java/com/android/server/am/ProcessRecord.java
@@ -653,10 +653,6 @@
return mWindowProcessController.hasActivities();
}
- void clearActivities() {
- mWindowProcessController.clearActivities();
- }
-
boolean hasActivitiesOrRecentTasks() {
return mWindowProcessController.hasActivitiesOrRecentTasks();
}
@@ -665,10 +661,6 @@
return mWindowProcessController.hasRecentTasks();
}
- void clearRecentTasks() {
- mWindowProcessController.clearRecentTasks();
- }
-
/**
* This method returns true if any of the activities within the process record are interesting
* to the user. See HistoryRecord.isInterestingToUserLocked()
diff --git a/services/core/java/com/android/server/am/UidRecord.java b/services/core/java/com/android/server/am/UidRecord.java
index 6cb1097..22a7de7 100644
--- a/services/core/java/com/android/server/am/UidRecord.java
+++ b/services/core/java/com/android/server/am/UidRecord.java
@@ -26,7 +26,6 @@
import android.util.proto.ProtoUtils;
import com.android.internal.annotations.GuardedBy;
-import com.android.server.wm.ActivityTaskManagerInternal;
/**
* Overall information about a uid that has actively running processes.
@@ -43,7 +42,6 @@
boolean idle;
boolean setIdle;
int numProcs;
- final ActivityTaskManagerInternal mAtmInternal;
/**
* Sequence number associated with the {@link #mCurProcState}. This is incremented using
@@ -117,10 +115,9 @@
ChangeItem pendingChange;
int lastReportedChange;
- public UidRecord(int _uid, ActivityTaskManagerInternal atmInternal) {
+ public UidRecord(int _uid) {
uid = _uid;
idle = true;
- mAtmInternal = atmInternal;
reset();
}
@@ -130,9 +127,6 @@
public void setCurProcState(int curProcState) {
mCurProcState = curProcState;
- if (mAtmInternal != null) {
- mAtmInternal.onUidProcStateChanged(uid, curProcState);
- }
}
public void reset() {
diff --git a/services/core/java/com/android/server/wm/ActivityTaskManagerService.java b/services/core/java/com/android/server/wm/ActivityTaskManagerService.java
index 5fabde4..09ef4b9 100644
--- a/services/core/java/com/android/server/wm/ActivityTaskManagerService.java
+++ b/services/core/java/com/android/server/wm/ActivityTaskManagerService.java
@@ -349,6 +349,14 @@
/* Global service lock used by the package the owns this service. */
final WindowManagerGlobalLock mGlobalLock = new WindowManagerGlobalLock();
+ /**
+ * It is the same instance as {@link mGlobalLock}, just declared as a type that the
+ * locked-region-code-injection does't recognize it. It is used to skip wrapping priority
+ * booster for places that are already in the scope of another booster (e.g. computing oom-adj).
+ *
+ * @see WindowManagerThreadPriorityBooster
+ */
+ final Object mGlobalLockWithoutBoost = mGlobalLock;
ActivityStackSupervisor mStackSupervisor;
RootActivityContainer mRootActivityContainer;
WindowManagerService mWindowManager;
@@ -6837,7 +6845,7 @@
@Override
public WindowProcessController getTopApp() {
- synchronized (mGlobalLock) {
+ synchronized (mGlobalLockWithoutBoost) {
final ActivityRecord top = mRootActivityContainer.getTopResumedActivity();
return top != null ? top.app : null;
}
@@ -6845,7 +6853,7 @@
@Override
public void rankTaskLayersIfNeeded() {
- synchronized (mGlobalLock) {
+ synchronized (mGlobalLockWithoutBoost) {
if (mRootActivityContainer != null) {
mRootActivityContainer.rankTaskLayersIfNeeded();
}
@@ -6889,28 +6897,28 @@
@Override
public void onUidActive(int uid, int procState) {
- synchronized (mGlobalLock) {
+ synchronized (mGlobalLockWithoutBoost) {
mActiveUids.put(uid, procState);
}
}
@Override
public void onUidInactive(int uid) {
- synchronized (mGlobalLock) {
+ synchronized (mGlobalLockWithoutBoost) {
mActiveUids.remove(uid);
}
}
@Override
public void onActiveUidsCleared() {
- synchronized (mGlobalLock) {
+ synchronized (mGlobalLockWithoutBoost) {
mActiveUids.clear();
}
}
@Override
public void onUidProcStateChanged(int uid, int procState) {
- synchronized (mGlobalLock) {
+ synchronized (mGlobalLockWithoutBoost) {
if (mActiveUids.get(uid) != null) {
mActiveUids.put(uid, procState);
}
@@ -6919,14 +6927,14 @@
@Override
public void onUidAddedToPendingTempWhitelist(int uid, String tag) {
- synchronized (mGlobalLock) {
+ synchronized (mGlobalLockWithoutBoost) {
mPendingTempWhitelist.put(uid, tag);
}
}
@Override
public void onUidRemovedFromPendingTempWhitelist(int uid) {
- synchronized (mGlobalLock) {
+ synchronized (mGlobalLockWithoutBoost) {
mPendingTempWhitelist.remove(uid);
}
}
diff --git a/services/core/java/com/android/server/wm/WindowProcessController.java b/services/core/java/com/android/server/wm/WindowProcessController.java
index 030cc05..5233edc 100644
--- a/services/core/java/com/android/server/wm/WindowProcessController.java
+++ b/services/core/java/com/android/server/wm/WindowProcessController.java
@@ -394,13 +394,13 @@
}
public void addPackage(String packageName) {
- synchronized (mAtm.mGlobalLock) {
+ synchronized (mAtm.mGlobalLockWithoutBoost) {
mPkgList.add(packageName);
}
}
public void clearPackageList() {
- synchronized (mAtm.mGlobalLock) {
+ synchronized (mAtm.mGlobalLockWithoutBoost) {
mPkgList.clear();
}
}
@@ -422,20 +422,18 @@
}
}
- public void clearActivities() {
- synchronized (mAtm.mGlobalLock) {
- mActivities.clear();
- }
+ void clearActivities() {
+ mActivities.clear();
}
public boolean hasActivities() {
- synchronized (mAtm.mGlobalLock) {
+ synchronized (mAtm.mGlobalLockWithoutBoost) {
return !mActivities.isEmpty();
}
}
public boolean hasVisibleActivities() {
- synchronized (mAtm.mGlobalLock) {
+ synchronized (mAtm.mGlobalLockWithoutBoost) {
for (int i = mActivities.size() - 1; i >= 0; --i) {
final ActivityRecord r = mActivities.get(i);
if (r.visible) {
@@ -447,7 +445,7 @@
}
public boolean hasActivitiesOrRecentTasks() {
- synchronized (mAtm.mGlobalLock) {
+ synchronized (mAtm.mGlobalLockWithoutBoost) {
return !mActivities.isEmpty() || !mRecentTasks.isEmpty();
}
}
@@ -462,15 +460,13 @@
}
}
- public void finishActivities() {
- synchronized (mAtm.mGlobalLock) {
- ArrayList<ActivityRecord> activities = new ArrayList<>(mActivities);
- for (int i = 0; i < activities.size(); i++) {
- final ActivityRecord r = activities.get(i);
- if (!r.finishing && r.isInStackLocked()) {
- r.getActivityStack().finishActivityLocked(r, Activity.RESULT_CANCELED,
- null, "finish-heavy", true);
- }
+ void finishActivities() {
+ ArrayList<ActivityRecord> activities = new ArrayList<>(mActivities);
+ for (int i = 0; i < activities.size(); i++) {
+ final ActivityRecord r = activities.get(i);
+ if (!r.finishing && r.isInStackLocked()) {
+ r.getActivityStack().finishActivityLocked(r, Activity.RESULT_CANCELED,
+ null, "finish-heavy", true);
}
}
}
@@ -531,15 +527,13 @@
}
- public void updateIntentForHeavyWeightActivity(Intent intent) {
- synchronized (mAtm.mGlobalLock) {
- if (mActivities.isEmpty()) {
- return;
- }
- ActivityRecord hist = mActivities.get(0);
- intent.putExtra(HeavyWeightSwitcherActivity.KEY_CUR_APP, hist.packageName);
- intent.putExtra(HeavyWeightSwitcherActivity.KEY_CUR_TASK, hist.getTaskRecord().taskId);
+ void updateIntentForHeavyWeightActivity(Intent intent) {
+ if (mActivities.isEmpty()) {
+ return;
}
+ ActivityRecord hist = mActivities.get(0);
+ intent.putExtra(HeavyWeightSwitcherActivity.KEY_CUR_APP, hist.packageName);
+ intent.putExtra(HeavyWeightSwitcherActivity.KEY_CUR_TASK, hist.getTaskRecord().taskId);
}
boolean shouldKillProcessForRemovedTask(TaskRecord tr) {
@@ -609,7 +603,7 @@
}
public int computeOomAdjFromActivities(int minTaskLayer, ComputeOomAdjCallback callback) {
- synchronized (mAtm.mGlobalLock) {
+ synchronized (mAtm.mGlobalLockWithoutBoost) {
final int activitiesSize = mActivities.size();
for (int j = 0; j < activitiesSize; j++) {
final ActivityRecord r = mActivities.get(j);
@@ -842,18 +836,16 @@
}
public boolean hasRecentTasks() {
- synchronized (mAtm.mGlobalLock) {
+ synchronized (mAtm.mGlobalLockWithoutBoost) {
return !mRecentTasks.isEmpty();
}
}
- public void clearRecentTasks() {
- synchronized (mAtm.mGlobalLock) {
- for (int i = mRecentTasks.size() - 1; i >= 0; i--) {
- mRecentTasks.get(i).clearRootProcess();
- }
- mRecentTasks.clear();
+ void clearRecentTasks() {
+ for (int i = mRecentTasks.size() - 1; i >= 0; i--) {
+ mRecentTasks.get(i).clearRootProcess();
}
+ mRecentTasks.clear();
}
public void appEarlyNotResponding(String annotation, Runnable killAppCallback) {
@@ -907,19 +899,19 @@
}
public void onTopProcChanged() {
- synchronized (mAtm.mGlobalLock) {
+ synchronized (mAtm.mGlobalLockWithoutBoost) {
mAtm.mVrController.onTopProcChangedLocked(this);
}
}
public boolean isHomeProcess() {
- synchronized (mAtm.mGlobalLock) {
+ synchronized (mAtm.mGlobalLockWithoutBoost) {
return this == mAtm.mHomeProcess;
}
}
public boolean isPreviousProcess() {
- synchronized (mAtm.mGlobalLock) {
+ synchronized (mAtm.mGlobalLockWithoutBoost) {
return this == mAtm.mPreviousProcess;
}
}
diff --git a/services/tests/servicestests/src/com/android/server/am/ActivityManagerInternalTest.java b/services/tests/servicestests/src/com/android/server/am/ActivityManagerInternalTest.java
index 00a60b9..5dafe07 100644
--- a/services/tests/servicestests/src/com/android/server/am/ActivityManagerInternalTest.java
+++ b/services/tests/servicestests/src/com/android/server/am/ActivityManagerInternalTest.java
@@ -122,7 +122,7 @@
private UidRecord addActiveUidRecord(int uid, long curProcStateSeq,
long lastNetworkUpdatedProcStateSeq) {
- final UidRecord record = new UidRecord(uid, null /* atmInternal */);
+ final UidRecord record = new UidRecord(uid);
record.lastNetworkUpdatedProcStateSeq = lastNetworkUpdatedProcStateSeq;
record.curProcStateSeq = curProcStateSeq;
record.waitingForNetwork = true;
diff --git a/services/tests/servicestests/src/com/android/server/am/ActivityManagerServiceTest.java b/services/tests/servicestests/src/com/android/server/am/ActivityManagerServiceTest.java
index 419c736..4e21fd0c 100644
--- a/services/tests/servicestests/src/com/android/server/am/ActivityManagerServiceTest.java
+++ b/services/tests/servicestests/src/com/android/server/am/ActivityManagerServiceTest.java
@@ -250,7 +250,7 @@
}
private UidRecord addUidRecord(int uid) {
- final UidRecord uidRec = new UidRecord(uid, null /* atmInternal */);
+ final UidRecord uidRec = new UidRecord(uid);
uidRec.waitingForNetwork = true;
uidRec.hasInternetPermission = true;
mAms.mProcessList.mActiveUids.put(uid, uidRec);
@@ -405,7 +405,7 @@
@Test
public void testBlockStateForUid() {
- final UidRecord uidRec = new UidRecord(TEST_UID, null /* atmInternal */);
+ final UidRecord uidRec = new UidRecord(TEST_UID);
int expectedBlockState;
final String errorTemplate = "Block state should be %s, prevState: %s, curState: %s";
@@ -732,7 +732,7 @@
@Test
public void testEnqueueUidChangeLocked_procStateSeqUpdated() {
- final UidRecord uidRecord = new UidRecord(TEST_UID, null /* atmInternal */);
+ final UidRecord uidRecord = new UidRecord(TEST_UID);
uidRecord.curProcStateSeq = TEST_PROC_STATE_SEQ1;
// Verify with no pending changes for TEST_UID.
@@ -778,7 +778,7 @@
@MediumTest
@Test
public void testEnqueueUidChangeLocked_dispatchUidsChanged() {
- final UidRecord uidRecord = new UidRecord(TEST_UID, null /* atmInternal */);
+ final UidRecord uidRecord = new UidRecord(TEST_UID);
final int expectedProcState = PROCESS_STATE_SERVICE;
uidRecord.setProcState = expectedProcState;
uidRecord.curProcStateSeq = TEST_PROC_STATE_SEQ1;
@@ -850,7 +850,7 @@
private void verifyWaitingForNetworkStateUpdate(long curProcStateSeq,
long lastDispatchedProcStateSeq, long lastNetworkUpdatedProcStateSeq,
final long procStateSeqToWait, boolean expectWait) throws Exception {
- final UidRecord record = new UidRecord(Process.myUid(), null /* atmInternal */);
+ final UidRecord record = new UidRecord(Process.myUid());
record.curProcStateSeq = curProcStateSeq;
record.lastDispatchedProcStateSeq = lastDispatchedProcStateSeq;
record.lastNetworkUpdatedProcStateSeq = lastNetworkUpdatedProcStateSeq;