From 1e6a4c4e0f94df59fec0f6b0d10a4d320b7e8ec5 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Wed, 23 Mar 2016 14:27:39 -0700 Subject: [PATCH] buffer: add Buffer.prototype.compare by offset Adds additional `targetStart`, `targetEnd`, `sourceStart, and `sourceEnd` arguments to `Buffer.prototype.compare` to allow comparison of sub-ranges of two Buffers without requiring Buffer.prototype.slice() Fixes: https://github.com/nodejs/node/issues/521 --- benchmark/buffers/buffer-compare-offset.js | 58 ++++++++++++++++ doc/api/buffer.markdown | 41 +++++++++-- lib/buffer.js | 37 ++++++++-- src/node_buffer.cc | 76 ++++++++++++++++----- test/parallel/test-buffer-compare-offset.js | 63 +++++++++++++++++ 5 files changed, 248 insertions(+), 27 deletions(-) create mode 100644 benchmark/buffers/buffer-compare-offset.js create mode 100644 test/parallel/test-buffer-compare-offset.js diff --git a/benchmark/buffers/buffer-compare-offset.js b/benchmark/buffers/buffer-compare-offset.js new file mode 100644 index 00000000000000..17b36f82883426 --- /dev/null +++ b/benchmark/buffers/buffer-compare-offset.js @@ -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); +} diff --git a/doc/api/buffer.markdown b/doc/api/buffer.markdown index f4f45c96cb3fbb..c1779b0d319ccf 100644 --- a/doc/api/buffer.markdown +++ b/doc/api/buffer.markdown @@ -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. ```js const buf1 = Buffer.from('ABC'); @@ -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 diff --git a/lib/buffer.js b/lib/buffer.js index 453652a0b6816a..f672dae558d320 100644 --- a/lib/buffer.js +++ b/lib/buffer.js @@ -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) { diff --git a/src/node_buffer.cc b/src/node_buffer.cc index ca901a108941c4..322cc5f57ce881 100644 --- a/src/node_buffer.cc +++ b/src/node_buffer.cc @@ -870,6 +870,62 @@ void ByteLengthUtf8(const FunctionCallbackInfo &args) { args.GetReturnValue().Set(args[0].As()->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 &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)); + + 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 &args) { Environment* env = Environment::GetCurrent(args); @@ -881,22 +937,9 @@ void Compare(const FunctionCallbackInfo &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); } @@ -1172,6 +1215,7 @@ void Initialize(Local 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); diff --git a/test/parallel/test-buffer-compare-offset.js b/test/parallel/test-buffer-compare-offset.js new file mode 100644 index 00000000000000..540d644c2847ff --- /dev/null +++ b/test/parallel/test-buffer-compare-offset.js @@ -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/);