Merge "Fix memory leak due to not removing ended or cancelled tasks" into sc-dev (a5e7d2ff) · Commits · e / os / android_frameworks_base

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

+8 −5

Original line number	Diff line number	Diff line
		@@ -27,6 +27,7 @@ import static android.view.SurfaceControl.JankData.SURFACE_FLINGER_SCHEDULING;
		import static com.android.internal.jank.InteractionJankMonitor.ACTION_METRICS_LOGGED;
		import static com.android.internal.jank.InteractionJankMonitor.ACTION_SESSION_BEGIN;
		import static com.android.internal.jank.InteractionJankMonitor.ACTION_SESSION_CANCEL;
		import static com.android.internal.jank.InteractionJankMonitor.ACTION_SESSION_END;

		import android.annotation.IntDef;
		import android.annotation.NonNull;
		@@ -215,7 +216,7 @@ public class FrameTracker extends SurfaceControl.OnJankDataListener
		mSurfaceControlWrapper.addJankStatsListener(this, mSurfaceControl);
		}
		if (mListener != null) {
		mListener.onNotifyCujEvents(mSession, ACTION_SESSION_BEGIN);
		mListener.onCujEvents(mSession, ACTION_SESSION_BEGIN);
		}
		}

		@@ -240,7 +241,9 @@ public class FrameTracker extends SurfaceControl.OnJankDataListener
		}
		Trace.endAsyncSection(mSession.getName(), (int) mBeginVsyncId);
		mSession.setReason(reason);
		InteractionJankMonitor.getInstance().removeTimeout(mSession.getCuj());
		if (mListener != null) {
		mListener.onCujEvents(mSession, ACTION_SESSION_END);
		}
		}
		// We don't remove observer here,
		// will remove it when all the frame metrics in this duration are called back.
		@@ -269,7 +272,7 @@ public class FrameTracker extends SurfaceControl.OnJankDataListener
		// Notify the listener the session has been cancelled.
		// We don't notify the listeners if the session never begun.
		if (mListener != null) {
		mListener.onNotifyCujEvents(mSession, ACTION_SESSION_CANCEL);
		mListener.onCujEvents(mSession, ACTION_SESSION_CANCEL);
		}
		}

		@@ -445,7 +448,7 @@ public class FrameTracker extends SurfaceControl.OnJankDataListener
		maxFrameTimeNanos,
		missedSfFramesCounts);
		if (mListener != null) {
		mListener.onNotifyCujEvents(mSession, ACTION_METRICS_LOGGED);
		mListener.onCujEvents(mSession, ACTION_METRICS_LOGGED);
		}
		}
		if (DEBUG) {
		@@ -587,6 +590,6 @@ public class FrameTracker extends SurfaceControl.OnJankDataListener
		* @param session the CUJ session
		* @param action the specific action
		*/
		void onNotifyCujEvents(Session session, String action);
		void onCujEvents(Session session, String action);
		}
		}

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

+42 −7

Original line number	Diff line number	Diff line
		@@ -19,7 +19,9 @@ package com.android.internal.jank;
		import static android.content.Intent.FLAG_RECEIVER_REGISTERED_ONLY;

		import static com.android.internal.jank.FrameTracker.ChoreographerWrapper;
		import static com.android.internal.jank.FrameTracker.REASON_CANCEL_NORMAL;
		import static com.android.internal.jank.FrameTracker.REASON_CANCEL_NOT_BEGUN;
		import static com.android.internal.jank.FrameTracker.REASON_END_NORMAL;
		import static com.android.internal.jank.FrameTracker.SurfaceControlWrapper;
		import static com.android.internal.util.FrameworkStatsLog.UIINTERACTION_FRAME_INFO_REPORTED__INTERACTION_TYPE__LAUNCHER_ALL_APPS_SCROLL;
		import static com.android.internal.util.FrameworkStatsLog.UIINTERACTION_FRAME_INFO_REPORTED__INTERACTION_TYPE__LAUNCHER_APP_CLOSE_TO_HOME;
		@@ -100,6 +102,7 @@ public class InteractionJankMonitor {
		private static final int DEFAULT_TRACE_THRESHOLD_FRAME_TIME_MILLIS = 64;

		public static final String ACTION_SESSION_BEGIN = ACTION_PREFIX + ".ACTION_SESSION_BEGIN";
		public static final String ACTION_SESSION_END = ACTION_PREFIX + ".ACTION_SESSION_END";
		public static final String ACTION_SESSION_CANCEL = ACTION_PREFIX + ".ACTION_SESSION_CANCEL";
		public static final String ACTION_METRICS_LOGGED = ACTION_PREFIX + ".ACTION_METRICS_LOGGED";
		public static final String BUNDLE_KEY_CUJ_NAME = ACTION_PREFIX + ".CUJ_NAME";
		@@ -275,10 +278,7 @@ public class InteractionJankMonitor {
		public FrameTracker createFrameTracker(View v, Session session) {
		final Context c = v.getContext().getApplicationContext();
		synchronized (this) {
		boolean needListener = SystemProperties.getBoolean(PROP_NOTIFY_CUJ_EVENT, false);
		FrameTrackerListener eventsListener =
		!needListener ? null : (s, act) -> notifyEvents(c, act, s);

		FrameTrackerListener eventsListener = (s, act) -> handleCujEvents(c, act, s);
		return new FrameTracker(session, mWorker.getThreadHandler(),
		new ThreadedRendererWrapper(v.getThreadedRenderer()),
		new ViewRootWrapper(v.getViewRootImpl()), new SurfaceControlWrapper(),
		@@ -287,6 +287,28 @@ public class InteractionJankMonitor {
		}
		}

		private void handleCujEvents(Context context, String action, Session session) {
		// Clear the running and timeout tasks if the end / cancel was fired within the tracker.
		// Or we might have memory leaks.
		if (needRemoveTasks(action, session)) {
		removeTimeout(session.getCuj());
		removeTracker(session.getCuj());
		}

		// Notify the receivers if necessary.
		if (session.shouldNotify()) {
		notifyEvents(context, action, session);
		}
		}

		private boolean needRemoveTasks(String action, Session session) {
		final boolean badEnd = action.equals(ACTION_SESSION_END)
		&& session.getReason() != REASON_END_NORMAL;
		final boolean badCancel = action.equals(ACTION_SESSION_CANCEL)
		&& session.getReason() != REASON_CANCEL_NORMAL;
		return badEnd \|\| badCancel;
		}

		private void notifyEvents(Context context, String action, Session session) {
		if (action.equals(ACTION_SESSION_CANCEL)
		&& session.getReason() == REASON_CANCEL_NOT_BEGUN) {
		@@ -299,7 +321,7 @@ public class InteractionJankMonitor {
		context.sendBroadcast(intent);
		}

		void removeTimeout(@CujType int cujType) {
		private void removeTimeout(@CujType int cujType) {
		synchronized (this) {
		Runnable timeout = mTimeoutActions.get(cujType);
		if (timeout != null) {
		@@ -371,7 +393,7 @@ public class InteractionJankMonitor {
		// Skip this call since we haven't started a trace yet.
		if (tracker == null) return false;
		tracker.end(FrameTracker.REASON_END_NORMAL);
		mRunningTrackers.remove(cujType);
		removeTracker(cujType);
		return true;
		}
		}
		@@ -390,7 +412,7 @@ public class InteractionJankMonitor {
		// Skip this call since we haven't started a trace yet.
		if (tracker == null) return false;
		tracker.cancel(FrameTracker.REASON_CANCEL_NORMAL);
		mRunningTrackers.remove(cujType);
		removeTracker(cujType);
		return true;
		}
		}
		@@ -401,6 +423,12 @@ public class InteractionJankMonitor {
		}
		}

		private void removeTracker(@CujType int cuj) {
		synchronized (this) {
		mRunningTrackers.remove(cuj);
		}
		}

		private void updateProperties(DeviceConfig.Properties properties) {
		synchronized (this) {
		mSamplingInterval = properties.getInt(SETTINGS_SAMPLING_INTERVAL_KEY,
		@@ -516,9 +544,11 @@ public class InteractionJankMonitor {
		private long mTimeStamp;
		@FrameTracker.Reasons
		private int mReason = FrameTracker.REASON_END_UNKNOWN;
		private boolean mShouldNotify;

		public Session(@CujType int cujType) {
		mCujType = cujType;
		mShouldNotify = SystemProperties.getBoolean(PROP_NOTIFY_CUJ_EVENT, false);
		}

		@CujType
		@@ -558,5 +588,10 @@ public class InteractionJankMonitor {
		public int getReason() {
		return mReason;
		}

		/** Determine if should notify the receivers of cuj events */
		public boolean shouldNotify() {
		return mShouldNotify;
		}
		}
		}