events: optimize once() and removeListener()

lib/events.js

@@ -86,7 +86,6 @@ 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 =
@@ -335,9 +334,6 @@ EventEmitter.init = function(opts) {
       this._events === ObjectGetPrototypeOf(this)._events) {
     this._events = { __proto__: null };
     this._eventsCount = 0;
-    this[kShapeMode] = false;
-  } else {
-    this[kShapeMode] = true;
   }
 
   this._maxListeners ||= undefined;
@@ -446,6 +442,39 @@ function enhanceStackTrace(err, own) {
   return err.stack + sep + ArrayPrototypeJoin(ownStack, '\n');
 }
 
+function getUnhandledErrorException(ee, args) {
+  let er;
+  if (args.length > 0)
+    er = args[0];
+  if (er instanceof Error) {
+    try {
+      const capture = {};
+      ErrorCaptureStackTrace(capture, EventEmitter.prototype.emit);
+      ObjectDefineProperty(er, kEnhanceStackBeforeInspector, {
+        __proto__: null,
+        value: FunctionPrototypeBind(enhanceStackTrace, ee, er, capture),
+        configurable: true,
+      });
+    } catch {
+      // Continue regardless of error.
+    }
+
+    return er;
+  }
+
+  let stringifiedEr;
+  try {
+    stringifiedEr = inspect(er);
+  } catch {
+    stringifiedEr = er;
+  }
+
+  // At least give some kind of context to the user
+  const err = new ERR_UNHANDLED_ERROR(stringifiedEr);
+  err.context = er;
+  return err;
+}
+
 /**
  * Synchronously calls each of the listeners registered
  * for the event.
@@ -466,38 +495,10 @@ EventEmitter.prototype.emit = function emit(type, ...args) {
 
   // If there is no 'error' event listener then throw.
   if (doError) {
-    let er;
-    if (args.length > 0)
-      er = args[0];
-    if (er instanceof Error) {
-      try {
-        const capture = {};
-        ErrorCaptureStackTrace(capture, EventEmitter.prototype.emit);
-        ObjectDefineProperty(er, kEnhanceStackBeforeInspector, {
-          __proto__: null,
-          value: FunctionPrototypeBind(enhanceStackTrace, this, er, capture),
-          configurable: true,
-        });
-      } catch {
-        // Continue regardless of error.
-      }
-
-      // Note: The comments on the `throw` lines are intentional, they show
-      // up in Node's output if this results in an unhandled exception.
-      throw er; // Unhandled 'error' event
-    }
-
-    let stringifiedEr;
-    try {
-      stringifiedEr = inspect(er);
-    } catch {
-      stringifiedEr = er;
-    }
-
-    // At least give some kind of context to the user
-    const err = new ERR_UNHANDLED_ERROR(stringifiedEr);
-    err.context = er;
-    throw err; // Unhandled 'error' event
+    const er = getUnhandledErrorException(this, args);
+    // Note: The comments on the `throw` lines are intentional, they show
+    // up in Node's output if this results in an unhandled exception.
+    throw er; // Unhandled 'error' event
   }
 
   const handler = events[type];
@@ -584,20 +585,23 @@ function _addListener(target, type, listener, prepend) {
 
     // Check for listener leak
     m = _getMaxListeners(target);
-    if (m > 0 && existing.length > m && !existing.warned) {
-      existing.warned = true;
-      // No error code for this since it is a Warning
-      const w = genericNodeError(
-        `Possible EventEmitter memory leak detected. ${existing.length} ${String(type)} listeners ` +
-        `added to ${inspect(target, { depth: -1 })}. MaxListeners is ${m}. Use emitter.setMaxListeners() to increase limit`,
-        { name: 'MaxListenersExceededWarning', emitter: target, type: type, count: existing.length });
-      process.emitWarning(w);
-    }
+    if (m > 0 && existing.length > m && !existing.warned)
+      warnMaxListenersExceeded(target, type, existing, m);
   }
 
   return target;
 }
 
+function warnMaxListenersExceeded(target, type, existing, m) {
+  existing.warned = true;
+  // No error code for this since it is a Warning
+  const w = genericNodeError(
+    `Possible EventEmitter memory leak detected. ${existing.length} ${String(type)} listeners ` +
+    `added to ${inspect(target, { depth: -1 })}. MaxListeners is ${m}. Use emitter.setMaxListeners() to increase limit`,
+    { name: 'MaxListenersExceededWarning', emitter: target, type: type, count: existing.length });
+  process.emitWarning(w);
+}
+
 /**
  * Adds a listener to the event emitter.
  * @param {string | symbol} type
@@ -622,22 +626,16 @@ EventEmitter.prototype.prependListener =
       return _addListener(this, type, listener, true);
     };
 
-function onceWrapper() {
-  if (!this.fired) {
-    this.target.removeListener(this.type, this.wrapFn);
-    this.fired = true;
-    if (arguments.length === 0)
-      return this.listener.call(this.target);
-    return ReflectApply(this.listener, this.target, arguments);
-  }
-}
-
 function _onceWrap(target, type, listener) {
-  const state = { fired: false, wrapFn: undefined, target, type, listener };
-  const wrapped = onceWrapper.bind(state);
-  wrapped.listener = listener;
-  state.wrapFn = wrapped;
-  return wrapped;
+  let fired = false;
+  function wrapper(...args) {
+    if (fired) return;
+    fired = true;
+    target.removeListener(type, wrapper);
+    return ReflectApply(listener, target, args);
+  }
+  wrapper.listener = listener;
+  return wrapper;
 }
 
 /**
@@ -689,13 +687,12 @@ EventEmitter.prototype.removeListener =
       if (list === listener || list.listener === listener) {
         this._eventsCount -= 1;
 
-        if (this[kShapeMode]) {
-          events[type] = undefined;
-        } else if (this._eventsCount === 0) {
-          this._events = { __proto__: null };
-        } else {
-          delete events[type];
-        }
+        // Leave the key in place with an `undefined` value: repeatedly
+        // adding and removing a listener for the same event this way keeps
+        // the `events` object in the same shape and avoids both a `delete`
+        // (which would put the object into dictionary mode) and allocating
+        // a fresh object when the last listener is removed.
+        events[type] = undefined;
 
         if (events.removeListener !== undefined)
           this.emit('removeListener', type, list.listener || listener);
@@ -755,7 +752,6 @@ EventEmitter.prototype.removeAllListeners =
           else
             delete events[type];
         }
-        this[kShapeMode] = false;
         return this;
       }
 
@@ -768,7 +764,6 @@ EventEmitter.prototype.removeAllListeners =
         this.removeAllListeners('removeListener');
         this._events = { __proto__: null };
         this._eventsCount = 0;
-        this[kShapeMode] = false;
         return this;
       }
 
@@ -868,7 +863,16 @@ EventEmitter.prototype.listenerCount = function listenerCount(type, listener) {
  * @returns {(string | symbol)[]}
  */
 EventEmitter.prototype.eventNames = function eventNames() {
-  return this._eventsCount > 0 ? ReflectOwnKeys(this._events) : [];
+  if (this._eventsCount === 0)
+    return [];
+  const events = this._events;
+  const names = [];
+  for (const key of ReflectOwnKeys(events)) {
+    // Removed listeners leave the key in place with an `undefined` value.
+    if (events[key] !== undefined)
+      ArrayPrototypePush(names, key);
+  }
+  return names;
 };
 
 function arrayClone(arr) {

test/fixtures/errors/events_unhandled_error_common_trace.snapshot

@@ -1,6 +1,6 @@
 node:events:<line>
-      throw er; // Unhandled 'error' event
-      ^
+    throw er; // Unhandled 'error' event
+    ^
 
 Error: foo:bar
     at bar (<project-root>/test/fixtures/errors/events_unhandled_error_common_trace.js:9:12)

test/fixtures/errors/events_unhandled_error_nexttick.snapshot

@@ -1,6 +1,6 @@
 node:events:<line>
-      throw er; // Unhandled 'error' event
-      ^
+    throw er; // Unhandled 'error' event
+    ^
 
 Error
     at Object.<anonymous> (<project-root>/test/fixtures/errors/events_unhandled_error_nexttick.js:6:12)

test/fixtures/errors/events_unhandled_error_sameline.snapshot

@@ -1,6 +1,6 @@
 node:events:<line>
-      throw er; // Unhandled 'error' event
-      ^
+    throw er; // Unhandled 'error' event
+    ^
 
 Error
     at Object.<anonymous> (<project-root>/test/fixtures/errors/events_unhandled_error_sameline.js:6:34)

test/fixtures/errors/events_unhandled_error_subclass.snapshot

@@ -1,6 +1,6 @@
 node:events:<line>
-      throw er; // Unhandled 'error' event
-      ^
+    throw er; // Unhandled 'error' event
+    ^
 
 Error
     at Object.<anonymous> (<project-root>/test/fixtures/errors/events_unhandled_error_subclass.js:7:25)