src,lib: optimize nodeTiming.uvMetricsInfo

[RafaelGSS]

$ node benchmark/compare.js --old ./node-without --new ./node-with --filter "nodetiming" perf_hooks > res.out && node-benchmark-compare res.out
[00:00:43|% 100| 1/1 files | 60/60 runs | 3/3 configs]: Done
                                                               confidence improvement accuracy (*)    (**)   (***)
perf_hooks/nodetiming-uvmetricsinfo.js events=1 n=1000000            ***    467.03 %      ±12.50% ±16.82% ±22.27%
perf_hooks/nodetiming-uvmetricsinfo.js events=1000 n=1000000         ***    479.78 %      ±11.29% ±15.17% ±20.04%
perf_hooks/nodetiming-uvmetricsinfo.js events=10000 n=1000000        ***    486.76 %       ±9.85% ±13.23% ±17.46%

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

[nodejs-github-bot]

Review requested:

• @nodejs/performance

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.45%. Comparing base (c63255b) to head (4df0ff2).
Report is 35 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #55614      +/-   ##
==========================================
+ Coverage   88.43%   88.45%   +0.01%     
==========================================
  Files         654      654              
  Lines      187662   187751      +89     
  Branches    36117    36149      +32     
==========================================
+ Hits       165962   166066     +104     
+ Misses      14938    14937       -1     
+ Partials     6762     6748      -14     

Files with missing lines	Coverage Δ
lib/internal/perf/nodetiming.js	94.28% <100.00%> (+0.23%)	⬆️
src/node_perf.cc	86.44% <100.00%> (-0.40%)	⬇️

... and 43 files with indirect coverage changes

[mcollina]

lgtm

[nodejs-github-bot]

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

[nodejs-github-bot]

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

[lemire]

@RafaelGSS

Why is this PR faster?

You create an array from C++, return it to JavaScript, then you convert the array into an object.

Previously, it just returned an object.

What is the catch?

[RafaelGSS]

@lemire The simple answer is: creating an object and transferring from C++ to is costly. Instead of returning just arraysize of 3 uint64_t an object requires allocation of each field value (for instance, event_label) as well as the object structure

In my experience, anything that is sent from C++ to JS (and vice versa) is costly and when you reduce the object size you very often get a performance gain.

[H4ad]

Just change the initial commit message, I think is not reflecting the current changes.

[lemire]

@RafaelGSS

The simple answer is: creating an object and transferring from C++ to is costly. Instead of returning just arraysize of 3 uint64_t an object requires allocation of each field value (for instance, event_label) as well as the object structure

This makes sense, but isn't there a design issue in the first place in the fact that a performance sensitive function (in JavaScript) returns an object?

[RafaelGSS]

Landed in 9b6cea6...d83e9fa

[RafaelGSS]

This makes sense, but isn't there a design issue in the first place in the fact that a performance sensitive function (in JavaScript) returns an object?

Maybe... but I believe this falls more on the V8 side than the Node.js one. I haven't investigated that communication closely so I might not be able to answer it with precision.