Add GeckoView example app performance logs

RESOLVED FIXED in Firefox 55

Status

enhancement
RESOLVED FIXED
2 years ago
3 months ago

People

(Reporter: esawin, Assigned: esawin)

Tracking

unspecified
mozilla55
All
Android
Dependency tree / graph

Firefox Tracking Flags

(firefox55 fixed)

Details

Attachments

(2 attachments, 1 obsolete attachment)

Assignee

Description

2 years ago
Add performance logs analogue to Fennec's S1/S2 logs to the GeckoView example app .
Assignee

Comment 1

2 years ago
Add GeckoView example app startup and page load times and rename Fennec throbber times for consistency.

Do we care about Gecko or GeckoView ready states?
Attachment #8861144 - Flags: review?(snorp)
Attachment #8861144 - Flags: review?(bob)
Comment on attachment 8861144 [details] [diff] [review]
0001-Bug-1359176-1.0-Add-performance-logs-to-GeckoView-ex.patch

Review of attachment 8861144 [details] [diff] [review]:
-----------------------------------------------------------------

We want to log System.uptimeMillis() in the log messages so bc doesn't have to rely on logcat time. Also, they should be easily machine parseable. JSON?
Attachment #8861144 - Flags: review?(snorp) → review-

Comment 3

2 years ago
java.lang.System.currentTimeMillis() ?

Comment 4

2 years ago
Actually, looking at https://developer.android.com/reference/android/os/SystemClock.html 

android.os.SystemClock.uptimeMillis()
This clock stops when the system enters deep sleep (CPU off, display dark, device waiting for external input), but is not affected by clock scaling, idle, or other power saving mechanisms. 

java.lang.System.currentTimeMillis()
The wall clock can be set by the user or the phone network (see setCurrentTimeMillis(long)), so the time may jump backwards or forwards unpredictably. This clock should only be used when correspondence with real-world dates and times is important, such as in a calendar or alarm clock application. Interval or elapsed time measurements should use a different clock. 

android.os.SystemClock.elapsedRealtime() 
the time since the system was booted, and include deep sleep. This clock is guaranteed to be monotonic, and continues to tick even when the CPU is in power saving modes, so is the recommend basis for general purpose interval timing. 

android.os.SystemClock.uptimeMillis() appears to be sufficient though it may be affected if we waiting for input or if the display is dark.

android.os.SystemClock.elapsedRealtime() actually seems the best choice.

android.os.SystemClock.uptimeMillis() is used in many different places in the code. I wonder if we shouldn't be consistent and just use android.os.SystemClock.elapsedRealtime() everywhere?
Yeah, according to that, elapsedRealtime() does look like the right choice.

Comment 6

2 years ago
what is bug 1357177 "use the new marker i.e. 'time to first nonblank paint'" and do we care?
Assignee

Comment 7

2 years ago
Replaced uptimeMillis() with elapsedRealtime() for all zerdatime logs.

Removed "* chrome startup finished" logs because their JS timestamps were not comparable and therefore not very useful.
"page load start" for "about:blank" should be close enough to the chrome startup finish, if we are interested in that event timestamp.

Is there anything else we could include into the logs to help with the measurement?
Attachment #8861144 - Attachment is obsolete: true
Attachment #8861144 - Flags: review?(bob)
Attachment #8861593 - Flags: review?(bob)
Attachment #8861593 - Flags: feedback?(snorp)

Comment 8

2 years ago
Handle the new messages in Autophone. This needs to land prior to the actual change. I've tested it with existing builds and with builds containing the changes.
Attachment #8861861 - Flags: review?(jmaher)

Comment 9

2 years ago
Comment on attachment 8861593 [details] [diff] [review]
0001-Bug-1359176-1.1-Add-consistent-performance-logs-acro.patch

lgtm.
Attachment #8861593 - Flags: review?(bob) → review+
Comment on attachment 8861861 [details] [diff] [review]
bug-1359176-v1.patch

Review of attachment 8861861 [details] [diff] [review]:
-----------------------------------------------------------------

this simplifies a lot of code.
Attachment #8861861 - Flags: review?(jmaher) → review+
Attachment #8861593 - Flags: feedback?(snorp) → feedback+

Comment 12

2 years ago
Pushed by esawin@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f811e55616db
[1.1] Add consistent performance logs across the GeckoView example app and Fennec. r=bc

Comment 13

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/f811e55616db
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Assignee

Updated

2 years ago
Blocks: 1360333

Updated

2 years ago
Depends on: 1362087

Updated

3 months ago
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.