Skip to content

Fix inaccurate activity screen metrics. #2736

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jun 25, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -150,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);
Expand Down Expand Up @@ -290,7 +293,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.
*/
Expand All @@ -307,8 +311,14 @@ 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
SparseIntArray[] arr = frameMetricsAggregator.remove(activity);
/**
* 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];
if (frameTimes != null) {
Expand Down Expand Up @@ -391,21 +401,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;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down