Explorar o código

Use sync options to get the http2 stream ID (#1229)

Motivation:

NIO added API for getting channel options synchronously. NIO HTTP/2
added support for this in 1.17.0. When configuring a server we configure
our new stream channels to get the stream ID to attach to the loggers.
We can now do this synchronously and save some allocations.

Modifications:

- Bump minimum H2 version to 1.17.0
- Use sync options to get the stream ID and sync pipeline operations to
  add the handler.
- Modify alloc-limits.sh so format better aligns with new CI yaml

Result:

Fewer allocations.
George Barnett %!s(int64=4) %!d(string=hai) anos
pai
achega
529d6914ca

+ 10 - 10
.github/workflows/ci.yaml

@@ -51,28 +51,28 @@ jobs:
         include:
           - image: swift:5.4-focal
             env:
-              MAX_ALLOCS_ALLOWED_bidi_1k_rpcs_10_requests: 515000
-              MAX_ALLOCS_ALLOWED_bidi_1k_rpcs_1_request: 227000
+              MAX_ALLOCS_ALLOWED_bidi_1k_rpcs_10_requests: 503000
+              MAX_ALLOCS_ALLOWED_bidi_1k_rpcs_1_request: 215000
               MAX_ALLOCS_ALLOWED_embedded_server_bidi_1k_rpcs_10_small_requests: 112000
               MAX_ALLOCS_ALLOWED_embedded_server_bidi_1k_rpcs_1_small_request: 67000
               MAX_ALLOCS_ALLOWED_embedded_server_unary_1k_rpcs_1_small_request: 63000
-              MAX_ALLOCS_ALLOWED_unary_1k_ping_pong: 216000
+              MAX_ALLOCS_ALLOWED_unary_1k_ping_pong: 204000
           - image: swift:5.3-focal
             env:
-              MAX_ALLOCS_ALLOWED_bidi_1k_rpcs_10_requests: 515000
-              MAX_ALLOCS_ALLOWED_bidi_1k_rpcs_1_request: 227000
+              MAX_ALLOCS_ALLOWED_bidi_1k_rpcs_10_requests: 504000
+              MAX_ALLOCS_ALLOWED_bidi_1k_rpcs_1_request: 216000
               MAX_ALLOCS_ALLOWED_embedded_server_bidi_1k_rpcs_10_small_requests: 112000
               MAX_ALLOCS_ALLOWED_embedded_server_bidi_1k_rpcs_1_small_request: 67000
               MAX_ALLOCS_ALLOWED_embedded_server_unary_1k_rpcs_1_small_request: 63000
-              MAX_ALLOCS_ALLOWED_unary_1k_ping_pong: 216000
+              MAX_ALLOCS_ALLOWED_unary_1k_ping_pong: 205000
           - image: swift:5.2-bionic
             env:
-              MAX_ALLOCS_ALLOWED_bidi_1k_rpcs_10_requests: 526000
-              MAX_ALLOCS_ALLOWED_bidi_1k_rpcs_1_request: 229000
+              MAX_ALLOCS_ALLOWED_bidi_1k_rpcs_10_requests: 515000
+              MAX_ALLOCS_ALLOWED_bidi_1k_rpcs_1_request: 218000
               MAX_ALLOCS_ALLOWED_embedded_server_bidi_1k_rpcs_10_small_requests: 112000
               MAX_ALLOCS_ALLOWED_embedded_server_bidi_1k_rpcs_1_small_request: 67000
               MAX_ALLOCS_ALLOWED_embedded_server_unary_1k_rpcs_1_small_request: 63000
-              MAX_ALLOCS_ALLOWED_unary_1k_ping_pong: 217000
+              MAX_ALLOCS_ALLOWED_unary_1k_ping_pong: 206000
     name: Performance Tests on ${{ matrix.image }}
     runs-on: ubuntu-latest
     container:
@@ -82,4 +82,4 @@ jobs:
     - name: 🧮 Allocation Counting Tests
       run: ./Performance/allocations/test-allocation-counts.sh
       env: ${{ matrix.env }}
-      timeout-minutes: 20
+      timeout-minutes: 20

+ 1 - 1
Package.swift

@@ -28,7 +28,7 @@ let package = Package(
     // Main SwiftNIO package
     .package(url: "https://github.com/apple/swift-nio.git", from: "2.28.0"),
     // HTTP2 via SwiftNIO
-    .package(url: "https://github.com/apple/swift-nio-http2.git", from: "1.16.1"),
+    .package(url: "https://github.com/apple/swift-nio-http2.git", from: "1.17.0"),
     // TLS via SwiftNIO
     .package(url: "https://github.com/apple/swift-nio-ssl.git", from: "2.14.0"),
     // Support for Network.framework where possible.

+ 14 - 10
Sources/GRPC/GRPCServerPipelineConfigurator.swift

@@ -103,18 +103,22 @@ final class GRPCServerPipelineConfigurator: ChannelInboundHandler, RemovableChan
       channel: channel,
       targetWindowSize: self.configuration.httpTargetWindowSize
     ) { stream in
-      // TODO: use sync options when NIO HTTP/2 support for them is released
-      // https://github.com/apple/swift-nio-http2/pull/283
-      stream.getOption(HTTP2StreamChannelOptions.streamID).map { streamID -> Logger in
-        logger[metadataKey: MetadataKey.h2StreamID] = "\(streamID)"
-        return logger
-      }.recover { _ in
-        logger[metadataKey: MetadataKey.h2StreamID] = "<unknown>"
-        return logger
-      }.flatMap { logger in
+      // Sync options were added to the HTTP/2 stream channel in 1.17.0 (we require at least this)
+      // so this shouldn't be `nil`, but it's not a problem if it is.
+      let http2StreamID = try? stream.syncOptions?.getOption(HTTP2StreamChannelOptions.streamID)
+      let streamID = http2StreamID.map { streamID in
+        return String(Int(streamID))
+      } ?? "<unknown>"
+
+      logger[metadataKey: MetadataKey.h2StreamID] = "\(streamID)"
+
+      do {
         // TODO: provide user configuration for header normalization.
         let handler = self.makeHTTP2ToRawGRPCHandler(normalizeHeaders: true, logger: logger)
-        return stream.pipeline.addHandler(handler)
+        try stream.pipeline.syncOperations.addHandler(handler)
+        return stream.eventLoop.makeSucceededVoidFuture()
+      } catch {
+        return stream.eventLoop.makeFailedFuture(error)
       }
     }
   }

+ 1 - 1
scripts/alloc-limits.sh

@@ -30,5 +30,5 @@
 grep 'test_.*\.total_allocations: ' \
   | sed 's/^test_/MAX_ALLOCS_ALLOWED_/' \
   | sed 's/.total_allocations://' \
-  | awk '{ print $1 "=" ((int($2 / 1000) + 1) * 1000) }' \
+  | awk '{ print "              " $1 ": " ((int($2 / 1000) + 1) * 1000) }' \
   | sort