Inaccurate Telemetry for Session Store time

RESOLVED WORKSFORME

Status

()

Firefox
Session Restore
P1
normal
RESOLVED WORKSFORME
2 years ago
2 years ago

People

(Reporter: vladan, Unassigned)

Tracking

(Blocks: 1 bug)

38 Branch
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(e10s+)

Details

(Reporter)

Description

2 years ago
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.
(Reporter)

Updated

2 years ago
Blocks: 1222849
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.
Depends on: 1214158
Does this measurement will be far off for e10s?
Should this be tracked for e10s?
tracking-e10s: --- → ?
Flags: needinfo?(vladan.bugzilla)
(Reporter)

Comment 3

2 years ago
(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?
Flags: needinfo?(vladan.bugzilla) → needinfo?(dteller)
(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`?
Flags: needinfo?(dteller)
tracking-e10s: ? → m9+
Priority: -- → P3
tracking-e10s: m9+ → +
Priority: P3 → P1
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`"
Filed as bug 1245867.
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.