events: optimize adding and removing of listeners more

[mscdex]

These optimizations result in >2x speedup in the ee-add-remove
benchmark:

• Don't mutate array.length when removing the last listener for
  an event
• Don't bother checking max listeners if listeners isn't an array
• Don't call delete when removing the last event in _events, just
  re-assign a new object instead

[evanlucas]

events/ee-add-remove.js n=250000: ./iojs: 1273100 /usr/local/bin/iojs: 311090 .......... 309.25% events/ee-emit-multi-args.js n=2000000: ./iojs: 5653500 /usr/local/bin/iojs: 1773000 ... 218.86% events/ee-emit.js n=2000000: ./iojs: 7866000 /usr/local/bin/iojs: 2100400 .............. 274.49% events/ee-listener-count.js n=50000000: ./iojs: 405780000 /usr/local/bin/iojs: 403700000 . 0.51% events/ee-listeners-many.js n=5000000: ./iojs: 6041800 /usr/local/bin/iojs: 6077800 ..... -0.59% events/ee-listeners.js n=5000000: ./iojs: 22134000 /usr/local/bin/iojs: 7548900 ........ 193.21% 

It seems to offer a significant improvement to more than just ee-add-remove. The ee-listener-count and ee-listeners-many seem to vary a little bit though.

[mscdex]

Huh... I thought I checked the other benchmarks also...

[mscdex]

@evanlucas You must be testing against a version of iojs that is pre-b677b844fc1. I tested relative to the current head of v1.x and ee-add-remove is the only one that really shows an improvement.

[evanlucas]

Ah, I had thought for some reason I was up to date on v1.x. Perhaps not. :]

[Fishrock123]

events/ee-add-remove.js n=250000: ./out/Release/iojs: 1127100 iojs: 465520 ........... 142.11% events/ee-emit-multi-args.js n=2000000: ./out/Release/iojs: 4900900 iojs: 3973500 ..... 23.34% events/ee-emit.js n=2000000: ./out/Release/iojs: 6609200 iojs: 5808800 ................ 13.78% events/ee-listener-count.js n=50000000: ./out/Release/iojs: 247180000 iojs: 253620000 . -2.54% events/ee-listeners-many.js n=5000000: ./out/Release/iojs: 4544800 iojs: 6103200 ..... -25.53% events/ee-listeners.js n=5000000: ./out/Release/iojs: 20549000 iojs: 19110000 .......... 7.53% 

ee-listerners-many continues to give wildly inconsistent results.

[mscdex]

@Fishrock123 Yeah listeners() and listenerCount() really shouldn't be affected by these changes. I never saw the high negative number you're seeing, even after many runs, but I'm just chalking it up to "normal fluctuations."

[evanlucas]

Should this be io.js instead of node now?

[mscdex]

shrug I didn't change anything in that whole block, just wrapped it all within an else{}.

[mscdex]

Code updated with suggested changes.

[bnoordhuis]

This can be made a little flatter and more readable by swapping the consequent and the alternate. Suggestion:

if(events)doError=(type==='error'&&events.error!=null);elseif(type==='error')doError=true;elsereturnfalse;

You can condense it but I'll leave it up to you to decide what's more readable.

doError=(type==='error');if(events)doError=(doError&&events.error==null);elseif(!doError)returnfalse;

[mscdex]

Updated again with suggested changes.

[bnoordhuis]

https://jenkins-iojs.nodesource.com/view/iojs/job/iojs+any-pr+multi/165/

[bnoordhuis]

Thanks Brian, landed in 630f636...7061669.

[ChALkeR]

This code in lib/_stream_readable.js (and the readable-stream module) does not take _eventsCount into account (does not increase it):

// This is a brutally ugly hack to make sure that our error handler// is attached before any userland ones. NEVER DO THIS.if(!dest._events||!dest._events.error)dest.on('error',onerror);elseif(Array.isArray(dest._events.error))dest._events.error.unshift(onerror);elsedest._events.error=[onerror,dest._events.error];

Edit: Sorry, I was wrong, it's fine =). dest.on increases the count, other two cases shouldn't. ☺