Correctly deal with method overrides in the Enum unboxer
Fixes: b/171784168
Fixes: b/191617665
Change-Id: Id1b775be6cb5281dec9ae9cd52002ec49c910a64
diff --git a/src/main/java/com/android/tools/r8/graph/PrunedItems.java b/src/main/java/com/android/tools/r8/graph/PrunedItems.java
index f49fcbd..200b351 100644
--- a/src/main/java/com/android/tools/r8/graph/PrunedItems.java
+++ b/src/main/java/com/android/tools/r8/graph/PrunedItems.java
@@ -32,6 +32,10 @@
this.removedMethods = removedMethods;
}
+ public static Builder concurrentBuilder() {
+ return new ConcurrentBuilder();
+ }
+
public static Builder builder() {
return new Builder();
}
@@ -111,15 +115,22 @@
private DexApplication prunedApp;
- private final Set<DexReference> additionalPinnedItems = Sets.newIdentityHashSet();
- private final Set<DexType> noLongerSyntheticItems = Sets.newIdentityHashSet();
- private Set<DexType> removedClasses = Sets.newIdentityHashSet();
- private final Set<DexField> removedFields = Sets.newIdentityHashSet();
- private Set<DexMethod> removedMethods = Sets.newIdentityHashSet();
+ private final Set<DexReference> additionalPinnedItems;
+ private final Set<DexType> noLongerSyntheticItems;
+ private Set<DexType> removedClasses;
+ private final Set<DexField> removedFields;
+ private Set<DexMethod> removedMethods;
- Builder() {}
+ Builder() {
+ additionalPinnedItems = newEmptySet();
+ noLongerSyntheticItems = newEmptySet();
+ removedClasses = newEmptySet();
+ removedFields = newEmptySet();
+ removedMethods = newEmptySet();
+ }
Builder(PrunedItems prunedItems) {
+ this();
additionalPinnedItems.addAll(prunedItems.getAdditionalPinnedItems());
noLongerSyntheticItems.addAll(prunedItems.getNoLongerSyntheticItems());
prunedApp = prunedItems.getPrunedApp();
@@ -128,6 +139,10 @@
removedMethods.addAll(prunedItems.getRemovedMethods());
}
+ <T> Set<T> newEmptySet() {
+ return Sets.newIdentityHashSet();
+ }
+
public Builder setPrunedApp(DexApplication prunedApp) {
this.prunedApp = prunedApp;
return this;
@@ -196,4 +211,22 @@
removedMethods);
}
}
+
+ public static class ConcurrentBuilder extends Builder {
+
+ @Override
+ <T> Set<T> newEmptySet() {
+ return Sets.newConcurrentHashSet();
+ }
+
+ @Override
+ public Builder setRemovedClasses(Set<DexType> removedClasses) {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public Builder setRemovedMethods(Set<DexMethod> removedMethods) {
+ throw new UnsupportedOperationException();
+ }
+ }
}
diff --git a/src/main/java/com/android/tools/r8/graph/fixup/ConcurrentMethodFixup.java b/src/main/java/com/android/tools/r8/graph/fixup/ConcurrentMethodFixup.java
new file mode 100644
index 0000000..6afd404
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/graph/fixup/ConcurrentMethodFixup.java
@@ -0,0 +1,224 @@
+// Copyright (c) 2023, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+package com.android.tools.r8.graph.fixup;
+
+import static com.android.tools.r8.graph.DexProgramClass.asProgramClassOrNull;
+
+import com.android.tools.r8.graph.AppView;
+import com.android.tools.r8.graph.ClasspathOrLibraryClass;
+import com.android.tools.r8.graph.DexClass;
+import com.android.tools.r8.graph.DexEncodedMethod;
+import com.android.tools.r8.graph.DexMethod;
+import com.android.tools.r8.graph.DexMethodSignature;
+import com.android.tools.r8.graph.DexProgramClass;
+import com.android.tools.r8.graph.DexType;
+import com.android.tools.r8.graph.ImmediateProgramSubtypingInfo;
+import com.android.tools.r8.optimize.argumentpropagation.utils.ProgramClassesBidirectedGraph;
+import com.android.tools.r8.shaking.AppInfoWithLiveness;
+import com.android.tools.r8.shaking.KeepMethodInfo;
+import com.android.tools.r8.utils.InternalOptions;
+import com.android.tools.r8.utils.ThreadUtils;
+import com.android.tools.r8.utils.Timing;
+import com.android.tools.r8.utils.collections.DexMethodSignatureSet;
+import com.google.common.collect.BiMap;
+import com.google.common.collect.HashBiMap;
+import com.google.common.collect.ImmutableBiMap;
+import com.google.common.collect.Sets;
+import java.util.ArrayList;
+import java.util.Comparator;
+import java.util.IdentityHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+
+public class ConcurrentMethodFixup {
+
+ private final AppView<AppInfoWithLiveness> appView;
+ private final Map<ClasspathOrLibraryClass, DexMethodSignatureSet> nonProgramVirtualMethods =
+ new ConcurrentHashMap<>();
+ private final ProgramClassFixer programClassFixer;
+
+ public ConcurrentMethodFixup(
+ AppView<AppInfoWithLiveness> appView, ProgramClassFixer programClassFixer) {
+ this.appView = appView;
+ this.programClassFixer = programClassFixer;
+ }
+
+ public void fixupClassesConcurrentlyByConnectedProgramComponents(
+ Timing timing, ExecutorService executorService) throws ExecutionException {
+ timing.begin("Concurrent method fixup");
+ timing.begin("Compute strongly connected components");
+ ImmediateProgramSubtypingInfo immediateSubtypingInfo =
+ ImmediateProgramSubtypingInfo.create(appView);
+ List<Set<DexProgramClass>> connectedComponents =
+ new ProgramClassesBidirectedGraph(appView, immediateSubtypingInfo)
+ .computeStronglyConnectedComponents();
+ timing.end();
+
+ timing.begin("Process strongly connected components");
+ ThreadUtils.processItems(
+ connectedComponents, this::processConnectedProgramComponents, executorService);
+ timing.end();
+ timing.end();
+ }
+
+ public interface ProgramClassFixer {
+ // When a class is fixed-up, it is guaranteed that its supertype and interfaces were processed
+ // before. In addition, all interfaces are processed before any class is processed.
+ void fixupProgramClass(DexProgramClass clazz, MethodNamingUtility namingUtility);
+ }
+
+ private void processConnectedProgramComponents(Set<DexProgramClass> classes) {
+ List<DexProgramClass> sorted = new ArrayList<>(classes);
+ sorted.sort(Comparator.comparing(DexClass::getType));
+ BiMap<DexMethodSignature, DexMethodSignature> componentSignatures = HashBiMap.create();
+
+ // 1) Reserve all library overrides and pinned virtual methods.
+ reserveComponentPinnedAndInterfaceMethodSignatures(sorted, componentSignatures);
+
+ // 2) Map all interfaces top-down updating the componentSignatures.
+ Set<DexProgramClass> processedInterfaces = Sets.newIdentityHashSet();
+ for (DexProgramClass clazz : sorted) {
+ if (clazz.isInterface()) {
+ processInterface(clazz, processedInterfaces, componentSignatures);
+ }
+ }
+
+ // 3) Map all classes top-down propagating the inherited signatures.
+ // The componentSignatures are already fully computed and should not be updated anymore.
+ BiMap<DexMethodSignature, DexMethodSignature> immutableComponentSignaturesForTesting =
+ InternalOptions.assertionsEnabled()
+ ? ImmutableBiMap.copyOf(componentSignatures)
+ : componentSignatures;
+ Map<DexProgramClass, BiMap<DexMethodSignature, DexMethodSignature>> processedClasses =
+ new IdentityHashMap<>();
+ for (DexProgramClass clazz : sorted) {
+ if (!clazz.isInterface()) {
+ processClass(clazz, processedClasses, immutableComponentSignaturesForTesting);
+ }
+ }
+ }
+
+ private void processClass(
+ DexProgramClass clazz,
+ Map<DexProgramClass, BiMap<DexMethodSignature, DexMethodSignature>> processedClasses,
+ BiMap<DexMethodSignature, DexMethodSignature> componentSignatures) {
+ assert !clazz.isInterface();
+ if (processedClasses.containsKey(clazz)) {
+ return;
+ }
+ // We need to process first the super-type for the top-down propagation of inherited signatures.
+ DexClass superClass = appView.definitionFor(clazz.superType);
+ BiMap<DexMethodSignature, DexMethodSignature> inheritedSignatures;
+ if (superClass == null || !superClass.isProgramClass()) {
+ inheritedSignatures = HashBiMap.create(componentSignatures);
+ } else {
+ DexProgramClass superProgramClass = superClass.asProgramClass();
+ processClass(superProgramClass, processedClasses, componentSignatures);
+ inheritedSignatures = HashBiMap.create(processedClasses.get(superProgramClass));
+ }
+ processedClasses.put(clazz, inheritedSignatures);
+ MethodNamingUtility utility = createMethodNamingUtility(inheritedSignatures, clazz);
+ programClassFixer.fixupProgramClass(clazz, utility);
+ }
+
+ private void processInterface(
+ DexProgramClass clazz,
+ Set<DexProgramClass> processedInterfaces,
+ BiMap<DexMethodSignature, DexMethodSignature> componentSignatures) {
+ assert clazz.isInterface();
+ if (!processedInterfaces.add(clazz)) {
+ return;
+ }
+ // We need to process first all super-interfaces to avoid generating collisions by renaming
+ // private or static methods into inherited virtual method signatures.
+ for (DexType superInterface : clazz.getInterfaces()) {
+ DexProgramClass superInterfaceClass =
+ asProgramClassOrNull(appView.definitionFor(superInterface));
+ if (superInterfaceClass != null) {
+ processInterface(superInterfaceClass, processedInterfaces, componentSignatures);
+ }
+ }
+ MethodNamingUtility utility = createMethodNamingUtility(componentSignatures, clazz);
+ programClassFixer.fixupProgramClass(clazz, utility);
+ }
+
+ private MethodNamingUtility createMethodNamingUtility(
+ BiMap<DexMethodSignature, DexMethodSignature> inheritedSignatures, DexProgramClass clazz) {
+ BiMap<DexMethod, DexMethod> localSignatures = HashBiMap.create();
+ clazz.forEachProgramInstanceInitializer(
+ method -> {
+ KeepMethodInfo keepInfo = appView.getKeepInfo(method);
+ if (!keepInfo.isOptimizationAllowed(appView.options())
+ || !keepInfo.isShrinkingAllowed(appView.options())) {
+ localSignatures.put(method.getReference(), method.getReference());
+ }
+ });
+ return new MethodNamingUtility(appView.dexItemFactory(), inheritedSignatures, localSignatures);
+ }
+
+ private void reserveComponentPinnedAndInterfaceMethodSignatures(
+ List<DexProgramClass> stronglyConnectedProgramClasses,
+ BiMap<DexMethodSignature, DexMethodSignature> componentSignatures) {
+ Set<ClasspathOrLibraryClass> seenNonProgramClasses = Sets.newIdentityHashSet();
+ for (DexProgramClass clazz : stronglyConnectedProgramClasses) {
+ // If a private or static method is pinned, we need to reserve the mapping to avoid creating
+ // a collision with a changed virtual method.
+ clazz.forEachProgramMethodMatching(
+ m -> !m.isInstanceInitializer(),
+ method -> {
+ KeepMethodInfo keepInfo = appView.getKeepInfo(method);
+ if (!keepInfo.isOptimizationAllowed(appView.options())
+ || !keepInfo.isShrinkingAllowed(appView.options())) {
+ componentSignatures.put(method.getMethodSignature(), method.getMethodSignature());
+ }
+ });
+ clazz.forEachImmediateSupertype(
+ supertype -> {
+ DexClass superclass = appView.definitionFor(supertype);
+ if (superclass != null
+ && !superclass.isProgramClass()
+ && seenNonProgramClasses.add(superclass.asClasspathOrLibraryClass())) {
+ for (DexMethodSignature vMethod :
+ getOrComputeNonProgramVirtualMethods(superclass.asClasspathOrLibraryClass())) {
+ componentSignatures.put(vMethod, vMethod);
+ }
+ }
+ });
+ }
+ }
+
+ private DexMethodSignatureSet getOrComputeNonProgramVirtualMethods(
+ ClasspathOrLibraryClass clazz) {
+ DexMethodSignatureSet libraryMethodsOnClass = nonProgramVirtualMethods.get(clazz);
+ if (libraryMethodsOnClass != null) {
+ return libraryMethodsOnClass;
+ }
+ return computeNonProgramVirtualMethods(clazz);
+ }
+
+ private DexMethodSignatureSet computeNonProgramVirtualMethods(
+ ClasspathOrLibraryClass classpathOrLibraryClass) {
+ DexClass clazz = classpathOrLibraryClass.asDexClass();
+ DexMethodSignatureSet libraryMethodsOnClass = DexMethodSignatureSet.create();
+ clazz.forEachImmediateSupertype(
+ supertype -> {
+ DexClass superclass = appView.definitionFor(supertype);
+ if (superclass != null) {
+ assert !superclass.isProgramClass();
+ libraryMethodsOnClass.addAll(
+ getOrComputeNonProgramVirtualMethods(superclass.asClasspathOrLibraryClass()));
+ }
+ });
+ clazz.forEachClassMethodMatching(
+ DexEncodedMethod::belongsToVirtualPool,
+ method -> libraryMethodsOnClass.add(method.getMethodSignature()));
+ nonProgramVirtualMethods.put(classpathOrLibraryClass, libraryMethodsOnClass);
+ return libraryMethodsOnClass;
+ }
+}
diff --git a/src/main/java/com/android/tools/r8/graph/fixup/MethodNamingUtility.java b/src/main/java/com/android/tools/r8/graph/fixup/MethodNamingUtility.java
new file mode 100644
index 0000000..977e01c
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/graph/fixup/MethodNamingUtility.java
@@ -0,0 +1,119 @@
+// Copyright (c) 2023, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+package com.android.tools.r8.graph.fixup;
+
+import com.android.tools.r8.graph.DexEncodedMethod;
+import com.android.tools.r8.graph.DexItemFactory;
+import com.android.tools.r8.graph.DexMethod;
+import com.android.tools.r8.graph.DexMethodSignature;
+import com.android.tools.r8.graph.DexProto;
+import com.android.tools.r8.graph.DexType;
+import com.google.common.collect.BiMap;
+import java.util.function.BiConsumer;
+
+public class MethodNamingUtility {
+
+ private final DexItemFactory factory;
+ private final BiMap<DexMethodSignature, DexMethodSignature> inheritedSignatures;
+ private final BiMap<DexMethod, DexMethod> localSignatures;
+
+ public MethodNamingUtility(
+ DexItemFactory factory,
+ BiMap<DexMethodSignature, DexMethodSignature> inheritedSignatures,
+ BiMap<DexMethod, DexMethod> localSignatures) {
+ this.factory = factory;
+ this.inheritedSignatures = inheritedSignatures;
+ this.localSignatures = localSignatures;
+ }
+
+ public DexMethod nextUniqueMethod(
+ DexEncodedMethod method, DexProto newProto, DexType initExtraType) {
+ DexMethod reference = method.getReference();
+
+ if (method.isClassInitializer()) {
+ assert reference.getProto() == newProto;
+ return reference;
+ }
+
+ if (method.isInstanceInitializer()) {
+ assert initExtraType != null;
+ return nextUniqueInitializer(reference, newProto, initExtraType);
+ }
+
+ if (method.isNonPrivateVirtualMethod()) {
+ return nextUniqueVirtualMethod(reference, newProto);
+ }
+
+ return nextUniquePrivateOrStaticMethod(reference, newProto);
+ }
+
+ private DexMethod nextUniqueInitializer(
+ DexMethod reference, DexProto newProto, DexType initExtraType) {
+ assert !inheritedSignatures.containsKey(reference.getSignature());
+
+ // 1) We check if the reference has already been reserved (pinning).
+ DexMethod remapped = localSignatures.get(reference);
+ if (remapped != null) {
+ assert remapped.getProto() == newProto;
+ return remapped.withHolder(reference.getHolderType(), factory);
+ }
+
+ // 2) We check for collision with already mapped methods.
+ DexMethod newMethod = reference.withProto(newProto, factory);
+ if (localSignatures.containsValue(newMethod)) {
+ // This collides with something that has been renamed into this.
+ newMethod =
+ factory.createInstanceInitializerWithFreshProto(
+ newMethod, initExtraType, tryMethod -> !localSignatures.containsValue(tryMethod));
+ }
+
+ // 3) Finally register the new method and return it.
+ assert !localSignatures.containsValue(newMethod);
+ localSignatures.put(reference, newMethod);
+ return newMethod;
+ }
+
+ private DexMethod nextUniquePrivateOrStaticMethod(DexMethod reference, DexProto newProto) {
+ return nextUniqueMethod(reference, newProto, localSignatures::put);
+ }
+
+ private DexMethod nextUniqueVirtualMethod(DexMethod reference, DexProto newProto) {
+ return nextUniqueMethod(
+ reference,
+ newProto,
+ (from, to) -> inheritedSignatures.put(from.getSignature(), to.getSignature()));
+ }
+
+ private boolean anyCollision(DexMethod method) {
+ return localSignatures.containsValue(method)
+ || inheritedSignatures.containsValue(method.getSignature());
+ }
+
+ private DexMethod nextUniqueMethod(
+ DexMethod reference, DexProto newProto, BiConsumer<DexMethod, DexMethod> registration) {
+ // 1) We check if the reference has already been reserved (pinning or override).
+ DexMethodSignature remappedSignature = inheritedSignatures.get(reference.getSignature());
+ if (remappedSignature != null) {
+ assert remappedSignature.getProto() == newProto;
+ return remappedSignature.withHolder(reference.getHolderType(), factory);
+ }
+
+ // 2) We check for collision with already mapped methods.
+ DexMethod newMethod = reference.withProto(newProto, factory);
+ if (anyCollision(newMethod)) {
+ newMethod =
+ factory.createFreshMethodNameWithoutHolder(
+ newMethod.getName().toString(),
+ newMethod.getProto(),
+ newMethod.getHolderType(),
+ tryMethod -> !anyCollision(tryMethod));
+ }
+
+ // 3) Finally register the new method and return it.
+ assert !anyCollision(newMethod);
+ registration.accept(reference, newMethod);
+ return newMethod;
+ }
+}
diff --git a/src/main/java/com/android/tools/r8/graph/TreeFixerBase.java b/src/main/java/com/android/tools/r8/graph/fixup/TreeFixerBase.java
similarity index 93%
rename from src/main/java/com/android/tools/r8/graph/TreeFixerBase.java
rename to src/main/java/com/android/tools/r8/graph/fixup/TreeFixerBase.java
index f3cc3e9..bf5e3de 100644
--- a/src/main/java/com/android/tools/r8/graph/TreeFixerBase.java
+++ b/src/main/java/com/android/tools/r8/graph/fixup/TreeFixerBase.java
@@ -2,8 +2,26 @@
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
-package com.android.tools.r8.graph;
+package com.android.tools.r8.graph.fixup;
+import com.android.tools.r8.graph.AppView;
+import com.android.tools.r8.graph.DexEncodedField;
+import com.android.tools.r8.graph.DexEncodedMethod;
+import com.android.tools.r8.graph.DexField;
+import com.android.tools.r8.graph.DexItemFactory;
+import com.android.tools.r8.graph.DexMethod;
+import com.android.tools.r8.graph.DexMethodSignature;
+import com.android.tools.r8.graph.DexProgramClass;
+import com.android.tools.r8.graph.DexProto;
+import com.android.tools.r8.graph.DexString;
+import com.android.tools.r8.graph.DexType;
+import com.android.tools.r8.graph.DexTypeList;
+import com.android.tools.r8.graph.EnclosingMethodAttribute;
+import com.android.tools.r8.graph.InnerClassAttribute;
+import com.android.tools.r8.graph.NestHostClassAttribute;
+import com.android.tools.r8.graph.NestMemberClassAttribute;
+import com.android.tools.r8.graph.PermittedSubclassAttribute;
+import com.android.tools.r8.graph.RecordComponentInfo;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.android.tools.r8.utils.ConsumerUtils;
import com.android.tools.r8.utils.DescriptorUtils;
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/TreeFixer.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/TreeFixer.java
index a12baa2..482b43f 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/TreeFixer.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/TreeFixer.java
@@ -18,7 +18,7 @@
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.graph.DexTypeList;
import com.android.tools.r8.graph.EnclosingMethodAttribute;
-import com.android.tools.r8.graph.TreeFixerBase;
+import com.android.tools.r8.graph.fixup.TreeFixerBase;
import com.android.tools.r8.horizontalclassmerging.HorizontalClassMerger.Mode;
import com.android.tools.r8.ir.conversion.ExtraUnusedNullParameter;
import com.android.tools.r8.profile.rewriting.ProfileCollectionAdditions;
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxingLens.java b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxingLens.java
index da8ac44..3aaff49 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxingLens.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxingLens.java
@@ -201,7 +201,7 @@
newMethodSignatures = new BidirectionalOneToManyRepresentativeHashMap<>();
private final Map<DexMethod, DexMethod> methodMap = new IdentityHashMap<>();
- private Map<DexMethod, RewrittenPrototypeDescription> prototypeChangesPerMethod =
+ private final Map<DexMethod, RewrittenPrototypeDescription> prototypeChangesPerMethod =
new IdentityHashMap<>();
Builder(AppView<AppInfoWithLiveness> appView) {
@@ -219,41 +219,31 @@
if (from == to) {
return;
}
- newFieldSignatures.put(from, to);
- }
-
- private RewrittenPrototypeDescription recordPrototypeChanges(
- DexMethod from,
- DexMethod to,
- boolean fromStatic,
- boolean toStatic,
- boolean virtualReceiverAlreadyRemapped,
- List<ExtraUnusedNullParameter> extraUnusedNullParameters) {
- assert from != to;
- RewrittenPrototypeDescription prototypeChanges =
- computePrototypeChanges(
- from,
- to,
- fromStatic,
- toStatic,
- virtualReceiverAlreadyRemapped,
- extraUnusedNullParameters);
- prototypeChangesPerMethod.put(to, prototypeChanges);
- return prototypeChanges;
+ synchronized (this) {
+ newFieldSignatures.put(from, to);
+ }
}
public void moveAndMap(DexMethod from, DexMethod to, boolean fromStatic) {
moveAndMap(from, to, fromStatic, true, Collections.emptyList());
}
- public RewrittenPrototypeDescription moveVirtual(DexMethod from, DexMethod to) {
- newMethodSignatures.put(from, to);
- return recordPrototypeChanges(from, to, false, true, false, Collections.emptyList());
+ public void moveVirtual(DexMethod from, DexMethod to) {
+ RewrittenPrototypeDescription prototypeChanges =
+ computePrototypeChanges(from, to, false, true, false, Collections.emptyList());
+ synchronized (this) {
+ newMethodSignatures.put(from, to);
+ prototypeChangesPerMethod.put(to, prototypeChanges);
+ }
}
- public RewrittenPrototypeDescription mapToDispatch(DexMethod from, DexMethod to) {
- methodMap.put(from, to);
- return recordPrototypeChanges(from, to, false, true, true, Collections.emptyList());
+ public void mapToDispatch(DexMethod from, DexMethod to) {
+ RewrittenPrototypeDescription prototypeChanges =
+ computePrototypeChanges(from, to, false, true, true, Collections.emptyList());
+ synchronized (this) {
+ methodMap.put(from, to);
+ prototypeChangesPerMethod.put(to, prototypeChanges);
+ }
}
public RewrittenPrototypeDescription moveAndMap(
@@ -262,10 +252,14 @@
boolean fromStatic,
boolean toStatic,
List<ExtraUnusedNullParameter> extraUnusedNullParameters) {
- newMethodSignatures.put(from, to);
- methodMap.put(from, to);
- return recordPrototypeChanges(
- from, to, fromStatic, toStatic, false, extraUnusedNullParameters);
+ RewrittenPrototypeDescription prototypeChanges =
+ computePrototypeChanges(from, to, fromStatic, toStatic, false, extraUnusedNullParameters);
+ synchronized (this) {
+ newMethodSignatures.put(from, to);
+ methodMap.put(from, to);
+ prototypeChangesPerMethod.put(to, prototypeChanges);
+ }
+ return prototypeChanges;
}
private RewrittenPrototypeDescription computePrototypeChanges(
@@ -275,6 +269,7 @@
boolean toStatic,
boolean virtualReceiverAlreadyRemapped,
List<ExtraUnusedNullParameter> extraUnusedNullParameters) {
+ assert from != to;
int offsetDiff = 0;
int toOffset = BooleanUtils.intValue(!toStatic);
ArgumentInfoCollection.Builder builder =
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxingTreeFixer.java b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxingTreeFixer.java
index 8d04a32..e0d0b36 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxingTreeFixer.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxingTreeFixer.java
@@ -9,7 +9,6 @@
import com.android.tools.r8.contexts.CompilationContext.ProcessorContext;
import com.android.tools.r8.graph.AppView;
-import com.android.tools.r8.graph.DexClass;
import com.android.tools.r8.graph.DexClassAndMethod;
import com.android.tools.r8.graph.DexEncodedField;
import com.android.tools.r8.graph.DexEncodedField.Builder;
@@ -26,6 +25,9 @@
import com.android.tools.r8.graph.ProgramField;
import com.android.tools.r8.graph.ProgramMethod;
import com.android.tools.r8.graph.PrunedItems;
+import com.android.tools.r8.graph.fixup.ConcurrentMethodFixup;
+import com.android.tools.r8.graph.fixup.ConcurrentMethodFixup.ProgramClassFixer;
+import com.android.tools.r8.graph.fixup.MethodNamingUtility;
import com.android.tools.r8.graph.lens.MethodLookupResult;
import com.android.tools.r8.graph.proto.RewrittenPrototypeDescription;
import com.android.tools.r8.ir.analysis.type.ClassTypeElement;
@@ -81,7 +83,7 @@
import java.util.concurrent.ExecutorService;
import java.util.function.Predicate;
-class EnumUnboxingTreeFixer {
+class EnumUnboxingTreeFixer implements ProgramClassFixer {
private final EnumUnboxingLens.Builder lensBuilder;
private final AppView<AppInfoWithLiveness> appView;
@@ -94,7 +96,9 @@
// we duplicate that here as DexProgramClasses.
private final Map<DexProgramClass, Set<DexProgramClass>> unboxedEnumHierarchy;
private final EnumUnboxingUtilityClasses utilityClasses;
- private final ProgramMethodMap<CfCodeWithLens> dispatchMethods = ProgramMethodMap.create();
+ private final ProgramMethodMap<CfCodeWithLens> dispatchMethods =
+ ProgramMethodMap.createConcurrent();
+ private final PrunedItems.Builder prunedItemsBuilder;
EnumUnboxingTreeFixer(
AppView<AppInfoWithLiveness> appView,
@@ -110,6 +114,7 @@
this.lensBuilder =
EnumUnboxingLens.enumUnboxingLensBuilder(appView).mapUnboxedEnums(getUnboxedEnums());
this.utilityClasses = utilityClasses;
+ this.prunedItemsBuilder = PrunedItems.concurrentBuilder();
}
private Set<DexProgramClass> computeUnboxedEnumClasses() {
@@ -128,39 +133,13 @@
Result fixupTypeReferences(IRConverter converter, ExecutorService executorService)
throws ExecutionException {
- PrunedItems.Builder prunedItemsBuilder = PrunedItems.builder();
// We do this before so that we can still perform lookup of definitions.
fixupSuperEnumClassInitializers(converter, executorService);
// Fix all methods and fields using enums to unbox.
- // TODO(b/191617665): Parallelize this fixup.
- for (DexProgramClass clazz : appView.appInfo().classes()) {
- if (enumDataMap.isSuperUnboxedEnum(clazz.getType())) {
-
- // Clear the initializers and move the other methods to the new location.
- LocalEnumUnboxingUtilityClass localUtilityClass =
- utilityClasses.getLocalUtilityClass(clazz);
- Collection<DexEncodedField> localUtilityFields =
- createLocalUtilityFields(clazz, localUtilityClass, prunedItemsBuilder);
- Collection<DexEncodedMethod> localUtilityMethods =
- createLocalUtilityMethods(
- clazz, unboxedEnumHierarchy.get(clazz), localUtilityClass, prunedItemsBuilder);
-
- // Cleanup old classes.
- cleanUpOldClass(clazz);
- for (DexProgramClass subEnum : unboxedEnumHierarchy.get(clazz)) {
- cleanUpOldClass(subEnum);
- }
-
- // Update members on the local utility class.
- localUtilityClass.getDefinition().setDirectMethods(localUtilityMethods);
- localUtilityClass.getDefinition().setStaticFields(localUtilityFields);
- } else if (!enumDataMap.isUnboxedEnum(clazz.getType())) {
- clazz.getMethodCollection().replaceMethods(this::fixupEncodedMethod);
- clazz.getFieldCollection().replaceFields(this::fixupEncodedField);
- }
- }
+ new ConcurrentMethodFixup(appView, this)
+ .fixupClassesConcurrentlyByConnectedProgramComponents(Timing.empty(), executorService);
// Install the new graph lens before processing any checkNotZero() methods.
EnumUnboxingLens lens = lensBuilder.build(appView);
@@ -191,6 +170,29 @@
clazz.getMethodCollection().clearVirtualMethods();
}
+ @Override
+ public void fixupProgramClass(DexProgramClass clazz, MethodNamingUtility utility) {
+ if (enumDataMap.isSuperUnboxedEnum(clazz.getType())) {
+ // Clear the initializers and move the other methods to the new location.
+ LocalEnumUnboxingUtilityClass localUtilityClass = utilityClasses.getLocalUtilityClass(clazz);
+ Collection<DexEncodedField> localUtilityFields =
+ createLocalUtilityFields(clazz, localUtilityClass);
+ Collection<DexEncodedMethod> localUtilityMethods =
+ createLocalUtilityMethods(clazz, unboxedEnumHierarchy.get(clazz), localUtilityClass);
+ // Cleanup old classes.
+ cleanUpOldClass(clazz);
+ for (DexProgramClass subEnum : unboxedEnumHierarchy.get(clazz)) {
+ cleanUpOldClass(subEnum);
+ }
+ // Update members on the local utility class.
+ localUtilityClass.getDefinition().setDirectMethods(localUtilityMethods);
+ localUtilityClass.getDefinition().setStaticFields(localUtilityFields);
+ } else if (!enumDataMap.isUnboxedEnum(clazz.getType())) {
+ clazz.getMethodCollection().replaceMethods(m -> fixupEncodedMethod(m, utility));
+ clazz.getFieldCollection().replaceFields(this::fixupEncodedField);
+ }
+ }
+
private BiMap<DexMethod, DexMethod> duplicateCheckNotNullMethods(
IRConverter converter, ExecutorService executorService) throws ExecutionException {
BiMap<DexMethod, DexMethod> checkNotNullToCheckNotZeroMapping = HashBiMap.create();
@@ -473,9 +475,7 @@
}
private Collection<DexEncodedField> createLocalUtilityFields(
- DexProgramClass unboxedEnum,
- LocalEnumUnboxingUtilityClass localUtilityClass,
- PrunedItems.Builder prunedItemsBuilder) {
+ DexProgramClass unboxedEnum, LocalEnumUnboxingUtilityClass localUtilityClass) {
EnumData enumData = enumDataMap.get(unboxedEnum);
Map<DexField, DexEncodedField> localUtilityFields =
new LinkedHashMap<>(unboxedEnum.staticFields().size());
@@ -532,7 +532,6 @@
private void processMethod(
ProgramMethod method,
- PrunedItems.Builder prunedItemsBuilder,
DexMethodSignatureSet nonPrivateVirtualMethods,
LocalEnumUnboxingUtilityClass localUtilityClass,
Map<DexMethod, DexEncodedMethod> localUtilityMethods) {
@@ -552,8 +551,7 @@
private Collection<DexEncodedMethod> createLocalUtilityMethods(
DexProgramClass unboxedEnum,
Set<DexProgramClass> subEnums,
- LocalEnumUnboxingUtilityClass localUtilityClass,
- PrunedItems.Builder prunedItemsBuilder) {
+ LocalEnumUnboxingUtilityClass localUtilityClass) {
Map<DexMethod, DexEncodedMethod> localUtilityMethods =
new LinkedHashMap<>(
localUtilityClass.getDefinition().getMethodCollection().size()
@@ -568,7 +566,6 @@
method ->
processMethod(
method,
- prunedItemsBuilder,
nonPrivateVirtualMethods,
localUtilityClass,
localUtilityMethods));
@@ -578,7 +575,6 @@
method ->
processMethod(
method,
- prunedItemsBuilder,
nonPrivateVirtualMethods,
localUtilityClass,
localUtilityMethods));
@@ -804,20 +800,22 @@
&& !field.getDefinition().getOptimizationInfo().isDead());
}
- private DexEncodedMethod fixupEncodedMethod(DexEncodedMethod method) {
+ private DexEncodedMethod fixupEncodedMethod(
+ DexEncodedMethod method, MethodNamingUtility utility) {
DexProto oldProto = method.getProto();
DexProto newProto = fixupProto(oldProto);
- if (newProto == method.getProto()) {
+ // Even if the protos are identical, we may generate collisions and decide to rename the
+ // unchanged method. In most cases this is a no-op if the proto are identical.
+ DexMethod newMethod =
+ utility.nextUniqueMethod(
+ method, newProto, utilityClasses.getSharedUtilityClass().getType());
+ if (newMethod == method.getReference()) {
return method;
}
assert !method.isClassInitializer();
assert !method.isLibraryMethodOverride().isTrue()
: "Enum unboxing is changing the signature of a library override in a non unboxed class.";
- // We add the $enumunboxing$ suffix to make sure we do not create a library override.
- String newMethodName =
- method.getName().toString() + (method.isNonPrivateVirtualMethod() ? "$enumunboxing$" : "");
- DexMethod newMethod = factory.createMethod(method.getHolderType(), newProto, newMethodName);
- newMethod = ensureUniqueMethod(method, newMethod);
+
List<ExtraUnusedNullParameter> extraUnusedNullParameters =
ExtraUnusedNullParameter.computeExtraUnusedNullParameters(method.getReference(), newMethod);
boolean isStatic = method.isStatic();
@@ -835,27 +833,6 @@
method.isNonPrivateVirtualMethod(), OptionalBool.FALSE));
}
- private DexMethod ensureUniqueMethod(DexEncodedMethod encodedMethod, DexMethod newMethod) {
- DexClass holder = appView.definitionFor(encodedMethod.getHolderType());
- assert holder != null;
- if (newMethod.isInstanceInitializer(appView.dexItemFactory())) {
- newMethod =
- factory.createInstanceInitializerWithFreshProto(
- newMethod,
- utilityClasses.getSharedUtilityClass().getType(),
- tryMethod -> holder.lookupMethod(tryMethod) == null);
- } else {
- int index = 0;
- while (holder.lookupMethod(newMethod) != null) {
- newMethod =
- newMethod.withName(
- encodedMethod.getName().toString() + "$enumunboxing$" + index++,
- appView.dexItemFactory());
- }
- }
- return newMethod;
- }
-
private DexEncodedField fixupEncodedField(DexEncodedField encodedField) {
DexField field = encodedField.getReference();
DexType newType = fixupType(field.type);
diff --git a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/ArgumentPropagatorApplicationFixer.java b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/ArgumentPropagatorApplicationFixer.java
index eff4f92..1f5b5ff 100644
--- a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/ArgumentPropagatorApplicationFixer.java
+++ b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/ArgumentPropagatorApplicationFixer.java
@@ -18,7 +18,7 @@
import com.android.tools.r8.graph.GenericSignature.MethodTypeSignature;
import com.android.tools.r8.graph.MethodCollection;
import com.android.tools.r8.graph.PrunedItems;
-import com.android.tools.r8.graph.TreeFixerBase;
+import com.android.tools.r8.graph.fixup.TreeFixerBase;
import com.android.tools.r8.graph.lens.GraphLens;
import com.android.tools.r8.graph.proto.RewrittenPrototypeDescription;
import com.android.tools.r8.ir.optimize.info.MethodOptimizationInfoFixer;
diff --git a/src/main/java/com/android/tools/r8/repackaging/Repackaging.java b/src/main/java/com/android/tools/r8/repackaging/Repackaging.java
index cffb1cf..ff4fc92 100644
--- a/src/main/java/com/android/tools/r8/repackaging/Repackaging.java
+++ b/src/main/java/com/android/tools/r8/repackaging/Repackaging.java
@@ -20,7 +20,7 @@
import com.android.tools.r8.graph.ProgramPackage;
import com.android.tools.r8.graph.ProgramPackageCollection;
import com.android.tools.r8.graph.SortedProgramPackageCollection;
-import com.android.tools.r8.graph.TreeFixerBase;
+import com.android.tools.r8.graph.fixup.TreeFixerBase;
import com.android.tools.r8.graph.lens.NestedGraphLens;
import com.android.tools.r8.naming.Minifier.MinificationPackageNamingStrategy;
import com.android.tools.r8.repackaging.RepackagingLens.Builder;
diff --git a/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
index 9b437c1..150e7c8 100644
--- a/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
+++ b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
@@ -56,10 +56,10 @@
import com.android.tools.r8.graph.PrunedItems;
import com.android.tools.r8.graph.SubtypingInfo;
import com.android.tools.r8.graph.TopDownClassHierarchyTraversal;
-import com.android.tools.r8.graph.TreeFixerBase;
import com.android.tools.r8.graph.UseRegistry;
import com.android.tools.r8.graph.UseRegistryWithResult;
import com.android.tools.r8.graph.classmerging.VerticallyMergedClasses;
+import com.android.tools.r8.graph.fixup.TreeFixerBase;
import com.android.tools.r8.graph.lens.FieldLookupResult;
import com.android.tools.r8.graph.lens.GraphLens;
import com.android.tools.r8.graph.lens.MethodLookupResult;
diff --git a/src/main/java/com/android/tools/r8/synthesis/SyntheticFinalization.java b/src/main/java/com/android/tools/r8/synthesis/SyntheticFinalization.java
index 68c2918..c955fad 100644
--- a/src/main/java/com/android/tools/r8/synthesis/SyntheticFinalization.java
+++ b/src/main/java/com/android/tools/r8/synthesis/SyntheticFinalization.java
@@ -20,7 +20,7 @@
import com.android.tools.r8.graph.DexString;
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.graph.PrunedItems;
-import com.android.tools.r8.graph.TreeFixerBase;
+import com.android.tools.r8.graph.fixup.TreeFixerBase;
import com.android.tools.r8.graph.lens.GraphLens;
import com.android.tools.r8.graph.lens.NestedGraphLens;
import com.android.tools.r8.graph.lens.NonIdentityGraphLens;
diff --git a/src/main/java/com/android/tools/r8/utils/collections/LongLivedProgramMethodSetBuilder.java b/src/main/java/com/android/tools/r8/utils/collections/LongLivedProgramMethodSetBuilder.java
index 81578f8..604695c 100644
--- a/src/main/java/com/android/tools/r8/utils/collections/LongLivedProgramMethodSetBuilder.java
+++ b/src/main/java/com/android/tools/r8/utils/collections/LongLivedProgramMethodSetBuilder.java
@@ -168,7 +168,7 @@
appView.graphLens().getRenamedMethodSignature(method, appliedGraphLens);
DexProgramClass holder = appView.definitionForHolder(rewrittenMethod).asProgramClass();
DexEncodedMethod definition = holder.lookupMethod(rewrittenMethod);
- assert definition != null;
+ assert definition != null : "Missing method: " + rewrittenMethod;
result.createAndAdd(holder, definition);
}
return result;
diff --git a/src/test/java/com/android/tools/r8/enumunboxing/VirtualMethodOverrideEnumUnboxingTest.java b/src/test/java/com/android/tools/r8/enumunboxing/VirtualMethodOverrideEnumUnboxingTest.java
index d14fe97..8308e0b 100644
--- a/src/test/java/com/android/tools/r8/enumunboxing/VirtualMethodOverrideEnumUnboxingTest.java
+++ b/src/test/java/com/android/tools/r8/enumunboxing/VirtualMethodOverrideEnumUnboxingTest.java
@@ -6,7 +6,7 @@
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
import static org.hamcrest.MatcherAssert.assertThat;
-import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
import com.android.tools.r8.KeepConstantArguments;
import com.android.tools.r8.NeverClassInline;
@@ -67,8 +67,7 @@
MethodSubject methodOnB =
inspector.clazz(B.class).uniqueMethodWithFinalName(methodOnA.getFinalName());
assertThat(methodOnB, isPresent());
- // TODO(b/171784168): Should be true.
- assertFalse(methodOnB.streamInstructions().anyMatch(x -> x.asDexInstruction().isInvokeSuper()));
+ assertTrue(methodOnB.streamInstructions().anyMatch(x -> x.asDexInstruction().isInvokeSuper()));
}
static class TestClass {
