diff options
author | 2022-11-30 19:42:15 +0000 | |
---|---|---|
committer | 2022-11-30 19:42:15 +0000 | |
commit | 38fc799aae333743a3ed67263f84841276e6917f (patch) | |
tree | 79d81444c3c1d7b7e61ebdaa96fed3da9de5604b | |
parent | fcf65b991562b0294afa1d90a98a56561adce464 (diff) | |
parent | fe11a033358ca8c250500547555c0d466aa146e5 (diff) |
Merge "Manually manage memory of native objects" into tm-qpr-dev am: 2971d9f3e0 am: fe11a03335
Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/20554503
Change-Id: I0bf096b6dd291b2aead4e37bf63a4f60d7170037
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
9 files changed, 198 insertions, 30 deletions
diff --git a/core/java/android/window/TransitionInfo.java b/core/java/android/window/TransitionInfo.java index c2da638aca8d..a35e13e3ac3b 100644 --- a/core/java/android/window/TransitionInfo.java +++ b/core/java/android/window/TransitionInfo.java @@ -421,6 +421,53 @@ public final class TransitionInfo implements Parcelable { return false; } + /** + * Releases temporary-for-animation surfaces referenced by this to potentially free up memory. + * This includes root-leash and snapshots. + */ + public void releaseAnimSurfaces() { + for (int i = mChanges.size() - 1; i >= 0; --i) { + final Change c = mChanges.get(i); + if (c.mSnapshot != null) { + c.mSnapshot.release(); + c.mSnapshot = null; + } + } + if (mRootLeash != null) { + mRootLeash.release(); + } + } + + /** + * Releases ALL the surfaces referenced by this to potentially free up memory. Do NOT use this + * if the surface-controls get stored and used elsewhere in the process. To just release + * temporary-for-animation surfaces, use {@link #releaseAnimSurfaces}. + */ + public void releaseAllSurfaces() { + releaseAnimSurfaces(); + for (int i = mChanges.size() - 1; i >= 0; --i) { + mChanges.get(i).getLeash().release(); + } + } + + /** + * Makes a copy of this as if it were parcel'd and unparcel'd. This implies that surfacecontrol + * refcounts are incremented which allows the "remote" receiver to release them without breaking + * the caller's references. Use this only if you need to "send" this to a local function which + * assumes it is being called from a remote caller. + */ + public TransitionInfo localRemoteCopy() { + final TransitionInfo out = new TransitionInfo(mType, mFlags); + for (int i = 0; i < mChanges.size(); ++i) { + out.mChanges.add(mChanges.get(i).localRemoteCopy()); + } + out.mRootLeash = mRootLeash != null ? new SurfaceControl(mRootLeash, "localRemote") : null; + // Doesn't have any native stuff, so no need for actual copy + out.mOptions = mOptions; + out.mRootOffset.set(mRootOffset); + return out; + } + /** Represents the change a WindowContainer undergoes during a transition */ public static final class Change implements Parcelable { private final WindowContainerToken mContainer; @@ -473,6 +520,27 @@ public final class TransitionInfo implements Parcelable { mSnapshotLuma = in.readFloat(); } + private Change localRemoteCopy() { + final Change out = new Change(mContainer, new SurfaceControl(mLeash, "localRemote")); + out.mParent = mParent; + out.mLastParent = mLastParent; + out.mMode = mMode; + out.mFlags = mFlags; + out.mStartAbsBounds.set(mStartAbsBounds); + out.mEndAbsBounds.set(mEndAbsBounds); + out.mEndRelOffset.set(mEndRelOffset); + out.mTaskInfo = mTaskInfo; + out.mAllowEnterPip = mAllowEnterPip; + out.mStartRotation = mStartRotation; + out.mEndRotation = mEndRotation; + out.mEndFixedRotation = mEndFixedRotation; + out.mRotationAnimation = mRotationAnimation; + out.mBackgroundColor = mBackgroundColor; + out.mSnapshot = mSnapshot != null ? new SurfaceControl(mSnapshot, "localRemote") : null; + out.mSnapshotLuma = mSnapshotLuma; + return out; + } + /** Sets the parent of this change's container. The parent must be a participant or null. */ public void setParent(@Nullable WindowContainerToken parent) { mParent = parent; diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/transition/OneShotRemoteHandler.java b/libs/WindowManager/Shell/src/com/android/wm/shell/transition/OneShotRemoteHandler.java index 4e1fa290270d..485b400f458d 100644 --- a/libs/WindowManager/Shell/src/com/android/wm/shell/transition/OneShotRemoteHandler.java +++ b/libs/WindowManager/Shell/src/com/android/wm/shell/transition/OneShotRemoteHandler.java @@ -77,10 +77,10 @@ public class OneShotRemoteHandler implements Transitions.TransitionHandler { if (mRemote.asBinder() != null) { mRemote.asBinder().unlinkToDeath(remoteDied, 0 /* flags */); } + if (sct != null) { + finishTransaction.merge(sct); + } mMainExecutor.execute(() -> { - if (sct != null) { - finishTransaction.merge(sct); - } finishCallback.onTransitionFinished(wct, null /* wctCB */); }); } @@ -90,7 +90,13 @@ public class OneShotRemoteHandler implements Transitions.TransitionHandler { if (mRemote.asBinder() != null) { mRemote.asBinder().linkToDeath(remoteDied, 0 /* flags */); } - mRemote.getRemoteTransition().startAnimation(transition, info, startTransaction, cb); + // If the remote is actually in the same process, then make a copy of parameters since + // remote impls assume that they have to clean-up native references. + final SurfaceControl.Transaction remoteStartT = RemoteTransitionHandler.copyIfLocal( + startTransaction, mRemote.getRemoteTransition()); + final TransitionInfo remoteInfo = + remoteStartT == startTransaction ? info : info.localRemoteCopy(); + mRemote.getRemoteTransition().startAnimation(transition, remoteInfo, remoteStartT, cb); // assume that remote will apply the start transaction. startTransaction.clear(); } catch (RemoteException e) { @@ -124,7 +130,13 @@ public class OneShotRemoteHandler implements Transitions.TransitionHandler { } }; try { - mRemote.getRemoteTransition().mergeAnimation(transition, info, t, mergeTarget, cb); + // If the remote is actually in the same process, then make a copy of parameters since + // remote impls assume that they have to clean-up native references. + final SurfaceControl.Transaction remoteT = + RemoteTransitionHandler.copyIfLocal(t, mRemote.getRemoteTransition()); + final TransitionInfo remoteInfo = remoteT == t ? info : info.localRemoteCopy(); + mRemote.getRemoteTransition().mergeAnimation( + transition, remoteInfo, remoteT, mergeTarget, cb); } catch (RemoteException e) { Log.e(Transitions.TAG, "Error merging remote transition.", e); } diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/transition/RemoteTransitionHandler.java b/libs/WindowManager/Shell/src/com/android/wm/shell/transition/RemoteTransitionHandler.java index 9469529de8f1..b4e05848882c 100644 --- a/libs/WindowManager/Shell/src/com/android/wm/shell/transition/RemoteTransitionHandler.java +++ b/libs/WindowManager/Shell/src/com/android/wm/shell/transition/RemoteTransitionHandler.java @@ -19,6 +19,7 @@ package com.android.wm.shell.transition; import android.annotation.NonNull; import android.annotation.Nullable; import android.os.IBinder; +import android.os.Parcel; import android.os.RemoteException; import android.util.ArrayMap; import android.util.Log; @@ -120,10 +121,10 @@ public class RemoteTransitionHandler implements Transitions.TransitionHandler { public void onTransitionFinished(WindowContainerTransaction wct, SurfaceControl.Transaction sct) { unhandleDeath(remote.asBinder(), finishCallback); + if (sct != null) { + finishTransaction.merge(sct); + } mMainExecutor.execute(() -> { - if (sct != null) { - finishTransaction.merge(sct); - } mRequestedRemotes.remove(transition); finishCallback.onTransitionFinished(wct, null /* wctCB */); }); @@ -131,8 +132,14 @@ public class RemoteTransitionHandler implements Transitions.TransitionHandler { }; Transitions.setRunningRemoteTransitionDelegate(remote.getAppThread()); try { + // If the remote is actually in the same process, then make a copy of parameters since + // remote impls assume that they have to clean-up native references. + final SurfaceControl.Transaction remoteStartT = + copyIfLocal(startTransaction, remote.getRemoteTransition()); + final TransitionInfo remoteInfo = + remoteStartT == startTransaction ? info : info.localRemoteCopy(); handleDeath(remote.asBinder(), finishCallback); - remote.getRemoteTransition().startAnimation(transition, info, startTransaction, cb); + remote.getRemoteTransition().startAnimation(transition, remoteInfo, remoteStartT, cb); // assume that remote will apply the start transaction. startTransaction.clear(); } catch (RemoteException e) { @@ -145,6 +152,28 @@ public class RemoteTransitionHandler implements Transitions.TransitionHandler { return true; } + static SurfaceControl.Transaction copyIfLocal(SurfaceControl.Transaction t, + IRemoteTransition remote) { + // We care more about parceling than local (though they should be the same); so, use + // queryLocalInterface since that's what Binder uses to decide if it needs to parcel. + if (remote.asBinder().queryLocalInterface(IRemoteTransition.DESCRIPTOR) == null) { + // No local interface, so binder itself will parcel and thus we don't need to. + return t; + } + // Binder won't be parceling; however, the remotes assume they have their own native + // objects (and don't know if caller is local or not), so we need to make a COPY here so + // that the remote can clean it up without clearing the original transaction. + // Since there's no direct `copy` for Transaction, we have to parcel/unparcel instead. + final Parcel p = Parcel.obtain(); + try { + t.writeToParcel(p, 0); + p.setDataPosition(0); + return SurfaceControl.Transaction.CREATOR.createFromParcel(p); + } finally { + p.recycle(); + } + } + @Override public void mergeAnimation(@NonNull IBinder transition, @NonNull TransitionInfo info, @NonNull SurfaceControl.Transaction t, @NonNull IBinder mergeTarget, @@ -175,7 +204,11 @@ public class RemoteTransitionHandler implements Transitions.TransitionHandler { } }; try { - remote.mergeAnimation(transition, info, t, mergeTarget, cb); + // If the remote is actually in the same process, then make a copy of parameters since + // remote impls assume that they have to clean-up native references. + final SurfaceControl.Transaction remoteT = copyIfLocal(t, remote); + final TransitionInfo remoteInfo = remoteT == t ? info : info.localRemoteCopy(); + remote.mergeAnimation(transition, remoteInfo, remoteT, mergeTarget, cb); } catch (RemoteException e) { Log.e(Transitions.TAG, "Error attempting to merge remote transition.", e); } diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/transition/Transitions.java b/libs/WindowManager/Shell/src/com/android/wm/shell/transition/Transitions.java index 56d51bda762f..c6935c054422 100644 --- a/libs/WindowManager/Shell/src/com/android/wm/shell/transition/Transitions.java +++ b/libs/WindowManager/Shell/src/com/android/wm/shell/transition/Transitions.java @@ -503,6 +503,7 @@ public class Transitions implements RemoteCallable<Transitions> { // Treat this as an abort since we are bypassing any merge logic and effectively // finishing immediately. onAbort(transitionToken); + releaseSurfaces(info); return; } @@ -607,6 +608,15 @@ public class Transitions implements RemoteCallable<Transitions> { onFinish(transition, wct, wctCB, false /* abort */); } + /** + * Releases an info's animation-surfaces. These don't need to persist and we need to release + * them asap so that SF can free memory sooner. + */ + private void releaseSurfaces(@Nullable TransitionInfo info) { + if (info == null) return; + info.releaseAnimSurfaces(); + } + private void onFinish(IBinder transition, @Nullable WindowContainerTransaction wct, @Nullable WindowContainerTransactionCallback wctCB, @@ -645,6 +655,11 @@ public class Transitions implements RemoteCallable<Transitions> { } ProtoLog.v(ShellProtoLogGroup.WM_SHELL_TRANSITIONS, "Transition animation finished (abort=%b), notifying core %s", abort, transition); + if (active.mStartT != null) { + // Applied by now, so close immediately. Do not set to null yet, though, since nullness + // is used later to disambiguate malformed transitions. + active.mStartT.close(); + } // Merge all relevant transactions together SurfaceControl.Transaction fullFinish = active.mFinishT; for (int iA = activeIdx + 1; iA < mActiveTransitions.size(); ++iA) { @@ -664,12 +679,14 @@ public class Transitions implements RemoteCallable<Transitions> { fullFinish.apply(); } // Now perform all the finishes. + releaseSurfaces(active.mInfo); mActiveTransitions.remove(activeIdx); mOrganizer.finishTransition(transition, wct, wctCB); while (activeIdx < mActiveTransitions.size()) { if (!mActiveTransitions.get(activeIdx).mMerged) break; ActiveTransition merged = mActiveTransitions.remove(activeIdx); mOrganizer.finishTransition(merged.mToken, null /* wct */, null /* wctCB */); + releaseSurfaces(merged.mInfo); } // sift through aborted transitions while (mActiveTransitions.size() > activeIdx @@ -682,8 +699,9 @@ public class Transitions implements RemoteCallable<Transitions> { } mOrganizer.finishTransition(aborted.mToken, null /* wct */, null /* wctCB */); for (int i = 0; i < mObservers.size(); ++i) { - mObservers.get(i).onTransitionFinished(active.mToken, true); + mObservers.get(i).onTransitionFinished(aborted.mToken, true); } + releaseSurfaces(aborted.mInfo); } if (mActiveTransitions.size() <= activeIdx) { ProtoLog.v(ShellProtoLogGroup.WM_SHELL_TRANSITIONS, "All active transition animations " diff --git a/packages/SystemUI/animation/src/com/android/systemui/animation/RemoteTransitionAdapter.kt b/packages/SystemUI/animation/src/com/android/systemui/animation/RemoteTransitionAdapter.kt index f9c6841f96b5..43bfa74119b3 100644 --- a/packages/SystemUI/animation/src/com/android/systemui/animation/RemoteTransitionAdapter.kt +++ b/packages/SystemUI/animation/src/com/android/systemui/animation/RemoteTransitionAdapter.kt @@ -320,9 +320,7 @@ class RemoteTransitionAdapter { counterWallpaper.cleanUp(finishTransaction) // Release surface references now. This is apparently to free GPU // memory while doing quick operations (eg. during CTS). - for (i in info.changes.indices.reversed()) { - info.changes[i].leash.release() - } + info.releaseAllSurfaces() for (i in leashMap.size - 1 downTo 0) { leashMap.valueAt(i).release() } @@ -331,6 +329,7 @@ class RemoteTransitionAdapter { null /* wct */, finishTransaction ) + finishTransaction.close() } catch (e: RemoteException) { Log.e( "ActivityOptionsCompat", @@ -364,6 +363,9 @@ class RemoteTransitionAdapter { ) { // TODO: hook up merge to recents onTaskAppeared if applicable. Until then, // ignore any incoming merges. + // Clean up stuff though cuz GC takes too long for benchmark tests. + t.close() + info.releaseAllSurfaces() } } } diff --git a/packages/SystemUI/shared/src/com/android/systemui/shared/system/RemoteAnimationRunnerCompat.java b/packages/SystemUI/shared/src/com/android/systemui/shared/system/RemoteAnimationRunnerCompat.java index 93c807352521..1b0dacc327c1 100644 --- a/packages/SystemUI/shared/src/com/android/systemui/shared/system/RemoteAnimationRunnerCompat.java +++ b/packages/SystemUI/shared/src/com/android/systemui/shared/system/RemoteAnimationRunnerCompat.java @@ -166,15 +166,14 @@ public abstract class RemoteAnimationRunnerCompat extends IRemoteAnimationRunner counterLauncher.cleanUp(finishTransaction); counterWallpaper.cleanUp(finishTransaction); // Release surface references now. This is apparently to free GPU memory - // while doing quick operations (eg. during CTS). - for (int i = info.getChanges().size() - 1; i >= 0; --i) { - info.getChanges().get(i).getLeash().release(); - } + // before GC would. + info.releaseAllSurfaces(); // Don't release here since launcher might still be using them. Instead // let launcher release them (eg. via RemoteAnimationTargets) leashMap.clear(); try { finishCallback.onTransitionFinished(null /* wct */, finishTransaction); + finishTransaction.close(); } catch (RemoteException e) { Log.e("ActivityOptionsCompat", "Failed to call app controlled animation" + " finished callback", e); @@ -203,10 +202,13 @@ public abstract class RemoteAnimationRunnerCompat extends IRemoteAnimationRunner synchronized (mFinishRunnables) { finishRunnable = mFinishRunnables.remove(mergeTarget); } + // Since we're not actually animating, release native memory now + t.close(); + info.releaseAllSurfaces(); if (finishRunnable == null) return; onAnimationCancelled(false /* isKeyguardOccluded */); finishRunnable.run(); } }; } -}
\ No newline at end of file +} diff --git a/packages/SystemUI/shared/src/com/android/systemui/shared/system/RemoteTransitionCompat.java b/packages/SystemUI/shared/src/com/android/systemui/shared/system/RemoteTransitionCompat.java index d4d3d2579b10..b7e2494ab839 100644 --- a/packages/SystemUI/shared/src/com/android/systemui/shared/system/RemoteTransitionCompat.java +++ b/packages/SystemUI/shared/src/com/android/systemui/shared/system/RemoteTransitionCompat.java @@ -126,15 +126,18 @@ public class RemoteTransitionCompat { public void mergeAnimation(IBinder transition, TransitionInfo info, SurfaceControl.Transaction t, IBinder mergeTarget, IRemoteTransitionFinishedCallback finishedCallback) { - if (!mergeTarget.equals(mToken)) return; - if (!mRecentsSession.merge(info, t, recents)) return; - try { - finishedCallback.onTransitionFinished(null /* wct */, null /* sct */); - } catch (RemoteException e) { - Log.e(TAG, "Error merging transition.", e); + if (mergeTarget.equals(mToken) && mRecentsSession.merge(info, t, recents)) { + try { + finishedCallback.onTransitionFinished(null /* wct */, null /* sct */); + } catch (RemoteException e) { + Log.e(TAG, "Error merging transition.", e); + } + // commit taskAppeared after merge transition finished. + mRecentsSession.commitTasksAppearedIfNeeded(recents); + } else { + t.close(); + info.releaseAllSurfaces(); } - // commit taskAppeared after merge transition finished. - mRecentsSession.commitTasksAppearedIfNeeded(recents); } }; return new RemoteTransition(remote, appThread); @@ -248,6 +251,8 @@ public class RemoteTransitionCompat { } // In this case, we are "returning" to an already running app, so just consume // the merge and do nothing. + info.releaseAllSurfaces(); + t.close(); return true; } final int layer = mInfo.getChanges().size() * 3; @@ -264,6 +269,8 @@ public class RemoteTransitionCompat { t.setLayer(targets[i].leash, layer); } t.apply(); + // not using the incoming anim-only surfaces + info.releaseAnimSurfaces(); mAppearedTargets = targets; return true; } @@ -380,9 +387,7 @@ public class RemoteTransitionCompat { } // Only release the non-local created surface references. The animator is responsible // for releasing the leashes created by local. - for (int i = 0; i < mInfo.getChanges().size(); ++i) { - mInfo.getChanges().get(i).getLeash().release(); - } + mInfo.releaseAllSurfaces(); // Reset all members. mWrapped = null; mFinishCB = null; diff --git a/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardService.java b/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardService.java index 021431399ab6..e631816991aa 100644 --- a/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardService.java +++ b/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardService.java @@ -220,6 +220,7 @@ public class KeyguardService extends Service { synchronized (mFinishCallbacks) { if (mFinishCallbacks.remove(transition) == null) return; } + info.releaseAllSurfaces(); Slog.d(TAG, "Finish IRemoteAnimationRunner."); finishCallback.onTransitionFinished(null /* wct */, null /* t */); } @@ -235,6 +236,8 @@ public class KeyguardService extends Service { synchronized (mFinishCallbacks) { origFinishCB = mFinishCallbacks.remove(transition); } + info.releaseAllSurfaces(); + t.close(); if (origFinishCB == null) { // already finished (or not started yet), so do nothing. return; @@ -423,12 +426,15 @@ public class KeyguardService extends Service { t.apply(); mBinder.setOccluded(true /* isOccluded */, true /* animate */); finishCallback.onTransitionFinished(null /* wct */, null /* wctCB */); + info.releaseAllSurfaces(); } @Override public void mergeAnimation(IBinder transition, TransitionInfo info, SurfaceControl.Transaction t, IBinder mergeTarget, IRemoteTransitionFinishedCallback finishCallback) { + t.close(); + info.releaseAllSurfaces(); } }; @@ -440,12 +446,15 @@ public class KeyguardService extends Service { t.apply(); mBinder.setOccluded(false /* isOccluded */, true /* animate */); finishCallback.onTransitionFinished(null /* wct */, null /* wctCB */); + info.releaseAllSurfaces(); } @Override public void mergeAnimation(IBinder transition, TransitionInfo info, SurfaceControl.Transaction t, IBinder mergeTarget, IRemoteTransitionFinishedCallback finishCallback) { + t.close(); + info.releaseAllSurfaces(); } }; diff --git a/services/core/java/com/android/server/wm/Transition.java b/services/core/java/com/android/server/wm/Transition.java index f6350021333b..9cb13e420184 100644 --- a/services/core/java/com/android/server/wm/Transition.java +++ b/services/core/java/com/android/server/wm/Transition.java @@ -744,6 +744,11 @@ class Transition implements BLASTSyncEngine.TransactionReadyListener { Trace.asyncTraceEnd(TRACE_TAG_WINDOW_MANAGER, TRACE_NAME_PLAY_TRANSITION, System.identityHashCode(this)); } + // Close the transactions now. They were originally copied to Shell in case we needed to + // apply them due to a remote failure. Since we don't need to apply them anymore, free them + // immediately. + if (mStartTransaction != null) mStartTransaction.close(); + if (mFinishTransaction != null) mFinishTransaction.close(); mStartTransaction = mFinishTransaction = null; if (mState < STATE_PLAYING) { throw new IllegalStateException("Can't finish a non-playing transition " + mSyncId); @@ -885,6 +890,7 @@ class Transition implements BLASTSyncEngine.TransactionReadyListener { mController.mAtm.mWindowManager.updateRotation(false /* alwaysSendConfiguration */, false /* forceRelayout */); } + cleanUpInternal(); } void abort() { @@ -927,6 +933,7 @@ class Transition implements BLASTSyncEngine.TransactionReadyListener { dc.getPendingTransaction().merge(transaction); mSyncId = -1; mOverrideOptions = null; + cleanUpInternal(); return; } @@ -1045,6 +1052,8 @@ class Transition implements BLASTSyncEngine.TransactionReadyListener { "Calling onTransitionReady: %s", info); mController.getTransitionPlayer().onTransitionReady( mToken, info, transaction, mFinishTransaction); + // Since we created root-leash but no longer reference it from core, release it now + info.releaseAnimSurfaces(); if (Trace.isTagEnabled(TRACE_TAG_WINDOW_MANAGER)) { Trace.asyncTraceBegin(TRACE_TAG_WINDOW_MANAGER, TRACE_NAME_PLAY_TRANSITION, System.identityHashCode(this)); @@ -1080,6 +1089,16 @@ class Transition implements BLASTSyncEngine.TransactionReadyListener { mController.finishTransition(mToken); } + private void cleanUpInternal() { + // Clean-up any native references. + for (int i = 0; i < mChanges.size(); ++i) { + final ChangeInfo ci = mChanges.valueAt(i); + if (ci.mSnapshot != null) { + ci.mSnapshot.release(); + } + } + } + /** @see RecentsAnimationController#attachNavigationBarToApp */ private void handleLegacyRecentsStartBehavior(DisplayContent dc, TransitionInfo info) { if ((mFlags & TRANSIT_FLAG_IS_RECENTS) == 0) { |