From: Riddle Hsu Date: Wed, 30 Jan 2019 05:04:50 +0000 (+0800) Subject: Update uid process state synchronously X-Git-Url: http://git.osdn.net/view?a=commitdiff_plain;h=d7088f8f76795808c530bed61926457db33bf7a0;p=android-x86%2Fframeworks-base.git 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 bec738666cbb..280d825a2228 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 @@ public class ActivityManagerService extends IActivityManager.Stub 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 @@ public class ActivityManagerService extends IActivityManager.Stub 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 @@ public class ActivityManagerService extends IActivityManager.Stub } 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 6e8646e476b3..4985c52ea118 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 @@ public final class OomAdjuster { 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 @@ public final class OomAdjuster { @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 @@ public final class OomAdjuster { 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 4aa71f9fbbe1..49c4bc486d2e 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 @@ public final class ProcessList { } 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 580d6882a82e..fb3cd40b70fd 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 @@ final class ProcessRecord implements WindowProcessListener { return mWindowProcessController.hasActivities(); } - void clearActivities() { - mWindowProcessController.clearActivities(); - } - boolean hasActivitiesOrRecentTasks() { return mWindowProcessController.hasActivitiesOrRecentTasks(); } @@ -665,10 +661,6 @@ final class ProcessRecord implements WindowProcessListener { 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 6cb1097c7ddf..22a7de74cc38 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.ProtoOutputStream; 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 @@ public final class UidRecord { boolean idle; boolean setIdle; int numProcs; - final ActivityTaskManagerInternal mAtmInternal; /** * Sequence number associated with the {@link #mCurProcState}. This is incremented using @@ -117,10 +115,9 @@ public final class UidRecord { 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 final class UidRecord { 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 5fabde45db55..09ef4b95e66b 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 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub { /* 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 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub { @Override public WindowProcessController getTopApp() { - synchronized (mGlobalLock) { + synchronized (mGlobalLockWithoutBoost) { final ActivityRecord top = mRootActivityContainer.getTopResumedActivity(); return top != null ? top.app : null; } @@ -6845,7 +6853,7 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub { @Override public void rankTaskLayersIfNeeded() { - synchronized (mGlobalLock) { + synchronized (mGlobalLockWithoutBoost) { if (mRootActivityContainer != null) { mRootActivityContainer.rankTaskLayersIfNeeded(); } @@ -6889,28 +6897,28 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub { @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 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub { @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 030cc05a2f7a..5233edcb7597 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 class WindowProcessController extends ConfigurationContainer= 0; --i) { final ActivityRecord r = mActivities.get(i); if (r.visible) { @@ -447,7 +445,7 @@ public class WindowProcessController extends ConfigurationContainer 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 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 class WindowProcessController extends ConfigurationContainer= 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 class WindowProcessController extends ConfigurationContainer