Skip to content

Commit

Permalink
Use sync options to get the http2 stream ID (#1229)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
glbrntt authored Jul 21, 2021
1 parent e9ce378 commit 529d691
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 22 deletions.
20 changes: 10 additions & 10 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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
2 changes: 1 addition & 1 deletion Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
24 changes: 14 additions & 10 deletions Sources/GRPC/GRPCServerPipelineConfigurator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion scripts/alloc-limits.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 529d691

Please sign in to comment.