-
Notifications
You must be signed in to change notification settings - Fork 38
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
Comments
Example implementation: https://ryhl.io/blog/actors-with-tokio/ |
Any thoughts on using an approach like 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. |
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 |
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
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 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) |
Here's the gist i mentioned: https://gist.github.com/Restioson/2b93c1b02ded2aa4422596b3fdd72ef6. It still has a lot of |
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 |
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. |
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. |
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.
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 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 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
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. |
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 |
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.
The text was updated successfully, but these errors were encountered: