Fix nondeterminism in service loader rewriter
Bug: 156054499
Change-Id: Iaf322a99a7c79c904665c7cdfd0b0f705f08a355
diff --git a/src/main/java/com/android/tools/r8/graph/MethodCollection.java b/src/main/java/com/android/tools/r8/graph/MethodCollection.java
index bad82e1..14655d5 100644
--- a/src/main/java/com/android/tools/r8/graph/MethodCollection.java
+++ b/src/main/java/com/android/tools/r8/graph/MethodCollection.java
@@ -20,7 +20,7 @@
private static final int ARRAY_BACKING_THRESHOLD = 30;
private final DexClass holder;
- private final MethodCollectionBacking backing;
+ private MethodCollectionBacking backing;
private DexEncodedMethod cachedClassInitializer = DexEncodedMethod.SENTINEL;
public MethodCollection(
@@ -291,6 +291,11 @@
.shouldBreak();
}
+ public void useSortedBacking() {
+ assert size() == 0;
+ backing = MethodMapBacking.createSorted();
+ }
+
public boolean verify() {
forEachMethod(
method -> {
diff --git a/src/main/java/com/android/tools/r8/graph/MethodMapBacking.java b/src/main/java/com/android/tools/r8/graph/MethodMapBacking.java
index 4ba116d..18257c1 100644
--- a/src/main/java/com/android/tools/r8/graph/MethodMapBacking.java
+++ b/src/main/java/com/android/tools/r8/graph/MethodMapBacking.java
@@ -10,7 +10,9 @@
import com.google.common.base.Equivalence.Wrapper;
import it.unimi.dsi.fastutil.objects.Object2ReferenceLinkedOpenHashMap;
import it.unimi.dsi.fastutil.objects.Object2ReferenceMap;
+import it.unimi.dsi.fastutil.objects.Object2ReferenceRBTreeMap;
import java.util.Collection;
+import java.util.Comparator;
import java.util.Map.Entry;
import java.util.Set;
import java.util.function.Function;
@@ -21,7 +23,16 @@
private Object2ReferenceMap<Wrapper<DexMethod>, DexEncodedMethod> methodMap;
public MethodMapBacking() {
- this.methodMap = createMap();
+ this(createMap());
+ }
+
+ private MethodMapBacking(Object2ReferenceMap<Wrapper<DexMethod>, DexEncodedMethod> methodMap) {
+ this.methodMap = methodMap;
+ }
+
+ public static MethodMapBacking createSorted() {
+ Comparator<Wrapper<DexMethod>> comparator = (x, y) -> x.get().slowCompareTo(y.get());
+ return new MethodMapBacking(new Object2ReferenceRBTreeMap<>(comparator));
}
private static Object2ReferenceMap<Wrapper<DexMethod>, DexEncodedMethod> createMap() {
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
index 71ff0c2..2fb1a28 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
@@ -1223,7 +1223,7 @@
if (serviceLoaderRewriter != null) {
assert appView.appInfo().hasLiveness();
timing.begin("Rewrite service loaders");
- serviceLoaderRewriter.rewrite(code);
+ serviceLoaderRewriter.rewrite(code, methodProcessingId);
timing.end();
}
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/MethodProcessingId.java b/src/main/java/com/android/tools/r8/ir/conversion/MethodProcessingId.java
index 60d970e..7e4397c 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/MethodProcessingId.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/MethodProcessingId.java
@@ -5,6 +5,8 @@
package com.android.tools.r8.ir.conversion;
import com.android.tools.r8.graph.ProgramMethod;
+import com.android.tools.r8.utils.InternalOptions;
+import com.android.tools.r8.utils.collections.ProgramMethodSet;
import com.android.tools.r8.utils.collections.SortedProgramMethodSet;
import java.util.function.BiConsumer;
@@ -58,6 +60,9 @@
private final int firstReservedId;
private final int numberOfReservedIds;
+ private final ProgramMethodSet seen =
+ InternalOptions.assertionsEnabled() ? ProgramMethodSet.create() : null;
+
public ReservedMethodProcessingIds(int firstReservedId, int numberOfReservedIds) {
this.firstReservedId = firstReservedId;
this.numberOfReservedIds = numberOfReservedIds;
@@ -66,6 +71,7 @@
public MethodProcessingId get(ProgramMethod method, int index) {
assert index >= 0;
assert index < numberOfReservedIds;
+ assert seen.add(method);
MethodProcessingId result = new MethodProcessingId(firstReservedId + index);
if (consumer != null) {
consumer.accept(method, result);
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/ServiceLoaderRewriter.java b/src/main/java/com/android/tools/r8/ir/optimize/ServiceLoaderRewriter.java
index 50666b6..d0ad73a 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/ServiceLoaderRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/ServiceLoaderRewriter.java
@@ -21,7 +21,6 @@
import com.android.tools.r8.graph.MethodAccessFlags;
import com.android.tools.r8.graph.MethodCollection;
import com.android.tools.r8.graph.ParameterAnnotationsList;
-import com.android.tools.r8.graph.ProgramMethod;
import com.android.tools.r8.ir.code.ConstClass;
import com.android.tools.r8.ir.code.IRCode;
import com.android.tools.r8.ir.code.Instruction;
@@ -29,11 +28,10 @@
import com.android.tools.r8.ir.code.InvokeStatic;
import com.android.tools.r8.ir.code.InvokeVirtual;
import com.android.tools.r8.ir.code.Value;
+import com.android.tools.r8.ir.conversion.MethodProcessingId;
import com.android.tools.r8.ir.desugar.ServiceLoaderSourceCode;
import com.android.tools.r8.origin.SynthesizedOrigin;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
-import com.android.tools.r8.utils.IntBox;
-import com.android.tools.r8.utils.StringUtils;
import com.google.common.collect.ImmutableList;
import java.util.ArrayList;
import java.util.Collections;
@@ -72,7 +70,6 @@
public static final String SERVICE_LOADER_CLASS_NAME = "$$ServiceLoaderMethods";
private static final String SERVICE_LOADER_METHOD_PREFIX_NAME = "$load";
- private static final int SERVICE_LOADER_METHOD_HASH_LENGTH = 7;
private AtomicReference<DexProgramClass> synthesizedClass = new AtomicReference<>();
private ConcurrentHashMap<DexType, DexEncodedMethod> synthesizedServiceLoaders =
@@ -88,10 +85,9 @@
return synthesizedClass.get();
}
- public void rewrite(IRCode code) {
+ public void rewrite(IRCode code, MethodProcessingId methodProcessingId) {
DexItemFactory factory = appView.dexItemFactory();
InstructionListIterator instructionIterator = code.instructionListIterator();
- IntBox synthesizedLoadMethodCounter = new IntBox();
while (instructionIterator.hasNext()) {
Instruction instruction = instructionIterator.next();
@@ -176,8 +172,7 @@
constClass.getValue(),
service -> {
DexEncodedMethod addedMethod =
- createSynthesizedMethod(
- service, classes, code.context(), synthesizedLoadMethodCounter);
+ createSynthesizedMethod(service, classes, methodProcessingId);
if (appView.options().isGeneratingClassFiles()) {
addedMethod.upgradeClassFileVersion(code.method().getClassFileVersion());
}
@@ -190,23 +185,9 @@
}
private DexEncodedMethod createSynthesizedMethod(
- DexType serviceType,
- List<DexClass> classes,
- ProgramMethod context,
- IntBox synthesizedLoadMethodCounter) {
+ DexType serviceType, List<DexClass> classes, MethodProcessingId methodProcessingId) {
MethodCollection methodCollection = getOrSetSynthesizedClass().getMethodCollection();
- String hashCode = Integer.toString(context.getReference().hashCode());
- String methodNamePrefix =
- SERVICE_LOADER_METHOD_PREFIX_NAME
- + "$"
- + StringUtils.replaceAll(context.getHolderType().toSourceString(), ".", "$")
- + "$"
- + (context.getDefinition().isInitializer()
- ? (context.getDefinition().isClassInitializer() ? "$clinit" : "$init")
- : context.getReference().name.toSourceString())
- + "$"
- + hashCode.substring(0, Math.min(SERVICE_LOADER_METHOD_HASH_LENGTH, hashCode.length()))
- + "$";
+ String methodNamePrefix = SERVICE_LOADER_METHOD_PREFIX_NAME + "$";
DexProto proto = appView.dexItemFactory().createProto(appView.dexItemFactory().iteratorType);
synchronized (methodCollection) {
DexMethod methodReference;
@@ -217,7 +198,7 @@
.createMethod(
appView.dexItemFactory().serviceLoaderRewrittenClassType,
proto,
- methodNamePrefix + "$" + synthesizedLoadMethodCounter.getAndIncrement());
+ methodNamePrefix + methodProcessingId.getAndIncrementId());
} while (methodCollection.getMethod(methodReference) != null);
DexEncodedMethod method =
new DexEncodedMethod(
@@ -241,30 +222,33 @@
ChecksumSupplier checksumSupplier = DexProgramClass::invalidChecksumRequest;
DexProgramClass clazz =
synthesizedClass.updateAndGet(
- existingClazz -> {
- if (existingClazz != null) {
- return existingClazz;
+ existingClass -> {
+ if (existingClass != null) {
+ return existingClass;
}
- return new DexProgramClass(
- appView.dexItemFactory().serviceLoaderRewrittenClassType,
- null,
- new SynthesizedOrigin("Service Loader desugaring", getClass()),
- ClassAccessFlags.fromDexAccessFlags(
- Constants.ACC_FINAL | Constants.ACC_SYNTHETIC | Constants.ACC_PUBLIC),
- appView.dexItemFactory().objectType,
- DexTypeList.empty(),
- appView.dexItemFactory().createString("ServiceLoader"),
- null,
- Collections.emptyList(),
- null,
- Collections.emptyList(),
- DexAnnotationSet.empty(),
- DexEncodedField.EMPTY_ARRAY, // Static fields.
- DexEncodedField.EMPTY_ARRAY, // Instance fields.
- DexEncodedMethod.EMPTY_ARRAY,
- DexEncodedMethod.EMPTY_ARRAY, // Virtual methods.
- appView.dexItemFactory().getSkipNameValidationForTesting(),
- checksumSupplier);
+ DexProgramClass newClass =
+ new DexProgramClass(
+ appView.dexItemFactory().serviceLoaderRewrittenClassType,
+ null,
+ new SynthesizedOrigin("Service Loader desugaring", getClass()),
+ ClassAccessFlags.fromDexAccessFlags(
+ Constants.ACC_FINAL | Constants.ACC_SYNTHETIC | Constants.ACC_PUBLIC),
+ appView.dexItemFactory().objectType,
+ DexTypeList.empty(),
+ appView.dexItemFactory().createString("ServiceLoader"),
+ null,
+ Collections.emptyList(),
+ null,
+ Collections.emptyList(),
+ DexAnnotationSet.empty(),
+ DexEncodedField.EMPTY_ARRAY, // Static fields.
+ DexEncodedField.EMPTY_ARRAY, // Instance fields.
+ DexEncodedMethod.EMPTY_ARRAY,
+ DexEncodedMethod.EMPTY_ARRAY, // Virtual methods.
+ appView.dexItemFactory().getSkipNameValidationForTesting(),
+ checksumSupplier);
+ newClass.getMethodCollection().useSortedBacking();
+ return newClass;
});
assert clazz != null;
appView.appInfo().addSynthesizedClass(clazz);
