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

Validate queueing strategies are sufficiently powerful #119

Open
domenic opened this issue Jun 17, 2014 · 4 comments
Open

Validate queueing strategies are sufficiently powerful #119

domenic opened this issue Jun 17, 2014 · 4 comments

Comments

@domenic
Copy link
Member

domenic commented Jun 17, 2014

I'm going to consolidate a few threads into this one.

Current Spec

The current spec is largely based around Node's high water marks, providing I believe the exact same amount of power, although in a different API package.

ReadableStream

  • When enqueuing a chunk, its size, determined by strategy.size, is added to the total queue size.
  • After the chunk has been enqueued, if the stream was currently "readable", consult strategy.needsMore(currentTotalSize), to determine the return value of enqueue(chunk).
  • Chunks can be dequeued by the user at any time by calling rs.read().

WritableStream

  • When enqueuing or dequeing a chunk, the total size is adjusted, and the call to [[syncStateWithQueue]]() adjusts ws.state to be either "waiting" or "writable", depending:
    • If the queue is empty, it always becomes writable.
    • Otherwise, it consults strategy.needsMore(totalCurrentSize); if the return value is truthy, the stream becomes "writable"; if falsy, the stream becomes "waiting".

Ideas and Help Wanted

@tyoshino previously mentioned, in #76 and summarized in #24 (comment) (with some substitutions for the new terminology):

The base ideas behind my proposals are:

  • Allow user defined code to hook any action that may result in [enqueue/dequeue] on the internal [queue]
  • Allow the code to investigate the contents of the [queue]. Either of:
    • whole contents every time
    • just see newly [enqueued chunk] and return the integer cost of it

@tyoshino, I am curious whether you think the current proposal accomplishes this or not? I suspect not. In which case, what would help? I think I got confused in the various previous threads.

In #76 the thread ended with:

So, I think

  • we should be passing along kind of information say, space available (HWM - filled amount) or speed in a RS on pull. It doesn't need to be limiting push, just work as a hint is OK.
  • and also there should be some interface for a consumer to tell its consuming speed to the RS (or strategy object?) in addition to the signal of read() calling speed.

This sounds more different than the current proposal, and more along the lines of things that came up while discussing with @slightlyoff and @willchan. It might tie into #111, which will probably be in a separate BinaryStream subclass, but we should be sure the base interface exposes enough information or hooks too.

@domenic
Copy link
Member Author

domenic commented Jun 17, 2014

Oops. I realized there's a bug in the current spec's strategy usage: #121. Will update the "current spec" section once it is fixed.

@domenic
Copy link
Member Author

domenic commented Jun 20, 2014

Bug fixed; let the discussion resume!

@tyoshino
Copy link
Member

Great progress, Domenic! Sorry for delay but I'm taking a look at the new revision. Let me reply to each sub-issue separately.

Allow user defined code to hook any action that may result in [enqueue/dequeue] on the internal [queue]

[[callPull]] is still invoked only when [[queue]] becomes empty for the read() method. So, this comment of mine is not yet addressed. That said, by calling wait(), the user can explicitly invoke [[callPull]]. This might be sufficient and may provide more flexibility. I continue to review.

@tyoshino
Copy link
Member

[[enqueue]] always returns true when [[state]] was "waiting". This is remainder of 6e2a21d, maybe. Now after merger, can we make this also consult strategy?

tyoshino added a commit to tyoshino/streams that referenced this issue Jul 23, 2014
Also adds code to handle an error in needsMore

Issue: whatwg#119 (comment)
tyoshino added a commit that referenced this issue Jul 24, 2014
Also adds code to handle an error in needsMore

Issue: #119 (comment)
domenic added a commit that referenced this issue Apr 7, 2015
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.
domenic added a commit that referenced this issue Apr 10, 2015
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.
domenic added a commit that referenced this issue Apr 14, 2015
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.
domenic added a commit that referenced this issue Apr 15, 2015
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.
domenic added a commit that referenced this issue Apr 20, 2015
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants