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

[ROS2] Use loaned messages to optimize the performance for image transport #216

Open
Barry-Xu-2018 opened this issue Nov 18, 2021 · 10 comments

Comments

@Barry-Xu-2018
Copy link

ROS2 supported zero copy via loaned messages. DDS (cyclonedds, fastrtps, etc) support zero copy.
Normally, the size of the image is big.
We can update code of image transport to use this feature. If transport on the same host, user can get huge performance improvement.

There is a comparison report.
https://discourse.ros.org/t/dds-implementation-performance-benchmark/19343

The design on zero coy via loaned message
https://design.ros2.org/articles/zero_copy.html

The example on how to use loaned message
https://github.com/ros2/demos/blob/7f8cd3aac2a179288bf02a412432f43c4d451355/demo_nodes_cpp/src/topics/talker_loaned_message.cpp#L66-L73

@clydemcqueen
Copy link

Image messages end with uint8[] data so they are not POD. Does the current design work with Image messages?

Related: #212

@Barry-Xu-2018
Copy link
Author

Barry-Xu-2018 commented Nov 19, 2021

Image messages end with uint8[] data so they are not POD. Does the current design work with Image messages?

Yes. Current implementation of zero copy requests POD message.
But sensor_msgs/msg/Image.msg isn't POD message.

If user knows the size of camera image, user can make updated image.msg (become POD message). And then can take advantage of zero copy.

@clydemcqueen
Copy link

clydemcqueen commented Nov 19, 2021

That works for raw images, but compressed sizes vary per image. And the plugin API would have to change.

An alternative is to use ros2 ipc and avoid the rmw layer. Please take a look at #215. The publisher plugin API changes, but not the subscriber plugin API.

[Edit: ignore this, my PR was flawed.]

@Barry-Xu-2018
Copy link
Author

Barry-Xu-2018 commented Nov 22, 2021

That works for raw images, but compressed sizes vary per image. And the plugin API would have to change.

Yes. For compressed image, we have to use maximum compressed size to make image.msg. This leads to waste RAM.
Cannot support the different image size by unique image.msg.

An alternative is to use ros2 ipc and avoid the rmw layer. Please take a look at #215. The publisher plugin API changes, but not the subscriber plugin API.

Yes. This is a feasible way to improve the performance on the same host. It's unsuitable for inter-process case.

@ZhenshengLee
Copy link

Yes. Current implementation of zero copy requests POD message. But sensor_msgs/msg/Image.msg isn't POD message.

If user knows the size of camera image, user can make updated image.msg (become POD message). And then can take advantage of zero copy.

@Barry-Xu-2018

Yes you are right about the current design of zero-copy transport in ROS2, and there is a repo that provide shm_msgs::msg::Image1m, Image2m, Image4m and more to support this.

Take a look at https://github.com/ZhenshengLee/ros2_shm_msgs , and you'll find it easy to use shm_msgs like cv_bridge

BTW, the zero-copy of compressed-image is not supported yet.

Thanks.

@ZhenshengLee
Copy link

There is an example of camera deriver, the custom version of https://github.com/ZhenshengLee/ros2_v4l2_camera that provide the zero-copy topic of image.

Ping me if you need any support to merge the ros2_shm_msgs feature into this repo.

Thanks.

@roncapat
Copy link

Well, we need intra_process_communication as minimum. #215 got self-closed due to having multiple plugins in need to get the same unique_ptr, but I think that even if copies are produced, bypassing DDS serialization is something that HAS to be done. I am just starting using image_transport, but I come from an extensive usage of composition and intra-process comms in ROS2, so I may try in future to figure out a patch.

@roncapat
Copy link

roncapat commented Mar 4, 2023

My considerations, for discussion:

  • image_transport (thus, compression) IMHO is needed only when transmitting between two machines (or communicating with python-based, ROS2 node);
  • in every other case, it is always sufficient (and efficient) to pass images (either as sensor_msgs/Image or custom rclcpp_adapter) using move semantic and rclcpp::Publisher;
  • this means that, at least in C++ world, image_transport plugin-based mechanism is not needed at API-level (publish/suscribe wrappers) but as a light component bridging a sensor_msgs/Image topic to a compressed one and vice-versa;
  • the system designer should then compose the bridge only when it is expected to stream specific topics across machines or process boundaries;
  • of course, not using image_transport in any component means that you lose the ability to subscribe (in a compressed way) to an image topic you didn't expected to subscribe to (externally from the container or from another machine), BUT you can still load inside a container a bridge component instance on-the-fly if you need it;

Feel free to comment...
My take here is that image_common may provide two composable nodes: from Image to CompressedImage and vice-versa. Any opinions?

Linked to #212.

@alexleel
Copy link

Can we see the performance comparison between zero copy and serialization communication by testing?

@ZhenshengLee
Copy link

ZhenshengLee commented Mar 29, 2023

@alexleel

Can we see the performance comparison between zero copy and serialization communication by testing?

if you need benchmarking test, check this https://github.com/ZhenshengLee/performance_test and https://github.com/ZhenshengLee/ros2_jetson_benchmarks

if you need onsite system test, check this https://github.com/ZhenshengLee/ros2_shm_msgs/tree/humble/src

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

5 participants