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

cork()/uncork() and pipe() #145

Closed
indutny opened this issue Jun 18, 2015 · 9 comments
Closed

cork()/uncork() and pipe() #145

indutny opened this issue Jun 18, 2015 · 9 comments

Comments

@indutny
Copy link
Member

indutny commented Jun 18, 2015

It would be great if pipe() could somehow do cork()/uncork() to minimize amount of the TCP packets when piping to the net.Socket. Does anyone have an idea how to expose this to users? Or how to handle it?

Should we just cork and uncork on next tick as we do in http?

@calvinmetcalf
Copy link
Contributor

@indutny
Copy link
Member Author

indutny commented Jun 18, 2015

Yeah, but with ._writev call in the end. It is not currently possible to control this.

@indutny
Copy link
Member Author

indutny commented Jun 18, 2015

My use case is following (if you are interested). I have a protocol framer (say HTTP2), which is basically a readable stream. It emits frames in chunks (i.e. separate chunk for header, for body, sometimes more), and always does this synchronously (i.o.w. without delay between the chunks).

The owner of the framer pipes it into the socket, and now all of these pushes in framer result in separate TCP packets (especially when using noDelay), but it could potentially be just a single _writev call. I can do .cork()/.uncork() manually, but it will mean that I will need to reimplement .pipe() in my code.

@calvinmetcalf
Copy link
Contributor

could be an option we could add to pipe

@indutny
Copy link
Member Author

indutny commented Jun 18, 2015

Oh, that sounds right! I forgot that .pipe() has options.

@trevnorris
Copy link

Why not auto cork on the initial write and queue a nextTick to uncork? Could be an option on the pipe, but the handling of packets is automated.

@indutny
Copy link
Member Author

indutny commented Jun 18, 2015

@trevnorris this is what I want :)

@calvinmetcalf
Copy link
Contributor

@indutny nodejs/node#2020

calvinmetcalf added a commit to calvinmetcalf/io.js that referenced this issue Jun 19, 2015
Adds an option to .pipe to cork it before each write and
then uncork it next tick, based on discussion at
nodejs/readable-stream#145
@mcollina
Copy link
Member

Closing now, as the PR has been debated and closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants