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

Remove refcounting from reply/sample. #440

Merged
merged 20 commits into from
Jul 2, 2024

Conversation

jean-roland
Copy link
Contributor

@jean-roland jean-roland commented Jun 25, 2024

Oudated.

_For Zenoh-C compatibility issue, we have a single z_loaned_sample_t type which corresponds both to a sample received by a sub and the sample contained in a reply.

This type is refcounted as required for potential out-of-context usage by a subscriber. As replies are themselves refcounted that create a double refcount which is slightly wasteful memory-wise in a Query/reply scenario.

This PR attemps to offer a way to avoid this double refcounting by storing the reply sample without refcounting, provide a zenoh pico api to interact with it, while a call to the Zenoh-C api will still generate the recounted sample._

@jean-roland
Copy link
Contributor Author

jean-roland commented Jun 25, 2024

Alternative solution after discussion with @sashacmc and @DenisBiryukov91: Make the allocated fields (payload, attachments...) refcounted and make the containers (reply, sample) copyable rather than refcounted.

To avoid doubling interfaces.

@jean-roland jean-roland force-pushed the ft_single_rc branch 9 times, most recently from 2508f63 to 0032525 Compare July 1, 2024 12:19
@jean-roland jean-roland changed the title Fix reply sample double refcounting. Remove refcounting from query/reply/sample. Jul 1, 2024
Copy link
Contributor

@DenisBiryukov91 DenisBiryukov91 left a comment

Choose a reason for hiding this comment

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

I just realized, that query likely should stay ref counted, because zenoh allows to make copies of queries and send multiple replies through them. The query is considered to be finalized when the last copy is dropped.

@jean-roland jean-roland changed the title Remove refcounting from query/reply/sample. Remove refcounting from reply/sample. Jul 2, 2024
@milyin milyin merged commit 27fb58e into eclipse-zenoh:dev/1.0.0 Jul 2, 2024
50 checks passed
@jean-roland jean-roland deleted the ft_single_rc branch July 3, 2024 08:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants