Skip to content

Commit 2828ee7

Browse files
glbrnttglbrntt
authored
Revert "Adopt h2handler multiplexer (#1587)" (#1641)
This reverts commit 75b390e.
1 parent 666e30d commit 2828ee7

18 files changed

+483
-866
lines changed

‎.github/workflows/ci.yaml

+20-21
Original file line numberDiff line numberDiff line change
@@ -57,45 +57,44 @@ jobs:
5757
include:
5858
- image: swiftlang/swift:nightly-focal
5959
env:
60-
MAX_ALLOCS_ALLOWED_bidi_1k_rpcs_10_requests: 246000
61-
MAX_ALLOCS_ALLOWED_bidi_1k_rpcs_1_request: 138000
60+
MAX_ALLOCS_ALLOWED_bidi_1k_rpcs_10_requests: 323000
61+
MAX_ALLOCS_ALLOWED_bidi_1k_rpcs_1_request: 161000
6262
MAX_ALLOCS_ALLOWED_embedded_server_bidi_1k_rpcs_10_small_requests: 110000
6363
MAX_ALLOCS_ALLOWED_embedded_server_bidi_1k_rpcs_1_small_request: 65000
6464
MAX_ALLOCS_ALLOWED_embedded_server_unary_1k_rpcs_1_small_request: 61000
65-
MAX_ALLOCS_ALLOWED_unary_1k_ping_pong: 129000
66-
MAX_ALLOCS_ALLOWED_unary_1k_ping_pong_interceptors_client: 136000
67-
MAX_ALLOCS_ALLOWED_unary_1k_ping_pong_interceptors_server: 136000
65+
MAX_ALLOCS_ALLOWED_unary_1k_ping_pong: 163000
66+
MAX_ALLOCS_ALLOWED_unary_1k_ping_pong_interceptors_client: 170000
67+
MAX_ALLOCS_ALLOWED_unary_1k_ping_pong_interceptors_server: 170000
6868
- image: swift:5.8-jammy
6969
env:
70-
MAX_ALLOCS_ALLOWED_bidi_1k_rpcs_10_requests: 246000
71-
MAX_ALLOCS_ALLOWED_bidi_1k_rpcs_1_request: 138000
70+
MAX_ALLOCS_ALLOWED_bidi_1k_rpcs_10_requests: 323000
71+
MAX_ALLOCS_ALLOWED_bidi_1k_rpcs_1_request: 161000
7272
MAX_ALLOCS_ALLOWED_embedded_server_bidi_1k_rpcs_10_small_requests: 110000
7373
MAX_ALLOCS_ALLOWED_embedded_server_bidi_1k_rpcs_1_small_request: 65000
7474
MAX_ALLOCS_ALLOWED_embedded_server_unary_1k_rpcs_1_small_request: 61000
75-
MAX_ALLOCS_ALLOWED_unary_1k_ping_pong: 129000
76-
MAX_ALLOCS_ALLOWED_unary_1k_ping_pong_interceptors_client: 136000
77-
MAX_ALLOCS_ALLOWED_unary_1k_ping_pong_interceptors_server: 136000
75+
MAX_ALLOCS_ALLOWED_unary_1k_ping_pong: 163000
76+
MAX_ALLOCS_ALLOWED_unary_1k_ping_pong_interceptors_client: 170000
77+
MAX_ALLOCS_ALLOWED_unary_1k_ping_pong_interceptors_server: 170000
7878
- image: swift:5.7-jammy
7979
env:
80-
MAX_ALLOCS_ALLOWED_bidi_1k_rpcs_10_requests: 246000
81-
MAX_ALLOCS_ALLOWED_bidi_1k_rpcs_1_request: 138000
80+
MAX_ALLOCS_ALLOWED_bidi_1k_rpcs_10_requests: 323000
81+
MAX_ALLOCS_ALLOWED_bidi_1k_rpcs_1_request: 161000
8282
MAX_ALLOCS_ALLOWED_embedded_server_bidi_1k_rpcs_10_small_requests: 110000
8383
MAX_ALLOCS_ALLOWED_embedded_server_bidi_1k_rpcs_1_small_request: 65000
8484
MAX_ALLOCS_ALLOWED_embedded_server_unary_1k_rpcs_1_small_request: 61000
85-
MAX_ALLOCS_ALLOWED_unary_1k_ping_pong: 129000
86-
MAX_ALLOCS_ALLOWED_unary_1k_ping_pong_interceptors_client: 136000
87-
MAX_ALLOCS_ALLOWED_unary_1k_ping_pong_interceptors_server: 136000
85+
MAX_ALLOCS_ALLOWED_unary_1k_ping_pong: 163000
86+
MAX_ALLOCS_ALLOWED_unary_1k_ping_pong_interceptors_client: 170000
87+
MAX_ALLOCS_ALLOWED_unary_1k_ping_pong_interceptors_server: 170000
8888
- image: swift:5.6-focal
8989
env:
90-
MAX_ALLOCS_ALLOWED_bidi_1k_rpcs_10_requests: 247000
91-
MAX_ALLOCS_ALLOWED_bidi_1k_rpcs_1_request: 139000
90+
MAX_ALLOCS_ALLOWED_bidi_1k_rpcs_10_requests: 324000
91+
MAX_ALLOCS_ALLOWED_bidi_1k_rpcs_1_request: 162000
9292
MAX_ALLOCS_ALLOWED_embedded_server_bidi_1k_rpcs_10_small_requests: 110000
9393
MAX_ALLOCS_ALLOWED_embedded_server_bidi_1k_rpcs_1_small_request: 65000
9494
MAX_ALLOCS_ALLOWED_embedded_server_unary_1k_rpcs_1_small_request: 61000
95-
MAX_ALLOCS_ALLOWED_unary_1k_ping_pong: 130000
96-
MAX_ALLOCS_ALLOWED_unary_1k_ping_pong_interceptors_client: 137000
97-
MAX_ALLOCS_ALLOWED_unary_1k_ping_pong_interceptors_server: 137000
98-
95+
MAX_ALLOCS_ALLOWED_unary_1k_ping_pong: 164000
96+
MAX_ALLOCS_ALLOWED_unary_1k_ping_pong_interceptors_client: 171000
97+
MAX_ALLOCS_ALLOWED_unary_1k_ping_pong_interceptors_server: 171000
9998
name: Performance Tests on ${{ matrix.image }}
10099
runs-on: ubuntu-latest
101100
container:

‎NOTICES.txt

-8
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,3 @@ framework: 'test_01_allocation_counts.sh', 'run-nio-alloc-counter-tests.sh' and
2525
* https://github.com/apple/swift-nio/blob/main/LICENSE.txt
2626
* HOMEPAGE:
2727
* https://github.com/apple/swift-nio
28-
29-
This product contains a simplified derivation of SwiftNIO HTTP/2's
30-
'HTTP2FrameEncoder' for testing purposes.
31-
32-
* LICENSE (Apache License 2.0):
33-
* https://github.com/apple/swift-nio-http2/blob/main/LICENSE.txt
34-
* HOMEPAGE:
35-
* https://github.com/apple/swift-nio-http2

‎Package.swift

+1-1
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ let packageDependencies: [Package.Dependency] = [
3636
),
3737
.package(
3838
url: "https://github.com/apple/swift-nio-http2.git",
39-
from: "1.26.0"
39+
from: "1.24.1"
4040
),
4141
.package(
4242
url: "https://github.com/apple/swift-nio-transport-services.git",

‎Sources/GRPC/ClientConnection.swift

+43-48
Original file line numberDiff line numberDiff line change
@@ -58,25 +58,28 @@ import SwiftProtobuf
5858
/// │ DelegatingErrorHandler │
5959
/// └──────────▲───────────────┘
6060
/// HTTP2Frame│
61-
/// │
62-
/// │
63-
/// │
64-
/// │
65-
/// │
66-
/// HTTP2Frame│ ⠇ ⠇ ⠇ ⠇ ⠇
67-
/// ┌─┴──────────────────▼─┐ ┌┴─▼┐ ┌┴─▼┐
68-
/// │ GRPCIdleHandler │ │ | │ | HTTP/2 streams
69-
/// └─▲──────────────────┬─┘ └▲─┬┘ └▲─┬┘
70-
/// HTTP2Frame│ │ │ │ │ │ HTTP2Frame
71-
/// ┌─┴──────────────────▼────────┴─▼───┴─▼┐
72-
/// │ NIOHTTP2Handler │
73-
/// └─▲──────────────────────────────────┬─┘
74-
/// ByteBuffer│ │ByteBuffer
75-
/// ┌─┴──────────────────────────────────▼─┐
76-
/// │ NIOSSLHandler │
77-
/// └─▲──────────────────────────────────┬─┘
78-
/// ByteBuffer│ │ByteBuffer
79-
/// │ ▼
61+
/// │ ⠇ ⠇ ⠇ ⠇
62+
/// │ ┌┴─▼┐ ┌┴─▼┐
63+
/// │ │ | │ | HTTP/2 streams
64+
/// │ └▲─┬┘ └▲─┬┘
65+
/// │ │ │ │ │ HTTP2Frame
66+
/// ┌─┴────────────────┴─▼───┴─▼┐
67+
/// │ HTTP2StreamMultiplexer |
68+
/// └─▲───────────────────────┬─┘
69+
/// HTTP2Frame│ │HTTP2Frame
70+
/// ┌─┴───────────────────────▼─┐
71+
/// │ GRPCIdleHandler │
72+
/// └─▲───────────────────────┬─┘
73+
/// HTTP2Frame│ │HTTP2Frame
74+
/// ┌─┴───────────────────────▼─┐
75+
/// │ NIOHTTP2Handler │
76+
/// └─▲───────────────────────┬─┘
77+
/// ByteBuffer│ │ByteBuffer
78+
/// ┌─┴───────────────────────▼─┐
79+
/// │ NIOSSLHandler │
80+
/// └─▲───────────────────────┬─┘
81+
/// ByteBuffer│ │ByteBuffer
82+
/// │ ▼
8083
///
8184
/// The 'GRPCIdleHandler' intercepts HTTP/2 frames and various events and is responsible for
8285
/// informing and controlling the state of the connection (idling and keepalive). The HTTP/2 streams
@@ -85,7 +88,7 @@ public final class ClientConnection: Sendable {
8588
private let connectionManager: ConnectionManager
8689

8790
/// HTTP multiplexer from the underlying channel handling gRPC calls.
88-
internal func getMultiplexer() -> EventLoopFuture<NIOHTTP2Handler.StreamMultiplexer> {
91+
internal func getMultiplexer() -> EventLoopFuture<HTTP2StreamMultiplexer> {
8992
return self.connectionManager.getHTTP2Multiplexer()
9093
}
9194

@@ -247,7 +250,7 @@ extension ClientConnection: GRPCChannel {
247250
}
248251

249252
private static func makeStreamChannel(
250-
using result: Result<NIOHTTP2Handler.StreamMultiplexer, Error>,
253+
using result: Result<HTTP2StreamMultiplexer, Error>,
251254
promise: EventLoopPromise<Channel>
252255
) {
253256
switch result {
@@ -618,31 +621,29 @@ extension ChannelPipeline.SynchronousOperations {
618621
HTTP2Setting(parameter: .initialWindowSize, value: httpTargetWindowSize),
619622
]
620623

621-
let grpcIdleHandler = GRPCIdleHandler(
624+
// We could use 'configureHTTP2Pipeline' here, but we need to add a few handlers between the
625+
// two HTTP/2 handlers so we'll do it manually instead.
626+
try self.addHandler(NIOHTTP2Handler(mode: .client, initialSettings: initialSettings))
627+
628+
let h2Multiplexer = HTTP2StreamMultiplexer(
629+
mode: .client,
630+
channel: channel,
631+
targetWindowSize: httpTargetWindowSize,
632+
inboundStreamInitializer: nil
633+
)
634+
635+
// The multiplexer is passed through the idle handler so it is only reported on
636+
// successful channel activation - with happy eyeballs multiple pipelines can
637+
// be constructed so it's not safe to report just yet.
638+
try self.addHandler(GRPCIdleHandler(
622639
connectionManager: connectionManager,
640+
multiplexer: h2Multiplexer,
623641
idleTimeout: connectionIdleTimeout,
624642
keepalive: connectionKeepalive,
625643
logger: logger
626-
)
627-
628-
var connectionConfiguration = NIOHTTP2Handler.ConnectionConfiguration()
629-
connectionConfiguration.initialSettings = initialSettings
630-
var streamConfiguration = NIOHTTP2Handler.StreamConfiguration()
631-
streamConfiguration.targetWindowSize = httpTargetWindowSize
632-
let h2Handler = NIOHTTP2Handler(
633-
mode: .client,
634-
eventLoop: channel.eventLoop,
635-
connectionConfiguration: connectionConfiguration,
636-
streamConfiguration: streamConfiguration,
637-
streamDelegate: grpcIdleHandler
638-
) { channel in
639-
channel.close()
640-
}
641-
try self.addHandler(h2Handler)
642-
643-
grpcIdleHandler.setMultiplexer(try h2Handler.syncMultiplexer())
644-
try self.addHandler(grpcIdleHandler)
644+
))
645645

646+
try self.addHandler(h2Multiplexer)
646647
try self.addHandler(DelegatingErrorHandler(logger: logger, delegate: errorDelegate))
647648
}
648649
}
@@ -652,13 +653,7 @@ extension Channel {
652653
errorDelegate: ClientErrorDelegate?,
653654
logger: Logger
654655
) -> EventLoopFuture<Void> {
655-
return self.configureHTTP2Pipeline(
656-
mode: .client,
657-
connectionConfiguration: .init(),
658-
streamConfiguration: .init()
659-
) { channel in
660-
channel.eventLoop.makeSucceededVoidFuture()
661-
}.flatMap { _ in
656+
return self.configureHTTP2Pipeline(mode: .client, inboundStreamInitializer: nil).flatMap { _ in
662657
self.pipeline.addHandler(DelegatingErrorHandler(logger: logger, delegate: errorDelegate))
663658
}
664659
}

‎Sources/GRPC/ConnectionManager.swift

+18-23
Original file line numberDiff line numberDiff line change
@@ -35,23 +35,19 @@ internal final class ConnectionManager: @unchecked Sendable {
3535
var reconnect: Reconnect
3636

3737
var candidate: EventLoopFuture<Channel>
38-
var readyChannelMuxPromise: EventLoopPromise<NIOHTTP2Handler.StreamMultiplexer>
39-
var candidateMuxPromise: EventLoopPromise<NIOHTTP2Handler.StreamMultiplexer>
38+
var readyChannelMuxPromise: EventLoopPromise<HTTP2StreamMultiplexer>
39+
var candidateMuxPromise: EventLoopPromise<HTTP2StreamMultiplexer>
4040
}
4141

4242
internal struct ConnectedState {
4343
var backoffIterator: ConnectionBackoffIterator?
4444
var reconnect: Reconnect
4545
var candidate: Channel
46-
var readyChannelMuxPromise: EventLoopPromise<NIOHTTP2Handler.StreamMultiplexer>
47-
var multiplexer: NIOHTTP2Handler.StreamMultiplexer
46+
var readyChannelMuxPromise: EventLoopPromise<HTTP2StreamMultiplexer>
47+
var multiplexer: HTTP2StreamMultiplexer
4848
var error: Error?
4949

50-
init(
51-
from state: ConnectingState,
52-
candidate: Channel,
53-
multiplexer: NIOHTTP2Handler.StreamMultiplexer
54-
) {
50+
init(from state: ConnectingState, candidate: Channel, multiplexer: HTTP2StreamMultiplexer) {
5551
self.backoffIterator = state.backoffIterator
5652
self.reconnect = state.reconnect
5753
self.candidate = candidate
@@ -62,7 +58,7 @@ internal final class ConnectionManager: @unchecked Sendable {
6258

6359
internal struct ReadyState {
6460
var channel: Channel
65-
var multiplexer: NIOHTTP2Handler.StreamMultiplexer
61+
var multiplexer: HTTP2StreamMultiplexer
6662
var error: Error?
6763

6864
init(from state: ConnectedState) {
@@ -73,7 +69,7 @@ internal final class ConnectionManager: @unchecked Sendable {
7369

7470
internal struct TransientFailureState {
7571
var backoffIterator: ConnectionBackoffIterator?
76-
var readyChannelMuxPromise: EventLoopPromise<NIOHTTP2Handler.StreamMultiplexer>
72+
var readyChannelMuxPromise: EventLoopPromise<HTTP2StreamMultiplexer>
7773
var scheduled: Scheduled<Void>
7874
var reason: Error
7975

@@ -256,8 +252,8 @@ internal final class ConnectionManager: @unchecked Sendable {
256252
}
257253
}
258254

259-
/// Returns the `NIOHTTP2Handler.StreamMultiplexer` from the 'ready' state or `nil` if it is not available.
260-
private var multiplexer: NIOHTTP2Handler.StreamMultiplexer? {
255+
/// Returns the `HTTP2StreamMultiplexer` from the 'ready' state or `nil` if it is not available.
256+
private var multiplexer: HTTP2StreamMultiplexer? {
261257
self.eventLoop.assertInEventLoop()
262258
switch self.state {
263259
case let .ready(state):
@@ -365,8 +361,8 @@ internal final class ConnectionManager: @unchecked Sendable {
365361
/// Get the multiplexer from the underlying channel handling gRPC calls.
366362
/// if the `ConnectionManager` was configured to be `fastFailure` this will have
367363
/// one chance to connect - if not reconnections are managed here.
368-
internal func getHTTP2Multiplexer() -> EventLoopFuture<NIOHTTP2Handler.StreamMultiplexer> {
369-
func getHTTP2Multiplexer0() -> EventLoopFuture<NIOHTTP2Handler.StreamMultiplexer> {
364+
internal func getHTTP2Multiplexer() -> EventLoopFuture<HTTP2StreamMultiplexer> {
365+
func getHTTP2Multiplexer0() -> EventLoopFuture<HTTP2StreamMultiplexer> {
370366
switch self.callStartBehavior {
371367
case .waitsForConnectivity:
372368
return self.getHTTP2MultiplexerPatient()
@@ -386,8 +382,8 @@ internal final class ConnectionManager: @unchecked Sendable {
386382

387383
/// Returns a future for the multiplexer which succeeded when the channel is connected.
388384
/// Reconnects are handled if necessary.
389-
private func getHTTP2MultiplexerPatient() -> EventLoopFuture<NIOHTTP2Handler.StreamMultiplexer> {
390-
let multiplexer: EventLoopFuture<NIOHTTP2Handler.StreamMultiplexer>
385+
private func getHTTP2MultiplexerPatient() -> EventLoopFuture<HTTP2StreamMultiplexer> {
386+
let multiplexer: EventLoopFuture<HTTP2StreamMultiplexer>
391387

392388
switch self.state {
393389
case .idle:
@@ -425,12 +421,11 @@ internal final class ConnectionManager: @unchecked Sendable {
425421
/// attempt, or if the state is 'idle' returns the future for the next connection attempt.
426422
///
427423
/// Note: if the state is 'transientFailure' or 'shutdown' then a failed future will be returned.
428-
private func getHTTP2MultiplexerOptimistic()
429-
-> EventLoopFuture<NIOHTTP2Handler.StreamMultiplexer> {
424+
private func getHTTP2MultiplexerOptimistic() -> EventLoopFuture<HTTP2StreamMultiplexer> {
430425
// `getHTTP2Multiplexer` makes sure we're on the event loop but let's just be sure.
431426
self.eventLoop.preconditionInEventLoop()
432427

433-
let muxFuture: EventLoopFuture<NIOHTTP2Handler.StreamMultiplexer> = { () in
428+
let muxFuture: EventLoopFuture<HTTP2StreamMultiplexer> = { () in
434429
switch self.state {
435430
case .idle:
436431
self.startConnecting()
@@ -661,7 +656,7 @@ internal final class ConnectionManager: @unchecked Sendable {
661656
}
662657

663658
/// The connecting channel became `active`. Must be called on the `EventLoop`.
664-
internal func channelActive(channel: Channel, multiplexer: NIOHTTP2Handler.StreamMultiplexer) {
659+
internal func channelActive(channel: Channel, multiplexer: HTTP2StreamMultiplexer) {
665660
self.eventLoop.preconditionInEventLoop()
666661
self.logger.debug("activating connection", metadata: [
667662
"connectivity_state": "\(self.state.label)",
@@ -978,7 +973,7 @@ extension ConnectionManager {
978973

979974
private func startConnecting(
980975
backoffIterator: ConnectionBackoffIterator?,
981-
muxPromise: EventLoopPromise<NIOHTTP2Handler.StreamMultiplexer>
976+
muxPromise: EventLoopPromise<HTTP2StreamMultiplexer>
982977
) {
983978
let timeoutAndBackoff = backoffIterator?.next()
984979

@@ -1065,7 +1060,7 @@ extension ConnectionManager {
10651060

10661061
/// Returns the `multiplexer` from a connection in the `ready` state or `nil` if it is any
10671062
/// other state.
1068-
internal var multiplexer: NIOHTTP2Handler.StreamMultiplexer? {
1063+
internal var multiplexer: HTTP2StreamMultiplexer? {
10691064
return self.manager.multiplexer
10701065
}
10711066

‎Sources/GRPC/ConnectionPool/ConnectionPool+PerConnectionState.swift

+3-3
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ extension ConnectionPool {
5858
}
5959

6060
@usableFromInline
61-
var multiplexer: NIOHTTP2Handler.StreamMultiplexer
61+
var multiplexer: HTTP2StreamMultiplexer
6262
/// Maximum number of available streams.
6363
@usableFromInline
6464
var maxAvailable: Int
@@ -78,7 +78,7 @@ extension ConnectionPool {
7878

7979
/// Increment the reserved streams and return the multiplexer.
8080
@usableFromInline
81-
mutating func reserve() -> NIOHTTP2Handler.StreamMultiplexer {
81+
mutating func reserve() -> HTTP2StreamMultiplexer {
8282
assert(!self.isQuiescing)
8383
self.reserved += 1
8484
return self.multiplexer
@@ -132,7 +132,7 @@ extension ConnectionPool {
132132
///
133133
/// The result may be safely unwrapped if `self.availableStreams > 0` when reserving a stream.
134134
@usableFromInline
135-
internal mutating func reserveStream() -> NIOHTTP2Handler.StreamMultiplexer? {
135+
internal mutating func reserveStream() -> HTTP2StreamMultiplexer? {
136136
return self._availability?.reserve()
137137
}
138138

‎Sources/GRPC/ConnectionPool/ConnectionPool+Waiter.swift

+3-3
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ extension ConnectionPool {
3030

3131
/// The channel initializer.
3232
@usableFromInline
33-
internal let _channelInitializer: @Sendable (Channel) -> EventLoopFuture<Void>
33+
internal let _channelInitializer: (Channel) -> EventLoopFuture<Void>
3434

3535
/// The deadline at which the timeout is scheduled.
3636
@usableFromInline
@@ -51,7 +51,7 @@ extension ConnectionPool {
5151
internal init(
5252
deadline: NIODeadline,
5353
promise: EventLoopPromise<Channel>,
54-
channelInitializer: @escaping @Sendable (Channel) -> EventLoopFuture<Void>
54+
channelInitializer: @escaping (Channel) -> EventLoopFuture<Void>
5555
) {
5656
self._deadline = deadline
5757
self._promise = promise
@@ -83,7 +83,7 @@ extension ConnectionPool {
8383

8484
/// Succeed the waiter with the given multiplexer.
8585
@usableFromInline
86-
internal func succeed(with multiplexer: NIOHTTP2Handler.StreamMultiplexer) {
86+
internal func succeed(with multiplexer: HTTP2StreamMultiplexer) {
8787
self._scheduledTimeout?.cancel()
8888
self._scheduledTimeout = nil
8989
multiplexer.createStreamChannel(promise: self._promise, self._channelInitializer)

0 commit comments

Comments
 (0)