Closed
Bug 854758
Opened 12 years ago
Closed 12 years ago
Set the MOZ_DISABLE_CONTEXT_SHARING_GLX env var for Cipc/Ripc on mozilla-central and related branches
Categories
(Release Engineering :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: mattwoodrow, Assigned: nrc)
References
Details
Attachments
(3 files, 5 obsolete files)
(deleted),
patch
|
bhearsum
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
mozilla
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
mozilla
:
review+
|
Details | Diff | Splinter Review |
With the graphics refactoring, we appear to be hitting a driver bug on Cipc/Ripc.
This env var disables context sharing which is the only workaround we know of.
The existing env var 'MOZ_LAYERS_FORCE_SHMEM_SURFACES' doesn't actually appear to be used for anything any more.
Attachment #729368 -
Flags: review?(lsblakk)
Assignee | ||
Updated•12 years ago
|
Blocks: layers-refactoring, 849261
Comment 1•12 years ago
|
||
Comment on attachment 729368 [details] [diff] [review]
Add MOZ_DISABLE_CONTEXT_SHARING_GLX for Cipc/Ripc
We should try to make this ride the trains...I'll see if I can figure something out.
Attachment #729368 -
Flags: review?(lsblakk)
Assignee | ||
Comment 2•12 years ago
|
||
Turns out we also need to add the MOZ_USE_OMTC flag for these tests too. Could you add that in as well? Do you want a new patch or will you be doing this by hand anyway?
Flags: needinfo?(bhearsum)
Comment 3•12 years ago
|
||
(In reply to Nick Cameron [:nrc] from comment #2)
> Turns out we also need to add the MOZ_USE_OMTC flag for these tests too.
> Could you add that in as well? Do you want a new patch or will you be doing
> this by hand anyway?
There's no doing it by hand, but don't bother with a new patch. I still haven't figured out how to make these changes ride the trains...
Flags: needinfo?(bhearsum)
Comment 4•12 years ago
|
||
I think I have a way to do this provided it's safe to set this pref for all unit tests. Matt, any idea if that would have any negative impact?
Flags: needinfo?(matt.woodrow)
Reporter | ||
Comment 5•12 years ago
|
||
I *think* that would be ok, but it would probably cause issues if we try enable OpenGL layers on the test machines.
I guess we're unlikely to do that on the ec2 boxes though, since their software OpenGL is the reason we need to do this.
Is there any reason we need to make this ride the trains? Both env vars that we're adding are new ones, so should have no effect if applied to other trees.
Unless the existing var that's being removed still exists in older version of the code.
Flags: needinfo?(matt.woodrow)
Comment 6•12 years ago
|
||
(In reply to Matt Woodrow (:mattwoodrow) from comment #5)
> I *think* that would be ok, but it would probably cause issues if we try
> enable OpenGL layers on the test machines.
>
> I guess we're unlikely to do that on the ec2 boxes though, since their
> software OpenGL is the reason we need to do this.
>
> Is there any reason we need to make this ride the trains? Both env vars that
> we're adding are new ones, so should have no effect if applied to other
> trees.
I was more concerned about MOZ_LAYERS_FORCE_SHMEM_SURFACES, to be honest. However, given that it only affects debug builds (https://hg.mozilla.org/releases/mozilla-esr17/file/default/gfx/layers/ipc/ShadowLayers.cpp#l452) I think it's probably okay to change...we should just watch out for new test failures on aurora/beta/release/esr17 when landing.
I'll whip up a new patch.
Comment 7•12 years ago
|
||
Assignee: nobody → bhearsum
Attachment #729368 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #733845 -
Flags: review?(rail)
Comment 8•12 years ago
|
||
I guess I should post a real patch....
Attachment #733845 -
Attachment is obsolete: true
Attachment #733845 -
Flags: review?(rail)
Attachment #733963 -
Flags: review?(rail)
Updated•12 years ago
|
Attachment #733963 -
Flags: review?(rail) → review+
Comment 9•12 years ago
|
||
Comment on attachment 733963 [details] [diff] [review]
add new flags
Landed on default - should get put into production on Monday.
Attachment #733963 -
Flags: checked-in+
Comment 10•12 years ago
|
||
in production
Comment 11•12 years ago
|
||
Matt, anything you want to do to verify that this is working as expected? Job started after Kim's comment should have the new flags set.
Comment 12•12 years ago
|
||
So, this torched Cipc/Ripc on the b2g18 branches...
Comment 13•12 years ago
|
||
Comment on attachment 733963 [details] [diff] [review]
add new flags
Ugh, I was afraid of that. I'm backing out.
Attachment #733963 -
Flags: checked-in+ → checked-in-
Comment 14•12 years ago
|
||
Comment 15•12 years ago
|
||
Matt, would it hurt to keep the MOZ_LAYERS_FORCE_SHMEM_SURFACES flag? Removing it seems to have busted some branches.
Reporter | ||
Comment 16•12 years ago
|
||
I believe it's actually the MOZ_USE_OMTC env var that we are adding.
We should change this to a new name, and update the graphics branch so that it reads the new var.
Comment 17•12 years ago
|
||
(In reply to Matt Woodrow (:mattwoodrow) from comment #16)
> I believe it's actually the MOZ_USE_OMTC env var that we are adding.
>
> We should change this to a new name, and update the graphics branch so that
> it reads the new var.
comment #5 says both env vars are new. Doesn't that mean MOZ_USE_OMTC should be unused on b2g18 (which is Gecko 18-based), or did I misunderstand something?
Reporter | ||
Comment 18•12 years ago
|
||
Unfortunately that comment was wrong :(
My intent was for both to be new, but that wasn't what actually landed.
Assignee | ||
Comment 19•12 years ago
|
||
Looks like we also need to set the layers.offmainthreadcomposition.enabled pref to true for these runs too. Can someone tell me how to do this please?
Assignee | ||
Comment 20•12 years ago
|
||
Use a fresh flag and pref
Attachment #733963 -
Attachment is obsolete: true
Attachment #734877 -
Flags: review?(rail)
Comment 21•12 years ago
|
||
Comment on attachment 734877 [details] [diff] [review]
add flags and pref
(In reply to Nick Cameron [:nrc] from comment #19)
> Looks like we also need to set the layers.offmainthreadcomposition.enabled
> pref to true for these runs too. Can someone tell me how to do this please?
Unfortunately, it's not this this simple anymore. Because the OMTC flag/pref affects older branches we need to make sure that this is only set on mozilla-central (and related) branches, and that it rides the trains forward.
Attachment #734877 -
Flags: review?(rail) → review-
Comment 22•12 years ago
|
||
Aki, we recently moved unit tests to mozharness. Do we have any known way in those scripts for train riding / branch specific settings?
Flags: needinfo?(aki)
Updated•12 years ago
|
Assignee: bhearsum → nobody
Component: Release Engineering → Release Engineering: Automation (General)
QA Contact: catlee
Summary: [buildbotcustom] Set the MOZ_DISABLE_CONTEXT_SHARING_GLX env var for Cipc/Ripc → Set the MOZ_DISABLE_CONTEXT_SHARING_GLX env var for Cipc/Ripc on mozilla-central and related branches
Comment 23•12 years ago
|
||
(In reply to Ben Hearsum [:bhearsum] from comment #22)
> Aki, we recently moved unit tests to mozharness. Do we have any known way in
> those scripts for train riding / branch specific settings?
Lots of options, but nothing implemented in a really easy way yet.
* different command line options or buildprops per branch. This will require buildbot-configs changes and a reconfig every merge day.
* looking at something in-tree or in-test-zip for clues in behavior.
* multiple config files. However, currently these are required to all exist, so if you did
script.py -c real_config.py -c BRANCH_config.py
then BRANCH_config.py would have to exist for every single branch we run tests for. Maybe hackable like we did generic mozconfigs. This would require a mozharness config change every merge day.
Some options that will require more work:
* multiple config files. If we made an optional additional config file, e.g. -c required_config -C optional_config, then optional_config could be path/to/OPTIONAL_CONFIG.py (possibly an in-tree/in-test-zip file)
* optional config file via url, pointing to hg.m.o/FULL_BRANCH_PATH/raw/FILE
* I have mozharness talos look in-tree for a talos.json that has a lot of config info; we could do something similar for unittests.
* Possibly a way for unittests to determine what version Firefox is, and specifying "turn this config option on for fx version >= X or w/e)
Possibly other solutions; these are the ones that come to mind.
Flags: needinfo?(aki)
Assignee | ||
Comment 24•12 years ago
|
||
(In reply to Ben Hearsum [:bhearsum] from comment #21)
> Comment on attachment 734877 [details] [diff] [review]
> add flags and pref
>
> (In reply to Nick Cameron [:nrc] from comment #19)
> > Looks like we also need to set the layers.offmainthreadcomposition.enabled
> > pref to true for these runs too. Can someone tell me how to do this please?
>
> Unfortunately, it's not this this simple anymore. Because the OMTC flag/pref
> affects older branches we need to make sure that this is only set on
> mozilla-central (and related) branches, and that it rides the trains forward.
I have used a completely new env var and pref, neither of which are used in existing code, and changed the code on the graphics branch to check for either the new or old pref. So existing code should no longer be affected.
Assignee | ||
Updated•12 years ago
|
Attachment #734877 -
Flags: review- → review?(bhearsum)
Comment 25•12 years ago
|
||
(In reply to Nick Cameron [:nrc] from comment #24)
> (In reply to Ben Hearsum [:bhearsum] from comment #21)
> > Comment on attachment 734877 [details] [diff] [review]
> > add flags and pref
> >
> > (In reply to Nick Cameron [:nrc] from comment #19)
> > > Looks like we also need to set the layers.offmainthreadcomposition.enabled
> > > pref to true for these runs too. Can someone tell me how to do this please?
> >
> > Unfortunately, it's not this this simple anymore. Because the OMTC flag/pref
> > affects older branches we need to make sure that this is only set on
> > mozilla-central (and related) branches, and that it rides the trains forward.
>
> I have used a completely new env var and pref, neither of which are used in
> existing code, and changed the code on the graphics branch to check for
> either the new or old pref. So existing code should no longer be affected.
Ah, sorry. I only searched for "offmainthreadcomposition" previously, and confused some of the results with the actual pref you're setting.
However, when we landed the previous patch it busted b2g18, so _something_ in there is affecting it. I see that MOZ_LAYERS_FORCE_SHMEM_SURFACES exists on that branch, so maybe the removal of it caused that problem?
Assignee | ||
Comment 26•12 years ago
|
||
(In reply to Ben Hearsum [:bhearsum] from comment #25)
> (In reply to Nick Cameron [:nrc] from comment #24)
> > (In reply to Ben Hearsum [:bhearsum] from comment #21)
> > > Comment on attachment 734877 [details] [diff] [review]
> > > add flags and pref
> > >
> > > (In reply to Nick Cameron [:nrc] from comment #19)
> > > > Looks like we also need to set the layers.offmainthreadcomposition.enabled
> > > > pref to true for these runs too. Can someone tell me how to do this please?
> > >
> > > Unfortunately, it's not this this simple anymore. Because the OMTC flag/pref
> > > affects older branches we need to make sure that this is only set on
> > > mozilla-central (and related) branches, and that it rides the trains forward.
> >
> > I have used a completely new env var and pref, neither of which are used in
> > existing code, and changed the code on the graphics branch to check for
> > either the new or old pref. So existing code should no longer be affected.
>
> Ah, sorry. I only searched for "offmainthreadcomposition" previously, and
> confused some of the results with the actual pref you're setting.
>
> However, when we landed the previous patch it busted b2g18, so _something_
> in there is affecting it. I see that MOZ_LAYERS_FORCE_SHMEM_SURFACES exists
> on that branch, so maybe the removal of it caused that problem?
We think what affected it was the env var - MOZ_USE_OMTC, but in the new patch I use MOZ_OMTC_ENABLED, which has not been used before. We could leave MOZ_LAYERS_FORCE_SHMEM_SURFACES to be on the safe side.
Assignee | ||
Comment 27•12 years ago
|
||
now without removing the surfaces flag
Attachment #734877 -
Attachment is obsolete: true
Attachment #734877 -
Flags: review?(bhearsum)
Attachment #735373 -
Flags: review?(bhearsum)
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → ncameron
Comment 28•12 years ago
|
||
Comment on attachment 735373 [details] [diff] [review]
add flags and pref
Review of attachment 735373 [details] [diff] [review]:
-----------------------------------------------------------------
(In reply to Nick Cameron [:nrc] from comment #26)
> (In reply to Ben Hearsum [:bhearsum] from comment #25)
> > (In reply to Nick Cameron [:nrc] from comment #24)
> > > (In reply to Ben Hearsum [:bhearsum] from comment #21)
> > > > Comment on attachment 734877 [details] [diff] [review]
> > > > add flags and pref
> > > >
> > > > (In reply to Nick Cameron [:nrc] from comment #19)
> > > > > Looks like we also need to set the layers.offmainthreadcomposition.enabled
> > > > > pref to true for these runs too. Can someone tell me how to do this please?
> > > >
> > > > Unfortunately, it's not this this simple anymore. Because the OMTC flag/pref
> > > > affects older branches we need to make sure that this is only set on
> > > > mozilla-central (and related) branches, and that it rides the trains forward.
> > >
> > > I have used a completely new env var and pref, neither of which are used in
> > > existing code, and changed the code on the graphics branch to check for
> > > either the new or old pref. So existing code should no longer be affected.
> >
> > Ah, sorry. I only searched for "offmainthreadcomposition" previously, and
> > confused some of the results with the actual pref you're setting.
> >
> > However, when we landed the previous patch it busted b2g18, so _something_
> > in there is affecting it. I see that MOZ_LAYERS_FORCE_SHMEM_SURFACES exists
> > on that branch, so maybe the removal of it caused that problem?
>
> We think what affected it was the env var - MOZ_USE_OMTC, but in the new
> patch I use MOZ_OMTC_ENABLED, which has not been used before. We could leave
> MOZ_LAYERS_FORCE_SHMEM_SURFACES to be on the safe side.
Ah, I see. This seems OK now. Feel free to land it on the default branch at your convenience. It'll get put into production with the next reconfig.
Attachment #735373 -
Flags: review?(bhearsum) → review+
Assignee | ||
Comment 29•12 years ago
|
||
Landed on hg.mozilla.org/build/buildbotcustom/
Comment 30•12 years ago
|
||
in production
Comment 31•12 years ago
|
||
I think this is failing to have any effect because m-c & co switched to running these tests through mozharness, rather than through the buildbot harnesses. The patch is making changes on branches like mozilla-b2g18*, mozilla-esr17 and mozilla-release because they still do things the old way.
Reporter | ||
Comment 32•12 years ago
|
||
It appears (from looking at log output) that we are only getting pref values from here, and not from buildbotcustom.
This is a patch to add the pref to mozharness, but doesn't add the env vars that we need. Any idea how I can do that?
Attachment #735553 -
Flags: review?(nthomas)
Comment 33•12 years ago
|
||
Comment on attachment 735553 [details] [diff] [review]
Add prefs to mozharness too
Forwarding to aki, who wrote this code.
Attachment #735553 -
Flags: review?(nthomas) → review?(aki)
Comment 34•12 years ago
|
||
Comment on attachment 735553 [details] [diff] [review]
Add prefs to mozharness too
Jordan wrote it :)
This will turn this flag on for linux cipc + ripc on m-c (and all m-c level repos, including try and inbound), and m-a. If that's the desired effect, r=me.
This should land in default, and we can merge into the production mozharness branch when tests pass.
Attachment #735553 -
Flags: review?(aki) → review+
Reporter | ||
Comment 35•12 years ago
|
||
Hope this is what you meant.
Obviously untested, not sure how I'd go about doing that.
Attachment #735564 -
Flags: review?(aki)
Reporter | ||
Comment 36•12 years ago
|
||
Added crashtest-ipc changes too, and fixed the syntax error.
Attachment #735564 -
Attachment is obsolete: true
Attachment #735564 -
Flags: review?(aki)
Attachment #735575 -
Flags: review?(aki)
Comment 37•12 years ago
|
||
Comment on attachment 735575 [details] [diff] [review]
Add env vars to mozharness v2
If you land on default, Cedar unittests will be affected and nothing else. You or Nick or a sheriff can trigger tests to verify (or merge m-c in or w/e).
Merging to the production branch will roll out everywhere.
Attachment #735575 -
Flags: review?(aki) → review+
Comment 38•12 years ago
|
||
Landed attachment 735553 [details] [diff] [review] and 735575 as
http://hg.mozilla.org/build/mozharness/rev/e2d502f58764
Reporter | ||
Comment 39•12 years ago
|
||
Landed a follow-up because I typo'd a variable name: http://hg.mozilla.org/build/mozharness/rev/2a81f599721d
Comment 40•12 years ago
|
||
Merged to production.
Comment 41•12 years ago
|
||
Comment on attachment 735373 [details] [diff] [review]
add flags and pref
Backed this out:
http://hg.mozilla.org/build/buildbotcustom/rev/21ea12b25bdb
Comment 42•12 years ago
|
||
Aki and Nick - thanks for jumping in here. How're things looking now, Matt?
Reporter | ||
Comment 43•12 years ago
|
||
Things are great, we've landed on m-c and tests are passing.
Thanks for all the help getting this fixed in a hurry.
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Comment 44•12 years ago
|
||
in production
Updated•11 years ago
|
Product: mozilla.org → Release Engineering
Updated•7 years ago
|
Component: General Automation → General
You need to log in
before you can comment on or make changes to this bug.
Description
•