Use %TypedArray%.prototype.subarray for Buffer.prototype.slice

[TimothyGu]

• Version: master
• Platform: all
• Subsystem: buffer

Currently, our implementation of Buffer.prototype.slice() operates identically to TypedArray.prototype.subarray() (spec) down to the last minutia (with the assumption that this is indeed a Buffer, and Buffer[Symbol.species] has not been tampered with). Ideally, we should just set Buffer.prototype.slice to Uint8Array.prototype.subarray instead of reimplementing it. The only problem is performance: subarray() is much slower than slice():

diff

diff --git a/lib/buffer.js b/lib/buffer.js
index b56c032f9e..112dc22d69 100644
--- a/lib/buffer.js
+++ b/lib/buffer.js
@@ -989,29 +989,7 @@ Buffer.prototype.toJSON = function toJSON() {
 };
 
 
-function adjustOffset(offset, length) {
-  // Use Math.trunc() to convert offset to an integer value that can be larger
-  // than an Int32. Hence, don't use offset | 0 or similar techniques.
-  offset = Math.trunc(offset);
-  // `x !== x`-style conditionals are a faster form of `isNaN(x)`
-  if (offset === 0 || offset !== offset) {
-    return 0;
-  } else if (offset < 0) {
-    offset += length;
-    return offset > 0 ? offset : 0;
-  } else {
-    return offset < length ? offset : length;
-  }
-}
-
-
-Buffer.prototype.slice = function slice(start, end) {
-  const srcLength = this.length;
-  start = adjustOffset(start, srcLength);
-  end = end !== undefined ? adjustOffset(end, srcLength) : srcLength;
-  const newLength = end > start ? end - start : 0;
-  return new FastBuffer(this.buffer, this.byteOffset + start, newLength);
-};
+Buffer.prototype.slice = Uint8Array.prototype.subarray;
 
 
 function checkOffset(offset, ext, length) {

$ ./node-master benchmark/buffers/buffer-slice.js 
buffers/buffer-slice.js n=1024 type="fast": 11,632.49754936561
buffers/buffer-slice.js n=1024 type="slow": 11,584.90297078093
$ ./node benchmark/buffers/buffer-slice.js 
buffers/buffer-slice.js n=1024 type="fast": 6,675.602412489698
buffers/buffer-slice.js n=1024 type="slow": 6,487.594815897296

I'd like to work with the V8 team to resolve the performance problem, and eventually use the language built-in for Buffer.prototype.slice.

/cc @bmeurer

[TimothyGu]

A fairer benchmark with mostly proper @@species support, suitable for running with d8:

'use strict';

if (typeof console === 'undefined') console = { log: print };

const N = 1e7;

// https://tc39.github.io/ecma262/#sec-speciesconstructor
function getSpeciesConstructor(o, defaultConstructor) {
  const C = o.constructor;
  if (C === undefined) return defaultConstructor;
  if (typeof C !== 'function' && typeof C !== 'object') throw new TypeError();

  const S = o[Symbol.species];
  if (S === undefined || S === null) return defaultConstructor;
  // Spec says IsConstructor, but a simpler test is used here instead.
  if (typeof S === 'function') return S;
  throw new TypeError();
}

function adjustOffset(offset, length) {
  // Use Math.trunc() to convert offset to an integer value that can be larger
  // than an Int32. Hence, don't use offset | 0 or similar techniques.
  offset = Math.trunc(offset);
  // `x !== x`-style conditionals are a faster form of `isNaN(x)`
  if (offset === 0 || offset !== offset) {
    return 0;
  } else if (offset < 0) {
    offset += length;
    return offset > 0 ? offset : 0;
  } else {
    return offset < length ? offset : length;
  }
}

class Buffer extends Uint8Array {
  customSubarray(begin, end) {
    const srcLength = this.length;
    const beginIndex = adjustOffset(begin, srcLength);
    const endIndex = end !== undefined ? adjustOffset(end, srcLength) : srcLength;
    const newLength = endIndex > beginIndex ? endIndex - beginIndex : 0;
    const beginByteOffset = this.byteOffset + beginIndex * this.BYTES_PER_ELEMENT;
    return new (getSpeciesConstructor(this, this.constructor))(this.buffer, beginByteOffset, newLength);
  }
}
Buffer.prototype[Symbol.species] = Buffer;

function testCustomSubarray(o) {
  let result = 0;
  for (let i = 0; i < N; ++i) {
    result += o.customSubarray(3, 4).length;
  }
  return result;
}

function testSubarray(o) {
  let result = 0;
  for (let i = 0; i < N; ++i) {
    result += o.subarray(3, 4).length;
  }
  return result;
}

var TESTS = [
  testCustomSubarray,
  testSubarray
];

const array = new Buffer(0x68, 0x65, 0x6c, 0x6c, 0x6f);

for (var j = 0; j < TESTS.length; j++) {
  TESTS[j](array);
}

for (var j = 0; j < TESTS.length; j++) {
  var startTime = Date.now();
  TESTS[j](array);
  console.log(TESTS[j].name + ':', (Date.now() - startTime), 'ms.');
}

Still shows customSubarray being ahead by much on ToT V8 (v8/v8@d161e0c):

testCustomSubarray: 714 ms.
testSubarray: 1308 ms.

[hashseed]

@psmarshall

[bmeurer]

Upstream bug: v8:7161

[targos]

Upstream bug is considered fixed but there is still a significant difference:

$ ./node-master benchmark/buffers/buffer-slice.js n=8192
buffers/buffer-slice.js n=8192 type="fast": 20,469.680926799585
buffers/buffer-slice.js n=8192 type="slow": 20,772.18650882746

$ ./node benchmark/buffers/buffer-slice.js n=8192
buffers/buffer-slice.js n=8192 type="fast": 6,449.032914218671
buffers/buffer-slice.js n=8192 type="slow": 6,481.283810735123

$ ./node test.js
testCustomSubarray: 464 ms.
testSubarray: 1456 ms.

[schuay]

I see similar results locally in ToT d8. Reopening the upstream bug.

[jasnell]

@TimothyGu .. is this still relevant? Does it need to remain open?

[TimothyGu]

Still pretty slow with Node.js v14

testCustomSubarray: 730 ms.
testSubarray: 2636 ms.

[targos]

What about now?

[Hypfer]

Is this still an issue?