1210634 - Inaccurate Telemetry for Session Store time

Status: RESOLVED WORKSFORME

(Firefox :: Session Restore, defect, P1)

Description

[Vladan Djeric (:vladan)]

I don't think the simpleMeasurements.sessionRestoreInit and simpleMeasurements.sessionRestored timestamps are accurate.

Anecdote:

It takes a long while for session restore to complete in my e10s nightly on Windows. The session consists of about ~5 windows and 10 tabs in e10s. The tab spinners spin for a while and the tab content isn't ready, but I notice in about:telemetry that the sessionRestored timestamp gets set well before the session is truly restored.

We should:

a) Figure out exactly what sessionRestored means
b) Add a new, more meaningful measurement of session restore time

This will also help with e10s perf comparisons.

Comment 1

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

1) sessionRestored is updated when we receive notification of sessionstore-windows-restored. In turn, this happens once Session Restore has determined that all windows (*not tabs*) have been restored. Roughly speaking, that's when the main process has done all it can do without having to wait for the content process or the in-process framescripts. The story is of course a bit more complicated, because there are at least 5 different paths that can lead to that notification, but it's the general idea.

From the point of view of the user, sessionRestore fires more or less when session restoration *starts*.

2) Sounds like bug 1214158.

Comment 2

[Georg Fritzsche [:gfritzsche]]

Does this measurement will be far off for e10s?
Should this be tracked for e10s?

Comment 3

[Vladan Djeric (:vladan)]

(In reply to Georg Fritzsche [:gfritzsche] from comment #2)
> Does this measurement will be far off for e10s?

If I understand correctly, this measurement should behave the same in e10s & non-e10s. The actual session restore timing is measured in bug 1214158, and should also be the same in e10s & non-e10s.

Yoric: can you confirm?

> Should this be tracked for e10s?

Yes, this measurement should be tracked for e10s.

Yoric, question #2: Should we rename this measurement to something else?

Comment 4

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

(In reply to Vladan Djeric (:vladan) -- please needinfo from comment #3)
> (In reply to Georg Fritzsche [:gfritzsche] from comment #2)
> > Does this measurement will be far off for e10s?
> 
> If I understand correctly, this measurement should behave the same in e10s &
> non-e10s. The actual session restore timing is measured in bug 1214158, and
> should also be the same in e10s & non-e10s.
> 
> Yoric: can you confirm?

Actually, I suspect that `sessionRestore` will be a little bit slower on e10s, because creating a remote <xul:browser> involves some cross-process messaging. Similarly, actual session restore will probably be a bit slower on e10s, because we need to not only create a <xul:browser> but also to wait for more messages that tell us it has been restored.

> > Should this be tracked for e10s?
> 
> Yes, this measurement should be tracked for e10s.
> 
> Yoric, question #2: Should we rename this measurement to something else?

Probably, but I can't think of a good name. Maybe `sessionRestoreEmptyWindowsReady`?

Comment 5

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

There is nothing left to do in this bug, except perhaps changing a name if we can find a good one.
I'd vote for FIXED => WFM, then open a new bug "Find a better name for `sessionRestored`"

Comment 6

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

Filed as bug 1245867.