Improve shm api #1950

yellowhatter · 2025-05-19T09:11:00Z

Series of SHM API improvements:

try_resize and try_relayout API to resize owned SHM buffer
improve owned and unowned shm buffer types isolation (now strictly follows str and String relation semantic)
introduce OwnedShmBuf trait to aggregate API related to owned shm buffer types
move some private impls from public API files
introduce alignment constants: ALIGN_1_BYTE .. ALIGN_8_BYTES to better express alignment in code
default Alignment is now ALIGN_1_BYTE (fixes the issue with user not able to allocate arbitrary-sized buffers on default-initialized POSIX SHM provider)
fix some tests
add tests for buffer resize

- improved relation between owned and borrowed shm buffers - add alignment constants - default alignment now is 1 - fix bug in shm and session tests

some test fixes

codecov · 2025-05-26T14:31:24Z

Codecov Report

Attention: Patch coverage is 55.55556% with 40 lines in your changes missing coverage. Please review.

Please upload report for BASE (main@e6579fe). Learn more about missing BASE report.

Files with missing lines	Patch %	Lines
commons/zenoh-shm/src/api/buffer/zshm.rs	36.36%	14 Missing ⚠️
commons/zenoh-shm/src/api/buffer/zshmmut.rs	23.52%	13 Missing ⚠️
commons/zenoh-shm/src/lib.rs	71.73%	13 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1950   +/-   ##
=======================================
  Coverage        ?   70.84%           
=======================================
  Files           ?      365           
  Lines           ?    61664           
  Branches        ?        0           
=======================================
  Hits            ?    43684           
  Misses          ?    17980           
  Partials        ?        0

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

YuanYuYuan · 2025-06-10T12:30:01Z

commons/zenoh-shm/src/api/buffer/zshm.rs

@@ -35,9 +38,19 @@ impl ShmBuf for ZShm {
    }
 }

+impl OwnedShmBuf for ZShm {
+    fn try_resize(&mut self, new_size: NonZeroUsize) -> Option<()> {


It's better to document its safety.

YuanYuYuan · 2025-06-10T12:30:30Z

commons/zenoh-shm/src/api/buffer/zshm.rs

+    }
+
+    fn try_relayout(&mut self, new_layout: MemoryLayout) -> Result<(), BufferRelayoutError> {
+        unsafe { self.0.try_relayout(new_layout) }


Please document the safety.

YuanYuYuan · 2025-06-10T12:31:35Z

commons/zenoh-shm/src/api/buffer/zshmmut.rs

-        Self(data)
+impl OwnedShmBuf for ZShmMut {
+    fn try_resize(&mut self, new_size: NonZeroUsize) -> Option<()> {
+        unsafe { self.0.try_resize(new_size) }


Please document the safety.

YuanYuYuan · 2025-06-10T12:31:41Z

commons/zenoh-shm/src/api/buffer/zshmmut.rs

-    fn eq(&self, other: &zshmmut) -> bool {
-        self.0 == other.0 .0
+    fn try_relayout(&mut self, new_layout: MemoryLayout) -> Result<(), BufferRelayoutError> {
+        unsafe { self.0.try_relayout(new_layout) }


Please document the safety.

YuanYuYuan · 2025-06-10T12:35:06Z

zenoh/tests/session.rs

@@ -84,7 +84,7 @@ async fn open_session_multicast(endpoint01: &str, endpoint02: &str) -> (Session,
        .endpoints
        .set(vec![endpoint01.parse().unwrap()])
        .unwrap();
-    config.scouting.multicast.set_enabled(Some(true)).unwrap();
+    config.scouting.multicast.set_enabled(Some(false)).unwrap();


Is it relevant to this PR?

No, this is small test bugfix - too small to deserve separate PR

YuanYuYuan · 2025-06-10T12:44:51Z

zenoh/tests/shm.rs

+    });
+}
+
+#[test]


Minor suggestion. We can use tokio::test macro as we do in other tests.

Nice catch! Fixed

YuanYuYuan · 2025-06-10T12:44:59Z

zenoh/tests/shm.rs

+    });
+}
+
+#[test]


Minor suggestion. We can use tokio::test macro as we do in other tests.

Nice catch! Fixed

YuanYuYuan · 2025-06-10T12:45:08Z

zenoh/tests/shm.rs

+    });
+}
+
+#[test]


Minor suggestion. We can use tokio::test macro as we do in other tests.

Nice catch! Fixed

YuanYuYuan · 2025-06-10T12:45:15Z

zenoh/tests/shm.rs

+    });
+}
+
+#[test]


Minor suggestion. We can use tokio::test macro as we do in other tests.

Nice catch! Fixed

YuanYuYuan · 2025-06-10T12:48:36Z

zenoh/tests/shm.rs


 const TIMEOUT: Duration = Duration::from_secs(60);
 const SLEEP: Duration = Duration::from_secs(1);

 const MSG_COUNT: usize = 1_00;
 const MSG_SIZE: [usize; 2] = [1_024, 100_000];

-async fn open_session_unicast(endpoints: &[&str]) -> (Session, Session) {
+async fn open_session_unicast<const NO_SHM_FOR_SECOND_PEER: bool>(


Just out of curiosity. Is there any reason to use a const generic rather than a plain bool argument?

Well, no reason, I just thought that this is more readable (nobody cares about endpoints list, but testing modes are more interesting)

yellowhatter added 3 commits May 14, 2025 13:55

- relayout and resize operation for shm buffers

b55194e

- improved relation between owned and borrowed shm buffers - add alignment constants - default alignment now is 1 - fix bug in shm and session tests

Merge commit 'b146561b80c93320a5a268a4657f2db5696b8874'

95a2552

Merge commit '63a6e33fe58eb98485c13dad0924f673baedc060'

3f8cd69

yellowhatter self-assigned this May 19, 2025

yellowhatter added enhancement Existing things could work better rust Pull requests that update rust code labels May 19, 2025

yellowhatter added 3 commits May 26, 2025 15:54

Merge commit 'c528884fd6f3ce73a8c11cdb9c596a5cbdaa378e'

157d078

code format

d76b640

some test fixes

Fix CI errors

f3d0b13

yellowhatter added 3 commits May 26, 2025 18:07

Add tests for shm and shm2nonshm

813cc0c

Add resize and relayout tests

230af13

Update shm.rs

8edd084

yellowhatter requested a review from YuanYuYuan May 26, 2025 17:00

yellowhatter marked this pull request as ready for review May 26, 2025 17:00

fix test ports

3bb7eca

yellowhatter changed the title ~~WIP: Improve shm api~~ Improve shm api May 27, 2025

YuanYuYuan requested changes Jun 10, 2025

View reviewed changes

yellowhatter added 3 commits June 10, 2025 17:04

Merge commit 'e6579fe8961c2ee919ceb3349dad3abe730798f3'

cc8e2f7

review fixes

7d1af91

Fix shm test cfg

385f116

yellowhatter requested a review from YuanYuYuan June 10, 2025 22:13

+                  });
+              }
+              #[test]

Improve shm api #1950

Are you sure you want to change the base?

Improve shm api #1950

Conversation

yellowhatter commented May 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented May 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

yellowhatter commented May 19, 2025 •

edited

Loading

codecov bot commented May 26, 2025 •

edited

Loading