852010 - Add ability to dump all strings as part of an about:memory dump

Status: RESOLVED WONTFIX

(Toolkit :: about:memory, defect)

Description

[Justin Lebar (not reading bugmail)]

We keep having problems with strings taking up a lot of memory on b2g.  I suggested in bug 850893 comment 6 that we should add the ability to dump them.

For each string, it would be helpful to know

 * which compartment it lives in, and
 * whether it is garbage (i.e., whether it will be collected by the next gc).

Since strings can contain private information, we'd have to be a bit careful about this.

Comment 1

[Nicholas Nethercote [inactive]]

I have an in-progress patch.  I'm assuming that we want strings from worker
runtimes as well as the main JS runtime.  In my patch I have an nsIStringDumper
interface which is similar to nsIMemoryMultiReporter, and I register string 
dumpers with the memory reporter manager.  Nothing too difficult so far.

But it gets complicated with workers.  I started cargo-culting the memory
reporting code for workers but I'm not sure if it's safe to do so if we have
both a memory reporter and a string dumper for a worker -- I'm a bit unclear
about the possible interactions between the two.

And having a general purpose string dumper interface feels like overkill for 
something that will only be used for (a) the main JS runtime, and (b) each 
worker.  I'm wondering if I can somehow piggyback the string dumping onto the 
memory reporters for (a) and (b), but I can't see how ATM.

Comment 2

[Ben Turner (not reading bugmail, use the needinfo flag!)]

I would really, really hate to add another BlockAndDoSomething to workers... Can we make the JS string dumping just be part of the normal memory report for JSRuntimes?

Comment 3

[Justin Lebar (not reading bugmail)]

(In reply to ben turner [:bent] from comment #2)
> I would really, really hate to add another BlockAndDoSomething to workers...
> Can we make the JS string dumping just be part of the normal memory report
> for JSRuntimes?

I'm not sure I understand how this would work.

We don't want to pass all of the strings through the memory reporter itself; we want to pass in an fd and have the memory reporter write the strings there (right?).

I guess we'd add a new interface nsIJSRuntimeStringsDumper and, when we want to do a memory report, we'd iterate over all our memory reporters and multi-reporters, try to QI to this, and then call the function if QI succeeds?

Comment 4

[Justin Lebar (not reading bugmail)]

I definitely want this for workers too, but I'd be happy if we did that separately.  We're hurting for the main-runtime functionality here in bug 851626.

Comment 5

[Justin Lebar (not reading bugmail)]

Nick, do you have time to unrot your old patch in the next few days?  Otherwise mikeh or I might take a stab at this.

Comment 6

[Justin Lebar (not reading bugmail)]

(Getting this to work in workers is not essential right now from my POV because these long strings are more often than not data: URIs for images, and those primarily appear only in the main runtime.)

Comment 7

[Mike Habicher [:mikeh] (high bugzilla latency)]

In the interests of expediency, I tried bumping the buffer[] from 32 to 1024 bytes. Turns out the 9114-byte PNG images have a LOT of (non-identifying) Adobe header in XML. Trying again with buffer[8192].

(There are also some 44737-byte JPEG images, but although I managed to decode the data: strings into something with a proper JFIF header, nothing would open them either.)

Comment 8

[Mike Habicher [:mikeh] (high bugzilla latency)]

Come to think of it, assuming we store enough of the image data in the about:memory strings, perhaps the about:memory viewer could display the images itself! :)

Comment 9

[Justin Lebar (not reading bugmail)]

(In reply to Mike Habicher [:mikeh] from comment #8)
> Come to think of it, assuming we store enough of the image data in the
> about:memory strings, perhaps the about:memory viewer could display the
> images itself! :)

Oh, that actually...might work.

I'd resisted putting the whole string in the proper about:memory dump because about:memory isn't the only consumer of this data -- areweslimyet.com also uses it.  But if it's just a matter of truncating versus not truncating, we can make that a flag when dumping the strings...

Comment 10

[Mike Habicher [:mikeh] (high bugzilla latency)]

Where does the about:memory chrome live?

Comment 11

[Justin Lebar (not reading bugmail)]

toolkit/components/aboutmemory/content

Comment 12

[Nicholas Nethercote [inactive]]

And the string memory reporting code lives in js/src/jsmemorymetrics.cpp, specifically in the function StatsCellCallback(), under the JSTRACE_STRING case.

Comment 13

[Nicholas Nethercote [inactive]]

I wonder if we still need this.  Bug 893222 changed the string reporting in about:memory so that both (a) large strings and (b) small strings that are duplicated many times get explicit mentions.  That doesn't say anything about whether the string is garbage, but getting that is hard anyway.  Is the current situation good enough?

Comment 14

[Nicholas Nethercote [inactive]]

Time for an executive decision.

Comment 15

[Andrew McCreight [:mccr8]]

Yeah, I think the current state is fine.  You get some kind of view of bad things from about:memory, like you said, and it is easy enough to pull a JS heap dump off the device if you want a more detailed view.