Skip to content

Commit 979f431

Browse files
authored
Optimize HTTP2ToHTTP1 client codec to reduce empty data frames (#535)
The codec now caches the most recent outbound frame and only writes it on flush or when receiving `.end`. This lets us set `endStream=true` on the final data or headers frame instead of sending a separate empty data frame. Changes: - Introduced `PendingFrameWrite` to cache frame payload and promise - Modified `processOutboundData` to return optional frames instead of writing immediately - Added `flush()` method to both client codecs to handle cached writes - Promise merging ensures correct completion notification when combining frames
1 parent c1bb956 commit 979f431

File tree

8 files changed

+276
-42
lines changed

8 files changed

+276
-42
lines changed

.licenseignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,3 +45,4 @@ dev/update-benchmark-thresholds
4545
FuzzTesting/FailCases/*
4646
Tests/hpack-test-case/*
4747
.flake8
48+
dev/alloc-limits-from-test-output

IntegrationTests/tests_01_allocation_counters/Thresholds/6.0.json

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@
33
"1k_requests_inline_noninterleaved": 29100,
44
"1k_requests_interleaved": 36150,
55
"1k_requests_noninterleaved": 35100,
6-
"client_server_h1_request_response": 284050,
7-
"client_server_h1_request_response_inline": 269050,
6+
"client_server_h1_request_response": 275050,
7+
"client_server_h1_request_response_inline": 263050,
88
"client_server_request_response": 253050,
99
"client_server_request_response_inline": 244050,
1010
"client_server_request_response_many": 1198050,
@@ -18,6 +18,6 @@
1818
"get_100000_headers_canonical_form_trimming_whitespace_from_long_string": 300050,
1919
"get_100000_headers_canonical_form_trimming_whitespace_from_short_string": 200050,
2020
"hpack_decoding": 5050,
21-
"stream_teardown_100_concurrent": 253550,
21+
"stream_teardown_100_concurrent": 253250,
2222
"stream_teardown_100_concurrent_inline": 252350
2323
}

IntegrationTests/tests_01_allocation_counters/Thresholds/6.1.json

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@
33
"1k_requests_inline_noninterleaved": 29100,
44
"1k_requests_interleaved": 36150,
55
"1k_requests_noninterleaved": 35100,
6-
"client_server_h1_request_response": 284050,
7-
"client_server_h1_request_response_inline": 269050,
6+
"client_server_h1_request_response": 275050,
7+
"client_server_h1_request_response_inline": 263050,
88
"client_server_request_response": 253050,
99
"client_server_request_response_inline": 244050,
1010
"client_server_request_response_many": 1198050,
@@ -18,6 +18,6 @@
1818
"get_100000_headers_canonical_form_trimming_whitespace_from_long_string": 300050,
1919
"get_100000_headers_canonical_form_trimming_whitespace_from_short_string": 200050,
2020
"hpack_decoding": 5050,
21-
"stream_teardown_100_concurrent": 253550,
21+
"stream_teardown_100_concurrent": 253250,
2222
"stream_teardown_100_concurrent_inline": 252350
2323
}

IntegrationTests/tests_01_allocation_counters/Thresholds/6.2.json

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@
33
"1k_requests_inline_noninterleaved": 29100,
44
"1k_requests_interleaved": 36150,
55
"1k_requests_noninterleaved": 35100,
6-
"client_server_h1_request_response": 284050,
7-
"client_server_h1_request_response_inline": 269050,
6+
"client_server_h1_request_response": 275050,
7+
"client_server_h1_request_response_inline": 263050,
88
"client_server_request_response": 253050,
99
"client_server_request_response_inline": 244050,
1010
"client_server_request_response_many": 1198050,
@@ -18,6 +18,6 @@
1818
"get_100000_headers_canonical_form_trimming_whitespace_from_long_string": 300050,
1919
"get_100000_headers_canonical_form_trimming_whitespace_from_short_string": 200050,
2020
"hpack_decoding": 5050,
21-
"stream_teardown_100_concurrent": 253550,
21+
"stream_teardown_100_concurrent": 253250,
2222
"stream_teardown_100_concurrent_inline": 252350
2323
}

IntegrationTests/tests_01_allocation_counters/Thresholds/nightly-main.json

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@
33
"1k_requests_inline_noninterleaved": 29100,
44
"1k_requests_interleaved": 36150,
55
"1k_requests_noninterleaved": 35100,
6-
"client_server_h1_request_response": 284050,
7-
"client_server_h1_request_response_inline": 269050,
6+
"client_server_h1_request_response": 275050,
7+
"client_server_h1_request_response_inline": 263050,
88
"client_server_request_response": 253050,
99
"client_server_request_response_inline": 244050,
1010
"client_server_request_response_many": 1198050,
@@ -18,6 +18,6 @@
1818
"get_100000_headers_canonical_form_trimming_whitespace_from_long_string": 300050,
1919
"get_100000_headers_canonical_form_trimming_whitespace_from_short_string": 200050,
2020
"hpack_decoding": 5050,
21-
"stream_teardown_100_concurrent": 253550,
21+
"stream_teardown_100_concurrent": 253250,
2222
"stream_teardown_100_concurrent_inline": 252350
2323
}

IntegrationTests/tests_01_allocation_counters/Thresholds/nightly-next.json

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@
33
"1k_requests_inline_noninterleaved": 29100,
44
"1k_requests_interleaved": 36150,
55
"1k_requests_noninterleaved": 35100,
6-
"client_server_h1_request_response": 284050,
7-
"client_server_h1_request_response_inline": 269050,
6+
"client_server_h1_request_response": 275050,
7+
"client_server_h1_request_response_inline": 263050,
88
"client_server_request_response": 253050,
99
"client_server_request_response_inline": 244050,
1010
"client_server_request_response_many": 1198050,
@@ -18,6 +18,6 @@
1818
"get_100000_headers_canonical_form_trimming_whitespace_from_long_string": 300050,
1919
"get_100000_headers_canonical_form_trimming_whitespace_from_short_string": 200050,
2020
"hpack_decoding": 5050,
21-
"stream_teardown_100_concurrent": 253550,
21+
"stream_teardown_100_concurrent": 253250,
2222
"stream_teardown_100_concurrent_inline": 252350
2323
}

Sources/NIOHTTP2/HTTP2ToHTTP1Codec.swift

Lines changed: 125 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
//
33
// This source file is part of the SwiftNIO open source project
44
//
5-
// Copyright (c) 2017-2021 Apple Inc. and the SwiftNIO project authors
5+
// Copyright (c) 2017-2026 Apple Inc. and the SwiftNIO project authors
66
// Licensed under Apache License v2.0
77
//
88
// See LICENSE.txt for license information
@@ -23,9 +23,23 @@ private struct BaseClientCodec {
2323
private let normalizeHTTPHeaders: Bool
2424

2525
private var headerStateMachine: HTTP2HeadersStateMachine = HTTP2HeadersStateMachine(mode: .client)
26-
2726
private var outgoingHTTP1RequestHead: HTTPRequestHead?
2827

28+
struct PendingFrameWrite {
29+
var frame: HTTP2Frame.FramePayload
30+
var promise: EventLoopPromise<Void>?
31+
32+
init(_ frame: HTTP2Frame.FramePayload, _ promise: EventLoopPromise<Void>?) {
33+
self.frame = frame
34+
self.promise = promise
35+
}
36+
}
37+
38+
/// Caches the most recent outbound frame until flush or .end is received.
39+
/// This allows us to set endStream=true on the final data/headers frame
40+
/// instead of sending a separate empty data frame just to signal stream closure.
41+
private var pendingFrameWrite: PendingFrameWrite?
42+
2943
/// Initializes a `BaseClientCodec`.
3044
///
3145
/// - Parameters:
@@ -102,8 +116,8 @@ private struct BaseClientCodec {
102116

103117
mutating func processOutboundData(
104118
_ data: HTTPClientRequestPart,
105-
allocator: ByteBufferAllocator
106-
) throws -> HTTP2Frame.FramePayload {
119+
promise: EventLoopPromise<Void>?
120+
) throws -> (first: PendingFrameWrite?, second: PendingFrameWrite?) {
107121
switch data {
108122
case .head(let head):
109123
precondition(self.outgoingHTTP1RequestHead == nil, "Only a single HTTP request allowed per HTTP2 stream")
@@ -115,25 +129,73 @@ private struct BaseClientCodec {
115129
normalizeHTTPHeaders: self.normalizeHTTPHeaders
116130
)
117131
)
118-
return .headers(headerContent)
132+
self.pendingFrameWrite = .init(.headers(headerContent), promise)
133+
return (nil, nil)
134+
119135
case .body(let body):
120-
return .data(HTTP2Frame.FramePayload.Data(data: body))
136+
let cached = self.pendingFrameWrite
137+
self.pendingFrameWrite = .init(.data(HTTP2Frame.FramePayload.Data(data: body)), promise)
138+
return (cached, nil)
139+
121140
case .end(let trailers):
122-
if let trailers = trailers {
123-
return .headers(
124-
.init(
125-
headers: HPACKHeaders(
126-
httpHeaders: trailers,
127-
normalizeHTTPHeaders: self.normalizeHTTPHeaders
128-
),
129-
endStream: true
130-
)
141+
defer { self.pendingFrameWrite = nil }
142+
143+
switch (self.pendingFrameWrite?.frame, trailers) {
144+
case (.none, .none):
145+
return (.init(.data(.init(data: .byteBuffer(ByteBuffer()), endStream: true)), promise), nil)
146+
147+
case (.none, .some(let trailers)):
148+
return (.init(self.makeH2TrailerFramePayload(trailers), promise), nil)
149+
150+
case (.data(var data), .none):
151+
data.endStream = true
152+
var pendingPromise = self.pendingFrameWrite!.promise
153+
pendingPromise.setOrCascade(to: promise)
154+
return (.init(.data(data), pendingPromise), nil)
155+
156+
case (.data(let data), .some(let trailers)):
157+
let trailerFrame = self.makeH2TrailerFramePayload(trailers)
158+
return (
159+
.init(.data(data), self.pendingFrameWrite!.promise),
160+
.init(trailerFrame, promise)
131161
)
132-
} else {
133-
return .data(.init(data: .byteBuffer(allocator.buffer(capacity: 0)), endStream: true))
162+
163+
case (.headers(var headers), .none):
164+
headers.endStream = true
165+
var pendingPromise = self.pendingFrameWrite!.promise
166+
pendingPromise.setOrCascade(to: promise)
167+
return (.init(.headers(headers), pendingPromise), nil)
168+
169+
case (.headers(let headers), .some(let trailers)):
170+
let trailers = self.makeH2TrailerFramePayload(trailers)
171+
return (
172+
.init(.headers(headers), self.pendingFrameWrite!.promise),
173+
.init(trailers, promise)
174+
)
175+
176+
case (.priority, _), (.rstStream, _), (.settings, _), (.pushPromise, _), (.ping, _), (.goAway, _),
177+
(.windowUpdate, _), (.alternativeService, _), (.origin, _):
178+
fatalError("Only header and data frames are cached here")
134179
}
135180
}
136181
}
182+
183+
mutating func clearCache() -> PendingFrameWrite? {
184+
defer { self.pendingFrameWrite = nil }
185+
return self.pendingFrameWrite
186+
}
187+
188+
private func makeH2TrailerFramePayload(_ trailers: HTTPHeaders) -> HTTP2Frame.FramePayload {
189+
HTTP2Frame.FramePayload.headers(
190+
.init(
191+
headers: HPACKHeaders(
192+
httpHeaders: trailers,
193+
normalizeHTTPHeaders: self.normalizeHTTPHeaders
194+
),
195+
endStream: true
196+
)
197+
)
198+
}
137199
}
138200

139201
/// A simple channel handler that translates HTTP/2 concepts into HTTP/1 data types,
@@ -199,17 +261,35 @@ public final class HTTP2ToHTTP1ClientCodec: ChannelInboundHandler, ChannelOutbou
199261
let responsePart = self.unwrapOutboundIn(data)
200262

201263
do {
202-
let transformedPayload = try self.baseCodec.processOutboundData(
203-
responsePart,
204-
allocator: context.channel.allocator
205-
)
206-
let part = HTTP2Frame(streamID: self.streamID, payload: transformedPayload)
207-
context.write(self.wrapOutboundOut(part), promise: promise)
264+
let (first, second) = try self.baseCodec.processOutboundData(responsePart, promise: promise)
265+
if let first = first {
266+
let part = HTTP2Frame(streamID: self.streamID, payload: first.frame)
267+
context.write(self.wrapOutboundOut(part), promise: first.promise)
268+
if let second = second {
269+
let part = HTTP2Frame(streamID: self.streamID, payload: second.frame)
270+
context.write(self.wrapOutboundOut(part), promise: second.promise)
271+
}
272+
}
208273
} catch {
209274
promise?.fail(error)
210275
context.fireErrorCaught(error)
211276
}
212277
}
278+
279+
public func flush(context: ChannelHandlerContext) {
280+
if let pending = self.baseCodec.clearCache() {
281+
let part = HTTP2Frame(streamID: self.streamID, payload: pending.frame)
282+
context.write(self.wrapOutboundOut(part), promise: pending.promise)
283+
}
284+
context.flush()
285+
}
286+
287+
public func errorCaught(context: ChannelHandlerContext, error: any Error) {
288+
if let pending = self.baseCodec.clearCache() {
289+
pending.promise?.fail(error)
290+
}
291+
context.fireErrorCaught(error)
292+
}
213293
}
214294

215295
@available(*, unavailable)
@@ -270,16 +350,35 @@ public final class HTTP2FramePayloadToHTTP1ClientCodec: ChannelInboundHandler, C
270350
let requestPart = self.unwrapOutboundIn(data)
271351

272352
do {
273-
let transformedPayload = try self.baseCodec.processOutboundData(
353+
let (first, second) = try self.baseCodec.processOutboundData(
274354
requestPart,
275-
allocator: context.channel.allocator
355+
promise: promise
276356
)
277-
context.write(self.wrapOutboundOut(transformedPayload), promise: promise)
357+
if let first {
358+
context.write(self.wrapOutboundOut(first.frame), promise: first.promise)
359+
if let second {
360+
context.write(self.wrapOutboundOut(second.frame), promise: second.promise)
361+
}
362+
}
278363
} catch {
279364
promise?.fail(error)
280365
context.fireErrorCaught(error)
281366
}
282367
}
368+
369+
public func flush(context: ChannelHandlerContext) {
370+
if let pending = self.baseCodec.clearCache() {
371+
context.write(self.wrapOutboundOut(pending.frame), promise: pending.promise)
372+
}
373+
context.flush()
374+
}
375+
376+
public func errorCaught(context: ChannelHandlerContext, error: any Error) {
377+
if let pending = self.baseCodec.clearCache() {
378+
pending.promise?.fail(error)
379+
}
380+
context.fireErrorCaught(error)
381+
}
283382
}
284383

285384
@available(*, unavailable)

0 commit comments

Comments
 (0)