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

Commit 02a161b4 authored by Jing Ji's avatar Jing Ji
Browse files

Re-compute oom adj in case cycles are found in partial update

The partial oom adj update could encounter cycles, in which case
its omm adj score shouldn't be used. We'll need to perform another
round of computation on all its associated processes.

Meanwhile, removed one of the legacy updateOomAdjLocked variants,
as it's always calling into another variant actually.

Bug: 184310015
Test: atest MockingOomAdjusterTests
Test: atest CtsAppTestCases
Change-Id: I67815344134ca122312c3242a18083cafe1519a5
parent 87d4c9a9
Loading
Loading
Loading
Loading
+4 −5
Original line number Diff line number Diff line
@@ -3799,7 +3799,7 @@ public final class ActiveServices {
            bumpServiceExecutingLocked(r, execInFg, "start");
            if (!oomAdjusted) {
                oomAdjusted = true;
                mAm.updateOomAdjLocked(r.app, true, OomAdjuster.OOM_ADJ_REASON_START_SERVICE);
                mAm.updateOomAdjLocked(r.app, OomAdjuster.OOM_ADJ_REASON_START_SERVICE);
            }
            if (r.fgRequired && !r.fgWaiting) {
                if (!r.isForeground) {
@@ -4070,7 +4070,7 @@ public final class ActiveServices {
            if (enqueueOomAdj) {
                mAm.enqueueOomAdjTargetLocked(r.app);
            } else {
                mAm.updateOomAdjLocked(r.app, true, OomAdjuster.OOM_ADJ_REASON_UNBIND_SERVICE);
                mAm.updateOomAdjLocked(r.app, OomAdjuster.OOM_ADJ_REASON_UNBIND_SERVICE);
            }
        }
        if (r.bindings.size() > 0) {
@@ -4168,8 +4168,7 @@ public final class ActiveServices {
                    if (enqueueOomAdj) {
                        mAm.enqueueOomAdjTargetLocked(s.app);
                    } else {
                        mAm.updateOomAdjLocked(s.app, true,
                                OomAdjuster.OOM_ADJ_REASON_UNBIND_SERVICE);
                        mAm.updateOomAdjLocked(s.app, OomAdjuster.OOM_ADJ_REASON_UNBIND_SERVICE);
                    }
                    b.intent.hasBound = false;
                    // Assume the client doesn't want to know about a rebind;
@@ -4333,7 +4332,7 @@ public final class ActiveServices {
                if (enqueueOomAdj) {
                    mAm.enqueueOomAdjTargetLocked(r.app);
                } else {
                    mAm.updateOomAdjLocked(r.app, true, OomAdjuster.OOM_ADJ_REASON_UNBIND_SERVICE);
                    mAm.updateOomAdjLocked(r.app, OomAdjuster.OOM_ADJ_REASON_UNBIND_SERVICE);
                }
            }
            r.executeFg = false;
+9 −21
Original line number Diff line number Diff line
@@ -6890,7 +6890,7 @@ public class ActivityManagerService extends IActivityManager.Stub
                    }
                }
                if (changed) {
                    updateOomAdjLocked(pr, true, OomAdjuster.OOM_ADJ_REASON_UI_VISIBILITY);
                    updateOomAdjLocked(pr, OomAdjuster.OOM_ADJ_REASON_UI_VISIBILITY);
                }
            }
        } finally {
@@ -12053,7 +12053,7 @@ public class ActivityManagerService extends IActivityManager.Stub
            mBackupTargets.put(targetUserId, r);
            // Try not to kill the process during backup
            updateOomAdjLocked(proc, true, OomAdjuster.OOM_ADJ_REASON_NONE);
            updateOomAdjLocked(proc, OomAdjuster.OOM_ADJ_REASON_NONE);
            // If the process is already attached, schedule the creation of the backup agent now.
            // If it is not yet live, this will be done when it attaches to the framework.
@@ -12170,7 +12170,7 @@ public class ActivityManagerService extends IActivityManager.Stub
                // Not backing this app up any more; reset its OOM adjustment
                final ProcessRecord proc = backupTarget.app;
                updateOomAdjLocked(proc, true, OomAdjuster.OOM_ADJ_REASON_NONE);
                updateOomAdjLocked(proc, OomAdjuster.OOM_ADJ_REASON_NONE);
                proc.setInFullBackup(false);
                oldBackupUid = backupTarget != null ? backupTarget.appInfo.uid : -1;
@@ -14430,20 +14430,6 @@ public class ActivityManagerService extends IActivityManager.Stub
        return r;
    }
    /**
     * Update OomAdj for a specific process.
     * @param app The process to update
     * @param oomAdjAll If it's ok to call updateOomAdjLocked() for all running apps
     *                  if necessary, or skip.
     * @param oomAdjReason
     * @return whether updateOomAdjLocked(app) was successful.
     */
    @GuardedBy("this")
    final boolean updateOomAdjLocked(ProcessRecord app, boolean oomAdjAll,
            String oomAdjReason) {
        return mOomAdjuster.updateOomAdjLocked(app, oomAdjAll, oomAdjReason);
    }
    /**
     * Enqueue the given process into a todo list, and the caller should
     * call {@link #updateOomAdjPendingTargetsLocked} to kick off a pass of the oom adj update.
@@ -14489,14 +14475,16 @@ public class ActivityManagerService extends IActivityManager.Stub
        mOomAdjuster.updateOomAdjLocked(oomAdjReason);
    }
    /*
    /**
     * Update OomAdj for a specific process and its reachable processes.
     *
     * @param app The process to update
     * @param oomAdjReason
     * @return whether updateOomAdjLocked(app) was successful.
     */
    @GuardedBy("this")
    final void updateOomAdjLocked(ProcessRecord app, String oomAdjReason) {
        mOomAdjuster.updateOomAdjLocked(app, oomAdjReason);
    final boolean updateOomAdjLocked(ProcessRecord app, String oomAdjReason) {
        return mOomAdjuster.updateOomAdjLocked(app, oomAdjReason);
    }
    @Override
@@ -15393,7 +15381,7 @@ public class ActivityManagerService extends IActivityManager.Stub
                }
                pr.mState.setHasOverlayUi(hasOverlayUi);
                //Slog.i(TAG, "Setting hasOverlayUi=" + pr.hasOverlayUi + " for pid=" + pid);
                updateOomAdjLocked(pr, true, OomAdjuster.OOM_ADJ_REASON_UI_VISIBILITY);
                updateOomAdjLocked(pr, OomAdjuster.OOM_ADJ_REASON_UI_VISIBILITY);
            }
        }
+2 −2
Original line number Diff line number Diff line
@@ -245,7 +245,7 @@ public class ContentProviderHelper {

                    checkTime(startTime, "getContentProviderImpl: before updateOomAdj");
                    final int verifiedAdj = cpr.proc.mState.getVerifiedAdj();
                    boolean success = mService.updateOomAdjLocked(cpr.proc, true,
                    boolean success = mService.updateOomAdjLocked(cpr.proc,
                            OomAdjuster.OOM_ADJ_REASON_GET_PROVIDER);
                    // XXX things have changed so updateOomAdjLocked doesn't actually tell us
                    // if the process has been successfully adjusted.  So to reduce races with
@@ -684,7 +684,7 @@ public class ContentProviderHelper {

            // update the app's oom adj value and each provider's usage stats
            if (providersPublished) {
                mService.updateOomAdjLocked(r, true, OomAdjuster.OOM_ADJ_REASON_GET_PROVIDER);
                mService.updateOomAdjLocked(r, OomAdjuster.OOM_ADJ_REASON_GET_PROVIDER);
                for (int i = 0, size = providers.size(); i < size; i++) {
                    ContentProviderHolder src = providers.get(i);
                    if (src == null || src.info == null || src.provider == null) {
+43 −87
Original line number Diff line number Diff line
@@ -238,6 +238,7 @@ public class OomAdjuster {
    private final ActiveUids mTmpUidRecords;
    private final ArrayDeque<ProcessRecord> mTmpQueue;
    private final ArraySet<ProcessRecord> mPendingProcessSet = new ArraySet<>();
    private final ArraySet<ProcessRecord> mProcessesInCycle = new ArraySet<>();

    /**
     * Flag to mark if there is an ongoing oomAdjUpdate: potentially the oomAdjUpdate
@@ -469,76 +470,12 @@ public class OomAdjuster {
    }

    /**
     * Update OomAdj for a specific process.
     * @param app The process to update
     * @param oomAdjAll If it's ok to call updateOomAdjLocked() for all running apps
     *                  if necessary, or skip.
     * @param oomAdjReason
     * @return whether updateOomAdjLocked(app) was successful.
     * Perform oom adj update on the given process. It does NOT do the re-computation
     * if there is a cycle, caller should check {@link #mProcessesInCycle} and do it on its own.
     */
    @GuardedBy("mService")
    boolean updateOomAdjLocked(ProcessRecord app, boolean oomAdjAll,
            String oomAdjReason) {
        synchronized (mProcLock) {
            return updateOomAdjLSP(app, oomAdjAll, oomAdjReason);
        }
    }

    @GuardedBy({"mService", "mProcLock"})
    private boolean updateOomAdjLSP(ProcessRecord app, boolean oomAdjAll,
            String oomAdjReason) {
        if (oomAdjAll && mConstants.OOMADJ_UPDATE_QUICK) {
            return updateOomAdjLSP(app, oomAdjReason);
        }
        if (checkAndEnqueueOomAdjTargetLocked(app)) {
            // Simply return true as there is an oomAdjUpdate ongoing
            return true;
        }
        try {
            mOomAdjUpdateOngoing = true;
            return performUpdateOomAdjLSP(app, oomAdjAll, oomAdjReason);
        } finally {
            // Kick off the handling of any pending targets enqueued during the above update
            mOomAdjUpdateOngoing = false;
            updateOomAdjPendingTargetsLocked(oomAdjReason);
        }
    }

    @GuardedBy({"mService", "mProcLock"})
    private boolean performUpdateOomAdjLSP(ProcessRecord app, boolean oomAdjAll,
            String oomAdjReason) {
        final ProcessRecord topApp = mService.getTopApp();
        final ProcessStateRecord state = app.mState;
        final boolean wasCached = state.isCached();
        final int oldCap = state.getSetCapability();

        mAdjSeq++;

        // This is the desired cached adjusment we want to tell it to use.
        // If our app is currently cached, we know it, and that is it.  Otherwise,
        // we don't know it yet, and it needs to now be cached we will then
        // need to do a complete oom adj.
        final int cachedAdj = state.getCurRawAdj() >= ProcessList.CACHED_APP_MIN_ADJ
                ? state.getCurRawAdj() : ProcessList.UNKNOWN_ADJ;
        // Check if this process is in the pending list too, remove from pending list if so.
        mPendingProcessSet.remove(app);
        boolean success = performUpdateOomAdjLSP(app, cachedAdj, topApp, false,
                SystemClock.uptimeMillis());
        if (oomAdjAll
                && (wasCached != state.isCached()
                    || oldCap != state.getSetCapability()
                    || state.getCurRawAdj() == ProcessList.UNKNOWN_ADJ)) {
            // Changed to/from cached state, so apps after it in the LRU
            // list may also be changed.
            performUpdateOomAdjLSP(oomAdjReason);
        }

        return success;
    }

    @GuardedBy({"mService", "mProcLock"})
    private boolean performUpdateOomAdjLSP(ProcessRecord app, int cachedAdj,
            ProcessRecord TOP_APP, boolean doingAll, long now) {
            ProcessRecord topApp, long now) {
        if (app.getThread() == null) {
            return false;
        }
@@ -555,7 +492,16 @@ public class OomAdjuster {
        // Check if this process is in the pending list too, remove from pending list if so.
        mPendingProcessSet.remove(app);

        computeOomAdjLSP(app, cachedAdj, TOP_APP, doingAll, now, false, true);
        mProcessesInCycle.clear();
        computeOomAdjLSP(app, cachedAdj, topApp, false, now, false, true);
        if (!mProcessesInCycle.isEmpty()) {
            // We can't use the score here if there is a cycle, abort.
            for (int i = mProcessesInCycle.size() - 1; i >= 0; i--) {
                // Reset the adj seq
                mProcessesInCycle.valueAt(i).mState.setCompletedAdjSeq(mAdjSeq - 1);
            }
            return true;
        }

        if (uidRec != null) {
            // After uidRec.reset() above, for UidRecord with multiple processes (ProcessRecord),
@@ -573,7 +519,7 @@ public class OomAdjuster {
            }
        }

        return applyOomAdjLSP(app, doingAll, now, SystemClock.elapsedRealtime());
        return applyOomAdjLSP(app, false, now, SystemClock.elapsedRealtime());
    }

    /**
@@ -670,12 +616,16 @@ public class OomAdjuster {
        state.resetCachedInfo();
        // Check if this process is in the pending list too, remove from pending list if so.
        mPendingProcessSet.remove(app);
        boolean success = performUpdateOomAdjLSP(app, cachedAdj, topApp, false,
        boolean success = performUpdateOomAdjLSP(app, cachedAdj, topApp,
                SystemClock.uptimeMillis());
        // The 'app' here itself might or might not be in the cycle, for example,
        // the case A <=> B vs. A -> B <=> C; anyway, if we spot a cycle here, re-compute them.
        if (!success || (wasCached == state.isCached() && oldAdj != ProcessList.INVALID_ADJ
                && mProcessesInCycle.isEmpty() /* Force re-compute if there is a cycle */
                && oldCap == state.getCurCapability()
                && wasBackground == ActivityManager.isProcStateBackground(
                        state.getSetProcState()))) {
            mProcessesInCycle.clear();
            // Okay, it's unchanged, it won't impact any service it binds to, we're done here.
            if (DEBUG_OOM_ADJ) {
                Slog.i(TAG_OOM_ADJ, "No oomadj changes for " + app);
@@ -690,15 +640,24 @@ public class OomAdjuster {
        ActiveUids uids = mTmpUidRecords;
        mPendingProcessSet.add(app);

        // Add all processes with cycles into the list to scan
        for (int i = mProcessesInCycle.size() - 1; i >= 0; i--) {
            mPendingProcessSet.add(mProcessesInCycle.valueAt(i));
        }
        mProcessesInCycle.clear();

        boolean containsCycle = collectReachableProcessesLocked(mPendingProcessSet,
                processes, uids);

        // Clear the pending set as they should've been included in 'processes'.
        mPendingProcessSet.clear();

        if (!containsCycle) {
            // Reset the flag
            state.setReachable(false);
            // Remove this app from the return list because we've done the computation on it.
            processes.remove(app);
        }

        int size = processes.size();
        if (size > 0) {
@@ -724,19 +683,13 @@ public class OomAdjuster {
            ArrayList<ProcessRecord> processes, ActiveUids uids) {
        final ArrayDeque<ProcessRecord> queue = mTmpQueue;
        queue.clear();
        processes.clear();
        for (int i = 0, size = apps.size(); i < size; i++) {
            final ProcessRecord app = apps.valueAt(i);
            app.mState.setReachable(true);
            queue.offer(app);
        }

        return collectReachableProcessesLocked(queue, processes, uids);
    }

    @GuardedBy("mService")
    private boolean collectReachableProcessesLocked(ArrayDeque<ProcessRecord> queue,
            ArrayList<ProcessRecord> processes, ActiveUids uids) {
        processes.clear();
        uids.clear();

        // Track if any of them reachables could include a cycle
@@ -773,8 +726,7 @@ public class OomAdjuster {
            for (int i = ppr.numberOfProviderConnections() - 1; i >= 0; i--) {
                ContentProviderConnection cpc = ppr.getProviderConnectionAt(i);
                ProcessRecord provider = cpc.provider.proc;
                if (provider == null || provider == pr
                        || (containsCycle |= provider.mState.isReachable())) {
                if (provider == null || provider == pr) {
                    continue;
                }
                containsCycle |= provider.mState.isReachable();
@@ -954,6 +906,7 @@ public class OomAdjuster {
                state.resetCachedInfo();
            }
        }
        mProcessesInCycle.clear();
        for (int i = numProc - 1; i >= 0; i--) {
            ProcessRecord app = activeProcesses.get(i);
            final ProcessStateRecord state = app.mState;
@@ -1005,6 +958,7 @@ public class OomAdjuster {
                }
            }
        }
        mProcessesInCycle.clear();

        mNumNonCachedProcs = 0;
        mNumCachedHiddenProcs = 0;
@@ -1552,6 +1506,7 @@ public class OomAdjuster {
                // The process is being computed, so there is a cycle. We cannot
                // rely on this process's state.
                state.setContainsCycle(true);
                mProcessesInCycle.add(app);

                return false;
            }
@@ -2026,7 +1981,7 @@ public class OomAdjuster {
                    final boolean clientIsSystem = clientProcState < PROCESS_STATE_TOP;

                    if ((cr.flags & Context.BIND_WAIVE_PRIORITY) == 0) {
                        if (shouldSkipDueToCycle(state, cstate, procState, adj, cycleReEval)) {
                        if (shouldSkipDueToCycle(app, cstate, procState, adj, cycleReEval)) {
                            continue;
                        }

@@ -2326,7 +2281,7 @@ public class OomAdjuster {
                    cstate.setCurRawProcState(cstate.getCurProcState());
                }

                if (shouldSkipDueToCycle(state, cstate, procState, adj, cycleReEval)) {
                if (shouldSkipDueToCycle(app, cstate, procState, adj, cycleReEval)) {
                    continue;
                }

@@ -2558,13 +2513,14 @@ public class OomAdjuster {
     *                    evaluation.
     * @return whether to skip using the client connection at this time
     */
    private boolean shouldSkipDueToCycle(ProcessStateRecord app, ProcessStateRecord client,
    private boolean shouldSkipDueToCycle(ProcessRecord app, ProcessStateRecord client,
            int procState, int adj, boolean cycleReEval) {
        if (client.containsCycle()) {
            // We've detected a cycle. We should retry computeOomAdjLSP later in
            // case a later-checked connection from a client  would raise its
            // priority legitimately.
            app.setContainsCycle(true);
            app.mState.setContainsCycle(true);
            mProcessesInCycle.add(app);
            // If the client has not been completely evaluated, check if it's worth
            // using the partial values.
            if (client.getCompletedAdjSeq() < mAdjSeq) {
+1 −1
Original line number Diff line number Diff line
@@ -711,7 +711,7 @@ final class ProcessStateRecord {
            Slog.i(TAG, "Setting runningRemoteAnimation=" + runningRemoteAnimation
                    + " for pid=" + mApp.getPid());
        }
        mService.updateOomAdjLocked(mApp, true, OomAdjuster.OOM_ADJ_REASON_UI_VISIBILITY);
        mService.updateOomAdjLocked(mApp, OomAdjuster.OOM_ADJ_REASON_UI_VISIBILITY);
    }

    @GuardedBy({"mService", "mProcLock"})
Loading