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

Commit 1f1b280e authored by Winson Chung's avatar Winson Chung
Browse files

Improve SurfaceControlRegistry logging for applied transactions



- For bugs like b/320403561, it's important to know both when specific
  SurfaceControl transaction calls are made, and when those changes are
  actually applied. The existing logging is insufficient because merges
  were not being tracked, and manually tracing when a call is actually
  applied through various merged transactions is complicated.

  This CL updates the apply() call tracking to include logs for all
  calls that are made on that transaction, filtered to the set of calls
  that are requested.  It only works if there are other specific calls
  requested (ie. it doesn't currently track all calls on a surface).

  ie. If you want to track show/hides across merged transactions:

  adb shell setprop persist.wm.debug.sc.tx.log_match_call show,hide,merge,apply
  adb reboot
  adb logcat -s "SurfaceControlRegistry"

  Outputs:
    java.lang.Throwable
	at android.view.SurfaceControlRegistry.checkCallStackDebugging()
	at android.view.SurfaceControl$Transaction.apply(
	at android.view.SurfaceControl$Transaction.apply(
	at android.view.SurfaceControl$Transaction.apply(
	at com.android.wm.shell.transition.Transitions.onFinish()
	at com.android.wm.shell.transition.Transitions.lambda$playTransition$2()
	at com.android.wm.shell.transition.Transitions.$r8$lambda$W7xYOftIaREiTW7mnjUmUz2jYkA()
	at com.android.wm.shell.transition.Transitions$$ExternalSyntheticLambda7.onTransitionFinished()
	at com.android.wm.shell.recents.RecentsTransitionHandler$RecentsController.finishInner()
	at com.android.wm.shell.recents.RecentsTransitionHandler$RecentsController.lambda$finish$9()
	at com.android.wm.shell.recents.RecentsTransitionHandler$RecentsController.$r8$lambda$dOp5QZTviEiK1rNS9-7xP7CDnzM()
	at com.android.wm.shell.recents.RecentsTransitionHandler$RecentsController$$ExternalSyntheticLambda4.run()
	at android.os.Handler.handleCallback(Handler.java:959)
	at android.os.Handler.dispatchMessage(Handler.java:100)
	at android.os.Looper.loopOnce(Looper.java:232)
	at android.os.Looper.loop(Looper.java:317)
	at android.os.HandlerThread.run(HandlerThread.java:85)
            hide (tx=135484743353255  sc=Task=13)
            show (tx=135484743353255  sc=Task=1)
            hide (tx=135484743353255  sc=Task=13)
            hide (tx=135484743353255  sc=Task=19)
            merge (tx=135484743353255 ) otherTx=136618614719772
            merge (tx=135484743353255 ) otherTx=135484743353292
            hide (tx=135484743353292  sc=Task=13)
            show (tx=135484743353292  sc=Task=19)

  Which can be cross referenced further with the actual calls on
  tx=135484743353292 earlier in the logs

Flag: EXEMPT bugfix
Bug: 320403561
Test: atest SurfaceControlRegistryTests

Change-Id: Iea772d0fdad5ae54cfa82136494538538e549e48
Signed-off-by: default avatarWinson Chung <winsonc@google.com>
parent 2a506a65
Loading
Loading
Loading
Loading
+40 −4
Original line number Diff line number Diff line
@@ -2812,6 +2812,10 @@ public final class SurfaceControl implements Parcelable {
        private final ArrayMap<SurfaceControl, Point> mResizedSurfaces = new ArrayMap<>();
        private final ArrayMap<SurfaceControl, SurfaceControl> mReparentedSurfaces =
                 new ArrayMap<>();
        // Only non-null if the SurfaceControlRegistry is enabled. This list tracks the set of calls
        // made through this transaction object, and is dumped (and cleared) when the transaction is
        // later applied.
        ArrayList<String> mCalls;

        Runnable mFreeNativeResources;
        private static final float[] INVALID_COLOR = {-1, -1, -1};
@@ -2837,13 +2841,28 @@ public final class SurfaceControl implements Parcelable {
        private Transaction(long nativeObject) {
            mNativeObject = nativeObject;
            mFreeNativeResources = sRegistry.registerNativeAllocation(this, mNativeObject);
            if (!SurfaceControlRegistry.sCallStackDebuggingInitialized) {
                SurfaceControlRegistry.initializeCallStackDebugging();
            }
            setUpForSurfaceControlRegistry();
        }

        private Transaction(Parcel in) {
            readFromParcel(in);
            setUpForSurfaceControlRegistry();
        }

        /**
         * Sets up this transaction for the SurfaceControlRegistry.
         */
        private void setUpForSurfaceControlRegistry() {
            if (!SurfaceControlRegistry.sCallStackDebuggingInitialized) {
                SurfaceControlRegistry.initializeCallStackDebugging();
            }
            mCalls = SurfaceControlRegistry.sLogAllTxCallsOnApply
                    ? new ArrayList<>()
                    : null;
            if (SurfaceControlRegistry.sCallStackDebuggingEnabled) {
                SurfaceControlRegistry.getProcessInstance().checkCallStackDebugging(
                        "ctor", this, null, null);
            }
        }

        /**
@@ -2893,6 +2912,9 @@ public final class SurfaceControl implements Parcelable {
            if (mNativeObject != 0) {
                nativeClearTransaction(mNativeObject);
            }
            if (mCalls != null) {
                mCalls.clear();
            }
        }

        /**
@@ -2904,6 +2926,9 @@ public final class SurfaceControl implements Parcelable {
            mReparentedSurfaces.clear();
            mFreeNativeResources.run();
            mNativeObject = 0;
            if (mCalls != null) {
                mCalls.clear();
            }
        }

        /**
@@ -2921,7 +2946,10 @@ public final class SurfaceControl implements Parcelable {

            if (SurfaceControlRegistry.sCallStackDebuggingEnabled) {
                SurfaceControlRegistry.getProcessInstance().checkCallStackDebugging(
                        "apply", this, null, null);
                        SurfaceControlRegistry.APPLY, this, null, null);
            }
            if (mCalls != null) {
                mCalls.clear();
            }
        }

@@ -4421,6 +4449,14 @@ public final class SurfaceControl implements Parcelable {
            if (this == other) {
                return this;
            }
            if (SurfaceControlRegistry.sCallStackDebuggingEnabled) {
                SurfaceControlRegistry.getProcessInstance().checkCallStackDebugging(
                        "merge", this, null, "otherTx=" + other.getId());
                if (mCalls != null) {
                    mCalls.addAll(other.mCalls);
                    other.mCalls.clear();
                }
            }
            mResizedSurfaces.putAll(other.mResizedSurfaces);
            other.mResizedSurfaces.clear();
            mReparentedSurfaces.putAll(other.mReparentedSurfaces);
+35 −9
Original line number Diff line number Diff line
@@ -44,6 +44,8 @@ import java.util.WeakHashMap;
 */
public class SurfaceControlRegistry {
    private static final String TAG = "SurfaceControlRegistry";
    // Special constant for identifying the Transaction#apply() calls
    static final String APPLY = "apply";

    /**
     * An interface for processing the registered SurfaceControls when the threshold is exceeded.
@@ -134,6 +136,10 @@ public class SurfaceControlRegistry {
    // sCallStackDebuggingEnabled is true.  Can be combined with the match name.
    private static String sCallStackDebuggingMatchCall;

    // When set, all calls on a SurfaceControl.Transaction will be stored and logged when the
    // transaction is applied.
    static boolean sLogAllTxCallsOnApply;

    // Mapping of the active SurfaceControls to the elapsed time when they were registered
    @GuardedBy("sLock")
    private final WeakHashMap<SurfaceControl, Long> mSurfaceControls;
@@ -185,6 +191,7 @@ public class SurfaceControlRegistry {
    public void setCallStackDebuggingParams(String matchName, String matchCall) {
        sCallStackDebuggingMatchName = matchName.toLowerCase();
        sCallStackDebuggingMatchCall = matchCall.toLowerCase();
        sLogAllTxCallsOnApply = sCallStackDebuggingMatchCall.contains("apply");
    }

    /**
@@ -294,13 +301,15 @@ public class SurfaceControlRegistry {
        sCallStackDebuggingMatchName =
                SystemProperties.get("persist.wm.debug.sc.tx.log_match_name", null)
                        .toLowerCase();
        sLogAllTxCallsOnApply = sCallStackDebuggingMatchCall.contains("apply");
        // Only enable stack debugging if any of the match filters are set
        sCallStackDebuggingEnabled = (!sCallStackDebuggingMatchCall.isEmpty()
                || !sCallStackDebuggingMatchName.isEmpty());
        sCallStackDebuggingEnabled = !sCallStackDebuggingMatchCall.isEmpty()
                || !sCallStackDebuggingMatchName.isEmpty();
        if (sCallStackDebuggingEnabled) {
            Log.d(TAG, "Enabling transaction call stack debugging:"
                    + " matchCall=" + sCallStackDebuggingMatchCall
                    + " matchName=" + sCallStackDebuggingMatchName);
                    + " matchName=" + sCallStackDebuggingMatchName
                    + " logCallsWithApply=" + sLogAllTxCallsOnApply);
        }
    }

@@ -319,15 +328,31 @@ public class SurfaceControlRegistry {
        if (!sCallStackDebuggingEnabled) {
            return;
        }
        if (!matchesForCallStackDebugging(sc != null ? sc.getName() : null, call)) {
            return;
        }

        final String txMsg = tx != null ? "tx=" + tx.getId() + " " : "";
        final String scMsg = sc != null ? " sc=" + sc.getName() + "" : "";
        final String msg = details != null
                ? call + " (" + txMsg + scMsg + ") " + details
                : call + " (" + txMsg + scMsg + ")";
        if (sLogAllTxCallsOnApply && tx != null) {
            if (call == APPLY) {
                // Log the apply and dump the calls on that transaction
                Log.e(TAG, msg, new Throwable());
                for (int i = 0; i < tx.mCalls.size(); i++) {
                    Log.d(TAG, "        " + tx.mCalls.get(i));
                }
            } else if (matchesForCallStackDebugging(sc != null ? sc.getName() : null, call)) {
                // Otherwise log this call to the transaction if it matches the tracked calls
                Log.e(TAG, msg, new Throwable());
                tx.mCalls.add(msg);
            }
        } else {
            // Log this call if it matches the tracked calls
            if (!matchesForCallStackDebugging(sc != null ? sc.getName() : null, call)) {
                return;
            }
            Log.e(TAG, msg, new Throwable());
        }
    }

    /**
@@ -388,6 +413,7 @@ public class SurfaceControlRegistry {
                pw.println("sCallStackDebuggingEnabled=" + sCallStackDebuggingEnabled);
                pw.println("sCallStackDebuggingMatchName=" + sCallStackDebuggingMatchName);
                pw.println("sCallStackDebuggingMatchCall=" + sCallStackDebuggingMatchCall);
                pw.println("sLogAllTxCallsOnApply=" + sLogAllTxCallsOnApply);
            }
        }
    }
+11 −1
Original line number Diff line number Diff line
@@ -73,7 +73,7 @@ stack traces when specific surface transaction calls are made, which is possible
following system properties for example:
```shell
# Enabling
adb shell setprop persist.wm.debug.sc.tx.log_match_call setAlpha  # matches the name of the SurfaceControlTransaction method
adb shell setprop persist.wm.debug.sc.tx.log_match_call setAlpha,setPosition  # matches the name of the SurfaceControlTransaction methods
adb shell setprop persist.wm.debug.sc.tx.log_match_name com.android.systemui # matches the name of the surface
adb reboot
adb logcat -s "SurfaceControlRegistry"
@@ -87,6 +87,16 @@ adb reboot
It is not necessary to set both `log_match_call` and `log_match_name`, but note logs can be quite
noisy if unfiltered.

It can sometimes be useful to trace specific logs and when they are applied (sometimes we build
transactions that can be applied later).  You can do this by adding the "merge" and "apply" calls to
the set of requested calls:
```shell
# Enabling
adb shell setprop persist.wm.debug.sc.tx.log_match_call setAlpha,merge,apply  # apply will dump logs of each setAlpha or merge call on that tx
adb reboot
adb logcat -s "SurfaceControlRegistry"
```

## Tracing activity starts in the app process

It's sometimes useful to know when to see a stack trace of when an activity starts in the app code