Skip to content

Commit

Permalink
Refactoring @LazyClassKey logic to avoid generating duplicated entr…
Browse files Browse the repository at this point in the history
…ies in child `LazyClassKeyProvider`.

RELNOTES=n/a
PiperOrigin-RevId: 684863374
  • Loading branch information
wanyingd1996 authored and Dagger Team committed Oct 15, 2024
1 parent dfcdc9c commit 3cfdce6
Show file tree
Hide file tree
Showing 10 changed files with 154 additions and 104 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -462,7 +462,6 @@ public final class ShardImplementation implements GeneratedImplementation {
private final UniqueNameSet assistedParamNames = new UniqueNameSet();
private final List<CodeBlock> initializations = new ArrayList<>();
private final SwitchingProviders switchingProviders;
private final LazyClassKeyProviders lazyClassKeyProviders;
private final Map<Key, CodeBlock> cancellations = new LinkedHashMap<>();
private final Map<XVariableElement, String> uniqueAssistedName = new LinkedHashMap<>();
private final List<CodeBlock> componentRequirementInitializations = new ArrayList<>();
Expand All @@ -479,7 +478,6 @@ public final class ShardImplementation implements GeneratedImplementation {
private ShardImplementation(ClassName name) {
this.name = name;
this.switchingProviders = new SwitchingProviders(this, processingEnv);
this.lazyClassKeyProviders = new LazyClassKeyProviders(this);
if (graph.componentDescriptor().isProduction()) {
claimMethodName(CANCELLATION_LISTENER_METHOD_NAME);
}
Expand Down Expand Up @@ -513,10 +511,6 @@ public SwitchingProviders getSwitchingProviders() {
return switchingProviders;
}

public LazyClassKeyProviders getLazyClassKeyProviders() {
return lazyClassKeyProviders;
}

/** Returns the {@link ComponentImplementation} that owns this shard. */
public ComponentImplementation getComponentImplementation() {
return ComponentImplementation.this;
Expand Down Expand Up @@ -618,7 +612,8 @@ public void addType(TypeSpecKind typeKind, TypeSpec typeSpec) {
}

/** Adds a {@link Supplier} for the SwitchingProvider for the component. */
void addTypeSupplier(Supplier<TypeSpec> typeSpecSupplier) {
@Override
public void addTypeSupplier(Supplier<TypeSpec> typeSpecSupplier) {
typeSuppliers.add(typeSpecSupplier);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import static javax.lang.model.element.Modifier.PRIVATE;
import static javax.lang.model.element.Modifier.PUBLIC;

import com.google.common.base.Supplier;
import com.google.common.collect.ListMultimap;
import com.google.common.collect.MultimapBuilder;
import com.squareup.javapoet.ClassName;
Expand All @@ -34,6 +35,8 @@
import dagger.internal.codegen.writing.ComponentImplementation.FieldSpecKind;
import dagger.internal.codegen.writing.ComponentImplementation.MethodSpecKind;
import dagger.internal.codegen.writing.ComponentImplementation.TypeSpecKind;
import java.util.ArrayList;
import java.util.List;
import javax.inject.Inject;

/** Represents the implementation of the generated holder for the components. */
Expand All @@ -48,6 +51,7 @@ public final class ComponentWrapperImplementation implements GeneratedImplementa
MultimapBuilder.enumKeys(MethodSpecKind.class).arrayListValues().build();
private final ListMultimap<TypeSpecKind, TypeSpec> typeSpecsMap =
MultimapBuilder.enumKeys(TypeSpecKind.class).arrayListValues().build();
private final List<Supplier<TypeSpec>> typeSuppliers = new ArrayList<>();

@Inject
ComponentWrapperImplementation(@TopLevel BindingGraph graph) {
Expand Down Expand Up @@ -80,6 +84,11 @@ public void addType(TypeSpecKind typeKind, TypeSpec typeSpec) {
typeSpecsMap.put(typeKind, typeSpec);
}

@Override
public void addTypeSupplier(Supplier<TypeSpec> typeSpecSupplier) {
typeSuppliers.add(typeSpecSupplier);
}

@Override
public TypeSpec generate() {
TypeSpec.Builder builder =
Expand All @@ -92,6 +101,7 @@ public TypeSpec generate() {
fieldSpecsMap.asMap().values().forEach(builder::addFields);
methodSpecsMap.asMap().values().forEach(builder::addMethods);
typeSpecsMap.asMap().values().forEach(builder::addTypes);
typeSuppliers.stream().map(Supplier::get).forEach(builder::addType);

return builder.addMethod(constructorBuilder().addModifiers(PRIVATE).build()).build();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package dagger.internal.codegen.writing;

import com.google.common.base.Supplier;
import com.squareup.javapoet.ClassName;
import com.squareup.javapoet.FieldSpec;
import com.squareup.javapoet.MethodSpec;
Expand All @@ -41,6 +42,9 @@ public interface GeneratedImplementation {
/** Adds the given type to the generated implementation. */
void addType(TypeSpecKind typeKind, TypeSpec typeSpec);

/** Adds a {@link Supplier} for a {@link TypeSpec} to the generated implementation. */
void addTypeSupplier(Supplier<TypeSpec> typeSupplier);

/** Returns the {@link TypeSpec} for this generated implementation. */
public TypeSpec generate();
}
175 changes: 108 additions & 67 deletions java/dagger/internal/codegen/writing/LazyClassKeyProviders.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,88 +30,129 @@
import dagger.internal.codegen.base.UniqueNameSet;
import dagger.internal.codegen.javapoet.TypeNames;
import dagger.internal.codegen.model.Key;
import dagger.internal.codegen.writing.ComponentImplementation.ShardImplementation;
import java.util.LinkedHashMap;
import java.util.Map;
import javax.inject.Inject;

/** Keeps track of all providers for DaggerMap keys. */
/**
* Keeps track of all providers for DaggerMap keys.
*
* <p>Generated class looks like below:
*
* <pre>{@code
* @IdentifierNameString
* static final class LazyClassKeyProvider {
* static final String com_google_foo_Bar = "com.google.foo.Bar";
* @KeepFieldType static final com.google.foo.Bar com_google_foo_Bar2;
* }
* }</pre>
*/
public final class LazyClassKeyProviders {
public static final String MAP_KEY_PROVIDER_NAME = "LazyClassKeyProvider";
private final ClassName mapKeyProviderType;
private final Map<Key, FieldSpec> entries = new LinkedHashMap<>();
private final Map<Key, FieldSpec> keepClassNamesFields = new LinkedHashMap<>();
private final UniqueNameSet uniqueFieldNames = new UniqueNameSet();
private final ShardImplementation shardImplementation;
private boolean providerAdded = false;
@PerGeneratedFile
static final class LazyClassKeyProviderCache {
// Map key to its corresponding the field reference expression from LazyClassKeyProvider.
final Map<ClassName, CodeBlock> mapKeyToProvider = new LinkedHashMap<>();
private final Map<ClassName, FieldSpec> entries = new LinkedHashMap<>();
private final Map<ClassName, FieldSpec> keepClassNamesFields = new LinkedHashMap<>();
private final UniqueNameSet uniqueFieldNames = new UniqueNameSet();
private ClassName mapKeyProviderType;

LazyClassKeyProviders(ShardImplementation shardImplementation) {
String name = shardImplementation.getUniqueClassName(MAP_KEY_PROVIDER_NAME);
mapKeyProviderType = shardImplementation.name().nestedClass(name);
this.shardImplementation = shardImplementation;
}
@Inject
LazyClassKeyProviderCache() {}

/** Returns a reference to a field in LazyClassKeyProvider that corresponds to this binding. */
CodeBlock getMapKeyExpression(Key key) {
// This is for avoid generating empty LazyClassKeyProvider in codegen tests
if (!providerAdded) {
shardImplementation.addTypeSupplier(this::build);
providerAdded = true;
private CodeBlock getMapKeyExpression(Key key) {
Preconditions.checkArgument(
key.multibindingContributionIdentifier().isPresent()
&& key.multibindingContributionIdentifier()
.get()
.bindingMethod()
.xprocessing()
.hasAnnotation(TypeNames.LAZY_CLASS_KEY));
XAnnotation lazyClassKeyAnnotation = getLazyClassKeyAnnotation(key);
ClassName lazyClassKey = getLazyClassKey(lazyClassKeyAnnotation);
if (mapKeyToProvider.containsKey(lazyClassKey)) {
return mapKeyToProvider.get(lazyClassKey);
}
addField(lazyClassKey, lazyClassKeyAnnotation, mapKeyProviderType.packageName());
mapKeyToProvider.put(
lazyClassKey, CodeBlock.of("$T.$N", mapKeyProviderType, entries.get(lazyClassKey)));
return mapKeyToProvider.get(lazyClassKey);
}
if (!entries.containsKey(key)) {
addField(key);

private ClassName getLazyClassKey(XAnnotation lazyClassKeyAnnotation) {
return lazyClassKeyAnnotation.getAsType("value").getTypeElement().getClassName();
}

private XAnnotation getLazyClassKeyAnnotation(Key key) {
return key.multibindingContributionIdentifier()
.get()
.bindingMethod()
.xprocessing()
.getAnnotation(TypeNames.LAZY_CLASS_KEY);
}
return CodeBlock.of("$T.$N", mapKeyProviderType, entries.get(key));
}

private void addField(Key key) {
Preconditions.checkArgument(
key.multibindingContributionIdentifier().isPresent()
&& key.multibindingContributionIdentifier()
.get()
.bindingMethod()
.xprocessing()
.hasAnnotation(TypeNames.LAZY_CLASS_KEY));
XAnnotation lazyClassKeyAnnotation =
key.multibindingContributionIdentifier()
.get()
.bindingMethod()
.xprocessing()
.getAnnotation(TypeNames.LAZY_CLASS_KEY);
ClassName lazyClassKey =
lazyClassKeyAnnotation.getAsType("value").getTypeElement().getClassName();
entries.put(
key,
FieldSpec.builder(
TypeNames.STRING,
uniqueFieldNames.getUniqueName(lazyClassKey.canonicalName().replace('.', '_')))
// TODO(b/217435141): Leave the field as non-final. We will apply @IdentifierNameString
// on the field, which doesn't work well with static final fields.
.addModifiers(STATIC)
.initializer("$S", lazyClassKey.reflectionName())
.build());
// To be able to apply -includedescriptorclasses rule to keep the class names referenced by
// LazyClassKey, we need to generate fields that uses those classes as type in
// LazyClassKeyProvider. For types that are not accessible from the generated component, we
// generate fields in the proxy class.
// Note: the generated field should not be initialized to avoid class loading.
if (isMapKeyAccessibleFrom(lazyClassKeyAnnotation, shardImplementation.name().packageName())) {
keepClassNamesFields.put(
key,
private void addField(
ClassName lazyClassKey, XAnnotation lazyClassKeyAnnotation, String accessingPackage) {
entries.put(
lazyClassKey,
FieldSpec.builder(
lazyClassKey,
TypeNames.STRING,
uniqueFieldNames.getUniqueName(lazyClassKey.canonicalName().replace('.', '_')))
.addAnnotation(TypeNames.KEEP_FIELD_TYPE)
// TODO(b/217435141): Leave the field as non-final. We will apply
// @IdentifierNameString on the field, which doesn't work well with static final
// fields.
.addModifiers(STATIC)
.initializer("$S", lazyClassKey.reflectionName())
.build());
// To be able to apply -includedescriptorclasses rule to keep the class names referenced by
// LazyClassKey, we need to generate fields that uses those classes as type in
// LazyClassKeyProvider. For types that are not accessible from the generated component, we
// generate fields in the proxy class.
// Note: the generated field should not be initialized to avoid class loading.
if (isMapKeyAccessibleFrom(lazyClassKeyAnnotation, accessingPackage)) {
keepClassNamesFields.put(
lazyClassKey,
FieldSpec.builder(
lazyClassKey,
uniqueFieldNames.getUniqueName(lazyClassKey.canonicalName().replace('.', '_')))
.addAnnotation(TypeNames.KEEP_FIELD_TYPE)
.build());
}
}

private TypeSpec build() {
return TypeSpec.classBuilder(mapKeyProviderType)
.addAnnotation(TypeNames.IDENTIFIER_NAME_STRING)
.addModifiers(PRIVATE, STATIC, FINAL)
.addFields(entries.values())
.addFields(keepClassNamesFields.values())
.build();
}
}

public static final String MAP_KEY_PROVIDER_NAME = "LazyClassKeyProvider";
private final GeneratedImplementation topLevelImplementation;
private final LazyClassKeyProviderCache cache;

@Inject
LazyClassKeyProviders(
@TopLevel GeneratedImplementation topLevelImplementation, LazyClassKeyProviderCache cache) {
this.topLevelImplementation = topLevelImplementation;
this.cache = cache;
}

/** Returns a reference to a field in LazyClassKeyProvider that corresponds to this binding. */
CodeBlock getMapKeyExpression(Key key) {
// This is for avoid generating empty LazyClassKeyProvider.
if (cache.entries.isEmpty()) {
String name = topLevelImplementation.getUniqueClassName(MAP_KEY_PROVIDER_NAME);
cache.mapKeyProviderType = topLevelImplementation.name().nestedClass(name);
topLevelImplementation.addTypeSupplier(this::build);
}
return cache.getMapKeyExpression(key);
}

private TypeSpec build() {
TypeSpec.Builder builder =
TypeSpec.classBuilder(mapKeyProviderType)
.addAnnotation(TypeNames.IDENTIFIER_NAME_STRING)
.addModifiers(PRIVATE, STATIC, FINAL)
.addFields(entries.values())
.addFields(keepClassNamesFields.values());
return builder.build();
return cache.build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ final class MapFactoryCreationExpression extends MultibindingFactoryCreationExpr
@AssistedInject
MapFactoryCreationExpression(
@Assisted MultiboundMapBinding binding,
LazyClassKeyProviders lazyClassKeyProviders,
XProcessingEnv processingEnv,
ComponentImplementation componentImplementation,
ComponentRequestRepresentations componentRequestRepresentations,
Expand All @@ -58,8 +59,7 @@ final class MapFactoryCreationExpression extends MultibindingFactoryCreationExpr
this.componentImplementation = componentImplementation;
this.graph = graph;
this.useLazyClassKey = MapKeys.useLazyClassKey(binding, graph);
this.lazyClassKeyProviders =
componentImplementation.shardImplementation(binding).getLazyClassKeyProviders();
this.lazyClassKeyProviders = lazyClassKeyProviders;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ final class MapRequestRepresentation extends RequestRepresentation {
@AssistedInject
MapRequestRepresentation(
@Assisted MultiboundMapBinding binding,
LazyClassKeyProviders lazyClassKeyProviders,
XProcessingEnv processingEnv,
BindingGraph graph,
ComponentImplementation componentImplementation,
Expand All @@ -73,8 +74,7 @@ final class MapRequestRepresentation extends RequestRepresentation {
this.dependencies =
Maps.toMap(binding.dependencies(), dep -> graph.contributionBinding(dep.key()));
this.useLazyClassKey = MapKeys.useLazyClassKey(binding, graph);
this.lazyClassKeyProviders =
componentImplementation.shardImplementation(binding).getLazyClassKeyProviders();
this.lazyClassKeyProviders = lazyClassKeyProviders;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,19 +54,19 @@ final class DaggerTestComponent {
public Map<Class<?>, Integer> classKey() {
return LazyClassKeyMap.<Integer>of(ImmutableMap.<String, Integer>of(LazyClassKeyProvider.test_Foo_Bar, MapKeyBindingsModule.classKey(), LazyClassKeyProvider.test_Foo_Bar3, MapKeyBindingsModule.classKey2()));
}
}

@IdentifierNameString
private static final class LazyClassKeyProvider {
static String test_Foo_Bar = "test.Foo_Bar";
@IdentifierNameString
private static final class LazyClassKeyProvider {
static String test_Foo_Bar = "test.Foo_Bar";

static String test_Foo_Bar3 = "test.Foo$Bar";
static String test_Foo_Bar3 = "test.Foo$Bar";

@KeepFieldType
Foo_Bar test_Foo_Bar2;
@KeepFieldType
Foo_Bar test_Foo_Bar2;

@KeepFieldType
Foo.Bar test_Foo_Bar4;
}
@KeepFieldType
Foo.Bar test_Foo_Bar4;
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -54,19 +54,19 @@ final class DaggerTestComponent {
public Map<Class<?>, Integer> classKey() {
return LazyClassKeyMap.<Integer>of(ImmutableMap.<String, Integer>of(LazyClassKeyProvider.test_Foo_Bar, MapKeyBindingsModule.classKey(), LazyClassKeyProvider.test_Foo_Bar3, MapKeyBindingsModule.classKey2()));
}
}

@IdentifierNameString
private static final class LazyClassKeyProvider {
static String test_Foo_Bar = "test.Foo_Bar";
@IdentifierNameString
private static final class LazyClassKeyProvider {
static String test_Foo_Bar = "test.Foo_Bar";

static String test_Foo_Bar3 = "test.Foo$Bar";
static String test_Foo_Bar3 = "test.Foo$Bar";

@KeepFieldType
Foo_Bar test_Foo_Bar2;
@KeepFieldType
Foo_Bar test_Foo_Bar2;

@KeepFieldType
Foo.Bar test_Foo_Bar4;
}
@KeepFieldType
Foo.Bar test_Foo_Bar4;
}
}

Loading

0 comments on commit 3cfdce6

Please sign in to comment.