841561 - Move session recording and activity monitoring to Gecko

Status: RESOLVED WONTFIX

(Firefox Health Report Graveyard :: Client: Desktop, defect, P2)

Description

[Gregory Szorc [:gps]]

FHR currently has session recording and activity monitoring implemented in JS. The Perf Team says it needs to live in C++ for perf reasons. This bug will track implementing things in C++.

Note that Metrics wants the recorded data to be slightly different. We need to figure out what Metrics wants before we blindly port code to C++.

Comment 1

[Gregory Szorc [:gps]]

When this is implemented, we may also want to look at supporting the statistical requirements implemented in bug 836010. IMO the slow startup detection should query a robust and authoritative session recorder for data rather than recording it itself. If nothing else, having it reach into this Gecko component would allow easy deployment to other Gecko applications (like Android) without code duplication.

Comment 2

[Gregory Szorc [:gps]]

This is #3 from the perf review:

3. User-activity tracking The FHR user-activity handler should be implemented natively. The overhead of one JS event running every 5 seconds (while the user is interacting with the browser) is not likely to cause any user-visible jank, but this functionality really belongs in platform code. Once this data provider is moved to native code, FHR will have 1) "pull type" data providers that are only polled at shutdown or upload time, and 2) "push type" providers that record specific user actions (e.g. Google searches). If we could move the "push type" providers out of FHR, FHR would only have to be in memory at shutdown or submission time.

Comment 3

[Gregory Szorc [:gps]]

To answer Yoric's IRC question...

The corresponding code lives in /services/datareporting/sessions.jsm. However, there may be some fundamental flaws with how that code is implemented. See bug 864537 for one. I think we should have someone with core knowledge of the startup/shutdown mechanism - like bsmedberg - review the design before code is written.

Also, mconnor *might* have been hacking on something here. I'm not sure where we are...

Comment 4

[David Teller [:Yoric] - still alive but not very active]

gps, could you provide an idl with the features you need?

Comment 5

[Gregory Szorc [:gps]]

Initial attempt: https://gist.github.com/indygreg/5490541

I don't write IDL, so the types are likely not proper. Please pay more attention to the APIs.

Before anyone starts coding, I'd like to have sign-off from a number of people:

* rnewman - Can Android make use of this feature in any way? What needs to change?
* bsmedberg - On the various types of shutdown that can occur.
* deinspanjer - There was talk of FHR's existing session metrics not being sufficient. If not, we should consider adapting this interface to suit FHR of tomorrow.

Comment 6

[Richard Newman [:rnewman]]

We *could* make some use of this on Android, if:

* We need to report fine-grained in-Gecko times on Android (I contend that we don't).
* It were done and shipped this week or next, or we decided that what we *can* get done this week and next without it is inadequate enough to need replacement.

In order to make use of it we would need a clock linkage -- a timestamp in this interface that could be compared to the realtime clock in Java. I'm not sure what 'main' is -- seems like it would be zero? -- but perhaps that could be defined as "the OS realtime clock value at which startup began"?

If there is no such linkage then there isn't much value here.


On Android I will probably just track app startup and individual points in startup (if we need the additional granularity) vs. Gecko:Ready, all in Java.

(Talos must already do this somehow, rite?)

Gecko:Ready is sent from browser.js, at the very end of BrowserApp.startup(), which I guess is equivalent to firstPaint. Session restore is not a meaningful concept on Android, so we can just timestamp:

- (Activity.onCreate)
- Activity.onStart
- Started launching Gecko
- Gecko:Ready
- Activity.onPause
- (Activity.onShutdown)

Note that Android has broader and narrower definitions of lifecycle; the concept of a "session" (user is doing something) corresponds to onStart/onPause, while Gecko startup/shutdown is kinda unrelated.

If this is fine by Daniel, and y'all agree that we don't care about what's happening inside Gecko, then we have no need of this module on Android.

Comment 7

[Alessio Placitelli [:Dexter]]

FHR is going away per bug 1209088, wontfix.