1533546 - DWrite contention on worker threads is too high

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect, P3)

Description

[Jeff Muizelaar [:jrmuizel]]

This shows up when profiling https://static.mozilla.com/moco/en-US/images/mozilla_eoy_2013_EN.svg

Comment 1

[Jeff Muizelaar [:jrmuizel]]

I should post a profile of this.

Comment 2

[Nicolas Silva [:nical]]

Related: bug 1533545, limiting the amount of worker threads would probably help here as well.

I should post a profile of this.

Would be wonderful!

Comment 3

[Lee Salzman [:lsalzman]]

Attachment: Bug 1533546 - disable Skia's global DWrite lock on Windows 10. r?jrmuizel

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6440419b9d33
disable Skia's global DWrite lock on Windows 10. r=jrmuizel

Comment 5

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/6440419b9d33

Comment 6

[Ionuț Goldan [:igoldan]]

Noticed some perf improvements:

== Change summary for alert #20967 (as of Thu, 16 May 2019 10:47:40 GMT) ==

Improvements:

9% tabswitch windows7-32-shippable opt e10s stylo 12.08 -> 11.02

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=20967

Comment 7

[Lee Salzman [:lsalzman]]

Attachment: Backed out changeset 6440419b9d33 (Bug 1533546). r?jrmuizel

Comment 8

[Lee Salzman [:lsalzman]]

Comment on attachment 9066795 [details]
Backed out changeset 6440419b9d33 (Bug 1533546). r?jrmuizel

Beta/Release Uplift Approval Request

• User impact if declined: Frequent crashes for Windows users as observed in bug 1552687.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This backs out a patch that was an experimental performance change that wasn't intended to make it into 68 beta in the first place. It just somehow got landed in 68 due to the weird timing of the train delay. Incorporating this backout just puts us back to where we started at, without the performance change that is regressing stability noticeably.
• String changes made/needed:

Comment 9

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7ed7938b3797
Backed out changeset 6440419b9d33 . r=jrmuizel

Comment 10

[Julien Cristau [:jcristau]]

Comment on attachment 9066795 [details]
Backed out changeset 6440419b9d33 (Bug 1533546). r?jrmuizel

backout to fix a stability regression, approved for 68.0b4

Sebastian, heads-up because this won't show up on the uplift query and I'm not sure how to handle the flags...

Comment 11

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

You can use the whiteboard and set it to [checkin-needed-beta].

https://hg.mozilla.org/releases/mozilla-beta/rev/290e613b8b2211e41209565318a63cafe0671082

Comment 12

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/7ed7938b3797

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f7878bcedd30
disable Skia's global DWrite lock on Windows 10. r=jrmuizel

Comment 14

[Lee Salzman [:lsalzman]]

Bug 1552687 should fix the underlying race condition that this bug aggravated. I relanded the original patch just so that we can purposely kick up potential crashes, and hopefully not actually crash anymore at all to verify that bug 1552687 fixed it. If it starts getting bad again, we can reland the backout. The important thing is that we meanwhile have it backed out of beta while we are trying to resolve and collect data on the issue.

Comment 15

[Ryan Hunt [:rhunt]]

If I understand this correctly, this would also really help us out with bug 1483436.

Comment 16

[Lee Salzman [:lsalzman]]

(In reply to Ryan Hunt [:rhunt] from comment #15)

If I understand this correctly, this would also really help us out with bug 1483436.

It should, yes, if we can manage to resolve the race condition that is blocking us on this.

Comment 17

[Andrei Ciure[:aciure]]

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

Comment 18

[Julien Cristau [:jcristau]]

Comment on attachment 9066795 [details]
Backed out changeset 6440419b9d33 (Bug 1533546). r?jrmuizel

clearing approval on the backout so this doesn't show up on uplift queries