From 5b0194aa634555bc8e00c1284f4619052c710f8d Mon Sep 17 00:00:00 2001 From: Cory Benfield Date: Thu, 24 Sep 2020 09:57:47 +0100 Subject: [PATCH] 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. --- ...st_client_server_h1_request_response.swift | 114 ++++++++++++++++++ Sources/NIOHTTP2/HTTP2ToHTTP1Codec.swift | 22 ++-- docker/docker-compose.1604.51.yaml | 2 + docker/docker-compose.1804.50.yaml | 2 + docker/docker-compose.1804.52.yaml | 3 + docker/docker-compose.1804.53.yaml | 2 + 6 files changed, 138 insertions(+), 7 deletions(-) create mode 100644 IntegrationTests/tests_01_allocation_counters/test_01_resources/test_client_server_h1_request_response.swift diff --git a/IntegrationTests/tests_01_allocation_counters/test_01_resources/test_client_server_h1_request_response.swift b/IntegrationTests/tests_01_allocation_counters/test_01_resources/test_client_server_h1_request_response.swift new file mode 100644 index 00000000..eb21c30d --- /dev/null +++ b/IntegrationTests/tests_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 index cad5356f..6271ecec 100644 --- 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 index b55744f0..ee8ab7ff 100644 --- 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 index fbacfd9c..a88f0fa2 100644 --- 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 index 78251185..cf6a78da 100644 --- 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 index 28771a7a..c1f6d4e7 100644 --- 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