add way to grab JS heap dump at the same time as the CC dump

RESOLVED FIXED in mozilla13

Status

()

Core
XPCOM
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: mccr8, Assigned: mccr8)

Tracking

(Blocks: 1 bug)

Trunk
mozilla13
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [MemShrink:P2])

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

6 years ago
When the cycle collector decides an object is live due to a marked JS object, it is useful to be able to see why the GC decided the marked JS object is alive.  We can do this by taking a JS heap dump at the same time as the CC heap dump.
(Assignee)

Updated

6 years ago
Assignee: nobody → continuation
Blocks: 695348
Depends on: 680482
(Assignee)

Comment 1

6 years ago
Created attachment 568475 [details] [diff] [review]
WIP that always dumps the GC heap when you CC

There's something weird about this patch.  It works fine for normal CCs, but hangs the browser when you manually invoke a CC using the button in about:memory.

Another problem with this patch is that it dumps the GC heap every time there is a CC, where we really only want it when we do a CC dump at the same time.
Whiteboard: [MemShrink] → [MemShrink:P2]
Created attachment 576507 [details] [diff] [review]
Patch
Attachment #568475 - Attachment is obsolete: true
Attachment #576507 - Flags: feedback?(continuation)
(Assignee)

Comment 3

6 years ago
Comment on attachment 576507 [details] [diff] [review]
Patch

Looks reasonable to me.  It would be kind of nice to have a way to dump the CC graph without dumping a GC graph, but I guess either way people are going to have tons of log files to delete, and any solutions I can think of would be annoying.  Thanks for picking this up!
Attachment #576507 - Flags: feedback?(continuation) → feedback+
Attachment #576507 - Flags: review?(peterv)
Review ping
Created attachment 588053 [details] [diff] [review]
Patch

Unbitrotted.
Attachment #576507 - Attachment is obsolete: true
Attachment #576507 - Flags: review?(peterv)
Attachment #588053 - Flags: review?(peterv)
Comment on attachment 588053 [details] [diff] [review]
Patch

Andrew, want to steal this review?
Attachment #588053 - Flags: review?(continuation)
(Assignee)

Comment 7

5 years ago
Sure I can look at it.

At a glance here, your inclusion of xpcprivate in the cycle collector seems sketchy.  Is that going to make Ms2ger sad?
Yes, it will make him sad.
Yes. Just add a xpc::DumpGCHeap to xpcpublic.h.

And use nsXPConnect::GetXPConnect instead of do_GetService("@mozilla.org/js/xpc/ContextStack;1"); inside xpconnect
(Assignee)

Comment 10

5 years ago
Comment on attachment 588053 [details] [diff] [review]
Patch

Yeah, Ms2ger's idea sounds better.  I can look into this if you don't have the time, Kyle.  Or are on vacation. ;)
Attachment #588053 - Flags: review?(peterv)
Attachment #588053 - Flags: review?(continuation)
Attachment #588053 - Flags: review-
If you could pick this up and run with it it would be greatly appreciated.
(Assignee)

Comment 12

5 years ago
Created attachment 594011 [details] [diff] [review]
add xpc::DumpJSHeap(), call it when doing a CC dump
Attachment #588053 - Attachment is obsolete: true
(Assignee)

Comment 13

5 years ago
Thanks for the suggestion, Ms2ger, it works a lot better than various hacky things I tried before to make this work!
(Assignee)

Comment 14

5 years ago
Comment on attachment 594011 [details] [diff] [review]
add xpc::DumpJSHeap(), call it when doing a CC dump

bholley for the XPConnect changes, smaug for the cycle collector changes.

All of these things have to be wrapped in #ifdef DEBUG because js::DumpHeapComplete is likewise wrapped.

The ++gLogCounter had to be hoisted out of the sprintf to ensure it is always called with or without the DEBUG.

I pushed a build to try on Linux to see if it builds with opt.
Attachment #594011 - Flags: review?(bugs)
Attachment #594011 - Flags: review?(bobbyholley+bmo)
(Assignee)

Updated

5 years ago
Blocks: 723783
Attachment #594011 - Flags: review?(bobbyholley+bmo) → review+

Updated

5 years ago
Attachment #594011 - Flags: review?(bugs) → review+
(Assignee)

Comment 15

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/8084cedbba17
Target Milestone: --- → mozilla13
(Assignee)

Comment 16

5 years ago
https://hg.mozilla.org/mozilla-central/rev/8084cedbba17
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.