Closed
Bug 1371435
Opened 7 years ago
Closed 7 years ago
Continue to use CRITICAL_SECTION for Windows Mutex impl on Nightly
Categories
(Core :: XPCOM, enhancement)
Tracking
()
RESOLVED
FIXED
mozilla56
Tracking | Status | |
---|---|---|
firefox56 | --- | fixed |
People
(Reporter: bugzilla, Assigned: erahm)
References
Details
Attachments
(1 file)
4.37 KB,
patch
|
froydnj
:
review+
|
Details | Diff | Splinter Review |
CRITICAL_SECTIONS provide a lot of really, really useful debugging info. In bug 1344827 I modified our mutex implementation to support that debug info on Nightly. Now that bug 1364624 has switched everything to SRWLOCKs, we don't have access to that rich debug info anymore. I'd like to propose that we switch back to CRITICAL_SECTIONS with debug info for Nightlies (both debug and release), but continue to use SRWLOCKs on Beta and Release.
Assignee | ||
Comment 2•7 years ago
|
||
(In reply to Aaron Klotz [:aklotz] (a11y work receiving priority right now, please send interceptor reviews to dmajor or handyman) from comment #1) > Eric, thoughts? I'm mostly okay with this, but am a little concerned about having such a different implementation b/w nightly and beta.
Flags: needinfo?(erahm)
Assignee | ||
Comment 3•7 years ago
|
||
We discussed this and think the debug info outweighs the small win in lock size and hypothetical (but unconfirmed) perf improvements.
Assignee | ||
Comment 4•7 years ago
|
||
CRITICAL_SECTIONs have useful debug info that we'd like to keep around at least on nightly, rather than having diverging signatures on crash stats we'd like to just keep a unified implementation. As we didn't see significant perf improvements after landing we're going to just back this out.
Attachment #8881944 -
Flags: review?(nfroyd)
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → erahm
Status: NEW → ASSIGNED
Comment 5•7 years ago
|
||
Comment on attachment 8881944 [details] [diff] [review] Backed out changeset 5b6d169feb92 Review of attachment 8881944 [details] [diff] [review]: ----------------------------------------------------------------- \o/
Attachment #8881944 -
Flags: review?(nfroyd) → review+
Assignee | ||
Comment 6•7 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/8c69b1e4d5a62688ac69657f926683adc04e8312 Bug 1371435 - Backed out changeset 5b6d169feb92. r=froydnj
Comment 7•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/8c69b1e4d5a6
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in
before you can comment on or make changes to this bug.
Description
•