Closed
Bug 1319206
Opened 8 years ago
Closed 8 years ago
Run a Telemetry experiment to vet GPU Process on Windows
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
firefox53 | --- | fixed |
People
(Reporter: u279076, Assigned: u279076)
References
(Blocks 1 open bug)
Details
(Whiteboard: [gfx-noted])
Attachments
(3 files, 1 obsolete file)
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.
(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.
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]
Attachment #8819458 -
Flags: review?(felipc)
Attachment #8819458 -
Flags: review?(dvander)
Attachment #8819458 -
Flags: review?(dvander) → review+
Updated•8 years ago
|
Attachment #8819458 -
Flags: review?(felipc) → review+
Comment 3•8 years ago
|
||
(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
(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.
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.
Flags: needinfo?(felipc)
Attachment #8819458 -
Attachment is obsolete: true
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.
Attachment #8820040 -
Flags: review?(felipc)
Comment 7•8 years ago
|
||
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..
Attachment #8820040 -
Flags: review?(felipc) → review+
Updated•8 years ago
|
Flags: needinfo?(felipc)
Jason, can you please sign this xpi?
Flags: needinfo?(jthomas)
Assignee | ||
Comment 10•8 years ago
|
||
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.
Flags: needinfo?(felipc)
Comment 11•8 years ago
|
||
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/
Flags: needinfo?(felipc)
Assignee | ||
Comment 12•8 years ago
|
||
Thank you Felipe. I have now sent the email to request approval to ship.
Assignee | ||
Comment 13•8 years ago
|
||
Felipe, this has approval from Release Management. Can you please push it to production? I don't have commit access.
Flags: needinfo?(felipc)
Assignee | ||
Comment 14•8 years ago
|
||
(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•8 years ago
|
||
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.
Flags: needinfo?(felipc)
Assignee | ||
Comment 16•8 years ago
|
||
(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.
Depends on: 1325786
Comment 17•8 years ago
|
||
Looks like this is deployed since Dec. 25.
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•