Closed
Bug 765065
Opened 11 years ago
Closed 11 years ago
Annotation for crash reports: "Are we GCing?"
Categories
(Core :: XPConnect, enhancement)
Core
XPConnect
Tracking
()
RESOLVED
FIXED
mozilla17
People
(Reporter: jruderman, Assigned: billm)
Details
Attachments
(1 file)
9.56 KB,
patch
|
benjamin
:
review+
lsblakk
:
approval-mozilla-aurora+
lsblakk
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
This would allow us to gather statistics on GC crashes more easily -- and even if stack walking fails. (For bonus points, say whether it's a compartment / multi-compartment / global GC)
Comment 1•11 years ago
|
||
This is a little bit of a PITA, because only the JS engine knows if we're GCing, right? And the JS engine doesn't know how to talk to Breakpad directly. Do we have a callback that happens when we enter/leave GC where Gecko could add this annotation?
Assignee | ||
Comment 2•11 years ago
|
||
Yeah, xpconnect already knows when we're in a GC, and it can talk to breakpad.
Comment 3•11 years ago
|
||
Great!
Assignee: general → nobody
Component: JavaScript Engine → XPConnect
QA Contact: general → xpconnect
Comment 4•11 years ago
|
||
We can either dump it in the xpconnect callback (http://mxr.mozilla.org/mozilla-central/source/js/xpconnect/src/XPCJSRuntime.cpp#583) or register a special callback (http://mxr.mozilla.org/mozilla-central/source/js/xpconnect/src/nsXPConnect.cpp?force=1#2299) to do so, which is conceptually a bit cleaner.
Assignee | ||
Comment 5•11 years ago
|
||
This seems to do the job, at least on Linux.
Comment 6•11 years ago
|
||
Comment on attachment 651060 [details] [diff] [review] patch Remove the stray printf in CrashReporter::SetGarbageCollecting. I'm torn on whether we should be writing "IsGarbageCollecting=0" ever; perhaps it would be better to only write that field if we are garbage collecting?
Attachment #651060 -
Flags: review?(benjamin) → review+
Assignee | ||
Comment 7•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/dad98b444955 I changed it so the field is only written if it's 1.
Assignee | ||
Comment 8•11 years ago
|
||
Comment on attachment 651060 [details] [diff] [review] patch Just a note about this request: This patch doesn't fix any bugs. It just adds data to crashreports that make it easier to see which crashes are GC-related. Given that incremental GC landed in FF16, it would be really useful to be able to compare crash rates between 15 and 16. That's why I'm requesting beta approval. [Approval Request Comment] Bug caused by (feature/regressing bug #): None. User impact if declined: None. Testing completed (on m-c, etc.): On m-c. Risk to taking this patch (and alternatives if risky): Very low. String or UUID changes made by this patch: None.
Attachment #651060 -
Flags: approval-mozilla-beta?
Attachment #651060 -
Flags: approval-mozilla-aurora?
Comment 9•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/dad98b444955
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
Comment 10•11 years ago
|
||
Comment on attachment 651060 [details] [diff] [review] patch Approving for branches so we're able to get better crash data.
Attachment #651060 -
Flags: approval-mozilla-beta?
Attachment #651060 -
Flags: approval-mozilla-beta+
Attachment #651060 -
Flags: approval-mozilla-aurora?
Attachment #651060 -
Flags: approval-mozilla-aurora+
Assignee | ||
Comment 11•11 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/08e55f351f6d https://hg.mozilla.org/releases/mozilla-beta/rev/49888a27cff0
Updated•11 years ago
|
status-firefox15:
--- → fixed
status-firefox16:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•