-
-
Notifications
You must be signed in to change notification settings - Fork 33.5k
buffer: improve creation performance #6893
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,8 +2,14 @@ | |
'use strict'; | ||
|
||
const binding = process.binding('buffer'); | ||
const { isArrayBuffer } = process.binding('util'); | ||
const bindingObj = {}; | ||
|
||
class FastBuffer extends Uint8Array {} | ||
|
||
FastBuffer.prototype.constructor = Buffer; | ||
Buffer.prototype = FastBuffer.prototype; | ||
|
||
exports.Buffer = Buffer; | ||
exports.SlowBuffer = SlowBuffer; | ||
exports.INSPECT_MAX_BYTES = 50; | ||
|
@@ -63,24 +69,18 @@ Buffer.prototype.swap32 = function swap32() { | |
// do not own the ArrayBuffer allocator. Zero fill is always on in that case. | ||
const zeroFill = bindingObj.zeroFill || [0]; | ||
|
||
function createBuffer(size, noZeroFill) { | ||
if (noZeroFill) | ||
zeroFill[0] = 0; | ||
|
||
function createUnsafeBuffer(size) { | ||
zeroFill[0] = 0; | ||
try { | ||
var ui8 = new Uint8Array(size); | ||
return new FastBuffer(size); | ||
} finally { | ||
if (noZeroFill) | ||
zeroFill[0] = 1; | ||
zeroFill[0] = 1; | ||
} | ||
|
||
Object.setPrototypeOf(ui8, Buffer.prototype); | ||
return ui8; | ||
} | ||
|
||
function createPool() { | ||
poolSize = Buffer.poolSize; | ||
allocPool = createBuffer(poolSize, true); | ||
allocPool = createUnsafeBuffer(poolSize); | ||
poolOffset = 0; | ||
} | ||
createPool(); | ||
|
@@ -138,7 +138,6 @@ Buffer.from = function(value, encodingOrOffset, length) { | |
return fromObject(value); | ||
}; | ||
|
||
Object.setPrototypeOf(Buffer.prototype, Uint8Array.prototype); | ||
Object.setPrototypeOf(Buffer, Uint8Array); | ||
|
||
function assertSize(size) { | ||
|
@@ -158,18 +157,16 @@ function assertSize(size) { | |
**/ | ||
Buffer.alloc = function(size, fill, encoding) { | ||
assertSize(size); | ||
if (size <= 0) | ||
return createBuffer(size); | ||
if (fill !== undefined) { | ||
if (size > 0 && fill !== undefined) { | ||
// Since we are filling anyway, don't zero fill initially. | ||
// Only pay attention to encoding if it's a string. This | ||
// prevents accidentally sending in a number that would | ||
// be interpretted as a start offset. | ||
return typeof encoding === 'string' ? | ||
createBuffer(size, true).fill(fill, encoding) : | ||
createBuffer(size, true).fill(fill); | ||
if (typeof encoding !== 'string') | ||
encoding = undefined; | ||
return createUnsafeBuffer(size).fill(fill, encoding); | ||
} | ||
return createBuffer(size); | ||
return new FastBuffer(size); | ||
}; | ||
|
||
/** | ||
|
@@ -188,15 +185,15 @@ Buffer.allocUnsafe = function(size) { | |
**/ | ||
Buffer.allocUnsafeSlow = function(size) { | ||
assertSize(size); | ||
return createBuffer(size, true); | ||
return createUnsafeBuffer(size); | ||
}; | ||
|
||
// If --zero-fill-buffers command line argument is set, a zero-filled | ||
// buffer is returned. | ||
function SlowBuffer(length) { | ||
if (+length != length) | ||
length = 0; | ||
return createBuffer(+length, true); | ||
return createUnsafeBuffer(+length); | ||
} | ||
|
||
Object.setPrototypeOf(SlowBuffer.prototype, Uint8Array.prototype); | ||
|
@@ -205,7 +202,7 @@ Object.setPrototypeOf(SlowBuffer, Uint8Array); | |
|
||
function allocate(size) { | ||
if (size <= 0) { | ||
return createBuffer(0); | ||
return new FastBuffer(); | ||
} | ||
if (size < (Buffer.poolSize >>> 1)) { | ||
if (size > (poolSize - poolOffset)) | ||
|
@@ -218,7 +215,7 @@ function allocate(size) { | |
// Even though this is checked above, the conditional is a safety net and | ||
|
||
// sanity check to prevent any subsequent typed array allocation from not | ||
// being zero filled. | ||
return createBuffer(size, true); | ||
return createUnsafeBuffer(size); | ||
} | ||
} | ||
|
||
|
@@ -231,7 +228,7 @@ function fromString(string, encoding) { | |
throw new TypeError('"encoding" must be a valid string encoding'); | ||
|
||
if (string.length === 0) | ||
return Buffer.alloc(0); | ||
return new FastBuffer(); | ||
|
||
var length = byteLength(string, encoding); | ||
|
||
|
@@ -251,18 +248,30 @@ function fromArrayLike(obj) { | |
const length = obj.length; | ||
const b = allocate(length); | ||
for (var i = 0; i < length; i++) | ||
b[i] = obj[i] & 255; | ||
b[i] = obj[i]; | ||
|
||
return b; | ||
} | ||
|
||
function fromArrayBuffer(obj, byteOffset, length) { | ||
if (!isArrayBuffer(obj)) | ||
throw new TypeError('argument is not an ArrayBuffer'); | ||
|
||
|
||
byteOffset >>>= 0; | ||
|
||
if (typeof length === 'undefined') | ||
return binding.createFromArrayBuffer(obj, byteOffset); | ||
const maxLength = obj.byteLength - byteOffset; | ||
|
||
if (maxLength <= 0) | ||
|
||
throw new RangeError("'offset' is out of bounds"); | ||
|
||
if (length === undefined) { | ||
length = maxLength; | ||
} else { | ||
length >>>= 0; | ||
if (length > maxLength) | ||
throw new RangeError("'length' is out of bounds"); | ||
|
||
} | ||
|
||
length >>>= 0; | ||
return binding.createFromArrayBuffer(obj, byteOffset, length); | ||
return new FastBuffer(obj, byteOffset, length); | ||
|
||
} | ||
|
||
function fromObject(obj) { | ||
|
@@ -279,7 +288,7 @@ function fromObject(obj) { | |
if (obj) { | ||
if (obj.buffer instanceof ArrayBuffer || 'length' in obj) { | ||
if (typeof obj.length !== 'number' || obj.length !== obj.length) { | ||
return allocate(0); | ||
return new FastBuffer(); | ||
} | ||
return fromArrayLike(obj); | ||
} | ||
|
@@ -346,7 +355,7 @@ Buffer.concat = function(list, length) { | |
throw new TypeError('"list" argument must be an Array of Buffers'); | ||
|
||
if (list.length === 0) | ||
return Buffer.alloc(0); | ||
return new FastBuffer(); | ||
|
||
if (length === undefined) { | ||
length = 0; | ||
|
@@ -823,10 +832,26 @@ Buffer.prototype.toJSON = function() { | |
}; | ||
|
||
|
||
function adjustOffset(offset, length) { | ||
offset = +offset; | ||
if (offset === 0 || Number.isNaN(offset)) { | ||
return 0; | ||
} | ||
if (offset < 0) { | ||
offset += length; | ||
return offset > 0 ? offset : 0; | ||
} else { | ||
return offset < length ? offset : length; | ||
} | ||
} | ||
|
||
|
||
Buffer.prototype.slice = function slice(start, end) { | ||
const buffer = this.subarray(start, end); | ||
Object.setPrototypeOf(buffer, Buffer.prototype); | ||
return buffer; | ||
const srcLength = this.length; | ||
start = adjustOffset(start, srcLength); | ||
end = end !== undefined ? adjustOffset(end, srcLength) : srcLength; | ||
const newLength = end > start ? end - start : 0; | ||
return new FastBuffer(this.buffer, this.byteOffset + start, newLength); | ||
}; | ||
|
||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does a separate check for
0
make sense here, i.e.size > 0 && fill !== undefined && fill !== 0
?Buffer.alloc(size, 0)
is equivalent toBuffer.alloc(size)
, so justnew FastBuffer(size)
should work faster in that case.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, I think that would require benchmarking –
createUnsafeBuffer()
returns a slice from the pool, so I’d actually expect that to be faster than an extra typed array allocation.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@addaleax It's not just
createUnsafeBuffer
, it'screateUnsafeBuffer
+fill(0)
.I believe that has been discussed before, and allocation was proven to be faster — that's why simple
Buffer.alloc(size)
does not use the pool.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ChALkeR I know there’s the extra
fill()
in there. But if you say it’s faster, I believe that :)Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@addaleax Btw, nothing in this function uses slices from the pool. Perhaps it should?
Both
new FastBuffer
andcreateUnsafeBuffer
just directly allocate a new instance.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, right. Maybe, but I’d leave that open for another PR, too, especially as it would introduce the subtle change that the return values of
Buffer.alloc()
would share theirbuffer
property.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'm down for that change. The kernel can probably optimize calls to
calloc()
a hair better. Though not going to consider it a blocking change.