From 3e67a88dd221c0f6b8277bb1f4301c417e30e5fa Mon Sep 17 00:00:00 2001 From: Denis Biryukov Date: Thu, 14 Nov 2024 19:46:31 +0100 Subject: [PATCH 1/2] z_bytes_to_slice and z_bytes_to_string no longer perform a copy if data is contiguous --- src/zbytes.rs | 35 +++++++++++++++++++++-------------- tests/z_api_payload_test.c | 2 +- 2 files changed, 22 insertions(+), 15 deletions(-) diff --git a/src/zbytes.rs b/src/zbytes.rs index 5cc2eb874..c85ab6de4 100644 --- a/src/zbytes.rs +++ b/src/zbytes.rs @@ -110,18 +110,8 @@ pub unsafe extern "C" fn z_bytes_to_string( this: &z_loaned_bytes_t, dst: &mut MaybeUninit, ) -> z_result_t { - let payload = this.as_rust_type_ref(); - match payload.try_to_string() { - Ok(s) => { - dst.as_rust_type_mut_uninit().write(s.into_owned().into()); - result::Z_OK - } - Err(e) => { - tracing::error!("Failed to convert the payload: {}", e); - dst.as_rust_type_mut_uninit().write(CStringOwned::default()); - result::Z_EINVAL - } - } + let dst_slice = std::mem::transmute(dst); + z_bytes_to_slice(this, dst_slice) } /// Converts data into an owned slice. @@ -135,8 +125,25 @@ pub unsafe extern "C" fn z_bytes_to_slice( dst: &mut MaybeUninit, ) -> z_result_t { let payload = this.as_rust_type_ref(); - dst.as_rust_type_mut_uninit() - .write(payload.to_bytes().into_owned().into()); + match payload.to_bytes() { + std::borrow::Cow::Borrowed(s) => { + let context = Box::leak(Box::new(payload.clone())) as *mut ZBytes as *mut _; + extern "C" fn __z_drop_bytes_inner(_data: *mut c_void, context: *mut c_void) { + let _ = unsafe { Box::from_raw(context as *mut ZBytes) }; + } + let cs = CSliceOwned::wrap( + s.as_ptr() as *mut _, + s.len(), + Some(__z_drop_bytes_inner), + context, + ) + .unwrap(); + dst.as_rust_type_mut_uninit().write(cs); + } + std::borrow::Cow::Owned(v) => { + dst.as_rust_type_mut_uninit().write(v.into()); + } + } result::Z_OK } diff --git a/tests/z_api_payload_test.c b/tests/z_api_payload_test.c index 745e49389..fbd8d48b7 100644 --- a/tests/z_api_payload_test.c +++ b/tests/z_api_payload_test.c @@ -158,10 +158,10 @@ void test_slice(void) { assert(cnt == 0); z_drop(z_move(payload)); - assert(cnt == 1); assert(!memcmp(data, z_slice_data(z_loan(out)), 10)); z_slice_drop(z_slice_move(&out)); + assert(cnt == 1); z_owned_bytes_t payload2; z_owned_slice_t s; From e95ff62d06317d27f468f4859b4ef56a6f776669 Mon Sep 17 00:00:00 2001 From: Denis Biryukov Date: Fri, 15 Nov 2024 11:50:44 +0100 Subject: [PATCH 2/2] enforce copy if data is in a shared memory buffer --- include/zenoh_commons.h | 4 ++-- src/zbytes.rs | 12 ++++++++++-- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/include/zenoh_commons.h b/include/zenoh_commons.h index 09d6b862a..2181ea0f5 100644 --- a/include/zenoh_commons.h +++ b/include/zenoh_commons.h @@ -1311,7 +1311,7 @@ z_result_t z_bytes_to_owned_shm(const struct z_loaned_bytes_t *this_, struct z_owned_shm_t *dst); #endif /** - * Converts data into an owned slice. + * @brief Gets data as an owned slice. * * @param this_: Data to convert. * @param dst: An uninitialized memory location where to construct a slice. @@ -1320,7 +1320,7 @@ ZENOHC_API z_result_t z_bytes_to_slice(const struct z_loaned_bytes_t *this_, struct z_owned_slice_t *dst); /** - * Converts data into an owned non-null-terminated string. + * Gets data as an owned non-null-terminated string. * * @param this_: Data to convert. * @param dst: An uninitialized memory location where to construct a string. diff --git a/src/zbytes.rs b/src/zbytes.rs index c85ab6de4..7d4b633a5 100644 --- a/src/zbytes.rs +++ b/src/zbytes.rs @@ -100,7 +100,7 @@ extern "C" fn z_bytes_len(this: &z_loaned_bytes_t) -> usize { this.as_rust_type_ref().len() } -/// Converts data into an owned non-null-terminated string. +/// Gets data as an owned non-null-terminated string. /// /// @param this_: Data to convert. /// @param dst: An uninitialized memory location where to construct a string. @@ -114,7 +114,7 @@ pub unsafe extern "C" fn z_bytes_to_string( z_bytes_to_slice(this, dst_slice) } -/// Converts data into an owned slice. +/// @brief Gets data as an owned slice. /// /// @param this_: Data to convert. /// @param dst: An uninitialized memory location where to construct a slice. @@ -127,6 +127,14 @@ pub unsafe extern "C" fn z_bytes_to_slice( let payload = this.as_rust_type_ref(); match payload.to_bytes() { 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() { + dst.as_rust_type_mut_uninit().write(s.to_owned().into()); + return result::Z_OK; + } + // otherwise we avoid a copy and instead just place zbytes clone into an owned_slice context to ensure + // that it will outlive the pointed data let context = Box::leak(Box::new(payload.clone())) as *mut ZBytes as *mut _; extern "C" fn __z_drop_bytes_inner(_data: *mut c_void, context: *mut c_void) { let _ = unsafe { Box::from_raw(context as *mut ZBytes) };