Make HEADERS frame payload non-indirect (#428)

Lukasa · glbrntt · gjcairo · web-flow · commit a2b4d284f4c0 · 2024-07-17T09:00:49.000-07:00
* Make HEADERS frame payload non-indirect

Motivation:

In previous patches we shrank the size of HTTP2Frame by
making various data types indirect in the frame payload.
This included HEADERS, which is unfortunte as HEADERS frames
are quite common.

This patch changes the layout of the HEADERS frame to remove the
indirect case.

Modifications:

- Move END_STREAM into an OptionSet.
- Turn the two optional bits into flags in the aforementioned
    OptionSet
- Replace the properties with computed properties.
- Remove the indirect case

Result:

HEADERS frames are cheaper.

* Update alloc limits

---------

Co-authored-by: George Barnett &lt;gbarnett@apple.com&gt;
Co-authored-by: Gustavo Cairo &lt;gus.cairo@apple.com&gt;
diff --git a/Sources/NIOHTTP2/HTTP2Frame.swift b/Sources/NIOHTTP2/HTTP2Frame.swift
@@ -27,9 +27,15 @@ public struct HTTP2Frame: Sendable {
 
     /// Stream priority data, used in PRIORITY frames and optionally in HEADERS frames.
     public struct StreamPriorityData: Equatable, Hashable, Sendable {
-        public var exclusive: Bool
         public var dependency: HTTP2StreamID
+        public var exclusive: Bool
         public var weight: UInt8
+
+        public init(exclusive: Bool, dependency: HTTP2StreamID, weight: UInt8) {
+            self.exclusive = exclusive
+            self.dependency = dependency
+            self.weight = weight
+        }
     }
 
     /// Frame-type-specific payload data.
@@ -46,7 +52,7 @@ public struct HTTP2Frame: Sendable {
         /// frames into a single ``HTTP2Frame/FramePayload/headers(_:)`` instance.
         ///
         /// See [RFC 7540 § 6.2](https://httpwg.org/specs/rfc7540.html#rfc.section.6.2).
-        indirect case headers(Headers)
+        case headers(Headers)
         
         /// A `PRIORITY` frame, used to change priority and dependency ordering among
         /// streams.
@@ -212,35 +218,106 @@ public struct HTTP2Frame: Sendable {
 
         /// The payload of a `HEADERS` frame.
         public struct Headers: Sendable {
+            /// An OptionSet that keeps track of the various boolean flags in HEADERS.
+            /// It allows us to elide having our two optionals by keeping track of their
+            /// optionality here, which frees up a byte and keeps the total size of
+            /// HTTP2Frame at 24 bytes.
+            @usableFromInline
+            struct Booleans: OptionSet, Sendable, Hashable {
+                @usableFromInline
+                var rawValue: UInt8
+
+                @inlinable
+                init(rawValue: UInt8) {
+                    self.rawValue = rawValue
+                }
+
+                @usableFromInline static let endStream = Booleans(rawValue: 1 << 0)
+                @usableFromInline static let priorityPresent = Booleans(rawValue: 1 << 1)
+                @usableFromInline static let paddingPresent = Booleans(rawValue: 1 << 2)
+            }
+
             /// The decoded header block belonging to this `HEADERS` frame.
             public var headers: HPACKHeaders
 
+            /// Stream priority data.
+            ///
+            /// If `.priorityPresent` is not set in our boolean flags, this value is ignored.
+            @usableFromInline
+            var _priorityData: StreamPriorityData
+
+            /// The number of padding bytes in this frame.
+            ///
+            /// If `.paddingPresent` is not set in our boolean flags, this value is ignored.
+            @usableFromInline
+            var _paddingBytes: UInt8
+
+            /// Boolean flags that control the presence of other values in this frame.
+            @usableFromInline
+            var booleans: Booleans
+
             /// The stream priority data transmitted on this frame, if any.
-            public var priorityData: StreamPriorityData?
+            @inlinable
+            public var priorityData: StreamPriorityData? {
+                get {
+                    if self.booleans.contains(.priorityPresent) {
+                        return self._priorityData
+                    } else {
+                        return nil
+                    }
+                }
+                set {
+                    if let newValue = newValue {
+                        self._priorityData = newValue
+                        self.booleans.insert(.priorityPresent)
+                    } else {
+                        self.booleans.remove(.priorityPresent)
+                    }
+                }
+            }
 
             /// The value of the `END_STREAM` flag on this frame.
-            public var endStream: Bool
-
-            /// The underlying number of padding bytes. If nil, no padding is present.
-            internal private(set) var _paddingBytes: UInt8?
+            @inlinable
+            public var endStream: Bool {
+                get {
+                    self.booleans.contains(.endStream)
+                }
+                set {
+                    if newValue {
+                        self.booleans.insert(.endStream)
+                    } else {
+                        self.booleans.remove(.endStream)
+                    }
+                }
+            }
 
             /// The number of padding bytes sent in this frame. If nil, this frame was not padded.
+            @inlinable
             public var paddingBytes: Int? {
                 get {
-                    return self._paddingBytes.map { Int($0) }
+                    if self.booleans.contains(.paddingPresent) {
+                        return Int(self._paddingBytes)
+                    } else {
+                        return nil
+                    }
                 }
                 set {
                     if let newValue = newValue {
                         precondition(newValue >= 0 && newValue <= Int(UInt8.max), "Invalid padding byte length: \(newValue)")
                         self._paddingBytes = UInt8(newValue)
+                        self.booleans.insert(.paddingPresent)
                     } else {
-                        self._paddingBytes = nil
+                        self.booleans.remove(.paddingPresent)
                     }
                 }
             }
 
             public init(headers: HPACKHeaders, priorityData: StreamPriorityData? = nil, endStream: Bool = false, paddingBytes: Int? = nil) {
                 self.headers = headers
+                self.booleans = .init(rawValue: 0)
+                self._paddingBytes = 0
+                self._priorityData = StreamPriorityData(exclusive: false, dependency: .rootStream, weight: 0)
+
                 self.priorityData = priorityData
                 self.endStream = endStream
                 self.paddingBytes = paddingBytes
diff --git a/Tests/NIOHTTP2Tests/HTTP2FrameParserTests.swift b/Tests/NIOHTTP2Tests/HTTP2FrameParserTests.swift
@@ -2584,4 +2584,8 @@ class HTTP2FrameParserTests: XCTestCase {
                                        payload: .data(.init(data: .byteBuffer(payload), endStream: true)))
         try assertReadsFrame(from: greaseBuf, matching: expectedFrame, expectedFlowControlledLength: 13)
     }
+
+    func testFrameFitsIntoAnExistentialContainer() throws {
+        XCTAssertLessThanOrEqual(MemoryLayout<HTTP2Frame>.size, 24)
+    }
 }
diff --git a/docker/docker-compose.2204.510.yaml b/docker/docker-compose.2204.510.yaml
@@ -28,16 +28,16 @@ services:
   test:
     image: swift-nio-http2:22.04-5.10
     environment:
-      - MAX_ALLOCS_ALLOWED_1k_requests_inline_interleaved=33150
-      - MAX_ALLOCS_ALLOWED_1k_requests_inline_noninterleaved=32100
-      - MAX_ALLOCS_ALLOWED_1k_requests_interleaved=39150
-      - MAX_ALLOCS_ALLOWED_1k_requests_noninterleaved=38100
-      - MAX_ALLOCS_ALLOWED_client_server_h1_request_response=284050
-      - MAX_ALLOCS_ALLOWED_client_server_h1_request_response_inline=269050
-      - MAX_ALLOCS_ALLOWED_client_server_request_response=253050
-      - MAX_ALLOCS_ALLOWED_client_server_request_response_inline=244050
-      - MAX_ALLOCS_ALLOWED_client_server_request_response_many=1198050
-      - MAX_ALLOCS_ALLOWED_client_server_request_response_many_inline=889050
+      - MAX_ALLOCS_ALLOWED_1k_requests_inline_interleaved=31150
+      - MAX_ALLOCS_ALLOWED_1k_requests_inline_noninterleaved=30100
+      - MAX_ALLOCS_ALLOWED_1k_requests_interleaved=37150
+      - MAX_ALLOCS_ALLOWED_1k_requests_noninterleaved=36100
+      - MAX_ALLOCS_ALLOWED_client_server_h1_request_response=280050
+      - MAX_ALLOCS_ALLOWED_client_server_h1_request_response_inline=265050
+      - MAX_ALLOCS_ALLOWED_client_server_request_response=249050
+      - MAX_ALLOCS_ALLOWED_client_server_request_response_inline=240050
+      - MAX_ALLOCS_ALLOWED_client_server_request_response_many=1194050
+      - MAX_ALLOCS_ALLOWED_client_server_request_response_many_inline=885050
       - MAX_ALLOCS_ALLOWED_create_client_stream_channel=37050
       - MAX_ALLOCS_ALLOWED_create_client_stream_channel_inline=37050
       - MAX_ALLOCS_ALLOWED_create_client_stream_channel_inline_no_promise_based_API=37050
@@ -47,8 +47,8 @@ services:
       - MAX_ALLOCS_ALLOWED_get_100000_headers_canonical_form_trimming_whitespace_from_long_string=300050
       - MAX_ALLOCS_ALLOWED_get_100000_headers_canonical_form_trimming_whitespace_from_short_string=200050
       - MAX_ALLOCS_ALLOWED_hpack_decoding=5050
-      - MAX_ALLOCS_ALLOWED_stream_teardown_100_concurrent=282650
-      - MAX_ALLOCS_ALLOWED_stream_teardown_100_concurrent_inline=281750
+      - MAX_ALLOCS_ALLOWED_stream_teardown_100_concurrent=272650
+      - MAX_ALLOCS_ALLOWED_stream_teardown_100_concurrent_inline=271750
       - IMPORT_CHECK_ARG=--explicit-target-dependency-import-check error
 
   shell:
diff --git a/docker/docker-compose.2204.58.yaml b/docker/docker-compose.2204.58.yaml
@@ -28,16 +28,16 @@ services:
   test:
     image: swift-nio-http2:22.04-5.8
     environment:
-      - MAX_ALLOCS_ALLOWED_1k_requests_inline_interleaved=33150
-      - MAX_ALLOCS_ALLOWED_1k_requests_inline_noninterleaved=32100
-      - MAX_ALLOCS_ALLOWED_1k_requests_interleaved=39150
-      - MAX_ALLOCS_ALLOWED_1k_requests_noninterleaved=38100
-      - MAX_ALLOCS_ALLOWED_client_server_h1_request_response=284050
-      - MAX_ALLOCS_ALLOWED_client_server_h1_request_response_inline=269050
-      - MAX_ALLOCS_ALLOWED_client_server_request_response=253050
-      - MAX_ALLOCS_ALLOWED_client_server_request_response_inline=244050
-      - MAX_ALLOCS_ALLOWED_client_server_request_response_many=1198050
-      - MAX_ALLOCS_ALLOWED_client_server_request_response_many_inline=889050
+      - MAX_ALLOCS_ALLOWED_1k_requests_inline_interleaved=31150
+      - MAX_ALLOCS_ALLOWED_1k_requests_inline_noninterleaved=30100
+      - MAX_ALLOCS_ALLOWED_1k_requests_interleaved=37150
+      - MAX_ALLOCS_ALLOWED_1k_requests_noninterleaved=36100
+      - MAX_ALLOCS_ALLOWED_client_server_h1_request_response=280050
+      - MAX_ALLOCS_ALLOWED_client_server_h1_request_response_inline=265050
+      - MAX_ALLOCS_ALLOWED_client_server_request_response=249050
+      - MAX_ALLOCS_ALLOWED_client_server_request_response_inline=240050
+      - MAX_ALLOCS_ALLOWED_client_server_request_response_many=1194050
+      - MAX_ALLOCS_ALLOWED_client_server_request_response_many_inline=885050
       - MAX_ALLOCS_ALLOWED_create_client_stream_channel=37050
       - MAX_ALLOCS_ALLOWED_create_client_stream_channel_inline=37050
       - MAX_ALLOCS_ALLOWED_create_client_stream_channel_inline_no_promise_based_API=37050
@@ -47,8 +47,8 @@ services:
       - MAX_ALLOCS_ALLOWED_get_100000_headers_canonical_form_trimming_whitespace_from_long_string=300050
       - MAX_ALLOCS_ALLOWED_get_100000_headers_canonical_form_trimming_whitespace_from_short_string=200050
       - MAX_ALLOCS_ALLOWED_hpack_decoding=5050
-      - MAX_ALLOCS_ALLOWED_stream_teardown_100_concurrent=282650
-      - MAX_ALLOCS_ALLOWED_stream_teardown_100_concurrent_inline=281750
+      - MAX_ALLOCS_ALLOWED_stream_teardown_100_concurrent=272650
+      - MAX_ALLOCS_ALLOWED_stream_teardown_100_concurrent_inline=271750
       - IMPORT_CHECK_ARG=--explicit-target-dependency-import-check error
 
   shell:
diff --git a/docker/docker-compose.2204.59.yaml b/docker/docker-compose.2204.59.yaml
@@ -28,16 +28,16 @@ services:
   test:
     image: swift-nio-http2:22.04-5.9
     environment:
-      - MAX_ALLOCS_ALLOWED_1k_requests_inline_interleaved=33150
-      - MAX_ALLOCS_ALLOWED_1k_requests_inline_noninterleaved=32100
-      - MAX_ALLOCS_ALLOWED_1k_requests_interleaved=39150
-      - MAX_ALLOCS_ALLOWED_1k_requests_noninterleaved=38100
-      - MAX_ALLOCS_ALLOWED_client_server_h1_request_response=284050
-      - MAX_ALLOCS_ALLOWED_client_server_h1_request_response_inline=269050
-      - MAX_ALLOCS_ALLOWED_client_server_request_response=253050
-      - MAX_ALLOCS_ALLOWED_client_server_request_response_inline=244050
-      - MAX_ALLOCS_ALLOWED_client_server_request_response_many=1198050
-      - MAX_ALLOCS_ALLOWED_client_server_request_response_many_inline=889050
+      - MAX_ALLOCS_ALLOWED_1k_requests_inline_interleaved=31150
+      - MAX_ALLOCS_ALLOWED_1k_requests_inline_noninterleaved=30100
+      - MAX_ALLOCS_ALLOWED_1k_requests_interleaved=37150
+      - MAX_ALLOCS_ALLOWED_1k_requests_noninterleaved=36100
+      - MAX_ALLOCS_ALLOWED_client_server_h1_request_response=280050
+      - MAX_ALLOCS_ALLOWED_client_server_h1_request_response_inline=265050
+      - MAX_ALLOCS_ALLOWED_client_server_request_response=249050
+      - MAX_ALLOCS_ALLOWED_client_server_request_response_inline=240050
+      - MAX_ALLOCS_ALLOWED_client_server_request_response_many=1194050
+      - MAX_ALLOCS_ALLOWED_client_server_request_response_many_inline=885050
       - MAX_ALLOCS_ALLOWED_create_client_stream_channel=37050
       - MAX_ALLOCS_ALLOWED_create_client_stream_channel_inline=37050
       - MAX_ALLOCS_ALLOWED_create_client_stream_channel_inline_no_promise_based_API=37050
@@ -47,8 +47,8 @@ services:
       - MAX_ALLOCS_ALLOWED_get_100000_headers_canonical_form_trimming_whitespace_from_long_string=300050
       - MAX_ALLOCS_ALLOWED_get_100000_headers_canonical_form_trimming_whitespace_from_short_string=200050
       - MAX_ALLOCS_ALLOWED_hpack_decoding=5050
-      - MAX_ALLOCS_ALLOWED_stream_teardown_100_concurrent=282650
-      - MAX_ALLOCS_ALLOWED_stream_teardown_100_concurrent_inline=281750
+      - MAX_ALLOCS_ALLOWED_stream_teardown_100_concurrent=272650
+      - MAX_ALLOCS_ALLOWED_stream_teardown_100_concurrent_inline=271750
       - IMPORT_CHECK_ARG=--explicit-target-dependency-import-check error
 
   shell:
diff --git a/docker/docker-compose.2204.main.yaml b/docker/docker-compose.2204.main.yaml
@@ -27,16 +27,16 @@ services:
   test:
     image: swift-nio-http2:22.04-main
     environment:
-      - MAX_ALLOCS_ALLOWED_1k_requests_inline_interleaved=33150
-      - MAX_ALLOCS_ALLOWED_1k_requests_inline_noninterleaved=32100
-      - MAX_ALLOCS_ALLOWED_1k_requests_interleaved=39150
-      - MAX_ALLOCS_ALLOWED_1k_requests_noninterleaved=38100
-      - MAX_ALLOCS_ALLOWED_client_server_h1_request_response=284050
-      - MAX_ALLOCS_ALLOWED_client_server_h1_request_response_inline=269050
-      - MAX_ALLOCS_ALLOWED_client_server_request_response=253050
-      - MAX_ALLOCS_ALLOWED_client_server_request_response_inline=244050
-      - MAX_ALLOCS_ALLOWED_client_server_request_response_many=1198050
-      - MAX_ALLOCS_ALLOWED_client_server_request_response_many_inline=889050
+      - MAX_ALLOCS_ALLOWED_1k_requests_inline_interleaved=31150
+      - MAX_ALLOCS_ALLOWED_1k_requests_inline_noninterleaved=30100
+      - MAX_ALLOCS_ALLOWED_1k_requests_interleaved=37150
+      - MAX_ALLOCS_ALLOWED_1k_requests_noninterleaved=36100
+      - MAX_ALLOCS_ALLOWED_client_server_h1_request_response=280050
+      - MAX_ALLOCS_ALLOWED_client_server_h1_request_response_inline=265050
+      - MAX_ALLOCS_ALLOWED_client_server_request_response=249050
+      - MAX_ALLOCS_ALLOWED_client_server_request_response_inline=240050
+      - MAX_ALLOCS_ALLOWED_client_server_request_response_many=1194050
+      - MAX_ALLOCS_ALLOWED_client_server_request_response_many_inline=885050
       - MAX_ALLOCS_ALLOWED_create_client_stream_channel=37050
       - MAX_ALLOCS_ALLOWED_create_client_stream_channel_inline=37050
       - MAX_ALLOCS_ALLOWED_create_client_stream_channel_inline_no_promise_based_API=37050
@@ -46,8 +46,8 @@ services:
       - MAX_ALLOCS_ALLOWED_get_100000_headers_canonical_form_trimming_whitespace_from_long_string=300050
       - MAX_ALLOCS_ALLOWED_get_100000_headers_canonical_form_trimming_whitespace_from_short_string=200050
       - MAX_ALLOCS_ALLOWED_hpack_decoding=5050
-      - MAX_ALLOCS_ALLOWED_stream_teardown_100_concurrent=282650
-      - MAX_ALLOCS_ALLOWED_stream_teardown_100_concurrent_inline=281750
+      - MAX_ALLOCS_ALLOWED_stream_teardown_100_concurrent=272650
+      - MAX_ALLOCS_ALLOWED_stream_teardown_100_concurrent_inline=271750
       - IMPORT_CHECK_ARG=--explicit-target-dependency-import-check error
 
   shell:

Original file line number	Diff line number	Diff line change
`@@ -2584,4 +2584,8 @@ class HTTP2FrameParserTests: XCTestCase {`
`2584`	`2584`	`payload: .data(.init(data: .byteBuffer(payload), endStream: true)))`
`2585`	`2585`	`try assertReadsFrame(from: greaseBuf, matching: expectedFrame, expectedFlowControlledLength: 13)`
`2586`	`2586`	`}`
	`2587`	`+`
	`2588`	`+ func testFrameFitsIntoAnExistentialContainer() throws {`
	`2589`	`+ XCTAssertLessThanOrEqual(MemoryLayout<HTTP2Frame>.size, 24)`
	`2590`	`+ }`
`2587`	`2591`	`}`