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

[20291] Gather-send implementation #4537

Merged
merged 52 commits into from
Jun 13, 2024
Merged

Conversation

cferreiragonz
Copy link
Contributor

@cferreiragonz cferreiragonz commented Mar 8, 2024

Description

This PR refactors the creation and sending of RTPS Messages by utilizing a list of NetworkBuffers (pointers + sizes) as the data transmission object, thereby eliminating an additional data message copy in the RTPSMessageGroup.
This refactor impacts both the Transports and the RTPS Layer, involving updates to the sender resources lambda functions, transport interfaces, statistics module, and the logic implemented in RTPS message creation.

Two extra ResourceLimitedVectors have been added to RTPSMessageGroup_t to act as NetworkBuffers and to contain the metadata of the payloads used in the creation of the RTPS message. In this way mallocs can be reduced as much as possible during runtime. TCP transport has mallocs during the creation of the TCP message and during the asio write operation.

SendBuffersAllocationAttributes has been extended with a new attribute defining the allocation configuration of the NetworkBuffers.

Opened as draft:

  • Refactor at Transport level
  • Refactor at Sender Resource level
  • Refactor at Participant level
  • Statistics
  • Redesign on how secure messages are managed
  • Doxygen
  • Tests. They have been updated with new API. Gather-send is implicitly tested in every other send.
  • Documentation

Contributor Checklist

  • Commit messages follow the project guidelines.
  • The code follows the style guidelines of this project.
  • [N/A] Tests that thoroughly check the new feature have been added/Regression tests checking the bug and its fix have been added; the added tests pass locally
  • Any new/modified methods have been properly documented using Doxygen.
  • ❌ Changes are ABI compatible.
  • ❌ Changes are API compatible.
  • New feature has been added to the versions.md file (if applicable).
  • New feature has been documented/Current behavior is correctly described in the documentation.
    Related documentation PR: [20291] Gather-send implementation Docs Fast-DDS-docs#724
  • [N/A] Applicable backports have been included in the description.

Reviewer Checklist

  • The PR has a milestone assigned.
  • The title and description correctly express the PR's purpose.
  • Check contributor checklist is correct.
  • Check CI results: changes do not issue any warning.
  • Check CI results: failing tests are unrelated with the changes.

@cferreiragonz cferreiragonz added this to the v3.0.0 milestone Mar 11, 2024
@cferreiragonz cferreiragonz force-pushed the poc/gather-send-implementation branch 2 times, most recently from aa6e6cc to 5de9ef4 Compare March 11, 2024 14:32
@cferreiragonz cferreiragonz marked this pull request as ready for review March 11, 2024 14:34
@cferreiragonz cferreiragonz force-pushed the poc/gather-send-implementation branch 2 times, most recently from 376e650 to 261c9a0 Compare March 12, 2024 09:17
@cferreiragonz cferreiragonz changed the base branch from master to 3.0.x-devel March 12, 2024 10:17
@cferreiragonz cferreiragonz added the needs-review PR that is ready to be reviewed label Mar 12, 2024
@cferreiragonz cferreiragonz force-pushed the poc/gather-send-implementation branch 3 times, most recently from 86c7d43 to c3741e5 Compare March 14, 2024 06:59
@EduPonz EduPonz force-pushed the 3.0.x-devel branch 2 times, most recently from 8193fef to a364ed8 Compare March 27, 2024 14:54
@EduPonz EduPonz force-pushed the 3.0.x-devel branch 3 times, most recently from 5148f5d to bd83bb2 Compare April 23, 2024 05:52
Signed-off-by: cferreiragonz <[email protected]>
Signed-off-by: cferreiragonz <[email protected]>
Signed-off-by: cferreiragonz <[email protected]>
Signed-off-by: cferreiragonz <[email protected]>
@cferreiragonz cferreiragonz force-pushed the poc/gather-send-implementation branch from 394adb8 to 8a9eede Compare June 13, 2024 05:58
@MiguelCompany MiguelCompany self-requested a review June 13, 2024 05:59
Copy link
Member

@MiguelCompany MiguelCompany left a comment

Choose a reason for hiding this comment

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

A nit that does not require to pass CI again

test/blackbox/common/DDSBlackboxTestsBasic.cpp Outdated Show resolved Hide resolved
@MiguelCompany
Copy link
Member

@richiprosima Please test_3 discovery-server

Signed-off-by: cferreiragonz <[email protected]>
@MiguelCompany MiguelCompany merged commit 371286c into master Jun 13, 2024
3 checks passed
@MiguelCompany MiguelCompany deleted the poc/gather-send-implementation branch June 13, 2024 12:23
JesusPoderoso pushed a commit that referenced this pull request Jun 13, 2024
* Refs #20337: Buffer list in UDP sending function

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

* Refs #20337: Buffer list in SHM sending function & Copy to shared buffer function

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

* Refs #20337: Buffer list in TCP sending function

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

* Refs #20342: Add new Buffer structure

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

* Refs #20342: Add new lambda to send buffers into SenderResources

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

* Refs #20342: Refactor on UDP transport

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

* Refs #20342: Refactor on UDP test_transport

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

* Refs #20342: Refactor on TCP Transport

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

* Refs #20342: Refactor on TCPChannelResource

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

* Refs #20342: Refactor on TCP tests

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

* Refs #20342: Refactor on SHM transport & enable copying multiple buffers

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

* Refs #20342: Refactor on SHM tests

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

* Refs #20342: Fix mock tests after rebase

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

* Refs #20352: Refactor on addSubmessageData/DataFrag

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

* Refs #20352: Add constructor overloads to NetworkBuffers

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

* Refs #20352: Add new attributes and methods to RTPSMessageGroup.h

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

* Refs #20352: Refactor on add_data()

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

* Refs #20352: Refactor on add_data_frag()

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

* Refs #20352: Refactor on insert_submessage()

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

* Refs #20352: Refactor on send()

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

* Refs #20352: Minor changes in RTPSMessageGroup

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

* Refs #20352: Refactor on RTPSMessageSenderInterface

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

* Refs #20352: Refactor on ChaningTransport and ABI compatible send_lambda_

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

* Refs #20352: Refactor on Statistics module

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

* Refs #20352: Add security support

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

* Refs #20352: Minor fixes

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

* Refs #20352: Doxygen

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

* Refs #20352: Delete Sender's Resource deprecated API

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

* Refs #20352: Uncrustify

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

* Refs #20352: Fix Windows build

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

* Refs #20352: Account for change of namespaces

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

* Refs #20352: Add NetworkBuffer.cpp

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

* Refs #20352: Revision minor changes

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

* Refs #20352: Revision minor changes 2

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

* Refs #20291: Avoid stats_msg dynamic malloc

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

* Refs #20291: Use vector instead of list

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

* Refs #20291: Refactor RTPSMessageGroup to avoid Mallocs

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

* Refs #20291: Use limited vector to avoid repeated mallocs

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

* Refs #20291: Fix rebase

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

* Refs #20291: Improve doxygen

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

* Refs #20291: Revision

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

* Refs #20291: Add ResourceLV config into QoS

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

* Refs #20291: XML - New QoS added

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

* Refs #20291: Get Payload in RTPSMessageGroup

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

* Refs #20291: Uncrustify

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

* Refs #20291: Revision - Use RLContainerConfig and minor changes

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

* Refs #20291: Revision - Get payload after correct RTPSMsg creation

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

* Refs #20291: Revision - Default values

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

* Refs #20291: Update versions.md & CMakeLists

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

* Refs #20291: Revision - Headers & versions.md

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

* Refs #20291: Adjust payload_pool test

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

* Refs #20291: Test comment

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

---------

Signed-off-by: cferreiragonz <[email protected]>
@elianalf elianalf mentioned this pull request Jun 14, 2024
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci-pending PR which CI is running
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants