Enable e10s when hardware acceleration is disabled

RESOLVED FIXED in Firefox 47

Status

()

Core
Graphics: Layers
P2
normal
RESOLVED FIXED
4 years ago
2 years ago

People

(Reporter: jimm, Assigned: mstange)

Tracking

unspecified
mozilla48
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(e10s+, firefox46 wontfix, firefox47 fixed, firefox48 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

4 years ago
+++ 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.
(Reporter)

Updated

4 years ago
Assignee: nobody → jmathies
(Reporter)

Comment 1

4 years ago
bug 1026521 was set to m4, so this is an m4.
tracking-e10s: m3+ → m4+
(Reporter)

Updated

4 years ago
Summary: Enable e10s when hardware acceleration is disabled → Enable e10s when hardware acceleration is disabled on Windows
(Reporter)

Comment 2

4 years ago
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.
tracking-e10s: m4+ → ?
tracking-e10s: ? → +
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
(Assignee)

Comment 7

2 years ago
Created attachment 8733034 [details]
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/
Attachment #8733034 - Flags: review?(jmathies)
(Assignee)

Comment 8

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

Comment 9

2 years ago
After looking at the patch that added this code in bug 1068199 I realized that I should also remove the nsIGfxInfo.h include.
(Reporter)

Comment 10

2 years ago
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+
(Reporter)

Updated

2 years ago
Assignee: gwright → mstange
(Assignee)

Comment 11

2 years ago
Oh, I found more stuff to remove.
(Assignee)

Comment 12

2 years ago
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)
(Reporter)

Updated

2 years ago
Attachment #8733034 - Flags: review?(jmathies) → review+
(Reporter)

Comment 13

2 years ago
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 15

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/4c254ac91a0e
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox48: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Does this need to be uplifted to 47?
Flags: needinfo?(jmathies)
(Reporter)

Comment 17

2 years ago
Yes looks like it should, marcus?
Flags: needinfo?(jmathies) → needinfo?(mstange)
(Assignee)

Comment 18

2 years ago
I agree.
Flags: needinfo?(mstange)
(Assignee)

Comment 19

2 years ago
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?
status-firefox46: --- → wontfix
status-firefox47: --- → affected
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+

Comment 21

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/0aae99c5315c
status-firefox47: affected → fixed
No longer depends on: 1263200
You need to log in before you can comment on or make changes to this bug.