DWrite contention on worker threads is too high

RESOLVED FIXED in Firefox 69

Status

()

defect
P3
normal
RESOLVED FIXED
3 months ago
20 days ago

People

(Reporter: jrmuizel, Assigned: lsalzman)

Tracking

(Blocks 1 bug)

unspecified
mozilla69
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox69 fixed)

Details

Attachments

(2 attachments)

Blocks: 1495170, wr-67
Priority: -- → P3

I should post a profile of this.

Flags: needinfo?(jmuizelaar)

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!

See Also: → 1533545
Blocks: wr-68
No longer blocks: wr-67
Flags: needinfo?(jmuizelaar)

Comment 4

Last month
Pushed by lsalzman@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6440419b9d33
disable Skia's global DWrite lock on Windows 10. r=jrmuizel

Comment 5

Last month
bugherder
Status: NEW → RESOLVED
Closed: Last month
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
Assignee: nobody → lsalzman

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

Assignee

Updated

25 days ago
Regressions: 1552687
Assignee

Comment 8

25 days ago

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:
Attachment #9066795 - Flags: approval-mozilla-beta?

Comment 9

25 days ago
Backout by lsalzman@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7ed7938b3797
Backed out changeset 6440419b9d33 . r=jrmuizel

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...

Flags: needinfo?(aryx.bugmail)
Attachment #9066795 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

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

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

Flags: needinfo?(aryx.bugmail)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: mozilla68 → ---

Comment 13

23 days ago
Pushed by lsalzman@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f7878bcedd30
disable Skia's global DWrite lock on Windows 10. r=jrmuizel
Assignee

Comment 14

23 days ago

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.

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

Blocks: 1483436
Assignee

Comment 16

23 days ago

(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

22 days ago
bugherder
Status: REOPENED → RESOLVED
Closed: Last month22 days ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla69

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

Attachment #9066795 - Flags: approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.