e10s beta cohort distribution improvements

RESOLVED FIXED in Firefox 54

Status

()

Firefox
General
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: jimm, Assigned: mrbkap)

Tracking

Trunk
Firefox 55
Points:
---

Firefox Tracking Flags

(firefox54 fixed, firefox55 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

a year ago
Doing some analysis over on re:dash, a few issues come to mind:

1) are the old multibucket1, 2, 4, 8 users getting converted over to the new webextension-multibuket1, 4 buckets? 

If not, we should probably do this to clean up the data and add more users to the buckets we're interested in.

2) We seem to have very low webextension-multibuket1, 4 client counts. This might be a side effect of #1.

3) The old multibuckets appear to have a number of non-e10s users in them. Can we figure out what's going on there and clean it up?

beta e10s cohort breakdown: 
https://sql.telemetry.mozilla.org/queries/4459/source#8959

beta non-e10s cohort breakdown:
https://sql.telemetry.mozilla.org/queries/4457/source#8955
Flags: needinfo?(felipc)
(In reply to Jim Mathies [:jimm] from comment #0)
> 1) are the old multibucket1, 2, 4, 8 users getting converted over to the new
> webextension-multibuket1, 4 buckets? 

Yes. We recompute the cohort name on every launch.

> 2) We seem to have very low webextension-multibuket1, 4 client counts. This
> might be a side effect of #1.

I guess? It seems more likely to me that webextension-based addons are just pretty rare.

> 3) The old multibuckets appear to have a number of non-e10s users in them.
> Can we figure out what's going on there and clean it up?

That could be because of a race on first startup where we don't detect that e10s is disabled due to a11y (overholt ran into this), but I'd expect that to fix itself on the second run (and then we'd end up with a cohort of either disqualified-control or disqualified-test).
Comment hidden (mozreview-request)
So let me add a bit of historical context here:

There has always existed a race between these prefs being set, and/or any code reading mozilla::BrowserTabsRemoteAutostart/Services.appinfo.browserTabsRemoteAutostart, including any add-on code  that can trigger it.

It used to be that at the time of our past experiments I tested this, and on a fresh profile this did not happen, and the e10s prefs were all set before the backend needed to check for e10s. (It was certainly the case that some profiles with add-ons could change the order of things, but it was not widespread).
Now, I wouldn't doubt that any code that landed in the last ~6months could have changed the order of things, guaranteeing that the e10srollout addon will lose the race.

This doesn't cause any issues, except that a user who gets tagged for multi-process might need to wait a restart for that to take effect. And that we'll see this in the data.


In talking with Blake on IRC, we decided to change the "multiBucket-N" cohort names to something entirely new, so that all data that we look is fresh. Let's see how that goes and when we look at better data, we can revisit this to see if there are any other problems.
Flags: needinfo?(felipc)

Comment 4

a year ago
mozreview-review
Comment on attachment 8865570 [details]
Bug 1362493 - Let users with mpc=true addons into the multi experiment.

https://reviewboard.mozilla.org/r/137196/#review140292

::: browser/extensions/e10srollout/bootstrap.js:198
(Diff revision 1)
>    let buckets = MULTI_BUCKETS[updateChannel];
>  
>    let multiUserSample = getUserSample(true);
>    for (let sampleName of Object.getOwnPropertyNames(buckets)) {
>      if (multiUserSample < buckets[sampleName]) {
>        setCohort(`${cohortPrefix}multiBucket${sampleName}`);

Let's change this to something else to clear the data
Attachment #8865570 - Flags: review?(felipc) → review+
(In reply to :Felipe Gomes (needinfo me!) from comment #4)
> Let's change this to something else to clear the data

I spoke to felipe on IRC and we agreed to keep the current names.
Comment on attachment 8865570 [details]
Bug 1362493 - Let users with mpc=true addons into the multi experiment.

Approval Request Comment
[Feature/Bug causing the regression]: e10s-multi experiment
[User impact if declined]: n/a
[Is this code covered by automated tests?]: n/a
[Has the fix been verified in Nightly?]: n/a
[Needs manual test from QE? If yes, steps to reproduce]: n/a
[List of other uplifts needed for the feature/fix]: n/a
[Is the change risky?]: no, it's well understood
[Why is the change risky/not risky?]: n/a
[String changes made/needed]: n/a
Attachment #8865570 - Flags: approval-mozilla-beta?

Comment 7

a year ago
Pushed by mrbkap@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4dff49910a61
Let users with mpc=true addons into the multi experiment. r=Felipe
https://hg.mozilla.org/mozilla-central/rev/4dff49910a61
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
status-firefox54: --- → affected
Hi Gerry, we are hoping to get this into Monday's GTB for Beta 8. Please NI Blake or Felipe with any questions. Thank you for the support!
Flags: needinfo?(gchang)
Comment on attachment 8865570 [details]
Bug 1362493 - Let users with mpc=true addons into the multi experiment.

Support for e10s-multi experiment. Beta54+. Should be in 54 beta 8.
Flags: needinfo?(gchang)
Attachment #8865570 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Assignee: nobody → mrbkap

Comment 11

a year ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/8be7562b858f
status-firefox54: affected → fixed
You need to log in before you can comment on or make changes to this bug.