Last Comment Bug 1241336 - Run an e10s A/B experiment on Beta 45 (first experiment)
: Run an e10s A/B experiment on Beta 45 (first experiment)
Status: RESOLVED FIXED
:
Product: Firefox
Classification: Client Software
Component: General (show other bugs)
: 45 Branch
: Unspecified Unspecified
-- normal (vote)
: ---
Assigned To: :Felipe Gomes (needinfo me!)
:
:
Mentors:
: 1225496 (view as bug list)
Depends on: 1224374 1228437 1241296 1241362 1241507 1243612
Blocks: e10s-beta e10s-measurement 1243867 1244187
  Show dependency treegraph
 
Reported: 2016-01-20 16:25 PST by Vladan Djeric (:vladan)
Modified: 2016-01-29 09:32 PST (History)
13 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
m8+


Attachments
Experiment code, mostly unchanged.patch (9.19 KB, patch)
2016-01-20 17:53 PST, Vladan Djeric (:vladan)
felipc: feedback+
Details | Diff | Splinter Review
in progress experiment.xpi (no addons blacklist) (1.76 KB, application/x-xpinstall)
2016-01-21 08:09 PST, :Felipe Gomes (needinfo me!)
no flags Details
signed experiment.xpi (5.64 KB, application/x-xpinstall)
2016-01-21 12:23 PST, Jason Thomas [:jason]
no flags Details
Filter old versions of lastpass (927 bytes, patch)
2016-01-25 14:42 PST, :Felipe Gomes (needinfo me!)
wmccloskey: review+
Details | Diff | Splinter Review
Block users of certain locales (777 bytes, patch)
2016-01-28 12:15 PST, :Felipe Gomes (needinfo me!)
no flags Details | Diff | Splinter Review

Description User image Vladan Djeric (:vladan) 2016-01-20 16:25:59 PST
We want to run an A/B experiment on Beta 45 on 50% of all profiles (with and without extensions).
Comment 1 User image Vladan Djeric (:vladan) 2016-01-20 17:53:57 PST
Created attachment 8710228 [details] [diff] [review]
Experiment code, mostly unchanged.patch

Start time: beginning of Monday, Jan 25th GMT
End time: beginning of Thursday, Feb 11th GMT
Maximum duration: 15 days
Sampling: 50% of Beta population (25% + 25%)
Filtered only on profiles that have extended Telemetry enabled (default on Beta)
Comment 2 User image Vladan Djeric (:vladan) 2016-01-20 17:58:14 PST
Is there some way to determine that e10s is blocked because of a11y now? Maybe experiment state "force-disabled" combined with the the values in Telemetry's A11Y_ histograms?
Comment 3 User image Vladan Djeric (:vladan) 2016-01-20 18:09:48 PST
Note that this experiment doesn't have the extension blacklist yet. I will add it tomorrow.
Comment 4 User image :Felipe Gomes (needinfo me!) 2016-01-21 05:25:41 PST
(In reply to Vladan Djeric (:vladan) -- please needinfo from comment #2)
> Is there some way to determine that e10s is blocked because of a11y now?
> Maybe experiment state "force-disabled" combined with the the values in
> Telemetry's A11Y_ histograms?

The E10S_STATUS histogram will have a value of 4. Possible values are here: http://mxr.mozilla.org/mozilla-central/source/toolkit/xre/nsAppRunner.cpp?rev=1626afe51b11#4646
(soon to be added 6 = disabled for certain locales, 7 = disabled for addons)
Comment 5 User image :Felipe Gomes (needinfo me!) 2016-01-21 07:36:21 PST
*** Bug 1225496 has been marked as a duplicate of this bug. ***
Comment 6 User image :Felipe Gomes (needinfo me!) 2016-01-21 07:44:40 PST
Comment on attachment 8710228 [details] [diff] [review]
Experiment code, mostly unchanged.patch

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

I'll give r+ after the addon whitelist is added, but the code here looks good.

::: experiments/e10s-enabled-beta45.1/code/bootstrap.js
@@ +32,5 @@
> +      case null:
> +        // Profile is ineligible if either of the two e10s prefs is non-default
> +        if (Preferences.isSet(OPTIN_PREF) || Preferences.isSet(PREF)) {
> +          TelemetryLog.log(SELF_ID, ["Setting branch to 'user-disabled' because e10s prefs have non-default values."]);
> +          yield Experiments.instance().setExperimentBranch(SELF_ID, "user-disabled");

if optin_pref = true, e10s will be enabled, so user-disabled is a bit of a misnomer here.. i.e., they will be running e10s, but just not due to the experiment.  Not sure how to better tag that, that's up to how the data analysis will be done afterwards.

Although that is a small concern since the optin_pref was never exposed to beta, so only a few users messing with about:config or reusing profiles will fall into this.

::: experiments/e10s-enabled-beta45.1/code/install.rdf
@@ +1,4 @@
> +<?xml version="1.0" encoding="utf-8"?>
> +<RDF xmlns="http://www.w3.org/1999/02/22-rdf-syntax-ns#" xmlns:em="http://www.mozilla.org/2004/em-rdf#">
> +  <Description about="urn:mozilla:install-manifest">
> +    <em:id>e10s-enabled-beta45.1-20160125@experiments.mozilla.org</em:id>

very nitpicky, but can you change the name to "e10s-beta45-withaddons", just so that this is more descriptive and easy to differentiate with a quick glance?  The ID in the manifest and bootstrap.js also needs to be changed.

::: experiments/e10s-enabled-beta45.1/manifest.json
@@ +1,3 @@
> +{
> +  "publish"     : true,
> +  "priority"    : 2,

I talked with bsmedberg about this. Since there's another experiment running at the same time (the unified urlbar), and that experiment has a smaller sample size than this one, this one needs to have a lower priority. That one is set to 2, so please set the priority here to 1.

@@ +11,5 @@
> +    "maxActiveSeconds" : 1296000,
> +    "appName"          : ["Firefox"],
> +    "channel"          : ["beta"],
> +    "minVersion"       : "44.0",
> +    "maxVersion"       : "44.0",

need to update min/max Version to 45.
Comment 7 User image :Felipe Gomes (needinfo me!) 2016-01-21 08:09:21 PST
Created attachment 8710492 [details]
in progress experiment.xpi (no addons blacklist)

Hello Jason, could you sign this xpi for us?

FWIW, there will be another signing needed soon, but we want to start testing this asap so this version is good enough for this. Hope that is ok!
Comment 8 User image Jason Thomas [:jason] 2016-01-21 12:23:34 PST
Created attachment 8710641 [details]
signed experiment.xpi

signed experiment.xpi
Comment 9 User image :Felipe Gomes (needinfo me!) 2016-01-21 12:36:23 PST
So I made the changes I mentioned here and published an experiment server at https://people.mozilla.org/~fgomes/e10s-beta45-withaddons/ .

With the standard experiment instructions, one can set experiments.manifest.uri to:
  
  https://people.mozilla.org/~fgomes/e10s-beta45-withaddons/firefox-manifest.json

To start testing it. The difference from what needs to land is that I set the sample size to 100% and I removed the other running experiment, in order to make it easier to test this one.
Comment 10 User image :Felipe Gomes (needinfo me!) 2016-01-21 12:44:23 PST
It's still an open question the about whether we'll include an add-on blocklist in this experiment or not, so I also landed what we already have: (which is everything except the blocklist)

https://hg.mozilla.org/webtools/telemetry-experiment-server/rev/b5ddc65fd536
Comment 11 User image :Felipe Gomes (needinfo me!) 2016-01-25 14:42:51 PST
Created attachment 8711893 [details] [diff] [review]
Filter old versions of lastpass

Filters out users of old versions of LastPass from being part of the experiment
Comment 12 User image Bill McCloskey (:billm) 2016-01-25 16:13:26 PST
Comment on attachment 8711893 [details] [diff] [review]
Filter old versions of lastpass

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

::: experiments/e10s-beta45-withaddons/filter.js
@@ +1,5 @@
>  function filter(c) {
> +  let activeAddons = {};
> +  try {
> +    activeAddons = c.telemetryEnvironment.addons.activeAddons;
> +  } catch (e) {}

How about we return false if we get an exception here?
Comment 13 User image :Felipe Gomes (needinfo me!) 2016-01-28 10:34:55 PST
Pushed the filtering of LastPass 3.* to the manifest

https://hg.mozilla.org/webtools/telemetry-experiment-server/rev/73b05e4d57c6

Once QA signs off I'll push the release_tag on this experiment and close this bug
Comment 14 User image :Felipe Gomes (needinfo me!) 2016-01-28 12:15:30 PST
Created attachment 8713316 [details] [diff] [review]
Block users of certain locales

The in-tree code that does the locale blocking blocks based on the OS's locale, rather than the actual Firefox locale. Not a huge deal (would miss users of these locales if they're running the OS in en-US for example), but also not ideal .. So let's block here in the manifest and let the code be fixed for beta2.
Comment 15 User image :Felipe Gomes (needinfo me!) 2016-01-28 13:08:38 PST
Comment on attachment 8713316 [details] [diff] [review]
Block users of certain locales

Actually, forget that. The bidi issue is minor and affects very few people. It will be better to have the in-tree code running to test it instead of filtering out these users
Comment 16 User image :Felipe Gomes (needinfo me!) 2016-01-28 18:08:21 PST
QA has signed off on the experiment, so I updated release_tag to point to it:

https://hg.mozilla.org/webtools/telemetry-experiment-server/rev/31f336659583

Final deployment to production will occur in bug 1243867

Note You need to log in before you can comment on or make changes to this bug.