From 656e8ce37fe4dc8f9bf6b705f6aaab9b87b16f24 Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Wed, 17 Feb 2016 10:19:08 -0800 Subject: [PATCH] core: Remove com.google.common.collect usages for Android This reduces the number of methods gRPC brings in by ~450, which is substantial. Each application will see different numbers though, depending on their usage and their other dependencies. A very rough (under) counting for number of methods included because of gRPC in android-interop-test is 2746, and that is reduced to 2313 (-433) by this change. That count includes grpc, guava, okhttp, okio, and nano. The actual reduction of methods is 447, with the discrepency due to reduction of methods in java.util and java.lang. Of the 433 removed methods, 377 are from com.google.common.collect and 61 from com.google.common.base. The removal costed an increase of 5 methods (total 1671) within io.grpc itself. --- .../src/java_plugin/cpp/java_generator.cpp | 5 -- core/src/main/java/io/grpc/Metadata.java | 54 ++++++++++++------- .../java/io/grpc/ServerServiceDefinition.java | 8 +-- .../main/java/io/grpc/internal/GrpcUtil.java | 38 ++++++++----- .../main/java/io/grpc/internal/Http2Ping.java | 5 +- .../internal/InternalHandlerRegistry.java | 17 +++--- .../grpc/internal/RoundRobinServerList.java | 15 +++--- core/src/test/java/io/grpc/MetadataTest.java | 12 +++++ .../io/grpc/ServerServiceDefinitionTest.java | 3 +- 9 files changed, 100 insertions(+), 57 deletions(-) diff --git a/compiler/src/java_plugin/cpp/java_generator.cpp b/compiler/src/java_plugin/cpp/java_generator.cpp index 05262e01c0..5ccbe4d8b3 100644 --- a/compiler/src/java_plugin/cpp/java_generator.cpp +++ b/compiler/src/java_plugin/cpp/java_generator.cpp @@ -1130,16 +1130,11 @@ void GenerateService(const ServiceDescriptor* service, vars["ServiceDescriptor"] = "io.grpc.ServiceDescriptor"; vars["AbstractStub"] = "io.grpc.stub.AbstractStub"; - vars["ImmutableList"] = "com.google.common.collect.ImmutableList"; - vars["Collection"] = "java.util.Collection"; vars["MethodDescriptor"] = "io.grpc.MethodDescriptor"; vars["NanoUtils"] = "io.grpc.protobuf.nano.NanoUtils"; vars["StreamObserver"] = "io.grpc.stub.StreamObserver"; vars["Iterator"] = "java.util.Iterator"; - vars["Map"] = "java.util.Map"; - vars["TimeUnit"] = "java.util.concurrent.TimeUnit"; vars["Generated"] = "javax.annotation.Generated"; - vars["Immutable"] = "javax.annotation.concurrent.Immutable"; vars["ListenableFuture"] = "com.google.common.util.concurrent.ListenableFuture"; vars["ExperimentalApi"] = "io.grpc.ExperimentalApi"; diff --git a/core/src/main/java/io/grpc/Metadata.java b/core/src/main/java/io/grpc/Metadata.java index 962c0c2d46..26a418bb9d 100644 --- a/core/src/main/java/io/grpc/Metadata.java +++ b/core/src/main/java/io/grpc/Metadata.java @@ -36,14 +36,13 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; import com.google.common.annotations.VisibleForTesting; -import com.google.common.base.Function; import com.google.common.base.Preconditions; -import com.google.common.collect.Iterables; import java.util.ArrayList; import java.util.Arrays; import java.util.BitSet; import java.util.Collections; +import java.util.Iterator; import java.util.LinkedHashMap; import java.util.List; import java.util.Locale; @@ -202,19 +201,12 @@ public final class Metadata { * parsed as T or null if there are none. The iterator is not guaranteed to be "live." It may or * may not be accurate if Metadata is mutated. */ - public Iterable getAll(final Key key) { + public Iterable getAll(Key key) { if (containsKey(key)) { /* This is unmodifiable currently, but could be made to support remove() in the future. If * removal support is added, the {@link #storeCount} variable needs to be updated * appropriately. */ - return Iterables.unmodifiableIterable(Iterables.transform( - store.get(key.name()), - new Function() { - @Override - public T apply(MetadataEntry entry) { - return entry.getParsed(key); - } - })); + return new ValueIterable(key, store.get(key.name())); } return null; } @@ -270,18 +262,13 @@ public final class Metadata { /** * Remove all values for the given key. If there were no values, {@code null} is returned. */ - public Iterable removeAll(final Key key) { + public Iterable removeAll(Key key) { List values = store.remove(key.name()); if (values == null) { return null; } storeCount -= values.size(); - return Iterables.transform(values, new Function() { - @Override - public T apply(MetadataEntry metadataEntry) { - return metadataEntry.getParsed(key); - } - }); + return new ValueIterable(key, values); } /** @@ -694,4 +681,35 @@ public final class Metadata { } } } + + private static class ValueIterable implements Iterable { + private final Key key; + private final Iterable entries; + + public ValueIterable(Key key, Iterable entries) { + this.key = key; + this.entries = entries; + } + + @Override + public Iterator iterator() { + final Iterator iterator = entries.iterator(); + class ValueIterator implements Iterator { + @Override public boolean hasNext() { + return iterator.hasNext(); + } + + @Override public T next() { + return iterator.next().getParsed(key); + } + + @Override public void remove() { + // Not implemented to not need to conditionally update {@link #storeCount}. + throw new UnsupportedOperationException(); + } + } + + return new ValueIterator(); + } + } } diff --git a/core/src/main/java/io/grpc/ServerServiceDefinition.java b/core/src/main/java/io/grpc/ServerServiceDefinition.java index 088cb6502c..43b71e3e57 100644 --- a/core/src/main/java/io/grpc/ServerServiceDefinition.java +++ b/core/src/main/java/io/grpc/ServerServiceDefinition.java @@ -35,10 +35,9 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkState; -import com.google.common.collect.ImmutableMap; - import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -55,12 +54,13 @@ public final class ServerServiceDefinition { } private final ServiceDescriptor serviceDescriptor; - private final ImmutableMap> methods; + private final Map> methods; private ServerServiceDefinition( ServiceDescriptor serviceDescriptor, Map> methods) { this.serviceDescriptor = checkNotNull(serviceDescriptor, "serviceDescriptor"); - this.methods = ImmutableMap.copyOf(methods); + this.methods = Collections.unmodifiableMap( + new HashMap>(methods)); } /** diff --git a/core/src/main/java/io/grpc/internal/GrpcUtil.java b/core/src/main/java/io/grpc/internal/GrpcUtil.java index ba06be52a4..ce0435bf11 100644 --- a/core/src/main/java/io/grpc/internal/GrpcUtil.java +++ b/core/src/main/java/io/grpc/internal/GrpcUtil.java @@ -38,7 +38,6 @@ import com.google.common.base.Preconditions; import com.google.common.base.Splitter; import com.google.common.base.Stopwatch; import com.google.common.base.Supplier; -import com.google.common.collect.ImmutableMap; import com.google.common.util.concurrent.MoreExecutors; import com.google.common.util.concurrent.ThreadFactoryBuilder; @@ -50,6 +49,12 @@ import java.lang.reflect.Method; import java.net.HttpURLConnection; import java.net.URI; import java.net.URISyntaxException; +import java.util.AbstractMap.SimpleImmutableEntry; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; import java.util.Map.Entry; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -485,23 +490,32 @@ public final class GrpcUtil { @VisibleForTesting static class TimeoutMarshaller implements Metadata.AsciiMarshaller { - // ImmutableMap's have consistent iteration order. - private static final ImmutableMap UNITS = - ImmutableMap.builder() - .put('n', TimeUnit.NANOSECONDS) - .put('u', TimeUnit.MICROSECONDS) - .put('m', TimeUnit.MILLISECONDS) - .put('S', TimeUnit.SECONDS) - .put('M', TimeUnit.MINUTES) - .put('H', TimeUnit.HOURS) - .build(); + @SuppressWarnings("unchecked") // asList uses an array which doesn't handle generics + private static final List> SERIALIZE_ORDER + = Collections.unmodifiableList(Arrays.>asList( + new SimpleImmutableEntry('n', TimeUnit.NANOSECONDS), + new SimpleImmutableEntry('u', TimeUnit.MICROSECONDS), + new SimpleImmutableEntry('m', TimeUnit.MILLISECONDS), + new SimpleImmutableEntry('S', TimeUnit.SECONDS), + new SimpleImmutableEntry('M', TimeUnit.MINUTES), + new SimpleImmutableEntry('H', TimeUnit.HOURS))); + + private static final Map UNITS = createUnits(); + + private static Map createUnits() { + Map units = new HashMap(); + for (Entry unit : SERIALIZE_ORDER) { + units.put(unit.getKey(), unit.getValue()); + } + return Collections.unmodifiableMap(units); + } @Override public String toAsciiString(Long timeoutNanos) { checkArgument(timeoutNanos >= 0, "Negative timeout"); // the smallest integer with 9 digits int cutoff = 100000000; - for (Entry unit : UNITS.entrySet()) { + for (Entry unit : SERIALIZE_ORDER) { long timeout = unit.getValue().convert(timeoutNanos, TimeUnit.NANOSECONDS); if (timeout < cutoff) { return Long.toString(timeout) + unit.getKey(); diff --git a/core/src/main/java/io/grpc/internal/Http2Ping.java b/core/src/main/java/io/grpc/internal/Http2Ping.java index 150cdbd5de..2b4df888af 100644 --- a/core/src/main/java/io/grpc/internal/Http2Ping.java +++ b/core/src/main/java/io/grpc/internal/Http2Ping.java @@ -32,10 +32,10 @@ package io.grpc.internal; import com.google.common.base.Stopwatch; -import com.google.common.collect.Maps; import io.grpc.internal.ClientTransport.PingCallback; +import java.util.LinkedHashMap; import java.util.Map; import java.util.concurrent.Executor; import java.util.concurrent.TimeUnit; @@ -68,7 +68,8 @@ public class Http2Ping { /** * The registered callbacks and the executor used to invoke them. */ - @GuardedBy("this") private Map callbacks = Maps.newLinkedHashMap(); + @GuardedBy("this") private Map callbacks + = new LinkedHashMap(); /** * False until the operation completes, either successfully (other side sent acknowledgement) or diff --git a/core/src/main/java/io/grpc/internal/InternalHandlerRegistry.java b/core/src/main/java/io/grpc/internal/InternalHandlerRegistry.java index 72ea6b12b4..7a228f98d4 100644 --- a/core/src/main/java/io/grpc/internal/InternalHandlerRegistry.java +++ b/core/src/main/java/io/grpc/internal/InternalHandlerRegistry.java @@ -31,18 +31,19 @@ package io.grpc.internal; -import com.google.common.collect.ImmutableMap; - import io.grpc.ServerMethodDefinition; import io.grpc.ServerServiceDefinition; +import java.util.Collections; import java.util.HashMap; +import java.util.Map; + import javax.annotation.Nullable; final class InternalHandlerRegistry { - private final ImmutableMap> methods; + private final Map> methods; - private InternalHandlerRegistry(ImmutableMap> methods) { + private InternalHandlerRegistry(Map> methods) { this.methods = methods; } @@ -62,14 +63,14 @@ final class InternalHandlerRegistry { } InternalHandlerRegistry build() { - ImmutableMap.Builder> mapBuilder = - ImmutableMap.builder(); + Map> map = + new HashMap>(); for (ServerServiceDefinition service : services.values()) { for (ServerMethodDefinition method : service.getMethods()) { - mapBuilder.put(method.getMethodDescriptor().getFullMethodName(), method); + map.put(method.getMethodDescriptor().getFullMethodName(), method); } } - return new InternalHandlerRegistry(mapBuilder.build()); + return new InternalHandlerRegistry(Collections.unmodifiableMap(map)); } } } diff --git a/core/src/main/java/io/grpc/internal/RoundRobinServerList.java b/core/src/main/java/io/grpc/internal/RoundRobinServerList.java index 7357e8875b..9fe8212682 100644 --- a/core/src/main/java/io/grpc/internal/RoundRobinServerList.java +++ b/core/src/main/java/io/grpc/internal/RoundRobinServerList.java @@ -32,7 +32,6 @@ package io.grpc.internal; import com.google.common.annotations.VisibleForTesting; -import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterators; import io.grpc.EquivalentAddressGroup; @@ -40,7 +39,9 @@ import io.grpc.Status; import io.grpc.TransportManager; import java.net.SocketAddress; +import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.Iterator; import java.util.List; @@ -95,8 +96,7 @@ public class RoundRobinServerList { @NotThreadSafe public static class Builder { - private final ImmutableList.Builder listBuilder = - ImmutableList.builder(); + private final List list = new ArrayList(); private final TransportManager tm; public Builder(TransportManager tm) { @@ -107,7 +107,7 @@ public class RoundRobinServerList { * Adds a server to the list, or {@code null} for a drop entry. */ public Builder addSocketAddress(@Nullable SocketAddress address) { - listBuilder.add(new EquivalentAddressGroup(address)); + list.add(new EquivalentAddressGroup(address)); return this; } @@ -117,7 +117,7 @@ public class RoundRobinServerList { * @param addresses the addresses to add */ public Builder add(EquivalentAddressGroup addresses) { - listBuilder.add(addresses); + list.add(addresses); return this; } @@ -127,12 +127,13 @@ public class RoundRobinServerList { * @param addresses the list of addresses group. */ public Builder addAll(Collection addresses) { - listBuilder.addAll(addresses); + list.addAll(addresses); return this; } public RoundRobinServerList build() { - return new RoundRobinServerList(tm, listBuilder.build()); + return new RoundRobinServerList(tm, + Collections.unmodifiableList(new ArrayList(list))); } } } diff --git a/core/src/test/java/io/grpc/MetadataTest.java b/core/src/test/java/io/grpc/MetadataTest.java index 274496594a..e741f47472 100644 --- a/core/src/test/java/io/grpc/MetadataTest.java +++ b/core/src/test/java/io/grpc/MetadataTest.java @@ -114,6 +114,18 @@ public class MetadataTest { assertEquals(null, metadata.get(KEY)); } + @Test + public void testGetAllNoRemove() { + Fish lance = new Fish(LANCE); + Metadata metadata = new Metadata(); + metadata.put(KEY, lance); + Iterator i = metadata.getAll(KEY).iterator(); + assertSame(lance, i.next()); + + thrown.expect(UnsupportedOperationException.class); + i.remove(); + } + @Test public void testWriteParsed() { Fish lance = new Fish(LANCE); diff --git a/core/src/test/java/io/grpc/ServerServiceDefinitionTest.java b/core/src/test/java/io/grpc/ServerServiceDefinitionTest.java index dbc7c365c0..00611c3969 100644 --- a/core/src/test/java/io/grpc/ServerServiceDefinitionTest.java +++ b/core/src/test/java/io/grpc/ServerServiceDefinitionTest.java @@ -161,7 +161,8 @@ public class ServerServiceDefinitionTest { .build(); assertEquals(Collections.>emptyList(), ssd.getServiceDescriptor().getMethods()); - assertEquals(Collections.>emptySet(), ssd.getMethods()); + assertEquals(Collections.>emptySet(), + new HashSet>(ssd.getMethods())); } private static class NoopServerCallHandler