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

Benchmark against mpsc channels #116

Open
thomaseizinger opened this issue Jul 11, 2022 · 10 comments · May be fixed by #181
Open

Benchmark against mpsc channels #116

thomaseizinger opened this issue Jul 11, 2022 · 10 comments · May be fixed by #181

Comments

@thomaseizinger
Copy link
Collaborator

xtra is by design small and thus, I think a big contestor for people making a decision on what to use might be tokio's or futures' mpsc channels, spawned as a task with an endless loop.

It would be interesting to benchmark xtra against such a naive actor implementation.

@Restioson
Copy link
Owner

Example implementation: https://ryhl.io/blog/actors-with-tokio/

@thomaseizinger thomaseizinger linked a pull request Aug 30, 2022 that will close this issue
@sinui0
Copy link
Contributor

sinui0 commented Dec 6, 2023

Any thoughts on using an approach like enum_dispatch to eliminate the dynamic dispatch overhead? Would be able to use off the shelf mpsc channels instead, while keeping the same ergonomics.

I'm torn on whether that would be an improvement. Could eliminate the channel impl, but moves more complexity into macros so maybe not smaller on net.

@thomaseizinger
Copy link
Collaborator Author

I think that would be a big ergonomic hit as users would need to interact with that. Some of the dynamic dispatch comes from the user implementing Handler multiple times.

@Restioson
Copy link
Owner

Restioson commented Dec 6, 2023

Any thoughts on using an approach like enum_dispatch to eliminate the dynamic dispatch overhead?

Thoughts? Yes, many 😄 plans, not currently. In the xtra-community matrix chat a while ago I sent a gist which I can try to find, which would basically be 'zero dynamic dispatch' xtra (iirc). or maybe it had one box in there somewhere, I don't remember

Would be able to use off the shelf mpsc channels instead, while keeping the same ergonomics

The reason we have custom channels is actually nothing to do with the message enveloping. For a long time, xtra did indeed use just flume or another off the shelf channel. The issue with the channels is for the multi-actor stuff (message stealing) & broadcast. It's a bit more space and time efficient (and just nicer with selecting etc) to roll it all together

As far as the channel impl, I don't mind getting rid of it if there's a good alternative which would let us go back to the way it was. What I really want is a pointer-sized address. That would just be so cool. Def possible with unsafe, not sure with safe rust only (I'd really like to keep xtra 100% safe as it's one of the major selling points IMO and a big strength)

@Restioson
Copy link
Owner

Restioson commented Dec 6, 2023

Here's the gist i mentioned: https://gist.github.com/Restioson/2b93c1b02ded2aa4422596b3fdd72ef6. It still has a lot of #[async_trait] so it's not box-free by any stretch of the imagination, but it does support static dispatch. It's also more of a sketch than a working prototype. With AFIT I do think that the static actors would be box-free, though.

@sinui0
Copy link
Contributor

sinui0 commented Dec 6, 2023

My thinking something along these lines:

trait Actor {
  type Envelope: MessageEnvelope;
  ..
}

struct PingActor;

impl Actor for PingActor {
  type Envelope = PingMessage;
}

impl Handler<Ping> for PingActor {
  ..
}

impl Handler<Pong> for PingActor {
  ..
}

// Must impl Handler for each variant
#[xtra::Envelope(PingActor)]
enum PingMessage {
  Ping,
  Pong
}

struct Ping;

struct Pong;

which expands to something like this:

struct PingActor;

impl Actor for PingActor {
  type Envelope = PingMessage;
}

impl Handler<Ping> for PingActor {
  ..
}

impl Handler<Pong> for PingActor {
  ..
}

enum PingMessage {
  Ping(Ping),
  Pong(Pong)
}

impl MessageEnvelope for PingMessage {
  type Actor = PingActor;

  async fn handle(self, act: &mut Self::Actor, ctx: &mut Context<Self::Actor>) {
    match self {
      Ping(ping) => {
        act.handle(ping, ctx).await
      },
      Pong(pong) => {
        act.handle(pong, ctx).await
      },
    }
  }
}

struct Ping;

struct Pong;

simplified loop:

async fn run<A: Actor>(mailbox: &mut MailBox<A>, actor: &mut A) {
  while let Some(msg) /* Option<A::Envelope> */ = mailbox.next().await {
    let mut ctx = Context {
      running: true,
      mailbox,
    };
    msg.handle(&mut actor, &mut ctx).await;
    ..
  }
}

I'm sure there is complexity I'm not accounting for here, but the ergonomics seem to remain intact with the exception of extra macro. No allocation or dynamic dispatch involved, though the user may want to Box some of their variants to keep the size of PingMessage under control.

@thomaseizinger
Copy link
Collaborator Author

If we - by default - encourage use of a macro for the creation of handlers, this could probably be hidden from the user altogether. See #111.

@Restioson
Copy link
Owner

How would we collect all the handlers together at compile time if using a macro?

Another thing is I'd rather always have a 'macro-escape-hatch' so that users don't have to use macros.

@sinui0
Copy link
Contributor

sinui0 commented Dec 7, 2023

The reason we have custom channels is actually nothing to do with the message enveloping

Firstly, sorry, I was tired yesterday and just realized I did stray off topic here. Let me know if we should move this discussion to a separate issue.

If we - by default - encourage use of a macro for the creation of handlers, this could probably be hidden from the user altogether.

I do quite like the macro API proposed in that issue, however, I think it would be best to keep it separate from this enum approach for the reason @Restioson mentioned above: "How do we collect all the handlers". Our case seems simpler than that of enum-dispatch as we don't need to achieve the same level of generality.

I think an acceptable burden to place on the user is that they must define a single enum which contains all the messages they want to handle. All the impl Handler<M> blocks can still exist in different modules if needed.

Though I'm realizing now we also need to define another type for returns to avoid dynamic dispatch when sending it back through the oneshot.

Something like this:

// I like the idea in the gist to have this generic over the actor
pub trait Message<A: Actor> {
  type Return;

  async fn handle(self, act: &mut A, ctx: &mut Context<A>) -> Self::Return;
}

// This is easy to instruct a user to define, eg "write an enum with a variant for each message".
pub enum PingMessage {
  Ping(Ping),
  Pong(Pong),
}

// A macro could generate this, otherwise the user could write it without the turbofish syntax
// Notice that we can lean on the compiler to collect the return types
pub enum PingReturn {
    Ping(<PingActor as Handler<Ping>>::Return),
    Pong(<PingActor as Handler<Pong>>::Return),
}

// This is the more burdensome part to put on the user if they want to avoid macros, though it is
// mostly boilerplate.
impl Message<PingActor> {
  type Return = PingReturn;

  async fn handle(self, act: &mut A, ctx: &mut Context<A>) -> Self::Return {
    match self {
      PingMessage::Ping(ping) => {
        PingReturn::Ping(act.handle(ping, ctx).await)
      },
      PingMessage::Pong(pong) => {
        PingReturn::Pong(act.handle(pong, ctx).await)
      }
    }
  }
}

And just a reminder that with a simple macro it could look like this for the user:

#[xtra::Message(PingActor)]
pub enum PingMessage {
  Ping,
  Pong
}

impl Actor for PingActor {
  type Message = PingMessage;
  ..
}

Everything else could be neatly tucked away from the user with the rest of xtra's components, and this would still be fully compatible with #111 which could be limited to just implementing the Handler<M> blocks.

Another thing is I'd rather always have a 'macro-escape-hatch' so that users don't have to use macros.

This is a fair point. This approach does put more burden on the user if they don't want to utilize macros, though I think documenting the required steps would be relatively simple.

Static Dispatch

I believe the "static dispatch" you were seeking in your gist would come for free with this enum approach, as the compiler knows to optimize branching away when an enum has 1 variant.

@Restioson
Copy link
Owner

I believe the "static dispatch" you were seeking in your gist would come for free with this enum approach, as the compiler knows to optimize branching away when an enum has 1 variant.

The static dispatch I was seeking was not having to box the message into an envelope which calls the actor handler, so I think this would also cover that if done right

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

Successfully merging a pull request may close this issue.

3 participants