buffer: implement native C++ fast-path for Buffer.concat

[mertcanaltin]

added fast path for Buffer.concat

node-main benchmark result:

                                                                            confidence improvement accuracy (*)   (**)  (***)
buffers/buffer-concat-fill.js n=800000 extraSize=1                                 ***    -62.84 %       ±2.39% ±3.21% ±4.26%
buffers/buffer-concat-fill.js n=800000 extraSize=1024                              ***    -56.12 %       ±2.62% ±3.52% ±4.65%
buffers/buffer-concat-fill.js n=800000 extraSize=256                               ***    -60.57 %       ±2.27% ±3.05% ±4.03%
buffers/buffer-concat.js n=800000 withTotalLength=0 pieceSize=1 pieces=16          ***    -55.03 %       ±0.70% ±0.93% ±1.21%
buffers/buffer-concat.js n=800000 withTotalLength=0 pieceSize=1 pieces=4           ***    -81.81 %       ±2.59% ±3.49% ±4.63%
buffers/buffer-concat.js n=800000 withTotalLength=0 pieceSize=16 pieces=16         ***    -52.95 %       ±2.28% ±3.07% ±4.04%
buffers/buffer-concat.js n=800000 withTotalLength=0 pieceSize=16 pieces=4          ***    -80.98 %       ±0.63% ±0.84% ±1.10%
buffers/buffer-concat.js n=800000 withTotalLength=0 pieceSize=256 pieces=16        ***    -20.61 %       ±2.81% ±3.77% ±4.97%
buffers/buffer-concat.js n=800000 withTotalLength=0 pieceSize=256 pieces=4         ***    -71.75 %       ±1.50% ±2.02% ±2.66%
buffers/buffer-concat.js n=800000 withTotalLength=1 pieceSize=1 pieces=16          ***    -55.56 %       ±1.07% ±1.43% ±1.86%
buffers/buffer-concat.js n=800000 withTotalLength=1 pieceSize=1 pieces=4           ***    -81.98 %       ±1.25% ±1.68% ±2.23%
buffers/buffer-concat.js n=800000 withTotalLength=1 pieceSize=16 pieces=16         ***    -54.23 %       ±1.08% ±1.44% ±1.87%
buffers/buffer-concat.js n=800000 withTotalLength=1 pieceSize=16 pieces=4          ***    -80.96 %       ±1.68% ±2.26% ±2.99%
buffers/buffer-concat.js n=800000 withTotalLength=1 pieceSize=256 pieces=16        ***    -18.87 %       ±2.88% ±3.86% ±5.09%
buffers/buffer-concat.js n=800000 withTotalLength=1 pieceSize=256 pieces=4         ***    -71.40 %       ±2.44% ±3.29% ±4.36%

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

[codecov]

Codecov Report

Attention: Patch coverage is 70.11494% with 26 lines in your changes missing coverage. Please review.

Project coverage is 90.18%. Comparing base (b395420) to head (cf61beb).
Report is 32 commits behind head on main.

Files with missing lines	Patch %	Lines
src/node_buffer.cc	60.00%	18 Missing and 8 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #58409      +/-   ##
==========================================
- Coverage   90.22%   90.18%   -0.04%     
==========================================
  Files         633      635       +2     
  Lines      187027   187226     +199     
  Branches    36720    36758      +38     
==========================================
+ Hits       168746   168853     +107     
- Misses      11083    11139      +56     
- Partials     7198     7234      +36     

Files with missing lines	Coverage Δ
lib/buffer.js	99.27% <100.00%> (-0.73%)	⬇️
src/node_buffer.cc	69.10% <60.00%> (+<0.01%)	⬆️

... and 50 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.

[BridgeAR]

The benchmark result seems to be a huge regression?

[joyeecheung]

Oh wait, I misread, this isn't using fast APIs..

[mertcanaltin]

The benchmark result seems to be a huge regression?

I will investigate this in more detail

[anonrig]

Triggered benchmark ci: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1718/console

[juanarbol]

accuracy (*)   (**)  (***)
16:53:39 buffers/buffer-concat-fill.js n=800000 extraSize=1                                 ***    -67.28 %       ±0.58% ±0.78% ±1.02%
16:53:39 buffers/buffer-concat-fill.js n=800000 extraSize=1024                              ***    -56.67 %       ±0.39% ±0.52% ±0.67%
16:53:39 buffers/buffer-concat-fill.js n=800000 extraSize=256                               ***    -66.14 %       ±0.54% ±0.72% ±0.95%
16:53:39 buffers/buffer-concat.js n=800000 withTotalLength=0 pieceSize=1 pieces=16          ***    -45.73 %       ±0.25% ±0.33% ±0.44%
16:53:39 buffers/buffer-concat.js n=800000 withTotalLength=0 pieceSize=1 pieces=4           ***    -73.00 %       ±0.48% ±0.64% ±0.85%
16:53:39 buffers/buffer-concat.js n=800000 withTotalLength=0 pieceSize=16 pieces=16         ***    -45.46 %       ±0.24% ±0.32% ±0.42%
16:53:39 buffers/buffer-concat.js n=800000 withTotalLength=0 pieceSize=16 pieces=4          ***    -72.62 %       ±0.70% ±0.95% ±1.25%
16:53:39 buffers/buffer-concat.js n=800000 withTotalLength=0 pieceSize=256 pieces=16        ***     -6.88 %       ±0.57% ±0.76% ±0.99%
16:53:39 buffers/buffer-concat.js n=800000 withTotalLength=0 pieceSize=256 pieces=4         ***    -71.07 %       ±0.48% ±0.65% ±0.85%
16:53:39 buffers/buffer-concat.js n=800000 withTotalLength=1 pieceSize=1 pieces=16          ***    -45.22 %       ±0.22% ±0.29% ±0.38%
16:53:39 buffers/buffer-concat.js n=800000 withTotalLength=1 pieceSize=1 pieces=4           ***    -72.25 %       ±0.54% ±0.72% ±0.95%
16:53:39 buffers/buffer-concat.js n=800000 withTotalLength=1 pieceSize=16 pieces=16         ***    -45.45 %       ±0.37% ±0.49% ±0.64%
16:53:39 buffers/buffer-concat.js n=800000 withTotalLength=1 pieceSize=16 pieces=4          ***    -71.87 %       ±0.66% ±0.89% ±1.18%
16:53:39 buffers/buffer-concat.js n=800000 withTotalLength=1 pieceSize=256 pieces=16        ***     -5.75 %       ±0.37% ±0.49% ±0.64%
16:53:39 buffers/buffer-concat.js n=800000 withTotalLength=1 pieceSize=256 pieces=4         ***    -71.07 %       ±0.50% ±0.66% ±0.87%

seems slower, maybe it is the cost of context switching. Although I was also expecting a V8 fast api call by the description.

[juanarbol]

With the amount of PRs causing regressions by using fast APIs, I start to wonder if we should check whether removing existing fast APIs can improve performance in the newer versions of V8 (while it should improve performance in theory for bindings that would benefit from it, it seems quite often we are not picking the right target or the target isn't rewritten in a way that would benefit from it instead of getting a hit).

Sometimes the turbofan opts are simply better, sometimes the perf gain is almost nothing by adding a fast-api call; V8 is very unpredictable.

[mertcanaltin]

With the amount of PRs causing regressions by using fast APIs, I start to wonder if we should check whether removing existing fast APIs can improve performance in the newer versions of V8 (while it should improve performance in theory for bindings that would benefit from it, it seems quite often we are not picking the right target or the target isn't rewritten in a way that would benefit from it instead of getting a hit).

Sometimes the turbofan opts are simply better, sometimes the perf gain is almost nothing by adding a fast-api call; V8 is very unpredictable.

maybe I can make an improvement for Turbofan like you said here, but when I looked at it, I saw that it was already optimized.

[mertcanaltin]

Thank you everyone for the review, I hope I can take this place to a better contribution in the future, I am closing this place