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: nodejs#12043 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Daijiro Wachi <[email protected]> Reviewed-By: Ron Korving <[email protected]>
marcushill · Apr 4, 2017 · 8d386ed · 8d386ed
1 parent f637703
commit 8d386ed
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 7 deletions.
diff --git a/lib/events.js b/lib/events.js
@@ -363,7 +363,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;
@@ -375,7 +375,6 @@ EventEmitter.prototype.removeListener =
  return this;
 
  if (list.length === 1) {
- list[0] = undefined;
  if (--this._eventsCount === 0) {
  this._events = Object.create(null);
  return this;
@@ -384,8 +383,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)
@@ -397,7 +400,7 @@ EventEmitter.prototype.removeListener =
 
 EventEmitter.prototype.removeAllListeners =
  function removeAllListeners(type) {
- var listeners, events;
+ var listeners, events, i;
 
  events = this._events;
  if (!events)
@@ -420,7 +423,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);
@@ -437,9 +441,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
@@ -157,3 +157,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);
+}