Repository: nodejs/node
Pull request: lib: flag to conditionally modify proto on deprecate

Refs: https://github.com/nodejs/node/issues/58218

In this specific benchmark, the performance difference between `modifyPrototype=0` and `modifyPrototype=1` is marginally significant at the 5% level:

* **Mean (modifyPrototype=0)**: 390,196.99 ops/sec
* **Mean (modifyPrototype=1)**: 382,521.21 ops/sec
* **p-value**: 0.05087

While this case is borderline, other benchmarks (e.g., [https://github.com/nodejs/node/pull/58907](https://github.com/nodejs/node/pull/58907)) have shown a more noticeable improvement, so I think this worth the change.


Changed files:
filename: benchmark/util/deprecate.js
patch: @@ -0,0 +1,36 @@
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+
+const bench = common.createBenchmark(main, {
+  n: [1e5],
+  modifyPrototype: [1, 0],
+  emitWarningSync: [1, 0],
+}, {
+  flags: ['--expose-internals'],
+});
+
+function simpleFunction(x) {
+  return x * 2 + (new Array(1000)).fill(0).map((_, i) => i).reduce((a, b) => a + b, 0);
+}
+
+function main({ n, modifyPrototype, emitWarningSync }) {
+  const { deprecate } = require('internal/util');
+
+  const fn = deprecate(
+    simpleFunction,
+    'This function is deprecated',
+    'DEP0000',
+    emitWarningSync,
+    !!modifyPrototype,
+  );
+
+  let sum = 0;
+  bench.start();
+  for (let i = 0; i < n; ++i) {
+    sum += fn(i);
+  }
+  bench.end(n);
+  assert.ok(sum);
+}

filename: lib/internal/util.js
patch: @@ -169,7 +169,7 @@ function pendingDeprecate(fn, msg, code) {
 // Mark that a method should not be used.
 // Returns a modified function which warns once by default.
 // If --no-deprecation is set, then it is a no-op.
-function deprecate(fn, msg, code, useEmitSync) {
+function deprecate(fn, msg, code, useEmitSync, modifyPrototype = true) {
   // Lazy-load to avoid a circular dependency.
   if (validateString === undefined)
     ({ validateString } = require('internal/validators'));
@@ -192,19 +192,23 @@ function deprecate(fn, msg, code, useEmitSync) {
     return ReflectApply(fn, this, args);
   }
 
-  // The wrapper will keep the same prototype as fn to maintain prototype chain
-  ObjectSetPrototypeOf(deprecated, fn);
-  if (fn.prototype) {
-    // Setting this (rather than using Object.setPrototype, as above) ensures
-    // that calling the unwrapped constructor gives an instanceof the wrapped
-    // constructor.
-    deprecated.prototype = fn.prototype;
-  }
+  if (modifyPrototype) {
+    // The wrapper will keep the same prototype as fn to maintain prototype chain
+    // Modifying the prototype does alter the object chains, and as observed in
+    // most cases, it slows the code.
+    ObjectSetPrototypeOf(deprecated, fn);
+    if (fn.prototype) {
+      // Setting this (rather than using Object.setPrototype, as above) ensures
+      // that calling the unwrapped constructor gives an instanceof the wrapped
+      // constructor.
+      deprecated.prototype = fn.prototype;
+    }
 
-  ObjectDefineProperty(deprecated, 'length', {
-    __proto__: null,
-    ...ObjectGetOwnPropertyDescriptor(fn, 'length'),
-  });
+    ObjectDefineProperty(deprecated, 'length', {
+      __proto__: null,
+      ...ObjectGetOwnPropertyDescriptor(fn, 'length'),
+    });
+  }
 
   return deprecated;
 }