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

Commit 86adfa19 authored by Yasin Kilicdere's avatar Yasin Kilicdere
Browse files

Call applyOomAdjLSP starting from the least recently used process. 

With this change, in cases where both client and service's oom
scores are updated, their order of killing by LMDK will reflect the
process LRU order, which means least recently used processes will be
killed before most recenlly used processes when they both have the same
oom score.

There are some cases where client's oom score is not updated, and its
position in LMKD queue does not get updated, resulting service to appear
closer to the most recently used end and getting killed after the
client (again if they have the same oom score, i.e. bound with
BIND_IMPORTANT flag). These cases will not benefit from this change,
but also would not get worse, since this is the current behavior before
this CL.

But in many cases, for example when app goes to background and get prev
oom score, or when the app gets cached, both the client and service
processes get updated. And this is when the most killing happen,
increasing the benefits of this change, without any side effects.

Also, this change will only be helpful if LMKD keeps having FIFO killing
order.

Bug: 378580264
Test: atest MockingOomAdjusterTests#testUpdateOomAdj_ApplyOomAdjInCorrectOrder
Flag: com.android.server.am.fix_apply_oomadj_order
Change-Id: I53d06354abaf0d5ce34f8d7a006579226c9254ae
parent a9812d3c

services/core/java/com/android/server/am/OomAdjuster.java

+18 −3
Original line number Diff line number Diff line
@@ -1376,10 +1376,12 @@ public class OomAdjuster { 
            ProcessRecord app = lruList.get(i);

            final ProcessStateRecord state = app.mState;

            if (!app.isKilledByAm() && app.getThread() != null) {

                if (!Flags.fixApplyOomadjOrder()) {

                    // We don't need to apply the update for the process which didn't get computed

                    if (state.getCompletedAdjSeq() == mAdjSeq) {

                        applyOomAdjLSP(app, doingAll, now, nowElapsed, oomAdjReason, true);

                    }

                }



                if (app.isPendingFinishAttach()) {

                    // Avoid trimming processes that are still initializing. If they aren't
@@ -1480,6 +1482,19 @@ public class OomAdjuster { 
            }

        }



        if (Flags.fixApplyOomadjOrder()) {

            // We need to apply the update starting from the least recently used.

            // Otherwise, they won't be in the correct LRU order in LMKD.

            for (int i = 0; i < numLru; i++) {

                ProcessRecord app = lruList.get(i);

                // We don't need to apply the update for the process which didn't get computed

                if (!app.isKilledByAm() && app.getThread() != null

                        && app.mState.getCompletedAdjSeq() == mAdjSeq) {

                    applyOomAdjLSP(app, doingAll, now, nowElapsed, oomAdjReason, true);

                }

            }

        }



        if (!mProcsToOomAdj.isEmpty()) {

            mInjector.batchSetOomAdj(mProcsToOomAdj);

            mProcsToOomAdj.clear();

services/core/java/com/android/server/am/flags.aconfig

+8 −0
Original line number Diff line number Diff line
@@ -244,6 +244,14 @@ flag { 
    bug: "359912586"

}



flag {

    name: "fix_apply_oomadj_order"

    namespace: "backstage_power"

    is_fixed_read_only: true

    description: "Fix the iteration direction of process LRU list when applying oom adj"

    bug: "378580264"

}



flag {

    name: "phantom_processes_fix"

    namespace: "backstage_power"

services/tests/mockingservicestests/src/com/android/server/am/MockingOomAdjusterTests.java

+28 −0
Original line number Diff line number Diff line
@@ -105,6 +105,7 @@ import android.os.PowerManagerInternal; 
import android.os.Process;

import android.os.SystemClock;

import android.os.UserHandle;

import android.platform.test.annotations.EnableFlags;

import android.platform.test.annotations.Presubmit;

import android.platform.test.flag.junit.SetFlagsRule;

import android.util.ArrayMap;
@@ -3260,6 +3261,24 @@ public class MockingOomAdjusterTests { 
                "cch-empty");

    }



    @SuppressWarnings("GuardedBy")

    @Test

    @EnableFlags(Flags.FLAG_FIX_APPLY_OOMADJ_ORDER)

    public void testUpdateOomAdj_ApplyOomAdjInCorrectOrder() {

        final int numberOfApps = 5;

        final ProcessRecord[] apps = new ProcessRecord[numberOfApps];

        for (int i = 0; i < numberOfApps; i++) {

            apps[i] = spy(makeDefaultProcessRecord(MOCKAPP_PID + i, MOCKAPP_UID + i,

                    MOCKAPP_PROCESSNAME + i, MOCKAPP_PACKAGENAME + i, true));

        }

        updateOomAdj(apps);

        for (int i = 1; i < numberOfApps; i++) {

            final int pre = mInjector.mSetOomAdjAppliedAt.get(apps[i - 1].mPid);

            final int cur = mInjector.mSetOomAdjAppliedAt.get(apps[i].mPid);

            assertTrue("setOomAdj is called in wrong order", pre < cur);

        }

    }



    private ProcessRecord makeDefaultProcessRecord(int pid, int uid, String processName,

            String packageName, boolean hasShownUi) {

        return new ProcessRecordBuilder(pid, uid, processName, packageName).setHasShownUi(
@@ -3589,9 +3608,16 @@ public class MockingOomAdjusterTests { 
        long mTimeOffsetMillis = 0;

        private SparseIntArray mLastSetOomAdj = new SparseIntArray();



        // A sequence number that increases every time setOomAdj is called

        int mLastAppliedAt = 0;

        // Holds the last sequence number setOomAdj is called for a pid

        private SparseIntArray mSetOomAdjAppliedAt = new SparseIntArray();



        void reset() {

            mTimeOffsetMillis = 0;

            mLastSetOomAdj.clear();

            mLastAppliedAt = 0;

            mSetOomAdjAppliedAt.clear();

        }



        void jumpUptimeAheadTo(long uptimeMillis) {
@@ -3616,6 +3642,7 @@ public class MockingOomAdjusterTests { 
                final int pid = proc.getPid();

                if (pid <= 0) continue;

                mLastSetOomAdj.put(pid, proc.mState.getCurAdj());

                mSetOomAdjAppliedAt.put(pid, mLastAppliedAt++);

            }

        }
@@ -3623,6 +3650,7 @@ public class MockingOomAdjusterTests { 
        void setOomAdj(int pid, int uid, int adj) {

            if (pid <= 0) return;

            mLastSetOomAdj.put(pid, adj);

            mSetOomAdjAppliedAt.put(pid, mLastAppliedAt++);

        }



        @Override