Use sync operations for configuring the client pipeline (#1160)

Motivation:

We can use synchronous pipeline operations to save some allocations. We
converted to this new API in a handful of places in #1149 but this one
was missed.

While NIOs async pipeline operations now use synchronous operations
where possible (i.e. if already on the right event loop), they rely on having
a cached void future to save allocations. This isn't yet supported by
NIOTS, so we'll only save allocations there.

Modifications:

- Synchronously configure the client connection channel pipeline

Result:

Fewer allocations (when using NIOTS).

Sources/GRPC/ClientConnection.swift

@@ -411,8 +411,9 @@ extension ClientBootstrapProtocol {
   }
 }
 
-extension Channel {
-  func configureGRPCClient(
+extension ChannelPipeline.SynchronousOperations {
+  internal func configureGRPCClient(
+    channel: Channel,
     httpTargetWindowSize: Int,
     tlsConfiguration: TLSConfiguration?,
     tlsServerHostname: String?,
@@ -423,70 +424,61 @@ extension Channel {
     requiresZeroLengthWriteWorkaround: Bool,
     logger: Logger,
     customVerificationCallback: NIOSSLCustomVerificationCallback?
-  ) -> EventLoopFuture<Void> {
-    // We add at most 8 handlers to the pipeline.
-    var handlers: [ChannelHandler] = []
-    handlers.reserveCapacity(7)
-
+  ) throws {
     #if canImport(Network)
     // This availability guard is arguably unnecessary, but we add it anyway.
     if requiresZeroLengthWriteWorkaround,
       #available(OSX 10.14, iOS 12.0, tvOS 12.0, watchOS 6.0, *) {
-      handlers.append(NIOFilterEmptyWritesHandler())
+      try self.addHandler(NIOFilterEmptyWritesHandler())
     }
     #endif
 
     if let tlsConfiguration = tlsConfiguration {
-      do {
-        if let customVerificationCallback = customVerificationCallback {
-          let sslClientHandler = try NIOSSLClientHandler(
-            context: try NIOSSLContext(configuration: tlsConfiguration),
-            serverHostname: tlsServerHostname,
-            customVerificationCallback: customVerificationCallback
-          )
-          handlers.append(sslClientHandler)
-        } else {
-          let sslClientHandler = try NIOSSLClientHandler(
-            context: try NIOSSLContext(configuration: tlsConfiguration),
-            serverHostname: tlsServerHostname
-          )
-          handlers.append(sslClientHandler)
-        }
-        handlers.append(TLSVerificationHandler(logger: logger))
-      } catch {
-        return self.eventLoop.makeFailedFuture(error)
+      let sslClientHandler: NIOSSLClientHandler
+      if let customVerificationCallback = customVerificationCallback {
+        sslClientHandler = try NIOSSLClientHandler(
+          context: try NIOSSLContext(configuration: tlsConfiguration),
+          serverHostname: tlsServerHostname,
+          customVerificationCallback: customVerificationCallback
+        )
+      } else {
+        sslClientHandler = try NIOSSLClientHandler(
+          context: try NIOSSLContext(configuration: tlsConfiguration),
+          serverHostname: tlsServerHostname
+        )
       }
+      try self.addHandler(sslClientHandler)
+      try self.addHandler(TLSVerificationHandler(logger: logger))
     }
 
     // We could use 'configureHTTP2Pipeline' here, but we need to add a few handlers between the
     // two HTTP/2 handlers so we'll do it manually instead.
+    try self.addHandler(NIOHTTP2Handler(mode: .client))
 
     let h2Multiplexer = HTTP2StreamMultiplexer(
       mode: .client,
-      channel: self,
+      channel: channel,
       targetWindowSize: httpTargetWindowSize,
       inboundStreamInitializer: nil
     )
 
-    handlers.append(NIOHTTP2Handler(mode: .client))
     // The multiplexer is passed through the idle handler so it is only reported on
     // successful channel activation - with happy eyeballs multiple pipelines can
     // be constructed so it's not safe to report just yet.
-    handlers.append(
-      GRPCIdleHandler(
-        connectionManager: connectionManager,
-        multiplexer: h2Multiplexer,
-        idleTimeout: connectionIdleTimeout,
-        keepalive: connectionKeepalive,
-        logger: logger
-      )
-    )
-    handlers.append(h2Multiplexer)
-    handlers.append(DelegatingErrorHandler(logger: logger, delegate: errorDelegate))
-
-    return self.pipeline.addHandlers(handlers)
+    try self.addHandler(GRPCIdleHandler(
+      connectionManager: connectionManager,
+      multiplexer: h2Multiplexer,
+      idleTimeout: connectionIdleTimeout,
+      keepalive: connectionKeepalive,
+      logger: logger
+    ))
+
+    try self.addHandler(h2Multiplexer)
+    try self.addHandler(DelegatingErrorHandler(logger: logger, delegate: errorDelegate))
   }
+}
 
+extension Channel {
   func configureGRPCClient(
     errorDelegate: ClientErrorDelegate?,
     logger: Logger

Sources/GRPC/ConnectionManagerChannelProvider.swift

@@ -67,29 +67,34 @@ extension ClientConnection.ChannelProvider: ConnectionManagerChannelProvider {
       .channelOption(ChannelOptions.socket(SocketOptionLevel(SOL_SOCKET), SO_REUSEADDR), value: 1)
       .channelOption(ChannelOptions.socket(IPPROTO_TCP, TCP_NODELAY), value: 1)
       .channelInitializer { channel in
-        let initialized = channel.configureGRPCClient(
-          httpTargetWindowSize: self.configuration.httpTargetWindowSize,
-          tlsConfiguration: self.configuration.tls?.configuration,
-          tlsServerHostname: serverHostname,
-          connectionManager: connectionManager,
-          connectionKeepalive: self.configuration.connectionKeepalive,
-          connectionIdleTimeout: self.configuration.connectionIdleTimeout,
-          errorDelegate: self.configuration.errorDelegate,
-          requiresZeroLengthWriteWorkaround: PlatformSupport.requiresZeroLengthWriteWorkaround(
-            group: eventLoop,
-            hasTLS: self.configuration.tls != nil
-          ),
-          logger: logger,
-          customVerificationCallback: self.configuration.tls?.customVerificationCallback
-        )
+        let sync = channel.pipeline.syncOperations
+
+        do {
+          try sync.configureGRPCClient(
+            channel: channel,
+            httpTargetWindowSize: self.configuration.httpTargetWindowSize,
+            tlsConfiguration: self.configuration.tls?.configuration,
+            tlsServerHostname: serverHostname,
+            connectionManager: connectionManager,
+            connectionKeepalive: self.configuration.connectionKeepalive,
+            connectionIdleTimeout: self.configuration.connectionIdleTimeout,
+            errorDelegate: self.configuration.errorDelegate,
+            requiresZeroLengthWriteWorkaround: PlatformSupport.requiresZeroLengthWriteWorkaround(
+              group: eventLoop,
+              hasTLS: self.configuration.tls != nil
+            ),
+            logger: logger,
+            customVerificationCallback: self.configuration.tls?.customVerificationCallback
+          )
+        } catch {
+          return channel.eventLoop.makeFailedFuture(error)
+        }
 
         // Run the debug initializer, if there is one.
         if let debugInitializer = self.configuration.debugChannelInitializer {
-          return initialized.flatMap {
-            debugInitializer(channel)
-          }
+          return debugInitializer(channel)
         } else {
-          return initialized
+          return channel.eventLoop.makeSucceededVoidFuture()
         }
       }