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

Commit 5a1eab34 authored by Chavi Weingarten's avatar Chavi Weingarten
Browse files

Only prevent overlapping SSG if a buffer is going to be synced 

In VRI, SSG are sometimes created as a way to notify WMS that a buffer
has been drawn and not always to sync the buffer. In those cases,
there's no need to prevent overlapping syncs since the buffers cannot be
submitted out of order. Only add the safeguard SSG when there's a buffer
getting synced to ensure multiple buffer syncs for the same VRI are
submitted in order.

Test: SurfaceSyncGroupTests SurfaceSyncGroupTest
Bug: 279096620
Bug: 279790175
Change-Id: I77841296297717e413237c183738b9ec9642f1cf
parent d33c5e3d

core/java/android/view/ViewRootImpl.java

+14 −6
Original line number Diff line number Diff line
@@ -3772,6 +3772,16 @@ public final class ViewRootImpl implements ViewParent, 
            createSyncIfNeeded();

            notifyDrawStarted(isInWMSRequestedSync());

            mDrewOnceForSync = true;



            // If the active SSG is also requesting to sync a buffer, the following needs to happen

            // 1. Ensure we keep track of the number of active syncs to know when to disable RT

            //    RT animations that conflict with syncing a buffer.

            // 2. Add a safeguard SSG to prevent multiple SSG that sync buffers from being submitted

            //    out of order.

            if (mActiveSurfaceSyncGroup != null && mSyncBuffer) {

                updateSyncInProgressCount(mActiveSurfaceSyncGroup);

                safeguardOverlappingSyncs(mActiveSurfaceSyncGroup);

            }

        }



        if (!isViewVisible) {
@@ -3836,14 +3846,11 @@ public final class ViewRootImpl implements ViewParent, 
            mWmsRequestSyncGroupState = WMS_SYNC_MERGED;

            reportDrawFinished(t, seqId);

        });

        Trace.traceBegin(Trace.TRACE_TAG_VIEW,

                "create WMS Sync group=" + mWmsRequestSyncGroup.getName());

        if (DEBUG_BLAST) {

            Log.d(mTag, "Setup new sync=" + mWmsRequestSyncGroup.getName());

        }



        mWmsRequestSyncGroup.add(this, null /* runnable */);

        Trace.traceEnd(Trace.TRACE_TAG_VIEW);

    }



    private void notifyContentCaptureEvents() {
@@ -4504,6 +4511,9 @@ public final class ViewRootImpl implements ViewParent, 
            Log.d(mTag, "reportDrawFinished");

        }



        if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) {

            Trace.instant(Trace.TRACE_TAG_VIEW, "reportDrawFinished " + mTag + " seqId=" + seqId);

        }

        try {

            mWindowSession.finishDrawing(mWindow, t, seqId);

        } catch (RemoteException e) {
@@ -11387,7 +11397,7 @@ public final class ViewRootImpl implements ViewParent, 
     * ensure the latter SSG always waits for the former SSG's transaction to get to SF.

     */

    private void safeguardOverlappingSyncs(SurfaceSyncGroup activeSurfaceSyncGroup) {

        SurfaceSyncGroup safeguardSsg = new SurfaceSyncGroup("VRI-Safeguard");

        SurfaceSyncGroup safeguardSsg = new SurfaceSyncGroup("Safeguard-" + mTag);

        // Always disable timeout on the safeguard sync

        safeguardSsg.toggleTimeout(false /* enable */);

        synchronized (mPreviousSyncSafeguardLock) {
@@ -11446,8 +11456,6 @@ public final class ViewRootImpl implements ViewParent, 
                    mHandler.post(runnable);

                }

            });

            safeguardOverlappingSyncs(mActiveSurfaceSyncGroup);

            updateSyncInProgressCount(mActiveSurfaceSyncGroup);

            newSyncGroup = true;

        }

core/java/android/window/SurfaceSyncGroup.java

+38 −41
Original line number Diff line number Diff line
@@ -53,7 +53,6 @@ import java.util.function.Supplier; 
 * This will also allow synchronization of surfaces across multiple processes. The caller can add

 * SurfaceControlViewHosts from another process to the SurfaceSyncGroup in a different process

 * and this clas will ensure all the surfaces are ready before applying everything together.

 * </p>

 * see the <a href="https://android.googlesource.com/platform/frameworks/base/+/master/core/java/android/window/SurfaceSyncGroup.md">SurfaceSyncGroup documentation</a>

 * </p>

 */
@@ -136,6 +135,7 @@ public final class SurfaceSyncGroup { 
    @GuardedBy("mLock")

    private boolean mTimeoutDisabled;



    private final String mTrackName;



    private static boolean isLocalBinder(IBinder binder) {

        return !(binder instanceof BinderProxy);
@@ -192,6 +192,7 @@ public final class SurfaceSyncGroup { 
        }



        mName = name + "#" + sCounter.getAndIncrement();

        mTrackName = "SurfaceSyncGroup " + name;



        mTransactionReadyConsumer = (transaction) -> {

            if (DEBUG && transaction != null) {
@@ -199,9 +200,10 @@ public final class SurfaceSyncGroup { 
                        + mName);

            }

            if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) {

                Trace.instant(Trace.TRACE_TAG_VIEW,

                        "Final TransactionCallback with " + transaction + " for " + mName);

                Trace.instantForTrack(Trace.TRACE_TAG_VIEW, mTrackName,

                        "Final TransactionCallback with " + transaction);

            }

            Trace.asyncTraceForTrackEnd(Trace.TRACE_TAG_VIEW, mTrackName, hashCode());

            transactionReadyConsumer.accept(transaction);

            synchronized (mLock) {

                // If there's a registered listener with WMS, that means we aren't actually complete
@@ -213,7 +215,7 @@ public final class SurfaceSyncGroup { 
        };



        if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) {

            Trace.instant(Trace.TRACE_TAG_VIEW, "new SurfaceSyncGroup " + mName);

            Trace.asyncTraceForTrackBegin(Trace.TRACE_TAG_VIEW, mTrackName, mName, hashCode());

        }



        if (DEBUG) {
@@ -257,7 +259,7 @@ public final class SurfaceSyncGroup { 
            Log.d(TAG, "markSyncReady " + mName);

        }

        if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) {

            Trace.traceBegin(Trace.TRACE_TAG_VIEW, "markSyncReady " + mName);

            Trace.instantForTrack(Trace.TRACE_TAG_VIEW, mTrackName, "markSyncReady");

        }

        synchronized (mLock) {

            if (mHasWMSync) {
@@ -269,9 +271,6 @@ public final class SurfaceSyncGroup { 
            mSyncReady = true;

            checkIfSyncIsComplete();

        }

        if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) {

            Trace.traceEnd(Trace.TRACE_TAG_VIEW);

        }

    }



    /**
@@ -399,14 +398,14 @@ public final class SurfaceSyncGroup { 
    public boolean add(ISurfaceSyncGroup surfaceSyncGroup, boolean parentSyncGroupMerge,

            @Nullable Runnable runnable) {

        if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) {

            Trace.traceBegin(Trace.TRACE_TAG_VIEW,

                    "addToSync token=" + mToken.hashCode() + " parent=" + mName);

            Trace.asyncTraceForTrackBegin(Trace.TRACE_TAG_VIEW, mTrackName,

                    "addToSync token=" + mToken.hashCode(), hashCode());

        }

        synchronized (mLock) {

            if (mSyncReady) {

                Log.w(TAG, "Trying to add to sync when already marked as ready " + mName);

                if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) {

                    Trace.traceEnd(Trace.TRACE_TAG_VIEW);

                    Trace.asyncTraceForTrackEnd(Trace.TRACE_TAG_VIEW, mTrackName, hashCode());

                }

                return false;

            }
@@ -419,7 +418,7 @@ public final class SurfaceSyncGroup { 
        if (isLocalBinder(surfaceSyncGroup.asBinder())) {

            boolean didAddLocalSync = addLocalSync(surfaceSyncGroup, parentSyncGroupMerge);

            if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) {

                Trace.traceEnd(Trace.TRACE_TAG_VIEW);

                Trace.asyncTraceForTrackEnd(Trace.TRACE_TAG_VIEW, mTrackName, hashCode());

            }

            return didAddLocalSync;

        }
@@ -447,7 +446,7 @@ public final class SurfaceSyncGroup { 
                        mSurfaceSyncGroupCompletedListener)) {

                    mSurfaceSyncGroupCompletedListener = null;

                    if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) {

                        Trace.traceEnd(Trace.TRACE_TAG_VIEW);

                        Trace.asyncTraceForTrackEnd(Trace.TRACE_TAG_VIEW, mTrackName, hashCode());

                    }

                    return false;

                }
@@ -459,13 +458,13 @@ public final class SurfaceSyncGroup { 
            surfaceSyncGroup.onAddedToSyncGroup(mToken, parentSyncGroupMerge);

        } catch (RemoteException e) {

            if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) {

                Trace.traceEnd(Trace.TRACE_TAG_VIEW);

                Trace.asyncTraceForTrackEnd(Trace.TRACE_TAG_VIEW, mTrackName, hashCode());

            }

            return false;

        }



        if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) {

            Trace.traceEnd(Trace.TRACE_TAG_VIEW);

            Trace.asyncTraceForTrackEnd(Trace.TRACE_TAG_VIEW, mTrackName, hashCode());

        }

        return true;

    }
@@ -510,15 +509,15 @@ public final class SurfaceSyncGroup { 
                        + ". Setting up Sync in WindowManager.");

            }

            if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) {

                Trace.traceBegin(Trace.TRACE_TAG_VIEW,

                        "addSyncToWm=" + token.hashCode() + " group=" + mName);

                Trace.asyncTraceForTrackBegin(Trace.TRACE_TAG_VIEW, mTrackName,

                        "addSyncToWm=" + token.hashCode(), hashCode());

            }

            AddToSurfaceSyncGroupResult addToSyncGroupResult = new AddToSurfaceSyncGroupResult();

            if (!WindowManagerGlobal.getWindowManagerService().addToSurfaceSyncGroup(token,

                    parentSyncGroupMerge, surfaceSyncGroupCompletedListener,

                    addToSyncGroupResult)) {

                if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) {

                    Trace.traceEnd(Trace.TRACE_TAG_VIEW);

                    Trace.asyncTraceForTrackEnd(Trace.TRACE_TAG_VIEW, mTrackName, hashCode());

                }

                return false;

            }
@@ -527,12 +526,12 @@ public final class SurfaceSyncGroup { 
                    addToSyncGroupResult.mTransactionReadyCallback);

        } catch (RemoteException e) {

            if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) {

                Trace.traceEnd(Trace.TRACE_TAG_VIEW);

                Trace.asyncTraceForTrackEnd(Trace.TRACE_TAG_VIEW, mTrackName, hashCode());

            }

            return false;

        }

        if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) {

            Trace.traceEnd(Trace.TRACE_TAG_VIEW);

            Trace.asyncTraceForTrackEnd(Trace.TRACE_TAG_VIEW, mTrackName, hashCode());

        }

        return true;

    }
@@ -550,8 +549,8 @@ public final class SurfaceSyncGroup { 
        }



        if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) {

            Trace.traceBegin(Trace.TRACE_TAG_VIEW,

                    "addLocalSync=" + childSurfaceSyncGroup.mName + " parent=" + mName);

            Trace.asyncTraceForTrackBegin(Trace.TRACE_TAG_VIEW, mTrackName,

                    "addLocalSync=" + childSurfaceSyncGroup.mName, hashCode());

        }

        ITransactionReadyCallback callback =

                createTransactionReadyCallback(parentSyncGroupMerge);
@@ -562,7 +561,7 @@ public final class SurfaceSyncGroup { 


        childSurfaceSyncGroup.setTransactionCallbackFromParent(mISurfaceSyncGroup, callback);

        if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) {

            Trace.traceEnd(Trace.TRACE_TAG_VIEW);

            Trace.asyncTraceForTrackEnd(Trace.TRACE_TAG_VIEW, mTrackName, hashCode());

        }

        return true;

    }
@@ -574,9 +573,9 @@ public final class SurfaceSyncGroup { 
        }



        if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) {

            Trace.traceBegin(Trace.TRACE_TAG_VIEW,

            Trace.asyncTraceForTrackBegin(Trace.TRACE_TAG_VIEW, mTrackName,

                    "setTransactionCallbackFromParent " + mName + " callback="

                            + transactionReadyCallback.hashCode());

                            + transactionReadyCallback.hashCode(), hashCode());

        }



        // Start the timeout when this SurfaceSyncGroup has been added to a parent SurfaceSyncGroup.
@@ -617,9 +616,9 @@ public final class SurfaceSyncGroup { 
                mParentSyncGroup = parentSyncGroup;

                mTransactionReadyConsumer = (transaction) -> {

                    if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) {

                        Trace.traceBegin(Trace.TRACE_TAG_VIEW,

                                "transactionReadyCallback " + mName + " callback="

                                        + transactionReadyCallback.hashCode());

                        Trace.asyncTraceForTrackBegin(Trace.TRACE_TAG_VIEW, mTrackName,

                                "Invoke transactionReadyCallback="

                                        + transactionReadyCallback.hashCode(), hashCode());

                    }

                    lastCallback.accept(null);
@@ -629,7 +628,7 @@ public final class SurfaceSyncGroup { 
                        transaction.apply();

                    }

                    if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) {

                        Trace.traceEnd(Trace.TRACE_TAG_VIEW);

                        Trace.asyncTraceForTrackEnd(Trace.TRACE_TAG_VIEW, mTrackName, hashCode());

                    }

                };

                addedToSyncListener = mAddedToSyncListener;
@@ -647,7 +646,7 @@ public final class SurfaceSyncGroup { 
            addedToSyncListener.run();

        }

        if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) {

            Trace.traceEnd(Trace.TRACE_TAG_VIEW);

            Trace.asyncTraceForTrackEnd(Trace.TRACE_TAG_VIEW, mTrackName, hashCode());

        }

    }
@@ -669,8 +668,8 @@ public final class SurfaceSyncGroup { 
        }



        if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) {

            Trace.instant(Trace.TRACE_TAG_VIEW,

                    "checkIfSyncIsComplete " + mName + " mSyncReady=" + mSyncReady

            Trace.instantForTrack(Trace.TRACE_TAG_VIEW, mTrackName,

                    "checkIfSyncIsComplete mSyncReady=" + mSyncReady

                            + " mPendingSyncs=" + mPendingSyncs.size());

        }
@@ -726,9 +725,8 @@ public final class SurfaceSyncGroup { 
                            }

                            mPendingSyncs.remove(this);

                            if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) {

                                Trace.instant(Trace.TRACE_TAG_VIEW,

                                        "onTransactionReady group=" + mName + " callback="

                                                + hashCode());

                                Trace.instantForTrack(Trace.TRACE_TAG_VIEW, mTrackName,

                                        "onTransactionReady callback=" + hashCode());

                            }

                            checkIfSyncIsComplete();

                        }
@@ -743,8 +741,8 @@ public final class SurfaceSyncGroup { 
            }

            mPendingSyncs.add(transactionReadyCallback);

            if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) {

                Trace.instant(Trace.TRACE_TAG_VIEW,

                        "createTransactionReadyCallback " + mName + " mPendingSyncs="

                Trace.instantForTrack(Trace.TRACE_TAG_VIEW, mTrackName,

                        "createTransactionReadyCallback mPendingSyncs="

                                + mPendingSyncs.size() + " transactionReady="

                                + transactionReadyCallback.hashCode());

            }
@@ -764,13 +762,12 @@ public final class SurfaceSyncGroup { 
        public boolean onAddedToSyncGroup(IBinder parentSyncGroupToken,

                boolean parentSyncGroupMerge) {

            if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) {

                Trace.traceBegin(Trace.TRACE_TAG_VIEW,

                        "onAddedToSyncGroup token=" + parentSyncGroupToken.hashCode() + " child="

                                + mName);

                Trace.asyncTraceForTrackBegin(Trace.TRACE_TAG_VIEW, mTrackName,

                        "onAddedToSyncGroup token=" + parentSyncGroupToken.hashCode(), hashCode());

            }

            boolean didAdd = addSyncToWm(parentSyncGroupToken, parentSyncGroupMerge, null);

            if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) {

                Trace.traceEnd(Trace.TRACE_TAG_VIEW);

                Trace.asyncTraceForTrackEnd(Trace.TRACE_TAG_VIEW, mTrackName, hashCode());

            }

            return didAdd;

        }