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]>
lpinca · Jun 19, 2017 · 41de99d · 41de99d
1 parent ae38adc
commit 41de99d
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 7 deletions.
diff --git 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
@@ -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);
+}