-
Notifications
You must be signed in to change notification settings - Fork 164
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
Generalized and precise flow/buffer control #301
Comments
As Yutaka has been advocating, we should stop attempting to specify direct connection between the "underlying source" which the JS code faces and the readable (byte) stream operation. Not directly, but just have a good API surface that could be utilized for optimization if the implementor of the readable (byte) stream wants. Bring your own buffer style reader allows the implementor to directly connect |
I agree with the first post. In #294 (comment) I summed it up in terms of a proposed On the second post, I also agree that there should be no necessity of connecting the view directly to ReadFile or similar. But just in case it wasn't clear, I think it's important to design the stream + underlying source constructor parameter combo so that you could code the underlying source purely in JS. That is, let's say we had almost-direct JavaScript bindings of syscalls (like this for files or this for sockets). Good underlying source code would still have to do more work, e.g. if the socket buffer is overflowing then the underlying source needs to copy into user-space and then later instead of reader.read() going directly to the syscall it copies the previously-saved buffer. But the important thing is that you could in theory express it all in JS using the underlying source constructor parameter + appropriate syscall bindings. So in this world e.g. |
Oh, yes. Sorry I forgot to cite that comment.
Agreed. Thanks for clarification. |
My current thinking based on discussions this morning:
With this change, and a strategy that says HWM = 0, read() will directly correspond to pull(), which is good. (The "if there are pending reads" ensures it.) With a strategy that has HWM > 0, then pull will be called whenever the queue size is less than the high water mark, and it will be called with spaceLeft (or "desiredSize" maybe) = HWM - current queue size, which allows it to make intelligent decisions about how to generate chunks that it will enqueue. |
Some logs of the last week's discussion
|
Instead of creating enqueue, close, and error functions per-stream, and passing (some subset of them) to the underlying source's start() and pull() methods, we instead create an instance of the ReadableStreamController class, which has methods enqueue(), close(), and error(). This closes #309, which contains more discussion and justification. It sets the stage for #301 (which will work by adding another property to the ReadableStreamController class).
Instead of creating enqueue, close, and error functions per-stream, and passing (some subset of them) to the underlying source's start() and pull() methods, we instead create an instance of the ReadableStreamController class, which has methods enqueue(), close(), and error(). This closes #309, which contains more discussion and justification. It sets the stage for #301 (which will work by adding another property to the ReadableStreamController class).
I'm working on this in the precise-flow-control branch. It's going pretty well. Gotta update some more tests (especially around writable streams) and the spec, but shouldn't be too bad. I went with |
This changes the way queuing strategies work, and how backpressure signals are propagated from readable stream controllers, in the service of giving more precise information to be used for flow control. Queuing strategies now explicitly embrace the concept of "high water mark," instead of having a general `shouldApplyBackpressure` method. This allows us to calculate the "desired size to fill a stream's internal queue", and expose it as a `desiredSize` property on the readable stream controller. The algorithm for determining whether to call pull was updated, per #301 (comment), to no longer be based on the boolean "should apply backpressure" logic. The logic is now to avoid calling pull if desired size is <=0 (unless there are pending read requests, in which case pull should be called anyway). In particular, this modifies test expectations in some cases where start enqueues chunks; after this change pull is no longer automatically called after start in such cases. The API changes in detail: - The readable and writable stream constructors now validate the passed queuing strategy and extract its values. - Queuing strategies are no longer treated as instances of classes, but instead as property bags. (That is, we no longer "invoke" `strategy.size`, but instead simply extract the `size` function from strategy and "call" it.) - The built-in queuing strategies no longer validate the passed high water mark argument and store it in an internal slot. Instead they simply define a data property on themselves whose value is the passed high water mark. - Queuing strategies no longer have `shouldApplyBackpressure` methods. - `ReadableStreamController.prototype` gained a `desiredSize` getter. - `ReadableStreamController.prototype.enqueue` (and EnqueueInReadableStream) no longer return a boolean to signal backpressure; instread consult `controller.desiredSize` for more precise information. The correspondence is: `enqueueReturnValue === controller.desiredSize > 0`. Note that writable streams do not yet expose the desired size. We expect to do that later, probably in conjunction with related changes. For now, they translate the desired size into a boolean distinction between "waiting" and "writable", giving the same results as before. Fixes #301. Addresses half of #119.
This changes the way queuing strategies work, and how backpressure signals are propagated from readable stream controllers, in the service of giving more precise information to be used for flow control. Queuing strategies now explicitly embrace the concept of "high water mark," instead of having a general `shouldApplyBackpressure` method. This allows us to calculate the "desired size to fill a stream's internal queue", and expose it as a `desiredSize` property on the readable stream controller. The algorithm for determining whether to call pull was updated, per #301 (comment), to no longer be based on the boolean "should apply backpressure" logic. The logic is now to avoid calling pull if desired size is <=0 (unless there are pending read requests, in which case pull should be called anyway). In particular, this modifies test expectations in some cases where start enqueues chunks; after this change pull is no longer automatically called after start in such cases. The API changes in detail: - The readable and writable stream constructors now validate the passed queuing strategy and extract its values. - Queuing strategies are no longer treated as instances of classes, but instead as property bags. (That is, we no longer "invoke" `strategy.size`, but instead simply extract the `size` function from strategy and "call" it.) - The built-in queuing strategies no longer validate the passed high water mark argument and store it in an internal slot. Instead they simply define a data property on themselves whose value is the passed high water mark. - Queuing strategies no longer have `shouldApplyBackpressure` methods. - `ReadableStreamController.prototype` gained a `desiredSize` getter. - `ReadableStreamController.prototype.enqueue` (and EnqueueInReadableStream) no longer return a boolean to signal backpressure; instread consult `controller.desiredSize` for more precise information. The correspondence is: `enqueueReturnValue === controller.desiredSize > 0`. Note that writable streams do not yet expose the desired size. We expect to do that later, probably in conjunction with related changes. For now, they translate the desired size into a boolean distinction between "waiting" and "writable", giving the same results as before. Fixes #301. Addresses half of #119.
This changes the way queuing strategies work, and how backpressure signals are propagated from readable stream controllers, in the service of giving more precise information to be used for flow control. Queuing strategies now explicitly embrace the concept of "high water mark," instead of having a general `shouldApplyBackpressure` method. This allows us to calculate the "desired size to fill a stream's internal queue", and expose it as a `desiredSize` property on the readable stream controller. The algorithm for determining whether to call pull was updated, per #301 (comment), to no longer be based on the boolean "should apply backpressure" logic. The logic is now to avoid calling pull if desired size is <=0 (unless there are pending read requests, in which case pull should be called anyway). In particular, this modifies test expectations in some cases where start enqueues chunks; after this change pull is no longer automatically called after start in such cases. The API changes in detail: - The readable and writable stream constructors now validate the passed queuing strategy and extract its values. - Queuing strategies are no longer treated as instances of classes, but instead as property bags. (That is, we no longer "invoke" `strategy.size`, but instead simply extract the `size` function from strategy and "call" it.) - The built-in queuing strategies no longer validate the passed high water mark argument and store it in an internal slot. Instead they simply define a data property on themselves whose value is the passed high water mark. - Queuing strategies no longer have `shouldApplyBackpressure` methods. - `ReadableStreamController.prototype` gained a `desiredSize` getter. - `ReadableStreamController.prototype.enqueue` (and EnqueueInReadableStream) no longer return a boolean to signal backpressure; instread consult `controller.desiredSize` for more precise information. The correspondence is: `enqueueReturnValue === controller.desiredSize > 0`. Note that writable streams do not yet expose the desired size. We expect to do that later, probably in conjunction with related changes. For now, they translate the desired size into a boolean distinction between "waiting" and "writable", giving the same results as before. Fixes #301. Addresses half of #119.
This changes the way queuing strategies work, and how backpressure signals are propagated from readable stream controllers, in the service of giving more precise information to be used for flow control. Queuing strategies now explicitly embrace the concept of "high water mark," instead of having a general `shouldApplyBackpressure` method. This allows us to calculate the "desired size to fill a stream's internal queue", and expose it as a `desiredSize` property on the readable stream controller. The algorithm for determining whether to call pull was updated, per #301 (comment), to no longer be based on the boolean "should apply backpressure" logic. The logic is now to avoid calling pull if desired size is <=0 (unless there are pending read requests, in which case pull should be called anyway). In particular, this modifies test expectations in some cases where start enqueues chunks; after this change pull is no longer automatically called after start in such cases. The API changes in detail: - The readable and writable stream constructors now validate the passed queuing strategy and extract its values. - Queuing strategies are no longer treated as instances of classes, but instead as property bags. (That is, we no longer "invoke" `strategy.size`, but instead simply extract the `size` function from strategy and "call" it.) - The built-in queuing strategies no longer validate the passed high water mark argument and store it in an internal slot. Instead they simply define a data property on themselves whose value is the passed high water mark. - Queuing strategies no longer have `shouldApplyBackpressure` methods. - `ReadableStreamController.prototype` gained a `desiredSize` getter. - `ReadableStreamController.prototype.enqueue` (and EnqueueInReadableStream) no longer return a boolean to signal backpressure; instread consult `controller.desiredSize` for more precise information. The correspondence is: `enqueueReturnValue === controller.desiredSize > 0`. Note that writable streams do not yet expose the desired size. We expect to do that later, probably in conjunction with related changes. For now, they translate the desired size into a boolean distinction between "waiting" and "writable", giving the same results as before. Fixes #301. Addresses half of #119.
Before recent redesign, we always had a queue in the JS world behind a readable stream, and we calculated how to exert backpressure to the "underlying source" based on the size of the queue.
We're now separating the streams API semantics visible to the consumer (mainly implemented in the reader) and a queue behind it which doesn't always exist (depending on who (JS, C++ code, kernel, device, etc.) buffers data). This made me think more generally, that is, there're many points of buffering behind streams. The number of such points depends on which API we're using, which platform we're using, etc.
It sounds like both the high water mark & strategy concept, and the precise flow control stuff I proposed only make sense for streams with a queue in JS world. It seems it's hard to well-define
queueSize
,highWaterMark
,pullAmount
, etc. as cross-platform, cross-API feature.To put it differently, for non-JS-queue-backed APIs, what do we guarantee for the consumer when HWM is set to X?
I still guess some kind of best-effort approach is still useful. The UA tries to keep the number of bytes buffered on the path between the ultimate data source (disk, the peer server, etc.) and the JS world. As Domenic originally suggested, the strategy itself should be chosen intelligently by the UA considering memory size, CPU power, browser architecture, etc. etc. and it's responsible only for computing how much data is buffered in the JS world. Combining buffer size info (C++, kernel, JS, ...), the UA would intelligently limits the amount of data to pull.
And, the
pullAmount
or something in the proposed precise flow control APIs will just give a hint about how quickly the consumer can consume data in the form of the number of bytes. It is not directly connected with such as the size of the queue in JS. The stream utilizes the info using some algorithm to control amount of data to pull.The text was updated successfully, but these errors were encountered: