-
Notifications
You must be signed in to change notification settings - Fork 8
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
Document the project's high-level approach #7
Conversation
@Fishrock123 Not really … I mean, basically what this says is “refactoring streams in Node”, and I’m totally on board with that, it’s just not really clear into what direction that refactoring goes. That is, besides a switch to pull-based streams – and I’m unsure how that would be integrated with libuv on the C++ level. Maybe that kind of thought is already too far ahead? |
My understanding is that streams3 is already mostly pull-based, so it's really about changing the protocol & implementation internally under the hood here. Building an interface to "transform" it into a streams3 endpoint should not be unreasonable and @mcollina has thought through it to some degree.
We've thought of it - I'm not a total fountain of knowledge about this part but it's some of what I'll be working towards next. My understanding is that libuv streaming interfaces already work closer to this than to StreamBase, or at least that's what my conversations with @jasnell and @trevnorris have lead me to believe. I don't think it would be an integration actually within libuv though, or at least, that has not been discussed yet. Just like stated in the points in the PR here, the goal is to attempt to switch node internals first and then discuss if we really like it before making it a public API. |
Yeah, I think that’s a pretty good goal. :) If the result is basically the pull-based parts of streams3, minus support for object mode, implicit string conversion etc. and there is an easy way to wrap it into a streams3 variant, then yay. 👍
I think I have to disagree with that statement (but maybe James or Trevor can clarify) – the StreamBase API currently has almost a 1:1 correspondence to the libuv API. Maybe I’m getting too much of an “ownership” feeling with respect to our C++ streams implementation given the amount of work I’ve been putting into it over the last months – I guess all I want to say is, it’s not obvious to me how the C++ API could well be shifted to a pull-based model, if at all, and how changes to the JS layer might relate to that. |
For posterity: here is a somewhat-lengthy, related IRC discussion: http://logs.nodejs.org/node-dev/2018-02-21#15:49:57.360 |
A key part of the overall goal is to enable a "fast pipe", meaning that it should be possible to pipe things directly in C++ without passing through JS. |
@mcollina That seems orthogonal to switching to a pull-based approach, or am I missing something? Also, it’s something I’m already working on, just fyi. |
@addaleax can you summarize what you are working on in this regard? If we can achieve that without a massive refactoring, that might be fantastic. |
@mcollina I’ll just go ahead and open a WIP PR against Node for what I have, and we can take it from there. |
Done: nodejs/node#18936 |
Also, to be clear, that PR is not supposed to replace the work we’re doing here. The linked IRC conversation with @Fishrock123 was pretty helpful for me, and I agree that if we can move libuv to a more pull-based approach, then moving towards pull streams in core would be great, too. |
👍 for that! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
e5ffffd
to
73a1561
Compare
I think we are at the point where this could be useful to document the project's approach more formally.
@jasnell & @mcollina eyes would be appreciated. :)
@addaleax Maybe this helps answer your question(s)?