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

Commit d43193fe authored by Xiang Wang's avatar Xiang Wang
Browse files

Only trigger cleanup when session goes background 

Avoid triggering cleanup if the session is already force paused

Bug: 309701979
Test: atest HintManagerServiceTest
Change-Id: I8f955fcd33ed6e397955763cf5aa8b3142fb120e
parent 6f67bf71

services/core/java/com/android/server/power/hint/HintManagerService.java

+46 −34
Original line number Diff line number Diff line
@@ -371,6 +371,7 @@ public final class HintManagerService extends SystemService { 
                    if (tokenMap == null) {

                        return;

                    }

                    Slog.d(TAG, "Uid gone for " + uid);

                    for (int i = tokenMap.size() - 1; i >= 0; i--) {

                        // Will remove the session from tokenMap

                        ArraySet<AppHintSession> sessionSet = tokenMap.valueAt(i);
@@ -400,27 +401,31 @@ public final class HintManagerService extends SystemService { 
        public void onUidStateChanged(int uid, int procState, long procStateSeq, int capability) {

            FgThread.getHandler().post(() -> {

                synchronized (mLock) {

                    boolean shouldCleanup = false;

                    if (powerhintThreadCleanup()) {

                        final boolean before = isUidForeground(uid);

                        mProcStatesCache.put(uid, procState);

                        final boolean after = isUidForeground(uid);

                        if (before != after) {

                            final Message msg = mCleanUpHandler.obtainMessage(EVENT_CLEAN_UP_UID,

                                    uid);

                            mCleanUpHandler.sendMessageDelayed(msg, CLEAN_UP_UID_DELAY_MILLIS);

                        int prevProcState = mProcStatesCache.get(uid, Integer.MAX_VALUE);

                        shouldCleanup =

                                prevProcState <= ActivityManager.PROCESS_STATE_IMPORTANT_FOREGROUND

                                        && procState

                                        > ActivityManager.PROCESS_STATE_IMPORTANT_FOREGROUND;

                    }

                    } else {



                    mProcStatesCache.put(uid, procState);

                    }

                    boolean shouldAllowUpdate = isUidForeground(uid);

                    ArrayMap<IBinder, ArraySet<AppHintSession>> tokenMap = mActiveSessions.get(uid);

                    if (tokenMap == null) {

                        return;

                    }

                    if (shouldCleanup && powerhintThreadCleanup()) {

                        final Message msg = mCleanUpHandler.obtainMessage(EVENT_CLEAN_UP_UID,

                                uid);

                        mCleanUpHandler.sendMessageDelayed(msg, CLEAN_UP_UID_DELAY_MILLIS);

                        Slog.d(TAG, "Sent cleanup message for uid " + uid);

                    }

                    boolean shouldAllowUpdate = isUidForeground(uid);

                    for (int i = tokenMap.size() - 1; i >= 0; i--) {

                        final ArraySet<AppHintSession> sessionSet = tokenMap.valueAt(i);

                        for (int j = sessionSet.size() - 1; j >= 0; j--) {

                            sessionSet.valueAt(j).onProcStateChanged(shouldAllowUpdate);

                            sessionSet.valueAt(j).updateHintAllowedByProcState(shouldAllowUpdate);

                        }

                    }

                }
@@ -552,8 +557,10 @@ public final class HintManagerService extends SystemService { 
                    removeEqualMessages(msg.what, msg.obj);

                    final Message newMsg = obtainMessage(msg.what, msg.obj);

                    sendMessageDelayed(newMsg, CLEAN_UP_UID_DELAY_MILLIS);

                    Slog.d(TAG, "Duplicate messages for " + msg.obj);

                    return;

                }

                Slog.d(TAG, "Starts cleaning for " + msg.obj);

                final int uid = (int) msg.obj;

                boolean isForeground = mUidObserver.isUidForeground(uid);

                // store all sessions in a list and release the global lock
@@ -621,7 +628,7 @@ public final class HintManagerService extends SystemService { 
                FrameworkStatsLog.write(FrameworkStatsLog.ADPF_HINT_SESSION_TID_CLEANUP, uid,

                        totalDurationUs, maxDurationUs, totalTidCnt, totalInvalidTidCnt,

                        maxInvalidTidCnt, sessionCnt, isForeground);

                Slog.d(TAG,

                Slog.w(TAG,

                        "Invalid tid found for UID" + uid + " in " + totalDurationUs + "us:\n\t"

                                + "count("

                                + " session: " + sessionCnt
@@ -643,7 +650,7 @@ public final class HintManagerService extends SystemService { 
        // kill(tid, 0) to only check if it exists. The result will be cached in checkedTids

        // map with tid as the key and checked status as value.

        public int cleanUpSession(AppHintSession session, SparseIntArray checkedTids, int[] total) {

            if (session.isClosed()) {

            if (session.isClosed() || session.isForcePaused()) {

                return 0;

            }

            final int pid = session.mPid;
@@ -705,7 +712,7 @@ public final class HintManagerService extends SystemService { 
                    final int[] filteredTids = filtered.toArray();

                    if (filteredTids.length == 0) {

                        session.mShouldForcePause = true;

                        if (session.mUpdateAllowed) {

                        if (session.mUpdateAllowedByProcState) {

                            session.pause();

                        }

                    } else {
@@ -951,7 +958,7 @@ public final class HintManagerService extends SystemService { 
        protected final IBinder mToken;

        protected long mHalSessionPtr;

        protected long mTargetDurationNanos;

        protected boolean mUpdateAllowed;

        protected boolean mUpdateAllowedByProcState;

        protected int[] mNewThreadIds;

        protected boolean mPowerEfficient;

        protected boolean mShouldForcePause;
@@ -969,11 +976,11 @@ public final class HintManagerService extends SystemService { 
            mThreadIds = threadIds;

            mHalSessionPtr = halSessionPtr;

            mTargetDurationNanos = durationNanos;

            mUpdateAllowed = true;

            mUpdateAllowedByProcState = true;

            mPowerEfficient = false;

            mShouldForcePause = false;

            final boolean allowed = mUidObserver.isUidForeground(mUid);

            updateHintAllowed(allowed);

            updateHintAllowedByProcState(allowed);

            try {

                token.linkToDeath(this, 0);

            } catch (RemoteException e) {
@@ -983,19 +990,23 @@ public final class HintManagerService extends SystemService { 
        }



        @VisibleForTesting

        boolean updateHintAllowed(boolean allowed) {

        boolean updateHintAllowedByProcState(boolean allowed) {

            synchronized (this) {

                if (allowed && !mUpdateAllowed && !mShouldForcePause) resume();

                if (!allowed && mUpdateAllowed) pause();

                mUpdateAllowed = allowed;

                return mUpdateAllowed;

                if (allowed && !mUpdateAllowedByProcState && !mShouldForcePause) resume();

                if (!allowed && mUpdateAllowedByProcState) pause();

                mUpdateAllowedByProcState = allowed;

                return mUpdateAllowedByProcState;

            }

        }



        boolean isHintAllowed() {

            return mHalSessionPtr != 0 && mUpdateAllowedByProcState && !mShouldForcePause;

        }



        @Override

        public void updateTargetWorkDuration(long targetDurationNanos) {

            synchronized (this) {

                if (mHalSessionPtr == 0 || !mUpdateAllowed || mShouldForcePause) {

                if (!isHintAllowed()) {

                    return;

                }

                Preconditions.checkArgument(targetDurationNanos > 0, "Expected"
@@ -1008,7 +1019,7 @@ public final class HintManagerService extends SystemService { 
        @Override

        public void reportActualWorkDuration(long[] actualDurationNanos, long[] timeStampNanos) {

            synchronized (this) {

                if (mHalSessionPtr == 0 || !mUpdateAllowed || mShouldForcePause) {

                if (!isHintAllowed()) {

                    return;

                }

                Preconditions.checkArgument(actualDurationNanos.length != 0, "the count"
@@ -1073,7 +1084,7 @@ public final class HintManagerService extends SystemService { 
        @Override

        public void sendHint(@PerformanceHintManager.Session.Hint int hint) {

            synchronized (this) {

                if (mHalSessionPtr == 0 || !mUpdateAllowed || mShouldForcePause) {

                if (!isHintAllowed()) {

                    return;

                }

                Preconditions.checkArgument(hint >= 0, "the hint ID value should be"
@@ -1095,7 +1106,7 @@ public final class HintManagerService extends SystemService { 
                if (mHalSessionPtr == 0) {

                    return;

                }

                if (!mUpdateAllowed) {

                if (!mUpdateAllowedByProcState) {

                    Slogf.v(TAG, "update hint not allowed, storing tids.");

                    mNewThreadIds = tids;

                    mShouldForcePause = false;
@@ -1160,10 +1171,15 @@ public final class HintManagerService extends SystemService { 
            }

        }



        boolean isForcePaused() {

            synchronized (this) {

                return mShouldForcePause;

            }

        }

        @Override

        public void setMode(int mode, boolean enabled) {

            synchronized (this) {

                if (mHalSessionPtr == 0 || !mUpdateAllowed || mShouldForcePause) {

                if (!isHintAllowed()) {

                    return;

                }

                Preconditions.checkArgument(mode >= 0, "the mode Id value should be"
@@ -1178,7 +1194,7 @@ public final class HintManagerService extends SystemService { 
        @Override

        public void reportActualWorkDuration2(WorkDuration[] workDurations) {

            synchronized (this) {

                if (mHalSessionPtr == 0 || !mUpdateAllowed || mShouldForcePause) {

                if (!isHintAllowed()) {

                    return;

                }

                Preconditions.checkArgument(workDurations.length != 0, "the count"
@@ -1241,10 +1257,6 @@ public final class HintManagerService extends SystemService { 
            }

        }



        private void onProcStateChanged(boolean updateAllowed) {

            updateHintAllowed(updateAllowed);

        }



        private void pause() {

            synchronized (this) {

                if (mHalSessionPtr == 0) return;
@@ -1270,7 +1282,7 @@ public final class HintManagerService extends SystemService { 
                pw.println(prefix + "SessionUID: " + mUid);

                pw.println(prefix + "SessionTIDs: " + Arrays.toString(mThreadIds));

                pw.println(prefix + "SessionTargetDurationNanos: " + mTargetDurationNanos);

                pw.println(prefix + "SessionAllowed: " + mUpdateAllowed);

                pw.println(prefix + "SessionAllowedByProcState: " + mUpdateAllowedByProcState);

                pw.println(prefix + "SessionForcePaused: " + mShouldForcePause);

                pw.println(prefix + "PowerEfficient: " + (mPowerEfficient ? "true" : "false"));

            }

services/tests/servicestests/src/com/android/server/power/hint/HintManagerServiceTest.java

+25 −14
Original line number Diff line number Diff line
@@ -361,7 +361,8 @@ public class HintManagerServiceTest { 
                .createHintSessionWithConfig(token, SESSION_TIDS_A, DEFAULT_TARGET_DURATION,

                        SessionTag.OTHER, new SessionConfig());



        // Set session to background and calling updateHintAllowed() would invoke pause();

        // Set session to background and calling updateHintAllowedByProcState() would invoke

        // pause();

        service.mUidObserver.onUidStateChanged(

                a.mUid, ActivityManager.PROCESS_STATE_TRANSIENT_BACKGROUND, 0, 0);
@@ -374,7 +375,8 @@ public class HintManagerServiceTest { 
        assertFalse(service.mUidObserver.isUidForeground(a.mUid));

        verify(mNativeWrapperMock, times(1)).halPauseHintSession(anyLong());



        // Set session to foreground and calling updateHintAllowed() would invoke resume();

        // Set session to foreground and calling updateHintAllowedByProcState() would invoke

        // resume();

        service.mUidObserver.onUidStateChanged(

                a.mUid, ActivityManager.PROCESS_STATE_IMPORTANT_FOREGROUND, 0, 0);
@@ -561,10 +563,12 @@ public class HintManagerServiceTest { 
    public void testCleanupDeadThreads() throws Exception {

        HintManagerService service = createService();

        IBinder token = new Binder();

        int threadCount = 2;



        // session 1 has 2 non-isolated tids

        long sessionPtr1 = 111;

        CountDownLatch stopLatch1 = new CountDownLatch(1);

        int threadCount = 3;

        int[] tids1 = createThreads(threadCount, stopLatch1);

        long sessionPtr1 = 111;

        when(mNativeWrapperMock.halCreateHintSessionWithConfig(eq(TGID), eq(UID), eq(tids1),

                eq(DEFAULT_TARGET_DURATION), anyInt(), any(SessionConfig.class)))

                .thenReturn(sessionPtr1);
@@ -573,19 +577,19 @@ public class HintManagerServiceTest { 
                        SessionTag.OTHER, new SessionConfig());

        assertNotNull(session1);



        // for test only to avoid conflicting with any real thread that exists on device

        // session 2 has 2 non-isolated tids and 2 isolated tids

        long sessionPtr2 = 222;

        CountDownLatch stopLatch2 = new CountDownLatch(1);

        // negative value used for test only to avoid conflicting with any real thread that exists

        int isoProc1 = -100;

        int isoProc2 = 9999;

        when(mAmInternalMock.getIsolatedProcesses(eq(UID))).thenReturn(List.of(0));



        CountDownLatch stopLatch2 = new CountDownLatch(1);

        int[] tids2 = createThreads(threadCount, stopLatch2);

        int[] tids2WithIsolated = Arrays.copyOf(tids2, tids2.length + 2);

        int[] expectedTids2 = Arrays.copyOf(tids2, tids2.length + 1);

        expectedTids2[tids2.length] = isoProc1;

        tids2WithIsolated[threadCount] = isoProc1;

        tids2WithIsolated[threadCount + 1] = isoProc2;

        long sessionPtr2 = 222;

        int[] expectedTids2 = Arrays.copyOf(tids2, tids2.length + 1);

        expectedTids2[tids2.length] = isoProc1;

        when(mNativeWrapperMock.halCreateHintSessionWithConfig(eq(TGID), eq(UID),

                eq(tids2WithIsolated), eq(DEFAULT_TARGET_DURATION), anyInt(),

                any(SessionConfig.class))).thenReturn(sessionPtr2);
@@ -594,7 +598,10 @@ public class HintManagerServiceTest { 
                        DEFAULT_TARGET_DURATION, SessionTag.OTHER, new SessionConfig());

        assertNotNull(session2);



        // trigger clean up through UID state change by making the process background

        // trigger clean up through UID state change by making the process foreground->background

        // this will remove the one unexpected isolated tid from session 2

        service.mUidObserver.onUidStateChanged(UID,

                ActivityManager.PROCESS_STATE_IMPORTANT_FOREGROUND, 0, 0);

        service.mUidObserver.onUidStateChanged(UID,

                ActivityManager.PROCESS_STATE_IMPORTANT_BACKGROUND, 0, 0);

        LockSupport.parkNanos(TimeUnit.MILLISECONDS.toNanos(500) + TimeUnit.MILLISECONDS.toNanos(
@@ -614,17 +621,21 @@ public class HintManagerServiceTest { 
        verify(mNativeWrapperMock, times(1)).halSetThreads(eq(sessionPtr2), eq(expectedTids2));

        reset(mNativeWrapperMock);



        // let all session 1 threads to exit and the cleanup should force pause the session

        // let all session 1 threads to exit and the cleanup should force pause the session 1

        stopLatch1.countDown();

        LockSupport.parkNanos(TimeUnit.MILLISECONDS.toNanos(100));

        service.mUidObserver.onUidStateChanged(UID,

                ActivityManager.PROCESS_STATE_IMPORTANT_FOREGROUND, 0, 0);

                ActivityManager.PROCESS_STATE_IMPORTANT_BACKGROUND, 0, 0);

        LockSupport.parkNanos(TimeUnit.MILLISECONDS.toNanos(500) + TimeUnit.MILLISECONDS.toNanos(

                CLEAN_UP_UID_DELAY_MILLIS));

        verify(mNativeWrapperMock, times(1)).halPauseHintSession(eq(sessionPtr1));

        verify(mNativeWrapperMock, never()).halSetThreads(eq(sessionPtr1), any());

        verify(mNativeWrapperMock, never()).halSetThreads(eq(sessionPtr2), any());

        // all hints will have no effect as the session is force paused while proc in foreground

        verifyAllHintsEnabled(session1, false);

        verifyAllHintsEnabled(session2, false);

        service.mUidObserver.onUidStateChanged(UID,

                ActivityManager.PROCESS_STATE_IMPORTANT_FOREGROUND, 0, 0);

        LockSupport.parkNanos(TimeUnit.MILLISECONDS.toNanos(500));

        verifyAllHintsEnabled(session1, false);

        verifyAllHintsEnabled(session2, true);

        reset(mNativeWrapperMock);