Skip to content

Commit 8bf64d1

Browse files
lpincaMylesBorins
authored andcommitted
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: #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>
1 parent ef133b3 commit 8bf64d1

File tree

2 files changed

+30
-7
lines changed

2 files changed

+30
-7
lines changed

lib/events.js

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -338,7 +338,7 @@ EventEmitter.prototype.removeListener =
338338
} else if (typeof list !== 'function') {
339339
position = -1;
340340

341-
for (i = list.length; i-- > 0;) {
341+
for (i = list.length - 1; i >= 0; i--) {
342342
if (list[i] === listener || list[i].listener === listener) {
343343
originalListener = list[i].listener;
344344
position = i;
@@ -350,15 +350,20 @@ EventEmitter.prototype.removeListener =
350350
return this;
351351

352352
if (list.length === 1) {
353-
list[0] = undefined;
354353
if (--this._eventsCount === 0) {
355354
this._events = new EventHandlers();
356355
return this;
357356
} else {
358357
delete events[type];
359358
}
359+
} else if (position === 0) {
360+
list.shift();
361+
if (list.length === 1)
362+
events[type] = list[0];
360363
} else {
361364
spliceOne(list, position);
365+
if (list.length === 1)
366+
events[type] = list[0];
362367
}
363368

364369
if (events.removeListener)
@@ -370,7 +375,7 @@ EventEmitter.prototype.removeListener =
370375

371376
EventEmitter.prototype.removeAllListeners =
372377
function removeAllListeners(type) {
373-
var listeners, events;
378+
var listeners, events, i;
374379

375380
events = this._events;
376381
if (!events)
@@ -393,7 +398,8 @@ EventEmitter.prototype.removeAllListeners =
393398
// emit removeListener for all listeners on all events
394399
if (arguments.length === 0) {
395400
var keys = Object.keys(events);
396-
for (var i = 0, key; i < keys.length; ++i) {
401+
var key;
402+
for (i = 0; i < keys.length; ++i) {
397403
key = keys[i];
398404
if (key === 'removeListener') continue;
399405
this.removeAllListeners(key);
@@ -410,9 +416,9 @@ EventEmitter.prototype.removeAllListeners =
410416
this.removeListener(type, listeners);
411417
} else if (listeners) {
412418
// LIFO order
413-
do {
414-
this.removeListener(type, listeners[listeners.length - 1]);
415-
} while (listeners[0]);
419+
for (i = listeners.length - 1; i >= 0; i--) {
420+
this.removeListener(type, listeners[i]);
421+
}
416422
}
417423

418424
return this;

test/parallel/test-event-emitter-remove-listeners.js

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,3 +136,20 @@ assert.throws(() => {
136136
const e = ee.removeListener('foo', listener);
137137
assert.strictEqual(e, ee);
138138
}
139+
140+
{
141+
const ee = new EventEmitter();
142+
143+
ee.on('foo', listener1);
144+
ee.on('foo', listener2);
145+
assert.deepStrictEqual(ee.listeners('foo'), [listener1, listener2]);
146+
147+
ee.removeListener('foo', listener1);
148+
assert.strictEqual(ee._events.foo, listener2);
149+
150+
ee.on('foo', listener1);
151+
assert.deepStrictEqual(ee.listeners('foo'), [listener2, listener1]);
152+
153+
ee.removeListener('foo', listener1);
154+
assert.strictEqual(ee._events.foo, listener2);
155+
}

0 commit comments

Comments
 (0)