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.

Sign up for GitHub

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

Jump to bottom

RTPSMessageGroup_t preallocation <1.9.x> [7165] #938

Merged
merged 20 commits into from
Jan 27, 2020
Merged

RTPSMessageGroup_t preallocation <1.9.x> [7165] #938

merged 20 commits into from
Jan 27, 2020

Conversation

MiguelCompany
Copy link
Member

This PR:

  • Adds a SendBuffersManager class, responsible of RTPSMessageGroup_t allocation
  • Adds a send_buffers field to RTPSParticipantAllocationAttributes with the allocation options for the pool of RTPSMessageGroup_t instances
  • Adds the corresponding XML parsing and test

@MiguelCompany
Refs #7116. Separating RTPSMessageGroup_t into a private header.
cc15eed
@MiguelCompany
Refs #7116. Passing send buffers management responsibility to RTPSPar…
c2517b7 
…ticipantImpl.
@MiguelCompany
Refs #7116. RAII on RTPSMessageGroup_t.
0671cfa
@MiguelCompany
Refs #7116. Protecting access to buffer pool.
991af62
@MiguelCompany
Refs #7116. Preallocating one buffer per expected thread.
21a3fb0
@MiguelCompany
Refs #7116. Protecting access to buffer pool with specific mutex.
c39a616
@MiguelCompany
Refs #7116. New SendBuffersManager class.
9526605
@MiguelCompany
Refs #7116. Allowing wrapping buffer on RTPSMessageGroup_t.
e3a9583
@MiguelCompany
Refs #7116. Using a single allocation for all initially allocated buf…
878109c 
…fers.
@MiguelCompany
Refs #7116. Adding assertion.
efd27d4
@MiguelCompany
Refs #7116. Fixed alignment.
2433ef1
@MiguelCompany
Refs #7116. Correct calculation of raw buffer size.
32fd0c5
@MiguelCompany
Refs #7116. Early return on security initialization failure.
9f67eb8
@MiguelCompany
Refs #7116. Making send buffers allocation configurable.
41d0905
@MiguelCompany
Refs #7116. Adding XML configuration for send_buffers allocation.
d691493
@MiguelCompany
Refs #7116. Adding participant allocation attributes to XML parser un…
a16a233 
…it test.
@MiguelCompany MiguelCompany added this to the v1.9.4 milestone Jan 3, 2020
@richiware
Copy link
Member

Build status:

  • Linux Build Status
  • Mac Build Status
  • Windows Build Status

@richiware
Copy link
Member

Build status:

  • Linux Build Status
  • Mac Build Status
  • Windows Build Status

@richiware
Copy link
Member

Build status:

  • Linux Build Status
  • Mac Build Status
  • Windows Build Status

@MiguelCompany MiguelCompany modified the milestones: v1.9.4, v1.9.5 Jan 13, 2020
@MiguelCompany MiguelCompany changed the title [1.9.x] RTPSMessageGroup_t preallocation [7165] RTPSMessageGroup_t preallocation <1.9.x> [7165] Jan 23, 2020
@richiware
Copy link
Member

Build status:

  • Linux Build Status
  • Mac Build Status
  • Windows Build Status

IkerLuengo
IkerLuengo approved these changes Jan 27, 2020
View reviewed changes
Copy link
Contributor

@IkerLuengo IkerLuengo left a comment

Choose a reason for hiding this comment

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

LGTM

@MiguelCompany MiguelCompany merged commit bf5877c into 1.9.x Jan 27, 2020
@MiguelCompany MiguelCompany deleted the feature/message-groups-allocation/1.9.x branch January 27, 2020 07:21
@fmorgner
Copy link

fmorgner commented Jan 27, 2020 •
edited
Loading

Sorry for commenting after the merge but 764718d breaks compilation when security is disabled. The dtor of RTPSMessageGroup always accesses the member participant_ which is only declared when HAVE_SECURITY is true.

dtor of RTPSMessageGroup:
https://github.com/eProsima/Fast-RTPS/blob/bf5877c42888de4bae0382b6c067efb495df1f5f/src/cpp/rtps/messages/RTPSMessageGroup.cpp#L137-L150

declaration of participant_
https://github.com/eProsima/Fast-RTPS/blob/bf5877c42888de4bae0382b6c067efb495df1f5f/include/fastrtps/rtps/messages/RTPSMessageGroup.h#L225-L229

@MiguelCompany MiguelCompany mentioned this pull request Jan 27, 2020
Merged
@MiguelCompany
Copy link
Member Author

@fmorgner Thanks for the catch! Fixed by #979

@IkerLuengo IkerLuengo mentioned this pull request Feb 26, 2020
Merged
MiguelCompany pushed a commit that referenced this pull request Feb 28, 2020
@IkerLuengo @MiguelCompany
Refs #7166 RTPSMessageGroup_t preallocation
3125b88 
This is a port of #938 and #979 and 3778a97 from 1.9.X

* Refs #7116. Separating RTPSMessageGroup_t into a private header.

* Refs #7116. Passing send buffers management responsibility to RTPSParticipantImpl.

* Refs #7116. RAII on RTPSMessageGroup_t.

* Refs #7116. Protecting access to buffer pool.

* Refs #7116. Preallocating one buffer per expected thread.

* Refs #7116. Protecting access to buffer pool with specific mutex.

* Refs #7116. New SendBuffersManager class.

* Refs #7116. Allowing wrapping buffer on RTPSMessageGroup_t.

* Refs #7116. Using a single allocation for all initially allocated buffers.

* Refs #7116. Adding assertion.

* Refs #7116. Fixed alignment.

* Refs #7116. Correct calculation of raw buffer size.

* Refs #7116. Early return on security initialization failure.

* Refs #7116. Making send buffers allocation configurable.

* Refs #7116. Adding XML configuration for send_buffers allocation.

* Refs #7116. Adding participant allocation attributes to XML parser unit test.

* Refs #7165. Fixed warnings.

* Refs #7165. Fixed leak.

* Refs #7165. Return send buffer even if an exception is thrown.

* Refs #7165. Correctly return send buffer.

* Fixed build without security.
MiguelCompany added a commit that referenced this pull request Feb 28, 2020
@IkerLuengo @MiguelCompany
RTPSMessageGroup_t preallocation (#1032)
9b7f143 
* Refs #7166 RTPSMessageGroup_t preallocation

This is a port of #938 and #979 and 3778a97 from 1.9.X

* Refs #7116. Separating RTPSMessageGroup_t into a private header.

* Refs #7116. Passing send buffers management responsibility to RTPSParticipantImpl.

* Refs #7116. RAII on RTPSMessageGroup_t.

* Refs #7116. Protecting access to buffer pool.

* Refs #7116. Preallocating one buffer per expected thread.

* Refs #7116. Protecting access to buffer pool with specific mutex.

* Refs #7116. New SendBuffersManager class.

* Refs #7116. Allowing wrapping buffer on RTPSMessageGroup_t.

* Refs #7116. Using a single allocation for all initially allocated buffers.

* Refs #7116. Adding assertion.

* Refs #7116. Fixed alignment.

* Refs #7116. Correct calculation of raw buffer size.

* Refs #7116. Early return on security initialization failure.

* Refs #7116. Making send buffers allocation configurable.

* Refs #7116. Adding XML configuration for send_buffers allocation.

* Refs #7116. Adding participant allocation attributes to XML parser unit test.

* Refs #7165. Fixed warnings.

* Refs #7165. Fixed leak.

* Refs #7165. Return send buffer even if an exception is thrown.

* Refs #7165. Correctly return send buffer.

* Fixed build without security.

* Refs #7166. Apply suggestions from code review

Co-authored-by: Miguel Company <miguelcompany@eprosima.com>
@MiguelCompany MiguelCompany mentioned this pull request Jul 30, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@IkerLuengo IkerLuengo IkerLuengo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v1.10.0
Development

Successfully merging this pull request may close these issues.
4 participants
@MiguelCompany @richiware @fmorgner @IkerLuengo