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

writable stream _write and _writev #28408

Closed
safqwf opened this issue Jun 24, 2019 · 6 comments
Closed

writable stream _write and _writev #28408

safqwf opened this issue Jun 24, 2019 · 6 comments
Labels
doc Issues and PRs related to the documentations. good first issue Issues that are suitable for first-time contributors. stream Issues and PRs related to the stream subsystem.

Comments

@safqwf
Copy link

safqwf commented Jun 24, 2019

Version: v12.4.0 or v10.15.3
Platform: Linux 4.15.0-51-generic #55~16.04.1-Ubuntu SMP Thu May 16 09:24:37 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
Subsystem: Stream Duplex, Object mode, piped into itself.

The documentation says that when I implement _writev method, it will receive chunks of data available in the buffer.

What I actually see is that both _write and _writev methods are used simultaneously.
If _write is fast enough to process the buffer until it fills up to more than one object, then only _write is used. Otherwise, for the first object in the buffer _write will be used, and the rest will be sent to _writev.

Example:

DEBUG: Calling _write with 1
DEBUG: Calling _writev with 15
DEBUG: Calling _write with 1
DEBUG: Pushing null to readable
DEBUG: Calling _writev with 15
DEBUG: Calling _write with 1
DEBUG: Calling _writev with 2
DEBUG: Writable final
DEBUG: _writev 16, _write 16
DEBUG: Read 243, Written 243

Is this an expected behavior? I was assuming that when _writev method is implemented, it will be used exclusively.

Thanks

@Fishrock123 Fishrock123 added the stream Issues and PRs related to the stream subsystem. label Jun 28, 2019
@lpinca
Copy link
Member

lpinca commented Jul 10, 2019

This is by design. The first chunk is written with _write(). When the write is complete, buffered chunks, if available, will be written all at once with _writev().

@addaleax addaleax added the doc Issues and PRs related to the documentations. label Jul 10, 2019
@addaleax
Copy link
Member

I’ve added the doc label because this is indeed missing from the documentation and I think a PR to address that would make sense.

@addaleax addaleax added the good first issue Issues that are suitable for first-time contributors. label Jul 10, 2019
parkerbjur added a commit to parkerbjur/node that referenced this issue Jul 15, 2019
The documentation gave the impression that when the _writev method is
used, it is used instead of _write. However, when _writev is used,
_write loads the first chunk of data, then _writev loads all the
remaining bufferred chunks in the write queue. The docs have been
changed to reflect this behavior.

Fixes: nodejs#28408
@asbillings07
Copy link

Does this still need to be worked on? If so I'd like to take it.

@ronag
Copy link
Member

ronag commented Oct 8, 2019

This might be relevant #29639

@addaleax
Copy link
Member

addaleax commented Oct 8, 2019

@asbillings07 You might want to look at #28690 – it’s an open PR for this, where @tyof45 started working on it but that has kind of stalled out. I’d suggest waiting a short time to see if there’s any movement in that PR, and if not, you can feel free to open a new one.

@asbillings07
Copy link

asbillings07 commented Oct 8, 2019

@addaleax Okay sounds good.

HarshithaKP added a commit to HarshithaKP/node that referenced this issue Jan 16, 2020
the exact context of invocation of _writev API is not well known
also, the choice between _write and _writev is not well known.
add a description to make it explicit.

Fixes: nodejs#28408
Refs: nodejs#28690

Co-authored-by: Parker Bjur <bjur.parker45@gmail.com>
@Trott Trott closed this as completed in e47a1eb Jan 16, 2020
codebytere pushed a commit that referenced this issue Feb 17, 2020
The exact context of invocation of _writev API is not well known.
Also, the choice between _write and _writev is not well known.
Add a description to make it explicit.

Fixes: #28408
Refs: #28690

Co-authored-by: Parker Bjur <bjur.parker45@gmail.com>

PR-URL: #31356
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
codebytere pushed a commit that referenced this issue Mar 14, 2020
The exact context of invocation of _writev API is not well known.
Also, the choice between _write and _writev is not well known.
Add a description to make it explicit.

Fixes: #28408
Refs: #28690

Co-authored-by: Parker Bjur <bjur.parker45@gmail.com>

PR-URL: #31356
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
codebytere pushed a commit that referenced this issue Mar 17, 2020
The exact context of invocation of _writev API is not well known.
Also, the choice between _write and _writev is not well known.
Add a description to make it explicit.

Fixes: #28408
Refs: #28690

Co-authored-by: Parker Bjur <bjur.parker45@gmail.com>

PR-URL: #31356
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. good first issue Issues that are suitable for first-time contributors. stream Issues and PRs related to the stream subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants