-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Do you have a strong opinion about EventEmitter? #2
Comments
This comment was a tongue-in-cheek reply to @Raynos. We have worked together with @jcorbin at Uber on projects including TChannel and Hyperbahn. Jake and Josh observed from iterating on performance improvements informed by production flame graphs that the EventEmitter was unnecessarily wasteful, and created an alternative. The alternative allows us to bypass a dictionary lookup from event name to emitter state, by placing the emitter state and interface on a property of the event emitter itself. function Beeper() {
this.beepEvent = this.defineEvent("beep");
}
util.inherits(Beeper, EventEmitter);
var beeper = new Beeper();
beeper.beepEvent.on(onBeep, recorder);
beeper.beepEvent.emit({volume: 1});
function onBeep(beep, recorder) {
} Consequently, looking up an event benefits from v8 hidden classes. We also observed that we do not need an array in the common case of a single observer, reducing garbage collection. The new event emitter also sends an observer-selected context object, which helps us re-use functions for multiple consumers and avoid closures in many cases. You will note that Node.js does not use event emitters internally. This optimization is just one of many steps in the walk away from abstractions to high performance. The next logical step is to completely eliminate the notion of extensible subscription and just have delegates or hooks for all of the anticipated consumers of an event and dispatch on them with individual, manually ordered method calls, much like DOM level 1 events like "onclick". In short, there are trade-offs. I think that we all go home happy if we take an opt-in layered approach that includes both higher and lower layers than what Node.js currently exposes. The higher level abstractions trade performance for robust composition, and the lower would trade ergonomics for speed. |
I have similar opinions that event emitter is not needed. The only thing I like about it is an error handling contract. "Ive just allocated this thing that may error in the future please deal with it lol" |
Thanks for the thorough reply! |
I found this comment of yours to be interesting, but I wondered if you could offer a little more context.
The text was updated successfully, but these errors were encountered: