V8 disables optimization for dispatch

[Hypercubed]

As described in various places online (for example nodejs/node-v0.x-archive#6631 and bevry/taskgroup#12) v8 disables optimization when the "arguments variable being passed to other functions and disables optimization for any calling method doing so".

I verified using the following test code:

var signals=require('./dist/signals');

var myObject = new MyObject();
myObject.started.add(testDispatch, this);

for(var i=0;i<20000; i++) {
    myObject.started.dispatch(1,2,3,4,5);
}

function MyObject() {
  this.started = new signals.Signal();
}

function testDispatch(a,b,c,d,e) {
    this || a || b;
};

The output includes:

[failed to optimize Signal.dispatch: Bad value context for arguments value]

If I replace #L366 with:

var paramsArr = [];
for (var index=0; index < arguments.length; index++) {
  paramsArr.push(arguments[index]);
}

I would submit a PR but:

• building and testing is not working for me.
• I haven't verified that this actually impacts performance.
• not sure if there are other places deoptimization will occur.

[Hypercubed]

I forgot to mention you need to run the test using the following command:

node --trace_opt --trace_deopt test.js

[Hypercubed]

FYI: http://jsperf.com/shotgun-js-vs-js-signals/4

[sompylasar]

FYI: http://www.youtube.com/watch?v=65-RbBwZQdU

[Hypercubed]

Yep, I've seen this. I don't entirely trust the jsperf myself.... what I do notice though is messages in the browser that signal.dispatch is being skipped by the v8 optimizer. Some of the other github issue I listed above seam to think it can be significant.

[sompylasar]

Sure, looking at the real outcome of the vm is the key for optimizing the right way.

[mhofman]

Looks like Array.prototype.push.apply is not subject deoptimization and is decently fast so the following should work too:

var paramsArr = [];
Array.prototype.push.apply(paramsArr, arguments);

See http://jsperf.com/copy-fn-arguments-test/16

[Hypercubed]

Still slower than the loop I think... but surprisingly does appear to allow v8 optimization.

[mhofman]

Not that surprising. When you think about it, second argument of Function.apply is guaranteed to not be modified, so no reason to deoptimize.
It seems to be about half the speed of pre-allocated copy in Chrome, but strangely in Node it's much closer to the un-allocated copy, which makes much more sense.

Oh Arrays and their dark magic performances...

Copy to pre-allocated x 9,517,368 ops/sec ±0.31% (99 runs sampled)
Copy to un-allocated x 7,222,102 ops/sec ±0.31% (99 runs sampled)
Copy using Slice x 1,219,995 ops/sec ±0.53% (98 runs sampled)
Copy using Slice Indirect x 1,149,504 ops/sec ±0.48% (91 runs sampled)
Copy using Push apply x 7,130,480 ops/sec ±0.36% (96 runs sampled)
Manual Copy x 10,188,893 ops/sec ±0.49% (95 runs sampled)

[Hypercubed]

Maybe can do even better. Apparently the one time you should can use arguments in a functional call without causing de-optimization is Function#apply (source https://github.com/petkaantonov/bluebird/wiki/Optimization-killers#what-is-safe-arguments-usage). I saw huge boost here: https://github.com/Hypercubed/mini-signals

[mhofman]

@Hypercubed, from what I understand your library doesn't allow for a binding to have its own prepended/saved params.

Regardless, the Function#apply approach is the same that I suggested in my previous comment, just that you use it to invoke the binding's function directly, where here it has to be used to make a copy of the params using Array#push. While in Chrome it is faster than slice.call(), it still isn't as fast as a manual copy as you originally suggested, so in that sense it isn't better. However it sure is more readable IMHO.

[Hypercubed]

You're right. In both cases v8 optimization works because of Function#apply. I can use the binding function directly because I am not adding any saved params. This is something that cannot be done in EventEmiiters because they need to remove the event name.