From f78644d7620771536428b3bc4c6471a8cba66ca1 Mon Sep 17 00:00:00 2001 From: Carl Mastrangelo Date: Wed, 7 Sep 2016 14:55:34 -0700 Subject: [PATCH] core: add Metadata.discardAll() Metadata.removeAll creates an iterator for looking through removed values even if the call doens't use it. This change adds a similar method which doesn't create garbage. This change makes it easier in the future to alter the internals of Metadata where it may be expensive to return removed values. --- core/src/main/java/io/grpc/Metadata.java | 10 ++++++++++ .../java/io/grpc/internal/AbstractServerStream.java | 4 ++-- .../main/java/io/grpc/internal/ClientCallImpl.java | 6 +++--- .../main/java/io/grpc/internal/Http2ClientStream.java | 6 +++--- .../main/java/io/grpc/internal/ServerCallImpl.java | 4 ++-- core/src/test/java/io/grpc/MetadataTest.java | 11 +++++++++++ .../main/java/io/grpc/netty/NettyClientStream.java | 2 +- .../main/java/io/grpc/okhttp/OkHttpClientStream.java | 2 +- 8 files changed, 33 insertions(+), 12 deletions(-) diff --git a/core/src/main/java/io/grpc/Metadata.java b/core/src/main/java/io/grpc/Metadata.java index 26a418bb9d0..43ad9c4d2f4 100644 --- a/core/src/main/java/io/grpc/Metadata.java +++ b/core/src/main/java/io/grpc/Metadata.java @@ -271,6 +271,16 @@ public Iterable removeAll(Key key) { return new ValueIterable(key, values); } + /** + * Remove all values for the given key without returning them. This is a minor performance + * optimization if you do not need the previous values. + */ + @ExperimentalApi + public void discardAll(Key key) { + List removed = store.remove(key.name()); + storeCount -= removed != null ? removed.size() : 0; + } + /** * Serialize all the metadata entries. * diff --git a/core/src/main/java/io/grpc/internal/AbstractServerStream.java b/core/src/main/java/io/grpc/internal/AbstractServerStream.java index 05033ad3818..d1f2b07cf56 100644 --- a/core/src/main/java/io/grpc/internal/AbstractServerStream.java +++ b/core/src/main/java/io/grpc/internal/AbstractServerStream.java @@ -145,8 +145,8 @@ public final void close(Status status, Metadata trailers) { } private void addStatusToTrailers(Metadata trailers, Status status) { - trailers.removeAll(Status.CODE_KEY); - trailers.removeAll(Status.MESSAGE_KEY); + trailers.discardAll(Status.CODE_KEY); + trailers.discardAll(Status.MESSAGE_KEY); trailers.put(Status.CODE_KEY, status); if (status.getDescription() != null) { trailers.put(Status.MESSAGE_KEY, status.getDescription()); diff --git a/core/src/main/java/io/grpc/internal/ClientCallImpl.java b/core/src/main/java/io/grpc/internal/ClientCallImpl.java index b6b2b53d2c4..86fd4ebccc6 100644 --- a/core/src/main/java/io/grpc/internal/ClientCallImpl.java +++ b/core/src/main/java/io/grpc/internal/ClientCallImpl.java @@ -140,12 +140,12 @@ ClientCallImpl setCompressorRegistry(CompressorRegistry compressorR @VisibleForTesting static void prepareHeaders(Metadata headers, DecompressorRegistry decompressorRegistry, Compressor compressor) { - headers.removeAll(MESSAGE_ENCODING_KEY); + headers.discardAll(MESSAGE_ENCODING_KEY); if (compressor != Codec.Identity.NONE) { headers.put(MESSAGE_ENCODING_KEY, compressor.getMessageEncoding()); } - headers.removeAll(MESSAGE_ACCEPT_ENCODING_KEY); + headers.discardAll(MESSAGE_ACCEPT_ENCODING_KEY); String advertisedEncodings = decompressorRegistry.getRawAdvertisedMessageEncodings(); if (!advertisedEncodings.isEmpty()) { headers.put(MESSAGE_ACCEPT_ENCODING_KEY, advertisedEncodings); @@ -251,7 +251,7 @@ public void runInContext() { */ private static void updateTimeoutHeaders(@Nullable Deadline effectiveDeadline, @Nullable Deadline callDeadline, @Nullable Deadline outerCallDeadline, Metadata headers) { - headers.removeAll(TIMEOUT_KEY); + headers.discardAll(TIMEOUT_KEY); if (effectiveDeadline == null) { return; diff --git a/core/src/main/java/io/grpc/internal/Http2ClientStream.java b/core/src/main/java/io/grpc/internal/Http2ClientStream.java index e2699caf709..5be2f08b0f1 100644 --- a/core/src/main/java/io/grpc/internal/Http2ClientStream.java +++ b/core/src/main/java/io/grpc/internal/Http2ClientStream.java @@ -238,8 +238,8 @@ private static Charset extractCharset(Metadata headers) { * the application layer. */ private static void stripTransportDetails(Metadata metadata) { - metadata.removeAll(HTTP2_STATUS); - metadata.removeAll(Status.CODE_KEY); - metadata.removeAll(Status.MESSAGE_KEY); + metadata.discardAll(HTTP2_STATUS); + metadata.discardAll(Status.CODE_KEY); + metadata.discardAll(Status.MESSAGE_KEY); } } diff --git a/core/src/main/java/io/grpc/internal/ServerCallImpl.java b/core/src/main/java/io/grpc/internal/ServerCallImpl.java index d2a4e902948..21f92aa7279 100644 --- a/core/src/main/java/io/grpc/internal/ServerCallImpl.java +++ b/core/src/main/java/io/grpc/internal/ServerCallImpl.java @@ -104,7 +104,7 @@ public void sendHeaders(Metadata headers) { checkState(!sendHeadersCalled, "sendHeaders has already been called"); checkState(!closeCalled, "call is closed"); - headers.removeAll(MESSAGE_ENCODING_KEY); + headers.discardAll(MESSAGE_ENCODING_KEY); if (compressor == null) { compressor = Codec.Identity.NONE; } else { @@ -125,7 +125,7 @@ public void sendHeaders(Metadata headers) { stream.setCompressor(compressor); - headers.removeAll(MESSAGE_ACCEPT_ENCODING_KEY); + headers.discardAll(MESSAGE_ACCEPT_ENCODING_KEY); String advertisedEncodings = decompressorRegistry.getRawAdvertisedMessageEncodings(); if (!advertisedEncodings.isEmpty()) { headers.put(MESSAGE_ACCEPT_ENCODING_KEY, advertisedEncodings); diff --git a/core/src/test/java/io/grpc/MetadataTest.java b/core/src/test/java/io/grpc/MetadataTest.java index e741f47472e..a2219b947f1 100644 --- a/core/src/test/java/io/grpc/MetadataTest.java +++ b/core/src/test/java/io/grpc/MetadataTest.java @@ -114,6 +114,17 @@ public void testMutations() { assertEquals(null, metadata.get(KEY)); } + @Test + public void discardAll() { + Fish lance = new Fish(LANCE); + Metadata metadata = new Metadata(); + + metadata.put(KEY, lance); + metadata.discardAll(KEY); + assertEquals(null, metadata.getAll(KEY)); + assertEquals(null, metadata.get(KEY)); + } + @Test public void testGetAllNoRemove() { Fish lance = new Fish(LANCE); diff --git a/netty/src/main/java/io/grpc/netty/NettyClientStream.java b/netty/src/main/java/io/grpc/netty/NettyClientStream.java index c6cefabb229..c47b77ec94d 100644 --- a/netty/src/main/java/io/grpc/netty/NettyClientStream.java +++ b/netty/src/main/java/io/grpc/netty/NettyClientStream.java @@ -105,7 +105,7 @@ public void start(ClientStreamListener listener) { defaultPath = new AsciiString("/" + method.getFullMethodName()); methodDescriptorAccessor.setRawMethodName(method, defaultPath); } - headers.removeAll(GrpcUtil.USER_AGENT_KEY); + headers.discardAll(GrpcUtil.USER_AGENT_KEY); Http2Headers http2Headers = Utils.convertClientHeaders(headers, scheme, defaultPath, authority, userAgent); headers = null; diff --git a/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientStream.java b/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientStream.java index 5c96c8166b4..ce8ccee77eb 100644 --- a/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientStream.java +++ b/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientStream.java @@ -139,7 +139,7 @@ public void setAuthority(String authority) { public void start(ClientStreamListener listener) { super.start(listener); String defaultPath = "/" + method.getFullMethodName(); - headers.removeAll(GrpcUtil.USER_AGENT_KEY); + headers.discardAll(GrpcUtil.USER_AGENT_KEY); List
requestHeaders = Headers.createRequestHeaders(headers, defaultPath, authority, userAgent); headers = null;