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

Enable TCP_NODELAY, alternative approach #184

Closed
wants to merge 1 commit into from

Conversation

edsko
Copy link
Collaborator

@edsko edsko commented Jul 9, 2024

This replaces #181, and depends on

Just to verify that we can set TCP_NODELAY both server-side and client-side, as well as with and without TLS, I did some measurements:

secure server TCP_NODELAY client TCP_NODELAY performance
no no no 11.717 RPCs/s
no no yes 22.517 RPCs/s
no yes no 21.683 RPCs/s
no yes yes 1735.617 RPCs/s ✔️
-- -- -- --
yes no no 11.417 RPCs/s
yes no yes 22.983 RPCs/s
yes yes no 21.383 RPCs/s
yes yes yes 973.000 RPCs/s ✔️

(all with --no-work-simulation)

@edsko edsko force-pushed the edsko/tcp-nodelay-alternative branch 6 times, most recently from 44b352e to 613ce07 Compare July 10, 2024 09:58
@edsko edsko mentioned this pull request Jul 10, 2024
@edsko edsko force-pushed the edsko/tcp-nodelay-alternative branch 2 times, most recently from b774fc2 to 574d8a2 Compare July 10, 2024 09:59
@edsko
Copy link
Collaborator Author

edsko commented Jul 10, 2024

@edsko
Copy link
Collaborator Author

edsko commented Jul 10, 2024

Verified that if we don't set TCP_NODELAY on the server socket, but do set it on the client socket (from accept), we still get the expected performance, both with and without TLS, confirming that the second commit on this PR does what it's intended to do.

Tests fail however, not yet sure why, seems related to #156 .

@edsko edsko force-pushed the edsko/tcp-nodelay-alternative branch from 4c1f20d to 456c4db Compare July 11, 2024 06:44
This also overrides the ping rate limit on server when requested.
@edsko edsko force-pushed the edsko/tcp-nodelay-alternative branch from 456c4db to d6825a7 Compare July 11, 2024 07:22
@edsko
Copy link
Collaborator Author

edsko commented Jul 17, 2024

Closing this in favour of #183 , which includes the changes here.

@edsko edsko closed this Jul 17, 2024
@edsko edsko deleted the edsko/tcp-nodelay-alternative branch July 17, 2024 12:06
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.

1 participant