improve mixin performance by fixing https://code.google.com/p/v8/issues/detail?id=4165

[alubbe]

(Part of #1975)
Found another weird bug in v8 - see jsperf http://jsperf.com/iterative-function-creation and v8 issue https://code.google.com/p/v8/issues/detail?id=4165

Anyhow, fixing it turns this

mixin-escape: 25284ms
mixin-no-escape: 17737ms

into

mixin-escape: 7463ms
mixin-no-escape: 2107ms

This PR assumes that all variables with names like jade_mixin_N are safe to use. If we don't want to make this assumption for the 1.x releases, we should bring it in 2.0. What do you think?

[ForbesLindesay]

Wow, that's dramatic indeed. I'm starting to think we need a more complete jade performance testing suite to be setup.

One thought: instead of generating the many redundant jade_mixin_index variables, could we just use jade_interp? I'm happy to treat all jade_ prefixed variable names as fair game.

I am slightly concerned by how much we are optimising for specific V8 bugs here. I think changes like this should be explicitly marked out with comments in the code that explain:

1. What was done
2. Which bug is forcing us to add this hack
3. When and How to remove the hack once the bug is fixed.

[alubbe]

Well the bad news is that they don't intend on fixing it as they have made a conscious decision to not optimize this case in favour of something else, but I keep fighting the fight ;) https://code.google.com/p/v8/issues/detail?id=4165#c4

[alubbe]

jade_interp is much more suited for the dirty job. It also makes the PR backwards-compatible.

[ForbesLindesay]

Excellent work :). Let's give the v8 team another day or two in case they re-consider.

assuming they don't change their minds, we can implement this hack while we look for a more per enact solution. In particular, I'd like to try making mixing be functions that return strings (rather than append to the buffer). We could then identify "pure" mixing (I.e. Those that don't depend on any locals) and hoist them outside of the template.

[alubbe]

👍
On waiting - I could not find a v8 version that was not affected and even when they fix it, it will be 12 weeks before it lands in v8 stable and another X weeks before it lands in the master of iojs. So let's merge it? ;)

[TimothyGu]

+1. It looks clean enough and the performance improvements are nontrivial.

[ForbesLindesay]

OK, lets get this released :)