node:events: make on() O(1) again by appending in place

src/js/internal/streams/legacy.ts

@@ -112,7 +112,7 @@ function prependListener(emitter, event, fn) {
   let events, existing;
   if (!(events = emitter._events) || !(existing = events[event])) emitter.on(event, fn);
   // A fresh array, not unshift(): node:events iterates stored arrays without
-  // cloning, so a stored `_events` array must never be mutated in place.
+  // cloning, so an index-shifting mutation must install a copy.
   else if (ArrayIsArray(existing)) events[event] = [fn, ...existing];
   else events[event] = [fn, existing];
 }

src/js/node/events.ts

@@ -141,9 +141,7 @@ function emitError(emitter, args) {
   throw err; // Unhandled 'error' event
 }
 
-// A listener list is a bare function for a single listener, else an array
-// (like node). Arrays are never mutated in place - mutators install a copy -
-// so a stored list can be iterated with no defensive clone.
+// Single listener is stored bare, else an array (like node); see the emit() loop for why no clone.
 function applyHandlers(handlers, emitter, args) {
   if (typeof handlers === "function") {
     handlers.$apply(emitter, args);
@@ -208,9 +206,7 @@ const emitWithoutRejectionCapture = function emit(type, ...args) {
     }
     return true;
   }
-  // No defensive clone: stored arrays are never mutated in place (mutators
-  // install a copy), so this list stays stable for the whole loop even if a
-  // listener adds/removes listeners.
+  // No clone: prepend/remove install a fresh array, append lands past the `length` latched here.
   for (let i = 0, { length } = handler; i < length; i++) {
     const listener = handler[i];
     switch (args.length) {
@@ -268,9 +264,7 @@ const emitWithRejectionCapture = function emit(type, ...args) {
     }
     return true;
   }
-  // No defensive clone: stored arrays are never mutated in place (mutators
-  // install a copy), so this list stays stable for the whole loop even if a
-  // listener adds/removes listeners.
+  // No clone: prepend/remove install a fresh array, append lands past the `length` latched here.
   for (let i = 0, { length } = handler; i < length; i++) {
     const listener = handler[i];
     let result;
@@ -322,8 +316,12 @@ function _addListener(target, type, fn, prepend) {
   var handlers;
   if (typeof existing === "function") {
     handlers = events[type] = prepend ? [fn, existing] : [existing, fn];
+  } else if (prepend) {
+    handlers = events[type] = copyWithPrepended(existing, fn);
   } else {
-    handlers = events[type] = copyWithInserted(existing, fn, prepend);
+    // Append in place: emit() latches `length` first, so the pushed slot is never visited mid-emit.
+    $arrayPush(existing, fn);
+    handlers = existing;
   }
   var m = _getMaxListeners(target);
   if (m > 0 && handlers.length > m && !handlers.warned) {
@@ -343,20 +341,12 @@ EventEmitterPrototype.prependListener = function prependListener(type, fn) {
   return this;
 };
 
-// Copy-on-write: emit iterates stored arrays with no clone, so new listeners
-// land in a fresh array; `warned` carries over so the leak warning fires once.
-// An inline loop beats concat/slice here ~10x (host-call boundary).
-function copyWithInserted(list, fn, prepend) {
+// Fresh array (prepend shifts indices an in-flight emit() is iterating); inline loop beats [fn, ...list] ~10x.
+function copyWithPrepended(list, fn) {
   const n = list.length;
   const copy = $newArrayWithSize(n + 1);
-  // Two straight copies, not a per-element ternary (measured ~25% slower).
-  if (prepend) {
-    copy[0] = fn;
-    for (let i = 0; i < n; i++) copy[i + 1] = list[i];
-  } else {
-    for (let i = 0; i < n; i++) copy[i] = list[i];
-    copy[n] = fn;
-  }
+  copy[0] = fn;
+  for (let i = 0; i < n; i++) copy[i + 1] = list[i];
   if (list.warned) copy.warned = true;
   return copy;
 }
@@ -447,8 +437,7 @@ EventEmitterPrototype.removeListener = function removeListener(type, listener) {
   }
   if (position < 0) return this;
 
-  // Copy-remove (arrays are never mutated in place), and store a lone
-  // survivor bare like node does, so `_events[type]` shape matches theirs.
+  // Copy-remove so an in-flight emit() keeps a stable view; store a lone survivor bare like node.
   const n = list.length;
   const copy = $newArrayWithSize(n - 1);
   for (let i = 0, j = 0; i < n; i++) {

test/js/node/events/event-emitter.test.ts

@@ -1,6 +1,6 @@
 import { sleep } from "bun";
 import { describe, expect, mock, test } from "bun:test";
-import { bunEnv, bunExe } from "harness";
+import { bunEnv, bunExe, isASAN, isDebug } from "harness";
 import { createRequire } from "module";
 
 // this is also testing that imports with default and named imports in the same statement work
@@ -1093,3 +1093,40 @@ test("once() wrapper releases its target after firing", async () => {
     exitCode: 0,
   });
 });
+
+// on() must be amortized O(1). A copy-on-write append pays N per add, so N
+// adds cost N^2: 12k adds took ~270ms release / ~1.6s debug+ASAN versus <2ms /
+// ~70ms for an in-place push.
+test("on() is amortized O(1), not O(N) per add", () => {
+  const fn = () => {};
+  function timeAdds(n: number) {
+    const ee = new EventEmitter();
+    ee.setMaxListeners(0);
+    const t0 = performance.now();
+    for (let i = 0; i < n; i++) ee.on("x", fn);
+    return performance.now() - t0;
+  }
+  timeAdds(2000); // warm up
+  const ms = timeAdds(12000);
+  expect(ms).toBeLessThan(isDebug || isASAN ? 500 : 100);
+});
+
+// on() during emit must not run the new listener in that same emit round
+// (matches Node). This is the invariant the in-place append relies on: emit
+// latches `length` before iterating, so the pushed slot is never visited.
+test("listener appended during emit is not called in that emit", () => {
+  const ee = new EventEmitter();
+  const calls: string[] = [];
+  ee.on("x", () => {
+    calls.push("a");
+    ee.on("x", () => calls.push("late"));
+  });
+  ee.on("x", () => calls.push("b"));
+  ee.emit("x");
+  expect(calls).toEqual(["a", "b"]);
+  expect(ee.listenerCount("x")).toBe(3);
+
+  calls.length = 0;
+  ee.emit("x");
+  expect(calls).toEqual(["a", "b", "late"]);
+});