From e47643b858b7483bc3f3f6bf74f584bf01c0cc84 Mon Sep 17 00:00:00 2001 From: Emmanuel Jimenez Date: Fri, 19 Dec 2025 09:52:32 -0800 Subject: [PATCH] Optimize Dagger Provisioning: Reuse Existing Framework Instances RELNOTES=N/A PiperOrigin-RevId: 846772103 --- .../ProvisionBindingRepresentation.java | 39 +++++++++++++++++-- .../test.DaggerTestComponent_DEFAULT_MODE | 6 +-- .../test.DaggerTestComponent_KT_DEFAULT_MODE | 10 ++--- .../test.DaggerTestComponent_DEFAULT_MODE | 6 +-- .../test.DaggerTestComponent_DEFAULT_MODE | 12 ++---- .../test.DaggerTestComponent_DEFAULT_MODE | 31 +++------------ .../test.DaggerTestComponent_DEFAULT_MODE | 23 ++--------- .../test.DaggerTestComponent_KT_DEFAULT_MODE | 23 ++--------- 8 files changed, 57 insertions(+), 93 deletions(-) diff --git a/dagger-compiler/main/java/dagger/internal/codegen/writing/ProvisionBindingRepresentation.java b/dagger-compiler/main/java/dagger/internal/codegen/writing/ProvisionBindingRepresentation.java index 35cdb0d2dc7..518d110b45b 100644 --- a/dagger-compiler/main/java/dagger/internal/codegen/writing/ProvisionBindingRepresentation.java +++ b/dagger-compiler/main/java/dagger/internal/codegen/writing/ProvisionBindingRepresentation.java @@ -26,6 +26,7 @@ import dagger.internal.codegen.binding.BindingRequest; import dagger.internal.codegen.binding.ContributionBinding; import dagger.internal.codegen.binding.DelegateBinding; +import dagger.internal.codegen.model.DependencyRequest; import dagger.internal.codegen.model.RequestKind; import dagger.internal.codegen.writing.ComponentImplementation.CompilerMode; @@ -80,6 +81,8 @@ private boolean usesDirectInstanceExpression(RequestKind requestKind) { } switch (binding.kind()) { + case SUBCOMPONENT_CREATOR: + return true; case MEMBERS_INJECTOR: // Currently, we always use a framework instance for MembersInjectors, e.g. // InstanceFactory.create(Foo_MembersInjector.create(...)). @@ -94,9 +97,19 @@ private boolean usesDirectInstanceExpression(RequestKind requestKind) { "Assisted injection binding shouldn't be requested with an instance request."); default: // We don't need to use Provider#get() if there's no caching, so use a direct instance. - // TODO(bcorso): This can be optimized in cases where we know a Provider field already - // exists, in which case even if it's not scoped we might as well call Provider#get(). - return !needsCaching(binding, graph); + // However, if there's no caching needed but we already have a framework instance requested + // for this binding, we can reuse that framework instance by calling Provider#get() instead + // of generating a direct instance. + // TODO(emjich): To be even more accurate, we should consider delegate bindings here + // too. For example, if we have: + // @Binds Foo -> @Binds FooIntermediate -> @Provides FooImpl + // Then we technically should be checking all of bindings for hasFrameworkRequest, + // e.g. if someone requests a Provider we should be able to reuse that same + // provider for FooIntermediate and FooImpl since they are all just delegates of + // each other. + return !needsCaching(binding, graph) + && !(graph.topLevelBindingGraph().hasFrameworkRequest(binding) + && bindingHasDependencies(binding, graph)); } } @@ -116,6 +129,26 @@ static boolean needsCaching(ContributionBinding binding, BindingGraph graph) { return true; } + /** + * Returns {@code true} if {@code binding} has dependencies. + * + *

If {@code binding} is a {@code DELEGATE}, it is only considered to have dependencies if the + * binding it delegates to has dependencies. Otherwise, a binding has dependencies if {@code + * binding.dependencies()} is not empty. + */ + private static boolean bindingHasDependencies(ContributionBinding binding, BindingGraph graph) { + if (binding.dependencies().isEmpty()) { + return false; + } + if (!binding.kind().equals(DELEGATE)) { + return true; + } + return binding.dependencies().stream() + .map(DependencyRequest::key) + .map(graph::contributionBinding) + .anyMatch(b -> bindingHasDependencies(b, graph)); + } + @AssistedFactory static interface Factory { ProvisionBindingRepresentation create(ContributionBinding binding); diff --git a/javatests/dagger/internal/codegen/goldens/ComponentProcessorTest/providerComponentType/test.DaggerTestComponent_DEFAULT_MODE b/javatests/dagger/internal/codegen/goldens/ComponentProcessorTest/providerComponentType/test.DaggerTestComponent_DEFAULT_MODE index 96dcdc3ca74..8b87e6daab3 100644 --- a/javatests/dagger/internal/codegen/goldens/ComponentProcessorTest/providerComponentType/test.DaggerTestComponent_DEFAULT_MODE +++ b/javatests/dagger/internal/codegen/goldens/ComponentProcessorTest/providerComponentType/test.DaggerTestComponent_DEFAULT_MODE @@ -50,10 +50,6 @@ public final class DaggerTestComponent { } - Foo foo() { - return new Foo(new Bar()); - } - @SuppressWarnings("unchecked") private void initialize() { this.fooProvider = Foo_Factory.create(Bar_Factory.create()); @@ -61,7 +57,7 @@ public final class DaggerTestComponent { @Override public SomeEntryPoint someEntryPoint() { - return new SomeEntryPoint(foo(), fooProvider); + return new SomeEntryPoint(fooProvider.get(), fooProvider); } } } diff --git a/javatests/dagger/internal/codegen/goldens/ComponentProcessorTest/providerComponentType/test.DaggerTestComponent_KT_DEFAULT_MODE b/javatests/dagger/internal/codegen/goldens/ComponentProcessorTest/providerComponentType/test.DaggerTestComponent_KT_DEFAULT_MODE index 0ba5565fe71..1968e001b1e 100644 --- a/javatests/dagger/internal/codegen/goldens/ComponentProcessorTest/providerComponentType/test.DaggerTestComponent_KT_DEFAULT_MODE +++ b/javatests/dagger/internal/codegen/goldens/ComponentProcessorTest/providerComponentType/test.DaggerTestComponent_KT_DEFAULT_MODE @@ -43,8 +43,8 @@ public final class DaggerTestComponent { private final TestComponentImpl testComponentImpl = this; /** - * {@code Provider} - */ + * {@code Provider} + */ Provider fooProvider; TestComponentImpl() { @@ -53,10 +53,6 @@ public final class DaggerTestComponent { } - Object foo() { - return Foo_Factory.newInstance(Bar_Factory.newInstance()); - } - @SuppressWarnings("unchecked") private void initialize() { this.fooProvider = Foo_Factory.create(Bar_Factory.create()); @@ -64,7 +60,7 @@ public final class DaggerTestComponent { @Override public SomeEntryPoint someEntryPoint() { - return (SomeEntryPoint) ((Object) (SomeEntryPoint_Factory.newInstance(foo(), fooProvider))); + return (SomeEntryPoint) ((Object) (SomeEntryPoint_Factory.newInstance(fooProvider.get(), fooProvider))); } } } diff --git a/javatests/dagger/internal/codegen/goldens/LazyClassKeyMapBindingComponentProcessorTest/mapBindingsWithInaccessibleKeys/test.DaggerTestComponent_DEFAULT_MODE b/javatests/dagger/internal/codegen/goldens/LazyClassKeyMapBindingComponentProcessorTest/mapBindingsWithInaccessibleKeys/test.DaggerTestComponent_DEFAULT_MODE index 7d9b1b3512c..bfcb93eb002 100644 --- a/javatests/dagger/internal/codegen/goldens/LazyClassKeyMapBindingComponentProcessorTest/mapBindingsWithInaccessibleKeys/test.DaggerTestComponent_DEFAULT_MODE +++ b/javatests/dagger/internal/codegen/goldens/LazyClassKeyMapBindingComponentProcessorTest/mapBindingsWithInaccessibleKeys/test.DaggerTestComponent_DEFAULT_MODE @@ -1,6 +1,5 @@ package test; -import com.google.common.collect.ImmutableMap; import dagger.internal.DaggerGenerated; import dagger.internal.LazyClassKeyMap; import dagger.internal.MapFactory; @@ -8,7 +7,6 @@ import dagger.internal.Provider; import java.util.Map; import javax.annotation.processing.Generated; import mapkeys.MapKeys; -import mapkeys.MapModule; import mapkeys.MapModule_ClassKeyFactory; import mapkeys.MapModule_ClassKey_LazyMapKey; import mapkeys.MapModule_ComplexKeyWithInaccessibleAnnotationValueFactory; @@ -88,7 +86,7 @@ final class DaggerTestComponent { @Override public Map, Integer> classKey() { - return LazyClassKeyMap.of(ImmutableMap.of(MapModule_ClassKey_LazyMapKey.lazyClassKeyName, MapModule.classKey())); + return mapOfClassOfAndIntegerProvider.get(); } @Override @@ -98,7 +96,7 @@ final class DaggerTestComponent { @Override public Map complexKey() { - return ImmutableMap.of(MapModule_ComplexKeyWithInaccessibleValueMapKey.create(), MapModule.complexKeyWithInaccessibleValue(), MapModule_ComplexKeyWithInaccessibleArrayValueMapKey.create(), MapModule.complexKeyWithInaccessibleArrayValue(), MapModule_ComplexKeyWithInaccessibleAnnotationValueMapKey.create(), MapModule.complexKeyWithInaccessibleAnnotationValue()); + return mapOfComplexKeyAndIntegerProvider.get(); } @Override diff --git a/javatests/dagger/internal/codegen/goldens/MapBindingComponentProcessorTest/mapBindingsWithInaccessibleKeys/test.DaggerTestComponent_DEFAULT_MODE b/javatests/dagger/internal/codegen/goldens/MapBindingComponentProcessorTest/mapBindingsWithInaccessibleKeys/test.DaggerTestComponent_DEFAULT_MODE index 94c6cba54fd..fa8f494b584 100644 --- a/javatests/dagger/internal/codegen/goldens/MapBindingComponentProcessorTest/mapBindingsWithInaccessibleKeys/test.DaggerTestComponent_DEFAULT_MODE +++ b/javatests/dagger/internal/codegen/goldens/MapBindingComponentProcessorTest/mapBindingsWithInaccessibleKeys/test.DaggerTestComponent_DEFAULT_MODE @@ -1,13 +1,11 @@ package test; -import com.google.common.collect.ImmutableMap; import dagger.internal.DaggerGenerated; import dagger.internal.MapFactory; import dagger.internal.Provider; import java.util.Map; import javax.annotation.processing.Generated; import mapkeys.MapKeys; -import mapkeys.MapModule; import mapkeys.MapModule_ClassKeyFactory; import mapkeys.MapModule_ClassKeyMapKey; import mapkeys.MapModule_ComplexKeyWithInaccessibleAnnotationValueFactory; @@ -70,10 +68,6 @@ final class DaggerTestComponent { } - Map mapOfPackagePrivateEnumAndInteger() { - return ImmutableMap.of(MapModule_EnumKeyMapKey.create(), MapModule.enumKey()); - } - @SuppressWarnings("unchecked") private void initialize() { this.mapOfClassOfAndIntegerProvider = mapOfClassOfAndIntegerBuilder(); @@ -103,7 +97,7 @@ final class DaggerTestComponent { @Override public Map, Integer> classKey() { - return ImmutableMap., Integer>of(MapModule_ClassKeyMapKey.create(), MapModule.classKey()); + return mapOfClassOfAndIntegerProvider.get(); } @Override @@ -113,7 +107,7 @@ final class DaggerTestComponent { @Override public Object inaccessibleEnum() { - return mapOfPackagePrivateEnumAndInteger(); + return mapOfPackagePrivateEnumAndIntegerProvider.get(); } @Override @@ -123,7 +117,7 @@ final class DaggerTestComponent { @Override public Map complexKey() { - return ImmutableMap.of(MapModule_ComplexKeyWithInaccessibleValueMapKey.create(), MapModule.complexKeyWithInaccessibleValue(), MapModule_ComplexKeyWithInaccessibleArrayValueMapKey.create(), MapModule.complexKeyWithInaccessibleArrayValue(), MapModule_ComplexKeyWithInaccessibleAnnotationValueMapKey.create(), MapModule.complexKeyWithInaccessibleAnnotationValue()); + return mapOfComplexKeyAndIntegerProvider.get(); } @Override diff --git a/javatests/dagger/internal/codegen/goldens/MapRequestRepresentationTest/lazyMaps/test.DaggerTestComponent_DEFAULT_MODE b/javatests/dagger/internal/codegen/goldens/MapRequestRepresentationTest/lazyMaps/test.DaggerTestComponent_DEFAULT_MODE index e2f9a67ed45..df4c586b9fa 100644 --- a/javatests/dagger/internal/codegen/goldens/MapRequestRepresentationTest/lazyMaps/test.DaggerTestComponent_DEFAULT_MODE +++ b/javatests/dagger/internal/codegen/goldens/MapRequestRepresentationTest/lazyMaps/test.DaggerTestComponent_DEFAULT_MODE @@ -2,12 +2,9 @@ package test; import dagger.Lazy; import dagger.internal.DaggerGenerated; -import dagger.internal.DoubleCheck; -import dagger.internal.MapBuilder; import dagger.internal.MapLazyFactory; import dagger.internal.MapProviderLazyFactory; import dagger.internal.Provider; -import dagger.internal.ProviderOfLazy; import java.util.Map; import javax.annotation.processing.Generated; @@ -59,29 +56,13 @@ final class DaggerTestComponent { } - Map mapOfStringAndLazyOfIntegerBuilder() { - MapBuilder mapBuilder = MapBuilder.>newMapBuilder(3); - mapBuilder.put("key0", DoubleCheck.lazy(LazyMaps_TestModule_Int0Factory.create())); - mapBuilder.put("key1", DoubleCheck.lazy(LazyMaps_TestModule_Int1Factory.create())); - mapBuilder.put("key2", DoubleCheck.lazy(LazyMaps_TestModule_Int2Factory.create())); - return mapBuilder.build(); - } - - Map mapOfStringAndProviderOfLazyOfIntegerBuilder() { - MapBuilder mapBuilder = MapBuilder.>>newMapBuilder(3); - mapBuilder.put("key0", ProviderOfLazy.create(LazyMaps_TestModule_Int0Factory.create())); - mapBuilder.put("key1", ProviderOfLazy.create(LazyMaps_TestModule_Int1Factory.create())); - mapBuilder.put("key2", ProviderOfLazy.create(LazyMaps_TestModule_Int2Factory.create())); - return mapBuilder.build(); - } - @SuppressWarnings("unchecked") private void initialize() { - this.mapOfStringAndLazyOfIntegerProvider = mapOfStringAndLazyOfIntegerBuilder2(); - this.mapOfStringAndProviderOfLazyOfIntegerProvider = mapOfStringAndProviderOfLazyOfIntegerBuilder2(); + this.mapOfStringAndLazyOfIntegerProvider = mapOfStringAndLazyOfIntegerBuilder(); + this.mapOfStringAndProviderOfLazyOfIntegerProvider = mapOfStringAndProviderOfLazyOfIntegerBuilder(); } - MapLazyFactory mapOfStringAndLazyOfIntegerBuilder2() { + MapLazyFactory mapOfStringAndLazyOfIntegerBuilder() { MapLazyFactory.Builder builder = MapLazyFactory.builder(3); builder.put("key0", LazyMaps_TestModule_Int0Factory.create()); builder.put("key1", LazyMaps_TestModule_Int1Factory.create()); @@ -89,7 +70,7 @@ final class DaggerTestComponent { return builder.build(); } - MapProviderLazyFactory mapOfStringAndProviderOfLazyOfIntegerBuilder2() { + MapProviderLazyFactory mapOfStringAndProviderOfLazyOfIntegerBuilder() { MapProviderLazyFactory.Builder builder = MapProviderLazyFactory.builder(3); builder.put("key0", LazyMaps_TestModule_Int0Factory.create()); builder.put("key1", LazyMaps_TestModule_Int1Factory.create()); @@ -99,12 +80,12 @@ final class DaggerTestComponent { @Override public Map> mapOfLazy() { - return mapOfStringAndLazyOfIntegerBuilder(); + return mapOfStringAndLazyOfIntegerProvider.get(); } @Override public Map>> mapOfProviderOfLazy() { - return mapOfStringAndProviderOfLazyOfIntegerBuilder(); + return (Map>>) (mapOfStringAndProviderOfLazyOfIntegerProvider.get()); } @Override diff --git a/javatests/dagger/internal/codegen/goldens/MapRequestRepresentationWithGuavaTest/lazyMaps/test.DaggerTestComponent_DEFAULT_MODE b/javatests/dagger/internal/codegen/goldens/MapRequestRepresentationWithGuavaTest/lazyMaps/test.DaggerTestComponent_DEFAULT_MODE index a30bcc588b9..cebfcbec991 100644 --- a/javatests/dagger/internal/codegen/goldens/MapRequestRepresentationWithGuavaTest/lazyMaps/test.DaggerTestComponent_DEFAULT_MODE +++ b/javatests/dagger/internal/codegen/goldens/MapRequestRepresentationWithGuavaTest/lazyMaps/test.DaggerTestComponent_DEFAULT_MODE @@ -7,7 +7,6 @@ import dagger.internal.DoubleCheck; import dagger.internal.MapLazyFactory; import dagger.internal.MapProviderLazyFactory; import dagger.internal.Provider; -import dagger.internal.ProviderOfLazy; import java.util.Map; import java.util.concurrent.atomic.AtomicInteger; import javax.annotation.processing.Generated; @@ -70,22 +69,6 @@ final class DaggerTestComponent { } - String string() { - return LazyMaps_TestModule_ProvideStringFactory.provideString(provideAtomicIntegerProvider.get()); - } - - String string0() { - return LazyMaps_TestModule_String0Factory.string0(string()); - } - - String string1() { - return LazyMaps_TestModule_String1Factory.string1(string()); - } - - String string2() { - return LazyMaps_TestModule_String2Factory.string2(string()); - } - @SuppressWarnings("unchecked") private void initialize() { this.provideAtomicIntegerProvider = DoubleCheck.provider(LazyMaps_TestModule_ProvideAtomicIntegerFactory.create()); @@ -115,17 +98,17 @@ final class DaggerTestComponent { @Override public Map mapOfString() { - return ImmutableMap.of("key0", string0(), "key1", string1(), "key2", string2()); + return ImmutableMap.of("key0", string0Provider.get(), "key1", string1Provider.get(), "key2", string2Provider.get()); } @Override public Map> mapOfLazy() { - return ImmutableMap.>of("key0", DoubleCheck.lazy(string0Provider), "key1", DoubleCheck.lazy(string1Provider), "key2", DoubleCheck.lazy(string2Provider)); + return mapOfStringAndLazyOfStringProvider.get(); } @Override public Map>> mapOfProviderOfLazy() { - return ImmutableMap.>>of("key0", ProviderOfLazy.create(string0Provider), "key1", ProviderOfLazy.create(string1Provider), "key2", ProviderOfLazy.create(string2Provider)); + return (Map>>) (mapOfStringAndProviderOfLazyOfStringProvider.get()); } @Override diff --git a/javatests/dagger/internal/codegen/goldens/MapRequestRepresentationWithGuavaTest/lazyMaps/test.DaggerTestComponent_KT_DEFAULT_MODE b/javatests/dagger/internal/codegen/goldens/MapRequestRepresentationWithGuavaTest/lazyMaps/test.DaggerTestComponent_KT_DEFAULT_MODE index 79b3650bdd7..8c87d73be33 100644 --- a/javatests/dagger/internal/codegen/goldens/MapRequestRepresentationWithGuavaTest/lazyMaps/test.DaggerTestComponent_KT_DEFAULT_MODE +++ b/javatests/dagger/internal/codegen/goldens/MapRequestRepresentationWithGuavaTest/lazyMaps/test.DaggerTestComponent_KT_DEFAULT_MODE @@ -7,7 +7,6 @@ import dagger.internal.DoubleCheck; import dagger.internal.MapLazyFactory; import dagger.internal.MapProviderLazyFactory; import dagger.internal.Provider; -import dagger.internal.ProviderOfLazy; import java.util.Map; import javax.annotation.processing.Generated; @@ -90,22 +89,6 @@ final class DaggerTestComponent { } - Object string() { - return LazyMaps_TestModule_ProvideStringFactory.provideString(provideAtomicIntegerProvider.get()); - } - - Object string0() { - return LazyMaps_TestModule_String0Factory.string0(string()); - } - - Object string1() { - return LazyMaps_TestModule_String1Factory.string1(string()); - } - - Object string2() { - return LazyMaps_TestModule_String2Factory.string2(string()); - } - @SuppressWarnings("unchecked") private void initialize() { this.provideAtomicIntegerProvider = DoubleCheck.provider(LazyMaps_TestModule_ProvideAtomicIntegerFactory.create()); @@ -135,17 +118,17 @@ final class DaggerTestComponent { @Override public Map mapOfString() { - return (Map) ((Object) (ImmutableMap.of("key0", string0(), "key1", string1(), "key2", string2()))); + return (Map) ((Object) (ImmutableMap.of("key0", string0Provider.get(), "key1", string1Provider.get(), "key2", string2Provider.get()))); } @Override public Map> mapOfLazy() { - return (Map>) ((Object) (ImmutableMap.of("key0", DoubleCheck.lazy(string0Provider), "key1", DoubleCheck.lazy(string1Provider), "key2", DoubleCheck.lazy(string2Provider)))); + return (Map>) ((Object) (mapOfStringAndLazyOfStringProvider.get())); } @Override public Map>> mapOfProviderOfLazy() { - return (Map>>) ((Object) (ImmutableMap.of("key0", ProviderOfLazy.create(string0Provider), "key1", ProviderOfLazy.create(string1Provider), "key2", ProviderOfLazy.create(string2Provider)))); + return (Map>>) ((Object) ((Map>>) (mapOfStringAndProviderOfLazyOfStringProvider.get()))); } @Override