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

Enhance PendingSend for multiple sends and add support for objgroup #688

Closed
lifflander opened this issue Feb 13, 2020 · 6 comments
Closed
Assignees

Comments

@lifflander
Copy link
Collaborator

What Needs to be Done?

We need support for multiple sends. It should push/pop also. We can cleanup much to the manual PendingSend creation code (like in sendMsgUntypedHandler in the VRT collection manager) if that becomes the case.

@PhilMiller
Copy link
Member

There's several requests kinda bundled up here. I generally see how they fit together, but if you have more elaborated thoughts on your vision, it would be useful to have them written down.

@PhilMiller
Copy link
Member

For the multiple-message use case, what do you think of the approach of adding a method along these lines:

template <typename MsgT>
void PendingSend::addMessage(MsgSharedPtr<MsgT> in_msg, SendActionType const& in_action)
{
  send_action_ = [=, old_msg = msg_, old_size_ = msg_size_, old_action = send_action_]
            (MsgVirtualPtr<BaseMsgType> &msg) {
    old_action(old_msg);
    in_action(in_msg);
  };
}

With a suitable overload for a simple sized message, assertions about the epochs matching , etc.

@nmm0
Copy link
Collaborator

nmm0 commented Feb 19, 2020

@lifflander @PhilMiller I can split objgroup support into a separate issue, and would be up for working on it now that I finally have some time. I need it for some of my contact code

@PhilMiller
Copy link
Member

PhilMiller commented Feb 19, 2020 via email

@lifflander
Copy link
Collaborator Author

The use case for multiple sends in EMPIRE has become moot.

In that case, let's leave this issue open until we are sure, but let development continue on #702 for the required use cases, headed up by @nmm0

@lifflander
Copy link
Collaborator Author

This is done.

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

3 participants