Simplify the ProvisionCallback API.

Every implementation needs access to the current `InternalContext` so we can save a bit of memory by just passing it. This is pretty minor since `ProvisionListeners` are rare, but it satisfies a long running TODO and allows us to eliminate some allocations from the normal provisioning path. PiperOrigin-RevId: 712269258
google · Jan 14, 2025 · 9842e3d · 9842e3d
1 parent c4401b8
commit 9842e3d
Show file tree

Hide file tree

Showing 7 changed files with 55 additions and 65 deletions.
diff --git a/core/src/com/google/inject/internal/ConstructorInjector.java b/core/src/com/google/inject/internal/ConstructorInjector.java
@@ -30,7 +30,7 @@
  *
  * @author crazybob@google.com (Bob Lee)
  */
-final class ConstructorInjector<T> {
+final class ConstructorInjector<T> implements ProvisionCallback<T> {
 
   private final ImmutableSet<InjectionPoint> injectableMembers;
   private final SingleParameterInjector<?>[] parameterInjectors;
@@ -82,17 +82,17 @@ Object construct(
     } else {
       // NOTE: `provision` always calls the callback, even if provision listeners
       // throw exceptions.
-      return provisionCallback.provision(
-          context,
-          new ProvisionCallback<T>() {
-            @Override
-            public T call() throws InternalProvisionException {
-              return provision(context);
-            }
-          });
+      return provisionCallback.provision(context, dependency, this);
     }
   }
 
+  // Implements ProvisionCallback<T>
+  @Override
+  public final T call(InternalContext context, Dependency<?> dependency)
+      throws InternalProvisionException {
+    return provision(context);
+  }
+
   /** Provisions a new T. */
   private T provision(InternalContext context) throws InternalProvisionException {
     MembersInjectorImpl<T> localMembersInjector = membersInjector;

diff --git a/core/src/com/google/inject/internal/InternalProviderInstanceBindingImpl.java b/core/src/com/google/inject/internal/InternalProviderInstanceBindingImpl.java
@@ -3,7 +3,6 @@
 import com.google.common.collect.ImmutableSet;
 import com.google.inject.Key;
 import com.google.inject.Provider;
-import com.google.inject.internal.ProvisionListenerStackCallback.ProvisionCallback;
 import com.google.inject.spi.Dependency;
 import com.google.inject.spi.HasDependencies;
 import com.google.inject.spi.InjectionPoint;
@@ -64,7 +63,11 @@ public void initialize(final InjectorImpl injector, final Errors errors) throws
   }
 
   /** A base factory implementation. */
-  abstract static class Factory<T> implements InternalFactory<T>, Provider<T>, HasDependencies {
+  abstract static class Factory<T>
+      implements InternalFactory<T>,
+          Provider<T>,
+          HasDependencies,
+          ProvisionListenerStackCallback.ProvisionCallback<T> {
     private final InitializationTiming initializationTiming;
     private Object source;
     private Provider<T> delegateProvider;
@@ -108,16 +111,17 @@ public T get(final InternalContext context, final Dependency<?> dependency, bool
       if (provisionCallback == null) {
         return doProvision(context, dependency);
       } else {
-        return provisionCallback.provision(
-            context,
-            new ProvisionCallback<T>() {
-              @Override
-              public T call() throws InternalProvisionException {
-                return doProvision(context, dependency);
-              }
-            });
+        return provisionCallback.provision(context, dependency, this);
       }
     }
+
+    // Implements ProvisionCallback<T>
+    @Override
+    public final T call(InternalContext context, Dependency<?> dependency)
+        throws InternalProvisionException {
+      return doProvision(context, dependency);
+    }
+
     /**
      * Creates an object to be injected.
      *

diff --git a/core/src/com/google/inject/internal/MembersInjectorImpl.java b/core/src/com/google/inject/internal/MembersInjectorImpl.java
@@ -20,7 +20,6 @@
 import com.google.common.collect.ImmutableSet;
 import com.google.inject.MembersInjector;
 import com.google.inject.TypeLiteral;
-import com.google.inject.internal.ProvisionListenerStackCallback.ProvisionCallback;
 import com.google.inject.spi.InjectionListener;
 import com.google.inject.spi.InjectionPoint;
 import javax.annotation.Nullable;
@@ -87,12 +86,10 @@ void injectAndNotify(
     if (provisionCallback != null) {
       provisionCallback.provision(
           context,
-          new ProvisionCallback<T>() {
-            @Override
-            public T call() throws InternalProvisionException {
-              injectMembers(instance, context, toolableOnly);
-              return instance;
-            }
+          /* dependency= */ null, // not used
+          (ctx, dep) -> {
+            injectMembers(instance, ctx, toolableOnly);
+            return instance;
           });
     } else {
       injectMembers(instance, context, toolableOnly);

diff --git a/core/src/com/google/inject/internal/ProviderInternalFactory.java b/core/src/com/google/inject/internal/ProviderInternalFactory.java
@@ -18,7 +18,6 @@
 
 import static com.google.common.base.Preconditions.checkNotNull;
 
-import com.google.inject.internal.ProvisionListenerStackCallback.ProvisionCallback;
 import com.google.inject.spi.Dependency;
 import javax.annotation.Nullable;
 import jakarta.inject.Provider;
@@ -55,14 +54,8 @@ protected T circularGet(
       return provision(provider, context, dependency);
     } else {
       return provisionCallback.provision(
-          context,
-          new ProvisionCallback<T>() {
-            @Override
-            public T call() throws InternalProvisionException {
-              return provision(provider, context, dependency);
-            }
-          });
-      }
+          context, dependency, (ctx, dep) -> provision(provider, ctx, dep));
+    }
   }
 
   /**

diff --git a/core/src/com/google/inject/internal/ProviderMethod.java b/core/src/com/google/inject/internal/ProviderMethod.java
@@ -24,7 +24,6 @@
 import com.google.inject.PrivateBinder;
 import com.google.inject.Provides;
 import com.google.inject.internal.InternalProviderInstanceBindingImpl.InitializationTiming;
-import com.google.inject.internal.ProvisionListenerStackCallback.ProvisionCallback;
 import com.google.inject.internal.util.StackTraceElements;
 import com.google.inject.spi.BindingTargetVisitor;
 import com.google.inject.spi.Dependency;
@@ -178,22 +177,11 @@ public final T get(final InternalContext context, final Dependency<?> dependency
       // We have a circular reference between bindings. Return a proxy.
       return result;
     }
-    // Optimization: Don't go through the callback stack if no one's listening.
-    if (provisionCallback == null) {
-      return provision(dependency, context);
-    } else {
-      return provisionCallback.provision(
-          context,
-          new ProvisionCallback<T>() {
-            @Override
-            public T call() throws InternalProvisionException {
-              return provision(dependency, context);
-            }
-          });
-    }
+    return super.get(context, dependency, linked);
   }
 
-  private T provision(Dependency<?> dependency, InternalContext context)
+  @Override
+  protected T doProvision(InternalContext context, Dependency<?> dependency)
       throws InternalProvisionException {
     T t = null;
     try {
@@ -216,12 +204,6 @@ private T provision(Dependency<?> dependency, InternalContext context)
     }
   }
 
-  @Override
-  protected T doProvision(InternalContext context, Dependency<?> dependency)
-      throws InternalProvisionException {
-    throw new AssertionError(); // should never be called since we override get()
-  }
-
   /** Extension point for our subclasses to implement the provisioning strategy. */
   abstract T doProvision(Object[] parameters)
       throws IllegalAccessException, InvocationTargetException;

diff --git a/core/src/com/google/inject/internal/ProvisionListenerStackCallback.java b/core/src/com/google/inject/internal/ProvisionListenerStackCallback.java
@@ -18,7 +18,9 @@
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Sets;
+import com.google.errorprone.annotations.CanIgnoreReturnValue;
 import com.google.inject.Binding;
+import com.google.inject.spi.Dependency;
 import com.google.inject.spi.ProvisionListener;
 import java.util.List;
 import java.util.Set;
@@ -58,9 +60,11 @@ public boolean hasListeners() {
     return listeners.length > 0;
   }
 
-  public T provision(InternalContext context, ProvisionCallback<T> callable)
+  @CanIgnoreReturnValue
+  public T provision(
+      InternalContext context, Dependency<?> dependency, ProvisionCallback<T> callable)
       throws InternalProvisionException {
-    Provision provision = new Provision(callable);
+    Provision provision = new Provision(context, dependency, callable);
     RuntimeException caught = null;
     try {
       provision.provision();
@@ -85,19 +89,22 @@ public T provision(InternalContext context, ProvisionCallback<T> callable)
     }
   }
 
-  // TODO(sameb): Can this be more InternalFactory-like?
-  public interface ProvisionCallback<T> {
-    public T call() throws InternalProvisionException;
+  interface ProvisionCallback<T> {
+    T call(InternalContext context, Dependency<?> dependency) throws InternalProvisionException;
   }
 
-  private class Provision extends ProvisionListener.ProvisionInvocation<T> {
+  private final class Provision extends ProvisionListener.ProvisionInvocation<T> {
     final ProvisionCallback<T> callable;
+    final InternalContext context;
+    final Dependency<?> dependency;
     int index = -1;
     T result;
     InternalProvisionException exceptionDuringProvision;
     ProvisionListener erredListener;
 
-    public Provision(ProvisionCallback<T> callable) {
+    Provision(InternalContext context, Dependency<?> dependency, ProvisionCallback<T> callable) {
+      this.context = context;
+      this.dependency = dependency;
       this.callable = callable;
     }
 
@@ -106,7 +113,7 @@ public T provision() {
       index++;
       if (index == listeners.length) {
         try {
-          result = callable.call();
+          result = callable.call(context, dependency);
         } catch (InternalProvisionException ipe) {
           exceptionDuringProvision = ipe;
           throw ipe.toProvisionException();

diff --git a/core/src/com/google/inject/internal/SingleParameterInjector.java b/core/src/com/google/inject/internal/SingleParameterInjector.java
@@ -56,8 +56,15 @@ static Object[] getAll(InternalContext context, SingleParameterInjector<?>[] par
     Object[] parameters = new Object[size];
 
     // optimization: use manual for/each to save allocating an iterator here
+    Dependency<?> dependency = null;
+    try {
     for (int i = 0; i < size; i++) {
-      parameters[i] = parameterInjectors[i].inject(context);
+        SingleParameterInjector<?> injector = parameterInjectors[i];
+        dependency = injector.dependency;
+        parameters[i] = injector.factory.get(context, dependency, false);
+      }
+    } catch (InternalProvisionException ipe) {
+      throw ipe.addSource(dependency);
     }
     return parameters;
   }