Repository: nodejs/node
Pull request: events: avoid cloning listeners array on every emit

This PR does 2 improvements:

- Do not clone the listeners on each emit, clone when we're either adding or removing listeners
- Even when we're adding or removing listeners, if we're not currently emitting, avoid cloning again

Results:

Branch:

```sh
./node benchmark/run.js --filter ee-emit events
events/ee-emit.js
events/ee-emit.js listeners=1 argc=0 n=2000000: 91,903,668.22922815
> events/ee-emit.js listeners=5 argc=0 n=2000000: 31,178,549.48221315
> events/ee-emit.js listeners=10 argc=0 n=2000000: 18,218,586.602051415
events/ee-emit.js listeners=1 argc=2 n=2000000: 97,625,065.27455927
> events/ee-emit.js listeners=5 argc=2 n=2000000: 30,786,518.583512284
> events/ee-emit.js listeners=10 argc=2 n=2000000: 17,759,695.723843552
events/ee-emit.js listeners=1 argc=4 n=2000000: 95,971,210.17248763
> events/ee-emit.js listeners=5 argc=4 n=2000000: 30,290,753.36889973
> events/ee-emit.js listeners=10 argc=4 n=2000000: 17,108,737.480991445
events/ee-emit.js listeners=1 argc=10 n=2000000: 93,870,086.24126433
> events/ee-emit.js listeners=5 argc=10 n=2000000: 28,639,532.56158767
> events/ee-emit.js listeners=10 argc=10 n=2000000: 16,616,102.896280617
```

main:

```sh
./node benchmark/run.js --filter ee-emit events
events/ee-emit.js
events/ee-emit.js listeners=1 argc=0 n=2000000: 92,364,529.17596893
> events/ee-emit.js listeners=5 argc=0 n=2000000: 29,932,147.413670897
> events/ee-emit.js listeners=10 argc=0 n=2000000: 15,313,442.157135993
events/ee-emit.js listeners=1 argc=2 n=2000000: 98,371,745.94257125
> events/ee-emit.js listeners=5 argc=2 n=2000000: 29,668,674.924437966
> events/ee-emit.js listeners=10 argc=2 n=2000000: 15,159,257.397702452
events/ee-emit.js listeners=1 argc=4 n=2000000: 96,956,966.8137633
> events/ee-emit.js listeners=5 argc=4 n=2000000: 29,187,937.01091416
> events/ee-emit.js listeners=10 argc=4 n=2000000: 14,862,263.968670348
events/ee-emit.js listeners=1 argc=10 n=2000000: 94,272,546.83507264
> events/ee-emit.js listeners=5 argc=10 n=2000000: 27,359,329.00041816
> events/ee-emit.js listeners=10 argc=10 n=2000000: 11,452,222.952059666
```

Changed files:
filename: lib/events.js
patch: @@ -87,6 +87,7 @@ const { addAbortListener } = require('internal/events/abort_listener');
 const kCapture = Symbol('kCapture');
 const kErrorMonitor = Symbol('events.errorMonitor');
 const kShapeMode = Symbol('shapeMode');
+const kEmitting = Symbol('events.emitting');
 const kMaxEventTargetListeners = Symbol('events.maxEventTargetListeners');
 const kMaxEventTargetListenersWarned =
   Symbol('events.maxEventTargetListenersWarned');
@@ -514,19 +515,22 @@ EventEmitter.prototype.emit = function emit(type, ...args) {
       addCatch(this, result, type, args);
     }
   } else {
-    const len = handler.length;
-    const listeners = arrayClone(handler);
-    for (let i = 0; i < len; ++i) {
-      const result = ReflectApply(listeners[i], this, args);
-
-      // We check if result is undefined first because that
-      // is the most common case so we do not pay any perf
-      // penalty.
-      // This code is duplicated because extracting it away
-      // would make it non-inlineable.
-      if (result !== undefined && result !== null) {
-        addCatch(this, result, type, args);
+    handler[kEmitting]++;
+    try {
+      for (let i = 0; i < handler.length; ++i) {
+        const result = ReflectApply(handler[i], this, args);
+
+        // We check if result is undefined first because that
+        // is the most common case so we do not pay any perf
+        // penalty.
+        // This code is duplicated because extracting it away
+        // would make it non-inlineable.
+        if (result !== undefined && result !== null) {
+          addCatch(this, result, type, args);
+        }
       }
+    } finally {
+      handler[kEmitting]--;
     }
   }
 
@@ -565,13 +569,17 @@ function _addListener(target, type, listener, prepend) {
   } else {
     if (typeof existing === 'function') {
       // Adding the second element, need to change to array.
-      existing = events[type] =
-        prepend ? [listener, existing] : [existing, listener];
+      existing = prepend ? [listener, existing] : [existing, listener];
+      existing[kEmitting] = 0;
+      events[type] = existing;
       // If we've already got an array, just append.
-    } else if (prepend) {
-      existing.unshift(listener);
     } else {
-      existing.push(listener);
+      existing = ensureMutableListenerArray(events, type, existing);
+      if (prepend) {
+        existing.unshift(listener);
+      } else {
+        existing.push(listener);
+      }
     }
 
     // Check for listener leak
@@ -674,7 +682,7 @@ EventEmitter.prototype.removeListener =
       if (events === undefined)
         return this;
 
-      const list = events[type];
+      let list = events[type];
       if (list === undefined)
         return this;
 
@@ -692,6 +700,7 @@ EventEmitter.prototype.removeListener =
         if (events.removeListener !== undefined)
           this.emit('removeListener', type, list.listener || listener);
       } else if (typeof list !== 'function') {
+        list = ensureMutableListenerArray(events, type, list);
         let position = -1;
 
         for (let i = list.length - 1; i >= 0; i--) {
@@ -875,6 +884,24 @@ function arrayClone(arr) {
   return ArrayPrototypeSlice(arr);
 }
 
+function cloneEventListenerArray(arr) {
+  const copy = arrayClone(arr);
+  copy[kEmitting] = 0;
+  if (arr.warned) {
+    copy.warned = true;
+  }
+  return copy;
+}
+
+function ensureMutableListenerArray(events, type, handler) {
+  if (handler[kEmitting] > 0) {
+    const copy = cloneEventListenerArray(handler);
+    events[type] = copy;
+    return copy;
+  }
+  return handler;
+}
+
 function unwrapListeners(arr) {
   const ret = arrayClone(arr);
   for (let i = 0; i < ret.length; ++i) {

filename: test/parallel/test-event-emitter-modify-in-emit.js
patch: @@ -78,3 +78,21 @@ assert.strictEqual(e.listeners('foo').length, 2);
 e.emit('foo');
 assert.deepStrictEqual(['callback2', 'callback3'], callbacks_called);
 assert.strictEqual(e.listeners('foo').length, 0);
+
+// Verify that removing all callbacks while in emit allows the current emit to
+// propagate to all listeners.
+callbacks_called = [];
+
+function callback4() {
+  callbacks_called.push('callback4');
+  e.removeAllListeners('foo');
+}
+
+e.on('foo', callback4);
+e.on('foo', callback2);
+e.on('foo', callback3);
+assert.strictEqual(e.listeners('foo').length, 3);
+e.emit('foo');
+assert.deepStrictEqual(['callback4', 'callback2', 'callback3'],
+                       callbacks_called);
+assert.strictEqual(e.listeners('foo').length, 0);