Closed Bug 1068674 Opened 5 years ago Closed 4 years ago

Enable e10s when hardware acceleration is disabled

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
e10s + ---
firefox46 --- wontfix
firefox47 --- fixed
firefox48 --- fixed

People

(Reporter: jimm, Assigned: mstange)

References

Details

Attachments

(1 file)

+++ 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.
Assignee: nobody → jmathies
bug 1026521 was set to m4, so this is an m4.
Summary: Enable e10s when hardware acceleration is disabled → Enable e10s when hardware acceleration is disabled on Windows
argh, sorry, wrong bug.
Summary: Enable e10s when hardware acceleration is disabled on Windows → Enable e10s when hardware acceleration is disabled
Assignee: jmathies → gwright
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.
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.
Andrew, as I remember, you don't use HW accel. Why was that again?
Flags: needinfo?(continuation)
(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.
Flags: needinfo?(continuation)
Priority: -- → P2
We can do this now. Bug 1187322 made BasicCompositor work on OS X in all scenarios, and it was uplifted to 45.
After looking at the patch that added this code in bug 1068199 I realized that I should also remove the nsIGfxInfo.h include.
Comment on attachment 8733034 [details]
MozReview Request: Bug 1068674 - Don't turn off e10s if hardware acceleration is disabled. r?jimm

Ok!
Attachment #8733034 - Flags: review?(jmathies) → review+
Assignee: gwright → mstange
Oh, I found more stuff to remove.
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/
Attachment #8733034 - Flags: review+ → review?(jmathies)
Attachment #8733034 - Flags: review?(jmathies) → review+
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
https://hg.mozilla.org/mozilla-central/rev/4c254ac91a0e
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Does this need to be uplifted to 47?
Flags: needinfo?(jmathies)
Yes looks like it should, marcus?
Flags: needinfo?(jmathies) → needinfo?(mstange)
I agree.
Flags: needinfo?(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
Attachment #8733034 - Flags: approval-mozilla-aurora?
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+
Attachment #8733034 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.