From adabed33f8282fee13ea772f924c8092e592cb32 Mon Sep 17 00:00:00 2001 From: Anatoli Papirovski Date: Sat, 14 Oct 2017 20:49:01 -0400 Subject: [PATCH 1/6] events: stricter prop & variable checks for perf Replace truthy/falsey checks of _events and _events[type] with comparisons to undefined for better performance: events/ee-add-remove.js n=250000 5.30 % *** 4.260028e-07 events/ee-emit.js n=2000000 4.18 % *** 1.026649e-05 This has a knock-on effect on modules that use lots of events, e.g.: http2/headers.js nheaders=0 n=1000 2.60 % *** 0.000298338 --- lib/events.js | 41 +++++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/lib/events.js b/lib/events.js index 1414a1429dfe3b..212bcd8a63ba2b 100644 --- a/lib/events.js +++ b/lib/events.js @@ -77,7 +77,8 @@ EventEmitter.init = function() { } } - if (!this._events || this._events === Object.getPrototypeOf(this)._events) { + if (this._events === undefined || + this._events === Object.getPrototypeOf(this)._events) { this._events = Object.create(null); this._eventsCount = 0; } @@ -169,8 +170,8 @@ EventEmitter.prototype.emit = function emit(type) { var doError = (type === 'error'); events = this._events; - if (events) - doError = (doError && events.error == null); + if (events !== undefined) + doError = (doError && events.error === undefined); else if (!doError) return false; @@ -180,7 +181,7 @@ EventEmitter.prototype.emit = function emit(type) { if (doError) { if (arguments.length > 1) er = arguments[1]; - if (domain) { + if (domain !== null && domain !== undefined) { if (!er) { const errors = lazyErrors(); er = new errors.Error('ERR_UNHANDLED_ERROR'); @@ -205,10 +206,10 @@ EventEmitter.prototype.emit = function emit(type) { handler = events[type]; - if (!handler) + if (handler === undefined) return false; - if (domain && this !== process) { + if (domain !== null && domain !== undefined && this !== process) { domain.enter(); needDomainExit = true; } @@ -254,13 +255,13 @@ function _addListener(target, type, listener, prepend) { } events = target._events; - if (!events) { + if (events === undefined) { events = target._events = Object.create(null); target._eventsCount = 0; } else { // To avoid recursion in the case that type === "newListener"! Before // adding it to the listeners, first emit "newListener". - if (events.newListener) { + if (events.newListener !== undefined) { target.emit('newListener', type, listener.listener ? listener.listener : listener); @@ -271,7 +272,7 @@ function _addListener(target, type, listener, prepend) { existing = events[type]; } - if (!existing) { + if (existing === undefined) { // Optimize the case of one listener. Don't need the extra array object. existing = events[type] = listener; ++target._eventsCount; @@ -385,11 +386,11 @@ EventEmitter.prototype.removeListener = } events = this._events; - if (!events) + if (events === undefined) return this; list = events[type]; - if (!list) + if (list === undefined) return this; if (list === listener || list.listener === listener) { @@ -422,7 +423,7 @@ EventEmitter.prototype.removeListener = if (list.length === 1) events[type] = list[0]; - if (events.removeListener) + if (events.removeListener !== undefined) this.emit('removeListener', type, originalListener || listener); } @@ -434,15 +435,15 @@ EventEmitter.prototype.removeAllListeners = var listeners, events, i; events = this._events; - if (!events) + if (events === undefined) return this; // not listening for removeListener, no need to emit - if (!events.removeListener) { + if (events.removeListener === undefined) { if (arguments.length === 0) { this._events = Object.create(null); this._eventsCount = 0; - } else if (events[type]) { + } else if (events[type] !== undefined) { if (--this._eventsCount === 0) this._events = Object.create(null); else @@ -470,7 +471,7 @@ EventEmitter.prototype.removeAllListeners = if (typeof listeners === 'function') { this.removeListener(type, listeners); - } else if (listeners) { + } else if (listeners !== undefined) { // LIFO order for (i = listeners.length - 1; i >= 0; i--) { this.removeListener(type, listeners[i]); @@ -485,11 +486,11 @@ EventEmitter.prototype.listeners = function listeners(type) { var ret; var events = this._events; - if (!events) + if (events === undefined) ret = []; else { evlistener = events[type]; - if (!evlistener) + if (evlistener === undefined) ret = []; else if (typeof evlistener === 'function') ret = [evlistener.listener || evlistener]; @@ -512,12 +513,12 @@ EventEmitter.prototype.listenerCount = listenerCount; function listenerCount(type) { const events = this._events; - if (events) { + if (events !== undefined) { const evlistener = events[type]; if (typeof evlistener === 'function') { return 1; - } else if (evlistener) { + } else if (evlistener !== undefined) { return evlistener.length; } } From 75571fbe64b0991798e2bb37c82b3b833027a22b Mon Sep 17 00:00:00 2001 From: Anatoli Papirovski Date: Sat, 14 Oct 2017 21:17:47 -0400 Subject: [PATCH 2/6] events: remove unnecessary console instantiation Previously, console had to be compiled in case it was not available but this is no longer necessary - remove it. Refs: https://github.com/nodejs/node/pull/15111 --- lib/events.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/lib/events.js b/lib/events.js index 212bcd8a63ba2b..12414db40bd93b 100644 --- a/lib/events.js +++ b/lib/events.js @@ -54,9 +54,6 @@ Object.defineProperty(EventEmitter, 'defaultMaxListeners', { return defaultMaxListeners; }, set: function(arg) { - // force global console to be compiled. - // see https://github.com/nodejs/node/issues/4467 - console; // check whether the input is a positive number (whose value is zero or // greater and not a NaN). if (typeof arg !== 'number' || arg < 0 || arg !== arg) { From 00698a6e188a2c6ec4f04a5e5a6259bf7e93b324 Mon Sep 17 00:00:00 2001 From: Anatoli Papirovski Date: Sat, 14 Oct 2017 21:40:51 -0400 Subject: [PATCH 3/6] events: return values directly in listeners Each conditional branch in EventEmitter.prototype.listeners assigns its return value to a variable ret which is returned at the end. Instead just return from within each branch. This is both clearer and more performant. events/ee-listeners.js n=5000000 3.65 % *** 3.359171e-10 --- lib/events.js | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/lib/events.js b/lib/events.js index 12414db40bd93b..deeedd2ea687c9 100644 --- a/lib/events.js +++ b/lib/events.js @@ -479,23 +479,19 @@ EventEmitter.prototype.removeAllListeners = }; EventEmitter.prototype.listeners = function listeners(type) { - var evlistener; - var ret; - var events = this._events; + const events = this._events; if (events === undefined) - ret = []; - else { - evlistener = events[type]; - if (evlistener === undefined) - ret = []; - else if (typeof evlistener === 'function') - ret = [evlistener.listener || evlistener]; - else - ret = unwrapListeners(evlistener); - } + return []; - return ret; + const evlistener = events[type]; + if (evlistener === undefined) + return []; + + if (typeof evlistener === 'function') + return [evlistener.listener || evlistener]; + + return unwrapListeners(evlistener); }; EventEmitter.listenerCount = function(emitter, type) { From 40a7e23786106842eb2c1bc0e8c00db663e3773c Mon Sep 17 00:00:00 2001 From: Anatoli Papirovski Date: Sun, 15 Oct 2017 00:44:47 -0400 Subject: [PATCH 4/6] events: use spread function param in emit With recent changes in V8, it is now as performant or faster to use spread parameter within EventEmitter.prototype.emit, especially in cases where looping over arguments is required. events/ee-emit.js n=2000000 4.40 % *** 1.505543e-06 events/ee-emit-1-arg.js n=2000000 2.16 % *** 2.434584e-10 events/ee-emit-2-args.js n=2000000 1.05 % ** 0.001764852 events/ee-emit-3-args.js n=2000000 2.18 % *** 3.234954e-08 events/ee-emit-6-args.js n=2000000 17.17 % *** 1.298702e-103 events/ee-emit-10-args.js n=2000000 17.14 % *** 1.144958e-97 This has a knock-on effect for modules that use events extensively, such as http2: http2/headers.js nheaders=0 n=1000 2.10 % *** 6.792106e-11 --- lib/events.js | 38 +++++++++++++++----------------------- 1 file changed, 15 insertions(+), 23 deletions(-) diff --git a/lib/events.js b/lib/events.js index deeedd2ea687c9..e696452c03cb20 100644 --- a/lib/events.js +++ b/lib/events.js @@ -161,23 +161,20 @@ function emitMany(handler, isFn, self, args) { } } -EventEmitter.prototype.emit = function emit(type) { - var er, handler, len, args, i, events, domain; - var needDomainExit = false; - var doError = (type === 'error'); +EventEmitter.prototype.emit = function emit(type, ...args) { + let doError = (type === 'error'); - events = this._events; + const events = this._events; if (events !== undefined) doError = (doError && events.error === undefined); else if (!doError) return false; - domain = this.domain; + const domain = this.domain; // If there is no 'error' event listener then throw. if (doError) { - if (arguments.length > 1) - er = arguments[1]; + let er = args[0]; if (domain !== null && domain !== undefined) { if (!er) { const errors = lazyErrors(); @@ -201,37 +198,32 @@ EventEmitter.prototype.emit = function emit(type) { return false; } - handler = events[type]; + const handler = events[type]; if (handler === undefined) return false; + let needDomainExit = false; if (domain !== null && domain !== undefined && this !== process) { domain.enter(); needDomainExit = true; } - var isFn = typeof handler === 'function'; - len = arguments.length; - switch (len) { - // fast cases - case 1: + const isFn = typeof handler === 'function'; + switch (args.length) { + case 0: emitNone(handler, isFn, this); break; + case 1: + emitOne(handler, isFn, this, args[0]); + break; case 2: - emitOne(handler, isFn, this, arguments[1]); + emitTwo(handler, isFn, this, args[0], args[1]); break; case 3: - emitTwo(handler, isFn, this, arguments[1], arguments[2]); - break; - case 4: - emitThree(handler, isFn, this, arguments[1], arguments[2], arguments[3]); + emitThree(handler, isFn, this, args[0], args[1], args[2]); break; - // slower default: - args = new Array(len - 1); - for (i = 1; i < len; i++) - args[i - 1] = arguments[i]; emitMany(handler, isFn, this, args); } From 05850476eca42fd553d9f5a57ae04958cb7a5bca Mon Sep 17 00:00:00 2001 From: Anatoli Papirovski Date: Sun, 15 Oct 2017 01:21:34 -0400 Subject: [PATCH 5/6] events: onceWrapper apply directly with arguments Due to changes in V8 in 6.0 & 6.1, it's no longer necessary to copy arguments to avoid deopt. Just call .apply with arguments. Retains fast cases for 0-3 arguments. events/ee-once-4-args.js n=20000000 11.58 % *** 1.310379e-05 --- lib/events.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/lib/events.js b/lib/events.js index e696452c03cb20..0e86420aa2b1aa 100644 --- a/lib/events.js +++ b/lib/events.js @@ -327,10 +327,7 @@ function onceWrapper() { return this.listener.call(this.target, arguments[0], arguments[1], arguments[2]); default: - const args = new Array(arguments.length); - for (var i = 0; i < args.length; ++i) - args[i] = arguments[i]; - this.listener.apply(this.target, args); + this.listener.apply(this.target, arguments); } } } From 05c065f8a10415240dc3f47d15f2270d63e8c6cf Mon Sep 17 00:00:00 2001 From: Anatoli Papirovski Date: Fri, 20 Oct 2017 13:49:12 -0400 Subject: [PATCH 6/6] !fixup events: properly check args.length --- lib/events.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/events.js b/lib/events.js index 0e86420aa2b1aa..3aded39489bea4 100644 --- a/lib/events.js +++ b/lib/events.js @@ -174,7 +174,9 @@ EventEmitter.prototype.emit = function emit(type, ...args) { // If there is no 'error' event listener then throw. if (doError) { - let er = args[0]; + let er; + if (args.length > 0) + er = args[0]; if (domain !== null && domain !== undefined) { if (!er) { const errors = lazyErrors();