Poor CPU performance with Type Adapters when using references

[Yadunund]

Bug report

Required Info:

• Operating System:
  • Ubuntu Jammy 22.04
• Installation type:
  • Source
• Version or commit hash:
  • rclcpp 21.1.0
• DDS implementation:
  • fastdds or cyclone
• Client library (if applicable):
  • rclcpp

Steps to reproduce issue

It appears when publishers move + publish a Message<T> and subscription callbacks have a signature of std::function<void(const<MessageT& msg)> , the CPU load is a lot higher compared to when dealing with shared_ptr<Message<T>> or unique_ptr<Message<T>>

From my experience the CPU load for the former is almost an order of magnitude greater.
I've setup a package here with minimal reproducible code.
It appears the Intraprocess manager might not be handing moved references properly.

Expected behavior

CPU load is the same for 1 pub + 1 sub.

Actual behavior

Additional information

---

Feature request

Feature description

Implementation considerations

[alsora]

I'm not the most familiar with type adaptation, but the intra-process manager currently requires full ownership of the data.
If you pass the message to publish by reference or by shared_ptr, there will be 1 additional copy.

[Yadunund]

@alsora in my example I am transferring ownership to the intra-process manager on the publisher side https://github.com/Yadunund/type_adapt_example/blob/f1740909b7122bb2a7defa2b0f78f6fa5ef597fa/src/image_pub_type_adapt_intra_node.cpp#L43

[alsora]

There are two overloads of the publish function that deal with type adaptation.
See them here: https://github.com/ros2/rclcpp/blob/rolling/rclcpp/include/rclcpp/publisher.hpp#L293-L356

The first one takes a unique_ptr and will not do copies when using intra-process communication.
The second one takes a reference and it will copy.

Looking at your example,

auto container = type_adapt_example::ROSCvMatContainer(std::move(frame), std::move(header));

is not a unique_ptr, even if its fields are. Thus it will use the second API.

[Yadunund]

I see, thanks for explaining that. I had assumed there was a publisher overload for rvalue references.

I wonder if the code in the demos might also assume the same as I had. https://github.com/ros2/demos/blob/fae18af7c158f9bab5af6a476971dd22f0ea29a0/image_tools/src/cam2image.cpp#L151-L155

I guess we could overload a publish() that accepts an rvalue reference but not sure how the subscription will get a const T& msg of the same msg 🤔