Fix arguments leaking all over the place

[Fishrock123]

Bluebird has done a bunch of research into Javascript optimization blockers, which includes strict arguments usage:

https://github.com/petkaantonov/bluebird/wiki/Optimization-killers#3-managing-arguments

See also: pixijs/pixijs#853

[Fishrock123]

Preliminary tests / benchmarks against an existing fn.bind polyfill on jsperf: http://jsperf.com/bind-vs-custom/5

[photonstorm]

There are several issues with this PR:

1. This fundamentally changes the way a number of Group methods work - they're now iterating in reverse over the children. You can't just make that kind of change, seemingly on a whim, and expect it to not have consequences.

2. The bluebird optimisations are meant for hot path code specifically, where you need a function to be marked and opted. Several of the methods changed are not hot path targets at all. No-one in their right mind should be calling PluginManager.add or StateManager.start in a core loop! So it was never a candidate for inlining or hot/stable recompilation in the first place.

3. Optimisations like this need more than a single jsperf test, as that's the wrong kind of test in this instance. What is needed is a trace-opt log proving that without the change Group.forEach is never optimised, and after the change it now is, and it remains that way!

I know from looking through stacks of V8 phaser trace outputs recently that a number of the Group functions are never flagged as hot, and removing the use of arguments in some of them might resolve that. Or they may still not be flagged anyway. Either way it should be a case of adding it in to one function at a time and profiling that explicitly, rather than a wholesale copy/replace.

If you look through a trace dump you'll see there are a number of deopts going on that would be far more useful to fix initially. For example Rectangle.clone gets evicted, but more worryingly (given how often it's used) PIXI.DisplayObjectContainer.updateTransform and even PIXI.CanvasRenderer.renderDisplayObject are deopted as well (mostly due to unknown maps in the polymorphic calls).

[Fishrock123]

1. When I get the time to update this PR, I will revert to the old behaviour.

2. It is still good practice to do this, but for those I can change it to at least do slice rather than splice. (Splicing arguments effectively sets them (to undefined) which is just bad)

3. I agree. I'm going on vacation this week for a while, so it may take a bit but I'll try to do a lot more testing if I can.

At minimum, would you consider a separate PR for the fn.bind patch when it lands in PIXI?

[photonstorm]

Closing due to inactivity.

[Fishrock123]

Yeah.

You might want to think about accepting it on the bind polyfill though, pixi is using my changes in that. Maybe I'll get time for this when I get back into game dev.

[photonstorm]

I did consider it, but I hardly use bind at all in Phaser, and the big popular browsers support bind natively now so the polyfill is ignored. I'm more interested in the implication on this approach on the use of arguments than bind, which seems like it could be more beneficial. Still more in depth performance checks needed (and a time machine to fit all this stuff in too).