Merge "Only trigger cleanup when session goes background" into main (1016e4b8) · Commits · e / os / android_frameworks_base

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);