riker-rs / riker

Easily build efficient, highly concurrent and resilient applications. An Actor Framework for Rust.
https://riker.rs
MIT License
1.02k stars 69 forks source link

Async `recv` Functions? #87

Open zicklag opened 4 years ago

zicklag commented 4 years ago

I'm curious, would it make sense to have Actor::recv an async fn? That way you could use futures inside of them with .await and avoid having to ctx.run() and block_on.

The problem I would see with it would be that you wouldn't want to do blocking operations in a recv implementation because it would stall the executor. Tokio handles those situations with a spawn_blocking(|| do_blocking_stuff()) pattern. That might be something to consider.

Anyway, I just wanted to open up the discussion and see what thoughts were on it. I'm really liking the look of Riker so far.

aav commented 4 years ago

As far as I know, rust currently does not support async in traits. Maybe, it makes sense to create AsyncActor that returns Future from recv (and other callbacks).

zicklag commented 4 years ago

The async_trait crate is one way to handle that. Also the blocking crate can handle spawning blocking operations.

aav commented 4 years ago

Meanwhile, I created a small prototype (based on the _asynctrait). I will take a closer look at what I've got and create a branch, so it could be reviewed.

#[async_trait]
impl Actor for MyActor {
    type Msg = MyMessage;

    async fn recv_async(&mut self, _ctx: &Context<Self::Msg>, _msg: Self::Msg, _sender: Sender) {
        println!("*** async message before");
        let _ = Delay::new(Duration::from_millis(1000)).await;
        println!("*** async message after delay");
    }
}
aav commented 4 years ago

Here is the proof of concept: https://github.com/aav/riker/tree/async_recv

There are two alternatives ahead

I personally prefer the first approach, because I believe async is more idiomatic here. And, once, async trait functions will get supported, we will be able to get rid of extra dependency. For consumers who don't need async, the extra trouble will be minimal.

@leenozara what do you think?

igalic commented 4 years ago

For consumers who don't need async, the extra trouble will be minimal.

how so?

aav commented 4 years ago

For consumers who don't need async, the extra trouble will be minimal.

how so?

for those who don't care if recv is async, the extra steps (compared to how it is now) will be:

that's not much, as I think

mankinskin commented 3 years ago

@aav would you mind rebasing your async_recv branch on the current master and maybe make a PR? I think with this patch we could actually get somewhere with this.

I would also like to make the other lifetime methods in actor (pre_start, post_stop, etc) async.