Skip to content

Commit

Permalink
events: do not keep arrays with a single listener
Browse files Browse the repository at this point in the history
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: #12043
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Ron Korving <ron@ronkorving.nl>
  • Loading branch information
lpinca authored and italoacasas committed Apr 10, 2017
1 parent 8058bae commit c814c7e
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 7 deletions.
18 changes: 11 additions & 7 deletions lib/events.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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);
Expand All @@ -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;
Expand Down
17 changes: 17 additions & 0 deletions test/parallel/test-event-emitter-remove-listeners.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

0 comments on commit c814c7e

Please sign in to comment.