Reduce further Nest Desugaring memory footprint
- Remove deferredBridgeToAdd Map which is now redundant
- clean-up: refactor extractNest
- clean-up: remove duplicate lookupGet/PutFieldForMethod
which are now redundant
Bug:132411997
Change-Id: I0c65bcdb2c4c5e875b2eae91121f83dc12290139
diff --git a/src/main/java/com/android/tools/r8/graph/GraphLense.java b/src/main/java/com/android/tools/r8/graph/GraphLense.java
index fc24fd2..16dab7e 100644
--- a/src/main/java/com/android/tools/r8/graph/GraphLense.java
+++ b/src/main/java/com/android/tools/r8/graph/GraphLense.java
@@ -458,19 +458,11 @@
public abstract DexField lookupField(DexField field);
- public DexMethod lookupStaticGetFieldForMethod(DexField field, DexMethod context) {
+ public DexMethod lookupGetFieldForMethod(DexField field, DexMethod context) {
return null;
}
- public DexMethod lookupStaticPutFieldForMethod(DexField field, DexMethod context) {
- return null;
- }
-
- public DexMethod lookupInstanceGetFieldForMethod(DexField field, DexMethod context) {
- return null;
- }
-
- public DexMethod lookupInstancePutFieldForMethod(DexField field, DexMethod context) {
+ public DexMethod lookupPutFieldForMethod(DexField field, DexMethod context) {
return null;
}
@@ -971,23 +963,13 @@
}
@Override
- public DexMethod lookupStaticGetFieldForMethod(DexField field, DexMethod context) {
- return previousLense.lookupStaticGetFieldForMethod(field, context);
+ public DexMethod lookupGetFieldForMethod(DexField field, DexMethod context) {
+ return previousLense.lookupGetFieldForMethod(field, context);
}
@Override
- public DexMethod lookupStaticPutFieldForMethod(DexField field, DexMethod context) {
- return previousLense.lookupStaticPutFieldForMethod(field, context);
- }
-
- @Override
- public DexMethod lookupInstanceGetFieldForMethod(DexField field, DexMethod context) {
- return previousLense.lookupInstanceGetFieldForMethod(field, context);
- }
-
- @Override
- public DexMethod lookupInstancePutFieldForMethod(DexField field, DexMethod context) {
- return previousLense.lookupInstancePutFieldForMethod(field, context);
+ public DexMethod lookupPutFieldForMethod(DexField field, DexMethod context) {
+ return previousLense.lookupPutFieldForMethod(field, context);
}
/**
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 d5c65ec..2559faa 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
@@ -786,7 +786,15 @@
public void optimizeSynthesizedMethodsConcurrently(
Collection<DexEncodedMethod> methods, ExecutorService executorService)
throws ExecutionException {
- List<Future<?>> futures = new ArrayList<>();
+ List<Future<?>> futures = new ArrayList<>(methods.size());
+ optimizeSynthesizedMethodsConcurrently(methods, executorService, futures);
+ ThreadUtils.awaitFutures(futures);
+ }
+
+ public void optimizeSynthesizedMethodsConcurrently(
+ Collection<DexEncodedMethod> methods,
+ ExecutorService executorService,
+ List<Future<?>> futures) {
for (DexEncodedMethod method : methods) {
futures.add(
executorService.submit(
@@ -800,7 +808,6 @@
return null; // we want a Callable not a Runnable to be able to throw
}));
}
- ThreadUtils.awaitFutures(futures);
}
private String logCode(InternalOptions options, DexEncodedMethod method) {
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/LensCodeRewriter.java b/src/main/java/com/android/tools/r8/ir/conversion/LensCodeRewriter.java
index cb56a09..959e9b7 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/LensCodeRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/LensCodeRewriter.java
@@ -278,7 +278,7 @@
DexField field = instanceGet.getField();
DexField actualField = graphLense.lookupField(field);
DexMethod replacementMethod =
- graphLense.lookupInstanceGetFieldForMethod(actualField, method.method);
+ graphLense.lookupGetFieldForMethod(actualField, method.method);
if (replacementMethod != null) {
iterator.replaceCurrentInstruction(
new InvokeStatic(replacementMethod, current.outValue(), current.inValues()));
@@ -295,7 +295,7 @@
DexField field = instancePut.getField();
DexField actualField = graphLense.lookupField(field);
DexMethod replacementMethod =
- graphLense.lookupInstancePutFieldForMethod(actualField, method.method);
+ graphLense.lookupPutFieldForMethod(actualField, method.method);
if (replacementMethod != null) {
iterator.replaceCurrentInstruction(
new InvokeStatic(replacementMethod, current.outValue(), current.inValues()));
@@ -309,7 +309,7 @@
DexField field = staticGet.getField();
DexField actualField = graphLense.lookupField(field);
DexMethod replacementMethod =
- graphLense.lookupStaticGetFieldForMethod(actualField, method.method);
+ graphLense.lookupGetFieldForMethod(actualField, method.method);
if (replacementMethod != null) {
iterator.replaceCurrentInstruction(
new InvokeStatic(replacementMethod, current.outValue(), current.inValues()));
@@ -323,7 +323,7 @@
DexField field = staticPut.getField();
DexField actualField = graphLense.lookupField(field);
DexMethod replacementMethod =
- graphLense.lookupStaticPutFieldForMethod(actualField, method.method);
+ graphLense.lookupPutFieldForMethod(actualField, method.method);
if (replacementMethod != null) {
iterator.replaceCurrentInstruction(
new InvokeStatic(replacementMethod, current.outValue(), current.inValues()));
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/D8NestBasedAccessDesugaring.java b/src/main/java/com/android/tools/r8/ir/desugar/D8NestBasedAccessDesugaring.java
index 6f7bd72..3b0a46b 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/D8NestBasedAccessDesugaring.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/D8NestBasedAccessDesugaring.java
@@ -43,8 +43,7 @@
private List<DexType> getNestFor(DexClass clazz) {
DexType nestHostType = clazz.getNestHost();
- DexClass nestHost = clazz.isNestHost() ? clazz : appView.definitionFor(nestHostType);
- return metNests.computeIfAbsent(nestHostType, host -> extractNest(nestHost, clazz));
+ return metNests.computeIfAbsent(nestHostType, host -> extractNest(clazz));
}
public void rewriteNestBasedAccesses(
@@ -121,8 +120,7 @@
processNestsConcurrently(metNests, executorService);
addDeferredBridges();
synthetizeNestConstructor(builder);
- converter.optimizeSynthesizedMethodsConcurrently(
- deferredBridgesToAdd.keySet(), executorService);
+ optimizeDeferredBridgesConcurrently(executorService, converter);
}
// In D8, programClass are processed on the fly so they do not need to be processed again here.
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/NestBasedAccessDesugaring.java b/src/main/java/com/android/tools/r8/ir/desugar/NestBasedAccessDesugaring.java
index 0265d20..865049c 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/NestBasedAccessDesugaring.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/NestBasedAccessDesugaring.java
@@ -18,14 +18,18 @@
import com.android.tools.r8.graph.DexTypeList;
import com.android.tools.r8.graph.NestMemberClassAttribute;
import com.android.tools.r8.graph.UseRegistry;
+import com.android.tools.r8.ir.conversion.IRConverter;
import com.android.tools.r8.origin.SynthesizedOrigin;
import com.android.tools.r8.utils.BooleanUtils;
+import com.android.tools.r8.utils.ThreadUtils;
import java.util.ArrayList;
+import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Future;
@@ -48,14 +52,11 @@
private static final String FULL_NEST_CONTRUCTOR_NAME = "L" + NEST_CONSTRUCTOR_NAME + ";";
protected final AppView<?> appView;
- // Following maps are there to avoid creating the bridges multiple times.
- private final Map<DexEncodedMethod, DexMethod> bridges = new ConcurrentHashMap<>();
- private final Map<DexEncodedField, DexMethod> getFieldBridges = new ConcurrentHashMap<>();
- private final Map<DexEncodedField, DexMethod> putFieldBridges = new ConcurrentHashMap<>();
- // The following map records the bridges to add in the program.
- // It may differ from the values of the previous maps
- // if some classes are on the classpath and not the program path.
- final Map<DexEncodedMethod, DexProgramClass> deferredBridgesToAdd = new ConcurrentHashMap<>();
+ // Following maps are there to avoid creating the bridges multiple times
+ // and remember the bridges to add once the nests are processed.
+ private final Map<DexEncodedMethod, DexEncodedMethod> bridges = new ConcurrentHashMap<>();
+ private final Map<DexEncodedField, DexEncodedMethod> getFieldBridges = new ConcurrentHashMap<>();
+ private final Map<DexEncodedField, DexEncodedMethod> putFieldBridges = new ConcurrentHashMap<>();
// Common single empty class for nest based private constructors
private final DexProgramClass nestConstructor;
private boolean nestConstructorUsed = false;
@@ -70,8 +71,9 @@
}
// Extract the list of types in the programClass' nest, of host hostClass
- List<DexType> extractNest(DexClass hostClass, DexClass clazz) {
+ List<DexType> extractNest(DexClass clazz) {
assert clazz != null;
+ DexClass hostClass = clazz.isNestHost() ? clazz : appView.definitionFor(clazz.getNestHost());
if (hostClass == null) {
throw abortCompilationDueToMissingNestHost(clazz);
}
@@ -112,11 +114,31 @@
protected abstract boolean shouldProcessClassInNest(DexClass clazz, List<DexType> nest);
void addDeferredBridges() {
- for (Map.Entry<DexEncodedMethod, DexProgramClass> entry : deferredBridgesToAdd.entrySet()) {
- entry.getValue().addMethod(entry.getKey());
+ addDeferredBridges(bridges.values());
+ addDeferredBridges(getFieldBridges.values());
+ addDeferredBridges(putFieldBridges.values());
+ }
+
+ private void addDeferredBridges(Collection<DexEncodedMethod> bridges) {
+ for (DexEncodedMethod bridge : bridges) {
+ DexClass holder = appView.definitionFor(bridge.method.holder);
+ assert holder != null && holder.isProgramClass();
+ holder.asProgramClass().addMethod(bridge);
}
}
+ void optimizeDeferredBridgesConcurrently(ExecutorService executorService, IRConverter converter)
+ throws ExecutionException {
+ List<Future<?>> futures =
+ new ArrayList<>(bridges.size() + getFieldBridges.size() + putFieldBridges.size());
+ converter.optimizeSynthesizedMethodsConcurrently(bridges.values(), executorService, futures);
+ converter.optimizeSynthesizedMethodsConcurrently(
+ getFieldBridges.values(), executorService, futures);
+ converter.optimizeSynthesizedMethodsConcurrently(
+ putFieldBridges.values(), executorService, futures);
+ ThreadUtils.awaitFutures(futures);
+ }
+
private RuntimeException abortCompilationDueToIncompleteNest(List<DexType> nest) {
List<String> programClassesFromNest = new ArrayList<>();
List<String> unavailableClasses = new ArrayList<>();
@@ -288,8 +310,7 @@
return true;
}
assert holder.isLibraryClass();
- DexClass host = appView.definitionFor(holder.getNestHost());
- throw abortCompilationDueToIncompleteNest(extractNest(host, holder));
+ throw abortCompilationDueToIncompleteNest(extractNest(holder));
}
DexMethod ensureFieldAccessBridge(DexEncodedField field, boolean isGet) {
@@ -299,17 +320,15 @@
if (holderRequiresBridge(holder)) {
return bridgeMethod;
}
- // The map is used to avoid creating multiple times the bridge.
- Map<DexEncodedField, DexMethod> fieldMap = isGet ? getFieldBridges : putFieldBridges;
- return fieldMap.computeIfAbsent(
+ // The map is used to avoid creating multiple times the bridge
+ // and remembers the bridges to add.
+ Map<DexEncodedField, DexEncodedMethod> fieldMap = isGet ? getFieldBridges : putFieldBridges;
+ fieldMap.computeIfAbsent(
field,
- k -> {
- DexEncodedMethod localBridge =
- DexEncodedMethod.createFieldAccessorBridge(
- new DexFieldWithAccess(field, isGet), holder, bridgeMethod);
- deferredBridgesToAdd.put(localBridge, holder.asProgramClass());
- return bridgeMethod;
- });
+ k ->
+ DexEncodedMethod.createFieldAccessorBridge(
+ new DexFieldWithAccess(field, isGet), holder, bridgeMethod));
+ return bridgeMethod;
}
DexMethod ensureInvokeBridge(DexEncodedMethod method) {
@@ -326,17 +345,15 @@
if (holderRequiresBridge(holder)) {
return bridgeMethod;
}
- // The map is used to avoid creating multiple times the bridge.
- return bridges.computeIfAbsent(
+ // The map is used to avoid creating multiple times the bridge
+ // and remembers the bridges to add.
+ bridges.computeIfAbsent(
method,
- k -> {
- DexEncodedMethod localBridge =
- method.isInstanceInitializer()
- ? method.toInitializerForwardingBridge(holder, bridgeMethod)
- : method.toStaticForwardingBridge(holder, computeMethodBridge(method, appView));
- deferredBridgesToAdd.put(localBridge, holder.asProgramClass());
- return bridgeMethod;
- });
+ k ->
+ method.isInstanceInitializer()
+ ? method.toInitializerForwardingBridge(holder, bridgeMethod)
+ : method.toStaticForwardingBridge(holder, computeMethodBridge(method, appView)));
+ return bridgeMethod;
}
protected class NestBasedAccessDesugaringUseRegistry extends UseRegistry {
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/NestedPrivateMethodLense.java b/src/main/java/com/android/tools/r8/ir/desugar/NestedPrivateMethodLense.java
index 19c4f33..a37b8ac 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/NestedPrivateMethodLense.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/NestedPrivateMethodLense.java
@@ -59,22 +59,12 @@
}
@Override
- public DexMethod lookupStaticGetFieldForMethod(DexField field, DexMethod context) {
+ public DexMethod lookupGetFieldForMethod(DexField field, DexMethod context) {
return lookupFieldForMethod(field, context, true);
}
@Override
- public DexMethod lookupStaticPutFieldForMethod(DexField field, DexMethod context) {
- return lookupFieldForMethod(field, context, false);
- }
-
- @Override
- public DexMethod lookupInstanceGetFieldForMethod(DexField field, DexMethod context) {
- return lookupFieldForMethod(field, context, true);
- }
-
- @Override
- public DexMethod lookupInstancePutFieldForMethod(DexField field, DexMethod context) {
+ public DexMethod lookupPutFieldForMethod(DexField field, DexMethod context) {
return lookupFieldForMethod(field, context, false);
}
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/R8NestBasedAccessDesugaring.java b/src/main/java/com/android/tools/r8/ir/desugar/R8NestBasedAccessDesugaring.java
index 1c96f03..e3e14a3 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/R8NestBasedAccessDesugaring.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/R8NestBasedAccessDesugaring.java
@@ -50,8 +50,7 @@
if (clazz.isInANest()) {
DexType hostType = clazz.getNestHost();
if (!nestMap.containsKey(hostType)) {
- DexClass host = clazz.isNestHost() ? clazz : appView.definitionFor(clazz.getNestHost());
- List<DexType> nest = extractNest(host, clazz);
+ List<DexType> nest = extractNest(clazz);
nestMap.put(hostType, nest);
futures.add(asyncProcessNest(nest, executorService));
}
