1068674 - Enable e10s when hardware acceleration is disabled

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect, P2)

Description

[Jim Mathies [:jimm]]

+++ This bug was initially created as a clone of Bug #1068199 +++

Remove the accel disable hacks landed in bug 1068199. bug 1026093 and 1026521 currently block m3, so this blocks m3 as well.

Comment 1

[Jim Mathies [:jimm]]

bug 1026521 was set to m4, so this is an m4.

Comment 2

[Jim Mathies [:jimm]]

argh, sorry, wrong bug.

Comment 3

[George Wright (:gw280) (needinfo me!)]

https://tbpl.mozilla.org/?tree=Try&rev=5af83c8b9c88

Let's see what the current status of this is. It looks like the fallback to non-e10s is only present on OS X right now.

Comment 4

[David Parks [:handyman]]

Renoming for triage since gw280, jimm and I discussed dropping this.

****************

Here is the report on OS coverage:

http://people.mozilla.org/~bgirard/gfx_features_stats/#mac

It says that mac Hardware Acceleration coverage is just south of 100%.  I've mined the raw data results from yesterday and determined that there are a handful of cases that seem to produce unaccelerated runs... and none of them are our concern.  I am recommending we don't attempt to fix this issue.

For the runs that show no HWA attempts at all:

* The broadest case for which we do not know about acceleration (not included in the graph) are old versions of Firefox (quite a few data points from Firefox 3!).

* There are also quite a few that don't get far enough to report GPU data.  This is also not included in the graph.  (Some have GPU IDs that don't match any known hardware but this must be junk, like the previous case.  Alternatively, it could be device manufacturers' internal hardware testing.)

* A number are for the ATI X1600.  I'm not sure how these are running later versions of the OS (many on OS 10.6.8) but this is probably hackintosh.  The GPU was used in 2008 macs.
Example: https://crash-stats.mozilla.com/report/index/39578c32-c0e3-4da0-ba5c-24ad12141215

* There is one that tests HWA and fails.  It is using Firefox 27, which the stats mention had a bug that caused this (its in FFox 27-30).
https://crash-stats.mozilla.com/report/index/52d117d9-ae32-4327-b45a-7f0e62141215

To look more deeply at the failed HWA attempts, I did a sort of manual version of the parsing that the code that builds the graph does.

* Get a list of the mac reports that the graph considers relevant (the denominator of the ratio):
grep "Mac OS X" 20141215-pub-crashdata.csv  | grep "GL Layers\!"
grep "Mac OS X" 20141215-pub-crashdata.csv  | grep "GL Layers?"
7590 matches.

* Get a list of the reports that show successful HWA (the numerator):
grep "Mac OS X" 20141215-pub-crashdata.csv  | grep "GL Layers+"
7579 matches.

* Get a list of the reports that show HWA failure (after an attempt):
grep "Mac OS X" 20141215-pub-crashdata.csv  | grep "GL Layers-"
11 matches.

This basically lines up but there is one unimportant outlier that matches both "GL Layers!" and "GL Layers?" ... it is the aforementioned one with the bug in Firefox 27.

Regarding the "Layers-" cases:

* 5 of them are Firefox 27-30.
* 2 do not match known hardware:
Unknown Intel:  https://crash-stats.mozilla.com/report/index/a3c7f774-be42-4be4-a00c-421032141215
Unknown NVIDIA: https://crash-stats.mozilla.com/report/index/108c9c12-9166-46d4-b1f2-2e6282141215
You can look up hardware by adapter IDs here: http://www.pcidatabase.com/index.php
* 2 match hardware I cant find in macs (likely Hackintosh).
ATI Radeon HD 6700M:         https://crash-stats.mozilla.com/report/index/6696586c-049e-488f-a1d5-c93e22141215
ATI RADEON HD 3200 Graphics: https://crash-stats.mozilla.com/report/index/16c467f1-52c1-487f-bf70-bfca72141215
* 2 are cards that were low-end in 5+ years ago.  Not sure if this is legitimately running later versions of OSX.
Intel GMA3100:                      https://crash-stats.mozilla.com/report/index/8170e286-fd99-46a6-992d-e94ef2141215
ATI Mobility Radeon HD 4670 (2009): https://crash-stats.mozilla.com/report/index/2863cfef-5f1a-4a07-9e80-06ba22141215

Apart from the 2 old machines, which might warrant further investigation, I don't see anything here that looks like things we should address.  This is obviously not exhaustive but suggests we don't have an issue here.  A more skeptical view might say that people who need BasicLayers aren't running Firefox because it doesn't work... but that would still mean that we have no data suggesting that we have a reason to do this.

If there is still an issue with some hardware, I'd prefer to tackle it on a case-by-case basis.  This might be easier (and better) than fixing the entire basic compositor.

Comment 5

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Andrew, as I remember, you don't use HW accel. Why was that again?

Comment 6

[Andrew McCreight [:mccr8]]

(In reply to Bill McCloskey (:billm) from comment #5)
> Andrew, as I remember, you don't use HW accel. Why was that again?

On my old MacBook Pro, I think the GPU is damaged, so any time I use HWA on Firefox or Chrome it gets a GPU panic and reboots the computer.

Comment 7

[Markus Stange [:mstange]]

Attachment: MozReview Request: Bug 1068674 - Don't turn off e10s if hardware acceleration is disabled. r?jimm

Review commit: https://reviewboard.mozilla.org/r/41527/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/41527/

Comment 8

[Markus Stange [:mstange]]

We can do this now. Bug 1187322 made BasicCompositor work on OS X in all scenarios, and it was uplifted to 45.

Comment 9

[Markus Stange [:mstange]]

After looking at the patch that added this code in bug 1068199 I realized that I should also remove the nsIGfxInfo.h include.

Comment 10

[Jim Mathies [:jimm]]

Comment on attachment 8733034 [details]
MozReview Request: Bug 1068674 - Don't turn off e10s if hardware acceleration is disabled. r?jimm

Ok!

Comment 11

[Markus Stange [:mstange]]

Oh, I found more stuff to remove.

Comment 12

[Markus Stange [:mstange]]

Comment on attachment 8733034 [details]
MozReview Request: Bug 1068674 - Don't turn off e10s if hardware acceleration is disabled. r?jimm

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/41527/diff/1-2/

Comment 13

[Jim Mathies [:jimm]]

Comment on attachment 8733034 [details]
MozReview Request: Bug 1068674 - Don't turn off e10s if hardware acceleration is disabled. r?jimm

https://reviewboard.mozilla.org/r/41527/#review38235

Comment 14

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4c254ac91a0e

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/4c254ac91a0e

Comment 16

[:Felipe Gomes (needinfo for replies!)]

Does this need to be uplifted to 47?

Comment 17

[Jim Mathies [:jimm]]

Yes looks like it should, marcus?

Comment 18

[Markus Stange [:mstange]]

I agree.

Comment 19

[Markus Stange [:mstange]]

Comment on attachment 8733034 [details]
MozReview Request: Bug 1068674 - Don't turn off e10s if hardware acceleration is disabled. r?jimm

Approval Request Comment
[Feature/regressing bug #]: BasicCompositor + e10s. BasicCompositor wasn't working on Mac until bug 1187322 fixed it, and BasicCompositor is a requirement for e10s with hardware acceleration off. Now that BasicCompositor works on Mac, we can enable e10s on Mac even if hardware acceleration is off.
[User impact if declined]: no e10s for people who disabled hardware accel
[Describe test coverage new/current, TreeHerder]: the tests we run for e10s Mac run with hardware acceleration enabled, so we're not testing this particular configuration
[Risks and why]: very low, BasicCompositor on Mac is already used for non-e10s, and on all other platforms BasicCompositor is already used for e10s.
[String/UUID change made/needed]: none

Comment 20

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8733034 [details]
MozReview Request: Bug 1068674 - Don't turn off e10s if hardware acceleration is disabled. r?jimm

This has been on Nightly for 2 weeks and the fix increases e10s eligible end-users, Aurora47+

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/0aae99c5315c