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
Backport-PR-URL: #13796
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 MylesBorins committed Jul 11, 2017
1 parent ef133b3 commit 8bf64d1
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 7 deletions.
20 changes: 13 additions & 7 deletions lib/events.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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);
Expand All @@ -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;
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 8bf64d1

Please sign in to comment.