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

Why fs.promises.writeFile write buffer multi times with 512KiB limit for each time? #38309

Closed
LongTengDao opened this issue Apr 20, 2021 · 5 comments
Labels
fs Issues and PRs related to the fs subsystem / file system. question Issues that look for answers.

Comments

@LongTengDao
Copy link
Contributor

Why not write the whole buffer one time?

Won't that be faster in fact?

@Ayase-252 Ayase-252 added question Issues that look for answers. fs Issues and PRs related to the fs subsystem / file system. labels Apr 20, 2021
@Ayase-252
Copy link
Member

Could you provide which code snippet in question?

@LongTengDao LongTengDao changed the title Why promises.writeFile writeFile writeFileSync etc write buffer multi times by 8192 for each time? Why promises.writeFile writeFile writeFileSync (readFile etc) write buffer multi times by 8192 for each time? Apr 20, 2021
@LongTengDao LongTengDao changed the title Why promises.writeFile writeFile writeFileSync (readFile etc) write buffer multi times by 8192 for each time? Why promises.writeFile writeFile writeFileSync (readFile etc) write buffer multi times? Apr 20, 2021
@aduh95
Copy link
Contributor

aduh95 commented Apr 20, 2021

Why not write the whole buffer one time?

I think Node.js tries to write the whole buffer, but depending on the host OS/FS/hardware, it might need to write in several bits. How did you come with this 8192 figure?

Won't that be faster in fact?

It certainly would be, but I'm afraid there's nothing we can do at Node.js level.

@RaisinTen
Copy link
Contributor

RaisinTen commented Apr 20, 2021

The only place where I could find 8192 related to fs is in the implementation of readFileSync:

node/lib/fs.js

Lines 427 to 428 in a0261d2

buffer = Buffer.allocUnsafe(8192);
bytesRead = tryReadSync(fd, isUserFd, buffer, 0, 8192);

If the file being read from is not a regular file (we can't tell the file size from statSync if it is not regular, stdin for instance), we create a buffer of size 8192 (value of the macro BUFSIZ of glibc) and read into it. We continue reading and appending the read buffer to an array of buffers while the number of bytes read (returned by tryReadSync) is not 0.

@LongTengDao
Copy link
Contributor Author

LongTengDao commented Apr 20, 2021

Could you provide which code snippet in question?

I think Node.js tries to write the whole buffer, but depending on the host OS/FS/hardware, it might need to write in several bits. How did you come with this 8192 figure?
It certainly would be, but I'm afraid there's nothing we can do at Node.js level.

The only place where I could find 8192 related to fs is in the implementation of readFileSync:

node/lib/fs.js

Lines 427 to 428 in a0261d2

buffer = Buffer.allocUnsafe(8192);
bytesRead = tryReadSync(fd, isUserFd, buffer, 0, 8192);

MathMin(kWriteFileMaxChunkSize, data.byteLength));

Sorry, I made a mistake while reading the source, in fact only fs.promises.writeFile (which I usually use) has a voluntary limit. And the limit is 512KiB, not 8192:

const kWriteFileMaxChunkSize = 512 * 1024;

@LongTengDao LongTengDao changed the title Why promises.writeFile writeFile writeFileSync (readFile etc) write buffer multi times? Why fs.promises.writeFile write buffer multi times with 512KiB limit for each time? Apr 20, 2021
@Linkgoron
Copy link
Member

Linkgoron commented Apr 20, 2021

The writeFile in the promises version does indeed write in chunks, this is in contrast to the callback version which doesn't. I'm not sure why this was done, but I assume that it was done in order to imitate what's done on readFile (#25741 and https://nodejs.org/api/fs.html#fs_performance_considerations).

Note that prior to Node 15.12 the limit was 16 kbytes, and now it's 512 kbytes - so it should be more performant in newer versions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fs Issues and PRs related to the fs subsystem / file system. question Issues that look for answers.
Projects
None yet
Development

No branches or pull requests

5 participants