Skip to content

Commit 8d386ed

Browse files
lpincajasnell
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 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]>
1 parent f637703 commit 8d386ed

File tree

2 files changed

+28
-7
lines changed

2 files changed

+28
-7
lines changed

lib/events.js

+11-7
Original file line numberDiff line numberDiff line change
@@ -363,7 +363,7 @@ EventEmitter.prototype.removeListener =
363363
} else if (typeof list !== 'function') {
364364
position = -1;
365365

366-
for (i = list.length; i-- > 0;) {
366+
for (i = list.length - 1; i >= 0; i--) {
367367
if (list[i] === listener || list[i].listener === listener) {
368368
originalListener = list[i].listener;
369369
position = i;
@@ -375,7 +375,6 @@ EventEmitter.prototype.removeListener =
375375
return this;
376376

377377
if (list.length === 1) {
378-
list[0] = undefined;
379378
if (--this._eventsCount === 0) {
380379
this._events = Object.create(null);
381380
return this;
@@ -384,8 +383,12 @@ EventEmitter.prototype.removeListener =
384383
}
385384
} else if (position === 0) {
386385
list.shift();
386+
if (list.length === 1)
387+
events[type] = list[0];
387388
} else {
388389
spliceOne(list, position);
390+
if (list.length === 1)
391+
events[type] = list[0];
389392
}
390393

391394
if (events.removeListener)
@@ -397,7 +400,7 @@ EventEmitter.prototype.removeListener =
397400

398401
EventEmitter.prototype.removeAllListeners =
399402
function removeAllListeners(type) {
400-
var listeners, events;
403+
var listeners, events, i;
401404

402405
events = this._events;
403406
if (!events)
@@ -420,7 +423,8 @@ EventEmitter.prototype.removeAllListeners =
420423
// emit removeListener for all listeners on all events
421424
if (arguments.length === 0) {
422425
var keys = Object.keys(events);
423-
for (var i = 0, key; i < keys.length; ++i) {
426+
var key;
427+
for (i = 0; i < keys.length; ++i) {
424428
key = keys[i];
425429
if (key === 'removeListener') continue;
426430
this.removeAllListeners(key);
@@ -437,9 +441,9 @@ EventEmitter.prototype.removeAllListeners =
437441
this.removeListener(type, listeners);
438442
} else if (listeners) {
439443
// LIFO order
440-
do {
441-
this.removeListener(type, listeners[listeners.length - 1]);
442-
} while (listeners[0]);
444+
for (i = listeners.length - 1; i >= 0; i--) {
445+
this.removeListener(type, listeners[i]);
446+
}
443447
}
444448

445449
return this;

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

+17
Original file line numberDiff line numberDiff line change
@@ -157,3 +157,20 @@ assert.throws(() => {
157157
const e = ee.removeListener('foo', listener);
158158
assert.strictEqual(e, ee);
159159
}
160+
161+
{
162+
const ee = new EventEmitter();
163+
164+
ee.on('foo', listener1);
165+
ee.on('foo', listener2);
166+
assert.deepStrictEqual(ee.listeners('foo'), [listener1, listener2]);
167+
168+
ee.removeListener('foo', listener1);
169+
assert.strictEqual(ee._events.foo, listener2);
170+
171+
ee.on('foo', listener1);
172+
assert.deepStrictEqual(ee.listeners('foo'), [listener2, listener1]);
173+
174+
ee.removeListener('foo', listener1);
175+
assert.strictEqual(ee._events.foo, listener2);
176+
}

0 commit comments

Comments
 (0)