722749 - add source information about where a JS object was allocated in DumpHeapComplete

Status: NEW

(Core :: JavaScript Engine, defect)

Description

[Jan Honza Odvarko [:Honza] (always need-info? me)]

This report is based on following thread:
https://groups.google.com/d/topic/mozilla.dev.platform/IT4SUHyiYeE/discussion

"@mozilla.org/cycle-collector-logger;1" is nice tool when hunting down memory leaks, but it's hard (mostly almost impossible at least in case of Firebug)
to identify an object in the log and match with related object in the
code.

Some more info in the log would be invaluable.

It could be: url and source line where the object is created or all existing properties of the object (especially primitive values, numbers, string props would be great).

It looks like this should be rather part of JS heap dump.

Honza

Comment 1

[Andrew McCreight (out of office until 8/21) [:mccr8]]

Luke had some ideas about leveraging Type Inference information for this.  Bill said he'd look into it.  I tried before, but I don't know TI stuff at all.

Comment 2

[Andrew McCreight (out of office until 8/21) [:mccr8]]

We could also turn this into a metabug for improvements to JSDumpHeapComplete.

Comment 3

[Andrew McCreight (out of office until 8/21) [:mccr8]]

Just making the summary more specific.

Comment 4

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Jason, I am just ccying you on this bug.
It's the one we have been talking about last week.

Honza

Comment 5

[Jan Honza Odvarko [:Honza] (always need-info? me)]

I am marking as firebug-p1 since this would really help to find memory leaks (e.g. Bug 730782)

Honza

Comment 6

[Jason Orendorff [:jorendorff]]

Information from bhackett on this:

1. TypeCompartment::AllocationSiteTable is a mapping from the locations of {} and [] expressions to TypeObjects. To make that data useful for this purpose, you'd have to reverse the table, then look up obj->getType(). Caveats:

  * This information only exists for Object and Array objects created via {} and
    [] expressions in scripts.

  * If the script gets collected by GC, this information is thrown away.

2. For objects created via 'new' expressions in scripts, obj->getType()->newScript has a pointer to the constructor's script.

  * This information only exists for Object objects created via the 'new' keyword.

  * You don't get the location of the 'new' expression itself. Instead you get
    the location of the constructor function.

Neither of those works for DOM objects. If we need that, we can do it in a special debug-only mode.

Exposing this information to the debugger is a smallish amount of work but I can't take it now.

Comment 7

[Jason Orendorff [:jorendorff]]

Honza, we mainly need to know whether or not it would be useful to expose the information we've already got (cases 1 and 2 above).

Comment 8

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to Jason Orendorff [:jorendorff] from comment #6)
> Information from bhackett on this:
> 
> 1. TypeCompartment::AllocationSiteTable is a mapping from the locations
So, 'location' means: URL and line number, correct?

> of
> {} and [] expressions to TypeObjects. To make that data useful for this
> purpose, you'd have to reverse the table, then look up obj->getType().
> Caveats:
> 
>   * This information only exists for Object and Array objects created via {}
> and
>     [] expressions in scripts.
> 
>   * If the script gets collected by GC, this information is thrown away.
This shouldn't be a problem as when looking for memory leaks - objects that are *not* collected by GC are actually important.

> 
> 2. For objects created via 'new' expressions in scripts,
> obj->getType()->newScript has a pointer to the constructor's script.
> 
>   * This information only exists for Object objects created via the 'new'
> keyword.
> 
>   * You don't get the location of the 'new' expression itself. Instead you
> get
>     the location of the constructor function.
'new' & constructor functions are usually on the same line so, I guess it's not a big deal.

> Neither of those works for DOM objects. If we need that, we can do it in a
> special debug-only mode.
DOM objects, specifically nsGenericElement(s) already displays list of attributes/values which allows to identify them pretty easily.

What about various kinds of wrappers (nsXPCWrappedJS, XPCWrappedNative, etc.) which are created automatically by XPC, would these also have the origin info?

> Exposing this information to the debugger is a smallish amount of work but I
> can't take it now.
I don't understand this note, how debugger is related here?

---

(In reply to Jason Orendorff [:jorendorff] from comment #7)
> Honza, we mainly need to know whether or not it would be useful to expose
> the information we've already got (cases 1 and 2 above).
Yes, I believe that this would be great progress!

Currently, I mostly see: "JS Object (Object)" without any glue what the object actually is so, having the origin info would solve that.

Honza

Comment 9

[Steve Fink [:sfink] [:s:]]

(In reply to Jan Honza Odvarko from comment #8)
> (In reply to Jason Orendorff [:jorendorff] from comment #6)
> > Information from bhackett on this:
> > 
> > 1. TypeCompartment::AllocationSiteTable is a mapping from the locations
> So, 'location' means: URL and line number, correct?

This is internal, so it's actually JSScript* and offset into its bytecode. But you can map to/from URL and line number.

> 
> > of
> > {} and [] expressions to TypeObjects. To make that data useful for this
> > purpose, you'd have to reverse the table, then look up obj->getType().
> > Caveats:
> > 
> >   * This information only exists for Object and Array objects created via {}
> > and
> >     [] expressions in scripts.
> > 
> >   * If the script gets collected by GC, this information is thrown away.
> This shouldn't be a problem as when looking for memory leaks - objects that
> are *not* collected by GC are actually important.

Assuming I understand correctly, then no, you still have a problem -- the script containing the allocation site could be GC'd even if the allocated object is not. But perhaps that's not a huge problem, since scripts for live tabs ordinarily won't go away.

Well, unless they're eval scripts. If you create a reachable object in a setTimeout string or something, it could easily be discarded before the object.

> > 2. For objects created via 'new' expressions in scripts,
> > obj->getType()->newScript has a pointer to the constructor's script.
> > 
> >   * This information only exists for Object objects created via the 'new'
> > keyword.
> > 
> >   * You don't get the location of the 'new' expression itself. Instead you
> > get
> >     the location of the constructor function.
> 'new' & constructor functions are usually on the same line so, I guess it's
> not a big deal.

I interpret the above as meaning that you get the location of the constructor function definition. So

1: function Thing { ... }
2: o = new Thing(...)

would report line 1, not line 2.

> DOM objects, specifically nsGenericElement(s) already displays list of
> attributes/values which allows to identify them pretty easily.

That sort of information isn't adequate for non-DOM objects? The shape of an object gives you the set of attributes. (I've been playing around with adding enough information to recover those from DumpHeapComplete.)

> Currently, I mostly see: "JS Object (Object)" without any glue what the
> object actually is so, having the origin info would solve that.

It still seems like getting the set of attributes should be relatively easy and robust, at least as compared to using the type inference information to get allocation sites.

Comment 10

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to Steve Fink [:sfink] from comment #9)
> This is internal, so it's actually JSScript* and offset into its bytecode.
> But you can map to/from URL and line number.
OK, good

> Assuming I understand correctly, then no, you still have a problem -- the
> script containing the allocation site could be GC'd even if the allocated
> object is not. But perhaps that's not a huge problem, since scripts for live
> tabs ordinarily won't go away.
What do you mean by "live tabs" here? I am mostly using following scenario to repro mem leaks.

1) load a page in a new tab
2) Activate Firebug for that tab and manually perform specific actions with it.
3) Close the tab
4) Check memory leaks

Now I am using CC graph to see what's still in the memory and I need to identify all the objects related (chained) to Firebug. Would the script (I guess JSScript?) be still there together with the AllocationSiteTable?


> Well, unless they're eval scripts. If you create a reachable object in a
> setTimeout string or something, it could easily be discarded before the
> object.
I see

> 1: function Thing { ... }
> 2: o = new Thing(...)
> 
> would report line 1, not line 2.
Ah, I see. Anyway, still useful.

> > DOM objects, specifically nsGenericElement(s) already displays list of
> > attributes/values which allows to identify them pretty easily.
> 
> That sort of information isn't adequate for non-DOM objects? The shape of an
> object gives you the set of attributes. (I've been playing around with
> adding enough information to recover those from DumpHeapComplete.)
I am lost here. I am seeing attributes for DOM elements (nsGenericElement) in the CC graph, but I don't understand what you mean by "The shape of an object gives you the set of attributes".

> > Currently, I mostly see: "JS Object (Object)" without any glue what the
> > object actually is so, having the origin info would solve that.
> 
> It still seems like getting the set of attributes should be relatively easy
> and robust, at least as compared to using the type inference information to
> get allocation sites.
By "set of attributes", do you mean set of object's properties?

Honza

Comment 11

[Jim Blandy :jimb]

(In reply to Steve Fink [:sfink] from comment #9)
> > >   * If the script gets collected by GC, this information is thrown away.
> > This shouldn't be a problem as when looking for memory leaks - objects that
> > are *not* collected by GC are actually important.
> 
> Assuming I understand correctly, then no, you still have a problem -- the
> script containing the allocation site could be GC'd even if the allocated
> object is not. But perhaps that's not a huge problem, since scripts for live
> tabs ordinarily won't go away.
> 
> Well, unless they're eval scripts. If you create a reachable object in a
> setTimeout string or something, it could easily be discarded before the
> object.

It seems like this shouldn't be too much of a problem in practice. As long as 1) the information is usually, there, and 2) tools can tell you when it's disappeared, without crashing themselves, we've still got a useful feature.

Comment 12

[Steve Fink [:sfink] [:s:]]

(In reply to Jan Honza Odvarko from comment #10)
> (In reply to Steve Fink [:sfink] from comment #9)
> > > DOM objects, specifically nsGenericElement(s) already displays list of
> > > attributes/values which allows to identify them pretty easily.
> > 
> > That sort of information isn't adequate for non-DOM objects? The shape of an
> > object gives you the set of attributes. (I've been playing around with
> > adding enough information to recover those from DumpHeapComplete.)
> I am lost here. I am seeing attributes for DOM elements (nsGenericElement)
> in the CC graph, but I don't understand what you mean by "The shape of an
> object gives you the set of attributes".

I'm talking about JS objects in the JS heap dump. I'm saying that if DOM objects in the CC dump are adequately described by their attributes and values, then would the analogue in the JS heap also be "good enough" for JS objects? Specifically, if the JS heap dump included the set of properties with each object? (Probably not directly, but by pointing to other nodes in the same graph from which you could compute the full property list.) It's not there today, but it seems like it could be added.

> > > Currently, I mostly see: "JS Object (Object)" without any glue what the
> > > object actually is so, having the origin info would solve that.
> > 
> > It still seems like getting the set of attributes should be relatively easy
> > and robust, at least as compared to using the type inference information to
> > get allocation sites.
> By "set of attributes", do you mean set of object's properties?

Yes. Sorry for the terminology collision.

Comment 13

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to Steve Fink [:sfink] from comment #12)
> I'm talking about JS objects in the JS heap dump. I'm saying that if DOM
> objects in the CC dump are adequately described by their attributes and
> values, then would the analogue in the JS heap also be "good enough" for JS
> objects? Specifically, if the JS heap dump included the set of properties
> with each object? (Probably not directly, but by pointing to other nodes in
> the same graph from which you could compute the full property list.) It's
> not there today, but it seems like it could be added.
I see, sounds great.

> > > > Currently, I mostly see: "JS Object (Object)" without any glue what the
> > > > object actually is so, having the origin info would solve that.
> > > 
> > > It still seems like getting the set of attributes should be relatively easy
> > > and robust, at least as compared to using the type inference information to
> > > get allocation sites.

Just to highlight that having the set of object's attributes (properties) available in the graph would be huge help.

Honza

Comment 14

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Just to describe on of my scenarios I am solving.

Here is a chain of objects I am seeing in the CC log:

---
nsGenericElement (xhtml) a class='objectLink objectLink-object a11yFocus ' chrome://firebug/content/panel.html 0x572ed60
JS Object (HTMLAnchorElement) 0x8b66840
JS Object (Proxy) 0x5261b20
JS Object (Proxy) 0x5261af0
JS Object (Proxy) 0x5261ac0
JS Object (Proxy) 0x8516af0
JS Object (Global Scope Polluter) 0x8534220
nsDocument normal (xhtml) http://legoas/firebug/tests/content/branches/1.10/dom/4386/issue4386.html 0x7eec400
---

It shows a path between a chrome element (begin) and a zombie document (end)

Two questions:
1) I am often seeing "JS Object (Proxy)" objects. They represent a reference between compartments? (Any reason why they are chained?)

2) The most interesting element in the log is probably "JS Object (Global Scope Polluter)". I think this one represents the bug. If I saw the origin for this object I guess it would be simple to fix that. So, the question is: would the origin info apply on this object?

Honza

Comment 15

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Another question:

3) I am often seeing "JS Object (call)" (a closer). Could the origin info somehow apply on these too?

Honza

Comment 16

[Jim Blandy :jimb]

(In reply to Jan Honza Odvarko from comment #15)
> Another question:
> 
> 3) I am often seeing "JS Object (call)" (a closer). Could the origin info
> somehow apply on these too?

For call objects, we can easily get you the exact URL and source line: call objects have a private slot which points to the JSFunction for whose call they were created. That function has a JSScript, and the JSScript has filename (that is, url) and lineno fields.

Comment 17

[Alex Keybl [:akeybl]]

This appears to be tracking an enhancement to debugging Firefox as opposed to a regression or major pain point for users. Can I get some more context for tracking?

Comment 18

[Andrew McCreight (out of office until 8/21) [:mccr8]]

I just put a ? for tracking-firefox14 because dmandelin marked it as + and it was accidentally (I presume) cleared.

Comment 19

[David Mandelin [:dmandelin]]

I don't remember anymore, but I think I marked it because it seemed like something that devtools needed in the near future. Maybe that's not something release-drivers need to see, though. Feel free to clear if it's not your kind of bug.

Comment 20

[Alex Keybl [:akeybl]]

Dave C - please re-nominate if this is something that should be on our radars for release.

Comment 21

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Is there anything I could help with to move this forward?
Honza

Comment 22

[Jim Blandy :jimb]

Attachment: Define a function to retrieve the filename (URL) and line of the callee of a Call object.

I don't think Jason or I will be able to work on this directly within the next month.

If you have a call object, the attached patch (untested) is how I think you can get its source location (if, indeed, it has one).

Comment 23

[Jim Blandy :jimb]

That's untested code, but hopefully it will be helpful. Perhaps that can be hooked into the CC dumping code somehow.

Comment 24

[Jim Blandy :jimb]

Note that bug 850026 added a very general object metadata hook, which would meet this need very nicely.

Comment 25

[Varada]

Before trying implementing file name and line number using the hook of bug 850026 in local build ( As per expert opinion requires a through understanding of spidermonkey data structure internals and memory management), for documentation purpose, listing the code which worked in my local build for function objects( Original solution author - Jim. Still testing in progress to be done regarding whether the line number reported is indeed the line number of object in question )

jsapifriend.cpp:

//To get the file name and line number for objects where (js::IsFunctionObject(obj)) is true
JS_FRIEND_API(bool) JS_GetCallObjectFilenameAndLine(JSContext *cx, JSObject *obj, char **filename, uint32_t *line)
{
    const char *originalName;
    uint32_t linenumber = 0;
	
    JSFunction * fun = JS_GetObjectFunction(obj);
    
    if (!fun) {
        originalName = "eval code";
	*line = linenumber;
       
	 		 		    	
    } else {
	if (!fun->isInterpreted()){
	originalName = "C file";
	*line = linenumber;
    } else {
	JSScript * script = fun->getOrCreateScript(cx);
	if (!script) return false;
        originalName = script->filename();
        if (!originalName)
            originalName = "<anonymous script>";
        linenumber = script->lineno;
	*line = linenumber;
			}
    }

    // Make a copy of the filename for our caller.
    size_t nameLen = strlen(originalName);
    char *nameCopy = (char *) cx->malloc_(nameLen + 1);
    if (!nameCopy)
        return false;
    memcpy(nameCopy, originalName, nameLen + 1);
    *filename = nameCopy;
	
    return true;
}

//To get the file name and line number of script type GCKind
JS_FRIEND_API(bool) JS_GetCallScriptFilenameAndLine(JSContext *cx, void *GCThing, char **filename, uint32_t *line)
{
    const char *originalName;
	uint32_t linenumber = 0;
	
    	
			
        JSScript * script = static_cast<JSScript*>(GCThing);
		if (!script) return false;
        originalName = script->filename();
        if (!originalName)
            originalName = "<anonymous script>";
        linenumber = script->lineno;
		*line = linenumber;
			
    

    // Make a copy of the filename for our caller.
    size_t nameLen = strlen(originalName);
    char *nameCopy = (char *) cx->malloc_(nameLen + 1);
    if (!nameCopy)
        return false;
    memcpy(nameCopy, originalName, nameLen + 1);
    *filename = nameCopy;
	
    return true;
}

Comment 26

[Varada]

Have any body tried the suggestion given by Jason Orendorff [:jorendorff] ?
Looks like two more GCKinds can be solved through the same

1) Reverse the table of TypeCompartment::AllocationSiteTable ,then look up obj->getType() to solve TypeObjects GCKind
2) For objects allocated via new, obj->getType()->newScript

Also, what was the exact procedure for the DOM objects
3) "Neither of those works for DOM objects. If we need that, we can do it in a special debug-only mode.

Exposing this information to the debugger is a smallish amount of work but I can't take it now. "