From 325ac9c863925d311119e917de8819c104f068ae Mon Sep 17 00:00:00 2001 From: Luke Sandberg Date: Tue, 14 Jan 2025 11:51:52 -0800 Subject: [PATCH] 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: 715467849 --- .../inject/internal/ConstructorInjector.java | 18 +++++++------- .../InternalProviderInstanceBindingImpl.java | 24 +++++++++++-------- .../inject/internal/MembersInjectorImpl.java | 11 ++++----- .../internal/ProviderInternalFactory.java | 11 ++------- .../inject/internal/ProviderMethod.java | 24 +++---------------- .../ProvisionListenerStackCallback.java | 23 +++++++++++------- .../internal/SingleParameterInjector.java | 9 ++++++- 7 files changed, 55 insertions(+), 65 deletions(-) diff --git a/core/src/com/google/inject/internal/ConstructorInjector.java b/core/src/com/google/inject/internal/ConstructorInjector.java index 2f61d3f16c..2671ab0065 100644 --- 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 { +final class ConstructorInjector implements ProvisionCallback { private final ImmutableSet 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() { - @Override - public T call() throws InternalProvisionException { - return provision(context); - } - }); + return provisionCallback.provision(context, dependency, this); } } + // Implements ProvisionCallback + @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 localMembersInjector = membersInjector; diff --git a/core/src/com/google/inject/internal/InternalProviderInstanceBindingImpl.java b/core/src/com/google/inject/internal/InternalProviderInstanceBindingImpl.java index 2a9accfc3c..574a6a2ec7 100644 --- 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 implements InternalFactory, Provider, HasDependencies { + abstract static class Factory + implements InternalFactory, + Provider, + HasDependencies, + ProvisionListenerStackCallback.ProvisionCallback { private final InitializationTiming initializationTiming; private Object source; private Provider 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() { - @Override - public T call() throws InternalProvisionException { - return doProvision(context, dependency); - } - }); + return provisionCallback.provision(context, dependency, this); } } + + // Implements ProvisionCallback + @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 index 9ad26a44a0..2292631db4 100644 --- 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() { - @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 index 14e38e60fd..9605294cfc 100644 --- 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() { - @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 index db9641433a..71aaa82a16 100644 --- 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() { - @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 index 74fbeeaa13..cde493d3ff 100644 --- 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 callable) + @CanIgnoreReturnValue + public T provision( + InternalContext context, Dependency dependency, ProvisionCallback 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 callable) } } - // TODO(sameb): Can this be more InternalFactory-like? - public interface ProvisionCallback { - public T call() throws InternalProvisionException; + interface ProvisionCallback { + T call(InternalContext context, Dependency dependency) throws InternalProvisionException; } - private class Provision extends ProvisionListener.ProvisionInvocation { + private final class Provision extends ProvisionListener.ProvisionInvocation { final ProvisionCallback callable; + final InternalContext context; + final Dependency dependency; int index = -1; T result; InternalProvisionException exceptionDuringProvision; ProvisionListener erredListener; - public Provision(ProvisionCallback callable) { + Provision(InternalContext context, Dependency dependency, ProvisionCallback 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 index b68e638311..0f39b751f7 100644 --- 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; }