medikoo / event-emitter

Environment agnostic event emitter solution for JavaScript
MIT License
241 stars 23 forks source link

event-chain with cache #9

Closed snowyu closed 9 years ago

snowyu commented 9 years ago

Add the event cache to improve performance and reduce memory usage.

the new with event cache:
➜  event-emitter git:(feature/event-chain) node benchmark/many-on.js
Emit for 3 listeners x1000000:

1:   582 EventEmitter2
2:   602 EventEmitter (Node.js native)
3:   851 event-emitter (this implementation)
4:  3355 Signals
➜  event-emitter git:(feature/event-chain) node benchmark/single-on.js
Emit for single listener x1000000:

1:   105 EventEmitter2
2:   122 EventEmitter (Node.js native)
3:   244 event-emitter (this implementation)
4:  3010 Signals

the old implementation (no cache) can not be endured:

Emit for single listener x1000000:

1:    94 EventEmitter2
2:   146 EventEmitter (Node.js native)
3:  2901 Signals
4: 11828 event-emitter (this implementation)
medikoo commented 9 years ago

@snowyu I'm not sure what you mean. Results as you pasted first are coming from current version as published.

$ node -v
v0.10.36
$ node benchmark/many-on.js 
Emit for 3 listeners x1000000:

1:   341 EventEmitter (Node.js native)
2:   341 EventEmitter2
3:   481 event-emitter (this implementation)
4:  1893 Signals

$ node benchmark/single-on.js 
Emit for single listener x1000000:

1:    54 EventEmitter2
2:    64 EventEmitter (Node.js native)
3:   100 event-emitter (this implementation)
4:  1486 Signals

and again this implementation is event agnostic, adding any complexity may just slow it down

snowyu commented 9 years ago

the first is the same complexity feature with second but not slow it down!

snowyu commented 9 years ago

I would open a new project for this support if u do not like it.

snowyu commented 9 years ago

It is mainly used as hooked event for database to validate data before saving.

medikoo commented 9 years ago

@snowyu sorry at first I understood that you suggest that currently event-emitter returns second result.

Still, event listener must be called in scope of it's emitter, we can't just change it so it's something else. It's common pattern for all event emitter implementations and it's used heavily by programs. So just for this reason this change is not acceptable.

Event object is always passed as an argument, and it's how event-emitters are used in both native and user land API's, just see for example DOM API.

snowyu commented 9 years ago

Ok, it doesn't matter. In deed it is a broken change. personally, I always use the 'that' pattern(=>), so it do not affect me. I will dig a new pit. I am too lazy to maintain hook and event both.

snowyu commented 9 years ago

The new pit is almost done as event-ex package.

medikoo commented 9 years ago

:+1:

snowyu commented 9 years ago

Would you want to help maintaining the events-ex package together?