Merge "Stop global locking on session calls and UID state cache" into main (656c6a8d) · Commits · e / os / android_frameworks_base

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

+42 −32

Original line number	Diff line number	Diff line
		@@ -34,7 +34,7 @@ import android.os.RemoteException;
		import android.os.SystemProperties;
		import android.util.ArrayMap;
		import android.util.ArraySet;
		import android.util.SparseArray;
		import android.util.SparseIntArray;
		import android.util.StatsEvent;

		import com.android.internal.annotations.GuardedBy;
		@@ -256,10 +256,11 @@ public final class HintManagerService extends SystemService {

		@VisibleForTesting
		final class MyUidObserver extends UidObserver {
		private final SparseArray<Integer> mProcStatesCache = new SparseArray<>();

		private final Object mCacheLock = new Object();
		@GuardedBy("mCacheLock")
		private final SparseIntArray mProcStatesCache = new SparseIntArray();
		public boolean isUidForeground(int uid) {
		synchronized (mLock) {
		synchronized (mCacheLock) {
		return mProcStatesCache.get(uid, ActivityManager.PROCESS_STATE_IMPORTANT_FOREGROUND)
		<= ActivityManager.PROCESS_STATE_IMPORTANT_FOREGROUND;
		}
		@@ -268,6 +269,9 @@ public final class HintManagerService extends SystemService {
		@Override
		public void onUidGone(int uid, boolean disabled) {
		FgThread.getHandler().post(() -> {
		synchronized (mCacheLock) {
		mProcStatesCache.delete(uid);
		}
		synchronized (mLock) {
		ArrayMap<IBinder, ArraySet<AppHintSession>> tokenMap = mActiveSessions.get(uid);
		if (tokenMap == null) {
		@@ -280,7 +284,6 @@ public final class HintManagerService extends SystemService {
		sessionSet.valueAt(j).close();
		}
		}
		mProcStatesCache.delete(uid);
		}
		});
		}
		@@ -292,15 +295,18 @@ public final class HintManagerService extends SystemService {
		@Override
		public void onUidStateChanged(int uid, int procState, long procStateSeq, int capability) {
		FgThread.getHandler().post(() -> {
		synchronized (mLock) {
		synchronized (mCacheLock) {
		mProcStatesCache.put(uid, procState);
		}
		boolean shouldAllowUpdate = isUidForeground(uid);
		synchronized (mLock) {
		ArrayMap<IBinder, ArraySet<AppHintSession>> tokenMap = mActiveSessions.get(uid);
		if (tokenMap == null) {
		return;
		}
		for (ArraySet<AppHintSession> sessionSet : tokenMap.values()) {
		for (AppHintSession s : sessionSet) {
		s.onProcStateChanged();
		s.onProcStateChanged(shouldAllowUpdate);
		}
		}
		}
		@@ -429,10 +435,10 @@ public final class HintManagerService extends SystemService {
		if (!DumpUtils.checkDumpPermission(getContext(), TAG, pw)) {
		return;
		}
		synchronized (mLock) {
		pw.println("HintSessionPreferredRate: " + mHintSessionPreferredRate);
		pw.println("HAL Support: " + isHalSupported());
		pw.println("Active Sessions:");
		synchronized (mLock) {
		for (int i = 0; i < mActiveSessions.size(); i++) {
		pw.println("Uid " + mActiveSessions.keyAt(i).toString() + ":");
		ArrayMap<IBinder, ArraySet<AppHintSession>> tokenMap =
		@@ -476,7 +482,8 @@ public final class HintManagerService extends SystemService {
		mHalSessionPtr = halSessionPtr;
		mTargetDurationNanos = durationNanos;
		mUpdateAllowed = true;
		updateHintAllowed();
		final boolean allowed = mUidObserver.isUidForeground(mUid);
		updateHintAllowed(allowed);
		try {
		token.linkToDeath(this, 0);
		} catch (RemoteException e) {
		@@ -486,9 +493,8 @@ public final class HintManagerService extends SystemService {
		}

		@VisibleForTesting
		boolean updateHintAllowed() {
		synchronized (mLock) {
		final boolean allowed = mUidObserver.isUidForeground(mUid);
		boolean updateHintAllowed(boolean allowed) {
		synchronized (this) {
		if (allowed && !mUpdateAllowed) resume();
		if (!allowed && mUpdateAllowed) pause();
		mUpdateAllowed = allowed;
		@@ -498,8 +504,8 @@ public final class HintManagerService extends SystemService {

		@Override
		public void updateTargetWorkDuration(long targetDurationNanos) {
		synchronized (mLock) {
		if (mHalSessionPtr == 0 \|\| !updateHintAllowed()) {
		synchronized (this) {
		if (mHalSessionPtr == 0 \|\| !mUpdateAllowed) {
		return;
		}
		Preconditions.checkArgument(targetDurationNanos > 0, "Expected"
		@@ -511,8 +517,8 @@ public final class HintManagerService extends SystemService {

		@Override
		public void reportActualWorkDuration(long[] actualDurationNanos, long[] timeStampNanos) {
		synchronized (mLock) {
		if (mHalSessionPtr == 0 \|\| !updateHintAllowed()) {
		synchronized (this) {
		if (mHalSessionPtr == 0 \|\| !mUpdateAllowed) {
		return;
		}
		Preconditions.checkArgument(actualDurationNanos.length != 0, "the count"
		@@ -534,11 +540,13 @@ public final class HintManagerService extends SystemService {
		/** TODO: consider monitor session threads and close session if any thread is dead. */
		@Override
		public void close() {
		synchronized (mLock) {
		synchronized (this) {
		if (mHalSessionPtr == 0) return;
		mNativeWrapper.halCloseHintSession(mHalSessionPtr);
		mHalSessionPtr = 0;
		mToken.unlinkToDeath(this, 0);
		}
		synchronized (mLock) {
		ArrayMap<IBinder, ArraySet<AppHintSession>> tokenMap = mActiveSessions.get(mUid);
		if (tokenMap == null) {
		Slogf.w(TAG, "UID %d is not present in active session map", mUid);
		@@ -557,8 +565,8 @@ public final class HintManagerService extends SystemService {

		@Override
		public void sendHint(@PerformanceHintManager.Session.Hint int hint) {
		synchronized (mLock) {
		if (mHalSessionPtr == 0 \|\| !updateHintAllowed()) {
		synchronized (this) {
		if (mHalSessionPtr == 0 \|\| !mUpdateAllowed) {
		return;
		}
		Preconditions.checkArgument(hint >= 0, "the hint ID value should be"
		@@ -568,7 +576,7 @@ public final class HintManagerService extends SystemService {
		}

		public void setThreads(@NonNull int[] tids) {
		synchronized (mLock) {
		synchronized (this) {
		if (mHalSessionPtr == 0) {
		return;
		}
		@@ -588,7 +596,7 @@ public final class HintManagerService extends SystemService {
		} finally {
		Binder.restoreCallingIdentity(identity);
		}
		if (!updateHintAllowed()) {
		if (!mUpdateAllowed) {
		Slogf.v(TAG, "update hint not allowed, storing tids.");
		mNewThreadIds = tids;
		return;
		@@ -599,13 +607,15 @@ public final class HintManagerService extends SystemService {
		}

		public int[] getThreadIds() {
		return mThreadIds;
		synchronized (this) {
		return Arrays.copyOf(mThreadIds, mThreadIds.length);
		}
		}

		@Override
		public void setMode(int mode, boolean enabled) {
		synchronized (mLock) {
		if (mHalSessionPtr == 0 \|\| !updateHintAllowed()) {
		synchronized (this) {
		if (mHalSessionPtr == 0 \|\| !mUpdateAllowed) {
		return;
		}
		Preconditions.checkArgument(mode >= 0, "the mode Id value should be"
		@@ -614,19 +624,19 @@ public final class HintManagerService extends SystemService {
		}
		}

		private void onProcStateChanged() {
		updateHintAllowed();
		private void onProcStateChanged(boolean updateAllowed) {
		updateHintAllowed(updateAllowed);
		}

		private void pause() {
		synchronized (mLock) {
		synchronized (this) {
		if (mHalSessionPtr == 0) return;
		mNativeWrapper.halPauseHintSession(mHalSessionPtr);
		}
		}

		private void resume() {
		synchronized (mLock) {
		synchronized (this) {
		if (mHalSessionPtr == 0) return;
		mNativeWrapper.halResumeHintSession(mHalSessionPtr);
		if (mNewThreadIds != null) {
		@@ -638,12 +648,12 @@ public final class HintManagerService extends SystemService {
		}

		private void dump(PrintWriter pw, String prefix) {
		synchronized (mLock) {
		synchronized (this) {
		pw.println(prefix + "SessionPID: " + mPid);
		pw.println(prefix + "SessionUID: " + mUid);
		pw.println(prefix + "SessionTIDs: " + Arrays.toString(mThreadIds));
		pw.println(prefix + "SessionTargetDurationNanos: " + mTargetDurationNanos);
		pw.println(prefix + "SessionAllowed: " + updateHintAllowed());
		pw.println(prefix + "SessionAllowed: " + mUpdateAllowed);
		}
		}

services/core/java/com/android/server/power/hint/TEST_MAPPING

0 → 100644

+15 −0

Original line number	Diff line number	Diff line
		{
		"postsubmit": [
		{
		"name": "FrameworksServicesTests",
		"options": [
		{
		"include-filter": "com.android.server.power.hint"
		},
		{
		"exclude-annotation": "androidx.test.filters.FlakyTest"
		}
		]
		}
		]
		}
		No newline at end of file

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

+227 −11

Original line number	Diff line number	Diff line
		@@ -25,8 +25,6 @@ import static org.junit.Assert.assertNotEquals;
		import static org.junit.Assert.assertNotNull;
		import static org.junit.Assert.assertThrows;
		import static org.junit.Assert.assertTrue;
		import static org.junit.Assume.assumeFalse;
		import static org.junit.Assume.assumeTrue;
		import static org.mockito.Mockito.any;
		import static org.mockito.Mockito.anyBoolean;
		import static org.mockito.Mockito.anyInt;
		@@ -46,6 +44,7 @@ import android.os.IBinder;
		import android.os.IHintSession;
		import android.os.PerformanceHintManager;
		import android.os.Process;
		import android.util.Log;

		import com.android.server.FgThread;
		import com.android.server.LocalServices;
		@@ -58,7 +57,14 @@ import org.junit.Test;
		import org.mockito.Mock;
		import org.mockito.MockitoAnnotations;

		import java.util.ArrayList;
		import java.util.Collections;
		import java.util.List;
		import java.util.concurrent.CountDownLatch;
		import java.util.concurrent.TimeUnit;
		import java.util.concurrent.atomic.AtomicInteger;
		import java.util.concurrent.atomic.AtomicReference;
		import java.util.concurrent.locks.LockSupport;

		/**
		* Tests for {@link com.android.server.power.hint.HintManagerService}.
		@@ -67,8 +73,11 @@ import java.util.concurrent.CountDownLatch;
		* atest FrameworksServicesTests:HintManagerServiceTest
		*/
		public class HintManagerServiceTest {
		private static final String TAG = "HintManagerServiceTest";

		private static final long DEFAULT_HINT_PREFERRED_RATE = 16666666L;
		private static final long DEFAULT_TARGET_DURATION = 16666666L;
		private static final long CONCURRENCY_TEST_DURATION_SEC = 10;
		private static final int UID = Process.myUid();
		private static final int TID = Process.myPid();
		private static final int TGID = Process.getThreadGroupLeader(TID);
		@@ -103,6 +112,55 @@ public class HintManagerServiceTest {
		LocalServices.addService(ActivityManagerInternal.class, mAmInternalMock);
		}

		static class NativeWrapperFake extends NativeWrapper {
		@Override
		public void halInit() {
		}

		@Override
		public long halGetHintSessionPreferredRate() {
		return 1;
		}

		@Override
		public long halCreateHintSession(int tgid, int uid, int[] tids, long durationNanos) {
		return 1;
		}

		@Override
		public void halPauseHintSession(long halPtr) {
		}

		@Override
		public void halResumeHintSession(long halPtr) {
		}

		@Override
		public void halCloseHintSession(long halPtr) {
		}

		@Override
		public void halUpdateTargetWorkDuration(long halPtr, long targetDurationNanos) {
		}

		@Override
		public void halReportActualWorkDuration(
		long halPtr, long[] actualDurationNanos, long[] timeStampNanos) {
		}

		@Override
		public void halSendHint(long halPtr, int hint) {
		}

		@Override
		public void halSetThreads(long halPtr, int[] tids) {
		}

		@Override
		public void halSetMode(long halPtr, int mode, boolean enabled) {
		}
		}

		private HintManagerService createService() {
		mService = new HintManagerService(mContext, new Injector() {
		NativeWrapper createNativeWrapper() {
		@@ -112,6 +170,15 @@ public class HintManagerServiceTest {
		return mService;
		}

		private HintManagerService createServiceWithFakeWrapper() {
		mService = new HintManagerService(mContext, new Injector() {
		NativeWrapper createNativeWrapper() {
		return new NativeWrapperFake();
		}
		});
		return mService;
		}

		@Test
		public void testInitializeService() {
		HintManagerService service = createService();
		@@ -166,8 +233,7 @@ public class HintManagerServiceTest {
		latch.countDown();
		});
		latch.await();

		assumeFalse(a.updateHintAllowed());
		assertFalse(service.mUidObserver.isUidForeground(a.mUid));
		verify(mNativeWrapperMock, times(1)).halPauseHintSession(anyLong());

		// Set session to foreground and calling updateHintAllowed() would invoke resume();
		@@ -181,7 +247,7 @@ public class HintManagerServiceTest {
		});
		latch2.await();

		assumeTrue(a.updateHintAllowed());
		assertTrue(service.mUidObserver.isUidForeground(a.mUid));
		verify(mNativeWrapperMock, times(1)).halResumeHintSession(anyLong());
		}

		@@ -254,7 +320,7 @@ public class HintManagerServiceTest {
		});
		latch.await();

		assumeFalse(a.updateHintAllowed());
		assertFalse(service.mUidObserver.isUidForeground(a.mUid));
		a.reportActualWorkDuration(DURATIONS_THREE, TIMESTAMPS_THREE);
		verify(mNativeWrapperMock, never()).halReportActualWorkDuration(anyLong(), any(), any());
		}
		@@ -280,7 +346,7 @@ public class HintManagerServiceTest {
		service.mUidObserver.onUidStateChanged(
		a.mUid, ActivityManager.PROCESS_STATE_TRANSIENT_BACKGROUND, 0, 0);
		FgThread.getHandler().runWithScissors(() -> { }, 500);
		assertFalse(a.updateHintAllowed());
		assertFalse(service.mUidObserver.isUidForeground(a.mUid));
		a.sendHint(PerformanceHintManager.Session.CPU_LOAD_RESET);
		verify(mNativeWrapperMock, never()).halSendHint(anyLong(), anyInt());
		}
		@@ -303,7 +369,7 @@ public class HintManagerServiceTest {
		});
		latch.await();

		assertFalse(a.updateHintAllowed());
		assertFalse(service.mUidObserver.isUidForeground(a.mUid));
		}

		@Test
		@@ -316,7 +382,7 @@ public class HintManagerServiceTest {

		service.mUidObserver.onUidStateChanged(
		a.mUid, ActivityManager.PROCESS_STATE_IMPORTANT_FOREGROUND, 0, 0);
		assertTrue(a.updateHintAllowed());
		assertTrue(service.mUidObserver.isUidForeground(a.mUid));
		}

		@Test
		@@ -342,7 +408,7 @@ public class HintManagerServiceTest {
		service.mUidObserver.onUidStateChanged(
		a.mUid, ActivityManager.PROCESS_STATE_TRANSIENT_BACKGROUND, 0, 0);
		FgThread.getHandler().runWithScissors(() -> { }, 500);
		assertFalse(a.updateHintAllowed());
		assertFalse(service.mUidObserver.isUidForeground(a.mUid));
		a.setThreads(SESSION_TIDS_A);
		verify(mNativeWrapperMock, never()).halSetThreads(anyLong(), any());
		}
		@@ -372,9 +438,159 @@ public class HintManagerServiceTest {
		service.mUidObserver.onUidStateChanged(
		a.mUid, ActivityManager.PROCESS_STATE_TRANSIENT_BACKGROUND, 0, 0);
		FgThread.getHandler().runWithScissors(() -> { }, 500);
		assertFalse(a.updateHintAllowed());
		assertFalse(service.mUidObserver.isUidForeground(a.mUid));
		a.setMode(0, true);
		verify(mNativeWrapperMock, never()).halSetMode(anyLong(), anyInt(), anyBoolean());
		}

		// This test checks that concurrent operations from different threads on IHintService,
		// IHintSession and UidObserver will not cause data race or deadlock. Ideally we should also
		// check the output of threads' reportActualDuration performance to detect lock starvation
		// but the result is not stable, so it's better checked manually.
		@Test
		public void testConcurrency() throws Exception {
		HintManagerService service = createServiceWithFakeWrapper();
		// initialize session threads to run in parallel
		final int sessionCount = 10;
		// the signal that the main thread will send to session threads to check for run or exit
		AtomicReference<Boolean> shouldRun = new AtomicReference<>(true);
		// the signal for main test thread to wait for session threads and notifier thread to
		// finish and exit
		CountDownLatch latch = new CountDownLatch(sessionCount + 1);
		// list of exceptions with one per session thread or notifier thread
		List<AtomicReference<Exception>> execs = new ArrayList<>(sessionCount + 1);
		List<Thread> threads = new ArrayList<>(sessionCount + 1);
		for (int i = 0; i < sessionCount; i++) {
		final AtomicReference<Exception> exec = new AtomicReference<>();
		execs.add(exec);
		int j = i;
		Thread app = new Thread(() -> {
		try {
		while (shouldRun.get()) {
		runAppHintSession(service, j, shouldRun);
		}
		} catch (Exception e) {
		exec.set(e);
		} finally {
		latch.countDown();
		}
		});
		threads.add(app);
		}

		// initialize a UID state notifier thread to run in parallel
		final AtomicReference<Exception> notifierExec = new AtomicReference<>();
		execs.add(notifierExec);
		Thread notifier = new Thread(() -> {
		try {
		long min = Long.MAX_VALUE;
		long max = Long.MIN_VALUE;
		long sum = 0;
		int count = 0;
		while (shouldRun.get()) {
		long start = System.nanoTime();
		service.mUidObserver.onUidStateChanged(UID,
		ActivityManager.PROCESS_STATE_IMPORTANT_FOREGROUND, 0, 0);
		long elapsed = System.nanoTime() - start;
		sum += elapsed;
		count++;
		min = Math.min(min, elapsed);
		max = Math.max(max, elapsed);
		LockSupport.parkNanos(TimeUnit.MILLISECONDS.toNanos(500));
		service.mUidObserver.onUidStateChanged(UID,
		ActivityManager.PROCESS_STATE_TRANSIENT_BACKGROUND, 0, 0);
		LockSupport.parkNanos(TimeUnit.MILLISECONDS.toNanos(500));
		}
		Log.d(TAG, "notifier thread min " + min + " max " + max + " avg " + sum / count);
		service.mUidObserver.onUidGone(UID, true);
		} catch (Exception e) {
		notifierExec.set(e);
		} finally {
		latch.countDown();
		}
		});
		threads.add(notifier);

		// start all the threads
		for (Thread thread : threads) {
		thread.start();
		}
		// keep the test running for a few seconds
		LockSupport.parkNanos(TimeUnit.SECONDS.toNanos(CONCURRENCY_TEST_DURATION_SEC));
		// send signal to stop all threads
		shouldRun.set(false);
		// wait for all threads to exit
		latch.await();
		// check if any thread throws exception
		for (AtomicReference<Exception> exec : execs) {
		if (exec.get() != null) {
		throw exec.get();
		}
		}
		}

		private void runAppHintSession(HintManagerService service, int logId,
		AtomicReference<Boolean> shouldRun) throws Exception {
		IBinder token = new Binder();
		AppHintSession a = (AppHintSession) service.getBinderServiceInstance()
		.createHintSession(token, SESSION_TIDS_A, DEFAULT_TARGET_DURATION);
		// we will start some threads and get their valid TIDs to update
		int threadCount = 3;
		// the list of TIDs
		int[] tids = new int[threadCount];
		// atomic index for each thread to set its TID in the list
		AtomicInteger k = new AtomicInteger(0);
		// signal for the session main thread to wait for child threads to finish updating TIDs
		CountDownLatch latch = new CountDownLatch(threadCount);
		// signal for the session main thread to notify child threads to exit
		CountDownLatch stopLatch = new CountDownLatch(1);
		for (int j = 0; j < threadCount; j++) {
		Thread thread = new Thread(() -> {
		try {
		tids[k.getAndIncrement()] = android.os.Process.myTid();
		latch.countDown();
		stopLatch.await();
		} catch (InterruptedException e) {
		throw new RuntimeException(e);
		}
		});
		thread.start();
		}
		latch.await();
		a.setThreads(tids);
		// we don't need the threads to exist after update
		stopLatch.countDown();
		a.updateTargetWorkDuration(5);
		// measure the time it takes in HintManagerService to run reportActualDuration
		long min = Long.MAX_VALUE;
		long max = Long.MIN_VALUE;
		long sum = 0;
		int count = 0;
		List<Long> values = new ArrayList<>();
		long testStart = System.nanoTime();
		// run report actual for 4-second per cycle
		while (shouldRun.get() && System.nanoTime() - testStart < TimeUnit.SECONDS.toNanos(
		Math.min(4, CONCURRENCY_TEST_DURATION_SEC))) {
		long start = System.nanoTime();
		a.reportActualWorkDuration(new long[]{5}, new long[]{start});
		long elapsed = System.nanoTime() - start;
		values.add(elapsed);
		LockSupport.parkNanos(TimeUnit.MILLISECONDS.toNanos(5));
		sum += elapsed;
		count++;
		min = Math.min(min, elapsed);
		max = Math.max(max, elapsed);
		}
		Collections.sort(values);
		if (!values.isEmpty()) {
		Log.d(TAG, "app thread " + logId + " min " + min + " max " + max
		+ " avg " + sum / count + " count " + count
		+ " 80th " + values.get((int) (values.size() * 0.8))
		+ " 90th " + values.get((int) (values.size() * 0.9))
		+ " 95th " + values.get((int) (values.size() * 0.95)));
		} else {
		Log.w(TAG, "No reportActualWorkDuration executed");
		}
		a.close();
		}
		}