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

Align functionality of send, request, and delegate #1189

Closed
dominiklohmann opened this issue Dec 12, 2020 · 5 comments
Closed

Align functionality of send, request, and delegate #1189

dominiklohmann opened this issue Dec 12, 2020 · 5 comments

Comments

@dominiklohmann
Copy link
Contributor

dominiklohmann commented Dec 12, 2020

send, request, and delegate are the big three for communication between actors. There are, however, two deficiencies in their APIs.

  1. request and delegate lack scheduled_ and delayed_ variants.
  2. delegate does not work on all types of actor handles. E.g., in the implementation of a typed actor's behavior where self is a typed_actor<...>::stateful_pointer<...>, it is often needed to express delegation to self as self->delegate(static_cast<typed_actor<...>>(self), ...). Simply calling self->delegate(self, ...) results in an incomprehensible static assertion trigger.

All of {scheduled,delayed}_{request,delegate} can already be implemented by users, but they're hard to get right, especially with the type checking for typed actors.

I think these convenience changes would be big wins for the usability of CAF, but I am not knowledgeable about the internals to implement them by myself correctly.

On a side note, thanks for maintaining CAF. It's really a great project. I've migrated lots of code to typed behaviors recently, and I really appreciate how they make complex actor behaviors maintainable.

@Neverlord
Copy link
Member

Not exactly a duplicate, but still related: #584. From a user perspective, I probably would expect the delayed_ and scheduled_ variants for all functions as well. Still a bit torn if run_delayed wouldn't be a more general solution. Perhaps the right answer is having both, though.

I think these convenience changes would be big wins for the usability of CAF, but I am not knowledgeable about the internals to implement them by myself correctly.

No worries. Thanks for opening the ticket!

On a side note, thanks for maintaining CAF. It's really a great project.

Thanks! Always nice to receive such feedback! 🙂

@dominiklohmann
Copy link
Contributor Author

I slightly reworded point (2) of the first post to clarify the issue.

@Neverlord
Copy link
Member

Just a quick heads-up. I've put a note for the delegate thing. That just sounds like a straight up bug and delayed_delegate like a missing feature that'd be easy to implement.

I'm still a bit torn on delayed_request. Yes, it breaks symmetry to not have that. And maybe that's all the reason we need to implement it. I'll reconsider it after 0.19 is out.

By the way, thanks a lot for driving so many discussions lately. I may not always agree, but your input is much appreciated. 🙂

@dominiklohmann
Copy link
Contributor Author

Just a quick heads-up. I've put a note for the delegate thing. That just sounds like a straight up bug

Yes, that would be nice to see so we can write rp.delegate(self, ...) and self->delegate(self, ...) to delegate from one handler to another in the same actor.

and delayed_delegate like a missing feature that'd be easy to implement.

Current workaround is this, which is fine considering how rare of an operation a delayed delegation is:

auto rp = self->make_response_promise<T>();
self->delayed_run(delay, [rp]() mutable {
  rp.delegate(...)
});
return rp;

That's fine for personal use, although it does have some overhead as you explained in #1362 (comment). At least personally I don't mind, as I do not use delayed/scheduled anything in performance-sensitive actors, but rather for sending out metrics or to stop something after some timeout.

If you implement this, I think the most important aspect is implementing it not only for scheduled actors, but also typed and untyped response promises, as I think delegations are much more commonly used through a promise.

I'm still a bit torn on delayed_request. Yes, it breaks symmetry to not have that. And maybe that's all the reason we need to implement it. I'll reconsider it after 0.19 is out.

I don't have a strong need for this currently, but it'd certainly be easier to teach to new developers using CAF than the workaround using delayed_run.

Feel free to consider this issue completed once the delegate handle argument bug is fixed.

@Neverlord
Copy link
Member

Rendered obsolete by #1745.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants