Manually manage memory of native objects
Even though these surfaces/transactions are unreachable, it still
takes time for the GC/NativeRegistry/FinalizerQueue to actually
flush/free references (even if you force GC, it won't flush the
NativeRegistry/FinalizerQueue). This skews the results of our
memory benchmark tests since they measure instantaneous memory.
So, add logic to manually release Transactions/SurfaceControls
when we know they won't be used. This is extra tricky due to
unparceling creating new refcounts. To resolve this, we assume
that all remotes are getting a COPY of all surfaces and then we
make deep-ish copies on the CALLER side if the binder is actually
local. We can't alter the logic on the receiver side because the
binders are one-way and thus PID information isn't available.
Bug: 258913831
Test: systemui-notification-memory-suite-demo and check aggregates
Change-Id: If094e7eeb248a8674e8094cf5cf3019b2cd58047
diff --git a/core/java/android/window/TransitionInfo.java b/core/java/android/window/TransitionInfo.java
index 0956a71..666f316 100644
--- a/core/java/android/window/TransitionInfo.java
+++ b/core/java/android/window/TransitionInfo.java
@@ -414,6 +414,53 @@
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 @@
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 4e1fa29..485b400 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 @@
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 @@
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 @@
}
};
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 9469529..b4e0584 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 @@
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 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 @@
};
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 @@
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 @@
}
};
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 857decf..b714d2e 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
@@ -500,6 +500,7 @@
// 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 @@
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 @@
}
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 @@
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 @@
}
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 f9c6841..43bfa74 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 @@
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 @@
null /* wct */,
finishTransaction
)
+ finishTransaction.close()
} catch (e: RemoteException) {
Log.e(
"ActivityOptionsCompat",
@@ -364,6 +363,9 @@
) {
// 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 93c8073..1b0dacc 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 @@
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 @@
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 d4d3d25..b7e2494 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 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 @@
}
// 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 @@
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 @@
}
// 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 8846bbd..c332a0d 100644
--- a/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardService.java
+++ b/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardService.java
@@ -249,6 +249,7 @@
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 @@
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 @@
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 @@
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 a5a71ba..7ce17d4 100644
--- a/services/core/java/com/android/server/wm/Transition.java
+++ b/services/core/java/com/android/server/wm/Transition.java
@@ -751,6 +751,11 @@
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 @@
mController.mAtm.mWindowManager.updateRotation(false /* alwaysSendConfiguration */,
false /* forceRelayout */);
}
+ cleanUpInternal();
}
void abort() {
@@ -934,6 +940,7 @@
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 @@
"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 @@
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) {