Skip to content

Commit

Permalink
buffer: improve copy() performance
Browse files Browse the repository at this point in the history
PR-URL: nodejs#29066
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
  • Loading branch information
mscdex authored and Trott committed Aug 14, 2019
1 parent e505a74 commit 6d351d4
Show file tree
Hide file tree
Showing 4 changed files with 105 additions and 10 deletions.
19 changes: 19 additions & 0 deletions benchmark/buffers/buffer-copy.js
Original file line number Diff line number Diff line change
@@ -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);
}
70 changes: 69 additions & 1 deletion lib/buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ const { Math, Object } = primordials;

const {
byteLengthUtf8,
copy: _copy,
compare: _compare,
compareOffset,
createFromString,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion test/parallel/test-buffer-alloc.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(), {
Expand Down
23 changes: 15 additions & 8 deletions test/parallel/test-buffer-copy.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

{
Expand Down Expand Up @@ -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
Expand All @@ -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);
Expand Down

0 comments on commit 6d351d4

Please sign in to comment.