The default bug view has changed. See this FAQ.

Remove E10S_WINDOW metric

RESOLVED FIXED in Firefox 54

Status

()

Toolkit
Telemetry
P4
normal
RESOLVED FIXED
2 years ago
2 months ago

People

(Reporter: rvitillo, Assigned: fionn_mac, Mentored)

Tracking

unspecified
mozilla54
Points:
1

Firefox Tracking Flags

(e10s+, firefox54 fixed)

Details

(Whiteboard: [measurement:client] [lang=js] [good next bug])

Attachments

(1 attachment)

In about 15% of pings coming from recent Nightly builds the e10sEnabled flag doesn't match the E10S_WINDOW histogram [1].

[1] https://gist.github.com/vitillo/6e7a04f8b92b2bcc2ffd
(Reporter)

Updated

2 years ago
Flags: needinfo?(gfritzsche)
In bug 1179189, we switched over e10sEnabled to using Services.appinfo.browserTabsRemoteAutostart.

E10S_WINDOW submits (AFAICT) whether e10s is enabled per-window:
https://dxr.mozilla.org/mozilla-central/rev/9ed17db42e3e46f1c712e4dffd62d54e915e0fac/browser/base/content/browser.js#1279
https://dxr.mozilla.org/mozilla-central/rev/9ed17db42e3e46f1c712e4dffd62d54e915e0fac/browser/base/content/browser.js#94

If we still have any features to open e10s enabled/disabled windows then that mismatch is expected.

mconley, does that sound right?
Flags: needinfo?(gfritzsche) → needinfo?(mconley)
I think Roberto compared e10sEnabled vs E10S_AUTOSTART instead, no?
(Reporter)

Comment 3

2 years ago
(In reply to Vladan Djeric (:vladan) -- please needinfo! from comment #2)
> I think Roberto compared e10sEnabled vs E10S_AUTOSTART instead, no?

Yeah I did, I actually started with E10S_WINDOW and later switched to E10S_AUTOSTART. Either way though, the conclusion is nearly the same.
(Reporter)

Updated

2 years ago
Summary: e10sEnabled doesn't match E10S_WINDOW → e10sEnabled doesn't match E10S_WINDOW nor E10S_AUTOSTART
Yes, gfritzsche is right - the mismatch between e10sEnabled and E10S_WINDOW is expected. E10S_WINDOW is incremented every time an e10s-enabled window is opened, and it is still very possible to open non-e10s windows.

The mismatch between e10sEnabled and E10S_AUTOSTART is less expected.

It actually looks like they're measuring the same things. I assume that TelemetryEnvironment is only reading this value once - and that's what's occurring with BrowserTabsRemoteAutostart: https://dxr.mozilla.org/mozilla-central/rev/9ed17db42e3e46f1c712e4dffd62d54e915e0fac/toolkit/xre/nsAppRunner.cpp#4611 (notice that it's memoized)

I agree with your results - there is a mismatch. I cannot explain where that mismatch is coming from. :(

Are there pings where e10sEnabled is true, but E10S_AUTOSTART is not? Or the other way around? Or both?
Flags: needinfo?(mconley) → needinfo?(rvitillo)
(Reporter)

Comment 5

2 years ago
(In reply to Mike Conley (:mconley) - Needinfo me! from comment #4)
> I agree with your results - there is a mismatch. I cannot explain where that
> mismatch is coming from. :(
> 
> Are there pings where e10sEnabled is true, but E10S_AUTOSTART is not? Or the
> other way around? Or both?

It appears that the mismatch is caused exclusively by pings for which e10sEnabled is true and E10S_AUTOSTART is false.
Flags: needinfo?(rvitillo)
(In reply to Roberto Agostino Vitillo (:rvitillo) from comment #5)
> (In reply to Mike Conley (:mconley) - Needinfo me! from comment #4)
> > I agree with your results - there is a mismatch. I cannot explain where that
> > mismatch is coming from. :(
> > 
> > Are there pings where e10sEnabled is true, but E10S_AUTOSTART is not? Or the
> > other way around? Or both?
> 
> It appears that the mismatch is caused exclusively by pings for which
> e10sEnabled is true and E10S_AUTOSTART is false.

Interesting... and for those cases, is E10S_AUTOSTART_STATUS set to a consistent value?
Flags: needinfo?(rvitillo)
(Reporter)

Comment 7

2 years ago
E10S_AUTOSTART_STATUS is always empty for those cases.
Flags: needinfo?(rvitillo)
Blocks: 1198187
(In reply to Roberto Agostino Vitillo (:rvitillo) from comment #7)
> E10S_AUTOSTART_STATUS is always empty for those cases.

That's very puzzling. Are there any other patterns you notice from those pings? A shared OS, for example?
(Reporter)

Comment 9

2 years ago
The only pattern I could notice is that the median subsession duration is about 5 times longer compared to the general population, but I am afraid it isn't very useful in tracking down the cause.
tracking-e10s: --- → +
Blocks: 1222849
Is this something we still need to fix or do we have this data in other places?
Flags: needinfo?(rvitillo)
Afaik this has not been fixed.
Flags: needinfo?(rvitillo)
E10S_AUTOSTART was removed in bug 1111701. E10S_WINDOW has never been in use. Lets convert this into a bug about removing the window probe.

http://mxr.mozilla.org/mozilla-central/source/toolkit/components/telemetry/Histograms.json#8512
Priority: -- → P4
Summary: e10sEnabled doesn't match E10S_WINDOW nor E10S_AUTOSTART → Remove E10S_WINDOW metric
This bug is about removing the histogram "E10S_WINDOW" and related code:
https://dxr.mozilla.org/mozilla-central/search?q=E10S_WINDOW&case=true
Mentor: gfritzsche@mozilla.com
Whiteboard: [measurement:client] [lang=js]
I think no one is looking at this probe. This measurement would be interesting to look at for users who don't have it to 100% in one or the other bucket (i.e., are there things causing non-e10s windows to be opened by accident). But I don't know if that's a real concern as I've never seen a bug reported on that.
No longer blocks: 1198187
Whiteboard: [measurement:client] [lang=js] → [measurement:client] [lang=js] [good next bug]
Vedant, would you be interested in taking this one?
Flags: needinfo?(vedant.sareen)
(Assignee)

Comment 16

2 months ago
(In reply to Alessio Placitelli [:Dexter] from comment #15)
> Vedant, would you be interested in taking this one?

Sure Sir. I'll start work on this ASAP.
Flags: needinfo?(vedant.sareen)
Assignee: nobody → vedant.sareen
(Assignee)

Comment 17

2 months ago
Created attachment 8828905 [details] [diff] [review]
Proposed patch for bug 1206048.

I also ran |./mach test toolkit/components/telemetry/|. All tests were successful.
Attachment #8828905 - Flags: review?(alessio.placitelli)
Comment on attachment 8828905 [details] [diff] [review]
Proposed patch for bug 1206048.

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

Thanks for the patch, this looks good Vedant, good job!

Let me push this to try for you before checking it in.
Attachment #8828905 - Flags: review?(alessio.placitelli) → review+
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f247de5e32d6
Looks good in try, let's land this!
Status: NEW → ASSIGNED
Points: --- → 1
Keywords: checkin-needed

Comment 21

2 months ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3aa2f67fa2bc
Remove E10S_WINDOW metric. r=Dexter
Keywords: checkin-needed

Comment 22

2 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/3aa2f67fa2bc
Status: ASSIGNED → RESOLVED
Last Resolved: 2 months ago
status-firefox54: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
(Assignee)

Comment 23

2 months ago
(In reply to Alessio Placitelli [:Dexter] from comment #18)
> -----------------------------------------------------------------
>
> Thanks for the patch, this looks good Vedant, good job!
> 
> Let me push this to try for you before checking it in.

Thank you! :)
You need to log in before you can comment on or make changes to this bug.