Re: [whatwg/streams] make TransformStream respect backpressure (#550)

isonmad commented on this pull request.

Looking at your fork, as far as I can see the only two changes is that you assume `readableBackpressure === true` at stream construction time (which is not accurate when `highWaterMark > 0`), and you stop suppressing `pull()` calls unconditionally at the start of every `enqueue` call, regardless of whether enqueueing the chunk causes desiredSize to fall to zero or lower.

The only technical purpose `pull()` being called fulfills is signaling that there is no backpressure anymore, isn't it?

You mention 'pending reads' but I don't see what they have to do with anything you changed, can you clarify?

AFAICS pending reads only occur in two cases.
- if the queue is empty and `highWaterMark > 0`, in which case there is no backpressure and calling `pull` doesn't tell us anything new
- the queue is empty and `highWaterMark === 0`, in which case there *is* backpressure and `pull()` calls are not being suppressed

> @@ -262,12 +238,32 @@ class TransformStreamSource {
 
     transformStream._readableController = c;
 
-    return this._startPromise;
+    // delay the first source.pull call until there is backpressure
+    return this._startPromise.then(() => {

Like the comment above this line says, the purpose of this `.then()` is to delay `pull()` being called. It doesn't actually *do* anything except delay the fulfillment of the promise returned by `source.start`, so putting it in the constructor would do nothing.

>      assert(this._writableController !== undefined);
     assert(this._readableController !== undefined);
 
+    const desiredSize = this._readableController.desiredSize;
+    TransformStreamSetBackpressure(this, desiredSize <= 0);

Are there ever going to be any active reads when we're still inside the TransformStream constructor? I thought it was physically impossible for someone to have called `ts.readable.getReader().read()` at this point.

And it already depends on `pull()` invocation and sets backpressure to false whenever `pull()` is called, so I'm not sure what you're suggesting here.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/streams/pull/550#pullrequestreview-5968136