Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

fs: pool size coincide with ReadStream bufferSize #4518

Closed
wants to merge 2 commits into from
Closed
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
8 changes: 3 additions & 5 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ var Readable = Stream.Readable;
var Writable = Stream.Writable;

var kMinPoolSpace = 128;
var kPoolSize = 40 * 1024;

var O_APPEND = constants.O_APPEND || 0;
var O_CREAT = constants.O_CREAT || 0;
Expand Down Expand Up @@ -1370,8 +1369,8 @@ fs.realpath = function realpath(p, cache, cb) {

var pool;

function allocNewPool() {
pool = new Buffer(kPoolSize);
function allocNewPool(poolSize) {
pool = new Buffer(poolSize);
pool.used = 0;
}

Expand Down Expand Up @@ -1468,7 +1467,7 @@ ReadStream.prototype._read = function(n, cb) {
// discard the old pool. Can't add to the free list because
// users might have refernces to slices on it.
pool = null;
allocNewPool();
allocNewPool(this._readableState.bufferSize);
Copy link
Member

Choose a reason for hiding this comment

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

What if this._readableState.bufferSize is very small?

Copy link
Author

Choose a reason for hiding this comment

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

if this._readableState.bufferSize is very small, buffer of fs.read() gets small so that it takes more time to finish reading as in https://github.com/joyent/node/blob/master/lib/fs.js#L1491

If options.bufferSize is 0, it is set to 16k by default as in https://github.com/joyent/node/blob/master/lib/_stream_readable.js#L35

}

// Grab another reference to the pool in the case that while we're
Expand Down Expand Up @@ -1560,7 +1559,6 @@ function WriteStream(path, options) {

// a little bit bigger buffer and water marks by default
options = util._extend({
bufferSize: 64 * 1024,
lowWaterMark: 16 * 1024,
highWaterMark: 64 * 1024
}, options || {});
Expand Down