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

WHATWG streams #2

Open
lgrahl opened this Issue Dec 28, 2017 · 8 comments

Comments

Projects
None yet
4 participants
@lgrahl
Copy link

lgrahl commented Dec 28, 2017

I've experimented with WHATWG streams for a few weeks. In my little demo it works pretty good now (apart from a few nits here and there that should be resolvable) and makes a much more composable API. Once more people start utilising WHATWG streams, piping from one stream to another or applying transformations can be made without much of a hassle. It also brings the advantage that data can be fed from the user application into the QUIC stream buffer directly without requiring to copy it.

I had to pause working on this as I need to work on my master thesis now. However, I don't want to hold back this idea any further and show you my current results. In case anyone else wants to jump in, feel free! Otherwise, I'll hopefully come back to this once I have some spare time.

Feedback welcome!

Spec

I've updated the spec but it's still work in progress. While methods and attributes of RTCQuicStream have been updated for WHATWG streams, general description and specification of the construction of a readable and a writable stream instance is still missing. However, you can have a look at the code if you want to see how I've written it for the demo.

The spec changes can be found here and on GitHub.

Note: This was originally created for the ORTC Spec, so my spec changes may not be entirely accurate for this spec here.

Demo

The demo can be started here and the code can be found here.

@annevk

This comment has been minimized.

Copy link
Member

annevk commented Dec 28, 2017

@pthatcherg

This comment has been minimized.

Copy link
Collaborator

pthatcherg commented Jan 11, 2018

I looked at this a little closer, and it does have a certain elegance to it for reading and writing vs. the lower level approach we have right now.

But do we really want to take on the ReadableStream and WritableStream as dependencies? It just seems like so much extra complexity/dependency to pull in without a big win.

@annevk

This comment has been minimized.

Copy link
Member

annevk commented Jan 11, 2018

The big win would be platform consistency and being able to tee to transform streams and such. E.g., the web's networking API uses these same APIs (well, just ReadableStream for now).

@lgrahl

This comment has been minimized.

Copy link
Author

lgrahl commented Jan 11, 2018

It also has at least one (theoretical) advantage over the current API: If the reader is fast enough and uses byob mode, the data can be written directly into the buffer that has been provided by the user.

I also want to mention two remaining issues:

  • writer.desiredSize does not return the value I'd like it to return. I use a ByteLengthQueuingStrategy with WritableStream and return a Promise in writer.write that resolves before all data of that write call has been written. Basically, writer.desiredSize as it is now does not factor in the remaining length of the currently pending write. (@annevk suggestions welcome)
  • It would be good to have a WritableStreamBYOBWriter, see whatwg/streams#495 but this is a spec issue.
@annevk

This comment has been minimized.

Copy link
Member

annevk commented Jan 11, 2018

@ricea, @domenic, and @tschneidereit should be able to help with those issues.

@lgrahl

This comment has been minimized.

Copy link
Author

lgrahl commented Sep 9, 2018

Still believe this is one of the most key API wins we could achieve with QUIC.

@annevk

This comment has been minimized.

Copy link
Member

annevk commented Oct 9, 2018

@aboba what does the icebox label mean? To be clear, non-adoption of the Streams API will likely lead to objections from Mozilla, the TAG, Chrome's API owners, etc.

And not engaging on this matter with @lgrahl is just rather rude.

@aboba aboba added PR exists and removed icebox labels Oct 10, 2018

@aboba

This comment has been minimized.

Copy link
Contributor

aboba commented Oct 10, 2018

@annevk Seth's PR #70 provides some of the pre-requisites for support of WHATWG streams (such as support for readable and writable streams). @pthatcherg is looking into what additional changes are necessary. To reflect this work-in-progress, I've changed the label to "PR Exists".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment