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.

Sign up for GitHub

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

Jump to bottom

v12 - avoid needless data copying in fs.promises.writeFileHandle when called with Uint8Array #35343

Closed
joelrbrandt opened this issue Sep 25, 2020 · 0 comments
Closed

v12 - avoid needless data copying in fs.promises.writeFileHandle when called with Uint8Array #35343

joelrbrandt opened this issue Sep 25, 2020 · 0 comments
Labels
fs Issues and PRs related to the fs subsystem / file system.

Comments

@joelrbrandt
Copy link
Contributor

Is your feature request related to a problem? Please describe.
In node v12, fs.promises.writeFileHandle performs needless data copying when writing a Uint8Array. See:
https://github.com/nodejs/node/blob/v12.x/lib/internal/fs/promises.js#L151

If data is a Uint8Array, the local variable buffer will actually be a Uint8Array object. Calling .slice on a Uint8Array creates a shallow copy of the values. This is documented both on MDN and in the node documentation.

Describe the solution you'd like
Don't use .slice on Uint8Arrays

Describe alternatives you've considered
Manually making a Buffer out of my Uint8Array before calling fs write functions.

Thanks for all your hard work in making Node awesome!
@watilde watilde added the fs Issues and PRs related to the fs subsystem / file system. label Sep 26, 2020
targos added a commit to targos/node that referenced this issue Sep 27, 2020
@targos
fs: fix fs.promises.writeFile with typed arrays
5c604bc 
Before this change, only the first part of typed arrays which have more
than 1 byte per element (e.g. Uint16Array) would be written.
This also removes the use of the `slice` method to avoid unnecessary
copying the data.

Fixes: nodejs#35343
@targos targos mentioned this issue Sep 27, 2020
Closed
MylesBorins pushed a commit that referenced this issue Sep 29, 2020
@targos @MylesBorins
fs: fix fs.promises.writeFile with typed arrays
99a79e3 
Before this change, only the first part of typed arrays which have more
than 1 byte per element (e.g. Uint16Array) would be written.
This also removes the use of the `slice` method to avoid unnecessary
copying the data.

Fixes: #35343

PR-URL: #35376
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
joesepi pushed a commit to joesepi/node that referenced this issue Jan 8, 2021
@targos @joesepi
fs: fix fs.promises.writeFile with typed arrays
19449d2 
Before this change, only the first part of typed arrays which have more
than 1 byte per element (e.g. Uint16Array) would be written.
This also removes the use of the `slice` method to avoid unnecessary
copying the data.

Fixes: nodejs#35343

PR-URL: nodejs#35376
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
fs Issues and PRs related to the fs subsystem / file system.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fs: fix fs.promises.writeFile with typed arrays targos/node
2 participants
@joelrbrandt @watilde