Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mitigate randomized Experiment selection, feature flags for performance analysis #45

Closed
mcomella opened this issue Dec 20, 2019 · 15 comments

Comments

@mcomella
Copy link
Contributor

mcomella commented Dec 20, 2019

Why/User Benefit/User Problem

In our analysis for debug vs. release builds mozilla-mobile/fenix#6931, we decided to use a release-like build. However, in release-like builds, Experimentation is enabled and users may randomly opt into various experiments: this bug is to understand the implications and mitigate them.

Furthermore, some features are behind "feature flags" so they're enabled in Nightly and debug builds only: we should figure out how to address these too (should this be a separate bug?).

Impact

Without this, performance results will not be reliable because random experiment opt-in may impact our ability to accurately measure performance.

Acceptance Criteria (how do I know when I’m done?)

  • Mitigate random experimentation opt-in in performance testing
    • Note: I've heard "mako is the future" and that fretboard is deprecated for experiments: please verify the behavior here
  • (separate issue?) Mitigate feature flags (e.g. nightly or debug) in performance testing
@mcomella mcomella added this to Needs prioritization in Performance, front-end roadmap via automation Dec 20, 2019
@mcomella mcomella moved this from Needs prioritization to Backlog (prioritized) in Performance, front-end roadmap Dec 30, 2019
@mcomella
Copy link
Contributor Author

Triage: we have no known experiments in Fenix GA so low priority. In fact, Colin is looking at taking experiments out of GA to improve startup perf.

@mcomella mcomella moved this from Backlog (prioritized) to Low impact (unprioritized) in Performance, front-end roadmap Feb 17, 2020
@mcomella mcomella moved this from Low impact (unprioritized) to Needs prioritization in Performance, front-end roadmap May 14, 2020
@mcomella
Copy link
Contributor Author

Maybe mozilla-mobile/fenix#6278 will help here.

@mcomella
Copy link
Contributor Author

mcomella commented May 21, 2020

Triage: the P1 ask is to opt-out of all experiments, especially on CI. We can file a follow-up to opt-in to experiments.

This hinges on when experiments will be reintroduced so I'll contact the folks involved.

@mcomella mcomella moved this from Needs prioritization to Needs prioritization: waiting in Performance, front-end roadmap Jun 10, 2020
@mcomella mcomella moved this from Needs prioritization: waiting to Needs prioritization in Performance, front-end roadmap Jul 8, 2020
@mcomella
Copy link
Contributor Author

mcomella commented Jul 8, 2020

triage: we want to wait for eric's opinion but we suspect this could be a high priority, just below startup work

@MarcLeclair MarcLeclair moved this from Needs prioritization to Backlog (prioritized) in Performance, front-end roadmap Jul 15, 2020
@MarcLeclair MarcLeclair moved this from Backlog (prioritized) to Waiting in Performance, front-end roadmap Jul 15, 2020
@MarcLeclair MarcLeclair moved this from Waiting to Needs prioritization: waiting in Performance, front-end roadmap Jul 15, 2020
@mcomella mcomella moved this from Needs prioritization: waiting to Needs prioritization in Performance, front-end roadmap Dec 7, 2020
@mcomella
Copy link
Contributor Author

mcomella commented Dec 7, 2020

We have experiment selection code on start up now: mozilla-mobile/fenix#16901 Moving to triage.

@mcomella
Copy link
Contributor Author

Triage: the secure storage experiment is landing and csadilek does not expect it to have a perf impact for start up or page load (where our tests are). We could double-check the PR though mozilla-mobile/fenix#18333

@mcomella
Copy link
Contributor Author

Triage: the secure storage experiment is landing and csadilek does not expect it to have a perf impact for start up or page load (where our tests are). We could double-check the PR though mozilla-mobile/fenix#18333

This shouldn't affect the measurements we take for start up or page load, afaict.

@eliserichards
Copy link

eliserichards commented Mar 26, 2021

Those three experiments (1 Leanplum and 2 Nimbus) are going to be implemented after the MR1 release (mid-April). They will be running simultaneously.

@mcomella
Copy link
Contributor Author

The top two issues don't seem like they'll affect start up. The last one mozilla-mobile/fenix#18375 might for FNPRMS where the message will pop up presumably on the homescreen after the third run (since we don't have conditioned profiles).

csadilek also mentions there's a secure storage experiment but it probably doesn't impact start up either.

csadilek also mentions that we have a menu in the secret settings that displays all the known experiments – this could be useful for figuring out when it's useful to work on this issue.

@mcomella
Copy link
Contributor Author

mcomella commented Jun 3, 2021

Triage: no new experiments off the top of our heads.

@MarcLeclair MarcLeclair moved this from Needs prioritization to Low impact (unprioritized) in Performance, front-end roadmap Jun 9, 2021
@mcomella mcomella moved this from Triaged (unordered) to Needs triage in Performance, front-end roadmap Jan 26, 2022
@mcomella
Copy link
Contributor Author

Triage: even if this doesn't affect perf tests, this affects experiments.

@mcomella
Copy link
Contributor Author

mcomella commented Feb 2, 2022

Triage: desktop runs into the same problem, let's ask Bas for their opinion. From our discussion, csadilek wants to test what we're shipping in Nightly – i.e. no preferences or overrides. mcomella wants to pin to one set of experiments for each test so our tests are more controlled.

@mcomella
Copy link
Contributor Author

mcomella commented Feb 17, 2022

Triage: if Bas remembers correctly, we opt out of all experiments for perf tests (though we should verify this with sparky edit: Sparky thinks this is set here for normandy). To ensure we don't regress performance for code behind experiment flags, 1) we expect developers to regularly run their patches against perf tests on try, 2) when we run the experiment in production, we can compare perf telemetry against the baseline (example), and 3) when the experiments finally get released, we test them against CI and users.

This ensures experiments are tested in isolation but does not test them in combination (particularly because cohorts allocated for experiments cannot be put into other experiments) so it's possible, but unlikely, that if two experiments are tested in isolation and don't cause perf issues, those experiments may regress when turned on simultaneously and we haven't tested that.

To get ourselves closer to desktop, we have some follow-ups to do:

@MarcLeclair
Copy link

Will close since this has become a meta issue.

Performance, front-end roadmap automation moved this from Needs triage to Done Feb 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants