Closed
Bug 1367246
Opened 7 years ago
Closed 7 years ago
Run a Telemetry experiment to vet the unaccelerated compositor process in Nightly 56
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
FIXED
mozilla56
People
(Reporter: u279076, Assigned: u279076)
References
Details
(Whiteboard: [gfx-noted])
Attachments
(5 files, 2 obsolete files)
This is bug tracks running a Telemetry Experiment to A/B test the unaccelerated compositor process as implemented in bug 1356091.
Assigning to myself, code incoming.
Initial patch to implement Telemetry Experiment.
Note, this experiment has three cohorts:
1) control: users with unaccelerated compositor process enabled
2) disabled: users with unaccelerated compositor process disabled
3) disqualified: users with hardware acceleration
The reason for the 'disqualified' cohort is so that accelerated users are not mistakenly factored in to the 'control' cohort data. All three conditions have been tested successfully on a local machine.
David, please take a quick look and let me know if you see any issues.
Felipe, please review this when you get a chance as we'd like this experiment to conclude before June 12th. I still need to get the XPI signed and approval from Release Management, pending your review.
Attachment #8870611 -
Flags: review?(felipc)
Attachment #8870611 -
Flags: feedback?(dvander)
I left the Nightly running overnight which had the experiment active and came back to this screen. It looks like Nightly had decided to disable my TelEx overnight and is prompting me to re-enable it. I've never seen this before. Is it expected?
(In reply to Anthony Hughes (:ashughes) [GFX][QA][Mentor] from comment #3)
> Created attachment 8870929 [details]
> Screenshot
>
> I left the Nightly running overnight which had the experiment active and
> came back to this screen. It looks like Nightly had decided to disable my
> TelEx overnight and is prompting me to re-enable it. I've never seen this
> before. Is it expected?
Sorry, forgot to flag the need-infos.
Flags: needinfo?(felipc)
Flags: needinfo?(dothayer)
Comment 5•7 years ago
|
||
I never saw this doorhanger before. Andy, do you know if this is from the work to disable legacy addons?
Flags: needinfo?(felipc) → needinfo?(amckay)
Comment 6•7 years ago
|
||
I believe thats the doorhanger that occurs for sideloading and add-on. Looking at the add-on it is not multi process compatible, according to the install.rdf. We shouldn't be shipping any non-e10s add-ons.
/cc'ing aswan to check what I said.
Flags: needinfo?(amckay) → needinfo?(aswan)
Comment 7•7 years ago
|
||
The doorhanger in the "Screenshot" attachment is indeed the one used for sideloading. I don't know much about telemetry experiments but I thought that we were trying to deprecate them in favor of shield?
As for multiprocessCompatible, we don't pay attention to it for addons of types other than "extension" (not intentionally anyway...)
Flags: needinfo?(aswan)
Comment 8•7 years ago
|
||
Ah this notice used to be an about: page, I didn't know it was changed to a doorhanger. Hm I'm guessing this might have been triggered by the fact that the xpi is not signed yet, but I'm not sure. If after signing the same problem continues to happen, we should investigate because it would be a recent bug.
Flags: needinfo?(dothayer)
Comment 9•7 years ago
|
||
Comment on attachment 8870611 [details] [diff] [review]
bug1367246_v1.patch
Review of attachment 8870611 [details] [diff] [review]:
-----------------------------------------------------------------
::: experiments/unaccelerated-compositor-process-nightly55/code/bootstrap.js
@@ +86,5 @@
> + if (compositor == "basic") {
> + initializeExperiment();
> + } else {
> + Experiments.instance().setExperimentBranch(kSELF_ID, "disqualified").then(null, Cu.reportError);
> + initializeExperiment();
since you're now already setting the branch here, you don't need to call initializeExperiment() here or in the similar case below.
r+ with this changed.
::: experiments/unaccelerated-compositor-process-nightly55/manifest.json
@@ +12,5 @@
> + "appName" : ["Firefox"],
> + "channel" : ["nightly"],
> + "minVersion" : "55.0a1",
> + "maxVersion" : "55.0a1",
> + "os" : ["WINNT"]
we should use a sample here to not go to 100% of users
Attachment #8870611 -
Flags: review?(felipc) → review+
Comment 10•7 years ago
|
||
(In reply to :Felipe Gomes (needinfo me!) from comment #8)
> Ah this notice used to be an about: page, I didn't know it was changed to a
> doorhanger. Hm I'm guessing this might have been triggered by the fact that
> the xpi is not signed yet, but I'm not sure. If after signing the same
> problem continues to happen, we should investigate because it would be a
> recent bug.
The doorhanger is unrelated to signing, it is shown for any sideloaded extension.
To be clear, your test uses the experiment manifest to have Experiments.jsm set up the experiment right?
Kris, could this (telemetry experiments showing the sideload doorhanger) be an (unintended?) side effect of bug 1358846?
Flags: needinfo?(kmaglione+bmo)
Comment 11•7 years ago
|
||
(In reply to Andrew Swan [:aswan] from comment #10)
> (In reply to :Felipe Gomes (needinfo me!) from comment #8)
> > Ah this notice used to be an about: page, I didn't know it was changed to a
> > doorhanger. Hm I'm guessing this might have been triggered by the fact that
> > the xpi is not signed yet, but I'm not sure. If after signing the same
> > problem continues to happen, we should investigate because it would be a
> > recent bug.
>
> The doorhanger is unrelated to signing, it is shown for any sideloaded
> extension.
> To be clear, your test uses the experiment manifest to have Experiments.jsm
> set up the experiment right?
Correct
Comment 12•7 years ago
|
||
(In reply to Andrew Swan [:aswan] from comment #10)
> Kris, could this (telemetry experiments showing the sideload doorhanger) be
> an (unintended?) side effect of bug 1358846?
That or bug 1356826 seem like likely culprits, yes.
I was under the impression that we were no longer deploying new telemetry experiments in favor of Shield studies, though.
Flags: needinfo?(kmaglione+bmo)
Comment 13•7 years ago
|
||
Ah no, telemetry experiments are still being used, and there's a few coming up soon.
Comment 14•7 years ago
|
||
OK, then I'll look into this.
Assignee | ||
Comment 15•7 years ago
|
||
(In reply to Kris Maglione [:kmag] (busy; behind on reviews) from comment #14)
> OK, then I'll look into this.
Do you want me to file a bug?
Comment 16•7 years ago
|
||
(In reply to Anthony Hughes (:ashughes) [GFX][QA][Mentor] from comment #15)
> (In reply to Kris Maglione [:kmag] (busy; behind on reviews) from comment
> #14)
> > OK, then I'll look into this.
>
> Do you want me to file a bug?
Yes, that would help. Thanks.
Assignee | ||
Comment 17•7 years ago
|
||
Be advised that we're not passed a point where we can run this experiment on Nightly 55 due to bug 1367823. With any luck we might be able to do this in Beta, assuming the lack of an experiment in Nightly doesn't block this feature from riding the trains.
Assignee | ||
Comment 18•7 years ago
|
||
Updating this to reflect that I plan to run this experiment in 56 now, not 55.
Summary: Run a Telemetry experiment to vet the unaccelerated compositor process in Nightly 55 → Run a Telemetry experiment to vet the unaccelerated compositor process in Nightly 56
Version: unspecified → 56 Branch
Assignee | ||
Comment 19•7 years ago
|
||
Hi Felipe,
Here is an updated patch. I've tested it locally and everything works as expected. I made the changes you previously requested but given the amount of time that's past can you please review it?
Thanks
Attachment #8883662 -
Flags: review?(felipc)
Updated•7 years ago
|
Attachment #8883662 -
Flags: review?(felipc) → review+
Assignee | ||
Comment 20•7 years ago
|
||
Felipe, thanks for the r+. I'm uploading a new patch with the following cosmetic changes:
- Revised timestamps and duration to start on July 10, end on July 20
- Removed previously commented out code
- Made new XPI with these changes
I trust this doesn't need re-review.
Attachment #8870611 -
Attachment is obsolete: true
Attachment #8883662 -
Attachment is obsolete: true
Attachment #8870611 -
Flags: feedback?(dvander)
Attachment #8884072 -
Flags: review+
Assignee | ||
Comment 21•7 years ago
|
||
Jason, can you please sign this XPI?
Attachment #8884075 -
Flags: feedback?(jthomas)
Comment 22•7 years ago
|
||
Please see attached.
Assignee | ||
Comment 23•7 years ago
|
||
Thanks Jason.
Felipe, can you please push this to staging so I can test it before requesting RelMan approval?
Flags: needinfo?(felipc)
Updated•7 years ago
|
Attachment #8884075 -
Flags: feedback?(jthomas)
Comment 24•7 years ago
|
||
Flags: needinfo?(felipc)
Assignee | ||
Comment 25•7 years ago
|
||
(In reply to :Felipe Gomes (needinfo me!) from comment #24)
> Done!
> https://hg.mozilla.org/webtools/telemetry-experiment-server/rev/
> 61df6e6c03cf4232a869699c1775ae6f51931c2e
Hi Felipe, I'm not seeing any active experiments when I point to https://telemetry-experiment-dev.allizom.org/firefox-manifest.json
Flags: needinfo?(felipc)
Comment 26•7 years ago
|
||
I cloned the repo in a different folder (to ensure it wasn't a problem of untracked files that weren't pushed), and built it locally without problems. I wonder if it's some infra issue
Flags: needinfo?(felipc)
Assignee | ||
Comment 27•7 years ago
|
||
(In reply to :Felipe Gomes (needinfo me!) from comment #26)
> I cloned the repo in a different folder (to ensure it wasn't a problem of
> untracked files that weren't pushed), and built it locally without problems.
> I wonder if it's some infra issue
I concur, locally it works but pointing to the staging server does not. I'm returned with no active experiments on staging. I don't see anything on the mailing lists that would indicate an infra issue. Who can I contact to look in to this?
Assignee | ||
Comment 28•7 years ago
|
||
:Usul, do you know what's happening here? I'm confused as to why this would work locally but not on staging.
Flags: needinfo?(ludovic)
Comment 29•7 years ago
|
||
(In reply to Anthony Hughes (:ashughes) [GFX][QA][Mentor] from comment #28)
> :Usul, do you know what's happening here? I'm confused as to why this would
> work locally but not on staging.
If staging doesn't have video drivers then you'd see a difference (wondering why you asked me though :))
Flags: needinfo?(ludovic)
Comment 30•7 years ago
|
||
(In reply to Ludovic Hirlimann [:Usul] from comment #29)
> (In reply to Anthony Hughes (:ashughes) [GFX][QA][Mentor] from comment #28)
> > :Usul, do you know what's happening here? I'm confused as to why this would
> > work locally but not on staging.
>
> If staging doesn't have video drivers then you'd see a difference (wondering
> why you asked me though :))
Reading more , I have no clue and no access to these systems ask someone from :jason's team
Assignee | ||
Comment 31•7 years ago
|
||
(In reply to Ludovic Hirlimann [:Usul] from comment #30)
> Reading more , I have no clue and no access to these systems ask someone from :jason's team
Sorry, it was suggested I reach out to you.
Jason, do you have any idea why I'm seeing no active experiments on staging? This experiment should be active there but I don't see it when I point to https://telemetry-experiment-dev.allizom.org/firefox-manifest.json.
Flags: needinfo?(jthomas)
Comment 32•7 years ago
|
||
Sorry I don't think I am going to be much help here. The service telemetry-experiment-dev.allizom.org is currently not managed by my team (Data Operations) and is in IT's control (resides on IT's infrastructure, specifically the generic cluster). Data Operations *should* take control over this service and move it out of IT's control.
IT Webops might be the right person to reach out. The Mozilla MOC should be able to assist with contacting the correct people.
https://mana.mozilla.org/wiki/pages/viewpage.action?pageId=38553804 has some documentation on this serivce.
Flags: needinfo?(jthomas)
Updated•7 years ago
|
Component: Graphics → WebOps: Other
Product: Core → Infrastructure & Operations
Version: 56 Branch → unspecified
Whiteboard: [gfx-noted] → [kanban:https://webops.kanbanize.com/ctrl_board/2/5061] [gfx-noted]
Comment 33•7 years ago
|
||
Taking a look at this now.
Comment 34•7 years ago
|
||
There was a hung hg pull that was clogging up things. I've cleared it out and this is working now (though you may have a local cache). If hg pull hangs are a known thing we should probably run this with a timeout command so it doesn't hang forever.
Assignee | ||
Comment 35•7 years ago
|
||
Thanks Eric, this is now working and I've confirmed my experiment is operating as expected on staging. Will now send an email to get approval to push to production.
Updated•7 years ago
|
Component: WebOps: Other → Graphics
Product: Infrastructure & Operations → Core
Whiteboard: [kanban:https://webops.kanbanize.com/ctrl_board/2/5061] [gfx-noted] → [gfx-noted]
Version: unspecified → 56 Branch
status-firefox56:
--- → affected
tracking-firefox56:
--- → +
Assignee | ||
Comment 36•7 years ago
|
||
Hi Felipe,
I just got sign-off from Release Management. The dates in the manifest will need to be updated before pushing to production. Given the setbacks I'd like this to start today (July 17) and run through July 27.
Thanks in advance.
Flags: needinfo?(felipc)
Comment 37•7 years ago
|
||
Alright, I updated the start/end dates in the manifest, and added the release_tag for it. Once it appears correctly on staging, all that's left is to file a bug for it to be deployed to production:
https://hg.mozilla.org/webtools/telemetry-experiment-server/rev/a8a91936dac30a53c64aa027a07a8b85c8e0e7c2
https://hg.mozilla.org/webtools/telemetry-experiment-server/rev/6022f3f24315a62063e09a1b6b9e37a2d0a03c1a
Flags: needinfo?(felipc)
Assignee | ||
Comment 38•7 years ago
|
||
(In reply to :Felipe Gomes (needinfo me!) from comment #37)
> All that's left is to file a bug for it to be deployed to production
Thanks Felipe!
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 39•7 years ago
|
||
Experiment is now live. Thanks again everyone for your help!
Updated•7 years ago
|
Target Milestone: --- → mozilla56
Assignee | ||
Comment 40•7 years ago
|
||
Flagging some folks to spread this around before it becomes a larger issue...
From bug 1371363:
The problem I was facing in bug 1367246 comment 3 related to this bug has just been shown to me in the wild on one of my officemates' computers. He updated his Nightly and saw the prompt to enable the experiment. I don't think there's anything I can do now but this may impact the number of users getting my experiment and the prompt may be "scary" to some users.
Flags: needinfo?(milan)
Flags: needinfo?(lhenry)
Flags: needinfo?(dvander)
Comment 41•7 years ago
|
||
Anthony, can you get the build details for the Firefox version that exhibited this problem?
Flags: needinfo?(anthony.s.hughes)
Assignee | ||
Comment 42•7 years ago
|
||
(In reply to Andrew Swan [:aswan] from comment #41)
> Anthony, can you get the build details for the Firefox version that
> exhibited this problem?
Adding Roland who should be able to get you the details from his system.
Flags: needinfo?(anthony.s.hughes) → needinfo?(rtanglao)
I'll track this issue in bug 1371363. I wouldn't want users on release to see this warning.
Flags: needinfo?(lhenry)
Assignee | ||
Comment 45•7 years ago
|
||
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #43)
> I'll track this issue in bug 1371363. I wouldn't want users on release to
> see this warning.
As a note, I originally saw this warning when I was developing this experiment back in Nightly 55 so I assume it'd affect 55 Beta users had I experimented there instead.
Updated•7 years ago
|
Flags: needinfo?(milan)
Flags: needinfo?(dvander)
You need to log in
before you can comment on or make changes to this bug.
Description
•