diff --git a/api/src/main/java/io/grpc/InternalServiceProviders.java b/api/src/main/java/io/grpc/InternalServiceProviders.java index c72e01db67a..d73604774f3 100644 --- a/api/src/main/java/io/grpc/InternalServiceProviders.java +++ b/api/src/main/java/io/grpc/InternalServiceProviders.java @@ -17,6 +17,7 @@ package io.grpc; import com.google.common.annotations.VisibleForTesting; +import java.util.Iterator; import java.util.List; @Internal @@ -27,12 +28,13 @@ private InternalServiceProviders() { /** * Accessor for method. */ - public static T load( + //@Deprecated + public static List loadAll( Class klass, - Iterable> hardcoded, + Iterable> hardCodedClasses, ClassLoader classLoader, PriorityAccessor priorityAccessor) { - return ServiceProviders.load(klass, hardcoded, classLoader, priorityAccessor); + return ServiceProviders.loadAll(klass, hardCodedClasses, classLoader, priorityAccessor); } /** @@ -40,10 +42,10 @@ public static T load( */ public static List loadAll( Class klass, + Iterator serviceLoader, Iterable> hardCodedClasses, - ClassLoader classLoader, PriorityAccessor priorityAccessor) { - return ServiceProviders.loadAll(klass, hardCodedClasses, classLoader, priorityAccessor); + return ServiceProviders.loadAll(klass, serviceLoader, hardCodedClasses, priorityAccessor); } /** diff --git a/api/src/main/java/io/grpc/NameResolverRegistry.java b/api/src/main/java/io/grpc/NameResolverRegistry.java index 26eb5552b9b..4b536a8c8ea 100644 --- a/api/src/main/java/io/grpc/NameResolverRegistry.java +++ b/api/src/main/java/io/grpc/NameResolverRegistry.java @@ -29,6 +29,7 @@ import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.ServiceLoader; import java.util.logging.Level; import java.util.logging.Logger; import javax.annotation.Nullable; @@ -125,8 +126,10 @@ public static synchronized NameResolverRegistry getDefaultRegistry() { if (instance == null) { List providerList = ServiceProviders.loadAll( NameResolverProvider.class, + ServiceLoader + .load(NameResolverProvider.class, NameResolverProvider.class.getClassLoader()) + .iterator(), getHardCodedClasses(), - NameResolverProvider.class.getClassLoader(), new NameResolverPriorityAccessor()); if (providerList.isEmpty()) { logger.warning("No NameResolverProviders found via ServiceLoader, including for DNS. This " diff --git a/api/src/main/java/io/grpc/ServiceProviders.java b/api/src/main/java/io/grpc/ServiceProviders.java index ac4b27d8783..afd30f362a5 100644 --- a/api/src/main/java/io/grpc/ServiceProviders.java +++ b/api/src/main/java/io/grpc/ServiceProviders.java @@ -20,7 +20,9 @@ import java.util.ArrayList; import java.util.Collections; import java.util.Comparator; +import java.util.Iterator; import java.util.List; +import java.util.ListIterator; import java.util.ServiceConfigurationError; import java.util.ServiceLoader; @@ -30,20 +32,18 @@ private ServiceProviders() { } /** - * If this is not Android, returns the highest priority implementation of the class via + * If this is not Android, returns all available implementations discovered via * {@link ServiceLoader}. - * If this is Android, returns an instance of the highest priority class in {@code hardcoded}. + * If this is Android, returns all available implementations in {@code hardcoded}. + * The list is sorted in descending priority order. */ - public static T load( + //@Deprecated + public static List loadAll( Class klass, Iterable> hardcoded, ClassLoader cl, - PriorityAccessor priorityAccessor) { - List candidates = loadAll(klass, hardcoded, cl, priorityAccessor); - if (candidates.isEmpty()) { - return null; - } - return candidates.get(0); + final PriorityAccessor priorityAccessor) { + return loadAll(klass, ServiceLoader.load(klass, cl).iterator(), hardcoded, priorityAccessor); } /** @@ -51,20 +51,39 @@ public static T load( * {@link ServiceLoader}. * If this is Android, returns all available implementations in {@code hardcoded}. * The list is sorted in descending priority order. + * + *

{@code serviceLoader} should be created with {@code ServiceLoader.load(MyClass.class, + * MyClass.class.getClassLoader()).iterator()} in order to be detected by R8 so that R8 full mode + * will keep the constructors for the provider classes. */ public static List loadAll( Class klass, + Iterator serviceLoader, Iterable> hardcoded, - ClassLoader cl, final PriorityAccessor priorityAccessor) { - Iterable candidates; - if (isAndroid(cl)) { - candidates = getCandidatesViaHardCoded(klass, hardcoded); + Iterator candidates; + if (serviceLoader instanceof ListIterator) { + // A rewriting tool has replaced the ServiceLoader with a List of some sort (R8 uses + // ArrayList, AppReduce uses singletonList). We prefer to use such iterators on Android as + // they won't need reflection like the hard-coded list does. In addition, the provider + // instances will have already been created, so it seems we should use them. + // + // R8: https://r8.googlesource.com/r8/+/490bc53d9310d4cc2a5084c05df4aadaec8c885d/src/main/java/com/android/tools/r8/ir/optimize/ServiceLoaderRewriter.java + // AppReduce: service_loader_pass.cc + candidates = serviceLoader; + } else if (isAndroid(klass.getClassLoader())) { + // Avoid getResource() on Android, which must read from a zip which uses a lot of memory + candidates = getCandidatesViaHardCoded(klass, hardcoded).iterator(); + } else if (!serviceLoader.hasNext()) { + // Attempt to load using the context class loader and ServiceLoader. + // This allows frameworks like http://aries.apache.org/modules/spi-fly.html to plug in. + candidates = ServiceLoader.load(klass).iterator(); } else { - candidates = getCandidatesViaServiceLoader(klass, cl); + candidates = serviceLoader; } List list = new ArrayList<>(); - for (T current: candidates) { + while (candidates.hasNext()) { + T current = candidates.next(); if (!priorityAccessor.isAvailable(current)) { continue; } @@ -101,15 +120,14 @@ static boolean isAndroid(ClassLoader cl) { } /** - * Loads service providers for the {@code klass} service using {@link ServiceLoader}. + * For testing only: Loads service providers for the {@code klass} service using {@link + * ServiceLoader}. Does not support spi-fly and related tricks. */ @VisibleForTesting public static Iterable getCandidatesViaServiceLoader(Class klass, ClassLoader cl) { Iterable i = ServiceLoader.load(klass, cl); - // Attempt to load using the context class loader and ServiceLoader. - // This allows frameworks like http://aries.apache.org/modules/spi-fly.html to plug in. if (!i.iterator().hasNext()) { - i = ServiceLoader.load(klass); + return null; } return i; } diff --git a/api/src/test/java/io/grpc/ServiceProvidersTest.java b/api/src/test/java/io/grpc/ServiceProvidersTest.java index 7d4388a5bb9..63809351c58 100644 --- a/api/src/test/java/io/grpc/ServiceProvidersTest.java +++ b/api/src/test/java/io/grpc/ServiceProvidersTest.java @@ -69,8 +69,7 @@ public void contextClassLoaderProvider() { Thread.currentThread().setContextClassLoader(rcll); assertEquals( Available7Provider.class, - ServiceProviders.load( - ServiceProvidersTestAbstractProvider.class, NO_HARDCODED, cl, ACCESSOR).getClass()); + load(ServiceProvidersTestAbstractProvider.class, NO_HARDCODED, cl, ACCESSOR).getClass()); } finally { Thread.currentThread().setContextClassLoader(ccl); } @@ -85,8 +84,7 @@ public void noProvider() { serviceFile, "io/grpc/ServiceProvidersTestAbstractProvider-doesNotExist.txt"); Thread.currentThread().setContextClassLoader(cl); - assertNull(ServiceProviders.load( - ServiceProvidersTestAbstractProvider.class, NO_HARDCODED, cl, ACCESSOR)); + assertNull(load(ServiceProvidersTestAbstractProvider.class, NO_HARDCODED, cl, ACCESSOR)); } finally { Thread.currentThread().setContextClassLoader(ccl); } @@ -98,8 +96,7 @@ public void multipleProvider() throws Exception { "io/grpc/ServiceProvidersTestAbstractProvider-multipleProvider.txt"); assertSame( Available7Provider.class, - ServiceProviders.load( - ServiceProvidersTestAbstractProvider.class, NO_HARDCODED, cl, ACCESSOR).getClass()); + load(ServiceProvidersTestAbstractProvider.class, NO_HARDCODED, cl, ACCESSOR).getClass()); List providers = ServiceProviders.loadAll( ServiceProvidersTestAbstractProvider.class, NO_HARDCODED, cl, ACCESSOR); @@ -116,8 +113,7 @@ public void unavailableProvider() { "io/grpc/ServiceProvidersTestAbstractProvider-unavailableProvider.txt"); assertEquals( Available7Provider.class, - ServiceProviders.load( - ServiceProvidersTestAbstractProvider.class, NO_HARDCODED, cl, ACCESSOR).getClass()); + load(ServiceProvidersTestAbstractProvider.class, NO_HARDCODED, cl, ACCESSOR).getClass()); } @Test @@ -125,7 +121,7 @@ public void unknownClassProvider() { ClassLoader cl = new ReplacingClassLoader(getClass().getClassLoader(), serviceFile, "io/grpc/ServiceProvidersTestAbstractProvider-unknownClassProvider.txt"); try { - ServiceProviders.load( + ServiceProviders.loadAll( ServiceProvidersTestAbstractProvider.class, NO_HARDCODED, cl, ACCESSOR); fail("Exception expected"); } catch (ServiceConfigurationError e) { @@ -140,7 +136,7 @@ public void exceptionSurfacedToCaller_failAtInit() { try { // Even though there is a working provider, if any providers fail then we should fail // completely to avoid returning something unexpected. - ServiceProviders.load( + ServiceProviders.loadAll( ServiceProvidersTestAbstractProvider.class, NO_HARDCODED, cl, ACCESSOR); fail("Expected exception"); } catch (ServiceConfigurationError expected) { @@ -154,7 +150,7 @@ public void exceptionSurfacedToCaller_failAtPriority() { "io/grpc/ServiceProvidersTestAbstractProvider-failAtPriorityProvider.txt"); try { // The exception should be surfaced to the caller - ServiceProviders.load( + ServiceProviders.loadAll( ServiceProvidersTestAbstractProvider.class, NO_HARDCODED, cl, ACCESSOR); fail("Expected exception"); } catch (FailAtPriorityProvider.PriorityException expected) { @@ -168,7 +164,7 @@ public void exceptionSurfacedToCaller_failAtAvailable() { "io/grpc/ServiceProvidersTestAbstractProvider-failAtAvailableProvider.txt"); try { // The exception should be surfaced to the caller - ServiceProviders.load( + ServiceProviders.loadAll( ServiceProvidersTestAbstractProvider.class, NO_HARDCODED, cl, ACCESSOR); fail("Expected exception"); } catch (FailAtAvailableProvider.AvailableException expected) { @@ -244,6 +240,18 @@ class RandomClass {} assertFalse(candidates.iterator().hasNext()); } + private static T load( + Class klass, + Iterable> hardcoded, + ClassLoader cl, + PriorityAccessor priorityAccessor) { + List candidates = ServiceProviders.loadAll(klass, hardcoded, cl, priorityAccessor); + if (candidates.isEmpty()) { + return null; + } + return candidates.get(0); + } + private static class BaseProvider extends ServiceProvidersTestAbstractProvider { private final boolean isAvailable; private final int priority;