http: correctly optimize debug function

[targos]

Exporting a variable that will be mutated later doesn't work.

Refs: #37937

[targos]

@mcollina can you verify the fix on your benchmarking server?

[mcollina]

Yes it does but not completely. There are probably a few others but this does improve things if landed on top of #37963 (nothing otherwise as the other commit is a dominant bottleneck).

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/37039/

[mcollina]

$ git diff
diff --git a/lib/internal/util/debuglog.js b/lib/internal/util/debuglog.js
index 3dc7a5157d..6eec7b28f2 100644
--- a/lib/internal/util/debuglog.js
+++ b/lib/internal/util/debuglog.js
@@ -81,7 +81,7 @@ function debuglog(set, cb) {
     debug = debuglogImpl(enabled, set);
     if (typeof cb === 'function')
       cb(debug);
-    debug(...new SafeArrayIterator(args));
+    debug(...args);
   };
   let enabled;
   let test = () => {
@@ -89,7 +89,7 @@ function debuglog(set, cb) {
     test = () => enabled;
     return enabled;
   };
-  const logger = (...args) => debug(...new SafeArrayIterator(args));
+  const logger = (...args) => debug(...args);
   ObjectDefineProperty(logger, 'enabled', {
     get() {
       return test();

Is still slightly faster than this fix alone (72k vs 70k).

Can we add this as well?

[targos]

$ git diff
diff --git a/lib/internal/util/debuglog.js b/lib/internal/util/debuglog.js
index 3dc7a5157d..6eec7b28f2 100644
--- a/lib/internal/util/debuglog.js
+++ b/lib/internal/util/debuglog.js
@@ -81,7 +81,7 @@ function debuglog(set, cb) {
     debug = debuglogImpl(enabled, set);
     if (typeof cb === 'function')
       cb(debug);
-    debug(...new SafeArrayIterator(args));
+    debug(...args);
   };
   let enabled;
   let test = () => {
@@ -89,7 +89,7 @@ function debuglog(set, cb) {
     test = () => enabled;
     return enabled;
   };
-  const logger = (...args) => debug(...new SafeArrayIterator(args));
+  const logger = (...args) => debug(...args);
   ObjectDefineProperty(logger, 'enabled', {
     get() {
       return test();

Is still slightly faster than this fix alone (72k vs 70k).

Can we add this as well?

I can add it, but I honestly don't see how the difference can be measured, as this code path is hit only eight times during the entire process life, regardless of the number of HTTP requests.

[mcollina]

lgtm

[mcollina]

I can add it, but I honestly don't see how the difference can be measured, as this code path is hit only eight times during the entire process life, regardless of the number of HTTP requests.

I'll check it further and verify again.

[mcollina]

This is ok to land, there are no other regression (I checked extensively)

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/37042/

[targos]

Landed in 3dee233