Fix concurrent method processing in D8 lambda desugaring
Fixes: 179976003
Change-Id: I397caf264df7dd6c49c3baed56b0c28e4251af33
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/D8MethodProcessor.java b/src/main/java/com/android/tools/r8/ir/conversion/D8MethodProcessor.java
index 38d3a32..bee8182 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/D8MethodProcessor.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/D8MethodProcessor.java
@@ -75,8 +75,9 @@
executorService));
}
- public void scheduleDesugaredMethodsForProcessing(Iterable<ProgramMethod> methods) {
+ public D8MethodProcessor scheduleDesugaredMethodsForProcessing(Iterable<ProgramMethod> methods) {
methods.forEach(this::scheduleDesugaredMethodForProcessing);
+ return this;
}
@Override
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 1cc9c0b..db7c135 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
@@ -371,11 +371,13 @@
}
private void synthesizeBridgesForNestBasedAccessesOnClasspath(
- MethodProcessor methodProcessor, ExecutorService executorService) throws ExecutionException {
+ D8MethodProcessor methodProcessor, ExecutorService executorService)
+ throws ExecutionException {
desugaring.withD8NestBasedAccessDesugaring(
d8NestBasedAccessDesugaring ->
d8NestBasedAccessDesugaring.synthesizeBridgesForNestBasedAccessesOnClasspath(
methodProcessor, executorService));
+ methodProcessor.awaitMethodProcessing();
}
private void reportNestDesugarDependencies() {
@@ -486,16 +488,18 @@
ClassConverter.create(appView, this, methodProcessor).convertClasses(executorService);
// The synthesis of accessibility bridges in lambda desugaring and nest based access desugaring
- // will schedule synthesized methods for processing.
+ // will schedule and await the processing of synthesized methods.
synthesizeBridgesForNestBasedAccessesOnClasspath(methodProcessor, executorService);
synthesizeAccessibilityBridgesForLambdaClasses(appView, classConverterResult, methodProcessor);
- methodProcessor.scheduleDesugaredMethodsForProcessing(
- IterableUtils.flatMap(
- classConverterResult.getSynthesizedLambdaClasses(),
- lambdaClass -> lambdaClass.getLambdaProgramClass().programMethods()));
+ methodProcessor
+ .scheduleDesugaredMethodsForProcessing(
+ IterableUtils.flatMap(
+ classConverterResult.getSynthesizedLambdaClasses(),
+ lambdaClass -> lambdaClass.getLambdaProgramClass().programMethods()))
+ .awaitMethodProcessing();
- // Await the processing of synthesized bridge methods.
- methodProcessor.awaitMethodProcessing();
+ // There should be no outstanding method processing.
+ methodProcessor.verifyNoPendingMethodProcessing();
}
void convertMethods(
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/LambdaClass.java b/src/main/java/com/android/tools/r8/ir/desugar/LambdaClass.java
index 676e127..6c6f19a 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/LambdaClass.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/LambdaClass.java
@@ -5,6 +5,7 @@
package com.android.tools.r8.ir.desugar;
import static com.android.tools.r8.ir.desugar.lambda.ForcefullyMovedLambdaMethodConsumer.emptyForcefullyMovedLambdaMethodConsumer;
+import static com.android.tools.r8.utils.ConsumerUtils.emptyConsumer;
import com.android.tools.r8.dex.Constants;
import com.android.tools.r8.errors.Unimplemented;
@@ -40,6 +41,7 @@
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
+import java.util.function.Consumer;
/**
* Represents lambda class generated for a lambda descriptor in context of lambda instantiation
@@ -458,20 +460,22 @@
// Ensure access of the referenced symbol(s).
abstract ProgramMethod ensureAccessibility(
- ForcefullyMovedLambdaMethodConsumer forcefullyMovedLambdaMethodConsumer);
+ ForcefullyMovedLambdaMethodConsumer forcefullyMovedLambdaMethodConsumer,
+ Consumer<ProgramMethod> needsProcessingConsumer);
public final void ensureAccessibilityIfNeeded() {
- ensureAccessibilityIfNeeded(emptyForcefullyMovedLambdaMethodConsumer());
+ ensureAccessibilityIfNeeded(emptyForcefullyMovedLambdaMethodConsumer(), emptyConsumer());
}
// Ensure access of the referenced symbol(s).
- public final ProgramMethod ensureAccessibilityIfNeeded(
- ForcefullyMovedLambdaMethodConsumer forcefullyMovedLambdaMethodConsumer) {
+ public final void ensureAccessibilityIfNeeded(
+ ForcefullyMovedLambdaMethodConsumer forcefullyMovedLambdaMethodConsumer,
+ Consumer<ProgramMethod> needsProcessingConsumer) {
if (!hasEnsuredAccessibility) {
- accessibilityBridge = ensureAccessibility(forcefullyMovedLambdaMethodConsumer);
+ accessibilityBridge =
+ ensureAccessibility(forcefullyMovedLambdaMethodConsumer, needsProcessingConsumer);
hasEnsuredAccessibility = true;
}
- return accessibilityBridge;
}
boolean isInterface() {
@@ -495,7 +499,8 @@
@Override
ProgramMethod ensureAccessibility(
- ForcefullyMovedLambdaMethodConsumer forcefullyMovedLambdaMethodConsumer) {
+ ForcefullyMovedLambdaMethodConsumer forcefullyMovedLambdaMethodConsumer,
+ Consumer<ProgramMethod> needsProcessingConsumer) {
return null;
}
}
@@ -512,7 +517,8 @@
@Override
ProgramMethod ensureAccessibility(
- ForcefullyMovedLambdaMethodConsumer forcefullyMovedLambdaMethodConsumer) {
+ ForcefullyMovedLambdaMethodConsumer forcefullyMovedLambdaMethodConsumer,
+ Consumer<ProgramMethod> needsProcessingConsumer) {
// We already found the static method to be called, just relax its accessibility.
MethodAccessFlags flags = target.getAccessFlags();
flags.unsetPrivate();
@@ -533,7 +539,8 @@
@Override
ProgramMethod ensureAccessibility(
- ForcefullyMovedLambdaMethodConsumer forcefullyMovedLambdaMethodConsumer) {
+ ForcefullyMovedLambdaMethodConsumer forcefullyMovedLambdaMethodConsumer,
+ Consumer<ProgramMethod> needsProcessingConsumer) {
// For all instantiation points for which the compiler creates lambda$
// methods, it creates these methods in the same class/interface.
DexMethod implMethod = descriptor.implHandle.asMethod();
@@ -570,6 +577,9 @@
return newMethod;
});
if (replacement != null) {
+ // Since we've copied the code object from an existing method, the code should already be
+ // processed, and thus we don't need to schedule it for processing in D8.
+ assert replacement.getCode().isDexCode();
return new ProgramMethod(implMethodHolder, replacement);
}
// The method might already have been moved by another invoke-dynamic targeting it.
@@ -592,7 +602,8 @@
@Override
ProgramMethod ensureAccessibility(
- ForcefullyMovedLambdaMethodConsumer forcefullyMovedLambdaMethodConsumer) {
+ ForcefullyMovedLambdaMethodConsumer forcefullyMovedLambdaMethodConsumer,
+ Consumer<ProgramMethod> needsProcessingConsumer) {
return null;
}
}
@@ -606,20 +617,14 @@
@Override
ProgramMethod ensureAccessibility(
- ForcefullyMovedLambdaMethodConsumer forcefullyMovedLambdaMethodConsumer) {
+ ForcefullyMovedLambdaMethodConsumer forcefullyMovedLambdaMethodConsumer,
+ Consumer<ProgramMethod> needsProcessingConsumer) {
// When compiling with whole program optimization, check that we are not inplace modifying.
// For all instantiation points for which the compiler creates lambda$
// methods, it creates these methods in the same class/interface.
DexMethod implMethod = descriptor.implHandle.asMethod();
DexProgramClass implMethodHolder = appView.definitionFor(implMethod.holder).asProgramClass();
- return modifyLambdaImplementationMethod(
- implMethod, implMethodHolder, forcefullyMovedLambdaMethodConsumer);
- }
- private ProgramMethod modifyLambdaImplementationMethod(
- DexMethod implMethod,
- DexProgramClass implMethodHolder,
- ForcefullyMovedLambdaMethodConsumer forcefullyMovedLambdaMethodConsumer) {
DexEncodedMethod replacement =
implMethodHolder
.getMethodCollection()
@@ -647,6 +652,9 @@
return newMethod;
});
if (replacement != null) {
+ // Since we've copied the code object from an existing method, the code should already be
+ // processed, and thus we don't need to schedule it for processing in D8.
+ assert replacement.getCode().isDexCode();
return new ProgramMethod(implMethodHolder, replacement);
}
// The method might already have been moved by another invoke-dynamic targeting it.
@@ -668,7 +676,8 @@
@Override
ProgramMethod ensureAccessibility(
- ForcefullyMovedLambdaMethodConsumer forcefullyMovedLambdaMethodConsumer) {
+ ForcefullyMovedLambdaMethodConsumer forcefullyMovedLambdaMethodConsumer,
+ Consumer<ProgramMethod> needsProcessingConsumer) {
// Create a static accessor with proper accessibility.
DexProgramClass accessorClass = appView.definitionForProgramType(callTarget.holder);
assert accessorClass != null;
@@ -684,18 +693,20 @@
// Always make the method public to provide access when r8 minification is allowed to move
// the lambda class accessing this method to another package (-allowaccessmodification).
- DexEncodedMethod accessorEncodedMethod =
- new DexEncodedMethod(
- callTarget,
- MethodAccessFlags.createPublicStaticSynthetic(),
- MethodTypeSignature.noSignature(),
- DexAnnotationSet.empty(),
- ParameterAnnotationsList.empty(),
- AccessorMethodSourceCode.build(LambdaClass.this, callTarget),
- true);
-
- accessorClass.addDirectMethod(accessorEncodedMethod);
- return new ProgramMethod(accessorClass, accessorEncodedMethod);
+ ProgramMethod accessorMethod =
+ new ProgramMethod(
+ accessorClass,
+ new DexEncodedMethod(
+ callTarget,
+ MethodAccessFlags.createPublicStaticSynthetic(),
+ MethodTypeSignature.noSignature(),
+ DexAnnotationSet.empty(),
+ ParameterAnnotationsList.empty(),
+ AccessorMethodSourceCode.build(LambdaClass.this, callTarget),
+ true));
+ accessorClass.addDirectMethod(accessorMethod.getDefinition());
+ needsProcessingConsumer.accept(accessorMethod);
+ return accessorMethod;
}
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/lambda/D8LambdaDesugaring.java b/src/main/java/com/android/tools/r8/ir/desugar/lambda/D8LambdaDesugaring.java
index 26ba8bb..2d89fce 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/lambda/D8LambdaDesugaring.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/lambda/D8LambdaDesugaring.java
@@ -8,32 +8,33 @@
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexProgramClass;
import com.android.tools.r8.graph.EnclosingMethodAttribute;
-import com.android.tools.r8.graph.ProgramMethod;
import com.android.tools.r8.ir.conversion.ClassConverterResult;
import com.android.tools.r8.ir.conversion.D8MethodProcessor;
import com.android.tools.r8.utils.collections.ProgramMethodSet;
import java.util.IdentityHashMap;
import java.util.Map;
+import java.util.concurrent.ExecutionException;
public class D8LambdaDesugaring {
public static void synthesizeAccessibilityBridgesForLambdaClasses(
AppView<?> appView,
ClassConverterResult classConverterResult,
- D8MethodProcessor methodProcessor) {
+ D8MethodProcessor methodProcessor)
+ throws ExecutionException {
Map<DexMethod, DexMethod> forcefullyMovedLambdaMethods = new IdentityHashMap<>();
ProgramMethodSet seenAccessibilityBridges = ProgramMethodSet.createConcurrent();
classConverterResult.forEachSynthesizedLambdaClassWithDeterministicOrdering(
lambdaClass -> {
- ProgramMethod accessibilityBridge =
- lambdaClass.target.ensureAccessibilityIfNeeded(forcefullyMovedLambdaMethods::put);
- if (accessibilityBridge != null
- // TODO(b/179976003): This check should be redundant with the seen-set (?).
- && !accessibilityBridge.getDefinition().getCode().isDexCode()
- && seenAccessibilityBridges.add(accessibilityBridge)) {
- methodProcessor.scheduleDesugaredMethodForProcessing(accessibilityBridge);
- }
+ // Collect the accessibility bridges that require processing. Note that we cannot schedule
+ // the methods for processing directly here, since that would lead to concurrent IR
+ // processing meanwhile we update the program (insert bridges on existing classes).
+ lambdaClass.target.ensureAccessibilityIfNeeded(
+ forcefullyMovedLambdaMethods::put, seenAccessibilityBridges::add);
});
+ methodProcessor
+ .scheduleDesugaredMethodsForProcessing(seenAccessibilityBridges)
+ .awaitMethodProcessing();
rewriteEnclosingMethodAttributes(appView, forcefullyMovedLambdaMethods);
}
diff --git a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
index a7fbd50..2f7edb2 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -255,9 +255,6 @@
*/
private final SetWithReportedReason<DexProgramClass> liveTypes = new SetWithReportedReason<>();
- private final Map<DexMethod, List<DexProgramClass>> liveTypesByEnclosingMethod =
- new IdentityHashMap<>();
-
/** Set of classes whose initializer may execute. */
private final SetWithReportedReason<DexProgramClass> initializedClasses =
new SetWithReportedReason<>();
@@ -1762,9 +1759,6 @@
: this::ignoreMissingClass;
if (enclosingMethod != null) {
recordMethodReference(enclosingMethod, clazz, missingClassConsumer);
- liveTypesByEnclosingMethod
- .computeIfAbsent(enclosingMethod, ignore -> new ArrayList<>())
- .add(clazz);
} else {
DexType enclosingClass = enclosingMethodAttribute.getEnclosingClass();
recordTypeReference(enclosingClass, clazz, missingClassConsumer);
