poll_* methods to support custom futures implementations

[dgrr]

[dgrr]

I'll implement the poll_* methods for the WebSocketRead & WebSocketWrite

[dgrr]

btw, if you implement Stream + Sink from futures you can call split. It's difficult for Stream to return a Frame<'_>, or maybe impossible

[matheus23]

Would love to see this land. This library would become a great alternative to tokio-tungstenite and tokio-websockets in that case. Especially since it supports true, lock-free stream splitting.

[matheus23]

FWIW, it'd be nice to have the same poll_ treatment for FragmentCollectorRead :eyes: :grin:

[matheus23]

btw, if you implement Stream + Sink from futures you can call split. It's difficult for Stream to return a Frame<'_>, or maybe impossible

This is true, but internally it means it'll simply create two references to the same thing using a BiLock. So you end up locking every time you use either the Stream or Sink.

Having a "true" split implementation that is lock free would be a lot cooler! :)

---

Also, while it's impossible to write an implementation of Stream<Item = Frame<'???>>, it's possible to map the Frame<'f> into your own domain-specific application struct that doesn't have a lifetime, e.g. by parsing binary and text frames into your own types, before returning them in your stream implementation as Items.

Just spelling this out for anyone reading this :v:

[dgrr]

Having a "true" split implementation that is lock free would be a lot cooler! :)

How is this not lock free? If you use tokio::io::split it will use a Mutex internally.

Also, while it's impossible to write an implementation of Stream<Item = Frame<'???>>, it's possible to map the Frame<'f> into your own domain-specific application struct that doesn't have a lifetime, e.g. by parsing binary and text frames into your own types, before returning them in your stream implementation as Items.

Yes, that's what I do mostly. Because I want my code to be compatible with futures::Stream and Sink. I shouldn't but I had to code it in a bit of a rush 👀

[matheus23]

Having a "true" split implementation that is lock free would be a lot cooler! :)

How is this not lock free? If you use tokio::io::split it will use a Mutex internally.

Yeah no I'm agreeing with you and that's exactly what I'm trying to say. You wrote "if you implement Stream + Sink from futures you can call split.", but that gives you worse performance than having the stream and sink already split from the start :v: (which this PR enables/makes easier!)

[dgrr]

Yeah no I'm agreeing with you and that's exactly what I'm trying to say. You wrote "if you implement Stream + Sink from futures you can call split.", but that gives you worse performance than having the stream and sink already split from the start ✌️ (which this PR enables/makes easier!)

Ah yes, but it is mainly for compatibility because sometimes it might be useful to use futures::StreamExt. Anyways, the user can implement it on their own, given that Frame<'_> cannot be returned as the Item in futures::Stream

[dgrr]

@matheus23 about poll in FragmentCollectorRead, what would you expect? Do you expect a callback also or that it returns early with the mandatory reply frame?

[matheus23]

Personally, I'd highly prefer returning the mandatory frame :)

[dgrr]

Personally, I'd highly prefer returning the mandatory frame :)

Indeed. Me too

[matheus23]

Just as an FYI, I've been trying to get the benchmarks to compile & run here on my NixOS to help diagnose the regression, but I'm still fighting my way through with the linker. Will of course post once I got something.

[dgrr]

Just as an FYI, I've been trying to get the benchmarks to compile & run here on my NixOS to help diagnose the regression, but I'm still fighting my way through with the linker. Will of course post once I got something.

I managed to reproduce the benchmarks, not in macos in Linux, but I cannot find where the regression is

[conradludgate]

Further testing shows that, for large buffers, removing the vectored write support is a significant source of the regressions. Now the difference it just 43500 Msg/sec compared to 44000 Msg/sec

Opened a PR against this PR 😅 https://github.com/dgrr/fastwebsockets/pull/1

[dgrr]

@littledivy it seems that this PR is ok now thanks to the contributions of @conradludgate

[ShabbirHasan1]

@littledivy All checks have passed successfully 🚀. Could you please review and merge this PR at your convenience 🙏.

Thank you @dgrr and @conradludgate for your kind contributions in making this library even better. 🫡

[dgrr]

@littledivy

[spy16]

Any chance this will be merged soon?

[dgrr]

Any chance this will be merged soon?

@spy16 I gave up, just use yawc