Closed Bug 1359176 Opened 4 years ago Closed 4 years ago

Add GeckoView example app performance logs

Categories

(Core Graveyard :: Embedding: APIs, enhancement)

All
Android
enhancement
Not set
normal

Tracking

(firefox55 fixed)

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: esawin, Assigned: esawin)

References

Details

Attachments

(2 files, 1 obsolete file)

Add performance logs analogue to Fennec's S1/S2 logs to the GeckoView example app .
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-
java.lang.System.currentTimeMillis() ?
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.
what is bug 1357177 "use the new marker i.e. 'time to first nonblank paint'" and do we care?
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)
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 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+
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
https://hg.mozilla.org/mozilla-central/rev/f811e55616db
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Blocks: 1360333
Depends on: 1362087
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.