events: optimize once() and removeListener()

Use a closure instead of a bound state object for the once() wrapper,
keep the events object shape in removeListener() by storing undefined
instead of deleting keys or reallocating the object, and outline the
cold unhandled 'error' path from emit() and the max-listeners warning
from _addListener() so both fit within V8 inlining budgets.

ee-once +25%, ee-add-remove +12-22%, ee-listen-unique +7-15%,
streams/pipe +13.9%.

Signed-off-by: Matteo Collina <hello@matteocollina.com>
PR-URL: #64373
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

Author: mcollina

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)