Skip to content

Commit

Permalink
Don't construct unnecessary buffers. (#925)
Browse files Browse the repository at this point in the history
Motivation:

Currently the HTTP1ToGRPCServerCodec constructs a CircularBuffer that it
only ever uses when doing grpc-web work. This is not the end of the
world, except that CircularBuffers require memory allocations: there is
no such thing as an empty CircularBuffer.

In general it's not a great idea to force the mainline grpc use-case to
pay for the cost of constructing a circular buffer it will never use, so
instead we replace this with an optional circular buffer. That optional
will be initialized on first use, so it doesn't add any complexity to
our state tracking. It does add some subtle complexity to the rest of
the grpc-web code, though, so we should consider increasing the priority
of dealing with grpc-web properly.

Modifications:

- Make HTTP1ToGRPCServerCodec.responseTextBuffers optional, delay its
  initialization.

Results:

Small performance improvement on HTTP/2 server benchmarks, around 0.7%
per RPC.
  • Loading branch information
Lukasa authored Aug 7, 2020
1 parent 6158fad commit da54baa
Showing 1 changed file with 24 additions and 7 deletions.
31 changes: 24 additions & 7 deletions Sources/GRPC/HTTP1ToGRPCServerCodec.swift
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ public final class HTTP1ToGRPCServerCodec {
// TODO(kaipi): Extract all gRPC Web processing logic into an independent handler only added on
// the HTTP1.1 pipeline, as it's starting to get in the way of readability.
private var requestTextBuffer: NIO.ByteBuffer!
private var responseTextBuffers: CircularBuffer<ByteBuffer> = []
private var responseTextBuffers: CircularBuffer<ByteBuffer>?

var inboundState = InboundState.expectingHeaders {
willSet {
Expand Down Expand Up @@ -336,7 +336,7 @@ extension HTTP1ToGRPCServerCodec: ChannelOutboundHandler {
allocator: context.channel.allocator,
compressed: messageContext.compressed
)
self.responseTextBuffers.append(buffer)
self.appendResponseText(buffer)

// Since we stored the written data, mark the write promise as successful so that the
// ServerStreaming provider continues sending the data.
Expand Down Expand Up @@ -381,21 +381,31 @@ extension HTTP1ToGRPCServerCodec: ChannelOutboundHandler {
trailersBuffer.writeInteger(UInt8(0x80))
trailersBuffer.writeInteger(UInt32(textTrailers.utf8.count))
trailersBuffer.writeString(textTrailers)
self.responseTextBuffers.append(trailersBuffer)
self.appendResponseText(trailersBuffer)

// The '!' is fine, we know it's not empty since we just added a buffer.
var responseTextBuffer = self.responseTextBuffers.popFirst()!
// This code can only be called on the grpc-web path, so we know the response text buffers must be non-nil
// and must contain at least one element.
guard var buffers = self.responseTextBuffers else {
preconditionFailure("Building web response text but responseTextBuffers are nil")
}

// Avoid a CoW
self.responseTextBuffers = nil
var responseTextBuffer = buffers.popFirst()!

// Read the data from the first buffer.
var accumulatedData = responseTextBuffer.readData(length: responseTextBuffer.readableBytes)!

// Reserve enough capacity and append the remaining buffers.
let requiredExtraCapacity = self.responseTextBuffers.lazy.map { $0.readableBytes }.reduce(0, +)
let requiredExtraCapacity = buffers.lazy.map { $0.readableBytes }.reduce(0, +)
accumulatedData.reserveCapacity(accumulatedData.count + requiredExtraCapacity)
while let buffer = self.responseTextBuffers.popFirst() {
while let buffer = buffers.popFirst() {
accumulatedData.append(contentsOf: buffer.readableBytesView)
}

// Restore the buffers.
self.responseTextBuffers = buffers

// TODO: Binary responses that are non multiples of 3 will end = or == when encoded in
// base64. Investigate whether this might have any effect on the transport mechanism and
// client decoding. Initial results say that they are innocuous, but we might have to keep
Expand Down Expand Up @@ -429,6 +439,13 @@ extension HTTP1ToGRPCServerCodec: ChannelOutboundHandler {
self.inboundState = .ignore
}
}

private func appendResponseText(_ buffer: ByteBuffer) {
if self.responseTextBuffers == nil {
self.responseTextBuffers = CircularBuffer()
}
self.responseTextBuffers!.append(buffer)
}
}

fileprivate extension HTTP1ToGRPCServerCodec {
Expand Down

0 comments on commit da54baa

Please sign in to comment.