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

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Andrew McCreight [:mccr8]]

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.

Comment 1

[Andrew McCreight [:mccr8]]

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

Comment 2

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Patch

Comment 3

[Andrew McCreight [:mccr8]]

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!

Comment 4

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Review ping

Comment 5

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Patch

Unbitrotted.

Comment 6

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 588053 [details] [diff] [review]
Patch

Andrew, want to steal this review?

Comment 7

[Andrew McCreight [:mccr8]]

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?

Comment 8

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Yes, it will make him sad.

Comment 9

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

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

Comment 10

[Andrew McCreight [:mccr8]]

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

Comment 11

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

If you could pick this up and run with it it would be greatly appreciated.

Comment 12

[Andrew McCreight [:mccr8]]

Attachment: add xpc::DumpJSHeap(), call it when doing a CC dump

Comment 13

[Andrew McCreight [:mccr8]]

Thanks for the suggestion, Ms2ger, it works a lot better than various hacky things I tried before to make this work!

Comment 14

[Andrew McCreight [:mccr8]]

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.

Comment 15

[Andrew McCreight [:mccr8]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8084cedbba17

Comment 16

[Andrew McCreight [:mccr8]]

https://hg.mozilla.org/mozilla-central/rev/8084cedbba17