Skip to content

[10384] Adding Zero Copy feature to the Latency performance tests #1742

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
Merged
merged 17 commits into from
Feb 19, 2021
Merged

[10384] Adding Zero Copy feature to the Latency performance tests #1742

merged 17 commits into from
Feb 19, 2021

Conversation

MiguelBarro
Copy link
Contributor

This PR should be merged after PR #1706, #1730 and #1705.

@MiguelBarro MiguelBarro added no-test Skip CI tests if PR marked with this label skip-ci Automatically pass CI labels Feb 5, 2021
@MiguelBarro MiguelBarro changed the title Adding Zero Copy feature to the Latency performance tests [10507] Adding Zero Copy feature to the Latency performance tests Feb 5, 2021
@richiware
Copy link
Member

Build status:

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

@MiguelBarro MiguelBarro force-pushed the feature/performance_latency_zero_copy branch from da5d556 to b2d87e4 Compare February 8, 2021 10:36
IkerLuengo
IkerLuengo suggested changes Feb 8, 2021
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.

I know we have discussed it already, and we all though it was a good solution, but how fair is to discard the echo bounce time in the measurements? Isn't that part of the 'consequences' of using, for example, the loan API? If having to wait for a loan makes our overall latency worse, should we assume it on the measurements?

IkerLuengo
IkerLuengo previously approved these changes Feb 9, 2021
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

IkerLuengo
IkerLuengo previously approved these changes Feb 9, 2021
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

@EduPonz EduPonz removed no-test Skip CI tests if PR marked with this label skip-ci Automatically pass CI labels Feb 10, 2021
Miguel Barro added 16 commits February 10, 2021 10:23
Update the CMake script to include the new tests
2431aec
Disable data sharing by default in all tests
207a49d
Update the python script to handle the new data sharing and loan flags
76d2953
Add options to the binary
01ccaf8
check parameters validity
79a96ff
loan feature implementation
9a5609c
Reordered atomic operations and improved error handling
e0a6755
Linter sweep
a9b9465
Solving loan errors on shared memory testing
8c20291
Update python script csv filename generation
c905569
correcting extra copy time due to loans on echoing
224c00a
publisher loan failure is not a terminating error
ea51e0a
Adding extra payloads to loan testing via QoS. This solves loan failu…
df44388 
…res due to resource outage.
Update test cases according with team quorum
3c93c74
#1507 Adressing reviewers comments
f00ce66
#1507 Strictly following the demand sizes
04d04d4
@MiguelBarro MiguelBarro force-pushed the feature/performance_latency_zero_copy branch from adaf353 to 04d04d4 Compare February 10, 2021 09:31
EduPonz
EduPonz suggested changes Feb 12, 2021
View reviewed changes
Copy link

@EduPonz EduPonz left a comment

Choose a reason for hiding this comment

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

For the not dynamic case, the CSV and STDOUT tables show and incorrect size for bytes larger than what the user requested (68 instead of 64). I have not tested with dynamic data though

@richiprosima
Copy link
Contributor

Build status:

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

@MiguelBarro MiguelBarro changed the title [10507] Adding Zero Copy feature to the Latency performance tests [10384] Adding Zero Copy feature to the Latency performance tests Feb 15, 2021
IkerLuengo
IkerLuengo approved these changes Feb 19, 2021
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.

👍

@MiguelCompany MiguelCompany merged commit 8ff9e0d into master Feb 19, 2021
@MiguelCompany MiguelCompany deleted the feature/performance_latency_zero_copy branch February 19, 2021 07:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@EduPonz EduPonz EduPonz requested changes

@IkerLuengo IkerLuengo IkerLuengo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@MiguelBarro @richiware @richiprosima @EduPonz @IkerLuengo @MiguelCompany