Fix invalid high water mark for tee branches #1234
Conversation
strategyHWM must be a positive number because ReadableStreamDefaultControllerShouldCallPull and ReadableByteStreamControllerShouldCallPull return true when desiredSize > 0, so if strategyHWM is zero (and thus desiredSize = strategyHWM - queueTotalSize will initially be 0), then the tee branches would signal backpressure forever.
There was a problem hiding this comment.
chromium doesn't implement tee for byte streams yet. For default streams, it uses highWaterMark 1.0, which is the same as the standard.
The reference implementation uses highWaterMark 0.0 for byte streams, and passes the tests, so I think it is correct. With highWaterMark 0.0, pull() is not called until read() is called, which avoids unnecessary buffering.
| @@ -6459,7 +6459,7 @@ abstract operations are used to implement these "cross-realm transforms". | |||
| 1. Otherwise, return [=a promise resolved with=] undefined. | |||
| 1. Let |sizeAlgorithm| be an algorithm that returns 1. | |||
| 1. Perform ! [$SetUpReadableStreamDefaultController$](|stream|, |controller|, |startAlgorithm|, | |||
| |pullAlgorithm|, |cancelAlgorithm|, 0, |sizeAlgorithm|). | |||
| |pullAlgorithm|, |cancelAlgorithm|, 1, |sizeAlgorithm|). | |||
There was a problem hiding this comment.
This is the SetUpCrossRealmTransformReadable abstract operation, not the ReadableStreamDefaultTee abstract operation.
| @@ -2073,7 +2073,7 @@ The following abstract operations operate on {{ReadableStream}} instances at a h | |||
| 1. Perform ! [$InitializeReadableStream$](|stream|). | |||
| 1. Let |controller| be a [=new=] {{ReadableByteStreamController}}. | |||
| 1. Perform ? [$SetUpReadableByteStreamController$](|stream|, |controller|, |startAlgorithm|, | |||
| |pullAlgorithm|, |cancelAlgorithm|, 0, undefined). | |||
There was a problem hiding this comment.
The value 0 matches what the reference implementation does, and the reference implementation passes the tests.
There was a problem hiding this comment.
Thanks. I did not realize that ReadableStreamDefaultControllerShouldCallPull ReadableByteStreamControllerShouldCallPull return early to so that when strategyHWM = 0, then no queuing is used. That is a very nice feature.
strategyHWM must be a positive number because ReadableStreamDefaultControllerShouldCallPull and ReadableByteStreamControllerShouldCallPull return true when desiredSize > 0, so if strategyHWM is zero (and thus desiredSize = strategyHWM - queueTotalSize will initially be 0) the tee branches would signal backpressure forever.
As written,
tee()would signal backpressure forever. If this were the case we could take the opportunity to fix the backpressure behavior to fully backpressure to the slower consumer rather than only partial backpressure to the faster consumer (#1233). But alas it looks like various vendors have fixed this bug already at least somewhat.Preview | Diff