326225 - Provide javascript bloat tracking by file(line)

Status: RESOLVED INCOMPLETE

(Other Applications Graveyard :: Venkman JS Debugger, enhancement)

Description

[timeless]

This is a rough implementation. feedback is welcome.

the oom branches are there because i would like to be able to use this stuff
very near the edge of memory constraints if at all possible, we really do run
out of memory and being able to attach and get a dump like this would be good,
although practically speaking, that branch should really favor a FILE* instead
of trying to play nicely w/ xpcom.

i'm not sure if this code belongs in the jsd_xpc layer, or beneath it. I think
any consumer of jsd is welcome to implement its own version of this, possibly
much better and therefor shouldn't be burdened by my crude impl. :).

wrt implementing this code in js, that's pretty much a bad idea, although it is
theoretically possible, and any js that chooses to pass in a supportscstring
could easily use this code. the oom flavor is pretty much disastrous for js
handling, since you really don't want to change what objects exist while
enumerating them. probably the code that unlocks and calls out should actually
rewalk to reach the point where it had stopped enumerating to make sure that its
pointer is still valid, possibly remembering an 'object' number and trying to
use that if things go bad.

yes this patch doesn't bump the iid, i'll do that when i have a patch to commit,
otherwise it conflicts w/ various other changes I have for this file.

yes this patch includes a useless change to jsfun.h, i'll clone the silly code
into jsd_xpc when someone demands it. in the interim, it said what i needed to
say instead of making me invent a new macro....

reportSink and flags should be aReportSink/aFlags, and i should probably change
oci to aOCI or something (suggestions on names for the structure and callbacks
... welcome).

Comment 1

[timeless]

Attachment: implement size property and reportLiveObjects method

Comment 2

[timeless]

Attachment: testcase

required files:
xpcshell[binary] (working is recommended, but a hosed one like the one i have is ok as long as you make sure to initialize RegExp engine before you call load) - current directory
326225.js (this attachment) - current directory
nsError.js <http://viper.haque.net/~timeless/nsError.js> (you can substitute something else that's inefficient and wasteful if you like, it happened to be handy) - r:/ [if you can't do r:/, you'll need to hack 326225.js a bit]

files destroyed and created:
nsError-js.bloat - r:/ [if you can't do r:/, you'll need to hack 326225.js a bit]

Comment 3

[timeless]

Attachment: output

The output format is csv, if you spend a lot of time looking at this, you'll decide that nsError.js is very inefficient since it creates an expensive toString function for each call to analyze instead of having a class with the method that it could share among instances.

Comment 4

[timeless]

Attachment: add idl constants, support type names and pointer addresses

support for stack traces for allocation location can be added later. it'd just be a matter of jsd grabbing that information at construction time and storing it, and then allowing one of the formats to include traces in its dumps.

it occurs to me that we'll want to try to give at least classinfo names for xpconnect objects, possibly interfaces if we don't have classinfo. unfortunately classinfo is dangerous since it might call js, and well, i can't have code running while i'm doing this. i might alter jsd to do that stuff for me, we'll see.

Comment 5

[timeless]

Comment on attachment 211214 [details] [diff] [review]
add idl constants, support type names and pointer addresses

yes objectTypeNamesLength is unused, i couldn't find a way to make it help me out, i'll try to remember to remove it.

Comment 6

[Brendan Eich [:brendan]]

Comment on attachment 211214 [details] [diff] [review]
add idl constants, support type names and pointer addresses

Old patch, not likely to r+ as is, not sure this is needed. Want a jsd peer to r+ anyway.

/be

Comment 7

[:Gijs (he/him)]

Comment on attachment 211214 [details] [diff] [review]
add idl constants, support type names and pointer addresses

Timeless asked me to review this on IRC. I will try to do so tomorrow/wednesday. Setting r? to remind myself.

Comment 8

[timeless]

bc offered to convert attachment 211022 [details] to unit tests

Comment 9

[:Gijs (he/him)]

Comment on attachment 211214 [details] [diff] [review]
add idl constants, support type names and pointer addresses

I'm going to pass on reviewing this one. Christopher, could you have a look?

Comment 10

[(dormant account)]

Comment on attachment 211214 [details] [diff] [review]
add idl constants, support type names and pointer addresses

I can't r+ this, but this does seem like useful stuff.

I can't say I completely agree with how certain details of the patch are implemented, but it's a good start. I might even end up using some of this.

Comment 11

[BMO Automation]

Component is obsolete so resolving bugs as INCOMPLETE