lib: revert primordials in a hot path

Evidence has shown that use of primordials have sometimes an impact of
performance. This commit reverts the changes who are most likely to be
responsible for performance regression in the HTTP response path.

PR-URL: #38248
Backport-PR-URL: #38972
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Rich Trott <[email protected]>

Author: aduh95

lib/_http_outgoing.js

@@ -380,7 +380,8 @@ function _storeHeader(firstLine, headers) {
       }
     } else if (ArrayIsArray(headers)) {
       if (headers.length && ArrayIsArray(headers[0])) {
-        for (const entry of headers) {
+        for (let i = 0; i < headers.length; i++) {
+          const entry = headers[i];
           processHeader(this, state, entry[0], entry[1], true);
         }
       } else {

lib/_http_server.js

@@ -398,8 +398,9 @@ Server.prototype[EE.captureRejectionSymbol] = function(err, event, ...args) {
       const [ , res] = args;
       if (!res.headersSent && !res.writableEnded) {
         // Don't leak headers.
-        for (const name of res.getHeaderNames()) {
-          res.removeHeader(name);
+        const names = res.getHeaderNames();
+        for (let i = 0; i < names.length; i++) {
+          res.removeHeader(names[i]);
         }
         res.statusCode = 500;
         res.end(STATUS_CODES[500]);
@@ -409,7 +410,7 @@ Server.prototype[EE.captureRejectionSymbol] = function(err, event, ...args) {
       break;
     default:
       net.Server.prototype[SymbolFor('nodejs.rejection')]
-        .call(this, err, event, ...args);
+        .apply(this, arguments);
   }
 };
 

lib/events.js

@@ -22,13 +22,10 @@
 'use strict';
 
 const {
-  ArrayPrototypeForEach,
-  ArrayPrototypePush,
   ArrayPrototypeSlice,
   Boolean,
   Error,
   ErrorCaptureStackTrace,
-  FunctionPrototypeCall,
   MathMin,
   NumberIsNaN,
   ObjectCreate,
@@ -39,7 +36,6 @@ const {
   Promise,
   PromiseReject,
   PromiseResolve,
-  ReflectApply,
   ReflectOwnKeys,
   String,
   Symbol,
@@ -84,7 +80,7 @@ const lazyDOMException = hideStackFrames((message, name) => {
 
 
 function EventEmitter(opts) {
-  FunctionPrototypeCall(EventEmitter.init, this, opts);
+  EventEmitter.init.call(this, opts);
 }
 module.exports = EventEmitter;
 module.exports.once = once;
@@ -175,8 +171,8 @@ EventEmitter.setMaxListeners =
       if (isEventTarget === undefined)
         isEventTarget = require('internal/event_target').isEventTarget;
 
-      // Performance for forEach is now comparable with regular for-loop
-      ArrayPrototypeForEach(eventTargets, (target) => {
+      for (let i = 0; i < eventTargets.length; i++) {
+        const target = eventTargets[i];
         if (isEventTarget(target)) {
           target[kMaxEventTargetListeners] = n;
           target[kMaxEventTargetListenersWarned] = false;
@@ -188,7 +184,7 @@ EventEmitter.setMaxListeners =
             ['EventEmitter', 'EventTarget'],
             target);
         }
-      });
+      }
     }
   };
 
@@ -227,7 +223,7 @@ function addCatch(that, promise, type, args) {
     const then = promise.then;
 
     if (typeof then === 'function') {
-      FunctionPrototypeCall(then, promise, undefined, function(err) {
+      then.call(promise, undefined, function(err) {
         // The callback is called with nextTick to avoid a follow-up
         // rejection from this promise.
         process.nextTick(emitUnhandledRejectionOrErr, that, err, type, args);
@@ -376,7 +372,7 @@ EventEmitter.prototype.emit = function emit(type, ...args) {
     return false;
 
   if (typeof handler === 'function') {
-    const result = ReflectApply(handler, this, args);
+    const result = handler.apply(this, args);
 
     // We check if result is undefined first because that
     // is the most common case so we do not pay any perf
@@ -388,7 +384,7 @@ EventEmitter.prototype.emit = function emit(type, ...args) {
     const len = handler.length;
     const listeners = arrayClone(handler);
     for (let i = 0; i < len; ++i) {
-      const result = ReflectApply(listeners[i], this, args);
+      const result = listeners[i].apply(this, args);
 
       // We check if result is undefined first because that
       // is the most common case so we do not pay any perf
@@ -698,7 +694,7 @@ function getEventListeners(emitterOrTarget, type) {
     const listeners = [];
     let handler = root?.next;
     while (handler?.listener !== undefined) {
-      ArrayPrototypePush(listeners, handler.listener);
+      listeners.push(handler.listener);
       handler = handler.next;
     }
     return listeners;
@@ -842,7 +838,7 @@ function on(emitter, event, options) {
 
       // Wait until an event happens
       return new Promise(function(resolve, reject) {
-        ArrayPrototypePush(unconsumedPromises, { resolve, reject });
+        unconsumedPromises.push({ resolve, reject });
       });
     },
 
@@ -906,7 +902,7 @@ function on(emitter, event, options) {
     if (promise) {
       promise.resolve(createIterResult(args, false));
     } else {
-      ArrayPrototypePush(unconsumedEvents, args);
+      unconsumedEvents.push(args);
     }
   }
 

lib/internal/async_hooks.js

@@ -1,15 +1,11 @@
 'use strict';
 
 const {
-  ArrayPrototypePop,
   ArrayPrototypeSlice,
-  ArrayPrototypeUnshift,
   ErrorCaptureStackTrace,
-  FunctionPrototypeBind,
   ObjectPrototypeHasOwnProperty,
   ObjectDefineProperty,
   Promise,
-  ReflectApply,
   Symbol,
 } = primordials;
 
@@ -129,16 +125,16 @@ function callbackTrampoline(asyncId, resource, cb, ...args) {
 
   let result;
   if (asyncId === 0 && typeof domain_cb === 'function') {
-    ArrayPrototypeUnshift(args, cb);
-    result = ReflectApply(domain_cb, this, args);
+    args.unshift(cb);
+    result = domain_cb.apply(this, args);
   } else {
-    result = ReflectApply(cb, this, args);
+    result = cb.apply(this, args);
   }
 
   if (asyncId !== 0 && hasHooks(kAfter))
     emitAfterNative(asyncId);
 
-  ArrayPrototypePop(execution_async_resources);
+  execution_async_resources.pop();
   return result;
 }
 
@@ -256,7 +252,7 @@ function emitHook(symbol, asyncId) {
 }
 
 function emitHookFactory(symbol, name) {
-  const fn = FunctionPrototypeBind(emitHook, undefined, symbol);
+  const fn = emitHook.bind(undefined, symbol);
 
   // Set the name property of the function as it looks good in the stack trace.
   ObjectDefineProperty(fn, 'name', {
@@ -429,14 +425,14 @@ function clearDefaultTriggerAsyncId() {
 
 function defaultTriggerAsyncIdScope(triggerAsyncId, block, ...args) {
   if (triggerAsyncId === undefined)
-    return ReflectApply(block, null, args);
+    return block.apply(null, args);
   // CHECK(NumberIsSafeInteger(triggerAsyncId))
   // CHECK(triggerAsyncId > 0)
   const oldDefaultTriggerAsyncId = async_id_fields[kDefaultTriggerAsyncId];
   async_id_fields[kDefaultTriggerAsyncId] = triggerAsyncId;
 
   try {
-    return ReflectApply(block, null, args);
+    return block.apply(null, args);
   } finally {
     async_id_fields[kDefaultTriggerAsyncId] = oldDefaultTriggerAsyncId;
   }
@@ -533,7 +529,7 @@ function popAsyncContext(asyncId) {
   const offset = stackLength - 1;
   async_id_fields[kExecutionAsyncId] = async_wrap.async_ids_stack[2 * offset];
   async_id_fields[kTriggerAsyncId] = async_wrap.async_ids_stack[2 * offset + 1];
-  ArrayPrototypePop(execution_async_resources);
+  execution_async_resources.pop();
   async_hook_fields[kStackLength] = offset;
   return offset > 0;
 }

lib/internal/per_context/primordials.js

@@ -6,11 +6,9 @@
 // so that Node.js's builtin modules do not need to later look these up from
 // the global proxy, which can be mutated by users.
 
-// TODO(joyeecheung): we can restrict access to these globals in builtin
-// modules through the JS linter, for example: ban access such as `Object`
-// (which falls back to a lookup in the global proxy) in favor of
-// `primordials.Object` where `primordials` is a lexical variable passed
-// by the native module compiler.
+// Use of primordials have sometimes a dramatic impact on performance, please
+// benchmark all changes made in performance-sensitive areas of the codebase.
+// See: https://github.com/nodejs/node/pull/38248
 
 // `uncurryThis` is equivalent to `func => Function.prototype.call.bind(func)`.
 // It is using `bind.bind(call)` to avoid using `Function.prototype.bind`

lib/internal/streams/readable.js

@@ -823,8 +823,8 @@ Readable.prototype.unpipe = function(dest) {
     state.pipes = [];
     this.pause();
 
-    for (const dest of dests)
-      dest.emit('unpipe', this, { hasUnpiped: false });
+    for (let i = 0; i < dests.length; i++)
+      dests[i].emit('unpipe', this, { hasUnpiped: false });
     return this;
   }
 

lib/internal/timers.js

@@ -78,8 +78,8 @@ const {
   NumberIsFinite,
   NumberMIN_SAFE_INTEGER,
   ObjectCreate,
-  Symbol,
   ReflectApply,
+  Symbol,
 } = primordials;
 
 const {

lib/net.js

@@ -24,18 +24,13 @@
 const {
   ArrayIsArray,
   ArrayPrototypeIndexOf,
-  ArrayPrototypePush,
-  ArrayPrototypeSplice,
   Boolean,
   Error,
-  FunctionPrototype,
-  FunctionPrototypeCall,
   Number,
   NumberIsNaN,
   NumberParseInt,
   ObjectDefineProperty,
   ObjectSetPrototypeOf,
-  ReflectApply,
   Symbol,
 } = primordials;
 
@@ -129,7 +124,7 @@ const DEFAULT_IPV6_ADDR = '::';
 
 const isWindows = process.platform === 'win32';
 
-const noop = FunctionPrototype;
+const noop = () => {};
 
 function getFlags(ipv6Only) {
   return ipv6Only === true ? TCPConstants.UV_TCP_IPV6ONLY : 0;
@@ -304,7 +299,7 @@ function Socket(options) {
   options.autoDestroy = false;
   // Handle strings directly.
   options.decodeStrings = false;
-  ReflectApply(stream.Duplex, this, [options]);
+  stream.Duplex.call(this, options);
 
   // Default to *not* allowing half open sockets.
   this.allowHalfOpen = Boolean(allowHalfOpen);
@@ -594,7 +589,8 @@ Socket.prototype._read = function(n) {
 
 
 Socket.prototype.end = function(data, encoding, callback) {
-  ReflectApply(stream.Duplex.prototype.end, this, [data, encoding, callback]);
+  stream.Duplex.prototype.end.call(this,
+                                   data, encoding, callback);
   DTRACE_NET_STREAM_END(this);
   return this;
 };
@@ -610,7 +606,7 @@ Socket.prototype.pause = function() {
         this.destroy(errnoException(err, 'read'));
     }
   }
-  return FunctionPrototypeCall(stream.Duplex.prototype.pause, this);
+  return stream.Duplex.prototype.pause.call(this);
 };
 
 
@@ -619,7 +615,7 @@ Socket.prototype.resume = function() {
       !this._handle.reading) {
     tryReadStart(this);
   }
-  return FunctionPrototypeCall(stream.Duplex.prototype.resume, this);
+  return stream.Duplex.prototype.resume.call(this);
 };
 
 
@@ -628,7 +624,7 @@ Socket.prototype.read = function(n) {
       !this._handle.reading) {
     tryReadStart(this);
   }
-  return ReflectApply(stream.Duplex.prototype.read, this, [n]);
+  return stream.Duplex.prototype.read.call(this, n);
 };
 
 
@@ -1167,7 +1163,7 @@ function Server(options, connectionListener) {
   if (!(this instanceof Server))
     return new Server(options, connectionListener);
 
-  FunctionPrototypeCall(EventEmitter, this);
+  EventEmitter.call(this);
 
   if (typeof options === 'function') {
     connectionListener = options;
@@ -1693,10 +1689,10 @@ ObjectDefineProperty(Socket.prototype, '_handle', {
 
 Server.prototype._setupWorker = function(socketList) {
   this._usingWorkers = true;
-  ArrayPrototypePush(this._workers, socketList);
+  this._workers.push(socketList);
   socketList.once('exit', (socketList) => {
     const index = ArrayPrototypeIndexOf(this._workers, socketList);
-    ArrayPrototypeSplice(this._workers, index, 1);
+    this._workers.splice(index, 1);
   });
 };
 

test/parallel/test-stream-pipe-await-drain.js

@@ -27,7 +27,7 @@ writer1.once('chunk-received', () => {
     reader._readableState.awaitDrainWriters.size,
     0,
     'awaitDrain initial value should be 0, actual is ' +
-    reader._readableState.awaitDrainWriters
+    reader._readableState.awaitDrainWriters.size
   );
   setImmediate(() => {
     // This one should *not* get through to writer1 because writer2 is not