Fireperf: use the new class `FrameMetricsRecorder` in `AppStateMonitor` and `FragmentStateMonitor` #3683

leotianlizhan · 2022-04-26T21:40:57Z

With FrameMetricsRecorder taking care of managing FrameMetricsAggregator, the need for AppStateMonitor to use FrameMetricsAggregator is gone. This PR is to make change to AppStateMonitor to use the FrameMetricsRecorder for measuring frame metrics.

As a part of this, the lifecycle event used to measure the start and stop of an activity screen trace is aligned to make it work for all versions of Android.

google-oss-bot · 2022-04-26T21:47:49Z

Coverage Report ¹

Affected Products

`firebase-perf`

Overall coverage changed from 71.46% (c76d022) to 71.36% (0bf7345) by -0.10%.

Filename	Base (`c76d022`)	Merge (0bf7345)	Diff
AppStateMonitor.java	88.57%	86.71%	-1.87%
FragmentStateMonitor.java	91.67%	89.47%	-2.19%
FrameMetricsRecorder.java	77.14%	78.38%	+1.24%
RateLimiter.java	90.98%	91.73%	+0.75%
TransportManager.java	95.81%	94.88%	-0.93%

Test Logs

Notes

Commit (0bf7345) is created by Prow via merging PR base commit (c76d022) and head commit (1a62a8e).
Run gradle <product>:checkCoverage to produce HTML coverage reports locally. After gradle commands finished, report files can be found under <product-build-dir>/reports/jacoco/.

https://storage.googleapis.com/firebase-sdk-metric-reports/ydZBWxg2ER.html

google-oss-bot · 2022-04-26T21:53:49Z

Size Report ¹

Affected Products

firebase-perf
Type Base (c76d022) Merge (0bf7345) Diff
aar 311 kB 310 kB -255 B (-0.1%)
apk (aggressive) 1.03 MB 1.03 MB +1.04 kB (+0.1%)
apk (release) 2.47 MB 2.47 MB +104 B (+0.0%)

Test Logs

Notes

Commit (0bf7345) is created by Prow via merging PR base commit (c76d022) and head commit (1a62a8e).

https://storage.googleapis.com/firebase-sdk-metric-reports/vr2gmcx4BW.html

…ts follow unit test practices

… WIP.

…untime.

…d when the flags are toggled at runtime.

firebase-perf/src/main/java/com/google/firebase/perf/application/AppStateMonitor.java

jposuna · 2022-05-03T17:50:23Z

firebase-perf/src/test/java/com/google/firebase/perf/application/AppStateMonitorTest.java

@@ -15,6 +15,7 @@
 package com.google.firebase.perf.application;


The behavior of AppStateMonitor has changed quite a bit. I'm surprised the tests haven't changed barely at all. I feel like we're probably missing tests (e.g. testing new behaviors), as well as having coverage for the behavior changes that have happened (some things happen in different points of the activity's lifecycle)

The only lifecycle change is reverting the activity screen trace that is from onResume -> onPostPaused to onStart -> onStop. onStart -> onStop was the original behavior before attempting to fix the b/210055697 that started all of this in the first place. That's why there's no new tests for that.

firebase-perf/src/test/java/com/google/firebase/perf/application/FragmentStateMonitorTest.java

… for testing.

…mended by google style guide.

firebase-perf/src/main/java/com/google/firebase/perf/application/AppStateMonitor.java

leotianlizhan · 2022-05-03T23:35:05Z

firebase-perf/src/test/java/com/google/firebase/perf/application/AppStateMonitorTest.java

@@ -15,6 +15,7 @@
 package com.google.firebase.perf.application;


The only lifecycle change is reverting the activity screen trace that is from onResume -> onPostPaused to onStart -> onStop. onStart -> onStop was the original behavior before attempting to fix the b/210055697 that started all of this in the first place. That's why there's no new tests for that.

…rder class.

visumickey · 2022-05-04T16:58:56Z

/retest

jeremyjiang-dev

LGTM on checks green!

firebase-perf/src/main/java/com/google/firebase/perf/application/AppStateMonitor.java

firebase-perf/src/test/java/com/google/firebase/perf/application/FragmentStateMonitorTest.java

google-oss-bot added do-not-merge/work-in-progress size/XL labels Apr 26, 2022

leotianlizhan force-pushed the perf-apply-recorder branch 2 times, most recently from 929d70e to ef812f9 Compare April 27, 2022 18:14

leotianlizhan changed the title ~~Perf apply recorder~~ Fireperf: use the new class FrameMetricsRecorder in AppStateMonitor and FragmentStateMonitor Apr 27, 2022

leotianlizhan changed the base branch from perf-fix-fma to master April 27, 2022 18:31

leotianlizhan changed the base branch from master to perf-fix-fma April 27, 2022 18:32

leotianlizhan added 11 commits April 27, 2022 11:45

Add tests for snapshot()

a5bab3d

Remove all null and use Optional instead

207a203

snapshot uses FrameMetricsCalculator, make snapshot private, make tes…

994ee80

…ts follow unit test practices

remove equals

f3ec1a0

use FrameMetricsRecorder in AppStateMonitor and FragmentStateMonitor.…

02fa988

… WIP.

undereference when activity is destroyed

5e745d1

cbetter javadoc and comments

f0c5110

use new api startFragment

ae95028

fix 1 test

29f1fd1

remove test that is already covered in FrameMetricsRecorderTest

17cd156

fix conflict

b27349d

leotianlizhan force-pushed the perf-apply-recorder branch from 5cca75f to b27349d Compare April 27, 2022 18:47

google-oss-bot added size/L and removed size/XL labels Apr 27, 2022

leotianlizhan and others added 4 commits April 27, 2022 11:51

fix mistake when rebasing

85ed9a4

Update tests for Fragment State Monitor tests.

877d37b

Update tests to be more relevant and accurate.

2c7eea1

Update the Google Java Format!

ed07861

visumickey marked this pull request as ready for review May 2, 2022 16:42

google-oss-bot removed the do-not-merge/work-in-progress label May 2, 2022

visumickey requested a review from jeremyjiang-dev May 2, 2022 16:42

visumickey added 7 commits May 2, 2022 18:26

Fix the failing tests since onActivityCreated need not be called.

59031eb

Revert the test change.

6246b4f

Start frame recording when the performance monitoring is enabled at r…

fa9616b

…untime.

Enable screen trace creation only when the app goes back to foregroun…

30439a0

…d when the flags are toggled at runtime.

Add the missing static import.

7946ac5

Fix the failing test by adding all the activity lifecycle events.

63ad346

Fix the failing unit tests.

252490b

jposuna reviewed May 3, 2022

View reviewed changes

visumickey added 8 commits May 3, 2022 11:20

Make the state of Fragment State Monitor and Metrics Recorder visible…

69aa698

… for testing.

Fix the failing fragment monitor tests.

14ea1fc

Fix the failing tests.

857d9f2

Address some of the github comments.

f1e6fbf

Address few more comments.

fc74585

Unify the mocks in a single group in unit tests.

d4aba4f

Move away from doReturn().when() and use when().thenReturn() as recom…

c02d236

…mended by google style guide.

Revert back one method conversion from Mockito.when to Mockito.doThen.

abc8dd3

leotianlizhan commented May 3, 2022

View reviewed changes

visumickey added 3 commits May 3, 2022 16:46

Start frame recording when the activity is not recorded already!

630843e

Move the FrameMetricsAggregator class reference to Frame Metrics Reco…

79cd01c

…rder class.

Static import PerfFrameMetrics.

23d4663

visumickey requested review from jposuna and jeremyjiang-dev May 4, 2022 00:04

Keep style gods happy!

09a4fb1

jeremyjiang-dev approved these changes May 4, 2022

View reviewed changes

jposuna approved these changes May 4, 2022

View reviewed changes

firebase-perf/src/main/java/com/google/firebase/perf/application/AppStateMonitor.java Outdated Show resolved Hide resolved

firebase-perf/src/test/java/com/google/firebase/perf/application/FragmentStateMonitorTest.java Show resolved Hide resolved

Address the last set of comments.

1a62a8e

visumickey merged commit 316735f into perf-fix-fma May 5, 2022

visumickey deleted the perf-apply-recorder branch May 5, 2022 03:37

firebase locked and limited conversation to collaborators Jun 5, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fireperf: use the new class `FrameMetricsRecorder` in `AppStateMonitor` and `FragmentStateMonitor` #3683

Fireperf: use the new class `FrameMetricsRecorder` in `AppStateMonitor` and `FragmentStateMonitor` #3683

Uh oh!

leotianlizhan commented Apr 26, 2022 •

edited by visumickey

Loading

Uh oh!

google-oss-bot commented Apr 26, 2022 •

edited

Loading

`firebase-perf`

Uh oh!

google-oss-bot commented Apr 26, 2022 •

edited

Loading

`firebase-perf`

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jposuna May 3, 2022

Uh oh!

leotianlizhan May 3, 2022 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

leotianlizhan May 3, 2022 •

edited

Loading

Uh oh!

visumickey commented May 4, 2022

Uh oh!

jeremyjiang-dev left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

		@@ -15,6 +15,7 @@
		package com.google.firebase.perf.application;

Fireperf: use the new class FrameMetricsRecorder in AppStateMonitor and FragmentStateMonitor #3683

Fireperf: use the new class FrameMetricsRecorder in AppStateMonitor and FragmentStateMonitor #3683

Uh oh!

Conversation

leotianlizhan commented Apr 26, 2022 • edited by visumickey Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

google-oss-bot commented Apr 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage Report 1

Affected Products

firebase-perf

Test Logs

Notes

Uh oh!

google-oss-bot commented Apr 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Size Report 1

Affected Products

firebase-perf

Test Logs

Notes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jposuna May 3, 2022

Choose a reason for hiding this comment

Uh oh!

leotianlizhan May 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

leotianlizhan May 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

visumickey commented May 4, 2022

Uh oh!

jeremyjiang-dev left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Fireperf: use the new class `FrameMetricsRecorder` in `AppStateMonitor` and `FragmentStateMonitor` #3683

Fireperf: use the new class `FrameMetricsRecorder` in `AppStateMonitor` and `FragmentStateMonitor` #3683

leotianlizhan commented Apr 26, 2022 •

edited by visumickey

Loading

google-oss-bot commented Apr 26, 2022 •

edited

Loading

Coverage Report ¹

`firebase-perf`

google-oss-bot commented Apr 26, 2022 •

edited

Loading

Size Report ¹

`firebase-perf`

leotianlizhan May 3, 2022 •

edited

Loading

leotianlizhan May 3, 2022 •

edited

Loading