Reduce allocations when peeking pseudo-headers. (#246)

Motivation: The 2 to 1 codecs do a number of operations where they "peek" the pseudo-headers. Each of these operations uses the HPACKHeaders subscript, meaning that they allocate a temporary array for a field that can, in a correct request/response, contain only one element. That's very wasteful! Modifications: - Rewrite peekPseudoHeader to iterate instead, avoiding the array. - Add allocaton counter test. Result: 4 fewer allocations per HTTP request transformed on the server side, 2 fewer allocations per HTTP response transformed on the client side.
apple · Sep 24, 2020 · 5b0194a · 5b0194a
1 parent cee48fd
commit 5b0194a
Show file tree

Hide file tree

Showing 6 changed files with 138 additions and 7 deletions.
diff --git a/...sts_01_allocation_counters/test_01_resources/test_client_server_h1_request_response.swift b/...sts_01_allocation_counters/test_01_resources/test_client_server_h1_request_response.swift
@@ -0,0 +1,114 @@
+//===----------------------------------------------------------------------===//
+//
+// This source file is part of the SwiftNIO open source project
+//
+// Copyright (c) 2020 Apple Inc. and the SwiftNIO project authors
+// Licensed under Apache License v2.0
+//
+// See LICENSE.txt for license information
+// See CONTRIBUTORS.txt for the list of SwiftNIO project authors
+//
+// SPDX-License-Identifier: Apache-2.0
+//
+//===----------------------------------------------------------------------===//
+
+import NIO
+import NIOHPACK
+import NIOHTTP1
+import NIOHTTP2
+
+/// Have two `EmbeddedChannel` objects send and receive data from each other until
+/// they make no forward progress.
+func interactInMemory(_ first: EmbeddedChannel, _ second: EmbeddedChannel) throws {
+    var operated: Bool
+
+    func readBytesFromChannel(_ channel: EmbeddedChannel) throws -> ByteBuffer? {
+        return try channel.readOutbound(as: ByteBuffer.self)
+    }
+
+    repeat {
+        operated = false
+        first.embeddedEventLoop.run()
+
+        if let data = try readBytesFromChannel(first) {
+            operated = true
+            try second.writeInbound(data)
+        }
+        if let data = try readBytesFromChannel(second) {
+            operated = true
+            try first.writeInbound(data)
+        }
+    } while operated
+}
+
+final class ServerHandler: ChannelInboundHandler {
+    typealias InboundIn = HTTPServerRequestPart
+    typealias OutboundOut = HTTPServerResponsePart
+
+    func channelRead(context: ChannelHandlerContext, data: NIOAny) {
+        let data = self.unwrapInboundIn(data)
+        switch data {
+        case .head, .body:
+            // Ignore this
+            return
+        case .end:
+            break
+        }
+
+        // We got .end. Let's send a response.
+        let head = HTTPResponseHead(version: .init(major: 2, minor: 0), status: .ok)
+        context.write(self.wrapOutboundOut(.head(head)), promise: nil)
+        context.writeAndFlush(self.wrapOutboundOut(.end(nil)), promise: nil)
+    }
+}
+
+
+final class ClientHandler: ChannelInboundHandler {
+    typealias InboundIn = HTTPClientResponsePart
+    typealias OutboundOut = HTTPClientRequestPart
+
+    func channelActive(context: ChannelHandlerContext) {
+        // Send a request.
+        let head = HTTPRequestHead(version: .init(major: 2, minor: 0), method: .GET, uri: "/", headers: HTTPHeaders([("host", "localhost")]))
+        context.write(self.wrapOutboundOut(.head(head)), promise: nil)
+        context.writeAndFlush(self.wrapOutboundOut(.end(nil)), promise: nil)
+    }
+}
+
+func run(identifier: String) {
+    let loop = EmbeddedEventLoop()
+
+    measure(identifier: identifier) {
+        var sumOfStreamIDs = 0
+
+        for _ in 0..<1000 {
+            let clientChannel = EmbeddedChannel(loop: loop)
+            let serverChannel = EmbeddedChannel(loop: loop)
+
+            let clientMultiplexer = try! clientChannel.configureHTTP2Pipeline(mode: .client).wait()
+            _ = try! serverChannel.configureHTTP2Pipeline(mode: .server) { channel in
+                return channel.pipeline.addHandlers([HTTP2FramePayloadToHTTP1ServerCodec(), ServerHandler()])
+            }.wait()
+            try! clientChannel.connect(to: SocketAddress(ipAddress: "1.2.3.4", port: 5678)).wait()
+            try! serverChannel.connect(to: SocketAddress(ipAddress: "1.2.3.4", port: 5678)).wait()
+
+            let promise = clientChannel.eventLoop.makePromise(of: Channel.self)
+            clientMultiplexer.createStreamChannel(promise: promise) { channel in
+                return channel.pipeline.addHandlers([HTTP2FramePayloadToHTTP1ClientCodec(httpProtocol: .https), ClientHandler()])
+            }
+            clientChannel.embeddedEventLoop.run()
+            let child = try! promise.futureResult.wait()
+            let streamID = try! Int(child.getOption(HTTP2StreamChannelOptions.streamID).wait())
+
+            sumOfStreamIDs += streamID
+            try! interactInMemory(clientChannel, serverChannel)
+            try! child.closeFuture.wait()
+
+            try! clientChannel.close().wait()
+            try! serverChannel.close().wait()
+        }
+
+        return sumOfStreamIDs
+    }
+}
+
diff --git a/Sources/NIOHTTP2/HTTP2ToHTTP1Codec.swift b/Sources/NIOHTTP2/HTTP2ToHTTP1Codec.swift
@@ -616,14 +616,22 @@ extension HPACKHeaders {
     /// - returns: The value for this pseudo-header.
     /// - throws: If there is no such header, or multiple.
     internal func peekPseudoHeader(name: String) throws -> String {
-        let value = self[name]
-        switch value.count {
-        case 0:
+        // This could be done with .lazy.filter.map but that generates way more ARC traffic.
+        var headerValue: String? = nil
+
+        for (fieldName, fieldValue, _) in self {
+            if name == fieldName {
+                guard headerValue == nil else {
+                    throw NIOHTTP2Errors.DuplicatePseudoHeader(name)
+                }
+                headerValue = fieldValue
+            }
+        }
+
+        if let headerValue = headerValue {
+            return headerValue
+        } else {
             throw NIOHTTP2Errors.MissingPseudoHeader(name)
-        case 1:
-            return value.first!
-        default:
-            throw NIOHTTP2Errors.DuplicatePseudoHeader(name)
         }
     }
 }

diff --git a/docker/docker-compose.1604.51.yaml b/docker/docker-compose.1604.51.yaml
@@ -19,6 +19,7 @@ services:
       - MAX_ALLOCS_ALLOWED_create_client_stream_channel=72010
       - MAX_ALLOCS_ALLOWED_hpack_decoding=5050
       - MAX_ALLOCS_ALLOWED_client_server_request_response=378000
+      - MAX_ALLOCS_ALLOWED_client_server_h1_request_response=409000
 
   performance-test:
     image: swift-nio-http2:16.04-5.1
@@ -32,6 +33,7 @@ services:
       - MAX_ALLOCS_ALLOWED_create_client_stream_channel=72010
       - MAX_ALLOCS_ALLOWED_hpack_decoding=5050
       - MAX_ALLOCS_ALLOWED_client_server_request_response=378000
+      - MAX_ALLOCS_ALLOWED_client_server_h1_request_response=409000
       - SANITIZER_ARG=--sanitize=thread
 
   shell:

diff --git a/docker/docker-compose.1804.50.yaml b/docker/docker-compose.1804.50.yaml
@@ -19,6 +19,7 @@ services:
       - MAX_ALLOCS_ALLOWED_create_client_stream_channel=72010
       - MAX_ALLOCS_ALLOWED_hpack_decoding=5050
       - MAX_ALLOCS_ALLOWED_client_server_request_response=384810
+      - MAX_ALLOCS_ALLOWED_client_server_h1_request_response=425000
 
   performance-test:
     image: swift-nio-http2:18.04-5.0
@@ -32,6 +33,7 @@ services:
       - MAX_ALLOCS_ALLOWED_create_client_stream_channel=72010
       - MAX_ALLOCS_ALLOWED_hpack_decoding=5050
       - MAX_ALLOCS_ALLOWED_client_server_request_response=384810
+      - MAX_ALLOCS_ALLOWED_client_server_h1_request_response=425000
 
   shell:
     image: swift-nio-http2:18.04-5.0
diff --git a/docker/docker-compose.1804.52.yaml b/docker/docker-compose.1804.52.yaml
@@ -19,6 +19,7 @@ services:
       - MAX_ALLOCS_ALLOWED_create_client_stream_channel=72010
       - MAX_ALLOCS_ALLOWED_hpack_decoding=5050
       - MAX_ALLOCS_ALLOWED_client_server_request_response=384810
+      - MAX_ALLOCS_ALLOWED_client_server_h1_request_response=409000
 
   performance-test:
     image: swift-nio-http2:18.04-5.2
@@ -32,6 +33,8 @@ services:
       - MAX_ALLOCS_ALLOWED_create_client_stream_channel=72010
       - MAX_ALLOCS_ALLOWED_hpack_decoding=5050
       - MAX_ALLOCS_ALLOWED_client_server_request_response=384810
+      - MAX_ALLOCS_ALLOWED_client_server_h1_request_response=409000
+
 
   shell:
     image: swift-nio-http2:18.04-5.2
diff --git a/docker/docker-compose.1804.53.yaml b/docker/docker-compose.1804.53.yaml
@@ -19,6 +19,7 @@ services:
       - MAX_ALLOCS_ALLOWED_create_client_stream_channel=72010
       - MAX_ALLOCS_ALLOWED_hpack_decoding=5050
       - MAX_ALLOCS_ALLOWED_client_server_request_response=384810
+      - MAX_ALLOCS_ALLOWED_client_server_h1_request_response=409000
 
   performance-test:
     image: swift-nio-http2:18.04-5.3
@@ -32,6 +33,7 @@ services:
       - MAX_ALLOCS_ALLOWED_create_client_stream_channel=72010
       - MAX_ALLOCS_ALLOWED_hpack_decoding=5050
       - MAX_ALLOCS_ALLOWED_client_server_request_response=384810
+      - MAX_ALLOCS_ALLOWED_client_server_h1_request_response=409000
 
   shell:
     image: swift-nio-http2:18.04-5.3