From 733437d8f512556839eff9fcd57a40a7051fc7da Mon Sep 17 00:00:00 2001 From: Jeremy Jiang Date: Tue, 15 Jun 2021 15:58:53 -0400 Subject: [PATCH 1/3] Fix inaccurate activity screen metrics. --- .../perf/application/AppStateMonitor.java | 15 +++------------ .../perf/application/AppStateMonitorTest.java | 3 +-- 2 files changed, 4 insertions(+), 14 deletions(-) diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/application/AppStateMonitor.java b/firebase-perf/src/main/java/com/google/firebase/perf/application/AppStateMonitor.java index b1456007167..8e79e35cf8c 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/application/AppStateMonitor.java +++ b/firebase-perf/src/main/java/com/google/firebase/perf/application/AppStateMonitor.java @@ -20,7 +20,6 @@ import android.content.Context; import android.os.Bundle; import android.util.SparseIntArray; -import android.view.WindowManager; import androidx.annotation.NonNull; import androidx.core.app.FrameMetricsAggregator; import com.google.android.gms.common.util.VisibleForTesting; @@ -308,7 +307,7 @@ private void sendScreenTrace(Activity activity) { int slowFrames = 0; int frozenFrames = 0; // Stops recording metrics for this Activity and returns the currently-collected metrics - SparseIntArray[] arr = frameMetricsAggregator.remove(activity); + SparseIntArray[] arr = frameMetricsAggregator.reset(); if (arr != null) { SparseIntArray frameTimes = arr[FrameMetricsAggregator.TOTAL_INDEX]; if (frameTimes != null) { @@ -391,21 +390,13 @@ private void sendSessionLog(String name, Timer startTime, Timer endTime) { } /** - * Only send screen trace if FrameMetricsAggregator exists and the activity is hardware - * accelerated. + * Only send screen trace if FrameMetricsAggregator exists. * * @param activity The Activity for which we're monitoring the screen rendering performance. * @return true if supported, false if not. */ private boolean isScreenTraceSupported(Activity activity) { - return hasFrameMetricsAggregator - // This check is needed because we can't observe frame rates for a non hardware accelerated - // view. - // See b/133827763. - && activity.getWindow() != null - && ((activity.getWindow().getAttributes().flags - & WindowManager.LayoutParams.FLAG_HARDWARE_ACCELERATED) - != 0); + return hasFrameMetricsAggregator; } /** diff --git a/firebase-perf/src/test/java/com/google/firebase/perf/application/AppStateMonitorTest.java b/firebase-perf/src/test/java/com/google/firebase/perf/application/AppStateMonitorTest.java index f6fe82fdcfb..1b1e35b823b 100644 --- a/firebase-perf/src/test/java/com/google/firebase/perf/application/AppStateMonitorTest.java +++ b/firebase-perf/src/test/java/com/google/firebase/perf/application/AppStateMonitorTest.java @@ -373,13 +373,12 @@ public void screenTrace_twoActivities_traceStartedAndStoppedWithActivityLifecycl } @Test - public void screenTrace_noHardwareAccelerated_traceNotCreated() { + public void screenTrace_noHardwareAccelerated_noExceptionThrown() { AppStateMonitor monitor = new AppStateMonitor(transportManager, clock); Activity activityWithNonHardwareAcceleratedView = createFakeActivity(/* isHardwareAccelerated =*/ false); monitor.onActivityStarted(activityWithNonHardwareAcceleratedView); - assertThat(monitor.getActivity2ScreenTrace()).isEmpty(); // Confirm that this doesn't throw an exception. monitor.onActivityStopped(activityWithNonHardwareAcceleratedView); From 5e9b2bf69b495f82463cc544b8b05696a757509f Mon Sep 17 00:00:00 2001 From: Jeremy Jiang Date: Wed, 16 Jun 2021 10:01:49 -0400 Subject: [PATCH 2/3] Add javadoc for hc acceleratin disabled. --- .../com/google/firebase/perf/application/AppStateMonitor.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/application/AppStateMonitor.java b/firebase-perf/src/main/java/com/google/firebase/perf/application/AppStateMonitor.java index 8e79e35cf8c..e9125c676e2 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/application/AppStateMonitor.java +++ b/firebase-perf/src/main/java/com/google/firebase/perf/application/AppStateMonitor.java @@ -289,7 +289,8 @@ public void onActivitySaveInstanceState(Activity activity, Bundle outState) {} public void onActivityPaused(Activity activity) {} /** - * Send screen trace. + * Send screen trace. If hardware acceleration is not enabled, all frame metrics will be zero and + * the trace will not be sent. * * @param activity activity object. */ From 8af227ed7bf1ddc145792c28466b3e04b8245218 Mon Sep 17 00:00:00 2001 From: Jeremy Jiang Date: Thu, 24 Jun 2021 18:45:07 -0400 Subject: [PATCH 3/3] Add comments and TODOs. --- .../firebase/perf/application/AppStateMonitor.java | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/application/AppStateMonitor.java b/firebase-perf/src/main/java/com/google/firebase/perf/application/AppStateMonitor.java index e9125c676e2..c1d2a7a63f3 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/application/AppStateMonitor.java +++ b/firebase-perf/src/main/java/com/google/firebase/perf/application/AppStateMonitor.java @@ -149,6 +149,10 @@ public void onActivityDestroyed(Activity activity) {} public synchronized void onActivityStarted(Activity activity) { if (isScreenTraceSupported(activity) && configResolver.isPerformanceMonitoringEnabled()) { // Starts recording frame metrics for this activity. + /** + * TODO: Only add activities that are hardware acceleration enabled so that calling {@link + * FrameMetricsAggregator#remove(Activity)} will not throw exceptions. + */ frameMetricsAggregator.add(activity); // Start the Trace Trace screenTrace = new Trace(getScreenTraceName(activity), transportManager, clock, this); @@ -307,7 +311,13 @@ private void sendScreenTrace(Activity activity) { int totalFrames = 0; int slowFrames = 0; int frozenFrames = 0; - // Stops recording metrics for this Activity and returns the currently-collected metrics + /** + * Resets the metrics data and returns the currently-collected metrics. Note that {@link + * FrameMetricsAggregator#reset()} will not stop recording for the activity. The reason of using + * {@link FrameMetricsAggregator#reset()} is that {@link + * FrameMetricsAggregator#remove(Activity)} will throw exceptions for hardware acceleration + * disabled activities. + */ SparseIntArray[] arr = frameMetricsAggregator.reset(); if (arr != null) { SparseIntArray frameTimes = arr[FrameMetricsAggregator.TOTAL_INDEX];