Fix race in horizontal merging leading to nondeterministic art profile
Bug: b/434861604
Change-Id: I56f5d8a4f95be92c56804c663cde7b56223ad702
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/UndoConstructorInlining.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/UndoConstructorInlining.java
index 70a2a9a..af621ab 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/UndoConstructorInlining.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/UndoConstructorInlining.java
@@ -63,7 +63,6 @@
import java.util.Set;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
-import java.util.function.Consumer;
import java.util.function.Function;
import java.util.stream.Collectors;
@@ -247,9 +246,8 @@
info.getProgramClass(),
info.getInvokedMethod(),
ensureConstructorsOnClasses,
- newConstructor ->
- profileCollectionAdditions.addMethodIfContextIsInProfile(
- newConstructor, method));
+ method,
+ profileCollectionAdditions);
if (newInvokedMethod.getArity() != info.getInvokedMethod().getArity()) {
assert newInvokedMethod.getArity() > info.getInvokedMethod().getArity();
return rewriteIR(method, code);
@@ -310,9 +308,8 @@
noSkipClass,
invokedMethod,
ensureConstructorsOnClasses,
- newConstructor ->
- profileCollectionAdditions.addMethodIfContextIsInProfile(
- newConstructor, method));
+ method,
+ profileCollectionAdditions);
InvokeDirect.Builder invokeDirectBuilder =
InvokeDirect.builder()
.setArguments(invoke.arguments())
@@ -484,19 +481,30 @@
DexProgramClass clazz,
DexMethod target,
Map<DexType, DexProgramClass> ensureConstructorsOnClasses,
- Consumer<ProgramMethod> creationConsumer) {
- return constructorCache
- .computeIfAbsent(clazz, ignoreKey(IdentityHashMap::new))
- .computeIfAbsent(
- target,
- k -> createConstructor(clazz, target, ensureConstructorsOnClasses, creationConsumer));
+ ProgramMethod context,
+ ProfileCollectionAdditions profileCollectionAdditions) {
+ ProgramMethod constructor =
+ constructorCache
+ .computeIfAbsent(clazz, ignoreKey(IdentityHashMap::new))
+ .computeIfAbsent(
+ target,
+ k ->
+ createConstructor(
+ clazz,
+ target,
+ ensureConstructorsOnClasses,
+ context,
+ profileCollectionAdditions));
+ profileCollectionAdditions.addMethodIfContextIsInProfile(constructor, context);
+ return constructor;
}
private ProgramMethod createConstructor(
DexProgramClass clazz,
DexMethod target,
Map<DexType, DexProgramClass> ensureConstructorsOnClasses,
- Consumer<ProgramMethod> creationConsumer) {
+ ProgramMethod context,
+ ProfileCollectionAdditions profileCollectionAdditions) {
// Create a fresh constructor on the given class that calls target. If there is a class in the
// hierarchy inbetween `clazz` and `target.holder`, which is also subject to class merging,
// then we must create a constructor that calls a constructor on that intermediate class,
@@ -510,7 +518,11 @@
if (ensureConstructorsOnClasses.containsKey(currentType)) {
target =
getOrCreateConstructor(
- currentClass, target, ensureConstructorsOnClasses, creationConsumer)
+ currentClass,
+ target,
+ ensureConstructorsOnClasses,
+ context,
+ profileCollectionAdditions)
.getReference();
break;
}
@@ -538,9 +550,7 @@
.setApiLevelForDefinition(
appView.apiLevelCompute().computeInitialMinApiLevel(appView.options()))
.build();
- ProgramMethod programMethod = method.asProgramMethod(clazz);
- creationConsumer.accept(programMethod);
- return programMethod;
+ return method.asProgramMethod(clazz);
}
private LirCode<Integer> createConstructorCode(DexMethod methodReference, DexMethod target) {
diff --git a/src/test/java/com/android/tools/r8/classmerging/horizontal/UndoConstructorInliningProfileFlagsTest.java b/src/test/java/com/android/tools/r8/classmerging/horizontal/UndoConstructorInliningProfileFlagsTest.java
index b74ec6c..68c15f2 100644
--- a/src/test/java/com/android/tools/r8/classmerging/horizontal/UndoConstructorInliningProfileFlagsTest.java
+++ b/src/test/java/com/android/tools/r8/classmerging/horizontal/UndoConstructorInliningProfileFlagsTest.java
@@ -10,7 +10,6 @@
import com.android.tools.r8.TestBase;
import com.android.tools.r8.TestParameters;
import com.android.tools.r8.TestParametersCollection;
-import com.android.tools.r8.errors.Unreachable;
import com.android.tools.r8.profile.art.ArtProfileMethodRuleInfoImpl;
import com.android.tools.r8.profile.art.model.ExternalArtProfile;
import com.android.tools.r8.references.Reference;
@@ -68,14 +67,11 @@
.assertContainsMethodRule(initializerSubject)
.inspectMethodRule(
initializerSubject,
- methodRuleInspector -> {
- try {
- methodRuleInspector.assertIsHot().assertIsPostStartup().assertIsStartup();
- throw new Unreachable();
- } catch (AssertionError e) {
- // Expected.
- }
- });
+ methodRuleInspector ->
+ methodRuleInspector
+ .assertIsHot()
+ .assertIsPostStartup()
+ .assertIsStartup());
});
}
