Support for constructor shrinking when compiling to api >= L
Bug: b/173751869
Change-Id: Iefbeec728f551b7b48f7a558ec39f636d113ee9a
diff --git a/src/main/java/com/android/tools/r8/R8.java b/src/main/java/com/android/tools/r8/R8.java
index d4e7f3a..d57d450 100644
--- a/src/main/java/com/android/tools/r8/R8.java
+++ b/src/main/java/com/android/tools/r8/R8.java
@@ -68,6 +68,7 @@
import com.android.tools.r8.naming.signature.GenericSignatureRewriter;
import com.android.tools.r8.optimize.AccessModifier;
import com.android.tools.r8.optimize.MemberRebindingAnalysis;
+import com.android.tools.r8.optimize.MemberRebindingIdentityLens;
import com.android.tools.r8.optimize.MemberRebindingIdentityLensFactory;
import com.android.tools.r8.optimize.RedundantBridgeRemover;
import com.android.tools.r8.optimize.bridgehoisting.BridgeHoisting;
@@ -460,7 +461,7 @@
// We can now remove redundant bridges. Note that we do not need to update the
// invoke-targets here, as the existing invokes will simply dispatch to the now
// visible super-method. MemberRebinding, if run, will then dispatch it correctly.
- new RedundantBridgeRemover(appView.withLiveness()).run(executorService);
+ new RedundantBridgeRemover(appView.withLiveness()).run(null, executorService);
}
}
@@ -688,10 +689,17 @@
}
}
+ // Insert a member rebinding oracle in the graph to ensure that all subsequent rewritings of
+ // the application has an applied oracle for looking up non-rebound references.
+ MemberRebindingIdentityLens memberRebindingIdentityLens =
+ MemberRebindingIdentityLensFactory.create(appView, executorService);
+ appView.setGraphLens(memberRebindingIdentityLens);
+
// Remove redundant bridges that have been inserted for member rebinding.
// This can only be done if we have AppInfoWithLiveness.
if (appView.appInfo().hasLiveness()) {
- new RedundantBridgeRemover(appView.withLiveness()).run(executorService);
+ new RedundantBridgeRemover(appView.withLiveness())
+ .run(memberRebindingIdentityLens, executorService);
} else {
// If we don't have AppInfoWithLiveness here, it must be because we are not shrinking. When
// we are not shrinking, we can't move visibility bridges. In principle, though, it would be
@@ -700,10 +708,6 @@
assert !options.isShrinking();
}
- // Insert a member rebinding oracle in the graph to ensure that all subsequent rewritings of
- // the application has an applied oracle for looking up non-rebound references.
- appView.setGraphLens(MemberRebindingIdentityLensFactory.create(appView, executorService));
-
if (appView.appInfo().hasLiveness()) {
SyntheticFinalization.finalizeWithLiveness(appView.withLiveness(), executorService, timing);
} else {
diff --git a/src/main/java/com/android/tools/r8/graph/AppInfoWithClassHierarchy.java b/src/main/java/com/android/tools/r8/graph/AppInfoWithClassHierarchy.java
index b891c89..42f5d96 100644
--- a/src/main/java/com/android/tools/r8/graph/AppInfoWithClassHierarchy.java
+++ b/src/main/java/com/android/tools/r8/graph/AppInfoWithClassHierarchy.java
@@ -688,6 +688,13 @@
return resolveMethodOnLegacy(invokedMethod.getHolderType(), invokedMethod, isInterface);
}
+ public MethodResolutionResult resolveMethodOn(DexClass clazz, DexMethod method) {
+ assert checkIfObsolete();
+ return clazz.isInterface()
+ ? resolveMethodOnInterface(clazz, method)
+ : resolveMethodOnClass(clazz, method);
+ }
+
public MethodResolutionResult resolveMethodOnLegacy(DexClass clazz, DexMethod method) {
assert checkIfObsolete();
return clazz.isInterface()
diff --git a/src/main/java/com/android/tools/r8/graph/DefaultInstanceInitializerCode.java b/src/main/java/com/android/tools/r8/graph/DefaultInstanceInitializerCode.java
index 5569e34..425c5d5 100644
--- a/src/main/java/com/android/tools/r8/graph/DefaultInstanceInitializerCode.java
+++ b/src/main/java/com/android/tools/r8/graph/DefaultInstanceInitializerCode.java
@@ -16,6 +16,7 @@
import com.android.tools.r8.dex.code.DexReturnVoid;
import com.android.tools.r8.graph.DexCode.Try;
import com.android.tools.r8.graph.DexCode.TryHandler;
+import com.android.tools.r8.graph.GraphLens.MethodLookupResult;
import com.android.tools.r8.graph.proto.RewrittenPrototypeDescription;
import com.android.tools.r8.ir.code.IRCode;
import com.android.tools.r8.ir.code.NumberGenerator;
@@ -178,8 +179,10 @@
IndexedItemCollection indexedItems,
ProgramMethod context,
LensCodeRewriterUtils rewriter) {
- getParentConstructor(context, rewriter.dexItemFactory())
- .collectIndexedItems(appView, indexedItems);
+ DexMethod parentConstructor = getParentConstructor(context, rewriter.dexItemFactory());
+ MethodLookupResult lookupResult =
+ appView.graphLens().lookupInvokeDirect(parentConstructor, context);
+ lookupResult.getReference().collectIndexedItems(appView, indexedItems);
}
@Override
@@ -370,7 +373,9 @@
GraphLens graphLens,
LensCodeRewriterUtils lensCodeRewriter,
ObjectToOffsetMapping mapping) {
- new DexInvokeDirect(1, getParentConstructor(context, mapping.dexItemFactory()), 0, 0, 0, 0, 0)
+ DexMethod parentConstructor = getParentConstructor(context, mapping.dexItemFactory());
+ MethodLookupResult lookupResult = graphLens.lookupInvokeDirect(parentConstructor, context);
+ new DexInvokeDirect(1, lookupResult.getReference(), 0, 0, 0, 0, 0)
.write(shortBuffer, context, graphLens, mapping, lensCodeRewriter);
new DexReturnVoid().write(shortBuffer, context, graphLens, mapping, lensCodeRewriter);
}
diff --git a/src/main/java/com/android/tools/r8/graph/DexClass.java b/src/main/java/com/android/tools/r8/graph/DexClass.java
index 3069a60..0564b9c 100644
--- a/src/main/java/com/android/tools/r8/graph/DexClass.java
+++ b/src/main/java/com/android/tools/r8/graph/DexClass.java
@@ -893,7 +893,7 @@
}
public final boolean classInitializationMayHaveSideEffectsInContext(
- AppView<?> appView, ProgramDefinition context) {
+ AppView<?> appView, Definition context) {
// Types that are a super type of the current context are guaranteed to be initialized already.
return classInitializationMayHaveSideEffects(
appView, type -> appView.isSubtype(context.getContextType(), type).isTrue());
diff --git a/src/main/java/com/android/tools/r8/optimize/MemberRebindingIdentityLens.java b/src/main/java/com/android/tools/r8/optimize/MemberRebindingIdentityLens.java
index e5b23f5..0cc861f 100644
--- a/src/main/java/com/android/tools/r8/optimize/MemberRebindingIdentityLens.java
+++ b/src/main/java/com/android/tools/r8/optimize/MemberRebindingIdentityLens.java
@@ -49,6 +49,11 @@
return new Builder(appView, previousLens);
}
+ public void addNonReboundMethodReference(
+ DexMethod nonReboundMethodReference, DexMethod reboundMethodReference) {
+ nonReboundMethodReferenceToDefinitionMap.put(nonReboundMethodReference, reboundMethodReference);
+ }
+
@Override
public boolean hasCodeRewritings() {
return false;
diff --git a/src/main/java/com/android/tools/r8/optimize/RedundantBridgeRemover.java b/src/main/java/com/android/tools/r8/optimize/RedundantBridgeRemover.java
index 6c2af55..c592a7d 100644
--- a/src/main/java/com/android/tools/r8/optimize/RedundantBridgeRemover.java
+++ b/src/main/java/com/android/tools/r8/optimize/RedundantBridgeRemover.java
@@ -6,6 +6,7 @@
import static com.android.tools.r8.utils.ThreadUtils.processItems;
import com.android.tools.r8.graph.AppView;
+import com.android.tools.r8.graph.DexClassAndMethod;
import com.android.tools.r8.graph.DexEncodedMethod;
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexProgramClass;
@@ -13,6 +14,7 @@
import com.android.tools.r8.graph.PrunedItems;
import com.android.tools.r8.ir.optimize.info.bridge.BridgeInfo;
import com.android.tools.r8.optimize.InvokeSingleTargetExtractor.InvokeKind;
+import com.android.tools.r8.optimize.redundantbridgeremoval.RedundantBridgeRemovalLens;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.android.tools.r8.utils.collections.ProgramMethodSet;
import java.util.Map;
@@ -28,17 +30,17 @@
this.appView = appView;
}
- private boolean isRedundantBridge(ProgramMethod method) {
+ private DexClassAndMethod getTargetForRedundantBridge(ProgramMethod method) {
// Clean-up the predicate check.
if (appView.appInfo().isPinned(method.getReference())) {
- return false;
+ return null;
}
DexEncodedMethod definition = method.getDefinition();
// TODO(b/197490164): Remove if method is abstract.
BridgeInfo bridgeInfo = definition.getOptimizationInfo().getBridgeInfo();
boolean isBridge = definition.isBridge() || bridgeInfo != null;
if (!isBridge || definition.isAbstract()) {
- return false;
+ return null;
}
InvokeSingleTargetExtractor targetExtractor = new InvokeSingleTargetExtractor(appView, method);
method.registerCodeReferences(targetExtractor);
@@ -46,31 +48,28 @@
// javac-generated visibility forward bridge method has same descriptor (name, signature and
// return type).
if (target == null || !target.match(method.getReference())) {
- return false;
+ return null;
}
if (!isTargetingSuperMethod(method, targetExtractor.getKind(), target)) {
- return false;
+ return null;
}
// This is a visibility forward, so check for the direct target.
- ProgramMethod targetMethod =
- appView
- .appInfo()
- .unsafeResolveMethodDueToDexFormatLegacy(target)
- .getResolvedProgramMethod();
+ DexClassAndMethod targetMethod =
+ appView.appInfo().unsafeResolveMethodDueToDexFormatLegacy(target).getResolutionPair();
if (targetMethod == null) {
- return false;
+ return null;
}
if (method.getAccessFlags().isPublic()) {
if (!targetMethod.getAccessFlags().isPublic()) {
- return false;
+ return null;
}
} else {
if (targetMethod.getAccessFlags().isProtected()
&& !targetMethod.getHolderType().isSamePackage(method.getHolderType())) {
- return false;
+ return null;
}
if (targetMethod.getAccessFlags().isPrivate()) {
- return false;
+ return null;
}
}
if (definition.isStatic()
@@ -78,9 +77,9 @@
&& method
.getHolder()
.classInitializationMayHaveSideEffectsInContext(appView, targetMethod)) {
- return false;
+ return null;
}
- return true;
+ return targetMethod;
}
private boolean isTargetingSuperMethod(ProgramMethod method, InvokeKind kind, DexMethod target) {
@@ -105,15 +104,44 @@
return false;
}
- public void run(ExecutorService executorService) throws ExecutionException {
+ public void run(
+ MemberRebindingIdentityLens memberRebindingIdentityLens, ExecutorService executorService)
+ throws ExecutionException {
+ assert memberRebindingIdentityLens == null
+ || memberRebindingIdentityLens == appView.graphLens();
+
// Collect all redundant bridges to remove.
+ RedundantBridgeRemovalLens.Builder lensBuilder = new RedundantBridgeRemovalLens.Builder();
Map<DexProgramClass, ProgramMethodSet> bridgesToRemove =
- computeBridgesToRemove(executorService);
+ computeBridgesToRemove(lensBuilder, executorService);
+ if (bridgesToRemove.isEmpty()) {
+ return;
+ }
+
pruneApp(bridgesToRemove, executorService);
+
+ if (!lensBuilder.isEmpty()) {
+ appView.setGraphLens(lensBuilder.build(appView));
+ }
+
+ if (memberRebindingIdentityLens != null) {
+ for (ProgramMethodSet bridgesToRemoveFromClass : bridgesToRemove.values()) {
+ for (ProgramMethod bridgeToRemove : bridgesToRemoveFromClass) {
+ DexClassAndMethod resolvedMethod =
+ appView
+ .appInfo()
+ .resolveMethodOn(bridgeToRemove.getHolder(), bridgeToRemove.getReference())
+ .getResolutionPair();
+ memberRebindingIdentityLens.addNonReboundMethodReference(
+ bridgeToRemove.getReference(), resolvedMethod.getReference());
+ }
+ }
+ }
}
private Map<DexProgramClass, ProgramMethodSet> computeBridgesToRemove(
- ExecutorService executorService) throws ExecutionException {
+ RedundantBridgeRemovalLens.Builder lensBuilder, ExecutorService executorService)
+ throws ExecutionException {
Map<DexProgramClass, ProgramMethodSet> bridgesToRemove = new ConcurrentHashMap<>();
processItems(
appView.appInfo().classes(),
@@ -121,8 +149,20 @@
ProgramMethodSet bridgesToRemoveForClass = ProgramMethodSet.create();
clazz.forEachProgramMethod(
method -> {
- if (isRedundantBridge(method)) {
+ DexClassAndMethod target = getTargetForRedundantBridge(method);
+ if (target != null) {
+ // Record that the redundant bridge should be removed.
bridgesToRemoveForClass.add(method);
+
+ // Rewrite invokes to the bridge to the target if it is accessible.
+ // TODO(b/173751869): Consider enabling this for constructors as well.
+ // TODO(b/245882297): Refine these visibility checks so that we also rewrite when
+ // the target is not public, but still accessible to call sites.
+ if (!method.getDefinition().isInstanceInitializer()
+ && target.getAccessFlags().isPublic()
+ && target.getHolder().isPublic()) {
+ lensBuilder.map(method, target);
+ }
}
});
if (!bridgesToRemoveForClass.isEmpty()) {
diff --git a/src/main/java/com/android/tools/r8/optimize/proto/ProtoNormalizerGraphLens.java b/src/main/java/com/android/tools/r8/optimize/proto/ProtoNormalizerGraphLens.java
index 1ce0703..6f7f61f 100644
--- a/src/main/java/com/android/tools/r8/optimize/proto/ProtoNormalizerGraphLens.java
+++ b/src/main/java/com/android/tools/r8/optimize/proto/ProtoNormalizerGraphLens.java
@@ -105,7 +105,8 @@
if (methodSignature == newMethodSignature) {
return previous;
}
- assert !previous.hasReboundReference();
+ assert !previous.hasReboundReference()
+ || previous.getReference() == previous.getReboundReference();
return MethodLookupResult.builder(this)
.setPrototypeChanges(
previous
diff --git a/src/main/java/com/android/tools/r8/optimize/redundantbridgeremoval/RedundantBridgeRemovalLens.java b/src/main/java/com/android/tools/r8/optimize/redundantbridgeremoval/RedundantBridgeRemovalLens.java
new file mode 100644
index 0000000..a680b36
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/optimize/redundantbridgeremoval/RedundantBridgeRemovalLens.java
@@ -0,0 +1,150 @@
+// Copyright (c) 2022, 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.optimize.redundantbridgeremoval;
+
+import com.android.tools.r8.graph.AppView;
+import com.android.tools.r8.graph.DexClassAndMethod;
+import com.android.tools.r8.graph.DexField;
+import com.android.tools.r8.graph.DexMethod;
+import com.android.tools.r8.graph.DexType;
+import com.android.tools.r8.graph.GraphLens;
+import com.android.tools.r8.graph.GraphLens.NonIdentityGraphLens;
+import com.android.tools.r8.graph.ProgramMethod;
+import com.android.tools.r8.graph.proto.RewrittenPrototypeDescription;
+import com.google.common.collect.Sets;
+import java.util.IdentityHashMap;
+import java.util.Map;
+import java.util.Set;
+
+public class RedundantBridgeRemovalLens extends NonIdentityGraphLens {
+
+ private final Set<DexType> interfaces;
+ private final Map<DexMethod, DexMethod> methodMap;
+
+ public RedundantBridgeRemovalLens(
+ AppView<?> appView, Set<DexType> interfaces, Map<DexMethod, DexMethod> methodMap) {
+ super(appView);
+ this.interfaces = interfaces;
+ this.methodMap = methodMap;
+ }
+
+ // Fields.
+
+ @Override
+ public DexField getOriginalFieldSignature(DexField field) {
+ return getPrevious().getOriginalFieldSignature(field);
+ }
+
+ @Override
+ public DexField getRenamedFieldSignature(DexField originalField, GraphLens codeLens) {
+ if (this == codeLens) {
+ return originalField;
+ }
+ return getPrevious().getRenamedFieldSignature(originalField, codeLens);
+ }
+
+ @Override
+ protected FieldLookupResult internalDescribeLookupField(FieldLookupResult previous) {
+ return previous;
+ }
+
+ // Methods.
+
+ @Override
+ public DexMethod getRenamedMethodSignature(DexMethod originalMethod, GraphLens applied) {
+ if (this == applied) {
+ return originalMethod;
+ }
+ DexMethod previousMethodSignature =
+ getPrevious().getRenamedMethodSignature(originalMethod, applied);
+ return methodMap.getOrDefault(previousMethodSignature, previousMethodSignature);
+ }
+
+ @Override
+ public DexMethod getNextMethodSignature(DexMethod method) {
+ return method;
+ }
+
+ @Override
+ public DexMethod getPreviousMethodSignature(DexMethod method) {
+ return method;
+ }
+
+ @Override
+ protected MethodLookupResult internalDescribeLookupMethod(
+ MethodLookupResult previous, DexMethod context) {
+ if (methodMap.containsKey(previous.getReference())) {
+ DexMethod newReference = previous.getReference();
+ do {
+ newReference = methodMap.get(newReference);
+ } while (methodMap.containsKey(newReference));
+ if (previous.getType().isSuper() && interfaces.contains(newReference.getHolderType())) {
+ return previous;
+ }
+ return MethodLookupResult.builder(this)
+ .setReference(newReference)
+ .setReboundReference(newReference)
+ .setPrototypeChanges(previous.getPrototypeChanges())
+ .setType(previous.getType())
+ .build();
+ }
+ return previous;
+ }
+
+ @Override
+ public RewrittenPrototypeDescription lookupPrototypeChangesForMethodDefinition(
+ DexMethod method, GraphLens codeLens) {
+ if (this == codeLens) {
+ return RewrittenPrototypeDescription.none();
+ }
+ return getPrevious().lookupPrototypeChangesForMethodDefinition(method, codeLens);
+ }
+
+ // Types.
+
+ @Override
+ public DexType getOriginalType(DexType type) {
+ return getPrevious().getOriginalType(type);
+ }
+
+ @Override
+ public Iterable<DexType> getOriginalTypes(DexType type) {
+ return getPrevious().getOriginalTypes(type);
+ }
+
+ @Override
+ protected DexType internalDescribeLookupClassType(DexType previous) {
+ return previous;
+ }
+
+ // Misc.
+
+ @Override
+ public boolean isContextFreeForMethods() {
+ return getPrevious().isContextFreeForMethods();
+ }
+
+ public static class Builder {
+
+ private final Set<DexType> interfaces = Sets.newIdentityHashSet();
+ private final Map<DexMethod, DexMethod> methodMap = new IdentityHashMap<>();
+
+ public synchronized Builder map(ProgramMethod from, DexClassAndMethod to) {
+ methodMap.put(from.getReference(), to.getReference());
+ if (to.getHolder().isInterface()) {
+ interfaces.add(to.getHolderType());
+ }
+ return this;
+ }
+
+ public boolean isEmpty() {
+ return methodMap.isEmpty();
+ }
+
+ public RedundantBridgeRemovalLens build(AppView<?> appView) {
+ return new RedundantBridgeRemovalLens(appView, interfaces, methodMap);
+ }
+ }
+}
