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

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

Open
Tracked by #181
aantron opened this issue Nov 28, 2021 · 1 comment

Comments

@aantron
Copy link

aantron commented Nov 28, 2021

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
Copy link
Owner

#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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants