SHM API improvement: ProtocolID and others... #1975

yellowhatter · 2025-06-02T15:18:24Z

Draft because it requires API alignment for C and C++ (to be done after we agree on this)

hide ProtocolID
simplify ShmProvider builder APIs
add minimal example for SHM
simplify some SHM examples using simplified SHM API

- simplify ShmProvider builder APIs

codecov · 2025-06-02T15:47:22Z

Codecov Report

Attention: Patch coverage is 49.29577% with 36 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/provider/types.rs	0.00%	22 Missing ⚠️
commons/zenoh-shm/src/api/provider/shm_provider.rs	65.85%	14 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1975   +/-   ##
=======================================
  Coverage        ?   70.86%           
=======================================
  Files           ?      365           
  Lines           ?    61617           
  Branches        ?        0           
=======================================
  Hits            ?    43666           
  Misses          ?    17951           
  Partials        ?        0

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

evshary

I'm happy that the ID disappeared and made the usage simpler 👍
I think we can polish the examples for better understanding.

evshary · 2025-06-04T05:34:18Z

examples/examples/z_queryable_shm.rs

-        .protocol_id::<POSIX_PROTOCOL_ID>()
-        .backend(backend)
-        .wait();
+    let provider = ShmProviderBuilder::backend(backend).wait();


Why can't we use let provider = ShmProviderBuilder::default_backend().wait().unwrap(); in this example?

...and provide the size.

evshary · 2025-06-04T05:51:51Z

examples/examples/z_alloc_shm.rs

+    run().await.unwrap();
+}
+
+async fn run_minimal() -> zenoh::Result<()> {


In fact, I'm not pretty sure if we want this. The basic usage should be already in other examples.

removed minimal

evshary · 2025-06-04T06:04:34Z

examples/examples/z_alloc_shm.rs

It took me some time to understand what the example was trying to convey. I suggest adding the description in the example README. (z_alloc_shm.rs is missing there)

added readme

evshary · 2025-06-04T06:13:59Z

examples/examples/z_alloc_shm.rs

I'm new to the Zenoh shared memory usage. While we provide two ways to allocate SHM (direct allocation and layout allocation), I would suggest using a block to isolate each way to avoid confusion.

// Option 1: let _direct_allocation = { ... }; // Option 2: let _layout_allocation = { let buffer_layout = {...}; let sbuf = {...}; sbuf };

I'm just taking an example. You could choose a better variable name.

Reorganized options in this example

evshary · 2025-06-04T06:19:44Z

commons/zenoh-shm/src/api/protocol_implementations/posix/posix_shm_client.rs

 };

 /// Client factory implementation for particular shared memory protocol
 #[zenoh_macros::unstable_doc]
 #[derive(Debug)]
 pub struct PosixShmClient;

+impl WithProtocolID for PosixShmClient {
+    fn id(&self) -> ProtocolID {
+        POSIX_PROTOCOL_ID


Just out of curiosity, do we plan to provide more ways to allocate SHM? I only see POSIX_PROTOCOL_ID now, so I'm not sure why we need to specify the ID.

There are reasons to keep the ID:

Some day we can implement some specific shared-memory backends like ones backed by SystemV shared memory or some device vendor things like CUDA shared-memory.

ShmProviderBackend trait is made public so that any user can implement specific backend.

IDs + file access mode flags can be used to artificially split SHM domains.

SHM is architectured to be extremely flexible. You might find some things excessive, for example all that stuff with Layout and Alignment - and it was made to smoothly support allocator API (like C++ STL-style allocators) and more specific stuff for GPU. Now I'm polishing it to make things easier for basic usage and hide too much specific things yet leaving them accessible for advanced use case.

I believe CUDA shared-memory is something useful. Make sense in this case.

kydos · 2025-06-06T15:16:38Z

examples/examples/z_alloc_shm.rs

+            .unwrap();
+        // ...and an SHM provider
+        ShmProviderBuilder::backend(comprehencive).wait()
+    };


I think we should define the alignment for the provider and completely remove the layout type.

kydos · 2025-06-06T15:16:55Z

examples/examples/z_alloc_shm.rs

+        // OPTION: comprehensive ShmProvider creation
+        // NOTE: For extended PosixShmProviderBackend API please check z_posix_shm_provider.rs
+        // create backend...
+        let comprehencive = PosixShmProviderBackend::builder()


s/comprehencive/comprehensive

kydos · 2025-06-06T15:17:59Z

examples/examples/z_bytes_shm.rs

-        .protocol_id::<POSIX_PROTOCOL_ID>()
-        .backend(backend)
-        .wait();
+    let provider = ShmProviderBuilder::backend(backend).wait();


For POSIX SHM it would be nice to be able to combine the two steps into one.

Yes, it is possible to do like this:

let provider = ShmProviderBuilder::default_backend(16 * 1024 * 1024).wait()?; let mut shm_buffer = provider.alloc(512).wait()?;

No layouts by default, nothing, just pure sizes.

However, I want to keep the layout API both for provider and allocations - just hide it behind streamlined defaults. The rationale to do that is:

allocator APIs

the integration of GPU and FPGA devices into our shared-memory - there you might easily have 1024 byte alignment for certain cases and sometimes operate with different differently-aligned arrays of data within the same project - that's where the layout API will have it's use.

custom provider backends for specific needs

So yes, I already take measures to hide layouts away from general use-case, and I vote to keep it for advanced scenario.

kydos · 2025-06-06T15:19:14Z

examples/examples/z_queryable_shm.rs

-        .protocol_id::<POSIX_PROTOCOL_ID>()
-        .backend(backend)
-        .wait();
+    let provider = ShmProviderBuilder::backend(backend).wait();


...and provide the size.

kydos · 2025-06-06T15:22:44Z

zenoh/tests/bytes.rs

-        .protocol_id::<POSIX_PROTOCOL_ID>()
-        .backend(backend)
-        .wait();
+    let provider = ShmProviderBuilder::backend(backend).wait();

    // Prepare a layout for allocations
    let layout = provider.alloc(1024).into_layout().unwrap();


The layout has confused many users. I suggest we fix the layout at the creation of the provider and then only expose a simple alloc. This would make it more user friendly and will streamline the implementation of the SHM. I don't think we should allow to allocate with different alignment within the same provider.

Pls check my answer above

- polish examples

Conflicts: commons/zenoh-shm/src/api/client_storage/mod.rs commons/zenoh-shm/src/api/protocol_implementations/posix/posix_shm_provider_backend.rs commons/zenoh-shm/src/api/provider/shm_provider.rs

- hide ProtocolID

d7954fa

- simplify ShmProvider builder APIs

yellowhatter requested a review from evshary June 2, 2025 15:18

yellowhatter self-assigned this Jun 2, 2025

yellowhatter added enhancement Existing things could work better new feature Something new is needed breaking-change Indicates that the issue implies a breaking change (be it at compile time or at runtime) api fix Correct API labels Jun 2, 2025

simplify SHM examples using new simplified API

8973e88

evshary reviewed Jun 4, 2025

View reviewed changes

kydos reviewed Jun 6, 2025

View reviewed changes

yellowhatter added 4 commits June 9, 2025 11:49

- add size to default backend

2cb05d5

- polish examples

Polish z_alloc_shm.rs and other SHM examples

3ef1aa6

Update README.md

e3c1f5b

Update README.md

f5942f6

yellowhatter marked this pull request as ready for review June 10, 2025 12:24

yellowhatter added 2 commits June 10, 2025 15:40

Merge commit 'e6579fe8961c2ee919ceb3349dad3abe730798f3'

6a3fa1d

Conflicts: commons/zenoh-shm/src/api/client_storage/mod.rs commons/zenoh-shm/src/api/protocol_implementations/posix/posix_shm_provider_backend.rs commons/zenoh-shm/src/api/provider/shm_provider.rs

Some fixes after merge

62272a4

yellowhatter requested review from evshary and kydos June 10, 2025 14:43

evshary approved these changes Jun 11, 2025

View reviewed changes

SHM API improvement: ProtocolID and others... #1975

Are you sure you want to change the base?

SHM API improvement: ProtocolID and others... #1975

Uh oh!

Conversation

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

Uh oh!

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

evshary left a comment

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!

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

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!

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

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 Jun 2, 2025 •

edited

Loading

codecov bot commented Jun 2, 2025 •

edited

Loading

yellowhatter Jun 5, 2025 •

edited

Loading

yellowhatter Jun 6, 2025 •

edited

Loading