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

Move high water mark to readers? #375

Closed
domenic opened this issue Jul 6, 2015 · 1 comment
Closed

Move high water mark to readers? #375

domenic opened this issue Jul 6, 2015 · 1 comment

Comments

@domenic
Copy link
Member

domenic commented Jul 6, 2015

In #294 (comment) we had a plan that high water mark would move to readers. There are many parts to this proposal, and I am not sure anymore that any of them are good. I want to spin this off into a separate issue to discuss each part.

(A) Should we move highWaterMark to the reader?

This moves the control of the high water mark from the stream creator to the stream consumer, and allows it to vary over time. This is arguably more correct.

(Currently, the stream creator could expose it to the consumer, e.g. by doing fetch(..., { highWaterMark: 1024 }). But that is up to the stream creator and is kind of awkward.)

However, there are a couple counterarguments:

  • Most consumers simply do not care about the high water mark. Evidence from Node.js streams back this up; everyone just sticks with the default. So maybe the stream creator should make this decision, instead of burdening the stream consumer with it.
    • Counterpoint: we could let the stream creator choose the default, overridden by the consumer when the consumer explicitly passes in a highWaterMark option to getReader.
  • If we say that the high water mark is part of the reader, then the stream has no guidance on whether to buffer or not while there is no reader. Is that OK?
    • Counterpoint: again, we could let the stream creator choose a default behavior for when there is no reader.

(B) Should we disallow ReadableStreams with no pull() from having a high water mark specified?

The idea here is that a ReadableStream wrapping a WebSocket will not be able to apply backpressure, and so if you do wsrs.getReader({ highWaterMark: 1024 }), it should fail, because there's no way for it to satisfy your desire to keep memory usage near 1024 bytes.

The problem with this idea is that I am not sure there is a one-to-one correspondence between streams whose underlying source implements pull(), and streams who can apply backpressure.

(C) Should we add similar underlying source vs. highWaterMark restrictions to ReadableByteStream?

The two ideas proposed there were:

  • If a high water mark is specified but the underlying byte source does not implement pull(), then the underlying byte source is only pull-on-demand, and will not be able to buffer up to the high water mark. This should throw.
  • If BYOB is specified but the underlying byte source does not implement pullInto(), then the underlying byte source is not able to do BYOB, and so this should throw.

My answers are:

  • (A) maybe... does this gain us very much?
  • (B) and (C) no. It is more important to allow uniform use of the APIs than it is to disallow inefficient use.
@domenic
Copy link
Member Author

domenic commented Oct 15, 2015

Closing; this can be added later if we discover it's the right design (by allowing the consumer-specified HWM to override the creator-specified one), but re-reading my arguments above, I don't think it is actually a great idea.

@domenic domenic closed this as completed Oct 15, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

1 participant