From 9acc558c29cd48d0f1a540d7fb02f87b3e385a5d Mon Sep 17 00:00:00 2001 From: Bo Liu Date: Thu, 17 Feb 2022 16:47:32 -0500 Subject: [PATCH] adpf: Use one client token per process for all hint sessions So it doesn't leak when gc is delayed in service process. Update HintManagerService to allow multiple sessions tied to the same token. Bug: 218129784 Test: chrome no longer gets killed Change-Id: I67a66041cc67d01e4cfcd3ded303a1bed6050f60 --- native/android/performance_hint.cpp | 4 +- .../server/power/hint/HintManagerService.java | 60 ++++++++++++++----- 2 files changed, 47 insertions(+), 17 deletions(-) diff --git a/native/android/performance_hint.cpp b/native/android/performance_hint.cpp index 0c360519ceb29..65428de955192 100644 --- a/native/android/performance_hint.cpp +++ b/native/android/performance_hint.cpp @@ -48,6 +48,7 @@ private: static APerformanceHintManager* create(sp iHintManager); sp mHintManager; + const sp mToken = sp::make(); const int64_t mPreferredRateNanos; }; @@ -119,11 +120,10 @@ APerformanceHintManager* APerformanceHintManager::create(sp manage APerformanceHintSession* APerformanceHintManager::createSession( const int32_t* threadIds, size_t size, int64_t initialTargetWorkDurationNanos) { - sp token = sp::make(); std::vector tids(threadIds, threadIds + size); sp session; binder::Status ret = - mHintManager->createHintSession(token, tids, initialTargetWorkDurationNanos, &session); + mHintManager->createHintSession(mToken, tids, initialTargetWorkDurationNanos, &session); if (!ret.isOk() || !session) { return nullptr; } diff --git a/services/core/java/com/android/server/power/hint/HintManagerService.java b/services/core/java/com/android/server/power/hint/HintManagerService.java index 2491565dd3763..8755662ac8136 100644 --- a/services/core/java/com/android/server/power/hint/HintManagerService.java +++ b/services/core/java/com/android/server/power/hint/HintManagerService.java @@ -27,6 +27,7 @@ import android.os.IHintSession; import android.os.Process; import android.os.RemoteException; import android.util.ArrayMap; +import android.util.ArraySet; import android.util.SparseArray; import com.android.internal.annotations.GuardedBy; @@ -51,8 +52,13 @@ public final class HintManagerService extends SystemService { private static final boolean DEBUG = false; @VisibleForTesting final long mHintSessionPreferredRate; + // Multi-levle map storing all active AppHintSessions. + // First level is keyed by the UID of the client process creating the session. + // Second level is keyed by an IBinder passed from client process. This is used to observe + // when the process exits. The client generally uses the same IBinder object across multiple + // sessions, so the value is a set of AppHintSessions. @GuardedBy("mLock") - private final ArrayMap> mActiveSessions; + private final ArrayMap>> mActiveSessions; /** Lock to protect HAL handles and listen list. */ private final Object mLock = new Object(); @@ -201,13 +207,16 @@ public final class HintManagerService extends SystemService { public void onUidGone(int uid, boolean disabled) { FgThread.getHandler().post(() -> { synchronized (mLock) { - ArrayMap tokenMap = mActiveSessions.get(uid); + ArrayMap> tokenMap = mActiveSessions.get(uid); if (tokenMap == null) { return; } for (int i = tokenMap.size() - 1; i >= 0; i--) { // Will remove the session from tokenMap - tokenMap.valueAt(i).close(); + ArraySet sessionSet = tokenMap.valueAt(i); + for (int j = sessionSet.size() - 1; j >= 0; j--) { + sessionSet.valueAt(j).close(); + } } mProcStatesCache.delete(uid); } @@ -231,12 +240,14 @@ public final class HintManagerService extends SystemService { FgThread.getHandler().post(() -> { synchronized (mLock) { mProcStatesCache.put(uid, procState); - ArrayMap tokenMap = mActiveSessions.get(uid); + ArrayMap> tokenMap = mActiveSessions.get(uid); if (tokenMap == null) { return; } - for (AppHintSession s : tokenMap.values()) { - s.onProcStateChanged(); + for (ArraySet sessionSet : tokenMap.values()) { + for (AppHintSession s : sessionSet) { + s.onProcStateChanged(); + } } } }); @@ -305,17 +316,25 @@ public final class HintManagerService extends SystemService { long halSessionPtr = mNativeWrapper.halCreateHintSession(callingTgid, callingUid, tids, durationNanos); - if (halSessionPtr == 0) return null; + if (halSessionPtr == 0) { + return null; + } AppHintSession hs = new AppHintSession(callingUid, callingTgid, tids, token, halSessionPtr, durationNanos); synchronized (mLock) { - ArrayMap tokenMap = mActiveSessions.get(callingUid); + ArrayMap> tokenMap = + mActiveSessions.get(callingUid); if (tokenMap == null) { tokenMap = new ArrayMap<>(1); mActiveSessions.put(callingUid, tokenMap); } - tokenMap.put(token, hs); + ArraySet sessionSet = tokenMap.get(token); + if (sessionSet == null) { + sessionSet = new ArraySet<>(1); + tokenMap.put(token, sessionSet); + } + sessionSet.add(hs); return hs; } } finally { @@ -339,10 +358,14 @@ public final class HintManagerService extends SystemService { pw.println("Active Sessions:"); for (int i = 0; i < mActiveSessions.size(); i++) { pw.println("Uid " + mActiveSessions.keyAt(i).toString() + ":"); - ArrayMap tokenMap = mActiveSessions.valueAt(i); + ArrayMap> tokenMap = + mActiveSessions.valueAt(i); for (int j = 0; j < tokenMap.size(); j++) { - pw.println(" Session " + j + ":"); - tokenMap.valueAt(j).dump(pw, " "); + ArraySet sessionSet = tokenMap.valueAt(j); + for (int k = 0; k < sessionSet.size(); ++k) { + pw.println(" Session:"); + sessionSet.valueAt(k).dump(pw, " "); + } } } } @@ -432,11 +455,18 @@ public final class HintManagerService extends SystemService { mNativeWrapper.halCloseHintSession(mHalSessionPtr); mHalSessionPtr = 0; mToken.unlinkToDeath(this, 0); - ArrayMap tokenMap = mActiveSessions.get(mUid); + ArrayMap> tokenMap = mActiveSessions.get(mUid); if (tokenMap == null) { - Slogf.w(TAG, "UID %d is note present in active session map", mUid); + Slogf.w(TAG, "UID %d is not present in active session map", mUid); + return; } - tokenMap.remove(mToken); + ArraySet sessionSet = tokenMap.get(mToken); + if (sessionSet == null) { + Slogf.w(TAG, "Token %s is not present in token map", mToken.toString()); + return; + } + sessionSet.remove(this); + if (sessionSet.isEmpty()) tokenMap.remove(mToken); if (tokenMap.isEmpty()) mActiveSessions.remove(mUid); } }