From e76ef0bd24ddb12979610453625ade2adf57b3d5 Mon Sep 17 00:00:00 2001 From: Tomislav Novak Date: Wed, 3 May 2023 14:06:07 -0700 Subject: [PATCH 1/2] Fix setAttachingSchedGroupLSP() to support use_fifo_ui The method added in aosp/1249555 ("Start process of next activity with top priority in advance") to set the priority of the newly-launched top app's UI thread doesn't handle the use_fifo_ui=1 case. By setting mSetSchedGroup it also prevents subsequent applyOomAdjLSP() calls from fixing the priority, so on devices with the sys.use_fifo_ui sysprop set, main thread may not actually use SCHED_FIFO. This is an issue mainly for the initial launch of an app -- once it's moved to another sched group and then back, the priority is adjusted correctly. Bug: 284355269 Test: set sys.use_fifo_ui, start a new app, and check thread priorities with `ps -lT ` Signed-off-by: Tomislav Novak Change-Id: Ic8afc2eb054717018d227263a93d9fcc25bfa180 Merged-In: Ic8afc2eb054717018d227263a93d9fcc25bfa180 --- services/core/java/com/android/server/am/OomAdjuster.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/services/core/java/com/android/server/am/OomAdjuster.java b/services/core/java/com/android/server/am/OomAdjuster.java index 1e5f187fee4ae..eab2bdad1c54f 100644 --- a/services/core/java/com/android/server/am/OomAdjuster.java +++ b/services/core/java/com/android/server/am/OomAdjuster.java @@ -3253,7 +3253,11 @@ public class OomAdjuster { // {@link SCHED_GROUP_TOP_APP}. We don't check render thread because it // is not ready when attaching. app.getWindowProcessController().onTopProcChanged(); - setThreadPriority(app.getPid(), THREAD_PRIORITY_TOP_APP_BOOST); + if (mService.mUseFifoUiScheduling) { + mService.scheduleAsFifoPriority(app.getPid(), true); + } else { + setThreadPriority(app.getPid(), THREAD_PRIORITY_TOP_APP_BOOST); + } initialSchedGroup = SCHED_GROUP_TOP_APP; } catch (Exception e) { Slog.w(TAG, "Failed to pre-set top priority to " + app + " " + e); From a18cd7c08b7d2e2445bf51c0444d1a11b7a84e0b Mon Sep 17 00:00:00 2001 From: Tomislav Novak Date: Wed, 3 May 2023 14:36:51 -0700 Subject: [PATCH 2/2] Ignore BIND_ABOVE_CLIENT for same-process connections Binding to a service using BIND_ABOVE_CLIENT affects the OOM adjustment of both the client and the service; client's value is dropped by one level in modifyRawOomAdj() to make sure it's lower than the service's. Doing this unconditionally, however, means that the process' OOM score will be reduced if binding to a service that lives in the same process as the client. For example, a top app would have its oom_score_adj set to 100 (visible) rather than 0 (foreground). Bug: 284355269 Test: atest MockingOomAdjusterTests Signed-off-by: Tomislav Novak Change-Id: Iee5024b8f13771bc98a39a20e5f96a89a4c79b4e Merged-In: Iee5024b8f13771bc98a39a20e5f96a89a4c79b4e --- .../com/android/server/am/ActiveServices.java | 4 +++- .../server/am/ProcessServiceRecord.java | 3 ++- .../server/am/MockingOomAdjusterTests.java | 23 +++++++++++++++++++ 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/services/core/java/com/android/server/am/ActiveServices.java b/services/core/java/com/android/server/am/ActiveServices.java index 0da25be8c8cc6..58c202c335669 100644 --- a/services/core/java/com/android/server/am/ActiveServices.java +++ b/services/core/java/com/android/server/am/ActiveServices.java @@ -3797,7 +3797,9 @@ public final class ActiveServices { } clientPsr.addConnection(c); c.startAssociationIfNeeded(); - if (c.hasFlag(Context.BIND_ABOVE_CLIENT)) { + // Don't set hasAboveClient if binding to self to prevent modifyRawOomAdj() from + // dropping the process' adjustment level. + if (b.client != s.app && c.hasFlag(Context.BIND_ABOVE_CLIENT)) { clientPsr.setHasAboveClient(true); } if (c.hasFlag(BIND_ALLOW_WHITELIST_MANAGEMENT)) { diff --git a/services/core/java/com/android/server/am/ProcessServiceRecord.java b/services/core/java/com/android/server/am/ProcessServiceRecord.java index 81d0b6ac700b1..7ff6d116baaf0 100644 --- a/services/core/java/com/android/server/am/ProcessServiceRecord.java +++ b/services/core/java/com/android/server/am/ProcessServiceRecord.java @@ -341,7 +341,8 @@ final class ProcessServiceRecord { mHasAboveClient = false; for (int i = mConnections.size() - 1; i >= 0; i--) { ConnectionRecord cr = mConnections.valueAt(i); - if (cr.hasFlag(Context.BIND_ABOVE_CLIENT)) { + if (cr.binding.service.app.mServices != this + && cr.hasFlag(Context.BIND_ABOVE_CLIENT)) { mHasAboveClient = true; break; } diff --git a/services/tests/mockingservicestests/src/com/android/server/am/MockingOomAdjusterTests.java b/services/tests/mockingservicestests/src/com/android/server/am/MockingOomAdjusterTests.java index cda5456723fbd..770f04a2db5e3 100644 --- a/services/tests/mockingservicestests/src/com/android/server/am/MockingOomAdjusterTests.java +++ b/services/tests/mockingservicestests/src/com/android/server/am/MockingOomAdjusterTests.java @@ -68,6 +68,7 @@ import static com.android.server.am.ProcessList.UNKNOWN_ADJ; import static com.android.server.am.ProcessList.VISIBLE_APP_ADJ; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertTrue; import static org.mockito.AdditionalAnswers.answer; @@ -2489,6 +2490,28 @@ public class MockingOomAdjusterTests { assertProcStates(app2, false, PROCESS_STATE_SERVICE, SERVICE_ADJ, "started-services"); } + @SuppressWarnings("GuardedBy") + @Test + public void testUpdateOomAdj_DoOne_AboveClient_SameProcess() { + ProcessRecord app = spy(makeDefaultProcessRecord(MOCKAPP_PID, MOCKAPP_UID, + MOCKAPP_PROCESSNAME, MOCKAPP_PACKAGENAME, true)); + doReturn(PROCESS_STATE_TOP).when(sService.mAtmInternal).getTopProcessState(); + doReturn(app).when(sService).getTopApp(); + sService.mWakefulness.set(PowerManagerInternal.WAKEFULNESS_AWAKE); + sService.mOomAdjuster.updateOomAdjLocked(app, OOM_ADJ_REASON_NONE); + + assertEquals(FOREGROUND_APP_ADJ, app.mState.getSetAdj()); + + // Simulate binding to a service in the same process using BIND_ABOVE_CLIENT and + // verify that its OOM adjustment level is unaffected. + bindService(app, app, null, Context.BIND_ABOVE_CLIENT, mock(IBinder.class)); + app.mServices.updateHasAboveClientLocked(); + assertFalse(app.mServices.hasAboveClient()); + + sService.mOomAdjuster.updateOomAdjLocked(app, OOM_ADJ_REASON_NONE); + assertEquals(FOREGROUND_APP_ADJ, app.mState.getSetAdj()); + } + private ProcessRecord makeDefaultProcessRecord(int pid, int uid, String processName, String packageName, boolean hasShownUi) { long now = SystemClock.uptimeMillis();