Closed
Bug 1475010
Opened 6 years ago
Closed 6 years ago
SharedSurfaceANGLE not getting a KeyedMutex
Categories
(Core :: Graphics: CanvasWebGL, defect)
Tracking
()
RESOLVED
FIXED
mozilla63
People
(Reporter: jgilbert, Assigned: jgilbert)
References
Details
(Keywords: regression, Whiteboard: gfx-noted)
Attachments
(2 files)
46 bytes,
text/x-phabricator-request
|
kvark
:
review+
jrmuizel
:
review+
RyanVM
:
approval-mozilla-beta+
RyanVM
:
approval-mozilla-esr60+
|
Details | Review |
46 bytes,
text/x-phabricator-request
|
kvark
:
review+
|
Details | Review |
This causes us to glFinish every frame.
Assignee | ||
Updated•6 years ago
|
Whiteboard: gfx-noted
Assignee | ||
Comment 1•6 years ago
|
||
Oops, we dropped ANGLE_ENABLE_KEYEDMUTEX at some point. I'll make sure we catch this in the future too.
Assignee | ||
Comment 2•6 years ago
|
||
Without KeyedMutex we use glFinish, which is bad. We accidentally stopped asking ANGLE for KeyedMutexes during some build changes.
Updated•6 years ago
|
Comment 4•6 years ago
|
||
Comment on attachment 8991494 [details] Build ANGLE with ANGLE_ENABLE_KEYEDMUTEX. Dzmitry Malyshau [:kvark] has approved the revision. https://phabricator.services.mozilla.com/D2084
Attachment #8991494 -
Flags: review+
Comment 5•6 years ago
|
||
Comment on attachment 8991494 [details] Build ANGLE with ANGLE_ENABLE_KEYEDMUTEX. Jeff Muizelaar [:jrmuizel] has approved the revision. https://phabricator.services.mozilla.com/D2084
Attachment #8991494 -
Flags: review+
Comment 6•6 years ago
|
||
checkin-needed?
Comment 7•6 years ago
|
||
Thanks, I triggered landing. It should land once trees reopen.
Pushed by kgupta@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/5e2c3f2fb8a0 Build ANGLE with ANGLE_ENABLE_KEYEDMUTEX. r=jrmuizel,kvark
Comment 9•6 years ago
|
||
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #3) > Regression from bug 1440849, I believe. https://hg.mozilla.org/mozilla-central/rev/f63d5bd89d78#l1.200
status-firefox61:
--- → affected
status-firefox62:
--- → affected
status-firefox63:
--- → affected
status-firefox-esr52:
--- → unaffected
status-firefox-esr60:
--- → affected
Keywords: regression
Assignee | ||
Comment 10•6 years ago
|
||
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #7) > Thanks, I triggered landing. It should land once trees reopen. Yo, that's not the final patch I wanted to land. Please let me land it myself in the future.
Flags: needinfo?(bugmail)
Comment 11•6 years ago
|
||
Sorry. You weren't around on IRC and also were non-responsive to my previous email on this topic. Want me to back it out?
Flags: needinfo?(bugmail)
Assignee | ||
Comment 12•6 years ago
|
||
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #11) > Sorry. You weren't around on IRC and also were non-responsive to my previous > email on this topic. Want me to back it out? I can diff it, but I don't see an email from you.
Comment 13•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/5e2c3f2fb8a0
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Assignee | ||
Comment 14•6 years ago
|
||
Disableable via env var.
Comment 15•6 years ago
|
||
Comment on attachment 8993865 [details] Bug 1475010 - Also, assert in DEBUG if we don't get a KeyedMutex. - r=kvark Dzmitry Malyshau [:kvark] has approved the revision. https://phabricator.services.mozilla.com/D2276
Attachment #8993865 -
Flags: review+
Comment 16•6 years ago
|
||
Pushed by jgilbert@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/dade23c56875 Also, assert in DEBUG if we don't get a KeyedMutex. - r=kvark
Comment 17•6 years ago
|
||
Is there a user impact here which justifies backport consideration, or can this ride the trains?
Flags: needinfo?(jgilbert)
Comment 18•6 years ago
|
||
Yes. This has a noticeable impact on WebGL performance.
Comment 19•6 years ago
|
||
Bug 1461336 is a case where a web developer was seeing performance problems with games in Firefox and not Chrome that was said to be helped by this (though I don't think anybody has verified that it helps yet).
Comment 20•6 years ago
|
||
In that case, sounds like Beta and ESR60 approval requests are in order :)
Comment 21•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/dade23c56875
Assignee | ||
Comment 23•6 years ago
|
||
Comment on attachment 8991494 [details] Build ANGLE with ANGLE_ENABLE_KEYEDMUTEX. [Approval Request Comment] If this is not a sec:{high,crit} bug, please state case for ESR consideration: Simple low-risk perf-regression fix. User impact if declined: Degraded WebGL perf on Windows. Fix Landed on Version: 63 Risk to taking this patch (and alternatives if risky): Low, especially after baking on Nightly63. String or UUID changes made by this patch: none See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info. Approval Request Comment [Feature/Bug causing the regression]: bug 1440849 (angle-60) [User impact if declined]: Degraded WebGL perf on Windows. [Is this code covered by automated tests?]: Test included. [Has the fix been verified in Nightly?]: Test passes on CI. [Needs manual test from QE? If yes, steps to reproduce]: no [List of other uplifts needed for the feature/fix]: beta62, esr60 [Is the change risky?]: Low risk. [Why is the change risky/not risky?]: Low risk because it's a flag we always used prior to the ANGLE update in 60, so it's not likely to have broken even for the differing demographics in Beta/ESR. [String changes made/needed]: none
Flags: needinfo?(jgilbert)
Attachment #8991494 -
Flags: approval-mozilla-esr60?
Attachment #8991494 -
Flags: approval-mozilla-beta?
Comment 24•6 years ago
|
||
Comment on attachment 8991494 [details]
Build ANGLE with ANGLE_ENABLE_KEYEDMUTEX.
Restores us to the same behavior we had prior to the ANGLE-60 update and wins us back some lost performance. Approved for 62.0b17 and ESR 60.2.
Attachment #8991494 -
Flags: approval-mozilla-esr60?
Attachment #8991494 -
Flags: approval-mozilla-esr60+
Attachment #8991494 -
Flags: approval-mozilla-beta?
Attachment #8991494 -
Flags: approval-mozilla-beta+
Comment 25•6 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/00a7708e3cff
Comment 26•6 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-esr60/rev/d3ff95b9faa1
You need to log in
before you can comment on or make changes to this bug.
Description
•