Make response body streams readable byte streams #1246
Conversation
|
We're also blocked by whatwg/streams#1114. When "clone a body" tees a stream, the result must also be a readable byte stream. |
fetch.bs
Outdated
| <li><p><a for=ReadableStream>Enqueue</a> a {{Uint8Array}} wrapping an {{ArrayBuffer}} | ||
| containing <var>bytes</var> into <var>stream</var>. | ||
| <li><p><a for=ReadableStream>Enqueue bytes</a> into <var>stream</var> given | ||
| <var>bytes</var>. |
There was a problem hiding this comment.
There is an existing problem that this needs to queue a task. I can PR that, but this will have to be rebased then.
There was a problem hiding this comment.
Re-reading this section it feels like most of the section needs to take place in a task. (I.e., everything dealing with stream.) Just the reading of the bytes from the network needs to be done in parallel.
annevk
force-pushed
the
readable-byte-streams
branch
from
fcf2e88 to
3f03a81
Compare
There was a problem hiding this comment.
I personally prefer not using commas before and if there are only two "simple" things.
What I don't understand here is that you still get a fresh view if the BYOB view cannot be written into due to its size. Wouldn't an error or request for a new view be more appropriate? If the idea is to avoid allocations presumably you would not want such allocations to nevertheless happen silently sometimes?
| <li><p>Let <var>view</var> be null. | ||
|
|
||
| <li><p>If <var>stream</var>'s <a for=ReadableStream>current BYOB request view</a> is | ||
| non-null, and <var>bytes</var>'s <a for="byte sequence">length</a> is less than |
There was a problem hiding this comment.
| non-null, and <var>bytes</var>'s <a for="byte sequence">length</a> is less than | |
| non-null and <var>bytes</var>'s <a for="byte sequence">length</a> is less than |
| non-null, and <var>bytes</var>'s <a for="byte sequence">length</a> is less than | ||
| <var>stream</var>'s <a for=ReadableStream>current BYOB request view</a>'s | ||
| <a for=BufferSource>byte length</a>, then set <var>view</var> to <var>stream</var>'s | ||
| <a for=ReadableStream>current BYOB request view</a>, and <a for=ArrayBufferView>write</a> |
There was a problem hiding this comment.
| <a for=ReadableStream>current BYOB request view</a>, and <a for=ArrayBufferView>write</a> | |
| <a for=ReadableStream>current BYOB request view</a> and <a for=ArrayBufferView>write</a> |
Interesting. This is the first time we're hearing this feedback. I'm not sure how these ideas would work. If you ask for a 1 KiB buffer, and Fetch is unable to restrict what it pulls from the network to less than 16 KiB, you'd get an error... and then what would you do? At that point Fetch has 16 KiB in memory in a new buffer. The only things we can do are:
The first option is more friendly toward people who are using BYOB readers to read specific numbers of bytes, e.g. when parsing a protocol, and so is what we chose. I don't quite understand what you mean by "request for a new view" as opposed to error. |
|
We discussed this further in https://matrixlogs.bakkot.com/WHATWG/2021-07-19#L105-L119. I'm a bit surprised at all the transfers, but I suppose that is to avoid exposing races. |
Helps with #1246. The main problem with this PR (which is why I marked it WIP) is that the stream itself is created in parallel and I don't see a good way to avoid that. If we had low-level streams this would be different.
|
Superseded by #1593; somehow I forgot this existed when reviewing that :) |
Depends on whatwg/streams#1130. Closes #267.
(See WHATWG Working Mode: Changes for more details.)
Preview | Diff