buffer: improve copy() performance

mscdex · Trott · commit 6d351d4cc0a6 · 2019-08-14T15:59:32.000-07:00
PR-URL: nodejs#29066 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
diff --git a/benchmark/buffers/buffer-copy.js b/benchmark/buffers/buffer-copy.js
@@ -0,0 +1,19 @@
+'use strict';
+const common = require('../common.js');
+
+const bench = common.createBenchmark(main, {
+  bytes: [0, 8, 128, 32 * 1024],
+  partial: ['true', 'false'],
+  n: [6e6]
+});
+
+function main({ n, bytes, partial }) {
+  const source = Buffer.allocUnsafe(bytes);
+  const target = Buffer.allocUnsafe(bytes);
+  const sourceStart = (partial === 'true' ? Math.floor(bytes / 2) : 0);
+  bench.start();
+  for (let i = 0; i < n; i++) {
+    source.copy(target, 0, sourceStart);
+  }
+  bench.end(n);
+}
diff --git a/lib/buffer.js b/lib/buffer.js
@@ -25,7 +25,6 @@ const { Math, Object } = primordials;
 
 const {
   byteLengthUtf8,
-  copy: _copy,
   compare: _compare,
   compareOffset,
   createFromString,
@@ -69,6 +68,7 @@ const {
     ERR_INVALID_ARG_VALUE,
     ERR_INVALID_BUFFER_SIZE,
     ERR_INVALID_OPT_VALUE,
+    ERR_OUT_OF_RANGE,
     ERR_UNKNOWN_ENCODING
   },
   hideStackFrames
@@ -157,6 +157,74 @@ function showFlaggedDeprecation() {
   bufferWarningAlreadyEmitted = true;
 }
 
+function toInteger(n, defaultVal) {
+  n = +n;
+  if (!Number.isNaN(n) &&
+      n >= Number.MIN_SAFE_INTEGER &&
+      n <= Number.MAX_SAFE_INTEGER) {
+    return ((n % 1) === 0 ? n : Math.floor(n));
+  }
+  return defaultVal;
+}
+
+function _copy(source, target, targetStart, sourceStart, sourceEnd) {
+  if (!isUint8Array(source))
+    throw new ERR_INVALID_ARG_TYPE('source', ['Buffer', 'Uint8Array'], source);
+  if (!isUint8Array(target))
+    throw new ERR_INVALID_ARG_TYPE('target', ['Buffer', 'Uint8Array'], target);
+
+  if (targetStart === undefined) {
+    targetStart = 0;
+  } else {
+    targetStart = toInteger(targetStart, 0);
+    if (targetStart < 0)
+      throw new ERR_OUT_OF_RANGE('targetStart', '>= 0', targetStart);
+  }
+
+  if (sourceStart === undefined) {
+    sourceStart = 0;
+  } else {
+    sourceStart = toInteger(sourceStart, 0);
+    if (sourceStart < 0)
+      throw new ERR_OUT_OF_RANGE('sourceStart', '>= 0', sourceStart);
+  }
+
+  if (sourceEnd === undefined) {
+    sourceEnd = source.length;
+  } else {
+    sourceEnd = toInteger(sourceEnd, 0);
+    if (sourceEnd < 0)
+      throw new ERR_OUT_OF_RANGE('sourceEnd', '>= 0', sourceEnd);
+  }
+
+  if (targetStart >= target.length || sourceStart >= sourceEnd)
+    return 0;
+
+  if (sourceStart > source.length) {
+    throw new ERR_OUT_OF_RANGE('sourceStart',
+                               `<= ${source.length}`,
+                               sourceStart);
+  }
+
+  if (sourceEnd - sourceStart > target.length - targetStart)
+    sourceEnd = sourceStart + target.length - targetStart;
+
+  let nb = sourceEnd - sourceStart;
+  const targetLen = target.length - targetStart;
+  const sourceLen = source.length - sourceStart;
+  if (nb > targetLen)
+    nb = targetLen;
+  if (nb > sourceLen)
+    nb = sourceLen;
+
+  if (sourceStart !== 0 || sourceEnd !== source.length)
+    source = new Uint8Array(source.buffer, source.byteOffset + sourceStart, nb);
+
+  target.set(source, targetStart);
+
+  return nb;
+}
+
 /**
  * The Buffer() constructor is deprecated in documentation and should not be
  * used moving forward. Rather, developers should use one of the three new
diff --git a/test/parallel/test-buffer-alloc.js b/test/parallel/test-buffer-alloc.js
@@ -967,7 +967,8 @@ common.expectsError(
   {
     code: 'ERR_INVALID_ARG_TYPE',
     type: TypeError,
-    message: 'argument must be a buffer'
+    message: 'The "target" argument must be one of type Buffer or Uint8Array.' +
+             ' Received type undefined'
   });
 
 assert.throws(() => Buffer.from(), {
diff --git a/test/parallel/test-buffer-copy.js b/test/parallel/test-buffer-copy.js
@@ -6,12 +6,6 @@ const assert = require('assert');
 const b = Buffer.allocUnsafe(1024);
 const c = Buffer.allocUnsafe(512);
 
-const errorProperty = {
-  code: 'ERR_OUT_OF_RANGE',
-  type: RangeError,
-  message: 'Index out of range'
-};
-
 let cntr = 0;
 
 {
@@ -116,7 +110,13 @@ b.copy(c, 0, 100, 10);
 // Copy throws at negative sourceStart
 common.expectsError(
   () => Buffer.allocUnsafe(5).copy(Buffer.allocUnsafe(5), 0, -1),
-  errorProperty);
+  {
+    code: 'ERR_OUT_OF_RANGE',
+    type: RangeError,
+    message: 'The value of "sourceStart" is out of range. ' +
+             'It must be >= 0. Received -1'
+  }
+);
 
 {
   // Check sourceEnd resets to targetEnd if former is greater than the latter
@@ -130,7 +130,14 @@ common.expectsError(
 
 // Throw with negative sourceEnd
 common.expectsError(
-  () => b.copy(c, 0, -1), errorProperty);
+  () => b.copy(c, 0, 0, -1),
+  {
+    code: 'ERR_OUT_OF_RANGE',
+    type: RangeError,
+    message: 'The value of "sourceEnd" is out of range. ' +
+             'It must be >= 0. Received -1'
+  }
+);
 
 // When sourceStart is greater than sourceEnd, zero copied
 assert.strictEqual(b.copy(c, 0, 100, 10), 0);

Original file line number	Diff line number	Diff line change
`@@ -967,7 +967,8 @@ common.expectsError(`
`967`	`967`	`{`
`968`	`968`	`code: 'ERR_INVALID_ARG_TYPE',`
`969`	`969`	`type: TypeError,`
`970`		`- message: 'argument must be a buffer'`
	`970`	`+ message: 'The "target" argument must be one of type Buffer or Uint8Array.' +`
	`971`	`+ ' Received type undefined'`
`971`	`972`	`});`
`972`	`973`
`973`	`974`	`assert.throws(() => Buffer.from(), {`