Repository: nodejs/node
Pull request: buffer: optimize buffer.concat performance

Removed the _copyActual indirection in the copy loop and called TypedArrayPrototypeSet directly.
Split auto-length and explicit-length paths so the auto-length copy loop is branch free. Replaced Buffer.allocUnsafe with allocate to skip redundant validation.

benchmark results:

```
➜  node git:(mert/buffer-concat-optimize-js) ✗ node-benchmark-compare ./result.csv
                                                                            confidence improvement accuracy (*)    (**)   (***)
buffers/buffer-concat-fill.js n=800000 extraSize=1                                   *     10.37 %       ±9.47% ±13.37% ±19.13%
buffers/buffer-concat-fill.js n=800000 extraSize=1024                                       2.91 %       ±6.22%  ±8.53% ±11.62%
buffers/buffer-concat-fill.js n=800000 extraSize=256                                 *      7.05 %       ±6.13%  ±8.41% ±11.49%
buffers/buffer-concat.js n=800000 withTotalLength=0 pieceSize=1 pieces=16           **      1.76 %       ±1.18%  ±1.64%  ±2.26%
buffers/buffer-concat.js n=800000 withTotalLength=0 pieceSize=1 pieces=4           ***      5.20 %       ±1.21%  ±1.65%  ±2.25%
buffers/buffer-concat.js n=800000 withTotalLength=0 pieceSize=16 pieces=16          **      1.96 %       ±1.17%  ±1.61%  ±2.21%
buffers/buffer-concat.js n=800000 withTotalLength=0 pieceSize=16 pieces=4          ***      3.06 %       ±1.50%  ±2.06%  ±2.84%
buffers/buffer-concat.js n=800000 withTotalLength=0 pieceSize=256 pieces=16                 1.93 %       ±8.35% ±11.48% ±15.73%
buffers/buffer-concat.js n=800000 withTotalLength=0 pieceSize=256 pieces=4                  2.37 %       ±6.69%  ±9.34% ±13.12%
buffers/buffer-concat.js n=800000 withTotalLength=1 pieceSize=1 pieces=16                  -0.59 %       ±0.99%  ±1.36%  ±1.86%
buffers/buffer-concat.js n=800000 withTotalLength=1 pieceSize=1 pieces=4           ***      5.17 %       ±1.25%  ±1.71%  ±2.33%
buffers/buffer-concat.js n=800000 withTotalLength=1 pieceSize=16 pieces=16                  0.98 %       ±1.45%  ±2.02%  ±2.81%
buffers/buffer-concat.js n=800000 withTotalLength=1 pieceSize=16 pieces=4          ***      4.44 %       ±1.89%  ±2.62%  ±3.62%
buffers/buffer-concat.js n=800000 withTotalLength=1 pieceSize=256 pieces=16          *     -1.97 %       ±1.89%  ±2.63%  ±3.67%
buffers/buffer-concat.js n=800000 withTotalLength=1 pieceSize=256 pieces=4         ***      3.55 %       ±1.08%  ±1.49%  ±2.04%

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 (***)
➜  node git:(mert/buffer-concat-optimize-js) ✗ 
```

Changed files:
filename: lib/buffer.js
patch: @@ -51,6 +51,7 @@ const {
   TypedArrayPrototypeGetLength,
   TypedArrayPrototypeSet,
   TypedArrayPrototypeSlice,
+  TypedArrayPrototypeSubarray,
   Uint8Array,
 } = primordials;
 
@@ -614,25 +615,55 @@ Buffer.concat = function concat(list, length) {
   if (length === undefined) {
     length = 0;
     for (let i = 0; i < list.length; i++) {
-      if (list[i].length) {
-        length += list[i].length;
+      const buf = list[i];
+      if (!isUint8Array(buf)) {
+        // TODO(BridgeAR): This should not be of type ERR_INVALID_ARG_TYPE.
+        // Instead, find the proper error code for this.
+        throw new ERR_INVALID_ARG_TYPE(
+          `list[${i}]`, ['Buffer', 'Uint8Array'], buf);
       }
+      length += TypedArrayPrototypeGetByteLength(buf);
     }
-  } else {
-    validateOffset(length, 'length');
+
+    const buffer = allocate(length);
+    let pos = 0;
+    for (let i = 0; i < list.length; i++) {
+      const buf = list[i];
+      const bufLength = TypedArrayPrototypeGetByteLength(buf);
+      TypedArrayPrototypeSet(buffer, buf, pos);
+      pos += bufLength;
+    }
+
+    if (pos < length) {
+      TypedArrayPrototypeFill(buffer, 0, pos, length);
+    }
+    return buffer;
   }
 
-  const buffer = Buffer.allocUnsafe(length);
-  let pos = 0;
+  validateOffset(length, 'length');
   for (let i = 0; i < list.length; i++) {
-    const buf = list[i];
-    if (!isUint8Array(buf)) {
+    if (!isUint8Array(list[i])) {
       // TODO(BridgeAR): This should not be of type ERR_INVALID_ARG_TYPE.
       // Instead, find the proper error code for this.
       throw new ERR_INVALID_ARG_TYPE(
         `list[${i}]`, ['Buffer', 'Uint8Array'], list[i]);
     }
-    pos += _copyActual(buf, buffer, pos, 0, buf.length, true);
+  }
+
+  const buffer = allocate(length);
+  let pos = 0;
+  for (let i = 0; i < list.length; i++) {
+    const buf = list[i];
+    const bufLength = TypedArrayPrototypeGetByteLength(buf);
+    if (pos + bufLength > length) {
+      TypedArrayPrototypeSet(buffer,
+                             TypedArrayPrototypeSubarray(buf, 0, length - pos),
+                             pos);
+      pos = length;
+      break;
+    }
+    TypedArrayPrototypeSet(buffer, buf, pos);
+    pos += bufLength;
   }
 
   // Note: `length` is always equal to `buffer.length` at this point

filename: test/parallel/test-buffer-concat.js
patch: @@ -123,3 +123,13 @@ assert.deepStrictEqual(
 assert.deepStrictEqual(Buffer.concat([new Uint8Array([0x41, 0x42]),
                                       new Uint8Array([0x43, 0x44])]),
                        Buffer.from('ABCD'));
+
+// Spoofed length getter should not cause uninitialized memory exposure
+{
+  const u8_1 = new Uint8Array([1, 2, 3, 4]);
+  const u8_2 = new Uint8Array([5, 6, 7, 8]);
+  Object.defineProperty(u8_1, 'length', { get() { return 100; } });
+  const buf = Buffer.concat([u8_1, u8_2]);
+  assert.strictEqual(buf.length, 8);
+  assert.deepStrictEqual(buf, Buffer.from([1, 2, 3, 4, 5, 6, 7, 8]));
+}