From c814c7e9ea2f91fab094c57913c0c30784b46bf3 Mon Sep 17 00:00:00 2001 From: Luigi Pinca Date: Sat, 25 Mar 2017 22:36:11 +0100 Subject: [PATCH] events: do not keep arrays with a single listener Use the remaining listener directly if the array of listeners has only one element after running `EventEmitter.prototype.removeListener()`. Advantages: - Better memory usage and better performance if no new listeners are added for the same event. Disadvantages: - A new array must be created if new listeners are added for the same event. PR-URL: https://github.com/nodejs/node/pull/12043 Reviewed-By: James M Snell Reviewed-By: Matteo Collina Reviewed-By: Daijiro Wachi Reviewed-By: Ron Korving --- lib/events.js | 18 +++++++++++------- .../test-event-emitter-remove-listeners.js | 17 +++++++++++++++++ 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/lib/events.js b/lib/events.js index ab167bb2fbd44b..968b95c9b34059 100644 --- a/lib/events.js +++ b/lib/events.js @@ -344,7 +344,7 @@ EventEmitter.prototype.removeListener = } else if (typeof list !== 'function') { position = -1; - for (i = list.length; i-- > 0;) { + for (i = list.length - 1; i >= 0; i--) { if (list[i] === listener || list[i].listener === listener) { originalListener = list[i].listener; position = i; @@ -356,7 +356,6 @@ EventEmitter.prototype.removeListener = return this; if (list.length === 1) { - list[0] = undefined; if (--this._eventsCount === 0) { this._events = new EventHandlers(); return this; @@ -365,8 +364,12 @@ EventEmitter.prototype.removeListener = } } else if (position === 0) { list.shift(); + if (list.length === 1) + events[type] = list[0]; } else { spliceOne(list, position); + if (list.length === 1) + events[type] = list[0]; } if (events.removeListener) @@ -378,7 +381,7 @@ EventEmitter.prototype.removeListener = EventEmitter.prototype.removeAllListeners = function removeAllListeners(type) { - var listeners, events; + var listeners, events, i; events = this._events; if (!events) @@ -401,7 +404,8 @@ EventEmitter.prototype.removeAllListeners = // emit removeListener for all listeners on all events if (arguments.length === 0) { var keys = Object.keys(events); - for (var i = 0, key; i < keys.length; ++i) { + var key; + for (i = 0; i < keys.length; ++i) { key = keys[i]; if (key === 'removeListener') continue; this.removeAllListeners(key); @@ -418,9 +422,9 @@ EventEmitter.prototype.removeAllListeners = this.removeListener(type, listeners); } else if (listeners) { // LIFO order - do { - this.removeListener(type, listeners[listeners.length - 1]); - } while (listeners[0]); + for (i = listeners.length - 1; i >= 0; i--) { + this.removeListener(type, listeners[i]); + } } return this; diff --git a/test/parallel/test-event-emitter-remove-listeners.js b/test/parallel/test-event-emitter-remove-listeners.js index dfdae52d1cf21d..727523eabdb29a 100644 --- a/test/parallel/test-event-emitter-remove-listeners.js +++ b/test/parallel/test-event-emitter-remove-listeners.js @@ -136,3 +136,20 @@ assert.throws(() => { const e = ee.removeListener('foo', listener); assert.strictEqual(e, ee); } + +{ + const ee = new EventEmitter(); + + ee.on('foo', listener1); + ee.on('foo', listener2); + assert.deepStrictEqual(ee.listeners('foo'), [listener1, listener2]); + + ee.removeListener('foo', listener1); + assert.strictEqual(ee._events.foo, listener2); + + ee.on('foo', listener1); + assert.deepStrictEqual(ee.listeners('foo'), [listener2, listener1]); + + ee.removeListener('foo', listener1); + assert.strictEqual(ee._events.foo, listener2); +}