anmonteiro / httpun-ws

Other
28 stars 14 forks source link

Current interface uses a mixture of push and pull style; pull style only should be better #34

Open aantron opened 2 years ago

aantron commented 2 years ago

For flow control by the reader, it's probably best to have a pull-only interface, i.e. one where buffers are filled and data is read only once the user/reader asks for it.

At the moment, AFAICT, the highest layer of interaction with websocket/af is a push-style interface, in which websocket/af and the underlying runtime appear to read as much data as they can, and expect to be able to call the input handlers at their own pace.

https://github.com/anmonteiro/websocketaf/blob/248a2cb0dcffa51996c3ad7643577dce75d67454/lib/websocketaf.mli#L229-L231

Once inside the frame handler, and the user has a Payload.t, the user reads payloads using a new pull-style interface:

https://github.com/anmonteiro/websocketaf/blob/248a2cb0dcffa51996c3ad7643577dce75d67454/lib/websocketaf.mli#L8-L12

It seems that only the user's delays in the reading of payloads, which are interleaved in the WebSocket stream with frame headers, would prevent the push interface from potentially becoming a problem, with frames being received at the speed they come over the network, rather than the speed the user is willing or able to receive them.

It seems it would be better to convert the frame handler layer into a pull interface as well, where the user is able to provide a one-time callback to be called for the next frame whenever the user is ready for one frame, rather than one callback to be called repeatedly.

In practice, I already convert frame-receiving to pull style using a queue. If websocket/af inherently was pull-only, Dream could be more sure that this queue cannot grow without bound.

anmonteiro commented 2 months ago

70 adds backpressure to payload read operations, which is a bit better than the previous state.

I'm a bit reluctant to convert the input handlers into a "pull style" callback to be called only when one is "scheduled", since the current approach sort of mirrors the "request handler" pattern a bit better IMO.