1319206 - Run a Telemetry experiment to vet GPU Process on Windows

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[u279076]

Similar to bug 12996505 (SKIA A/B test on Windows) we want to A/B test GPU Process in Nightly 53 on Windows in support of bug 1264543. The basic idea is that Nightly users who qualify will be put on GPU Process by default; we want to switch half of these users off GPU Process to compare the stability between these two different groups.

Proposed Criteria:
>      Product: Firefox Nightly
>      Version: 53.0a1
>           OS: Windows (all)
>      Channel: nightly
>     Build ID: 2016-11-15+
>     Language: en-US
> Sample ratio: 50%
>   Start Date: November 28, 2016
>     End Date: December 12, 2016

Assigning to myself to develop the experiment. More details to follow.

Comment 1

[u279076]

(In reply to Anthony Hughes (:ashughes) [GFX][QA][Mentor] from comment #0)
> Similar to bug 12996505 (SKIA A/B test on Windows)

Sorry this should be bug 1299605.

Comment 2

[u279076]

Attachment: gpu-process-nightly53.patch

This patch creates a Telemetry experiment to A/B test GPU process stability by disabling GPU process for 50% of those on Nightly who have it enabled by default. A few notes about this patch:

1) It includes the change in bug 1323304 as I need that for this to pass testing. I'm not sure how you want to handle landing both of these changes, whether that change should get landed in it's bug and removed from my patch here or whether to include both changes in *this* bug.

2) I encountered an issue in testing where the code within the (reason != APP_SHUTDOWN) conditional was running on shutdown. I'm not sure if this is a bug but I managed to work around it with the new conditional you see in the patch.

3) I still need to get the .xpi signed and will do so once this patch is reviewed, before this patch gets pushed to staging.

4) The start/end times in the manifest will need to be updated to more accurately reflect the date this goes live +10 days. The current numbers were used for testing purposes.

[Note to self: As a follow-up task I need to create a Telemetry dashboard to track stability numbers for this experiment]

Comment 3

[:Felipe Gomes (needinfo for replies!)]

(In reply to Anthony Hughes (:ashughes) [GFX][QA][Mentor] from comment #2)
> Created attachment 8819458 [details] [diff] [review]
> gpu-process-nightly53.patch
> 
> This patch creates a Telemetry experiment to A/B test GPU process stability
> by disabling GPU process for 50% of those on Nightly who have it enabled by
> default. A few notes about this patch:
> 
> 1) It includes the change in bug 1323304 as I need that for this to pass
> testing. I'm not sure how you want to handle landing both of these changes,
> whether that change should get landed in it's bug and removed from my patch
> here or whether to include both changes in *this* bug.

Yeah, please remove it from this patch and we'll land it as its own bug (1323304).

> 
> 2) I encountered an issue in testing where the code within the (reason !=
> APP_SHUTDOWN) conditional was running on shutdown. I'm not sure if this is a
> bug but I managed to work around it with the new conditional you see in the
> patch.

Oh wow, interesting.. never noticed this before, but now I'm wondering if it has always happened. Let's file a new bug to track that

Comment 4

[u279076]

(In reply to :Felipe Gomes (needinfo me!) from comment #3)
> > 1) It includes the change in bug 1323304 as I need that for this to pass
> > testing. I'm not sure how you want to handle landing both of these changes,
> > whether that change should get landed in it's bug and removed from my patch
> > here or whether to include both changes in *this* bug.
> 
> Yeah, please remove it from this patch and we'll land it as its own bug
> (1323304).

Thanks. I will attach a new patch to this bug that removes that code and updates the timestamps that we want for production.

> > 2) I encountered an issue in testing where the code within the (reason !=
> > APP_SHUTDOWN) conditional was running on shutdown. I'm not sure if this is a
> > bug but I managed to work around it with the new conditional you see in the
> > patch.
> 
> Oh wow, interesting.. never noticed this before, but now I'm wondering if it
> has always happened. Let's file a new bug to track that.

I filed bug 1324562.

Comment 5

[u279076]

Attachment: gpu-process-nightly53.patch

Here is a new version of my patch with the following changes:
* removed code to fix bug 1323304
* changes start timestamp to Wednesday, December 21, 2016 at 12:00am
* changed end timestamp to Wednesday, January 4, 2017 at 11:59pm
* changed experiment duration to 2-weeks to accommodate lower holiday usage

Felipe, I still need to get the XPI signed per https://wiki.mozilla.org/QA/Telemetry/Developing_a_Telemetry_Experiment#Signing_the_XPI but I no longer see an Addon Validation component. Please advise.

Comment 6

[u279076]

Comment on attachment 8820040 [details] [diff] [review]
gpu-process-nightly53.patch

Sorry for the bug spam -- not sure if this needs review since the other patch was r+'d but I'm doing it anyway just in case.

Comment 7

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 8820040 [details] [diff] [review]
gpu-process-nightly53.patch

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

I usually just attach the XPI to the bug and ask :jason to sign it..

Comment 8

[u279076]

Attachment: experiment.xpi

Jason, can you please sign this xpi?

Comment 9

[Jason Thomas [:jason]]

Attachment: experiment.xpi signed

Please see attached.

Comment 10

[u279076]

Thank you Jason.

Felipe, can you please push to staging for me? Once that's done I'll send out the Intent-to-Ship email.

Comment 11

[:Felipe Gomes (needinfo for replies!)]

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

I'll let you verify the push and close the bug.

FTR, the experiment will appear in the staging server in a couple of minutes here: https://telemetry-experiment-dev.allizom.org/

Comment 12

[u279076]

Thank you Felipe. I have now sent the email to request approval to ship.

Comment 13

[u279076]

Felipe, this has approval from Release Management. Can you please push it to production? I don't have commit access.

Comment 14

[u279076]

(In reply to Anthony Hughes (:ashughes) [GFX][QA][Mentor] from comment #13)
> Felipe, this has approval from Release Management. Can you please push it to
> production? I don't have commit access.

I was unable to find anyone to push this today so it looks like this will not go to production until next Wednesday December 28 at the earliest. Given this I think we need to extend endTime in manifest.json to 1484179199 (Jan 11, 2017 instead of Jan 4, 2017). Is this possible without submitting a new patch/xpi?

Comment 15

[:Felipe Gomes (needinfo for replies!)]

Hey Anthony, I can't push this to production. You need to file a bug like bug 1305586.

I updated the endTime and also the release_tag and pushed it to the repo. So now all that's left is to file that bug to deploy.

Comment 16

[u279076]

(In reply to :Felipe Gomes (needinfo me!) from comment #15)
> Hey Anthony, I can't push this to production. You need to file a bug like
> bug 1305586.
> 
> I updated the endTime and also the release_tag and pushed it to the repo. So
> now all that's left is to file that bug to deploy.

Thanks, I filed bug 1325786 to deploy.

Comment 17

[Liz Henry (:lizzard) (relman/hg->git project)]

Looks like this is deployed since Dec. 25.