enable TCP_NODELAY by default (#1020)

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.
apple · Jun 19, 2019 · 7f20464 · 7f20464
1 parent 1549cd7
commit 7f20464
Show file tree

Hide file tree

Showing 15 changed files with 57 additions and 29 deletions.
diff --git a/IntegrationTests/tests_04_performance/test_01_resources/shared.swift b/IntegrationTests/tests_04_performance/test_01_resources/shared.swift
@@ -99,7 +99,6 @@ private final class SimpleHTTPServer: ChannelInboundHandler {
 func doRequests(group: EventLoopGroup, number numberOfRequests: Int) throws -> Int {
     let serverChannel = try ServerBootstrap(group: group)
         .serverChannelOption(ChannelOptions.socket(SocketOptionLevel(SOL_SOCKET), SO_REUSEADDR), value: 1)
-        .childChannelOption(ChannelOptions.socket(IPPROTO_TCP, TCP_NODELAY), value: 1)
         .childChannelInitializer { channel in
             channel.pipeline.configureHTTPServerPipeline(withPipeliningAssistance: true,
                                                          withErrorHandling: false).flatMap {
@@ -120,7 +119,6 @@ func doRequests(group: EventLoopGroup, number numberOfRequests: Int) throws -> I
                 channel.pipeline.addHandler(repeatedRequestsHandler)
             }
         }
-        .channelOption(ChannelOptions.socket(IPPROTO_TCP, TCP_NODELAY), value: 1)
         .connect(to: serverChannel.localAddress!)
         .wait()
 

diff --git a/...grationTests/tests_04_performance/test_01_resources/test_ping_pong_1000_reqs_1_conn.swift b/...grationTests/tests_04_performance/test_01_resources/test_ping_pong_1000_reqs_1_conn.swift
@@ -112,7 +112,6 @@ private final class PongHandler: ChannelInboundHandler {
 func doPingPongRequests(group: EventLoopGroup, number numberOfRequests: Int) throws -> Int {
     let serverChannel = try ServerBootstrap(group: group)
         .serverChannelOption(ChannelOptions.socket(SocketOptionLevel(SOL_SOCKET), SO_REUSEADDR), value: 1)
-        .childChannelOption(ChannelOptions.socket(IPPROTO_TCP, TCP_NODELAY), value: 1)
         .childChannelOption(ChannelOptions.recvAllocator, value: FixedSizeRecvByteBufferAllocator(capacity: 4))
         .childChannelInitializer { channel in
             channel.pipeline.addHandler(ByteToMessageHandler(PongDecoder())).flatMap {
@@ -129,7 +128,6 @@ func doPingPongRequests(group: EventLoopGroup, number numberOfRequests: Int) thr
         .channelInitializer { channel in
             channel.pipeline.addHandler(pingHandler)
         }
-        .channelOption(ChannelOptions.socket(IPPROTO_TCP, TCP_NODELAY), value: 1)
         .channelOption(ChannelOptions.recvAllocator, value: FixedSizeRecvByteBufferAllocator(capacity: 4))
         .connect(to: serverChannel.localAddress!)
         .wait()

diff --git a/Sources/NIO/Bootstrap.swift b/Sources/NIO/Bootstrap.swift
@@ -36,8 +36,7 @@
 ///             }
 ///         }
 ///
-///         // Enable TCP_NODELAY and SO_REUSEADDR for the accepted Channels
-///         .childChannelOption(ChannelOptions.socket(IPPROTO_TCP, TCP_NODELAY), value: 1)
+///         // Enable SO_REUSEADDR for the accepted Channels
 ///         .childChannelOption(ChannelOptions.socket(SocketOptionLevel(SOL_SOCKET), SO_REUSEADDR), value: 1)
 ///         .childChannelOption(ChannelOptions.maxMessagesPerRead, value: 16)
 ///         .childChannelOption(ChannelOptions.recvAllocator, value: AdaptiveRecvByteBufferAllocator())
@@ -80,6 +79,7 @@ public final class ServerBootstrap {
     public init(group: EventLoopGroup, childGroup: EventLoopGroup) {
         self.group = group
         self.childGroup = childGroup
+        self._serverChannelOptions.append(key: ChannelOptions.socket(IPPROTO_TCP, TCP_NODELAY), value: 1)
     }
 
     /// Initialize the `ServerSocketChannel` with `initializer`. The most common task in initializer is to add
@@ -363,6 +363,7 @@ public final class ClientBootstrap {
     ///     - group: The `EventLoopGroup` to use.
     public init(group: EventLoopGroup) {
         self.group = group
+        self._channelOptions.append(key: ChannelOptions.socket(IPPROTO_TCP, TCP_NODELAY), value: 1)
     }
 
     /// Initialize the connected `SocketChannel` with `initializer`. The most common task in initializer is to add

diff --git a/Sources/NIO/ChannelOption.swift b/Sources/NIO/ChannelOption.swift
@@ -227,7 +227,9 @@ extension ChannelOptions {
         @usableFromInline
         internal var _storage: [(Any, (Any, (Channel) -> (Any, Any) -> EventLoopFuture<Void>))] = []
 
-        public init() { }
+        public init() {
+            self._storage.reserveCapacity(2)
+        }
 
         /// Add `Options`, a `ChannelOption` to the `ChannelOptions.Storage`.
         ///

diff --git a/Sources/NIOChatServer/main.swift b/Sources/NIOChatServer/main.swift
@@ -132,8 +132,7 @@ let bootstrap = ServerBootstrap(group: group)
         }
     }
 
-    // Enable TCP_NODELAY and SO_REUSEADDR for the accepted Channels
-    .childChannelOption(ChannelOptions.socket(IPPROTO_TCP, TCP_NODELAY), value: 1)
+    // Enable SO_REUSEADDR for the accepted Channels
     .childChannelOption(ChannelOptions.socket(SocketOptionLevel(SOL_SOCKET), SO_REUSEADDR), value: 1)
     .childChannelOption(ChannelOptions.maxMessagesPerRead, value: 16)
     .childChannelOption(ChannelOptions.recvAllocator, value: AdaptiveRecvByteBufferAllocator())

diff --git a/Sources/NIOEchoServer/main.swift b/Sources/NIOEchoServer/main.swift
@@ -50,8 +50,7 @@ let bootstrap = ServerBootstrap(group: group)
         }
     }
 
-    // Enable TCP_NODELAY and SO_REUSEADDR for the accepted Channels
-    .childChannelOption(ChannelOptions.socket(IPPROTO_TCP, TCP_NODELAY), value: 1)
+    // Enable SO_REUSEADDR for the accepted Channels
     .childChannelOption(ChannelOptions.socket(SocketOptionLevel(SOL_SOCKET), SO_REUSEADDR), value: 1)
     .childChannelOption(ChannelOptions.maxMessagesPerRead, value: 16)
     .childChannelOption(ChannelOptions.recvAllocator, value: AdaptiveRecvByteBufferAllocator())

diff --git a/Sources/NIOHTTP1/HTTPPipelineSetup.swift b/Sources/NIOHTTP1/HTTPPipelineSetup.swift
@@ -122,4 +122,3 @@ extension ChannelPipeline {
         return self.addHandlers(handlers, position: position)
     }
 }
-
diff --git a/Sources/NIOHTTP1Server/main.swift b/Sources/NIOHTTP1Server/main.swift
@@ -525,8 +525,7 @@ let bootstrap = ServerBootstrap(group: group)
         }
     }
 
-    // Enable TCP_NODELAY and SO_REUSEADDR for the accepted Channels
-    .childChannelOption(ChannelOptions.socket(IPPROTO_TCP, TCP_NODELAY), value: 1)
+    // Enable SO_REUSEADDR for the accepted Channels
     .childChannelOption(ChannelOptions.socket(SocketOptionLevel(SOL_SOCKET), SO_REUSEADDR), value: 1)
     .childChannelOption(ChannelOptions.maxMessagesPerRead, value: 1)
     .childChannelOption(ChannelOptions.allowRemoteHalfClosure, value: allowHalfClosure)

diff --git a/Sources/NIOPerformanceTester/main.swift b/Sources/NIOPerformanceTester/main.swift
@@ -124,7 +124,6 @@ defer {
 
 let serverChannel = try ServerBootstrap(group: group)
     .serverChannelOption(ChannelOptions.socket(SocketOptionLevel(SOL_SOCKET), SO_REUSEADDR), value: 1)
-    .childChannelOption(ChannelOptions.socket(IPPROTO_TCP, TCP_NODELAY), value: 1)
     .childChannelInitializer { channel in
         channel.pipeline.configureHTTPServerPipeline(withPipeliningAssistance: true).flatMap {
             channel.pipeline.addHandler(SimpleHTTPServer())
@@ -578,7 +577,6 @@ measureAndPrint(desc: "http1_10k_reqs_1_conn") {
     let repeatedRequestsHandler = RepeatedRequests(numberOfRequests: 10_000, eventLoop: group.next())
 
     let clientChannel = try! ClientBootstrap(group: group)
-        .channelOption(ChannelOptions.socket(IPPROTO_TCP, TCP_NODELAY), value: 1)
         .channelInitializer { channel in
             channel.pipeline.addHTTPClientHandlers().flatMap {
                 channel.pipeline.addHandler(repeatedRequestsHandler)

diff --git a/Sources/NIOWebSocketServer/main.swift b/Sources/NIOWebSocketServer/main.swift
@@ -226,8 +226,7 @@ let bootstrap = ServerBootstrap(group: group)
         }
     }
 
-    // Enable TCP_NODELAY and SO_REUSEADDR for the accepted Channels
-    .childChannelOption(ChannelOptions.socket(IPPROTO_TCP, TCP_NODELAY), value: 1)
+    // Enable SO_REUSEADDR for the accepted Channels
     .childChannelOption(ChannelOptions.socket(SocketOptionLevel(SOL_SOCKET), SO_REUSEADDR), value: 1)
 
 defer {

diff --git a/Tests/NIOTests/ChannelTests+XCTest.swift b/Tests/NIOTests/ChannelTests+XCTest.swift
@@ -78,6 +78,7 @@ extension ChannelTests {
                 ("testApplyingTwoDistinctSocketOptionsOfSameTypeWorks", testApplyingTwoDistinctSocketOptionsOfSameTypeWorks),
                 ("testUnprocessedOutboundUserEventFailsOnServerSocketChannel", testUnprocessedOutboundUserEventFailsOnServerSocketChannel),
                 ("testAcceptHandlerDoesNotSwallowCloseErrorsWhenQuiescing", testAcceptHandlerDoesNotSwallowCloseErrorsWhenQuiescing),
+                ("testTCP_NODELAYisOnByDefault", testTCP_NODELAYisOnByDefault),
            ]
    }
 }

diff --git a/Tests/NIOTests/ChannelTests.swift b/Tests/NIOTests/ChannelTests.swift
@@ -2592,7 +2592,7 @@ public final class ChannelTests: XCTestCase {
             .serverChannelOption(ChannelOptions.socket(SocketOptionLevel(SOL_SOCKET), SO_REUSEADDR), value: 1)
             .serverChannelOption(ChannelOptions.socket(SocketOptionLevel(SOL_SOCKET), SO_TIMESTAMP), value: 1)
             .childChannelOption(ChannelOptions.socket(SocketOptionLevel(SOL_SOCKET), SO_KEEPALIVE), value: 1)
-            .childChannelOption(ChannelOptions.socket(SocketOptionLevel(IPPROTO_TCP), TCP_NODELAY), value: 1)
+            .childChannelOption(ChannelOptions.socket(SocketOptionLevel(IPPROTO_TCP), TCP_NODELAY), value: 0)
             .childChannelInitializer { channel in
                 acceptedChannels[numberOfAcceptedChannel].succeed(channel)
                 numberOfAcceptedChannel += 1
@@ -2608,7 +2608,7 @@ public final class ChannelTests: XCTestCase {
 
         let client1 = try assertNoThrowWithValue(ClientBootstrap(group: singleThreadedELG)
             .channelOption(ChannelOptions.socket(SocketOptionLevel(SOL_SOCKET), SO_REUSEADDR), value: 1)
-            .channelOption(ChannelOptions.socket(SocketOptionLevel(IPPROTO_TCP), TCP_NODELAY), value: 1)
+            .channelOption(ChannelOptions.socket(SocketOptionLevel(IPPROTO_TCP), TCP_NODELAY), value: 0)
             .connect(to: server.localAddress!)
             .wait())
         let accepted1 = try assertNoThrowWithValue(acceptedChannels[0].futureResult.wait())
@@ -2633,27 +2633,27 @@ public final class ChannelTests: XCTestCase {
 
         XCTAssertTrue(try getBoolSocketOption(channel: client1, level: SOL_SOCKET, name: SO_REUSEADDR))
 
-        XCTAssertTrue(try getBoolSocketOption(channel: client1, level: IPPROTO_TCP, name: TCP_NODELAY))
+        XCTAssertFalse(try getBoolSocketOption(channel: client1, level: IPPROTO_TCP, name: TCP_NODELAY))
 
         XCTAssertTrue(try getBoolSocketOption(channel: accepted1, level: SOL_SOCKET, name: SO_KEEPALIVE))
 
-        XCTAssertTrue(try getBoolSocketOption(channel: accepted1, level: IPPROTO_TCP, name: TCP_NODELAY))
+        XCTAssertFalse(try getBoolSocketOption(channel: accepted1, level: IPPROTO_TCP, name: TCP_NODELAY))
 
         XCTAssertTrue(try getBoolSocketOption(channel: client2, level: SOL_SOCKET, name: SO_REUSEADDR))
 
-        XCTAssertFalse(try getBoolSocketOption(channel: client2, level: IPPROTO_TCP, name: TCP_NODELAY))
+        XCTAssertTrue(try getBoolSocketOption(channel: client2, level: IPPROTO_TCP, name: TCP_NODELAY))
 
         XCTAssertTrue(try getBoolSocketOption(channel: accepted2, level: SOL_SOCKET, name: SO_KEEPALIVE))
 
-        XCTAssertTrue(try getBoolSocketOption(channel: accepted2, level: IPPROTO_TCP, name: TCP_NODELAY))
+        XCTAssertFalse(try getBoolSocketOption(channel: accepted2, level: IPPROTO_TCP, name: TCP_NODELAY))
 
         XCTAssertFalse(try getBoolSocketOption(channel: client3, level: SOL_SOCKET, name: SO_REUSEADDR))
 
-        XCTAssertFalse(try getBoolSocketOption(channel: client3, level: IPPROTO_TCP, name: TCP_NODELAY))
+        XCTAssertTrue(try getBoolSocketOption(channel: client3, level: IPPROTO_TCP, name: TCP_NODELAY))
 
         XCTAssertTrue(try getBoolSocketOption(channel: accepted3, level: SOL_SOCKET, name: SO_KEEPALIVE))
 
-        XCTAssertTrue(try getBoolSocketOption(channel: accepted3, level: IPPROTO_TCP, name: TCP_NODELAY))
+        XCTAssertFalse(try getBoolSocketOption(channel: accepted3, level: IPPROTO_TCP, name: TCP_NODELAY))
     }
 
     func testUnprocessedOutboundUserEventFailsOnServerSocketChannel() throws {
@@ -2714,6 +2714,42 @@ public final class ChannelTests: XCTestCase {
         XCTAssertEqual(["userInboundEventTriggered", "close", "errorCaught"], counter.allTriggeredEvents())
         XCTAssertEqual(1, counter.errorCaughtCalls)
     }
+
+    func testTCP_NODELAYisOnByDefault() throws {
+        let singleThreadedELG = MultiThreadedEventLoopGroup(numberOfThreads: 1)
+        defer {
+            XCTAssertNoThrow(try singleThreadedELG.syncShutdownGracefully())
+        }
+        var numberOfAcceptedChannel = 0
+        var acceptedChannel = singleThreadedELG.next().makePromise(of: Channel.self)
+        let server = try assertNoThrowWithValue(ServerBootstrap(group: singleThreadedELG)
+            .childChannelInitializer { channel in
+                acceptedChannel.succeed(channel)
+                return channel.eventLoop.makeSucceededFuture(())
+            }
+            .bind(host: "127.0.0.1", port: 0)
+            .wait())
+        defer {
+            XCTAssertNoThrow(try server.close().wait())
+        }
+        XCTAssertNoThrow(XCTAssertTrue(try getBoolSocketOption(channel: server,
+                                                               level: IPPROTO_TCP,
+                                                               name: TCP_NODELAY)))
+
+        let client = try assertNoThrowWithValue(ClientBootstrap(group: singleThreadedELG)
+            .connect(to: server.localAddress!)
+            .wait())
+        let accepted = try assertNoThrowWithValue(acceptedChannel.futureResult.wait())
+        defer {
+            XCTAssertNoThrow(try client.close().wait())
+        }
+        XCTAssertNoThrow(XCTAssertTrue(try getBoolSocketOption(channel: accepted,
+                                                               level: IPPROTO_TCP,
+                                                               name: TCP_NODELAY)))
+        XCTAssertNoThrow(XCTAssertTrue(try getBoolSocketOption(channel: client,
+                                                               level: IPPROTO_TCP,
+                                                               name: TCP_NODELAY)))
+    }
 }
 
 fileprivate final class FailRegistrationAndDelayCloseHandler: ChannelOutboundHandler {

diff --git a/Tests/NIOTests/EchoServerClientTest.swift b/Tests/NIOTests/EchoServerClientTest.swift
@@ -636,7 +636,6 @@ class EchoServerClientTest : XCTestCase {
         }
         let serverChannel = try assertNoThrowWithValue(ServerBootstrap(group: group)
             .serverChannelOption(ChannelOptions.socket(SocketOptionLevel(SOL_SOCKET), SO_REUSEADDR), value: 1)
-            .childChannelOption(ChannelOptions.socket(SocketOptionLevel(IPPROTO_TCP), TCP_NODELAY), value: 1)
             .childChannelInitializer { channel in
                 channel.pipeline.addHandler(WriteWhenActiveHandler(str, dpGroup))
             }.bind(host: "127.0.0.1", port: 0).wait())

diff --git a/docker/docker-compose.1604.51.yaml b/docker/docker-compose.1604.51.yaml
@@ -22,7 +22,7 @@ services:
     image: swift-nio:16.04-5.1
     environment:
       - MAX_ALLOCS_ALLOWED_1000_reqs_1_conn=30600
-      - MAX_ALLOCS_ALLOWED_1_reqs_1000_conn=584100
+      - MAX_ALLOCS_ALLOWED_1_reqs_1000_conn=592100
       - MAX_ALLOCS_ALLOWED_ping_pong_1000_reqs_1_conn=4600
       - MAX_ALLOCS_ALLOWED_bytebuffer_lots_of_rw=2100
       - MAX_ALLOCS_ALLOWED_future_lots_of_callbacks=99100

diff --git a/docker/docker-compose.1804.50.yaml b/docker/docker-compose.1804.50.yaml
@@ -20,7 +20,7 @@ services:
     image: swift-nio:18.04-5.0
     environment:
       - MAX_ALLOCS_ALLOWED_1000_reqs_1_conn=31200
-      - MAX_ALLOCS_ALLOWED_1_reqs_1000_conn=1054050
+      - MAX_ALLOCS_ALLOWED_1_reqs_1000_conn=1062050
       - MAX_ALLOCS_ALLOWED_ping_pong_1000_reqs_1_conn=4600
       - MAX_ALLOCS_ALLOWED_bytebuffer_lots_of_rw=2100
       - MAX_ALLOCS_ALLOWED_future_lots_of_callbacks=99100