Trigger R8's ServiceLoader optimization

This simplifies R8 Full Mode's configuration when paired with R8
optimizations (which is required in AGP 9), as when the optimization is
triggered Full Mode will automatically keep the constructor for the
relevant classes.

android-interop-test doesn't currently enable R8 optimizations, so it
doesn't actually demonstrate the benefit, but I have manually confirmed
that enabling proguard-android-optimize.txt does cause the ServiceLoader
optimization in android-interop-test. Note that full mode is a separate
configuration and not necessary to get the ServiceLoader optimization.

Author: ejona86

api/src/main/java/io/grpc/InternalServiceProviders.java

@@ -17,7 +17,9 @@
 package io.grpc;
 
 import com.google.common.annotations.VisibleForTesting;
+import java.util.Iterator;
 import java.util.List;
+import java.util.ServiceLoader;
 
 @Internal
 public final class InternalServiceProviders {
@@ -27,12 +29,28 @@ private InternalServiceProviders() {
   /**
    * Accessor for method.
    */
+  @Deprecated
   public static <T> List<T> loadAll(
       Class<T> klass,
       Iterable<Class<?>> hardCodedClasses,
       ClassLoader classLoader,
       PriorityAccessor<T> priorityAccessor) {
-    return ServiceProviders.loadAll(klass, hardCodedClasses, classLoader, priorityAccessor);
+    return loadAll(
+        klass,
+        ServiceLoader.load(klass, classLoader).iterator(),
+        hardCodedClasses,
+        priorityAccessor);
+  }
+
+  /**
+   * Accessor for method.
+   */
+  public static <T> List<T> loadAll(
+      Class<T> klass,
+      Iterator<T> serviceLoader,
+      Iterable<Class<?>> hardCodedClasses,
+      PriorityAccessor<T> priorityAccessor) {
+    return ServiceProviders.loadAll(klass, serviceLoader, hardCodedClasses, priorityAccessor);
   }
 
   /**

api/src/main/java/io/grpc/LoadBalancerRegistry.java

@@ -26,6 +26,7 @@
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.ServiceLoader;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import javax.annotation.Nullable;
@@ -101,8 +102,10 @@ public static synchronized LoadBalancerRegistry getDefaultRegistry() {
     if (instance == null) {
       List<LoadBalancerProvider> providerList = ServiceProviders.loadAll(
           LoadBalancerProvider.class,
+          ServiceLoader
+            .load(LoadBalancerProvider.class, LoadBalancerProvider.class.getClassLoader())
+            .iterator(),
           HARDCODED_CLASSES,
-          LoadBalancerProvider.class.getClassLoader(),
           new LoadBalancerPriorityAccessor());
       instance = new LoadBalancerRegistry();
       for (LoadBalancerProvider provider : providerList) {

api/src/main/java/io/grpc/ManagedChannelRegistry.java

@@ -29,6 +29,7 @@
 import java.util.Comparator;
 import java.util.LinkedHashSet;
 import java.util.List;
+import java.util.ServiceLoader;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import javax.annotation.concurrent.ThreadSafe;
@@ -100,8 +101,10 @@ public static synchronized ManagedChannelRegistry getDefaultRegistry() {
     if (instance == null) {
       List<ManagedChannelProvider> providerList = ServiceProviders.loadAll(
           ManagedChannelProvider.class,
+          ServiceLoader
+            .load(ManagedChannelProvider.class, ManagedChannelProvider.class.getClassLoader())
+            .iterator(),
           getHardCodedClasses(),
-          ManagedChannelProvider.class.getClassLoader(),
           new ManagedChannelPriorityAccessor());
       instance = new ManagedChannelRegistry();
       for (ManagedChannelProvider provider : providerList) {

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<NameResolverProvider> 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 "

api/src/main/java/io/grpc/ServerRegistry.java

@@ -24,6 +24,7 @@
 import java.util.Comparator;
 import java.util.LinkedHashSet;
 import java.util.List;
+import java.util.ServiceLoader;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import javax.annotation.concurrent.ThreadSafe;
@@ -93,8 +94,9 @@ public static synchronized ServerRegistry getDefaultRegistry() {
     if (instance == null) {
       List<ServerProvider> providerList = ServiceProviders.loadAll(
           ServerProvider.class,
+          ServiceLoader.load(ServerProvider.class, ServerProvider.class.getClassLoader())
+            .iterator(),
           getHardCodedClasses(),
-          ServerProvider.class.getClassLoader(),
           new ServerPriorityAccessor());
       instance = new ServerRegistry();
       for (ServerProvider provider : providerList) {

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;
 
@@ -34,20 +36,39 @@ private ServiceProviders() {
    * {@link ServiceLoader}.
    * If this is Android, returns all available implementations in {@code hardcoded}.
    * The list is sorted in descending priority order.
+   *
+   * <p>{@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 <T> List<T> loadAll(
       Class<T> klass,
+      Iterator<T> serviceLoader,
       Iterable<Class<?>> hardcoded,
-      ClassLoader cl,
       final PriorityAccessor<T> priorityAccessor) {
-    Iterable<T> candidates;
-    if (isAndroid(cl)) {
-      candidates = getCandidatesViaHardCoded(klass, hardcoded);
+    Iterator<T> 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<T> list = new ArrayList<>();
-    for (T current: candidates) {
+    while (candidates.hasNext()) {
+      T current = candidates.next();
       if (!priorityAccessor.isAvailable(current)) {
         continue;
       }
@@ -84,15 +105,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 <T> Iterable<T> getCandidatesViaServiceLoader(Class<T> klass, ClassLoader cl) {
     Iterable<T> 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;
   }

api/src/test/java/io/grpc/ServiceProvidersTest.java

@@ -29,6 +29,7 @@
 import java.util.Iterator;
 import java.util.List;
 import java.util.ServiceConfigurationError;
+import java.util.ServiceLoader;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
@@ -98,7 +99,7 @@ public void multipleProvider() throws Exception {
         Available7Provider.class,
         load(ServiceProvidersTestAbstractProvider.class, NO_HARDCODED, cl, ACCESSOR).getClass());
 
-    List<ServiceProvidersTestAbstractProvider> providers = ServiceProviders.loadAll(
+    List<ServiceProvidersTestAbstractProvider> providers = loadAll(
         ServiceProvidersTestAbstractProvider.class, NO_HARDCODED, cl, ACCESSOR);
     assertEquals(3, providers.size());
     assertEquals(Available7Provider.class, providers.get(0).getClass());
@@ -121,8 +122,7 @@ public void unknownClassProvider() {
     ClassLoader cl = new ReplacingClassLoader(getClass().getClassLoader(), serviceFile,
         "io/grpc/ServiceProvidersTestAbstractProvider-unknownClassProvider.txt");
     try {
-      ServiceProviders.loadAll(
-          ServiceProvidersTestAbstractProvider.class, NO_HARDCODED, cl, ACCESSOR);
+      loadAll(ServiceProvidersTestAbstractProvider.class, NO_HARDCODED, cl, ACCESSOR);
       fail("Exception expected");
     } catch (ServiceConfigurationError e) {
       // noop
@@ -136,8 +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.loadAll(
-          ServiceProvidersTestAbstractProvider.class, NO_HARDCODED, cl, ACCESSOR);
+      loadAll(ServiceProvidersTestAbstractProvider.class, NO_HARDCODED, cl, ACCESSOR);
       fail("Expected exception");
     } catch (ServiceConfigurationError expected) {
       // noop
@@ -150,8 +149,7 @@ public void exceptionSurfacedToCaller_failAtPriority() {
         "io/grpc/ServiceProvidersTestAbstractProvider-failAtPriorityProvider.txt");
     try {
       // The exception should be surfaced to the caller
-      ServiceProviders.loadAll(
-          ServiceProvidersTestAbstractProvider.class, NO_HARDCODED, cl, ACCESSOR);
+      loadAll(ServiceProvidersTestAbstractProvider.class, NO_HARDCODED, cl, ACCESSOR);
       fail("Expected exception");
     } catch (FailAtPriorityProvider.PriorityException expected) {
       // noop
@@ -164,8 +162,7 @@ public void exceptionSurfacedToCaller_failAtAvailable() {
         "io/grpc/ServiceProvidersTestAbstractProvider-failAtAvailableProvider.txt");
     try {
       // The exception should be surfaced to the caller
-      ServiceProviders.loadAll(
-          ServiceProvidersTestAbstractProvider.class, NO_HARDCODED, cl, ACCESSOR);
+      loadAll(ServiceProvidersTestAbstractProvider.class, NO_HARDCODED, cl, ACCESSOR);
       fail("Expected exception");
     } catch (FailAtAvailableProvider.AvailableException expected) {
       // noop
@@ -245,13 +242,25 @@ private static <T> T load(
       Iterable<Class<?>> hardcoded,
       ClassLoader cl,
       PriorityAccessor<T> priorityAccessor) {
-    List<T> candidates = ServiceProviders.loadAll(klass, hardcoded, cl, priorityAccessor);
+    List<T> candidates = loadAll(klass, hardcoded, cl, priorityAccessor);
     if (candidates.isEmpty()) {
       return null;
     }
     return candidates.get(0);
   }
 
+  private static <T> List<T> loadAll(
+      Class<T> klass,
+      Iterable<Class<?>> hardcoded,
+      ClassLoader classLoader,
+      PriorityAccessor<T> priorityAccessor) {
+    return ServiceProviders.loadAll(
+        klass,
+        ServiceLoader.load(klass, classLoader).iterator(),
+        hardcoded,
+        priorityAccessor);
+  }
+
   private static class BaseProvider extends ServiceProvidersTestAbstractProvider {
     private final boolean isAvailable;
     private final int priority;

xds/src/main/java/io/grpc/xds/XdsCredentialsRegistry.java

@@ -29,6 +29,7 @@
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.ServiceLoader;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import javax.annotation.Nullable;
@@ -109,8 +110,10 @@ public static synchronized XdsCredentialsRegistry getDefaultRegistry() {
     if (instance == null) {
       List<XdsCredentialsProvider> providerList = InternalServiceProviders.loadAll(
               XdsCredentialsProvider.class,
+              ServiceLoader
+                .load(XdsCredentialsProvider.class, XdsCredentialsProvider.class.getClassLoader())
+                .iterator(),
               getHardCodedClasses(),
-              XdsCredentialsProvider.class.getClassLoader(),
               new XdsCredentialsProviderPriorityAccessor());
       if (providerList.isEmpty()) {
         logger.warning("No XdsCredsRegistry found via ServiceLoader, including for GoogleDefault, "