Last Comment Bug 1319206 - Run a Telemetry experiment to vet GPU Process on Windows
: Run a Telemetry experiment to vet GPU Process on Windows
Status: RESOLVED FIXED
[gfx-noted]
:
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: 53 Branch
: All Windows
-- normal (vote)
: ---
Assigned To: Anthony Hughes (:ashughes) [GFX][QA][Mentor]
:
: Milan Sreckovic [:milan] (PTO through 8/30)
Mentors:
Depends on: 1323304 1325786
Blocks: e10s-gpu
  Show dependency treegraph
 
Reported: 2016-11-21 13:56 PST by Anthony Hughes (:ashughes) [GFX][QA][Mentor]
Modified: 2017-03-01 13:27 PST (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed


Attachments
gpu-process-nightly53.patch (8.65 KB, patch)
2016-12-16 15:36 PST, Anthony Hughes (:ashughes) [GFX][QA][Mentor]
felipc: review+
dvander: review+
Details | Diff | Splinter Review
gpu-process-nightly53.patch (8.12 KB, patch)
2016-12-19 13:42 PST, Anthony Hughes (:ashughes) [GFX][QA][Mentor]
felipc: review+
Details | Diff | Splinter Review
experiment.xpi (1.40 KB, application/x-xpinstall)
2016-12-20 09:59 PST, Anthony Hughes (:ashughes) [GFX][QA][Mentor]
no flags Details
experiment.xpi signed (5.29 KB, application/x-xpinstall)
2016-12-21 07:47 PST, Jason Thomas [:jason]
no flags Details

Description User image Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2016-11-21 13:56:01 PST
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 User image Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2016-11-21 13:57:20 PST
(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 User image Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2016-12-16 15:36:23 PST
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.

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 User image :Felipe Gomes (needinfo me!) 2016-12-19 08:38:27 PST
(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 User image Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2016-12-19 13:26:50 PST
(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 User image Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2016-12-19 13:42:16 PST
Created attachment 8820040 [details] [diff] [review]
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 User image Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2016-12-19 13:45:34 PST
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 User image :Felipe Gomes (needinfo me!) 2016-12-19 15:53:07 PST
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 User image Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2016-12-20 09:59:15 PST
Created attachment 8820351 [details]
experiment.xpi

Jason, can you please sign this xpi?
Comment 9 User image Jason Thomas [:jason] 2016-12-21 07:47:49 PST
Created attachment 8820752 [details]
experiment.xpi signed

Please see attached.
Comment 10 User image Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2016-12-21 10:24:22 PST
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 User image :Felipe Gomes (needinfo me!) 2016-12-21 11:46:09 PST
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 User image Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2016-12-21 13:10:45 PST
Thank you Felipe. I have now sent the email to request approval to ship.
Comment 13 User image Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2016-12-23 09:49:43 PST
Felipe, this has approval from Release Management. Can you please push it to production? I don't have commit access.
Comment 14 User image Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2016-12-23 20:10:43 PST
(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 User image :Felipe Gomes (needinfo me!) 2016-12-24 09:21:58 PST
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 User image Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2016-12-24 11:50:38 PST
(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 User image Liz Henry (:lizzard) (needinfo? me) 2017-01-03 08:40:40 PST
Looks like this is deployed since Dec. 25.

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