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

Commit 9a458e39 authored by Pascal Mütschard's avatar Pascal Mütschard
Browse files

Jank callback API refactor. 

Removes the old work-arounds for missing jank callbacks.
Removes the jank data from the transaction completed callback.
Adds new function to ISurfaceComposer to register jank listeners.

With the new API, jank data is only sent over binder periodically
(every ~50 frames) and on a background thread. It is also only tracked
for layers where there is a listener registered.

Test: manual, libsurfaceflinger_unittest
Bug: http://b/336461947
Flag: EXEMPT refactor
Change-Id: I6637a11a6236fdd48998fa63ad823060a0ec3b2a
parent 7a304964

core/java/android/view/SurfaceControl.java

+61 −22
Original line number Diff line number Diff line
@@ -72,7 +72,6 @@ import android.view.Surface.OutOfResourcesException; 


import com.android.internal.annotations.GuardedBy;

import com.android.internal.util.Preconditions;

import com.android.internal.util.VirtualRefBasePtr;

import com.android.window.flags.Flags;



import dalvik.system.CloseGuard;
@@ -273,10 +272,12 @@ public final class SurfaceControl implements Parcelable { 
            String windowName, int displayId);

    private static native void nativeSetFrameTimelineVsync(long transactionObj,

            long frameTimelineVsyncId);

    private static native void nativeAddJankDataListener(long nativeListener,

            long nativeSurfaceControl);

    private static native void nativeRemoveJankDataListener(long nativeListener);

    private static native long nativeCreateJankDataListenerWrapper(OnJankDataListener listener);

    private static native long nativeCreateJankDataListenerWrapper(

            long surfaceControl, OnJankDataListener listener);

    private static native long nativeGetJankDataListenerWrapperFinalizer();

    private static native void nativeAddJankDataListener(long nativeListener);

    private static native void nativeFlushJankData(long nativeListener);

    private static native void nativeRemoveJankDataListener(long nativeListener, long afterVsync);

    private static native int nativeGetGPUContextPriority();

    private static native void nativeSetTransformHint(long nativeObject,

            @SurfaceControl.BufferTransform int transformHint);
@@ -461,17 +462,63 @@ public final class SurfaceControl implements Parcelable { 
     * @see #addJankDataListener

     * @hide

     */

    public static abstract class OnJankDataListener {

        private final VirtualRefBasePtr mNativePtr;

    public interface OnJankDataListener {

        /**

         * Called when new jank classifications are available.

         */

        void onJankDataAvailable(JankData[] jankData);



        public OnJankDataListener() {

            mNativePtr = new VirtualRefBasePtr(nativeCreateJankDataListenerWrapper(this));

    }



    /**

         * Called when new jank classifications are available.

     * Handle to a registered {@link OnJankDatalistener}.

     * @hide

     */

        public abstract void onJankDataAvailable(JankData[] jankStats);

    public static class OnJankDataListenerRegistration {

        private final long mNativeObject;



        private static final NativeAllocationRegistry sRegistry =

                NativeAllocationRegistry.createMalloced(

                       OnJankDataListenerRegistration.class.getClassLoader(),

                       nativeGetJankDataListenerWrapperFinalizer());



        private final Runnable mFreeNativeResources;

        private boolean mRemoved = false;



        OnJankDataListenerRegistration(SurfaceControl surface, OnJankDataListener listener) {

            mNativeObject = nativeCreateJankDataListenerWrapper(surface.mNativeObject, listener);

            mFreeNativeResources = (mNativeObject == 0) ? () -> {} :

                    sRegistry.registerNativeAllocation(this, mNativeObject);

        }



        /**

         * Request a flush of any pending jank classification data. May cause the registered

         * listener to be invoked inband.

         */

        public void flush() {

            nativeFlushJankData(mNativeObject);

        }



        /**

         * Request the removal of the registered listener after the VSync with the provided ID. Use

         * a value <= 0 for afterVsync to remove the listener immediately. The given listener will

         * not be removed before the given VSync, but may still reveive data for frames past the

         * provided VSync.

         */

        public void removeAfter(long afterVsync) {

            mRemoved = true;

            nativeRemoveJankDataListener(mNativeObject, afterVsync);

        }



        /**

         * Free the native resources associated with the listener registration.

         */

        public void release() {

            if (!mRemoved) {

                removeAfter(0);

            }

            mFreeNativeResources.run();

        }

    }



    private final CloseGuard mCloseGuard = CloseGuard.get();
@@ -2632,19 +2679,11 @@ public final class SurfaceControl implements Parcelable { 
    }



    /**

     * Adds a callback to be informed about SF's jank classification for a specific surface.

     * @hide

     */

    public static void addJankDataListener(OnJankDataListener listener, SurfaceControl surface) {

        nativeAddJankDataListener(listener.mNativePtr.get(), surface.mNativeObject);

    }



    /**

     * Removes a jank callback previously added with {@link #addJankDataListener}

     * Adds a callback to be informed about SF's jank classification for this surface.

     * @hide

     */

    public static void removeJankDataListener(OnJankDataListener listener) {

        nativeRemoveJankDataListener(listener.mNativePtr.get());

    public OnJankDataListenerRegistration addJankDataListener(OnJankDataListener listener) {

        return new OnJankDataListenerRegistration(this, listener);

    }



    /**

core/java/com/android/internal/jank/FrameTracker.java

+20 −12
Original line number Diff line number Diff line
@@ -63,8 +63,8 @@ import java.util.concurrent.TimeUnit; 
 * A class that allows the app to get the frame metrics from HardwareRendererObserver.

 * @hide

 */

public class FrameTracker extends SurfaceControl.OnJankDataListener

        implements HardwareRendererObserver.OnFrameMetricsAvailableListener {

public class FrameTracker implements HardwareRendererObserver.OnFrameMetricsAvailableListener,

         SurfaceControl.OnJankDataListener {

    private static final String TAG = "FrameTracker";



    private static final long INVALID_ID = -1;
@@ -118,6 +118,7 @@ public class FrameTracker extends SurfaceControl.OnJankDataListener 
    public final boolean mSurfaceOnly;



    private SurfaceControl mSurfaceControl;

    private SurfaceControl.OnJankDataListenerRegistration mJankDataListenerRegistration;

    private long mBeginVsyncId = INVALID_ID;

    private long mEndVsyncId = INVALID_ID;

    private boolean mMetricsFinalized;
@@ -316,6 +317,7 @@ public class FrameTracker extends SurfaceControl.OnJankDataListener 
        Trace.asyncTraceForTrackBegin(Trace.TRACE_TAG_APP, name, name, (int) mBeginVsyncId);

        markEvent("FT#beginVsync", mBeginVsyncId);

        markEvent("FT#layerId", mSurfaceControl.getLayerId());

        mJankDataListenerRegistration =

                mSurfaceControlWrapper.addJankStatsListener(this, mSurfaceControl);

        if (!mSurfaceOnly) {

            mRendererWrapper.addObserver(mObserver);
@@ -342,6 +344,10 @@ public class FrameTracker extends SurfaceControl.OnJankDataListener 
            markEvent("FT#endVsync", mEndVsyncId);

            Trace.asyncTraceForTrackEnd(Trace.TRACE_TAG_APP, name, (int) mBeginVsyncId);



            if (mJankDataListenerRegistration != null) {

                mJankDataListenerRegistration.removeAfter(mEndVsyncId);

            }



            // We don't remove observer here,

            // will remove it when all the frame metrics in this duration are called back.

            // See onFrameMetricsAvailable for the logic of removing the observer.
@@ -358,6 +364,9 @@ public class FrameTracker extends SurfaceControl.OnJankDataListener 
                    // Send a flush jank data transaction.

                    if (mSurfaceControl != null && mSurfaceControl.isValid()) {

                        SurfaceControl.Transaction.sendSurfaceFlushJankData(mSurfaceControl);

                        if (mJankDataListenerRegistration != null) {

                            mJankDataListenerRegistration.flush();

                        }

                    }



                    long delay;
@@ -680,7 +689,10 @@ public class FrameTracker extends SurfaceControl.OnJankDataListener 
    @VisibleForTesting

    @UiThread

    public void removeObservers() {

        mSurfaceControlWrapper.removeJankStatsListener(this);

        if (mJankDataListenerRegistration != null) {

            mJankDataListenerRegistration.release();

            mJankDataListenerRegistration = null;

        }

        if (!mSurfaceOnly) {

            // HWUI part.

            mRendererWrapper.removeObserver(mObserver);
@@ -796,14 +808,10 @@ public class FrameTracker extends SurfaceControl.OnJankDataListener 
    }



    public static class SurfaceControlWrapper {



        public void addJankStatsListener(SurfaceControl.OnJankDataListener listener,

                SurfaceControl surfaceControl) {

            SurfaceControl.addJankDataListener(listener, surfaceControl);

        }



        public void removeJankStatsListener(SurfaceControl.OnJankDataListener listener) {

            SurfaceControl.removeJankDataListener(listener);

        /** adds the jank listener to the given surface */

        public SurfaceControl.OnJankDataListenerRegistration addJankStatsListener(

                SurfaceControl.OnJankDataListener listener, SurfaceControl surfaceControl) {

            return surfaceControl.addJankDataListener(listener);

        }

    }

core/jni/android_view_SurfaceControl.cpp

+48 −21
Original line number Diff line number Diff line
@@ -22,6 +22,7 @@ 
#include <android/graphics/properties.h>

#include <android/graphics/region.h>

#include <android/gui/BnWindowInfosReportedListener.h>

#include <android/gui/JankData.h>

#include <android/hardware/display/IDeviceProductInfoConstants.h>

#include <android/os/IInputConstants.h>

#include <android_runtime/AndroidRuntime.h>
@@ -2062,11 +2063,13 @@ public: 
        env->DeleteWeakGlobalRef(mOnJankDataListenerWeak);

    }



    void onJankDataAvailable(const std::vector<JankData>& jankData) {

    bool onJankDataAvailable(const std::vector<gui::JankData>& jankData) override {

        JNIEnv* env = getEnv();



        jobject target = env->NewLocalRef(mOnJankDataListenerWeak);

        if (target == nullptr) return;

        if (target == nullptr) {

            return false;

        }



        jobjectArray jJankDataArray = env->NewObjectArray(jankData.size(),

                gJankDataClassInfo.clazz, nullptr);
@@ -2082,6 +2085,8 @@ public: 
                jJankDataArray);

        env->DeleteLocalRef(jJankDataArray);

        env->DeleteLocalRef(target);



        return true;

    }



private:
@@ -2096,29 +2101,49 @@ private: 
    jobject mOnJankDataListenerWeak;

};



static void nativeAddJankDataListener(JNIEnv* env, jclass clazz,

                                       jlong jankDataCallbackListenerPtr,

                                       jlong nativeSurfaceControl) {

static jlong nativeCreateJankDataListenerWrapper(JNIEnv* env, jclass clazz,

                                                 jlong nativeSurfaceControl, jobject listener) {

    sp<SurfaceControl> surface(reinterpret_cast<SurfaceControl *>(nativeSurfaceControl));

    if (surface == nullptr) {

        return 0;

    }



    sp<JankDataListenerWrapper> wrapper = sp<JankDataListenerWrapper>::make(env, listener);

    if (wrapper->addListener(std::move(surface)) != OK) {

        return 0;

    }



    wrapper->incStrong((void*)nativeCreateJankDataListenerWrapper);

    return reinterpret_cast<jlong>(wrapper.get());

}



static void destroyJankDatalistenerWrapper(void* ptr) {

    JankDataListenerWrapper* wrapper = reinterpret_cast<JankDataListenerWrapper*>(ptr);

    if (wrapper == nullptr) {

        return;

    }

    sp<JankDataListenerWrapper> wrapper =

            reinterpret_cast<JankDataListenerWrapper*>(jankDataCallbackListenerPtr);

    TransactionCompletedListener::getInstance()->addJankListener(wrapper, surface);

    wrapper->decStrong((void*)nativeCreateJankDataListenerWrapper);

}



static void nativeRemoveJankDataListener(JNIEnv* env, jclass clazz,

                                          jlong jankDataCallbackListenerPtr) {

    sp<JankDataListenerWrapper> wrapper =

            reinterpret_cast<JankDataListenerWrapper*>(jankDataCallbackListenerPtr);

    TransactionCompletedListener::getInstance()->removeJankListener(wrapper);

static jlong nativeGetJankDataListenerWrapperFinalizer() {

    return reinterpret_cast<jlong>(&destroyJankDatalistenerWrapper);

}



static jlong nativeCreateJankDataListenerWrapper(JNIEnv* env, jclass clazz,

                                                 jobject jankDataListenerObject) {

    return reinterpret_cast<jlong>(

            new JankDataListenerWrapper(env, jankDataListenerObject));

static void nativeFlushJankData(JNIEnv* env, jclass clazz, jlong listener) {

    sp<JankDataListenerWrapper> wrapper = reinterpret_cast<JankDataListenerWrapper*>(listener);

    if (wrapper == nullptr) {

        return;

    }

    wrapper->flushJankData();

}



static void nativeRemoveJankDataListener(JNIEnv* env, jclass clazz, jlong listener,

                                         jlong afterVsync) {

    sp<JankDataListenerWrapper> wrapper = reinterpret_cast<JankDataListenerWrapper*>(listener);

    if (wrapper == nullptr) {

        return;

    }

    wrapper->removeListener(afterVsync);

}



static jint nativeGetGPUContextPriority(JNIEnv* env, jclass clazz) {
@@ -2436,12 +2461,14 @@ static const JNINativeMethod sSurfaceControlMethods[] = { 
            (void*)nativeRemoveCurrentInputFocus},

    {"nativeSetFrameTimelineVsync", "(JJ)V",

            (void*)nativeSetFrameTimelineVsync },

    {"nativeAddJankDataListener", "(JJ)V",

            (void*)nativeAddJankDataListener },

    {"nativeRemoveJankDataListener", "(J)V",

    {"nativeFlushJankData", "(J)V",

            (void*)nativeFlushJankData },

    {"nativeRemoveJankDataListener", "(JJ)V",

            (void*)nativeRemoveJankDataListener },

    {"nativeCreateJankDataListenerWrapper", "(Landroid/view/SurfaceControl$OnJankDataListener;)J",

    {"nativeCreateJankDataListenerWrapper", "(JLandroid/view/SurfaceControl$OnJankDataListener;)J",

            (void*)nativeCreateJankDataListenerWrapper },

    {"nativeGetJankDataListenerWrapperFinalizer", "()J",

            (void*)nativeGetJankDataListenerWrapperFinalizer },

    {"nativeGetGPUContextPriority", "()I",

            (void*)nativeGetGPUContextPriority },

    {"nativeSetTransformHint", "(JI)V",

core/tests/coretests/src/com/android/internal/jank/FrameTrackerTest.java

+10 −7
Original line number Diff line number Diff line
@@ -29,6 +29,7 @@ import static com.android.internal.util.FrameworkStatsLog.UI_INTERACTION_FRAME_I 
import static com.google.common.truth.Truth.assertThat;



import static org.mockito.ArgumentMatchers.any;

import static org.mockito.ArgumentMatchers.anyLong;

import static org.mockito.ArgumentMatchers.eq;

import static org.mockito.Mockito.doNothing;

import static org.mockito.Mockito.doReturn;
@@ -83,6 +84,7 @@ public class FrameTrackerTest { 
    private ChoreographerWrapper mChoreographer;

    private StatsLogWrapper mStatsLog;

    private ArgumentCaptor<OnJankDataListener> mListenerCapture;

    private SurfaceControl.OnJankDataListenerRegistration mJankStatsRegistration;

    private SurfaceControl mSurfaceControl;

    private FrameTracker.FrameTrackerListener mTrackerListener;

    private ArgumentCaptor<Runnable> mRunnableArgumentCaptor;
@@ -107,10 +109,11 @@ public class FrameTrackerTest { 
        mSurfaceControlWrapper = mock(SurfaceControlWrapper.class);



        mListenerCapture = ArgumentCaptor.forClass(OnJankDataListener.class);

        doNothing().when(mSurfaceControlWrapper).addJankStatsListener(

        mJankStatsRegistration = mock(SurfaceControl.OnJankDataListenerRegistration.class);

        doReturn(mJankStatsRegistration).when(mSurfaceControlWrapper).addJankStatsListener(

                mListenerCapture.capture(), any());

        doNothing().when(mSurfaceControlWrapper).removeJankStatsListener(

                mListenerCapture.capture());

        doNothing().when(mJankStatsRegistration).flush();

        doNothing().when(mJankStatsRegistration).removeAfter(anyLong());



        mChoreographer = mock(ChoreographerWrapper.class);

        mStatsLog = mock(StatsLogWrapper.class);
@@ -483,7 +486,7 @@ public class FrameTrackerTest { 
        // an extra frame to trigger finish

        sendFrame(tracker, JANK_NONE, 103L);



        verify(mSurfaceControlWrapper).removeJankStatsListener(any());

        verify(mJankStatsRegistration).removeAfter(anyLong());

        verify(mTrackerListener).triggerPerfetto(any());



        verify(mStatsLog).write(eq(UI_INTERACTION_FRAME_INFO_REPORTED),
@@ -520,7 +523,7 @@ public class FrameTrackerTest { 
        // an extra frame to trigger finish

        sendFrame(tracker, JANK_NONE, 103L);



        verify(mSurfaceControlWrapper).removeJankStatsListener(any());

        verify(mJankStatsRegistration).removeAfter(anyLong());

        verify(mTrackerListener, never()).triggerPerfetto(any());



        verify(mStatsLog).write(eq(UI_INTERACTION_FRAME_INFO_REPORTED),
@@ -557,7 +560,7 @@ public class FrameTrackerTest { 
        // janky frame, should be ignored, trigger finish

        sendFrame(tracker, JANK_APP_DEADLINE_MISSED, 103L);



        verify(mSurfaceControlWrapper).removeJankStatsListener(any());

        verify(mJankStatsRegistration).removeAfter(anyLong());

        verify(mTrackerListener, never()).triggerPerfetto(any());



        verify(mStatsLog).write(eq(UI_INTERACTION_FRAME_INFO_REPORTED),
@@ -589,7 +592,7 @@ public class FrameTrackerTest { 
        tracker.end(FrameTracker.REASON_END_NORMAL);

        sendFrame(tracker, JANK_SURFACEFLINGER_DEADLINE_MISSED, 106L);

        sendFrame(tracker, JANK_SURFACEFLINGER_DEADLINE_MISSED, 107L);

        verify(mSurfaceControlWrapper).removeJankStatsListener(any());

        verify(mJankStatsRegistration).removeAfter(anyLong());

        verify(mTrackerListener).triggerPerfetto(any());

        verify(mStatsLog).write(eq(UI_INTERACTION_FRAME_INFO_REPORTED),

                eq(42), /* displayId */

core/tests/coretests/src/com/android/internal/jank/InteractionJankMonitorTest.java

+0 −2
Original line number Diff line number Diff line
@@ -183,8 +183,6 @@ public class InteractionJankMonitorTest { 
        doNothing().when(viewRoot).removeSurfaceChangedCallback(any());



        SurfaceControlWrapper surfaceControl = mock(SurfaceControlWrapper.class);

        doNothing().when(surfaceControl).addJankStatsListener(any(), any());

        doNothing().when(surfaceControl).removeJankStatsListener(any());



        final ChoreographerWrapper choreographer = mock(ChoreographerWrapper.class);

        doReturn(SystemClock.elapsedRealtime()).when(choreographer).getVsyncId();