Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

buffer: add Buffer compare by offset #5880

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 58 additions & 0 deletions benchmark/buffers/buffer-compare-offset.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
'use strict';
const common = require('../common.js');
const v8 = require('v8');

const bench = common.createBenchmark(main, {
method: ['offset', 'slice'],
size: [16, 512, 1024, 4096, 16386],
millions: [1]
});

function compareUsingSlice(b0, b1, len, iter) {

// Force optimization before starting the benchmark
Buffer.compare(b0.slice(1, len), b1.slice(1, len));
v8.setFlagsFromString('--allow_natives_syntax');
eval('%OptimizeFunctionOnNextCall(Buffer.compare)');
eval('%OptimizeFunctionOnNextCall(b0.slice)');
eval('%OptimizeFunctionOnNextCall(b1.slice)');
Buffer.compare(b0.slice(1, len), b1.slice(1, len));
doCompareUsingSlice(b0, b1, len, iter);
}

function doCompareUsingSlice(b0, b1, len, iter) {
var i;
bench.start();
for (i = 0; i < iter; i++)
Buffer.compare(b0.slice(1, len), b1.slice(1, len));
bench.end(iter / 1e6);
}

function compareUsingOffset(b0, b1, len, iter) {
len = len + 1;
// Force optimization before starting the benchmark
b0.compare(b1, 1, len, 1, len);
v8.setFlagsFromString('--allow_natives_syntax');
eval('%OptimizeFunctionOnNextCall(b0.compare)');
b0.compare(b1, 1, len, 1, len);
doCompareUsingOffset(b0, b1, len, iter);
}

function doCompareUsingOffset(b0, b1, len, iter) {
var i;
bench.start();
for (i = 0; i < iter; i++)
b0.compare(b1, 1, len, 1, len);
bench.end(iter / 1e6);
}

function main(conf) {
const iter = (conf.millions >>> 0) * 1e6;
const size = (conf.size >>> 0);
const method = conf.method === 'slice' ?
compareUsingSlice : compareUsingOffset;
method(Buffer.alloc(size, 'a'),
Buffer.alloc(size, 'b'),
size >> 1,
iter);
}
41 changes: 34 additions & 7 deletions doc/api/buffer.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -675,18 +675,26 @@ console.log(buf.toString('ascii'));
// Prints: Node.js
```

### buf.compare(otherBuffer)

* `otherBuffer` {Buffer}
### buf.compare(target[, targetStart[, targetEnd[, sourceStart[, sourceEnd]]]])

* `target` {Buffer}
* `targetStart` {Integer} The offset within `target` at which to begin
comparison. default = `0`.
* `targetEnd` {Integer} The offset with `target` at which to end comparison.
Ignored when `targetStart` is `undefined`. default = `target.byteLength`.
* `sourceStart` {Integer} The offset within `buf` at which to begin comparison.
Ignored when `targetStart` is `undefined`. default = `0`
* `sourceEnd` {Integer} The offset within `buf` at which to end comparison.
Ignored when `targetStart` is `undefined`. default = `buf.byteLength`.
* Return: {Number}

Compares two Buffer instances and returns a number indicating whether `buf`
comes before, after, or is the same as the `otherBuffer` in sort order.
comes before, after, or is the same as the `target` in sort order.
Comparison is based on the actual sequence of bytes in each Buffer.

* `0` is returned if `otherBuffer` is the same as `buf`
* `1` is returned if `otherBuffer` should come *before* `buf` when sorted.
* `-1` is returned if `otherBuffer` should come *after* `buf` when sorted.
* `0` is returned if `target` is the same as `buf`
* `1` is returned if `target` should come *before* `buf` when sorted.
* `-1` is returned if `target` should come *after* `buf` when sorted.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should document whether this will throw if, say, sourceEnd > buf.length, or if it will simply return -1.


```js
const buf1 = Buffer.from('ABC');
Expand All @@ -708,6 +716,25 @@ console.log(buf2.compare(buf3));
// produces sort order [buf1, buf3, buf2]
```

The optional `targetStart`, `targetEnd`, `sourceStart`, and `sourceEnd`
arguments can be used to limit the comparison to specific ranges within the two
`Buffer` objects.

```js
const buf1 = Buffer.from([1, 2, 3, 4, 5, 6, 7, 8, 9]);
const buf2 = Buffer.from([5, 6, 7, 8, 9, 1, 2, 3, 4]);

console.log(buf1.compare(buf2, 5, 9, 0, 4));
// Prints: 0
console.log(buf1.compare(buf2, 0, 6, 4));
// Prints: -1
console.log(buf1.compare(buf2, 5, 6, 5));
// Prints: 1
```

A `RangeError` will be thrown if: `targetStart < 0`, `sourceStart < 0`,
`targetEnd > target.byteLength` or `sourceEnd > source.byteLength`.

### buf.copy(targetBuffer[, targetStart[, sourceStart[, sourceEnd]]])

* `targetBuffer` {Buffer} Buffer to copy into
Expand Down
37 changes: 33 additions & 4 deletions lib/buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -537,15 +537,44 @@ Buffer.prototype.inspect = function inspect() {
return '<' + this.constructor.name + ' ' + str + '>';
};

Buffer.prototype.compare = function compare(target,
start,
end,
thisStart,
thisEnd) {

Buffer.prototype.compare = function compare(b) {
if (!(b instanceof Buffer))
if (!(target instanceof Buffer))
throw new TypeError('Argument must be a Buffer');

if (this === b)
if (start === undefined)
start = 0;
if (end === undefined)
end = target ? target.length : 0;
if (thisStart === undefined)
thisStart = 0;
if (thisEnd === undefined)
thisEnd = this.length;

if (start < 0 ||
end > target.length ||
thisStart < 0 ||
thisEnd > this.length) {
throw new RangeError('out of range index');
}

if (thisStart >= thisEnd && start >= end)
return 0;
if (thisStart >= thisEnd)
return -1;
if (start >= end)
return 1;

start >>>= 0;
end >>>= 0;
thisStart >>>= 0;
thisEnd >>>= 0;

return binding.compare(this, b);
return binding.compareOffset(this, target, start, thisStart, end, thisEnd);
};

function slowIndexOf(buffer, val, byteOffset, encoding) {
Expand Down
76 changes: 60 additions & 16 deletions src/node_buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -870,6 +870,62 @@ void ByteLengthUtf8(const FunctionCallbackInfo<Value> &args) {
args.GetReturnValue().Set(args[0].As<String>()->Utf8Length());
}

// Normalize val to be an integer in the range of [1, -1] since
// implementations of memcmp() can vary by platform.
static int normalizeCompareVal(int val, size_t a_length, size_t b_length) {
if (val == 0) {
if (a_length > b_length)
return 1;
else if (a_length < b_length)
return -1;
} else {
if (val > 0)
return 1;
else
return -1;
}
return val;
}

void CompareOffset(const FunctionCallbackInfo<Value> &args) {
Environment* env = Environment::GetCurrent(args);

THROW_AND_RETURN_UNLESS_BUFFER(env, args[0]);
THROW_AND_RETURN_UNLESS_BUFFER(env, args[1]);
SPREAD_ARG(args[0], ts_obj);
SPREAD_ARG(args[1], target);

size_t target_start;
size_t source_start;
size_t source_end;
size_t target_end;

CHECK_NOT_OOB(ParseArrayIndex(args[2], 0, &target_start));
CHECK_NOT_OOB(ParseArrayIndex(args[3], 0, &source_start));
CHECK_NOT_OOB(ParseArrayIndex(args[4], target_length, &target_end));
CHECK_NOT_OOB(ParseArrayIndex(args[5], ts_obj_length, &source_end));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

curse me for using CHECK in the macro name.


if (source_start > ts_obj_length)
return env->ThrowRangeError("out of range index");
if (target_start > target_length)
return env->ThrowRangeError("out of range index");

CHECK_LE(source_start, source_end);
CHECK_LE(target_start, target_end);

size_t to_cmp = MIN(MIN(source_end - source_start,
target_end - target_start),
ts_obj_length - source_start);

int val = normalizeCompareVal(to_cmp > 0 ?
memcmp(ts_obj_data + source_start,
target_data + target_start,
to_cmp) : 0,
source_end - source_start,
target_end - target_start);

args.GetReturnValue().Set(val);
}

void Compare(const FunctionCallbackInfo<Value> &args) {
Environment* env = Environment::GetCurrent(args);
Expand All @@ -881,22 +937,9 @@ void Compare(const FunctionCallbackInfo<Value> &args) {

size_t cmp_length = MIN(obj_a_length, obj_b_length);

int val = cmp_length > 0 ? memcmp(obj_a_data, obj_b_data, cmp_length) : 0;

// Normalize val to be an integer in the range of [1, -1] since
// implementations of memcmp() can vary by platform.
if (val == 0) {
if (obj_a_length > obj_b_length)
val = 1;
else if (obj_a_length < obj_b_length)
val = -1;
} else {
if (val > 0)
val = 1;
else
val = -1;
}

int val = normalizeCompareVal(cmp_length > 0 ?
memcmp(obj_a_data, obj_b_data, cmp_length) : 0,
obj_a_length, obj_b_length);
args.GetReturnValue().Set(val);
}

Expand Down Expand Up @@ -1172,6 +1215,7 @@ void Initialize(Local<Object> target,

env->SetMethod(target, "byteLengthUtf8", ByteLengthUtf8);
env->SetMethod(target, "compare", Compare);
env->SetMethod(target, "compareOffset", CompareOffset);
env->SetMethod(target, "fill", Fill);
env->SetMethod(target, "indexOfBuffer", IndexOfBuffer);
env->SetMethod(target, "indexOfNumber", IndexOfNumber);
Expand Down
63 changes: 63 additions & 0 deletions test/parallel/test-buffer-compare-offset.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
'use strict';

require('../common');
const assert = require('assert');

const a = Buffer.from([1, 2, 3, 4, 5, 6, 7, 8, 9, 0]);
const b = Buffer.from([5, 6, 7, 8, 9, 0, 1, 2, 3, 4]);

assert.equal(-1, a.compare(b));

// Equivalent to a.compare(b).
assert.equal(-1, a.compare(b, 0));
assert.equal(-1, a.compare(b, '0'));

// Equivalent to a.compare(b).
assert.equal(-1, a.compare(b, 0, undefined, 0));

// Zero-length targer, return 1
assert.equal(1, a.compare(b, 0, 0, 0));
assert.equal(1, a.compare(b, '0', '0', '0'));

// Equivalent to Buffer.compare(a, b.slice(6, 10))
assert.equal(1, a.compare(b, 6, 10));

// Zero-length source, return -1
assert.equal(-1, a.compare(b, 6, 10, 0, 0));

// Equivalent to Buffer.compare(a.slice(4), b.slice(0, 5))
assert.equal(1, a.compare(b, 0, 5, 4));

// Equivalent to Buffer.compare(a.slice(1), b.slice(5))
assert.equal(1, a.compare(b, 5, undefined, 1));

// Equivalent to Buffer.compare(a.slice(2), b.slice(2, 4))
assert.equal(-1, a.compare(b, 2, 4, 2));

// Equivalent to Buffer.compare(a.slice(4), b.slice(0, 7))
assert.equal(-1, a.compare(b, 0, 7, 4));

// Equivalent to Buffer.compare(a.slice(4, 6), b.slice(0, 7));
assert.equal(-1, a.compare(b, 0, 7, 4, 6));

// zero length target
assert.equal(1, a.compare(b, 0, null));

// coerces to targetEnd == 5
assert.equal(-1, a.compare(b, 0, {valueOf: () => 5}));

// zero length target
assert.equal(1, a.compare(b, Infinity, -Infinity));

// zero length target because default for targetEnd <= targetSource
assert.equal(1, a.compare(b, '0xff'));

const oor = /out of range index/;

assert.throws(() => a.compare(b, 0, 100, 0), oor);
assert.throws(() => a.compare(b, 0, 1, 0, 100), oor);
assert.throws(() => a.compare(b, -1), oor);
assert.throws(() => a.compare(b, 0, '0xff'), oor);
assert.throws(() => a.compare(b, 0, Infinity), oor);
assert.throws(() => a.compare(b, -Infinity, Infinity), oor);
assert.throws(() => a.compare(), /Argument must be a Buffer/);