Skip to content

Commit

Permalink
src: use subarray() in Buffer#slice() for speedup
Browse files Browse the repository at this point in the history
Use the built-in Typed Array method subarray() to improve performance of
Buffer#slice().

Benchmark improvements:

    benchmark/buffer-slice: 40%
    benchmark/buffer-creation (pool): 25%

Additional tests also added.

PR-URL: nodejs#2777
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
  • Loading branch information
skomski authored and trevnorris committed Sep 15, 2015
1 parent 7d927ab commit e7a3ca3
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 50 deletions.
31 changes: 5 additions & 26 deletions lib/buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ function allocate(size) {
if (size < (Buffer.poolSize >>> 1)) {
if (size > (poolSize - poolOffset))
createPool();
var b = binding.slice(allocPool, poolOffset, poolOffset + size);
var b = allocPool.slice(poolOffset, poolOffset + size);
poolOffset += size;
alignPool();
return b;
Expand All @@ -94,7 +94,7 @@ function fromString(string, encoding) {
if (length > (poolSize - poolOffset))
createPool();
var actual = allocPool.write(string, poolOffset, encoding);
var b = binding.slice(allocPool, poolOffset, poolOffset + actual);
var b = allocPool.slice(poolOffset, poolOffset + actual);
poolOffset += actual;
alignPool();
return b;
Expand Down Expand Up @@ -552,30 +552,9 @@ Buffer.prototype.toJSON = function() {
// TODO(trevnorris): currently works like Array.prototype.slice(), which
// doesn't follow the new standard for throwing on out of range indexes.
Buffer.prototype.slice = function slice(start, end) {
var len = this.length;
start = ~~start;
end = end === undefined ? len : ~~end;

if (start < 0) {
start += len;
if (start < 0)
start = 0;
} else if (start > len) {
start = len;
}

if (end < 0) {
end += len;
if (end < 0)
end = 0;
} else if (end > len) {
end = len;
}

if (end < start)
end = start;

return binding.slice(this, start, end);
const buffer = this.subarray(start, end);
Object.setPrototypeOf(buffer, Buffer.prototype);
return buffer;
};


Expand Down
24 changes: 0 additions & 24 deletions src/node_buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -456,29 +456,6 @@ void CreateFromArrayBuffer(const FunctionCallbackInfo<Value>& args) {
}


void Slice(const FunctionCallbackInfo<Value>& args) {
CHECK(args[0]->IsUint8Array());
CHECK(args[1]->IsNumber());
CHECK(args[2]->IsNumber());
Environment* env = Environment::GetCurrent(args);
Local<Uint8Array> ab_ui = args[0].As<Uint8Array>();
Local<ArrayBuffer> ab = ab_ui->Buffer();
ArrayBuffer::Contents ab_c = ab->GetContents();
size_t offset = ab_ui->ByteOffset();
size_t start = args[1]->NumberValue() + offset;
size_t end = args[2]->NumberValue() + offset;
CHECK_GE(end, start);
size_t size = end - start;
CHECK_GE(ab_c.ByteLength(), start + size);
Local<Uint8Array> ui = Uint8Array::New(ab, start, size);
Maybe<bool> mb =
ui->SetPrototype(env->context(), env->buffer_prototype_object());
if (!mb.FromMaybe(false))
return env->ThrowError("Unable to set Object prototype");
args.GetReturnValue().Set(ui);
}


template <encoding encoding>
void StringSlice(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
Expand Down Expand Up @@ -1002,7 +979,6 @@ void Initialize(Local<Object> target,
env->SetMethod(target, "createFromString", CreateFromString);
env->SetMethod(target, "createFromArrayBuffer", CreateFromArrayBuffer);

env->SetMethod(target, "slice", Slice);
env->SetMethod(target, "byteLengthUtf8", ByteLengthUtf8);
env->SetMethod(target, "compare", Compare);
env->SetMethod(target, "fill", Fill);
Expand Down
32 changes: 32 additions & 0 deletions test/parallel/test-buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -1061,14 +1061,46 @@ assert.equal(buf.readInt8(0), -1);
assert.equal(buf.slice(-10, 10), '0123456789');
assert.equal(buf.slice(-20, 10), '0123456789');
assert.equal(buf.slice(-20, -10), '');
assert.equal(buf.slice(), '0123456789');
assert.equal(buf.slice(0), '0123456789');
assert.equal(buf.slice(0, 0), '');
assert.equal(buf.slice(undefined), '0123456789');
assert.equal(buf.slice('foobar'), '0123456789');
assert.equal(buf.slice(undefined, undefined), '0123456789');

assert.equal(buf.slice(2), '23456789');
assert.equal(buf.slice(5), '56789');
assert.equal(buf.slice(10), '');
assert.equal(buf.slice(5, 8), '567');
assert.equal(buf.slice(8, -1), '8');
assert.equal(buf.slice(-10), '0123456789');
assert.equal(buf.slice(0, -9), '0');
assert.equal(buf.slice(0, -10), '');
assert.equal(buf.slice(0, -1), '012345678');
assert.equal(buf.slice(2, -2), '234567');
assert.equal(buf.slice(0, 65536), '0123456789');
assert.equal(buf.slice(65536, 0), '');
assert.equal(buf.slice(-5, -8), '');
assert.equal(buf.slice(-5, -3), '56');
assert.equal(buf.slice(-10, 10), '0123456789');
for (var i = 0, s = buf.toString(); i < buf.length; ++i) {
assert.equal(buf.slice(i), s.slice(i));
assert.equal(buf.slice(0, i), s.slice(0, i));
assert.equal(buf.slice(-i), s.slice(-i));
assert.equal(buf.slice(0, -i), s.slice(0, -i));
}

var utf16Buf = new Buffer('0123456789', 'utf16le');
assert.deepEqual(utf16Buf.slice(0, 6), Buffer('012', 'utf16le'));

assert.equal(buf.slice('0', '1'), '0');
assert.equal(buf.slice('-5', '10'), '56789');
assert.equal(buf.slice('-10', '10'), '0123456789');
assert.equal(buf.slice('-10', '-5'), '01234');
assert.equal(buf.slice('-10', '-0'), '');
assert.equal(buf.slice('111'), '');
assert.equal(buf.slice('0', '-111'), '');

// try to slice a zero length Buffer
// see https://github.com/joyent/node/issues/5881
SlowBuffer(0).slice(0, 1);
Expand Down

0 comments on commit e7a3ca3

Please sign in to comment.