-
Notifications
You must be signed in to change notification settings - Fork 30
WIP: Unbounded futures mpmc channel #38
WIP: Unbounded futures mpmc channel #38
Conversation
This is a proof-of-concept futures-aware mpsc channel which builds on the internal bounded and unbounded channel flavors.
Sorry for the late reply! The code looks very good so far, and I especially like the abundance of comments. :)
@danburkert I don't know the answer to this question either - is the current notification system in this PR (inherited from your MPSC implementation) going to work? @seunlanlege Just in case you missed the discussion, the subtleties around notifying blocked tasks are explained in this comment. The most important thing to keep in mind is that methods |
@seunlanlege if you want to get more familiar with the internals of futures so that you can implement this more efficiently, I highly recommend @aturon's (wip) book on asynchrony in Rust: https://aturon.github.io/apr/async-in-rust/chapter.html. FWIW, I'm also really excited about the prospect of a futures-aware MPMC channel. Currently I have to do sad things like https://github.com/jonhoo/trawler/blob/17d4f1efcc0a340dfda8be4a8b8cf5ad07a3b441/src/execution/issuer.rs#L34-L40. |
Out of curiosity, what's the current status of this PR? I see some references to how this could be implemented more efficiently, but not being familiar with how this is being benchmarked I'm not sure what the obvious issues are. Would love to see this pushed over the line 👍 |
This PR is currently stalled and based off of an old version of the crate, which has undergone significant changes since then. Futures support is still on the table, but I think we should take a different approach:
|
@habnabit has been trying to make a similar change to the one @stjepang suggests above for the |
oof! Yeah I tried to do this but it ended up being a bit of a pain. Porting over a synchronous API took a lot of work to nail down, because you have to ensure that every path through the async code hits something that will schedule an unpark later, unless you can return The PR linked above shows the initial work I did when I was attempting to factor this behind a trait. I ended up doing a pure-async refactor just to figure out what the semantics even needed to be. Originally I intended to fold that back into the above PR in the factored-out way, but at this point I think it might obfuscate more than help. Keeping track of control flow and which things will schedule an unpark was more difficult when abstracted. Another issue was that synchronous code can do work and block a little in Honestly, the final straw that tipped me into abandoning the refactor was lacking HKT. I switched from using I hope my experiences prove useful, at least. I'll keep subscribed to the issue too. |
So I've played with this a little bit and tried porting channels to futures, but ultimately gave up because the thread wakeup mechanism is so deeply tied to all parts of the crate. It'd probably be easier to make a new crate with futures support from scratch. I'll give that a shot. :) |
@stjepang unfortunately I don't have enough knowledge to contribute to this, but I'm interested in crossing futures and crossbeam-channel. If you already started hacking on this, is it somewhere public? |
notes:
I've tested this here(yes I actually have a need for an mpmc futures channel) and it works for the most part.
https://github.com/SeunLanLege/arc-reactor/blob/futures-mpmc/src/ArcCore/reactor.rs#L64-L139
A lot of work still needs to be done, as regards to drop semantics and a more efficient approach to task notification. plus there's a lot i don't understand like:
1.why we need to track a task's notification state? or maybe i don't fully understand what tasks really are. 😢
but yeah, pointers are welcome.