Improve HTTPHeader normalisation performance for large inputs (#347)

Motivation:

Header normalisation is expensive when the input headers contain a large
number of connection header values.

Modifications:

- When the headers to normalise contain more than 32 connection header
  values use a set to provide constant time lookup, rather than linear.

Result:

Better perf for HTTPHeader normalisation with large connection header
value lists.

Sources/NIOHPACK/HPACKHeader.swift

@@ -18,6 +18,22 @@ import NIOHTTP1
 /// Very similar to `NIOHTTP1.HTTPHeaders`, but with extra data for storing indexing
 /// information.
 public struct HPACKHeaders: ExpressibleByDictionaryLiteral, NIOSendable {
+    /// The maximum size of the canonical connection header value array to use when removing
+    /// connection headers during `HTTPHeaders` normalisation. When using an array the removal
+    /// is O(H·C) where H is the length of headers to noramlize and C is the length of the
+    /// connection header value array.
+    ///
+    /// Beyond this limit we construct a set of the connection header values to reduce the
+    /// complexity to O(H).
+    ///
+    /// We use an array for small connection header lists as it is cheaper (values don't need to be
+    /// hashed and constructing a set incurs an additional allocation). The value of 32 was picked
+    /// as the crossover point where using an array became more expensive than using a set when
+    /// running the `hpackheaders_normalize_httpheaders_removing_10k_conn_headers` performance test
+    /// with varying input sizes.
+    @usableFromInline
+    static let connectionHeaderValueArraySizeLimit = 32
+
     @usableFromInline
     internal var headers: [HPACKHeader]
 
@@ -33,10 +49,21 @@ public struct HPACKHeaders: ExpressibleByDictionaryLiteral, NIOSendable {
             self.headers = httpHeaders.map { HPACKHeader(name: $0.name.lowercased(), value: $0.value) }
 
             let connectionHeaderValue = httpHeaders[canonicalForm: "connection"]
-
-            self.headers.removeAll { header in
-                connectionHeaderValue.contains(header.name[...]) ||
-                    HPACKHeaders.illegalHeaders.contains(header.name)
+
+            // Above a limit we use a set rather than scanning the connection header value array.
+            // See `Self.connectionHeaderValueArraySizeLimit`.
+            if connectionHeaderValue.count > Self.connectionHeaderValueArraySizeLimit {
+                var headersToRemove = Set(connectionHeaderValue)
+                // Since we have a set we can just merge in the illegal headers.
+                headersToRemove.formUnion(Self.illegalHeaders.lazy.map { $0[...] })
+                self.headers.removeAll { header in
+                    headersToRemove.contains(header.name[...])
+                }
+            } else {
+                self.headers.removeAll { header in
+                    connectionHeaderValue.contains(header.name[...]) ||
+                        HPACKHeaders.illegalHeaders.contains(header.name)
+                }
             }
         } else {
             self.headers = httpHeaders.map { HPACKHeader(name: $0.name, value: $0.value) }

Tests/NIOHPACKTests/HPACKHeadersTests+XCTest.swift

@@ -28,6 +28,8 @@ extension HPACKHeadersTests {
    static var allTests : [(String, (HPACKHeadersTests) -> () throws -> Void)] {
       return [
                 ("testHPACKHeadersAreHashable", testHPACKHeadersAreHashable),
+                ("testNormalizationOfHTTPHeaders", testNormalizationOfHTTPHeaders),
+                ("testNormalizationOfHTTPHeadersWithManyConnectionHeaderValues", testNormalizationOfHTTPHeadersWithManyConnectionHeaderValues),
            ]
    }
 }

Tests/NIOHPACKTests/HPACKHeadersTests.swift

@@ -15,6 +15,7 @@
 import XCTest
 import NIOCore
 import NIOHPACK
+import NIOHTTP1
 
 final class HPACKHeadersTests: XCTestCase {
     func testHPACKHeadersAreHashable() throws {
@@ -71,4 +72,51 @@ final class HPACKHeadersTests: XCTestCase {
             Set([firstHeaders, thirdHeaders, fourthHeaders])
         )
     }
+
+    func testNormalizationOfHTTPHeaders() {
+        let httpHeaders: HTTPHeaders = [
+            "connection": "keepalive",
+            "connection": "remove-me, and-me",
+            "connection": "also-me-please",
+            "remove-me": "",
+            "and-me": "",
+            "also-me-please": "",
+            "but-not-me": "",
+            "keep-alive": "remove-me",
+            "proxy-connection": "me too",
+            "transfer-encoding": "me three"
+        ]
+
+        let normalized = HPACKHeaders(httpHeaders: httpHeaders, normalizeHTTPHeaders: true)
+        let expected: HPACKHeaders = [
+            "but-not-me": ""
+        ]
+
+        XCTAssertEqual(normalized, expected)
+    }
+
+    func testNormalizationOfHTTPHeadersWithManyConnectionHeaderValues() {
+        var httpHeaders: HTTPHeaders = [
+            "keep-alive": "remove-me",
+            "proxy-connection": "me too",
+            "transfer-encoding": "me three",
+            "but-not-me": "",
+        ]
+
+        // Add a bunch of connection headers to remove. We add a large number because the
+        // implementation of the normalizing init branches on the number of connection header
+        // values.
+        for i in 0 ..< 512 {
+            let toRemove = "value-\(i)"
+            httpHeaders.add(name: "connection", value: toRemove)
+            httpHeaders.add(name: toRemove, value: "")
+        }
+
+        let normalized = HPACKHeaders(httpHeaders: httpHeaders, normalizeHTTPHeaders: true)
+        let expected: HPACKHeaders = [
+            "but-not-me": ""
+        ]
+
+        XCTAssertEqual(normalized, expected)
+    }
 }