Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce allocations when peeking pseudo-headers. #246

Merged
merged 1 commit into from
Sep 24, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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
}
}

22 changes: 15 additions & 7 deletions Sources/NIOHTTP2/HTTP2ToHTTP1Codec.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The subscript does a case insensitive check, I assume we don't need to do that here because this is only ever called internally and we know that the headers must be lowercase so we'll always provide a lowercased name ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pseudo-headers are required to be lower-case. This is a nice perf win here: if the weren't lowercase, the request was malformed anyway.

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)
}
}
}
Expand Down
2 changes: 2 additions & 0 deletions docker/docker-compose.1604.51.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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:
Expand Down
2 changes: 2 additions & 0 deletions docker/docker-compose.1804.50.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
3 changes: 3 additions & 0 deletions docker/docker-compose.1804.52.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
2 changes: 2 additions & 0 deletions docker/docker-compose.1804.53.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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