Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 34a703b4 authored by Evan Rosky's avatar Evan Rosky
Browse files

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
parent c2081458

core/java/android/window/TransitionInfo.java

+68 −0
Original line number Diff line number Diff line
@@ -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;

libs/WindowManager/Shell/src/com/android/wm/shell/transition/OneShotRemoteHandler.java

+17 −5
Original line number Diff line number Diff line
@@ -77,10 +77,10 @@ public class OneShotRemoteHandler implements Transitions.TransitionHandler { 
                if (mRemote.asBinder() != null) {

                    mRemote.asBinder().unlinkToDeath(remoteDied, 0 /* flags */);

                }

                mMainExecutor.execute(() -> {

                if (sct != null) {

                    finishTransaction.merge(sct);

                }

                mMainExecutor.execute(() -> {

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

        }

libs/WindowManager/Shell/src/com/android/wm/shell/transition/RemoteTransitionHandler.java

+38 −5
Original line number Diff line number Diff line
@@ -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);

                mMainExecutor.execute(() -> {

                if (sct != null) {

                    finishTransaction.merge(sct);

                }

                mMainExecutor.execute(() -> {

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

        }

libs/WindowManager/Shell/src/com/android/wm/shell/transition/Transitions.java

+19 −1
Original line number Diff line number Diff line
@@ -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 "

packages/SystemUI/animation/src/com/android/systemui/animation/RemoteTransitionAdapter.kt

+5 −3
Original line number Diff line number Diff line
@@ -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()

                }

            }

        }