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

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
6 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

6 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

6 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

6 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

6 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

6 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

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

Updated

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

Comment 15

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

Comment 16

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