gazebosim / ros_gz

Integration between ROS (1 and 2) and Gazebo simulation
https://gazebosim.org
Apache License 2.0
233 stars 135 forks source link

Use memcpy instead of std::copy when bridging images #565

Closed caguero closed 3 months ago

caguero commented 3 months ago

🦟 Optimization

Summary

While testing ros <-> gz communication using the bridge I noticed that the bridge was talking quite a bit of time copying images from Gazebo to ROS. I found that the std::copy operation that we're doing is substantially slower than the memcpy alternative. I think that in principle this shouldn't happen but the numbers are quite clear. Perhaps std::copy is doing something that doesn't use cache effectively...

How to test it?

First, modify this code to see some stats:

auto beg = std::chrono::high_resolution_clock::now();
// Option 1: Using memcpy
// memcpy(ros_msg.data.data(), gz_msg.data().c_str(), gz_msg.data().size());

// Option 2: Using std::copy
auto count = ros_msg.step * ros_msg.height;
std::copy(
  gz_msg.data().begin(),
  gz_msg.data().begin() + count,
  ros_msg.data.begin());

auto end = std::chrono::high_resolution_clock::now();
auto duration = std::chrono::duration_cast<std::chrono::microseconds>(end - beg);
std::cerr << "Elapsed Time: " << duration.count() << " us" << std::endl; 

Recompile and launch one of our examples that publish 320x240 images:

ros2 launch ros_gz_sim_demos gpu_lidar_bridge.launch.py

The default code shows:

[parameter_bridge-2] Elapsed Time: 548 us

Enable the memcpy, comment the old code and relaunch the example again. The new code shows:

[parameter_bridge-2] Elapsed Time: 11 us

Checklist

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

traversaro commented 3 months ago

Not blocking, but a comment on why std::copy may be slow: perhaps the type being copied is not trivially copiable? In theory this can be tested via a static_assert with std::is_trivially_copyable (that in theory may be a good thing to check even before using memcpy).

azeey commented 3 months ago

CI seems to be broken on the ros2 branch. I'll be taking a look tomorrow.

ahcorde commented 2 months ago

https://github.com/Mergifyio backport jazzy

mergify[bot] commented 2 months ago

backport jazzy

✅ Backports have been created

* [#585 Use memcpy instead of std::copy when bridging images (backport #565)](https://github.com/gazebosim/ros_gz/pull/585) has been created for branch `jazzy`