From 5e30b2f7ba57e306ef7818f485c854435fa9802b Mon Sep 17 00:00:00 2001 From: Carl Mastrangelo Date: Mon, 23 May 2016 15:05:56 -0700 Subject: [PATCH] netty: speed up header conversion by caching user agent string MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Performance gain seems to be about 100-200ns based on a a couple trials. Benchmark (headerCount) (validate) Mode Cnt Score Error Units HeadersBenchmark.convertClientHeadersOld 10 false sample 187490 858.234 ± 3.992 ns/op HeadersBenchmark.convertClientHeadersOld 20 false sample 113589 1407.557 ± 45.178 ns/op HeadersBenchmark.convertClientHeadersOld 50 false sample 100725 3141.936 ± 55.175 ns/op HeadersBenchmark.convertClientHeadersOld 100 false sample 109742 5707.748 ± 38.222 ns/op HeadersBenchmark.convertHeaders 10 false sample 109137 748.486 ± 4.060 ns/op HeadersBenchmark.convertHeaders 20 false sample 133639 1238.528 ± 51.914 ns/op HeadersBenchmark.convertHeaders 50 false sample 107914 2915.602 ± 10.017 ns/op HeadersBenchmark.convertHeaders 100 false sample 110305 5682.404 ± 44.032 ns/op --- .../java/io/grpc/netty/HeadersBenchmark.java | 3 ++- .../main/java/io/grpc/internal/GrpcUtil.java | 20 ++++++++++++------- .../java/io/grpc/netty/NettyClientStream.java | 10 +++++++--- .../io/grpc/netty/NettyClientTransport.java | 9 ++++++++- netty/src/main/java/io/grpc/netty/Utils.java | 7 ++++--- .../io/grpc/netty/NettyClientStreamTest.java | 11 ++++++---- 6 files changed, 41 insertions(+), 19 deletions(-) diff --git a/benchmarks/src/jmh/java/io/grpc/netty/HeadersBenchmark.java b/benchmarks/src/jmh/java/io/grpc/netty/HeadersBenchmark.java index db262294b9..5675f163ea 100644 --- a/benchmarks/src/jmh/java/io/grpc/netty/HeadersBenchmark.java +++ b/benchmarks/src/jmh/java/io/grpc/netty/HeadersBenchmark.java @@ -76,6 +76,7 @@ public class HeadersBenchmark { private AsciiString scheme = new AsciiString("https"); private AsciiString defaultPath = new AsciiString("/Service.MethodMethodMethod"); private AsciiString authority = new AsciiString("authority.googleapis.bogus"); + private AsciiString userAgent = new AsciiString("grpc-java-netty"); @Setup public void setUp() throws Exception { @@ -89,7 +90,7 @@ public class HeadersBenchmark { @BenchmarkMode(Mode.SampleTime) @OutputTimeUnit(TimeUnit.NANOSECONDS) public Http2Headers convertHeaders() { - return Utils.convertClientHeaders(metadata, scheme, defaultPath, authority); + return Utils.convertClientHeaders(metadata, scheme, defaultPath, authority, userAgent); } } diff --git a/core/src/main/java/io/grpc/internal/GrpcUtil.java b/core/src/main/java/io/grpc/internal/GrpcUtil.java index 4b90540e73..5011eb640a 100644 --- a/core/src/main/java/io/grpc/internal/GrpcUtil.java +++ b/core/src/main/java/io/grpc/internal/GrpcUtil.java @@ -145,6 +145,8 @@ public final class GrpcUtil { public static final Joiner ACCEPT_ENCODING_JOINER = Joiner.on(','); + private static final String IMPLEMENTATION_VERION = getImplementationVersion(); + /** * Maps HTTP error response status codes to transport codes. */ @@ -303,8 +305,8 @@ public final class GrpcUtil { /** * Gets the User-Agent string for the gRPC transport. */ - public static String getGrpcUserAgent(String transportName, - @Nullable String applicationUserAgent) { + public static String getGrpcUserAgent( + String transportName, @Nullable String applicationUserAgent) { StringBuilder builder = new StringBuilder(); if (applicationUserAgent != null) { builder.append(applicationUserAgent); @@ -312,11 +314,7 @@ public final class GrpcUtil { } builder.append("grpc-java-"); builder.append(transportName); - String version = GrpcUtil.class.getPackage().getImplementationVersion(); - if (version != null) { - builder.append("/"); - builder.append(version); - } + builder.append(IMPLEMENTATION_VERION); return builder.toString(); } @@ -491,4 +489,12 @@ public final class GrpcUtil { } private GrpcUtil() {} + + private static String getImplementationVersion() { + String version = GrpcUtil.class.getPackage().getImplementationVersion(); + if (version != null) { + return "/" + version; + } + return ""; + } } diff --git a/netty/src/main/java/io/grpc/netty/NettyClientStream.java b/netty/src/main/java/io/grpc/netty/NettyClientStream.java index ff78d85499..826179b243 100644 --- a/netty/src/main/java/io/grpc/netty/NettyClientStream.java +++ b/netty/src/main/java/io/grpc/netty/NettyClientStream.java @@ -60,14 +60,17 @@ abstract class NettyClientStream extends Http2ClientStream implements StreamIdHo private Metadata headers; private final Channel channel; private final NettyClientHandler handler; - private AsciiString authority; private final AsciiString scheme; + private final AsciiString userAgent; + private AsciiString authority; + private Http2Stream http2Stream; private Integer id; private WriteQueue writeQueue; NettyClientStream(MethodDescriptor method, Metadata headers, Channel channel, - NettyClientHandler handler, int maxMessageSize, AsciiString authority, AsciiString scheme) { + NettyClientHandler handler, int maxMessageSize, AsciiString authority, AsciiString scheme, + AsciiString userAgent) { super(new NettyWritableBufferAllocator(channel.alloc()), maxMessageSize); this.method = checkNotNull(method, "method"); this.headers = checkNotNull(headers, "headers"); @@ -76,6 +79,7 @@ abstract class NettyClientStream extends Http2ClientStream implements StreamIdHo this.handler = checkNotNull(handler, "handler"); this.authority = checkNotNull(authority, "authority"); this.scheme = checkNotNull(scheme, "scheme"); + this.userAgent = userAgent; } @Override @@ -91,7 +95,7 @@ abstract class NettyClientStream extends Http2ClientStream implements StreamIdHo // Convert the headers into Netty HTTP/2 headers. AsciiString defaultPath = new AsciiString("/" + method.getFullMethodName()); Http2Headers http2Headers - = Utils.convertClientHeaders(headers, scheme, defaultPath, authority); + = Utils.convertClientHeaders(headers, scheme, defaultPath, authority, userAgent); headers = null; ChannelFutureListener failureListener = new ChannelFutureListener() { diff --git a/netty/src/main/java/io/grpc/netty/NettyClientTransport.java b/netty/src/main/java/io/grpc/netty/NettyClientTransport.java index c589f36a93..25ab26d651 100644 --- a/netty/src/main/java/io/grpc/netty/NettyClientTransport.java +++ b/netty/src/main/java/io/grpc/netty/NettyClientTransport.java @@ -60,6 +60,9 @@ import java.util.concurrent.Executor; * A Netty-based {@link ManagedClientTransport} implementation. */ class NettyClientTransport implements ManagedClientTransport { + private static final AsciiString DEFAULT_AGENT = + new AsciiString(GrpcUtil.getGrpcUserAgent("netty", null)); + private final SocketAddress address; private final Class channelType; private final EventLoopGroup group; @@ -68,6 +71,7 @@ class NettyClientTransport implements ManagedClientTransport { private final int flowControlWindow; private final int maxMessageSize; private final int maxHeaderListSize; + private ProtocolNegotiator.Handler negotiationHandler; private NettyClientHandler handler; // We should not send on the channel until negotiation completes. This is a hard requirement @@ -110,8 +114,11 @@ class NettyClientTransport implements ManagedClientTransport { public ClientStream newStream(MethodDescriptor method, Metadata headers) { Preconditions.checkNotNull(method, "method"); Preconditions.checkNotNull(headers, "headers"); + AsciiString userAgent = headers.containsKey(GrpcUtil.USER_AGENT_KEY) + ? new AsciiString(GrpcUtil.getGrpcUserAgent("netty", headers.get(GrpcUtil.USER_AGENT_KEY))) + : DEFAULT_AGENT; return new NettyClientStream(method, headers, channel, handler, maxMessageSize, authority, - negotiationHandler.scheme()) { + negotiationHandler.scheme(), userAgent) { @Override protected Status statusFromFailedFuture(ChannelFuture f) { return NettyClientTransport.this.statusFromFailedFuture(f); diff --git a/netty/src/main/java/io/grpc/netty/Utils.java b/netty/src/main/java/io/grpc/netty/Utils.java index 3cb53b1ef1..d2ad66af60 100644 --- a/netty/src/main/java/io/grpc/netty/Utils.java +++ b/netty/src/main/java/io/grpc/netty/Utils.java @@ -117,7 +117,8 @@ class Utils { public static Http2Headers convertClientHeaders(Metadata headers, AsciiString scheme, AsciiString defaultPath, - AsciiString authority) { + AsciiString authority, + AsciiString userAgent) { Preconditions.checkNotNull(defaultPath, "defaultPath"); Preconditions.checkNotNull(authority, "authority"); // Add any application-provided headers first. @@ -132,8 +133,8 @@ class Utils { .set(TE_HEADER, TE_TRAILERS); // Set the User-Agent header. - String userAgent = GrpcUtil.getGrpcUserAgent("netty", headers.get(USER_AGENT_KEY)); - http2Headers.set(USER_AGENT, new AsciiString(userAgent.getBytes(UTF_8))); + //String userAgent = GrpcUtil.getGrpcUserAgent("netty", headers.get(USER_AGENT_KEY)); + http2Headers.set(USER_AGENT, userAgent); return http2Headers; } diff --git a/netty/src/test/java/io/grpc/netty/NettyClientStreamTest.java b/netty/src/test/java/io/grpc/netty/NettyClientStreamTest.java index 66d1c35eee..1b4aa7dd5c 100644 --- a/netty/src/test/java/io/grpc/netty/NettyClientStreamTest.java +++ b/netty/src/test/java/io/grpc/netty/NettyClientStreamTest.java @@ -358,7 +358,8 @@ public class NettyClientStreamTest extends NettyStreamTestBase method, Metadata headers, Channel channel, - NettyClientHandler handler, int maxMessageSize, AsciiString authority, AsciiString scheme) { - super(method, headers, channel, handler, maxMessageSize, authority, scheme); + NettyClientHandler handler, int maxMessageSize, AsciiString authority, AsciiString scheme, + AsciiString userAgent) { + super(method, headers, channel, handler, maxMessageSize, authority, scheme, userAgent); } @Override