Optimize HTTP2ToHTTP1 client codec to reduce empty data frames

.licenseignore

@@ -45,3 +45,4 @@ dev/update-benchmark-thresholds
 FuzzTesting/FailCases/*
 Tests/hpack-test-case/*
 .flake8
+dev/alloc-limits-from-test-output

IntegrationTests/tests_01_allocation_counters/Thresholds/6.0.json

@@ -3,8 +3,8 @@
     "1k_requests_inline_noninterleaved": 29100,
     "1k_requests_interleaved": 36150,
     "1k_requests_noninterleaved": 35100,
-    "client_server_h1_request_response": 284050,
-    "client_server_h1_request_response_inline": 269050,
+    "client_server_h1_request_response": 275050,
+    "client_server_h1_request_response_inline": 263050,
     "client_server_request_response": 253050,
     "client_server_request_response_inline": 244050,
     "client_server_request_response_many": 1198050,
@@ -18,6 +18,6 @@
     "get_100000_headers_canonical_form_trimming_whitespace_from_long_string": 300050,
     "get_100000_headers_canonical_form_trimming_whitespace_from_short_string": 200050,
     "hpack_decoding": 5050,
-    "stream_teardown_100_concurrent": 253550,
+    "stream_teardown_100_concurrent": 253250,
     "stream_teardown_100_concurrent_inline": 252350
 }

IntegrationTests/tests_01_allocation_counters/Thresholds/6.1.json

@@ -3,8 +3,8 @@
     "1k_requests_inline_noninterleaved": 29100,
     "1k_requests_interleaved": 36150,
     "1k_requests_noninterleaved": 35100,
-    "client_server_h1_request_response": 284050,
-    "client_server_h1_request_response_inline": 269050,
+    "client_server_h1_request_response": 275050,
+    "client_server_h1_request_response_inline": 263050,
     "client_server_request_response": 253050,
     "client_server_request_response_inline": 244050,
     "client_server_request_response_many": 1198050,
@@ -18,6 +18,6 @@
     "get_100000_headers_canonical_form_trimming_whitespace_from_long_string": 300050,
     "get_100000_headers_canonical_form_trimming_whitespace_from_short_string": 200050,
     "hpack_decoding": 5050,
-    "stream_teardown_100_concurrent": 253550,
+    "stream_teardown_100_concurrent": 253250,
     "stream_teardown_100_concurrent_inline": 252350
 }

IntegrationTests/tests_01_allocation_counters/Thresholds/6.2.json

@@ -3,8 +3,8 @@
     "1k_requests_inline_noninterleaved": 29100,
     "1k_requests_interleaved": 36150,
     "1k_requests_noninterleaved": 35100,
-    "client_server_h1_request_response": 284050,
-    "client_server_h1_request_response_inline": 269050,
+    "client_server_h1_request_response": 275050,
+    "client_server_h1_request_response_inline": 263050,
     "client_server_request_response": 253050,
     "client_server_request_response_inline": 244050,
     "client_server_request_response_many": 1198050,
@@ -18,6 +18,6 @@
     "get_100000_headers_canonical_form_trimming_whitespace_from_long_string": 300050,
     "get_100000_headers_canonical_form_trimming_whitespace_from_short_string": 200050,
     "hpack_decoding": 5050,
-    "stream_teardown_100_concurrent": 253550,
+    "stream_teardown_100_concurrent": 253250,
     "stream_teardown_100_concurrent_inline": 252350
 }

IntegrationTests/tests_01_allocation_counters/Thresholds/nightly-main.json

@@ -3,8 +3,8 @@
     "1k_requests_inline_noninterleaved": 29100,
     "1k_requests_interleaved": 36150,
     "1k_requests_noninterleaved": 35100,
-    "client_server_h1_request_response": 284050,
-    "client_server_h1_request_response_inline": 269050,
+    "client_server_h1_request_response": 275050,
+    "client_server_h1_request_response_inline": 263050,
     "client_server_request_response": 253050,
     "client_server_request_response_inline": 244050,
     "client_server_request_response_many": 1198050,
@@ -18,6 +18,6 @@
     "get_100000_headers_canonical_form_trimming_whitespace_from_long_string": 300050,
     "get_100000_headers_canonical_form_trimming_whitespace_from_short_string": 200050,
     "hpack_decoding": 5050,
-    "stream_teardown_100_concurrent": 253550,
+    "stream_teardown_100_concurrent": 253250,
     "stream_teardown_100_concurrent_inline": 252350
 }

IntegrationTests/tests_01_allocation_counters/Thresholds/nightly-next.json

@@ -3,8 +3,8 @@
     "1k_requests_inline_noninterleaved": 29100,
     "1k_requests_interleaved": 36150,
     "1k_requests_noninterleaved": 35100,
-    "client_server_h1_request_response": 284050,
-    "client_server_h1_request_response_inline": 269050,
+    "client_server_h1_request_response": 275050,
+    "client_server_h1_request_response_inline": 263050,
     "client_server_request_response": 253050,
     "client_server_request_response_inline": 244050,
     "client_server_request_response_many": 1198050,
@@ -18,6 +18,6 @@
     "get_100000_headers_canonical_form_trimming_whitespace_from_long_string": 300050,
     "get_100000_headers_canonical_form_trimming_whitespace_from_short_string": 200050,
     "hpack_decoding": 5050,
-    "stream_teardown_100_concurrent": 253550,
+    "stream_teardown_100_concurrent": 253250,
     "stream_teardown_100_concurrent_inline": 252350
 }

Sources/NIOHTTP2/HTTP2ToHTTP1Codec.swift

@@ -2,7 +2,7 @@
 //
 // This source file is part of the SwiftNIO open source project
 //
-// Copyright (c) 2017-2021 Apple Inc. and the SwiftNIO project authors
+// Copyright (c) 2017-2026 Apple Inc. and the SwiftNIO project authors
 // Licensed under Apache License v2.0
 //
 // See LICENSE.txt for license information
@@ -23,9 +23,23 @@ private struct BaseClientCodec {
     private let normalizeHTTPHeaders: Bool
 
     private var headerStateMachine: HTTP2HeadersStateMachine = HTTP2HeadersStateMachine(mode: .client)
-
     private var outgoingHTTP1RequestHead: HTTPRequestHead?
 
+    struct PendingFrameWrite {
+        var frame: HTTP2Frame.FramePayload
+        var promise: EventLoopPromise<Void>?
+
+        init(_ frame: HTTP2Frame.FramePayload, _ promise: EventLoopPromise<Void>?) {
+            self.frame = frame
+            self.promise = promise
+        }
+    }
+
+    /// Caches the most recent outbound frame until flush or .end is received.
+    /// This allows us to set endStream=true on the final data/headers frame
+    /// instead of sending a separate empty data frame just to signal stream closure.
+    private var pendingFrameWrite: PendingFrameWrite?
+
     /// Initializes a `BaseClientCodec`.
     ///
     /// - Parameters:
@@ -102,8 +116,8 @@ private struct BaseClientCodec {
 
     mutating func processOutboundData(
         _ data: HTTPClientRequestPart,
-        allocator: ByteBufferAllocator
-    ) throws -> HTTP2Frame.FramePayload {
+        promise: EventLoopPromise<Void>?
+    ) throws -> (first: PendingFrameWrite?, second: PendingFrameWrite?) {
         switch data {
         case .head(let head):
             precondition(self.outgoingHTTP1RequestHead == nil, "Only a single HTTP request allowed per HTTP2 stream")
@@ -115,25 +129,73 @@ private struct BaseClientCodec {
                     normalizeHTTPHeaders: self.normalizeHTTPHeaders
                 )
             )
-            return .headers(headerContent)
+            self.pendingFrameWrite = .init(.headers(headerContent), promise)
+            return (nil, nil)
+
         case .body(let body):
-            return .data(HTTP2Frame.FramePayload.Data(data: body))
+            let cached = self.pendingFrameWrite
+            self.pendingFrameWrite = .init(.data(HTTP2Frame.FramePayload.Data(data: body)), promise)
+            return (cached, nil)
+
         case .end(let trailers):
-            if let trailers = trailers {
-                return .headers(
-                    .init(
-                        headers: HPACKHeaders(
-                            httpHeaders: trailers,
-                            normalizeHTTPHeaders: self.normalizeHTTPHeaders
-                        ),
-                        endStream: true
-                    )
+            defer { self.pendingFrameWrite = nil }
+
+            switch (self.pendingFrameWrite?.frame, trailers) {
+            case (.none, .none):
+                return (.init(.data(.init(data: .byteBuffer(ByteBuffer()), endStream: true)), promise), nil)
+
+            case (.none, .some(let trailers)):
+                return (.init(self.makeH2TrailerFramePayload(trailers), promise), nil)
+
+            case (.data(var data), .none):
+                data.endStream = true
+                var pendingPromise = self.pendingFrameWrite!.promise
+                pendingPromise.setOrCascade(to: promise)
+                return (.init(.data(data), pendingPromise), nil)
+
+            case (.data(let data), .some(let trailers)):
+                let trailerFrame = self.makeH2TrailerFramePayload(trailers)
+                return (
+                    .init(.data(data), self.pendingFrameWrite!.promise),
+                    .init(trailerFrame, promise)
                 )
-            } else {
-                return .data(.init(data: .byteBuffer(allocator.buffer(capacity: 0)), endStream: true))
+
+            case (.headers(var headers), .none):
+                headers.endStream = true
+                var pendingPromise = self.pendingFrameWrite!.promise
+                pendingPromise.setOrCascade(to: promise)
+                return (.init(.headers(headers), pendingPromise), nil)
+
+            case (.headers(let headers), .some(let trailers)):
+                let trailers = self.makeH2TrailerFramePayload(trailers)
+                return (
+                    .init(.headers(headers), self.pendingFrameWrite!.promise),
+                    .init(trailers, promise)
+                )
+
+            case (.priority, _), (.rstStream, _), (.settings, _), (.pushPromise, _), (.ping, _), (.goAway, _),
+                (.windowUpdate, _), (.alternativeService, _), (.origin, _):
+                fatalError("Only header and data frames are cached here")
             }
         }
     }
+
+    mutating func clearCache() -> PendingFrameWrite? {
+        defer { self.pendingFrameWrite = nil }
+        return self.pendingFrameWrite
+    }
+
+    private func makeH2TrailerFramePayload(_ trailers: HTTPHeaders) -> HTTP2Frame.FramePayload {
+        HTTP2Frame.FramePayload.headers(
+            .init(
+                headers: HPACKHeaders(
+                    httpHeaders: trailers,
+                    normalizeHTTPHeaders: self.normalizeHTTPHeaders
+                ),
+                endStream: true
+            )
+        )
+    }
 }
 
 /// A simple channel handler that translates HTTP/2 concepts into HTTP/1 data types,
@@ -199,17 +261,35 @@ public final class HTTP2ToHTTP1ClientCodec: ChannelInboundHandler, ChannelOutbou
         let responsePart = self.unwrapOutboundIn(data)
 
         do {
-            let transformedPayload = try self.baseCodec.processOutboundData(
-                responsePart,
-                allocator: context.channel.allocator
-            )
-            let part = HTTP2Frame(streamID: self.streamID, payload: transformedPayload)
-            context.write(self.wrapOutboundOut(part), promise: promise)
+            let (first, second) = try self.baseCodec.processOutboundData(responsePart, promise: promise)
+            if let first = first {
+                let part = HTTP2Frame(streamID: self.streamID, payload: first.frame)
+                context.write(self.wrapOutboundOut(part), promise: first.promise)
+                if let second = second {
+                    let part = HTTP2Frame(streamID: self.streamID, payload: second.frame)
+                    context.write(self.wrapOutboundOut(part), promise: second.promise)
+                }
+            }
         } catch {
             promise?.fail(error)
             context.fireErrorCaught(error)
         }
     }
+
+    public func flush(context: ChannelHandlerContext) {
+        if let pending = self.baseCodec.clearCache() {
+            let part = HTTP2Frame(streamID: self.streamID, payload: pending.frame)
+            context.write(self.wrapOutboundOut(part), promise: pending.promise)
+        }
+        context.flush()
+    }
+
+    public func errorCaught(context: ChannelHandlerContext, error: any Error) {
+        if let pending = self.baseCodec.clearCache() {
+            pending.promise?.fail(error)
+        }
+        context.fireErrorCaught(error)
+    }
 }
 
 @available(*, unavailable)
@@ -270,16 +350,35 @@ public final class HTTP2FramePayloadToHTTP1ClientCodec: ChannelInboundHandler, C
         let requestPart = self.unwrapOutboundIn(data)
 
         do {
-            let transformedPayload = try self.baseCodec.processOutboundData(
+            let (first, second) = try self.baseCodec.processOutboundData(
                 requestPart,
-                allocator: context.channel.allocator
+                promise: promise
             )
-            context.write(self.wrapOutboundOut(transformedPayload), promise: promise)
+            if let first {
+                context.write(self.wrapOutboundOut(first.frame), promise: first.promise)
+                if let second {
+                    context.write(self.wrapOutboundOut(second.frame), promise: second.promise)
+                }
+            }
         } catch {
             promise?.fail(error)
             context.fireErrorCaught(error)
         }
     }
+
+    public func flush(context: ChannelHandlerContext) {
+        if let pending = self.baseCodec.clearCache() {
+            context.write(self.wrapOutboundOut(pending.frame), promise: pending.promise)
+        }
+        context.flush()
+    }
+
+    public func errorCaught(context: ChannelHandlerContext, error: any Error) {
+        if let pending = self.baseCodec.clearCache() {
+            pending.promise?.fail(error)
+        }
+        context.fireErrorCaught(error)
+    }
 }
 
 @available(*, unavailable)