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

Share a custom PayloadPool between DDS and RTPS participants #70

Merged
merged 12 commits into from
Dec 21, 2023

Conversation

Tempate
Copy link
Contributor

@Tempate Tempate commented Oct 30, 2023

In the previous version, DDS and RTPS had two different Payload Pools, and the data forwarded through the DDS Router would get copied from one to the other. In this version, making use of Fast-DDS's recently added functionality to create DataWriters and DataReaders with a custom Payload Pool, it's become possible to share the same Payload Pool between DDS and RTPS participants and avoid the copies. This version also fixes the leaks that were taking place when using DDS participants.

In the following example, we launch a DDS Router with two DDS participants, and publish a hundred 1MB messages on a reliable-volatile topic. Here's the heap memory consumption of the DDS Router before and after this update.

Before (Peak Heap Memory 6.0 MiB):
dds_dds

After (Peak Heap Memory 5.0 MiB):
dds_dds

Merge after:

  • Fast-DDS 2.12.1

@Tempate Tempate force-pushed the feature/shared_payload_pool branch from e93e5ff to 1252aa2 Compare November 6, 2023 09:00
@Tempate Tempate force-pushed the feature/shared_payload_pool branch from 1252aa2 to cecc3dd Compare November 13, 2023 08:00
@Tempate
Copy link
Contributor Author

Tempate commented Nov 13, 2023

The tests are failing because it has to be run with Fast-CDR 1.1.x & Fast-DDS in master until commit b913df0a6. Merge when Fast-DDS 2.12.1 comes out.

Copy link
Contributor

@juanlofer-eprosima juanlofer-eprosima left a comment

Choose a reason for hiding this comment

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

LGTM

@codecov-commenter
Copy link

Codecov Report

Attention: 33 lines in your changes are missing coverage. Please review.

Comparison is base (5579d49) 36.91% compared to head (2d6d245) 36.76%.

Files Patch % Lines
...src/cpp/efficiency/payload/PayloadPoolMediator.cpp 17.39% 19 Missing ⚠️
...e_participants/src/cpp/types/dds/TopicDataType.cpp 0.00% 6 Missing ⚠️
...e_participants/src/cpp/reader/dds/CommonReader.cpp 25.00% 3 Missing ⚠️
...e_participants/src/cpp/writer/dds/CommonWriter.cpp 40.00% 2 Missing and 1 partial ⚠️
...ore/src/cpp/efficiency/payload/FastPayloadPool.cpp 33.33% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #70      +/-   ##
==========================================
- Coverage   36.91%   36.76%   -0.16%     
==========================================
  Files         115      116       +1     
  Lines        4946     4975      +29     
  Branches     1893     1905      +12     
==========================================
+ Hits         1826     1829       +3     
- Misses       2310     2332      +22     
- Partials      810      814       +4     

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

@Tempate Tempate changed the title Share a custom Payload Pool between Fast-DDS and Fast-RTPS Share a custom PayloadPool between DDS and RTPS participants Dec 21, 2023
@Tempate Tempate merged commit 20606d1 into main Dec 21, 2023
@Tempate Tempate deleted the feature/shared_payload_pool branch December 21, 2023 10:16
Tempate added a commit that referenced this pull request Mar 8, 2024
* Use the router's PayloadPool to avoid copies

Signed-off-by: tempate <[email protected]>

* Update TypeSupport to use the PayloadPool

Signed-off-by: tempate <[email protected]>

* Mediator PoC: no leaks and copies

Signed-off-by: Juan Lopez Fernandez <[email protected]>

* Fix memory leaks: XML participants without ignore_local_endpoints

Signed-off-by: tempate <[email protected]>

* Replace the PoC with the actual implementation

Signed-off-by: tempate <[email protected]>

* Destroy saved payloads after getting them

Signed-off-by: tempate <[email protected]>

* Destroy the discarded payloads

Signed-off-by: tempate <[email protected]>

* Uncrustify

Signed-off-by: tempate <[email protected]>

* Apply suggestions

Signed-off-by: tempate <[email protected]>

* Complete TODOs

Signed-off-by: tempate <[email protected]>

* Apply suggestions

Signed-off-by: tempate <[email protected]>

* Apply more suggestions

Signed-off-by: tempate <[email protected]>

---------

Signed-off-by: tempate <[email protected]>
Signed-off-by: Juan Lopez Fernandez <[email protected]>
Co-authored-by: Juan Lopez Fernandez <[email protected]>
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.

3 participants