diff options
6 files changed, 162 insertions, 6 deletions
diff --git a/core/java/android/app/ActivityManagerInternal.java b/core/java/android/app/ActivityManagerInternal.java index ce29937c7cad..0fa1a378ed39 100644 --- a/core/java/android/app/ActivityManagerInternal.java +++ b/core/java/android/app/ActivityManagerInternal.java @@ -955,4 +955,12 @@ public abstract class ActivityManagerInternal { * @hide */ public abstract void stopForegroundServiceDelegate(@NonNull ServiceConnection connection); + + /** + * Called by PowerManager. Return whether a given procstate is allowed to hold + * wake locks in deep doze. Because it's called with the power manager lock held, we can't + * hold AM locks in it. + * @hide + */ + public abstract boolean canHoldWakeLocksInDeepDoze(int uid, int procstate); } diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index 10d50c29f42f..d9c38a12f458 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -32,6 +32,7 @@ import static android.app.ActivityManager.INSTR_FLAG_DISABLE_TEST_API_CHECKS; import static android.app.ActivityManager.INSTR_FLAG_NO_RESTART; import static android.app.ActivityManager.INTENT_SENDER_ACTIVITY; import static android.app.ActivityManager.PROCESS_CAPABILITY_ALL; +import static android.app.ActivityManager.PROCESS_STATE_BOUND_FOREGROUND_SERVICE; import static android.app.ActivityManager.PROCESS_STATE_IMPORTANT_FOREGROUND; import static android.app.ActivityManager.PROCESS_STATE_NONEXISTENT; import static android.app.ActivityManager.PROCESS_STATE_TOP; @@ -3341,6 +3342,7 @@ public class ActivityManagerService extends IActivityManager.Stub } mBatteryStatsService.noteProcessDied(app.info.uid, pid); + mOomAdjuster.updateShortFgsOwner(app.info.uid, pid, false); if (!app.isKilled()) { if (!fromBinderDied) { @@ -18305,6 +18307,23 @@ public class ActivityManagerService extends IActivityManager.Stub public void unregisterStrictModeCallback(int callingPid) { mStrictModeCallbacks.remove(callingPid); } + + @Override + public boolean canHoldWakeLocksInDeepDoze(int uid, int procstate) { + // This method is called with the PowerManager lock held. Do not hold AM here. + + // If the procstate is high enough, it's always allowed. + if (procstate <= PROCESS_STATE_BOUND_FOREGROUND_SERVICE) { + return true; + } + // IF it's too low, it's not allowed. + if (procstate > PROCESS_STATE_IMPORTANT_FOREGROUND) { + return false; + } + // If it's PROCESS_STATE_IMPORTANT_FOREGROUND, then we allow it only wheen the UID + // has a SHORT_FGS. + return mOomAdjuster.hasUidShortForegroundService(uid); + } } long inputDispatchingTimedOut(int pid, final boolean aboveSystem, TimeoutRecord timeoutRecord) { diff --git a/services/core/java/com/android/server/am/OomAdjuster.java b/services/core/java/com/android/server/am/OomAdjuster.java index 114e2c139794..e02dda642b40 100644 --- a/services/core/java/com/android/server/am/OomAdjuster.java +++ b/services/core/java/com/android/server/am/OomAdjuster.java @@ -126,6 +126,7 @@ import android.os.Trace; import android.util.ArrayMap; import android.util.ArraySet; import android.util.Slog; +import android.util.SparseSetArray; import android.util.proto.ProtoOutputStream; import com.android.internal.annotations.CompositeRWLock; @@ -364,6 +365,19 @@ public class OomAdjuster { @GuardedBy("mService") private boolean mPendingFullOomAdjUpdate = false; + /** + * PIDs that has a SHORT_SERVICE. We need to access it with the PowerManager lock held, + * so we use a fine-grained lock here. + */ + @GuardedBy("mPidsWithShortFgs") + private final ArraySet<Integer> mPidsWithShortFgs = new ArraySet<>(); + + /** + * UIDs -> PIDs map, used with mPidsWithShortFgs. + */ + @GuardedBy("mPidsWithShortFgs") + private final SparseSetArray<Integer> mUidsToPidsWithShortFgs = new SparseSetArray<>(); + /** Overrideable by a test */ @VisibleForTesting protected boolean isChangeEnabled(@CachedCompatChangeId int cachedCompatChangeId, @@ -1849,6 +1863,11 @@ public class OomAdjuster { int capabilityFromFGS = 0; // capability from foreground service. + final boolean hasForegroundServices = psr.hasForegroundServices(); + final boolean hasNonShortForegroundServices = psr.hasNonShortForegroundServices(); + final boolean hasShortForegroundServices = hasForegroundServices + && !psr.areAllShortForegroundServicesProcstateTimedOut(now); + // Adjust for FGS or "has-overlay-ui". if (adj > PERCEPTIBLE_APP_ADJ || procState > PROCESS_STATE_FOREGROUND_SERVICE) { @@ -1856,7 +1875,7 @@ public class OomAdjuster { int newAdj = 0; int newProcState = 0; - if (psr.hasForegroundServices() && psr.hasNonShortForegroundServices()) { + if (hasForegroundServices && hasNonShortForegroundServices) { // For regular (non-short) FGS. adjType = "fg-service"; newAdj = PERCEPTIBLE_APP_ADJ; @@ -1867,11 +1886,11 @@ public class OomAdjuster { newAdj = PERCEPTIBLE_APP_ADJ; newProcState = PROCESS_STATE_IMPORTANT_FOREGROUND; - } else if (psr.hasForegroundServices()) { + } else if (hasForegroundServices) { // If we get here, hasNonShortForegroundServices() must be false. // TODO(short-service): Proactively run OomAjudster when the grace period finish. - if (psr.areAllShortForegroundServicesProcstateTimedOut(now)) { + if (!hasShortForegroundServices) { // All the short-FGSes within this process are timed out. Don't promote to FGS. // TODO(short-service): Should we set some unique oom-adj to make it detectable, // in a long trace? @@ -1907,6 +1926,7 @@ public class OomAdjuster { } } } + updateShortFgsOwner(psr.mApp.uid, psr.mApp.mPid, hasShortForegroundServices); // If the app was recently in the foreground and moved to a foreground service status, // allow it to get a higher rank in memory for some time, compared to other foreground @@ -3337,4 +3357,40 @@ public class OomAdjuster { mCachedAppOptimizer.unfreezeAppLSP(app, oomAdjReason); } } + + /** + * Update {@link #mPidsWithShortFgs} and {@link #mUidsToPidsWithShortFgs} to keep track + * of which UID/PID has a short FGS. + * + * TODO(short-FGS): Remove it and all the relevant code once SHORT_FGS use the FGS procstate. + */ + void updateShortFgsOwner(int uid, int pid, boolean add) { + synchronized (mPidsWithShortFgs) { + if (add) { + mUidsToPidsWithShortFgs.add(uid, pid); + mPidsWithShortFgs.add(pid); + } else { + mUidsToPidsWithShortFgs.remove(uid, pid); + mPidsWithShortFgs.remove(pid); + } + } + } + + /** + * Whether a UID has a (non-timed-out) short FGS or not. + * It's indirectly called by PowerManager, so we can't hold the AM lock in it. + */ + boolean hasUidShortForegroundService(int uid) { + synchronized (mPidsWithShortFgs) { + final ArraySet<Integer> pids = mUidsToPidsWithShortFgs.get(uid); + if (pids == null || pids.size() == 0) { + return false; + } + for (int i = pids.size() - 1; i >= 0; i--) { + final int pid = pids.valueAt(i); + return mPidsWithShortFgs.contains(pid); + } + } + return false; + } } diff --git a/services/core/java/com/android/server/power/PowerManagerService.java b/services/core/java/com/android/server/power/PowerManagerService.java index c29ab09fa386..e8cb4e207629 100644 --- a/services/core/java/com/android/server/power/PowerManagerService.java +++ b/services/core/java/com/android/server/power/PowerManagerService.java @@ -40,6 +40,7 @@ import android.annotation.Nullable; import android.annotation.RequiresPermission; import android.annotation.UserIdInt; import android.app.ActivityManager; +import android.app.ActivityManagerInternal; import android.app.AppOpsManager; import android.app.SynchronousUserSwitchObserver; import android.content.BroadcastReceiver; @@ -311,6 +312,7 @@ public final class PowerManagerService extends SystemService private SettingsObserver mSettingsObserver; private DreamManagerInternal mDreamManager; private LogicalLight mAttentionLight; + private ActivityManagerInternal mAmInternal; private final InattentiveSleepWarningController mInattentiveSleepWarningOverlayController; private final AmbientDisplaySuppressionController mAmbientDisplaySuppressionController; @@ -1237,6 +1239,7 @@ public final class PowerManagerService extends SystemService mDisplayManagerInternal = getLocalService(DisplayManagerInternal.class); mPolicy = getLocalService(WindowManagerPolicy.class); mBatteryManagerInternal = getLocalService(BatteryManagerInternal.class); + mAmInternal = getLocalService(ActivityManagerInternal.class); mAttentionDetector.systemReady(mContext); SensorManager sensorManager = new SystemSensorManager(mContext, mHandler.getLooper()); @@ -4077,9 +4080,8 @@ public final class PowerManagerService extends SystemService final UidState state = wakeLock.mUidState; if (Arrays.binarySearch(mDeviceIdleWhitelist, appid) < 0 && Arrays.binarySearch(mDeviceIdleTempWhitelist, appid) < 0 && - state.mProcState != ActivityManager.PROCESS_STATE_NONEXISTENT && - state.mProcState > - ActivityManager.PROCESS_STATE_BOUND_FOREGROUND_SERVICE) { + (mAmInternal != null && !mAmInternal.canHoldWakeLocksInDeepDoze( + state.mUid, state.mProcState))) { disabled = true; } } diff --git a/services/tests/servicestests/src/com/android/server/am/OomAdjusterTests.java b/services/tests/servicestests/src/com/android/server/am/OomAdjusterTests.java index 8cbed2c7ffb8..4412cfe4a3b5 100644 --- a/services/tests/servicestests/src/com/android/server/am/OomAdjusterTests.java +++ b/services/tests/servicestests/src/com/android/server/am/OomAdjusterTests.java @@ -305,4 +305,60 @@ public class OomAdjusterTests { assertEquals("Interaction event time was not updated correctly.", interactionEventTime, mProcessRecord.mState.getInteractionEventTime()); } + + private void updateShortFgsOwner(int uid, int pid, boolean add) { + sService.mOomAdjuster.updateShortFgsOwner(uid, pid, add); + } + + private void assertHasUidShortForegroundService(int uid, boolean expected) { + assertEquals(expected, sService.mOomAdjuster.hasUidShortForegroundService(uid)); + } + + @Test + public void testHasUidShortForegroundService() { + assertHasUidShortForegroundService(1, false); + assertHasUidShortForegroundService(2, false); + assertHasUidShortForegroundService(3, false); + assertHasUidShortForegroundService(100, false); + assertHasUidShortForegroundService(101, false); + + updateShortFgsOwner(1, 100, true); + assertHasUidShortForegroundService(1, true); + assertHasUidShortForegroundService(100, false); + assertHasUidShortForegroundService(2, false); + + updateShortFgsOwner(1, 101, true); + assertHasUidShortForegroundService(1, true); + assertHasUidShortForegroundService(101, false); + assertHasUidShortForegroundService(2, false); + + updateShortFgsOwner(2, 200, true); + assertHasUidShortForegroundService(1, true); + assertHasUidShortForegroundService(2, true); + assertHasUidShortForegroundService(200, false); + + updateShortFgsOwner(1, 101, false); + assertHasUidShortForegroundService(1, true); + assertHasUidShortForegroundService(2, true); + + updateShortFgsOwner(1, 99, false); // unused PID + assertHasUidShortForegroundService(1, true); + assertHasUidShortForegroundService(2, true); + + updateShortFgsOwner(1, 100, false); + assertHasUidShortForegroundService(1, false); + assertHasUidShortForegroundService(2, true); + + updateShortFgsOwner(1, 100, true); + assertHasUidShortForegroundService(1, true); + assertHasUidShortForegroundService(2, true); + + updateShortFgsOwner(2, 200, false); + assertHasUidShortForegroundService(1, true); + assertHasUidShortForegroundService(2, false); + + updateShortFgsOwner(2, 201, true); + assertHasUidShortForegroundService(1, true); + assertHasUidShortForegroundService(2, true); + } } diff --git a/services/tests/servicestests/src/com/android/server/power/PowerManagerServiceTest.java b/services/tests/servicestests/src/com/android/server/power/PowerManagerServiceTest.java index a0fb3deeb131..d71deaf3a88d 100644 --- a/services/tests/servicestests/src/com/android/server/power/PowerManagerServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/power/PowerManagerServiceTest.java @@ -16,6 +16,7 @@ package com.android.server.power; +import static android.app.ActivityManager.PROCESS_STATE_BOUND_FOREGROUND_SERVICE; import static android.app.ActivityManager.PROCESS_STATE_BOUND_TOP; import static android.app.ActivityManager.PROCESS_STATE_FOREGROUND_SERVICE; import static android.app.AppOpsManager.MODE_ALLOWED; @@ -29,6 +30,8 @@ import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.mockito.AdditionalMatchers.gt; +import static org.mockito.AdditionalMatchers.leq; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyInt; @@ -150,6 +153,9 @@ public class PowerManagerServiceTest { @Mock private InattentiveSleepWarningController mInattentiveSleepWarningControllerMock; + @Mock + private ActivityManagerInternal mActivityManagerInternal; + private PowerManagerService mService; private ContextWrapper mContextSpy; private BatteryReceiver mBatteryReceiver; @@ -205,6 +211,7 @@ public class PowerManagerServiceTest { addLocalServiceMock(ActivityManagerInternal.class, mActivityManagerInternalMock); addLocalServiceMock(AttentionManagerInternal.class, mAttentionManagerInternalMock); addLocalServiceMock(DreamManagerInternal.class, mDreamManagerInternalMock); + addLocalServiceMock(ActivityManagerInternal.class, mActivityManagerInternal); mContextSpy = spy(new ContextWrapper(ApplicationProvider.getApplicationContext())); mResourcesSpy = spy(mContextSpy.getResources()); @@ -221,6 +228,14 @@ public class PowerManagerServiceTest { mClock = new OffsettableClock.Stopped(); mTestLooper = new TestLooper(mClock::now); + + // Set up canHoldWakeLocksInDeepDoze. + // - procstate <= PROCESS_STATE_BOUND_FOREGROUND_SERVICE -> true + // - procstate > PROCESS_STATE_BOUND_FOREGROUND_SERVICE -> false + when(mActivityManagerInternal.canHoldWakeLocksInDeepDoze( + anyInt(), leq(PROCESS_STATE_BOUND_FOREGROUND_SERVICE))).thenReturn(true); + when(mActivityManagerInternal.canHoldWakeLocksInDeepDoze( + anyInt(), gt(PROCESS_STATE_BOUND_FOREGROUND_SERVICE))).thenReturn(false); } private PowerManagerService createService() { |