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

Disconnect entities when they are using intra-process communication #17

Open
alsora opened this issue Jul 10, 2019 · 4 comments
Open

Disconnect entities when they are using intra-process communication #17

alsora opened this issue Jul 10, 2019 · 4 comments

Comments

@alsora
Copy link
Contributor

alsora commented Jul 10, 2019

A relevant problem of ROS2 is that it wastes resources when you have both inter and intra-process communication enabled.

For example:

  • Process 1: Node A publishes on my_topic, Node B subscribes on my_topic. They use intra-process communication.
  • Process 2: Node C subscribes on my_topic.

In this configuration Node A is publishing both inter and intra-process.
The RMW is not aware of the existence of Intra-process communication, so, every message published inter-process is delivered to both nodes.
Currently the subscription in Node B would discard the inter-process message received from Node A (since they are in the same intra-process context). However, this is done at the rclcpp layer, after the message has been sent and deserialized.

Is there a plan for implementing a feature for "disconnecting" entities that are already using intra-process communication?

Does CycloneDDS already provide functions for disconnecting a pair of entities without deleting them?

Thank you

@eboasson
Copy link
Collaborator

It is something that worth considering, certainly, but the various "ignore" functions in the DDS specification are not (yet) available in Cyclone. All there is today is the "ignore local publications" flag in the rmw_create_subscription function, which I implemented in the Cyclone RMW as ignoring publications local to the same ROS node/DDS participant:

https://github.com/atolab/rmw_cyclonedds/blob/5299a8391638276267808ffbfa262205fc26ce52/rmw_cyclonedds_cpp/src/rmw_node.cpp#L634-L636

In Cyclone the "ignore local" setting is actually a tri-state that also allows you to ignore everything in the same process. It sounds like that might the behaviour that you are looking for. What I am unsure about is the expected behaviour of the RMW layer, whether there is a way for the RMW implementation to know whether intra-process communication in ROS is enabled in the first place, and whether that setting should be used regardless of the "ignore local publications" flag if intra-process communication is enabled.

The true solution to the problem (in my, perhaps a bit biased, view) is that ROS2 should not even do any intra-process optimization: and instead leave that to the RMW/DDS implementations ... I believe that certainly in the case of Cyclone it should be fairly straightforward to avoid the serialization and copying in the cases where the intra-process optimizations of ROS2 apply.

@alsora
Copy link
Contributor Author

alsora commented Jul 11, 2019

Thank you!
I was not aware of the different values for that setting.
I changed that line and I was able to achieve my desired behavior.

 if (ignore_local_publications) { 
     dds_qset_ignorelocal (qos, DDS_IGNORELOCAL_PROCESS); 
} 

This works fine in my use-case, where I know that all the nodes will have Intra-Process Communication enabled.

From my point of view, Intra-Process Communication in ROS2 is something coupled to the concept of ROS2 context. Assuming that you have a single context per process (I haven't seen any example of a different case), the RMW layer can use the context implementation to keep track of which entities are using intra-process communication or not.

ROS2 architecture [1] depicts that intra-process communication (along with type masquerading optimizations) belongs in the language-specific client library.

[1] http://docs.ros2.org/dashing/developer_overview.html#internal-ros-interfaces

@joxoby
Copy link

joxoby commented Jul 11, 2019

The true solution to the problem (in my, perhaps a bit biased, view) is that ROS2 should not even do any intra-process optimization: and instead leave that to the RMW/DDS implementations.

In theory, I agree. The problem is that, as of today, none of the RMW providers that we are aware of, have the required mechanisms (avoiding serialization, copies) for efficient intra-process communication.
This fact pushed us to create a new intra-process-manager in rclcpp.
ros2/design#239
ros2/rclcpp#778

@eboasson
Copy link
Collaborator

@joxoby thanks for the links ... too bad I didn't discover that proposal and those changes a month earlier.

I was thinking of a simple extension to RMW along the lines of passing a smart pointer to publish & from take. For Cyclone, that would be sufficient to defer serialisation until the message would actually have to be transmitted. If all matching subscriptions would be in the same process, that moment would then never come (well, probably unless a C/C++ mixture were to be used), because the intra-process communication in Cyclone already completely avoids the protocol stack.

So now these optimisations really are being done twice ...

Cyclone will keep them because they were and will remain valuable for it outside the context of ROS2. I can imagine that people might still be interested in relying on the lower layers, as that would eliminate having to extend the rclcpp implementation if the set of features of DDS that ROS2 relies on were to grow. (And I think it will: keys and a keep-last history per key value is bound to come one day, for example driven by sensor suites tracking multiple objects.)

But that's a discussion for another day. In any case, @alsora, I'm happy that you found that setting helpful. And thank you for giving me the background on the "context"s, I wondered what they were for and ignored them for the time being. It sounds like something smarter than ignoring them is possible.

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

No branches or pull requests

3 participants