console: optimize single-string logging

[gurgunday]

After:

console/log.js
console/log.js variant="plain" n=2000000: 10,760,828.681086333
console/log.js variant="format" n=2000000: 3,104,163.7037100764
console/log.js variant="object" n=2000000: 1,304,646.6897291192
console/log.js variant="group" n=2000000: 5,147,729.648404505
console/log.js variant="info" n=2000000: 10,760,667.090984877
console/log.js variant="warn" n=2000000: 10,730,321.278856428
console/log.js variant="error" n=2000000: 10,739,136.76713795

Before:

console/log.js
console/log.js variant="plain" n=2000000: 6,362,790.401730679
console/log.js variant="format" n=2000000: 3,067,171.4347502897
console/log.js variant="object" n=2000000: 1,260,652.1490095456
console/log.js variant="group" n=2000000: 2,917,442.398797694
console/log.js variant="info" n=2000000: 6,465,884.779549699
console/log.js variant="warn" n=2000000: 6,477,797.477083796
console/log.js variant="error" n=2000000: 6,345,277.494387991

This peaked my interest in the flame graph:

• Adds a fast path for when we log one string, which avoids ReflectApply and options validation. inspect has a similar fast path, but it's too late for console.log.
• Replaces a SafeWeakMap that acts as if it's present in every instance with a symbol. It should be faster since we avoid method calls and || on every invocation.

P.S. Couldn't find any benchmarks for console.log. And inspect benchmarks won't pick this up...

[nodejs-github-bot]

Review requested:

• @nodejs/performance

[BridgeAR]

It would also be possible to speed up other arguments by calling inspect directly for these. I guess that would also improve their performance a bit.

About the map vs symbol property: I doubt that it changes the performance, while it does make the property visible.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.56%. Comparing base (fb84f35) to head (03fec6a).
⚠️ Report is 59 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #60422      +/-   ##
==========================================
- Coverage   88.57%   88.56%   -0.02%     
==========================================
  Files         704      704              
  Lines      207778   207798      +20     
  Branches    40030    40027       -3     
==========================================
- Hits       184049   184040       -9     
- Misses      15768    15799      +31     
+ Partials     7961     7959       -2     

Files with missing lines	Coverage Δ
lib/internal/console/constructor.js	98.19% <100.00%> (+0.05%)	⬆️

... and 36 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[gurgunday]

@BridgeAR I'm getting a measurable, consistent difference:

With the WeakMap change:

console/log.js
console/log.js variant="plain" n=2000000: 10,883,048.308746813
console/log.js variant="format" n=2000000: 3,149,799.334788839
console/log.js variant="object" n=2000000: 1,264,378.6850980576
console/log.js variant="group" n=2000000: 5,179,205.360995469
console/log.js variant="info" n=2000000: 10,917,626.507997163
console/log.js variant="warn" n=2000000: 10,770,223.543582655
console/log.js variant="error" n=2000000: 10,905,653.149791192

Without it:

console/log.js
console/log.js variant="plain" n=2000000: 10,081,197.747104334
console/log.js variant="format" n=2000000: 3,018,537.215869073
console/log.js variant="object" n=2000000: 1,261,919.5199784338
console/log.js variant="group" n=2000000: 4,423,174.704583957
console/log.js variant="info" n=2000000: 10,469,331.438858952
console/log.js variant="warn" n=2000000: 10,366,440.721452443
console/log.js variant="error" n=2000000: 10,422,488.603660118

I'll let you decide, but it seems to be there

[aduh95]

Benchmark CI: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1749/

                                          confidence improvement accuracy (*)   (**)  (***)
console/log.js variant='error' n=2000000         ***     73.77 %       ±2.08% ±2.78% ±3.66%
console/log.js variant='format' n=2000000                 0.78 %       ±0.89% ±1.18% ±1.54%
console/log.js variant='group' n=2000000         ***    103.48 %       ±2.58% ±3.47% ±4.58%
console/log.js variant='info' n=2000000          ***     74.01 %       ±1.35% ±1.80% ±2.35%
console/log.js variant='object' n=2000000                -0.06 %       ±1.06% ±1.42% ±1.85%
console/log.js variant='plain' n=2000000         ***     73.56 %       ±2.21% ±2.96% ±3.90%
console/log.js variant='warn' n=2000000          ***     74.45 %       ±1.45% ±1.94% ±2.54%

Be aware that when doing many comparisons the risk of a false-positive
result increases. In this case, there are 7 comparisons, you can thus
expect the following amount of false-positive results:
  0.35 false positives, when considering a   5% risk acceptance (*, **, ***),
  0.07 false positives, when considering a   1% risk acceptance (**, ***),
  0.01 false positives, when considering a 0.1% risk acceptance (***)

[nodejs-github-bot]

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

[mcollina]

lgtm

[nodejs-github-bot]

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

[gurgunday]

Can we run the CI? I might have a good follow-up to this

[nodejs-github-bot]

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

[nodejs-github-bot]

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

[nodejs-github-bot]

Landed in c2a4acb