We think there might be some unknown issues with WARP on Windows 7 that might make some machines unusable. Let's disable it for windows 7 for now.

Comment on attachment 8628514 [details] [diff] [review] Disable WARP for Windows 7 on Aurora. Review of attachment 8628514 [details] [diff] [review]: ----------------------------------------------------------------- Ok.

Thanks for the speed of this Bas; if this is Aurora only patch, it means we request uplift before we land on nightly?

Hi Bas, Can you help me understand the anticipated user experience and any implications that has on QA? In particular, I might want to do some testing on Windows XP/Vista but I may also want to do some testing on Windows 7/8/10 to make sure nothing is inadvertently broken. And it's okay if you think this change doesn't need QA but I'd like to understand this better so I can at least keep an eye out in our typical feedback channels for issues in the wild. Thanks

(In reply to Anthony Hughes, QA Mentor (:ashughes) from comment #3) > Hi Bas, > > Can you help me understand the anticipated user experience and any > implications that has on QA? In particular, I might want to do some testing > on Windows XP/Vista but I may also want to do some testing on Windows 7/8/10 > to make sure nothing is inadvertently broken. And it's okay if you think > this change doesn't need QA but I'd like to understand this better so I can > at least keep an eye out in our typical feedback channels for issues in the > wild. > > Thanks I don't think we really need QA here in practice. The user implications will be very machine dependent. Basically we're moving a set of users on Windows 7 from their current WARP D3D11 compositor to the traditional basic compositor, which is alreadys ort of the 'safe mode' for these users.

Comment on attachment 8628514 [details] [diff] [review] Disable WARP for Windows 7 on Aurora. Approval Request Comment [Feature/regressing bug #]: D3D11 WARP [User impact if declined]: We suspect D3D11 WARP may not be working correctly for some users on Windows 7, resulting in a black screen. [Describe test coverage new/current, TreeHerder]: Try [Risks and why]: Extremely low, moving to safer configuration. [String/UUID change made/needed]: None

Comment on attachment 8628514 [details] [diff] [review] Disable WARP for Windows 7 on Aurora. Approving for uplift to Aurora. This is a top crash and therefore the sooner we know how well the fix impacts stability the better for us.

Given the incidence rate of crashes due to WARP I'm tracking for 40 and 41.

Release Note Request (optional, but appreciated) [Why is this notable]: This is going to de-accelerate, but also (hopefully) stabilize a number of Windows 7 systems. Again, unclear if this should be a release note or not, but it is noteworthy. Also, notice we uplifted to at least 41, so if we are to release note it, it shouldn't wait for 42.

Comment on attachment 8628514 [details] [diff] [review] Disable WARP for Windows 7 on Aurora. Just like Ritu said.

Comment on attachment 8628514 [details] [diff] [review] Disable WARP for Windows 7 on Aurora. Take away beta+ - this patch is not enough to disable warp. Yes, it does in some, but not in all cases. Bas is getting a patch.

Carrying review from bug 1159751.

Comment on attachment 8628514 [details] [diff] [review] Disable WARP for Windows 7 on Aurora. Approval Request Comment [Feature/regressing bug #]: WARP [User impact if declined]: Some unknown amount of bugs for Win7 users [Describe test coverage new/current, TreeHerder]: Nightly and aurora [Risks and why]: Very few, switching users to fallback [String/UUID change made/needed]: None

Comment on attachment 8633525 [details] [diff] [review] Part 2: Ensure WARP is always unused on Windows 7. Approval Request Comment [Feature/regressing bug #]: WARP [User impact if declined]: Some unknown amount of bugs for a small group of Win7 users [Describe test coverage new/current, TreeHerder]: Try [Risks and why]: Very few, switching users to fallback [String/UUID change made/needed]: None

(In reply to Bas Schouten (:bas.schouten) from comment #14) > [User impact if declined]: Some unknown amount of bugs for a small group of Win7 users FWIW Betabreakers is doing a testrun against Dev Edition later this week which should hopefully get additional data on the impact of this change. They'll be testing a selection of 30 systems across all Windows versions with various AMD, Intel, and NVIDIA GPUs (old and new).

Bas, will this patch apply to dev edition?

(In reply to Milan Sreckovic [:milan] from comment #16) > Bas, will this patch apply to dev edition? It should.

Comment on attachment 8633525 [details] [diff] [review] Part 2: Ensure WARP is always unused on Windows 7. Let's take it now to have it quickly in beta.

Comment on attachment 8628514 [details] [diff] [review] Disable WARP for Windows 7 on Aurora. Let's take it now to have it quickly in beta.

Comment on attachment 8628514 [details] [diff] [review] Disable WARP for Windows 7 on Aurora. This caused massive failures on Aurora. I'm renomming for Beta. We shouldn't take this change until we confirm that fix on an earlier channel.

(In reply to Lawrence Mandel [:lmandel] (use needinfo) from comment #20) > This caused massive failures on Aurora. I'm renomming for Beta. We shouldn't > take this change until we confirm that fix on an earlier channel. Well, it was the patch attached to bug 1159751 that caused the issues. The patch here might be fine.

Comment on attachment 8633525 [details] [diff] [review] Part 2: Ensure WARP is always unused on Windows 7. Nah, this patch is bad. The one above is incomplete, in that it doesn't catch all the problems, but it isn't bad - just doesn't fix all the cases. This one, on the other hand, makes things worse. We're working on a better one, but will need separate version for aurora/beta and nightly as there were other changes in nightly in the same code.

Comment on attachment 8628514 [details] [diff] [review] Disable WARP for Windows 7 on Aurora. Beta- based on the recent comments. We need a better patch for uplift.

This is the uncrashing version of part 2 that is already on Aurora via bug 1159751

Comment on attachment 8628514 [details] [diff] [review] Disable WARP for Windows 7 on Aurora. The problems from the other part of the patch have been fixed and landed on Aurora. This part was always safe.

Comment on attachment 8628514 [details] [diff] [review] Disable WARP for Windows 7 on Aurora. Now that this fix has stabilized, we'll get it into beta7. Beta+

https://hg.mozilla.org/releases/mozilla-beta/rev/ad8e3522ff7c Part 2 landed on Aurora/Beta in bug 1159751. AFAICT, nothing has landed on trunk yet. Not sure if that's intentional or not.

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #27) > https://hg.mozilla.org/releases/mozilla-beta/rev/ad8e3522ff7c > > Part 2 landed on Aurora/Beta in bug 1159751. AFAICT, nothing has landed on > trunk yet. Not sure if that's intentional or not. Intentional - there is a major rewrite of that code tracked in bug 1179051.

This disabling of WARP on Win7 didn't only make the shutdownhangs of bug 1159751 go away, it also reduced OOMs a lot.

(In reply to Robert Kaiser (:kairo@mozilla.com) from comment #29) > This disabling of WARP on Win7 didn't only make the shutdownhangs of bug > 1159751 go away, it also reduced OOMs a lot. Do you have numbers illustrating how much "a lot" is?

(In reply to Anthony Hughes, QA Mentor (:ashughes) from comment #30) > (In reply to Robert Kaiser (:kairo@mozilla.com) from comment #29) > > This disabling of WARP on Win7 didn't only make the shutdownhangs of bug > > 1159751 go away, it also reduced OOMs a lot. > > Do you have numbers illustrating how much "a lot" is? Not as nicely as I'd like to, I'm still working on getting a good display of OOM and other types of crashes up. I'm already creating the dataaset for that, though, here are some raw numbers of total OOM crashes (signatures starting with "OOM | ") in the last week or so on the beta channel: July 20 4237 July 21 4521 July 22 4506 July 23 4456 July 24 3894 (release of b7; note: we did have 5-10% crash data loss on that day) July 25 2816 (Sat, lower numbers normal on weekend) July 26 2013 (Sun, lower numbers normal on weekend) July 27 3209 July 28 2917

Added to FF41 release notes in Nucleus.

I renamed the title to be more accurate. If we have no aim of fixing this on trunk because of bug 1179051, which has landed, then we should close this as FIXED. ni? :dvander who fixed bug 1179051. Can you confirm that we don't need to disable WARP on 42+ because of the improvements made in bug 1179051? If so I think we're good to close this.

(In reply to Benoit Girard (:BenWa) from comment #34) > I renamed the title to be more accurate. > > If we have no aim of fixing this on trunk because of bug 1179051, which has > landed, then we should close this as FIXED. > > ni? :dvander who fixed bug 1179051. Can you confirm that we don't need to > disable WARP on 42+ because of the improvements made in bug 1179051? If so I > think we're good to close this. Yup, WARP is gated on Windows 8 or later on 42+.

(In reply to David Anderson [:dvander] from comment #35) > (In reply to Benoit Girard (:BenWa) from comment #34) > > I renamed the title to be more accurate. > > > > If we have no aim of fixing this on trunk because of bug 1179051, which has > > landed, then we should close this as FIXED. > > > > ni? :dvander who fixed bug 1179051. Can you confirm that we don't need to > > disable WARP on 42+ because of the improvements made in bug 1179051? If so I > > think we're good to close this. > > Yup, WARP is gated on Windows 8 or later on 42+. I guess "Windows 8 or later on 42+" means that we still need to have it disabled on Win7 for 42+ ... Not sure if we should call this fixed based on this.

Actually, "WARP is gated on Windows 8 or later on 42+" meant "you need Windows 8 or later to get WARP as of 42 and subsequent releases", so we should be OK.