389368 - investigate removing the names of function expressions assigned to object properties

Status: RESOLVED WONTFIX

(Firefox :: General, enhancement)

Description

[Myk Melez [:myk] [@mykmelez]]

Over in bug 389347, comment 1, Brendan has noted that giving names to function expressions assigned to object properties has a performance impact, and that the practice should be unnecessary, as it should be possible to identify the "name" of a function assigned to a property (i.e. the name of the property itself) using bytecode inspection under the JS debugger API.

Once bug 389347 is fixed, we should investigate removing the names of function expressions from the Firefox code.

Comment 1

[Myk Melez [:myk] [@mykmelez]]

If we do this, we should make sure to update the JavaScript Style Guide for Firefox, which currently recommends these names <http://developer.mozilla.org/en/docs/JavaScript_style_guide#Code_style>.

Comment 2

[Benjamin Smedberg]

Unless the performance win is really noticeable (which I doubt), I would prefer to keep the style guide recommendation: JS console errors are much more obvious and useful when a function name is available.

Comment 3

[Myk Melez [:myk] [@mykmelez]]

(In reply to comment #2)
> Unless the performance win is really noticeable (which I doubt), I would prefer
> to keep the style guide recommendation: JS console errors are much more obvious
> and useful when a function name is available.

We should be able to display property names for anonymous functions in the console "using bytecode inspection under the JS debugger API." (bug 389347, comment 1).

Comment 4

[Benjamin Smedberg]

Sure, once that works then we can consider this.

Comment 5

[Myk Melez [:myk] [@mykmelez]]

> http://mxr.mozilla.org/mozilla/search?string=%3A+function+[a-zA-Z_][a-zA-Z_]*\(&regexp=on&case=on&tree=mozilla

Hmm, that works pretty well.  About the only thing it wouldn't catch are cases where a property was defined with a newline after the colon:

var foo = {
  bar:
  function foo_bar() {}
}

Comment 6

[timeless]

myk: I've used Venkman for years and its intelligence IIRC is supposedly better than what you describe. the results are very depressing. 

Note that some people use this syntax:

A.prototype.x = function () {}

and some people use:

A.__defineGetter__("x", function () {})

That said, if you actually have this magically working, and can deal with some of the more interesting problems.

like:
A.x=A.y;

So now when an exception from A.x happens, I know that A.x threw it, but I have no useful way of finding it because there's no function named A.x;

The code I was using which may or may not have influenced this style guide (my code predates it by a few years) often did dynamic dispatch where you had a generic method name, either

a.x=a[y]; a.x();
Or:
a[y]();

I know that most gecko code isn't quite this evil, although I'm not sure how universally true that is. And extensions are probably more evil (but less than web sites?).

Comment 7

[:Gavin Sharp [email: [e-mail]]]

I don't think the perf impact is significant enough for this to be worth the trouble of "fixing" old code (though others seem to think it maybe is, post-http://javascript-reverse.tumblr.com/post/34328529526/javascript-function-name-inference-aka-stop-function, see bug 805904). We're not adding these unnecessary names to functions going forward.

Comment 8

[Robert Strong (they/them - no direct email)]

iirc I asked Brendan about this a couple of years ago and the perf impact was significantly lessened to the point where he was fine with adding them.