Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Annotation for crash reports: "Are we GCing?"

RESOLVED FIXED in Firefox 15

Status

()

Core
XPConnect
--
enhancement
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Jesse Ruderman, Assigned: billm)

Tracking

Trunk
mozilla17
Points:
---

Firefox Tracking Flags

(firefox15 fixed, firefox16 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
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)
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

5 years ago
Yeah, xpconnect already knows when we're in a GC, and it can talk to breakpad.
Great!
Assignee: general → nobody
Component: JavaScript Engine → XPConnect
QA Contact: general → xpconnect

Comment 4

5 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

5 years ago
Created attachment 651060 [details] [diff] [review]
patch

This seems to do the job, at least on Linux.
Assignee: nobody → wmccloskey
Status: NEW → ASSIGNED
Attachment #651060 - Flags: review?(benjamin)
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

5 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

5 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?
https://hg.mozilla.org/mozilla-central/rev/dad98b444955
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
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

5 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/08e55f351f6d
https://hg.mozilla.org/releases/mozilla-beta/rev/49888a27cff0

Updated

5 years ago
status-firefox15: --- → fixed
status-firefox16: --- → fixed
You need to log in before you can comment on or make changes to this bug.