Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit d7088f8f authored by Riddle Hsu's avatar Riddle Hsu
Browse files

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
parent 195bafa1
Loading
Loading
Loading
Loading
+3 −3
Original line number Diff line number Diff line
@@ -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) {
+19 −1
Original line number Diff line number Diff line
@@ -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) {
+1 −1
Original line number Diff line number Diff line
@@ -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);
+0 −8
Original line number Diff line number Diff line
@@ -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()
+1 −7
Original line number Diff line number Diff line
@@ -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() {
Loading