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

fs: expose scatter/gather syscalls writev() and readv() #2298

Closed
reqshark opened this issue Aug 4, 2015 · 11 comments
Closed

fs: expose scatter/gather syscalls writev() and readv() #2298

reqshark opened this issue Aug 4, 2015 · 11 comments
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. feature request Issues that request new features to be added to Node.js. fs Issues and PRs related to the fs subsystem / file system. good first issue Issues that are suitable for first-time contributors.

Comments

@reqshark
Copy link

reqshark commented Aug 4, 2015

with the awesome work achieved here by @ronkorving in libuv/libuv@2bf7827

the upcoming libuv release will unblock #2167

first mentioned by @trevnorris in that PR, i think it makes a lot of sense to expose scatter/gather syscalls writev and also readv from node.js

@thefourtheye thefourtheye added fs Issues and PRs related to the fs subsystem / file system. discuss Issues opened for discussions and feedbacks. labels Aug 4, 2015
@Trott
Copy link
Member

Trott commented Mar 11, 2016

@reqshark Can you elaborate a little on this? Or maybe link to the specific comment in the PR that you're referring to? Apologies in advance for my ignorance.

@reqshark
Copy link
Author

@Trott oh ok, I remember now what this was about.

i was just thinking how cool would it be to do fs.writev and fs.readv?

API would operate by using fs.open first to get an fd and use it like we do on fs.read/fs.write

this would open the door to more efficient I/O for a wide variety of buffer sizes

@jasnell jasnell added the feature request Issues that request new features to be added to Node.js. label Jun 6, 2016
@Trott
Copy link
Member

Trott commented Jul 7, 2017

#2167 landed. Anyone thinking this feature is a good idea and willing to try to implement and submit a pull request?

@Trott Trott removed the discuss Issues opened for discussions and feedbacks. label Mar 9, 2018
@refack refack changed the title fs: expose scatter/gather syscalls writev() and readv() fs: expose setter/getter syscalls writev() and readv() Nov 11, 2018
@refack
Copy link
Contributor

refack commented Nov 11, 2018

Put into https://github.com/nodejs/node/projects/13 backlog

@refack refack closed this as completed Nov 11, 2018
@zbjornson
Copy link
Contributor

zbjornson commented Jan 26, 2019

@refack did you intentionally change the title from "scatter/gather" to "setter/getter"? writev/readv are correctly called "scatter/gather" syscalls.

@reqshark
Copy link
Author

nice catch @zbjornson!

@reqshark reqshark changed the title fs: expose setter/getter syscalls writev() and readv() fs: expose scatter/gather syscalls writev() and readv() Jan 26, 2019
@addaleax
Copy link
Member

Fwiw, writev is already available on the internal fs binding (as writeBuffers), so that should be very easy to implement. readv shouldn’t be much harder.

I’d like to reopen this. It’s not a trivial first contribution, but if you know some C++ and can copy-paste the rest together, this should be doable (and I’m happy to help), so I’m adding good first contribution.

@addaleax addaleax reopened this Jan 26, 2019
@addaleax addaleax added c++ Issues and PRs that require attention from people who are familiar with C++. good first issue Issues that are suitable for first-time contributors. mentor-available labels Jan 26, 2019
@AnasAboreeda
Copy link
Contributor

@addaleax Is this still available, If yes I would like to pick it up and work on it as my first contribution, and definitely I will need your guidance.

@addaleax
Copy link
Member

@anasfullstack Awesome! I think your best point to start would be to look at how fs.write() for single Buffers is implemented, and try to come up with something similar that calls binding.writeBuffers rather than binding.writeBuffer?

@thangktran
Copy link
Contributor

has this issue gone Stale?

@refack
Copy link
Contributor

refack commented Apr 24, 2019

For anyone interested, all the issues in https://github.com/nodejs/node/projects/13 are looking for someone to "adopt" them. In general they represent ideas that did not raise objections, but have no champion to push them forward.

@Trott Trott closed this as completed in e4bbbcc Aug 17, 2019
targos pushed a commit that referenced this issue Aug 19, 2019
fs with writev allow many buffers to be pushed to underlying OS
APIs in one batch, so this should improve write speed to files.

Refs: #2298

PR-URL: #25925
Fixes: #2298
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@SheikhSajid SheikhSajid mentioned this issue Mar 19, 2020
4 tasks
addaleax pushed a commit that referenced this issue Mar 30, 2020
Fixes: #2298
PR-URL: #32356
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
addaleax pushed a commit that referenced this issue Mar 30, 2020
Fixes: #2298
PR-URL: #32356
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit to targos/node that referenced this issue Apr 25, 2020
Fixes: nodejs#2298
PR-URL: nodejs#32356
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this issue Apr 28, 2020
Fixes: #2298
PR-URL: #32356
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
abhishekumar-tyagi pushed a commit to abhishekumar-tyagi/node that referenced this issue May 5, 2024
fs with writev allow many buffers to be pushed to underlying OS
APIs in one batch, so this should improve write speed to files.

Refs: nodejs/node#2298
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. feature request Issues that request new features to be added to Node.js. fs Issues and PRs related to the fs subsystem / file system. good first issue Issues that are suitable for first-time contributors.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants