From 8bf64d135fe22d95510e361ee88074f112b62002 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 Backport-PR-URL: https://github.com/nodejs/node/pull/13796 Reviewed-By: James M Snell Reviewed-By: Matteo Collina Reviewed-By: Daijiro Wachi Reviewed-By: Ron Korving --- lib/events.js | 20 ++++++++++++------- .../test-event-emitter-remove-listeners.js | 17 ++++++++++++++++ 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/lib/events.js b/lib/events.js index 5cb837cc6c3877..5daa2e82f3e066 100644 --- a/lib/events.js +++ b/lib/events.js @@ -338,7 +338,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; @@ -350,15 +350,20 @@ EventEmitter.prototype.removeListener = return this; if (list.length === 1) { - list[0] = undefined; if (--this._eventsCount === 0) { this._events = new EventHandlers(); return this; } else { delete events[type]; } + } 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) @@ -370,7 +375,7 @@ EventEmitter.prototype.removeListener = EventEmitter.prototype.removeAllListeners = function removeAllListeners(type) { - var listeners, events; + var listeners, events, i; events = this._events; if (!events) @@ -393,7 +398,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); @@ -410,9 +416,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); +}