Controller functions stringified twice when directives are compiled, even in strict DI mode

[robertknight]

Whilst investigating poor performance when instantiating a component, I was surprised to see Function.prototype.toString() showing up in the profile taking a significant amount of time (about 2ms per instance of a component in a list of potentially hundreds of items) despite the app using strict DI.

It appears that when instantiating a component, Angular calls Function.prototype.toString on every controller twice. The first time is in setupControllers due to what looks like a typo in controller == '@' and the second is to determine whether a function is an ES2015 class (!!), which seems silly considering that the proportion of users using native classes in production apps will be tiny.

In testing, it appears that stringifying a large function is somewhat more expensive in Firefox than Chrome/Node/IE.

Steps to reproduce:

1. Open this Plunker: http://plnkr.co/edit/RbdcKbymucZWvOF4V6Co
2. Open dev tools and refresh, observe 2 traces from a monkey-patched Function.prototype.toString invoked on AppController

[Narretz]

and the second is to determine whether a function is an ES2015 class (!!), which seems silly considering that the proportion of users using native classes in production apps will be tiny.

We want to support this, why do you consider this silly? This is going to be used more and more.

The first time is in setupControllers due to what looks like a typo in controller == '@'

This is to support an old API that ngController uses. I think it's undocumented. Maybe we don't need it anymore.

[Narretz]

Or use controller === '@' ....

[Narretz]

It's also worth a shot to raise the issue of toString being slow in Firefox itself.

[robertknight]

We want to support this, why do you consider this silly? This is going to be used more and more.

Apologies, perhaps I should rephrase, I think it is very unfortunate to be paying a performance penalty for something that very few users are using.

It's also worth a shot to raise the issue of toString being slow in Firefox itself.

I can do that, though I don't know whether it is reasonable to expect that stringifying a function will always be very fast. It looks like Firefox might be disposing of the original source in some contexts, possibly as an optimization? - https://dxr.mozilla.org/mozilla-central/source/js/src/jsfun.cpp#936

[Narretz]

Can you test if you see an actual improvement when you change == to === here

angular.js/src/ng/compile.js

Line 2488 in 82a4545

if (controller == '@') {

?

I've tested in a benchmark, but the results vary too strongly, so in this articial case the difference seems negligble.

[robertknight]

Yes, that saves about 30% of the cost of setupControllers in Firefox according to Firefox dev tools' profiler, that's about 5-15ms (0.5-1%) off startup time in our app - there does seem to be quite a lot of variance though.

[gkalpak]

If anything else === '@' saves as a "stringification", so it can't be a bad thing 😃

Now regarding native Class detection, unfortunately we:

1. Need to differentiate between ES2015 Classes and good old constructor functions.
   WHY ? If we don't, we can't assign bindings before calling the constructor (for constructor functions), which will be a massive breaking change.
2. As much as we'd love to have a more robust and performant way of detecting Classes, we don't.
   WHY ? Because we don't 😃

So, as long as (1) and (2) hold, there is not much we can do. Maybe browsers will provide a solution for (2) in the future.

Just m2c...