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

make z_bytes_to_slice and z_bytes_to_string avoid making copy for non-fragmented non-shm data #822

Conversation

DenisBiryukov91
Copy link
Contributor

make z_bytes_to_slice and z_bytes_to_string avoid making copy for non-fragmented non-sim data

@DenisBiryukov91 DenisBiryukov91 added the enhancement New feature or request label Nov 15, 2024
@DenisBiryukov91 DenisBiryukov91 changed the title make z_bytes_to_slice and z_bytes_to_string avoid making copy for non-fragmented non-sim data make z_bytes_to_slice and z_bytes_to_string avoid making copy for non-fragmented non-shm data Nov 15, 2024
@fuzzypixelz
Copy link
Member

The iRobot benchmark shows (roughly) a 40% decrease in the latency of the tagus topic (modified to be 1 MB instead of 250KB) on the mandalay, ponce and geneva nodes with the Mont Blanc topology using ZettaScaleLabs/rmw_zenoh.

I used an Intel® NUC 12 Pro NUC12WSHi5 (12th Gen Intel(R) Core(TM) i5-1240P, 16GiB RAM) running Ubuntu 24.04.1 LTS and ROS2 rolling.

std::borrow::Cow::Borrowed(s) => {
// force a copy if zbytes is a single shared memory slice
#[cfg(all(feature = "unstable", feature = "shared-memory"))]
if payload.as_shm().is_some() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a problem: if compiled as not "unstable" and with "shared-memory" zenoh will still be able to receive SHM buffers, but this check won't be performed. Thus we will have further code possibly executed on SHM buffer.....

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

which is probably fine today, since there is no way to mutate shm buffers without unstable - so getting an increased ref count on shm-slice should not be an issue in this context, right ?

@DenisBiryukov91 DenisBiryukov91 deleted the z_bytes_to_slice_string_without_copy branch December 16, 2024 14:14
@DenisBiryukov91 DenisBiryukov91 restored the z_bytes_to_slice_string_without_copy branch December 17, 2024 09:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants