Merge "Manually manage memory of native objects" into tm-qpr-dev

This commit is contained in:
Evan Rosky
2022-11-29 18:55:29 +00:00
committed by Android (Google) Code Review
9 changed files with 198 additions and 30 deletions

View File

@@ -414,6 +414,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;
@@ -466,6 +513,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;

View File

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

View File

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

View File

@@ -500,6 +500,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;
}
@@ -604,6 +605,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,
@@ -642,6 +652,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) {
@@ -661,12 +676,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
@@ -679,8 +696,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 "

View File

@@ -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()
}
}
}

View File

@@ -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();
}
};
}
}
}

View File

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

View File

@@ -249,6 +249,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 */);
}
@@ -264,6 +265,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;
@@ -459,12 +462,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();
}
};
@@ -476,12 +482,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();
}
};

View File

@@ -751,6 +751,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);
@@ -892,6 +897,7 @@ class Transition implements BLASTSyncEngine.TransactionReadyListener {
mController.mAtm.mWindowManager.updateRotation(false /* alwaysSendConfiguration */,
false /* forceRelayout */);
}
cleanUpInternal();
}
void abort() {
@@ -934,6 +940,7 @@ class Transition implements BLASTSyncEngine.TransactionReadyListener {
dc.getPendingTransaction().merge(transaction);
mSyncId = -1;
mOverrideOptions = null;
cleanUpInternal();
return;
}
// Ensure that wallpaper visibility is updated with the latest wallpaper target.
@@ -1053,6 +1060,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));
@@ -1088,6 +1097,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) {