896088 - Make analysis of variables unnecessarily entrained by inner functions available as a devtool

Status: NEW

(DevTools :: General, enhancement, P3)

Description

[Till Schneidereit [:till]]

Bug 894669 introduced a shell-only analysis of unnecessarily entrained vars in closures. It would be great to have that as a devtool.

Not only is that helpful for the analysis itself, I think it'd also help people to better understand how the engine deals with their code. For the latter, maybe it could be part of a larger code-quality-analysis thing.

Just spewing all the unnecessarily entrained vars probably isn't too helpful, so maybe it could check if the numbers are below a certain ratio or threshold. That, or display all functions in an ordered list, with those with the most unnecessarily entrained var at the top.

Comment 1

[Rob Campbell [:rc] (:robcee)]

This feels like it might be more useful as part of a larger linting or code-analysis tool.

Alternatively, it might make sense to expose through GCLI as a command.

What should the output of something like this be? List of vars (maybe beyond a certain threshold) + source location?

Comment 2

[Till Schneidereit [:till]]

(In reply to Rob Campbell [:rc] (:robcee) from comment #1)
> This feels like it might be more useful as part of a larger linting or
> code-analysis tool.

I very much agree: such a tool would be fantastic to have, in general. I described how I believe the output of h4writer's fantastic tracelogging tool[1] should be exposed in such a tool in a recent discussion on dev-tech-js-internals[2].

[1]: http://alasal.be/ionmonkey/

[2]: https://groups.google.com/d/msg/mozilla.dev.tech.js-engine.internals/tzAqxbDKXAI/LcxhhLuDZ5cJ

> Alternatively, it might make sense to expose through GCLI as a command.

As a first step, that'd be great, yes. Medium or long term, I think there's great value to be had in a code quality tool.

> 
> What should the output of something like this be? List of vars (maybe beyond
> a certain threshold) + source location?

Something like that. Plus info about the inner function that causes the vars to be entrained. Instead of a threshold, maybe the list could be sorted in decending order of number of entrained vars?

Comment 3

[Rob Campbell [:rc] (:robcee)]

Rereading the initial comment #0, you describe the feature created in bug 894669 as "shell-only". That's going to make my GCLI command suggestion not possible.

https://developer.mozilla.org/en-US/docs/Tools/GCLI

If this were available in the browser it might become possible.

The tracelogging tools you mention in comment #2 are pretty interesting though. We should talk more about those as we currently have some tracing efforts in progress.

Comment 4

[Andrew McCreight [:mccr8]]

> Instead of a threshold, maybe the list could be sorted in decending order of number of entrained vars

The problem with both of those is that you don't really care about most vars.  You only care about vars that
a) entrain a lot of other stuff (like an iframe)
b) live much longer than what is rightly going to use it

I don't think you can easily get information about either without some dynamic component to the analysis, so maybe just matching the ordering in the source is simplest.

I'm clearing the dependence because it doesn't sound like this is going to land on a leo+ timescale.

Comment 5

[Till Schneidereit [:till]]

(In reply to Andrew McCreight [:mccr8] from comment #4)
> > Instead of a threshold, maybe the list could be sorted in decending order of number of entrained vars
> 
> The problem with both of those is that you don't really care about most
> vars.  You only care about vars that
> a) entrain a lot of other stuff (like an iframe)
> b) live much longer than what is rightly going to use it

That's a good point. Maybe it should tie into a heap visualization tool (bug 738973)?

Actually, maybe instead of creating a devtool, we should try to just fix the issue of having unnecessarily entrained vars in the first place. That would, however, still leave vars that are entrained necessarily, but could be left out if the user restructured their code.

So maybe what it boils down to is a heap visualizer that highlights common patterns by which objects are leaked.

Comment 6

[David Bruant]

(In reply to Till Schneidereit [:till] from comment #5)
> Actually, maybe instead of creating a devtool, we should try to just fix the
> issue of having unnecessarily entrained vars in the first place.
Agreed. If this problem (bug 894971) can be solved in a short time frame, the tool we're talking about here won't be useful (because it addresses 'unnecessary' vars).
Setting dependency as appropriate.

> That would, however, still leave vars that are entrained necessarily,
> but could be left out if the user restructured their code.
A tool is indeed needed for that.