1475010 - SharedSurfaceANGLE not getting a KeyedMutex

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

[Kelsey Gilbert [:jgilbert]]

This causes us to glFinish every frame.

Comment 1

[Kelsey Gilbert [:jgilbert]]

Oops, we dropped ANGLE_ENABLE_KEYEDMUTEX at some point.

I'll make sure we catch this in the future too.

Comment 2

[Kelsey Gilbert [:jgilbert]]

Attachment: Build ANGLE with ANGLE_ENABLE_KEYEDMUTEX.

Without KeyedMutex we use glFinish, which is bad.
We accidentally stopped asking ANGLE for KeyedMutexes during some build changes.

Comment 3

[Kartikaya Gupta (email:[e-mail])]

Regression from bug 1440849, I believe.

Comment 4

[Phabricator Automation]

Comment on attachment 8991494 [details]
Build ANGLE with ANGLE_ENABLE_KEYEDMUTEX.

Dzmitry Malyshau [:kvark] has approved the revision.

https://phabricator.services.mozilla.com/D2084

Comment 5

[Phabricator Automation]

Comment on attachment 8991494 [details]
Build ANGLE with ANGLE_ENABLE_KEYEDMUTEX.

Jeff Muizelaar [:jrmuizel] has approved the revision.

https://phabricator.services.mozilla.com/D2084

Comment 6

[Darkspirit]

checkin-needed?

Comment 7

[Kartikaya Gupta (email:[e-mail])]

Thanks, I triggered landing. It should land once trees reopen.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5e2c3f2fb8a0
Build ANGLE with ANGLE_ENABLE_KEYEDMUTEX. r=jrmuizel,kvark

Comment 9

[Darkspirit]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #3)
> Regression from bug 1440849, I believe.

https://hg.mozilla.org/mozilla-central/rev/f63d5bd89d78#l1.200

Comment 10

[Kelsey Gilbert [:jgilbert]]

(In reply to Kartikaya Gupta (email:[e-mail]) 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.

Comment 11

[Kartikaya Gupta (email:[e-mail])]

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?

Comment 12

[Kelsey Gilbert [:jgilbert]]

(In reply to Kartikaya Gupta (email:[e-mail]) 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

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/5e2c3f2fb8a0

Comment 14

[Kelsey Gilbert [:jgilbert]]

Attachment: Bug 1475010 - Also, assert in DEBUG if we don't get a KeyedMutex. - r=kvark

Disableable via env var.

Comment 15

[Phabricator Automation]

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

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dade23c56875
Also, assert in DEBUG if we don't get a KeyedMutex. - r=kvark

Comment 17

[Ryan VanderMeulen [:RyanVM]]

Is there a user impact here which justifies backport consideration, or can this ride the trains?

Comment 18

[Jeff Muizelaar [:jrmuizel]]

Yes. This has a noticeable impact on WebGL performance.

Comment 19

[Andrew McCreight (out until 2026) [:mccr8]]

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

[Ryan VanderMeulen [:RyanVM]]

In that case, sounds like Beta and ESR60 approval requests are in order :)

Comment 21

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/dade23c56875

Comment 22

[Liz Henry (:lizzard) (relman/hg->git project)]

Emailing jgilbert with a reminder.

Comment 23

[Kelsey Gilbert [:jgilbert]]

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

Comment 24

[Ryan VanderMeulen [:RyanVM]]

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.

Comment 25

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/00a7708e3cff

Comment 26

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr60/rev/d3ff95b9faa1