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 by default #1020

Merged
merged 3 commits into from Jun 19, 2019

Conversation

Projects
None yet
2 participants
@weissi
Copy link
Member

commented May 30, 2019

Motivation:

Networking software like SwiftNIO that always has explicit flushes
usually does not benefit from having TCP_NODELAY switched off. The
benefits of having it turned on are usually quite substantial and yet we
forced our users for the longest time to enable it manually.

Quite a bit of engineering time has been lost finding performance
problems and it turns out switching TCP_NODELAY on solves them
magically.

Netty has made the switch to TCP_NODELAY on by default, SwiftNIO should
follow.

Modifications:

Enable TCP_NODELAY by default.

Result:

If the user forgot to enable TCP_NODELAY, their software should now be
faster.

@weissi weissi requested a review from Lukasa May 30, 2019

@weissi

This comment has been minimized.

Copy link
Member Author

commented May 30, 2019

@weissi weissi force-pushed the weissi:jw-enable-tcp-nodelay-by-default branch from e2915b1 to 2000e5f May 30, 2019

@Lukasa Lukasa added this to the 2.3.0 milestone May 30, 2019

@Lukasa

Lukasa approved these changes May 30, 2019

@Lukasa
Copy link
Contributor

left a comment

Regresses allocations due to reserveCapacity.

@weissi

This comment has been minimized.

Copy link
Member Author

commented May 30, 2019

Regresses allocations due to reserveCapacity.

unfortunately with and without :|. I need to look into what's going on there exactly

@Lukasa

This comment has been minimized.

Copy link
Contributor

commented May 30, 2019

Oh sure, I saw that you needed to do it. Just means that at the bare minimum we need to increase the threshold because the CI gets mad. 😉

Lukasa added a commit to Lukasa/swift-nio-transport-services that referenced this pull request May 30, 2019

Enable TCP_NODELAY by default.
Motivation:

Networking software like SwiftNIO that always has explicit flushes
usually does not benefit from having TCP_NODELAY switched off. The
benefits of having it turned on are usually quite substantial and yet we
forced our users for the longest time to enable it manually.

Quite a bit of engineering time has been lost finding performance
problems and it turns out switching TCP_NODELAY on solves them
magically.

Netty has made the switch to TCP_NODELAY on by default, SwiftNIO should
follow. This patch is the equivalent of apple/swift-nio#1020.

Modifications:

Enable TCP_NODELAY by default.

Result:

If the user forgot to enable TCP_NODELAY, their software should now be
faster.

@weissi weissi force-pushed the weissi:jw-enable-tcp-nodelay-by-default branch from 2000e5f to 433ff74 May 30, 2019

Lukasa added a commit to apple/swift-nio-transport-services that referenced this pull request Jun 18, 2019

Enable TCP_NODELAY by default. (#46)
Motivation:

Networking software like SwiftNIO that always has explicit flushes
usually does not benefit from having TCP_NODELAY switched off. The
benefits of having it turned on are usually quite substantial and yet we
forced our users for the longest time to enable it manually.

Quite a bit of engineering time has been lost finding performance
problems and it turns out switching TCP_NODELAY on solves them
magically.

Netty has made the switch to TCP_NODELAY on by default, SwiftNIO should
follow. This patch is the equivalent of apple/swift-nio#1020.

Modifications:

Enable TCP_NODELAY by default.

Result:

If the user forgot to enable TCP_NODELAY, their software should now be
faster.

@weissi weissi force-pushed the weissi:jw-enable-tcp-nodelay-by-default branch from 433ff74 to b6e1d1e Jun 19, 2019

@Lukasa Lukasa modified the milestones: 2.3.0, 2.4.0 Jun 19, 2019

enable TCP_NODELAY by default
Motivation:

Networking software like SwiftNIO that always has explicit flushes
usually does not benefit from having TCP_NODELAY switched off. The
benefits of having it turned on are usually quite substantial and yet we
forced our users for the longest time to enable it manually.

Quite a bit of engineering time has been lost finding performance
problems and it turns out switching TCP_NODELAY on solves them
magically.

Netty has made the switch to TCP_NODELAY on by default, SwiftNIO should
follow.

Modifications:

Enable TCP_NODELAY by default.

Result:

If the user forgot to enable TCP_NODELAY, their software should now be
faster.

@weissi weissi force-pushed the weissi:jw-enable-tcp-nodelay-by-default branch from b6e1d1e to e8f1211 Jun 19, 2019

@weissi weissi requested a review from Lukasa Jun 19, 2019

@weissi weissi modified the milestones: 2.4.0, 2.3.0 Jun 19, 2019

weissi and others added some commits Jun 19, 2019

@Lukasa

Lukasa approved these changes Jun 19, 2019

@Lukasa Lukasa merged commit 7f20464 into apple:master Jun 19, 2019

2 checks passed

pull request validation (5.0) Build finished.
Details
pull request validation (5.1) Build finished.
Details

pushkarnk added a commit to IBM-Swift/Kitura-NIO that referenced this pull request Jun 21, 2019

Remove setting of the TCP_NODELAY option, its now default
Setting TCP_NODELAY option that disables Nagles algorithm, thereby improving
throughput, has been made the default in NIO 2.3.1. We shouldn't repeat this in
Kitura-NIO. See apple/swift-nio#1020 for details.

pushkarnk added a commit to IBM-Swift/Kitura-NIO that referenced this pull request Jun 27, 2019

Remove setting of the TCP_NODELAY option, its now default (#209)
* Remove setting of the TCP_NODELAY option, its now default

Setting TCP_NODELAY option that disables Nagles algorithm, thereby improving
throughput, has been made the default in NIO 2.3.1. We shouldn't repeat this in
Kitura-NIO. See apple/swift-nio#1020 for details.

* Rename HTTPUpgradeConfiguration to NIOHTTPServerUpgradeConfiguration
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.