Version 2.1.29
Cherry pick: Fix bug in proto builder optimization
CL: https://r8-review.googlesource.com/c/r8/+/51692
Cherry pick: Reproduce proto builder optimization bug
CL: https://r8-review.googlesource.com/c/r8/+/51702
Bug: 155416893
Change-Id: I8fbf0a3224d32e65bfd2da7010a130e4ad5cdc04
diff --git a/src/main/java/com/android/tools/r8/R8.java b/src/main/java/com/android/tools/r8/R8.java
index 8406706..622224b 100644
--- a/src/main/java/com/android/tools/r8/R8.java
+++ b/src/main/java/com/android/tools/r8/R8.java
@@ -697,7 +697,7 @@
appView.withGeneratedMessageLiteBuilderShrinker(
shrinker ->
- shrinker.removeDeadBuilderReferencesFromDynamicMethods(
+ shrinker.rewriteDeadBuilderReferencesFromDynamicMethods(
appViewWithLiveness, executorService, timing));
if (options.isShrinking()) {
@@ -914,6 +914,10 @@
shrinker ->
shrinker.setDeadProtoTypes(appViewWithLiveness.appInfo().getDeadProtoTypes()));
}
+ appView.withGeneratedMessageLiteBuilderShrinker(
+ shrinker ->
+ shrinker.rewriteDeadBuilderReferencesFromDynamicMethods(
+ appViewWithLiveness, executorService, timing));
return appViewWithLiveness;
}
diff --git a/src/main/java/com/android/tools/r8/Version.java b/src/main/java/com/android/tools/r8/Version.java
index 283f647..47f1b12 100644
--- a/src/main/java/com/android/tools/r8/Version.java
+++ b/src/main/java/com/android/tools/r8/Version.java
@@ -11,7 +11,7 @@
// This field is accessed from release scripts using simple pattern matching.
// Therefore, changing this field could break our release scripts.
- public static final String LABEL = "2.1.28";
+ public static final String LABEL = "2.1.29";
private Version() {
}
diff --git a/src/main/java/com/android/tools/r8/graph/ClassAccessFlags.java b/src/main/java/com/android/tools/r8/graph/ClassAccessFlags.java
index 030497d..e33792c 100644
--- a/src/main/java/com/android/tools/r8/graph/ClassAccessFlags.java
+++ b/src/main/java/com/android/tools/r8/graph/ClassAccessFlags.java
@@ -167,6 +167,10 @@
return isSet(Constants.ACC_ABSTRACT);
}
+ public void demoteFromAbstract() {
+ demote(Constants.ACC_ABSTRACT);
+ }
+
public void setAbstract() {
set(Constants.ACC_ABSTRACT);
}
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/fieldaccess/FieldAssignmentTracker.java b/src/main/java/com/android/tools/r8/ir/analysis/fieldaccess/FieldAssignmentTracker.java
index fbb6443..1481061 100644
--- a/src/main/java/com/android/tools/r8/ir/analysis/fieldaccess/FieldAssignmentTracker.java
+++ b/src/main/java/com/android/tools/r8/ir/analysis/fieldaccess/FieldAssignmentTracker.java
@@ -64,8 +64,13 @@
FieldAssignmentTracker(AppView<AppInfoWithLiveness> appView) {
this.appView = appView;
- this.fieldAccessGraph = new FieldAccessGraph(appView);
- this.objectAllocationGraph = new ObjectAllocationGraph(appView);
+ this.fieldAccessGraph = new FieldAccessGraph();
+ this.objectAllocationGraph = new ObjectAllocationGraph();
+ }
+
+ public void initialize() {
+ fieldAccessGraph.initialize(appView);
+ objectAllocationGraph.initialize(appView);
initializeAbstractInstanceFieldValues();
}
@@ -308,10 +313,11 @@
private final Reference2IntMap<DexEncodedField> pendingFieldWrites =
new Reference2IntOpenHashMap<>();
- FieldAccessGraph(AppView<AppInfoWithLiveness> appView) {
+ FieldAccessGraph() {}
+
+ public void initialize(AppView<AppInfoWithLiveness> appView) {
FieldAccessInfoCollection<?> fieldAccessInfoCollection =
appView.appInfo().getFieldAccessInfoCollection();
- fieldAccessInfoCollection.flattenAccessContexts();
fieldAccessInfoCollection.forEach(
info -> {
DexEncodedField field =
@@ -356,7 +362,9 @@
private final Reference2IntMap<DexProgramClass> pendingObjectAllocations =
new Reference2IntOpenHashMap<>();
- ObjectAllocationGraph(AppView<AppInfoWithLiveness> appView) {
+ ObjectAllocationGraph() {}
+
+ public void initialize(AppView<AppInfoWithLiveness> appView) {
ObjectAllocationInfoCollection objectAllocationInfos =
appView.appInfo().getObjectAllocationInfoCollection();
objectAllocationInfos.forEachClassWithKnownAllocationSites(
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/proto/GeneratedMessageLiteBuilderShrinker.java b/src/main/java/com/android/tools/r8/ir/analysis/proto/GeneratedMessageLiteBuilderShrinker.java
index deadfa0..c2cd2aa 100644
--- a/src/main/java/com/android/tools/r8/ir/analysis/proto/GeneratedMessageLiteBuilderShrinker.java
+++ b/src/main/java/com/android/tools/r8/ir/analysis/proto/GeneratedMessageLiteBuilderShrinker.java
@@ -4,41 +4,48 @@
package com.android.tools.r8.ir.analysis.proto;
+import static com.android.tools.r8.graph.DexProgramClass.asProgramClassOrNull;
+import static com.android.tools.r8.ir.analysis.type.Nullability.definitelyNotNull;
+import static com.android.tools.r8.ir.analysis.type.Nullability.maybeNull;
+
import com.android.tools.r8.graph.AppInfoWithClassHierarchy;
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexEncodedMethod;
+import com.android.tools.r8.graph.DexField;
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexProgramClass;
import com.android.tools.r8.graph.DexType;
-import com.android.tools.r8.graph.EnumValueInfoMapCollection.EnumValueInfo;
-import com.android.tools.r8.graph.EnumValueInfoMapCollection.EnumValueInfoMap;
import com.android.tools.r8.graph.ProgramMethod;
import com.android.tools.r8.graph.SubtypingInfo;
+import com.android.tools.r8.graph.analysis.EnqueuerAnalysis;
import com.android.tools.r8.ir.analysis.type.ClassTypeElement;
import com.android.tools.r8.ir.analysis.type.TypeAnalysis;
-import com.android.tools.r8.ir.analysis.type.TypeElement;
import com.android.tools.r8.ir.code.CheckCast;
import com.android.tools.r8.ir.code.IRCode;
import com.android.tools.r8.ir.code.Instruction;
import com.android.tools.r8.ir.code.InstructionListIterator;
+import com.android.tools.r8.ir.code.InvokeDirect;
import com.android.tools.r8.ir.code.InvokeVirtual;
-import com.android.tools.r8.ir.code.Switch;
+import com.android.tools.r8.ir.code.NewInstance;
+import com.android.tools.r8.ir.code.StaticGet;
import com.android.tools.r8.ir.code.Value;
import com.android.tools.r8.ir.conversion.CallGraph.Node;
import com.android.tools.r8.ir.conversion.IRConverter;
import com.android.tools.r8.ir.conversion.MethodProcessor;
-import com.android.tools.r8.ir.optimize.CodeRewriter;
import com.android.tools.r8.ir.optimize.Inliner;
import com.android.tools.r8.ir.optimize.Inliner.Reason;
-import com.android.tools.r8.ir.optimize.controlflow.SwitchCaseAnalyzer;
import com.android.tools.r8.ir.optimize.enums.EnumValueOptimizer;
import com.android.tools.r8.ir.optimize.info.OptimizationFeedback;
import com.android.tools.r8.ir.optimize.info.OptimizationFeedbackSimple;
import com.android.tools.r8.ir.optimize.inliner.FixedInliningReasonStrategy;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
+import com.android.tools.r8.shaking.Enqueuer;
+import com.android.tools.r8.shaking.EnqueuerWorklist;
+import com.android.tools.r8.utils.ObjectUtils;
import com.android.tools.r8.utils.PredicateSet;
import com.android.tools.r8.utils.ThreadUtils;
import com.android.tools.r8.utils.Timing;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.Sets;
import java.util.ArrayList;
import java.util.IdentityHashMap;
@@ -53,6 +60,7 @@
private final AppView<? extends AppInfoWithClassHierarchy> appView;
private final ProtoReferences references;
+ private final boolean enableAggressiveBuilderOptimization;
private final Map<DexProgramClass, ProgramMethod> builders = new IdentityHashMap<>();
@@ -60,11 +68,98 @@
AppView<? extends AppInfoWithClassHierarchy> appView, ProtoReferences references) {
this.appView = appView;
this.references = references;
+ this.enableAggressiveBuilderOptimization = computeEnableAggressiveBuilderOptimization();
+ // If this fails it is likely an unsupported version of the protobuf library.
+ assert enableAggressiveBuilderOptimization;
+ }
+
+ private boolean computeEnableAggressiveBuilderOptimization() {
+ boolean unexpectedGeneratedMessageLiteBuilder =
+ ObjectUtils.getBooleanOrElse(
+ appView
+ .appInfo()
+ .definitionForWithoutExistenceAssert(references.generatedMessageLiteBuilderType),
+ clazz -> clazz.getMethodCollection().hasMethods(DexEncodedMethod::isAbstract),
+ true);
+ boolean unexpectedGeneratedMessageLiteExtendableBuilder =
+ ObjectUtils.getBooleanOrElse(
+ appView
+ .appInfo()
+ .definitionForWithoutExistenceAssert(
+ references.generatedMessageLiteExtendableBuilderType),
+ clazz -> clazz.getMethodCollection().hasMethods(DexEncodedMethod::isAbstract),
+ true);
+ if (unexpectedGeneratedMessageLiteBuilder) {
+ appView
+ .options()
+ .reporter
+ .warning(
+ "Unexpected implementation of `"
+ + references.generatedMessageLiteBuilderType.toSourceString()
+ + "`: disabling aggressive protobuf builder optimization.");
+ return false;
+ }
+ if (unexpectedGeneratedMessageLiteExtendableBuilder) {
+ appView
+ .options()
+ .reporter
+ .warning(
+ "Unexpected implementation of `"
+ + references.generatedMessageLiteExtendableBuilderType.toSourceString()
+ + "`: disabling aggressive protobuf builder optimization.");
+ return false;
+ }
+ return true;
+ }
+
+ public EnqueuerAnalysis createEnqueuerAnalysis() {
+ Set<DexProgramClass> seen = Sets.newIdentityHashSet();
+ return new EnqueuerAnalysis() {
+ @Override
+ public void notifyFixpoint(Enqueuer enqueuer, EnqueuerWorklist worklist, Timing timing) {
+ builders.forEach(
+ (builder, dynamicMethod) -> {
+ if (seen.add(builder)) {
+ // This builder class is never used in the program except from dynamicMethod(),
+ // which creates an instance of the builder. Instead of creating an instance of the
+ // builder class, we just instantiate the parent builder class. For this to work,
+ // we make the parent builder non-abstract.
+ DexProgramClass superClass =
+ asProgramClassOrNull(appView.definitionFor(builder.superType));
+ assert superClass != null;
+ superClass.accessFlags.demoteFromAbstract();
+ if (superClass.type == references.generatedMessageLiteBuilderType) {
+ // Manually trace `new GeneratedMessageLite.Builder(DEFAULT_INSTANCE)` since we
+ // haven't rewritten the code yet.
+ worklist.enqueueTraceNewInstanceAction(
+ references.generatedMessageLiteBuilderType, dynamicMethod);
+ worklist.enqueueTraceInvokeDirectAction(
+ references.generatedMessageLiteBuilderMethods.constructorMethod,
+ dynamicMethod);
+ } else {
+ assert superClass.type == references.generatedMessageLiteExtendableBuilderType;
+ // Manually trace `new GeneratedMessageLite.ExtendableBuilder(DEFAULT_INSTANCE)`
+ // since we haven't rewritten the code yet.
+ worklist.enqueueTraceNewInstanceAction(
+ references.generatedMessageLiteExtendableBuilderType, dynamicMethod);
+ worklist.enqueueTraceInvokeDirectAction(
+ references.generatedMessageLiteExtendableBuilderMethods.constructorMethod,
+ dynamicMethod);
+ }
+ worklist.enqueueTraceStaticFieldRead(
+ references.getDefaultInstanceField(dynamicMethod.getHolder()), dynamicMethod);
+ }
+ });
+ }
+ };
}
/** Returns true if an action was deferred. */
public boolean deferDeadProtoBuilders(
DexProgramClass clazz, ProgramMethod method, BooleanSupplier register) {
+ if (!enableAggressiveBuilderOptimization) {
+ return false;
+ }
DexEncodedMethod definition = method.getDefinition();
if (references.isDynamicMethod(definition) && references.isGeneratedMessageLiteBuilder(clazz)) {
if (register.getAsBoolean()) {
@@ -77,40 +172,79 @@
}
/**
- * Reprocesses each dynamicMethod() that references a dead builder to remove the dead builder
+ * Reprocesses each dynamicMethod() that references a dead builder to rewrite the dead builder
* references.
*/
- public void removeDeadBuilderReferencesFromDynamicMethods(
+ public void rewriteDeadBuilderReferencesFromDynamicMethods(
AppView<AppInfoWithLiveness> appView, ExecutorService executorService, Timing timing)
throws ExecutionException {
+ if (builders.isEmpty()) {
+ return;
+ }
timing.begin("Remove dead builder references");
AppInfoWithLiveness appInfo = appView.appInfo();
IRConverter converter = new IRConverter(appView, Timing.empty());
- CodeRewriter codeRewriter = new CodeRewriter(appView, converter);
- MethodToInvokeSwitchCaseAnalyzer switchCaseAnalyzer = new MethodToInvokeSwitchCaseAnalyzer();
- if (switchCaseAnalyzer.isInitialized()) {
- ThreadUtils.processItems(
- builders.entrySet(),
- entry -> {
- if (!appInfo.isLiveProgramClass(entry.getKey())) {
- removeDeadBuilderReferencesFromDynamicMethod(
- appView, entry.getValue(), converter, codeRewriter, switchCaseAnalyzer);
- }
- },
- executorService);
- }
+ ThreadUtils.processMap(
+ builders,
+ (builder, dynamicMethod) -> {
+ if (!appInfo.isLiveProgramClass(builder)) {
+ rewriteDeadBuilderReferencesFromDynamicMethod(
+ appView, builder, dynamicMethod, converter);
+ }
+ },
+ executorService);
builders.clear();
timing.end(); // Remove dead builder references
}
- private void removeDeadBuilderReferencesFromDynamicMethod(
+ private void rewriteDeadBuilderReferencesFromDynamicMethod(
AppView<AppInfoWithLiveness> appView,
+ DexProgramClass builder,
ProgramMethod dynamicMethod,
- IRConverter converter,
- CodeRewriter codeRewriter,
- SwitchCaseAnalyzer switchCaseAnalyzer) {
+ IRConverter converter) {
IRCode code = dynamicMethod.buildIR(appView);
- codeRewriter.rewriteSwitch(code, switchCaseAnalyzer);
+ InstructionListIterator instructionIterator = code.instructionListIterator();
+
+ assert builder.superType == references.generatedMessageLiteBuilderType
+ || builder.superType == references.generatedMessageLiteExtendableBuilderType;
+
+ DexField defaultInstanceField = references.getDefaultInstanceField(dynamicMethod.getHolder());
+ Value builderValue =
+ code.createValue(ClassTypeElement.create(builder.superType, definitelyNotNull(), appView));
+ Value defaultInstanceValue =
+ code.createValue(ClassTypeElement.create(defaultInstanceField.type, maybeNull(), appView));
+
+ // Replace `new Message.Builder()` by `new GeneratedMessageLite.Builder()`
+ // (or `new GeneratedMessageLite.ExtendableBuilder()`).
+ NewInstance newInstance =
+ instructionIterator.nextUntil(
+ instruction ->
+ instruction.isNewInstance() && instruction.asNewInstance().clazz == builder.type);
+ assert newInstance != null;
+ instructionIterator.replaceCurrentInstruction(new NewInstance(builder.superType, builderValue));
+
+ // Replace `builder.<init>()` by `builder.<init>(Message.DEFAULT_INSTANCE)`.
+ //
+ // We may also see an accessibility bridge constructor, because the Builder constructor is
+ // private. The accessibility bridge takes null as an argument.
+ InvokeDirect constructorInvoke =
+ instructionIterator.nextUntil(
+ instruction -> {
+ assert instruction.isInvokeDirect() || instruction.isConstNumber();
+ return instruction.isInvokeDirect();
+ });
+ assert constructorInvoke != null;
+ instructionIterator.replaceCurrentInstruction(
+ new StaticGet(defaultInstanceValue, defaultInstanceField));
+ instructionIterator.setInsertionPosition(constructorInvoke.getPosition());
+ instructionIterator.add(
+ new InvokeDirect(
+ builder.superType == references.generatedMessageLiteBuilderType
+ ? references.generatedMessageLiteBuilderMethods.constructorMethod
+ : references.generatedMessageLiteExtendableBuilderMethods.constructorMethod,
+ null,
+ ImmutableList.of(builderValue, defaultInstanceValue)));
+
converter.removeDeadCodeAndFinalizeIR(
dynamicMethod, code, OptimizationFeedbackSimple.getInstance(), Timing.empty());
}
@@ -232,51 +366,6 @@
}
}
- private class MethodToInvokeSwitchCaseAnalyzer extends SwitchCaseAnalyzer {
-
- private final int newBuilderOrdinal;
-
- private MethodToInvokeSwitchCaseAnalyzer() {
- EnumValueInfoMap enumValueInfoMap =
- appView.appInfo().withLiveness().getEnumValueInfoMap(references.methodToInvokeType);
- if (enumValueInfoMap != null) {
- EnumValueInfo newBuilderValueInfo =
- enumValueInfoMap.getEnumValueInfo(references.methodToInvokeMembers.newBuilderField);
- if (newBuilderValueInfo != null) {
- newBuilderOrdinal = newBuilderValueInfo.ordinal;
- return;
- }
- }
- newBuilderOrdinal = -1;
- }
-
- public boolean isInitialized() {
- return newBuilderOrdinal >= 0;
- }
-
- @Override
- public boolean switchCaseIsUnreachable(Switch theSwitch, int index) {
- if (theSwitch.isStringSwitch()) {
- assert false : "Unexpected string-switch instruction in dynamicMethod()";
- return false;
- }
- if (index != newBuilderOrdinal) {
- return false;
- }
- Value switchValue = theSwitch.value();
- if (!switchValue.isDefinedByInstructionSatisfying(Instruction::isInvokeVirtual)) {
- return false;
- }
- InvokeVirtual definition = switchValue.definition.asInvokeVirtual();
- if (definition.getInvokedMethod() != appView.dexItemFactory().enumMethods.ordinal) {
- return false;
- }
- TypeElement enumType = definition.getReceiver().getType();
- return enumType.isClassType()
- && enumType.asClassType().getClassType() == references.methodToInvokeType;
- }
- }
-
private static class RootSetExtension {
private final AppView<? extends AppInfoWithClassHierarchy> appView;
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/proto/ProtoInliningReasonStrategy.java b/src/main/java/com/android/tools/r8/ir/analysis/proto/ProtoInliningReasonStrategy.java
index ca4ca1e..e8770c7 100644
--- a/src/main/java/com/android/tools/r8/ir/analysis/proto/ProtoInliningReasonStrategy.java
+++ b/src/main/java/com/android/tools/r8/ir/analysis/proto/ProtoInliningReasonStrategy.java
@@ -43,11 +43,19 @@
return Reason.ALWAYS;
}
return references.isDynamicMethod(target) || references.isDynamicMethodBridge(target)
- ? computeInliningReasonForDynamicMethod(invoke)
+ ? computeInliningReasonForDynamicMethod(invoke, target, context)
: parent.computeInliningReason(invoke, target, context);
}
- private Reason computeInliningReasonForDynamicMethod(InvokeMethod invoke) {
+ private Reason computeInliningReasonForDynamicMethod(
+ InvokeMethod invoke, ProgramMethod target, ProgramMethod context) {
+ // Do not allow inlining of dynamicMethod() into a proto library class. This should only happen
+ // if there is exactly one proto message in the program, since we would otherwise not be able
+ // to conclude a single target.
+ if (references.isDynamicMethod(target) && references.isProtoLibraryClass(context.getHolder())) {
+ return Reason.NEVER;
+ }
+
Value methodToInvokeValue =
invoke
.inValues()
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/proto/ProtoReferences.java b/src/main/java/com/android/tools/r8/ir/analysis/proto/ProtoReferences.java
index 861c0cd..8fd43ee 100644
--- a/src/main/java/com/android/tools/r8/ir/analysis/proto/ProtoReferences.java
+++ b/src/main/java/com/android/tools/r8/ir/analysis/proto/ProtoReferences.java
@@ -17,6 +17,8 @@
public class ProtoReferences {
+ private final DexItemFactory dexItemFactory;
+
public final DexType enumLiteMapType;
public final DexType extendableMessageType;
public final DexType extensionDescriptorType;
@@ -25,6 +27,7 @@
public final DexType generatedMessageLiteType;
public final DexType generatedMessageLiteBuilderType;
public final DexType generatedMessageLiteExtendableBuilderType;
+ public final DexType generatedMessageLiteExtendableMessageType;
public final DexType rawMessageInfoType;
public final DexType messageLiteType;
public final DexType methodToInvokeType;
@@ -37,10 +40,13 @@
generatedMessageLiteExtendableBuilderMethods;
public final MethodToInvokeMembers methodToInvokeMembers;
+ public final DexString defaultInstanceFieldName;
public final DexString dynamicMethodName;
public final DexString findLiteExtensionByNumberName;
public final DexString newBuilderMethodName;
+ public final DexString protobufPackageDescriptorPrefix;
+
public final DexProto dynamicMethodProto;
public final DexProto findLiteExtensionByNumberProto;
@@ -49,6 +55,8 @@
public final DexMethod rawMessageInfoConstructor;
public ProtoReferences(DexItemFactory factory) {
+ dexItemFactory = factory;
+
// Types.
enumLiteMapType = factory.createType("Lcom/google/protobuf/Internal$EnumLiteMap;");
extendableMessageType =
@@ -63,6 +71,8 @@
factory.createType("Lcom/google/protobuf/GeneratedMessageLite$Builder;");
generatedMessageLiteExtendableBuilderType =
factory.createType("Lcom/google/protobuf/GeneratedMessageLite$ExtendableBuilder;");
+ generatedMessageLiteExtendableMessageType =
+ factory.createType("Lcom/google/protobuf/GeneratedMessageLite$ExtendableMessage;");
rawMessageInfoType = factory.createType("Lcom/google/protobuf/RawMessageInfo;");
messageLiteType = factory.createType("Lcom/google/protobuf/MessageLite;");
methodToInvokeType =
@@ -70,10 +80,14 @@
wireFormatFieldType = factory.createType("Lcom/google/protobuf/WireFormat$FieldType;");
// Names.
+ defaultInstanceFieldName = factory.createString("DEFAULT_INSTANCE");
dynamicMethodName = factory.createString("dynamicMethod");
findLiteExtensionByNumberName = factory.createString("findLiteExtensionByNumber");
newBuilderMethodName = factory.createString("newBuilder");
+ // Other names.
+ protobufPackageDescriptorPrefix = factory.createString("Lcom/google/protobuf/");
+
// Protos.
dynamicMethodProto =
factory.createProto(
@@ -105,6 +119,10 @@
methodToInvokeMembers = new MethodToInvokeMembers(factory);
}
+ public DexField getDefaultInstanceField(DexProgramClass holder) {
+ return dexItemFactory.createField(holder.type, holder.type, defaultInstanceFieldName);
+ }
+
public boolean isAbstractGeneratedMessageLiteBuilder(DexProgramClass clazz) {
return clazz.type == generatedMessageLiteBuilderType
|| clazz.type == generatedMessageLiteExtendableBuilderType;
@@ -123,7 +141,8 @@
}
public boolean isDynamicMethodBridge(DexMethod method) {
- return method == generatedMessageLiteMethods.dynamicMethodBridgeMethod;
+ return method == generatedMessageLiteMethods.dynamicMethodBridgeMethod
+ || method == generatedMessageLiteMethods.dynamicMethodBridgeMethodWithObject;
}
public boolean isDynamicMethodBridge(DexEncodedMethod method) {
@@ -154,6 +173,10 @@
return method.match(newMessageInfoMethod) || method == rawMessageInfoConstructor;
}
+ public boolean isProtoLibraryClass(DexProgramClass clazz) {
+ return clazz.type.descriptor.startsWith(protobufPackageDescriptorPrefix);
+ }
+
public class GeneratedExtensionMethods {
public final DexMethod constructor;
@@ -192,6 +215,7 @@
public final DexMethod createBuilderMethod;
public final DexMethod dynamicMethodBridgeMethod;
+ public final DexMethod dynamicMethodBridgeMethodWithObject;
public final DexMethod isInitializedMethod;
public final DexMethod newRepeatedGeneratedExtension;
public final DexMethod newSingularGeneratedExtension;
@@ -207,6 +231,12 @@
generatedMessageLiteType,
dexItemFactory.createProto(dexItemFactory.objectType, methodToInvokeType),
"dynamicMethod");
+ dynamicMethodBridgeMethodWithObject =
+ dexItemFactory.createMethod(
+ generatedMessageLiteType,
+ dexItemFactory.createProto(
+ dexItemFactory.objectType, methodToInvokeType, dexItemFactory.objectType),
+ "dynamicMethod");
isInitializedMethod =
dexItemFactory.createMethod(
generatedMessageLiteType,
@@ -241,7 +271,7 @@
}
}
- class GeneratedMessageLiteBuilderMethods {
+ public class GeneratedMessageLiteBuilderMethods {
public final DexMethod buildPartialMethod;
public final DexMethod constructorMethod;
@@ -260,9 +290,10 @@
}
}
- class GeneratedMessageLiteExtendableBuilderMethods {
+ public class GeneratedMessageLiteExtendableBuilderMethods {
public final DexMethod buildPartialMethod;
+ public final DexMethod constructorMethod;
private GeneratedMessageLiteExtendableBuilderMethods(DexItemFactory dexItemFactory) {
buildPartialMethod =
@@ -270,6 +301,12 @@
generatedMessageLiteExtendableBuilderType,
dexItemFactory.createProto(extendableMessageType),
"buildPartial");
+ constructorMethod =
+ dexItemFactory.createMethod(
+ generatedMessageLiteExtendableBuilderType,
+ dexItemFactory.createProto(
+ dexItemFactory.voidType, generatedMessageLiteExtendableMessageType),
+ dexItemFactory.constructorMethodName);
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/IRCodeInstructionListIterator.java b/src/main/java/com/android/tools/r8/ir/code/IRCodeInstructionListIterator.java
index b9cc832..b4860aa 100644
--- a/src/main/java/com/android/tools/r8/ir/code/IRCodeInstructionListIterator.java
+++ b/src/main/java/com/android/tools/r8/ir/code/IRCodeInstructionListIterator.java
@@ -151,6 +151,11 @@
}
@Override
+ public void setInsertionPosition(Position position) {
+ instructionIterator.setInsertionPosition(position);
+ }
+
+ @Override
public void replaceCurrentInstruction(Instruction newInstruction, Set<Value> affectedValues) {
instructionIterator.replaceCurrentInstruction(newInstruction, affectedValues);
}
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 844ccce..c0f75e9 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
@@ -675,6 +675,9 @@
// 2) Revisit DexEncodedMethods for the collected candidates.
printPhase("Primary optimization pass");
+ if (fieldAccessAnalysis != null) {
+ fieldAccessAnalysis.fieldAssignmentTracker().initialize();
+ }
// Process the application identifying outlining candidates.
GraphLense graphLenseForIR = appView.graphLense();
diff --git a/src/main/java/com/android/tools/r8/optimize/MemberRebindingAnalysis.java b/src/main/java/com/android/tools/r8/optimize/MemberRebindingAnalysis.java
index b992f8d..1c1de57 100644
--- a/src/main/java/com/android/tools/r8/optimize/MemberRebindingAnalysis.java
+++ b/src/main/java/com/android/tools/r8/optimize/MemberRebindingAnalysis.java
@@ -347,9 +347,9 @@
computeMethodRebinding(appInfo.directInvokes, this::anyLookup, Type.DIRECT);
// Likewise static invokes.
computeMethodRebinding(appInfo.staticInvokes, this::anyLookup, Type.STATIC);
-
computeFieldRebinding();
-
- return builder.build(lense);
+ GraphLense lens = builder.build(lense);
+ appInfo.getFieldAccessInfoCollection().flattenAccessContexts();
+ return lens;
}
}
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 8a54cca..1cc091d 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -374,10 +374,15 @@
this.useRegistryFactory = createUseRegistryFactory();
this.workList = EnqueuerWorklist.createWorklist(appView);
- if (options.protoShrinking().enableGeneratedMessageLiteShrinking
- && mode.isInitialOrFinalTreeShaking()) {
- registerAnalysis(new ProtoEnqueuerExtension(appView));
+ if (mode.isInitialOrFinalTreeShaking()) {
+ if (options.protoShrinking().enableGeneratedMessageLiteShrinking) {
+ registerAnalysis(new ProtoEnqueuerExtension(appView));
+ }
+ appView.withGeneratedMessageLiteBuilderShrinker(
+ shrinker -> registerAnalysis(shrinker.createEnqueuerAnalysis()));
}
+
+
liveTypes = new SetWithReportedReason<>();
initializedTypes = new SetWithReportedReason<>();
targetedMethods = new SetWithReason<>(graphReporter::registerMethod);
diff --git a/src/test/examplesProto/proto2/BuilderOnlyReferencedFromDynamicMethodTestClass.java b/src/test/examplesProto/proto2/BuilderOnlyReferencedFromDynamicMethodTestClass.java
new file mode 100644
index 0000000..e3707dc
--- /dev/null
+++ b/src/test/examplesProto/proto2/BuilderOnlyReferencedFromDynamicMethodTestClass.java
@@ -0,0 +1,24 @@
+// Copyright (c) 2020, 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 proto2;
+
+import com.android.tools.r8.proto2.TestProto.Primitives;
+import com.google.protobuf.GeneratedMessageLite;
+import com.google.protobuf.InvalidProtocolBufferException;
+
+public class BuilderOnlyReferencedFromDynamicMethodTestClass {
+
+ public static void main(String[] args) {
+ GeneratedMessageLite<?, ?> primitivesInDisguise;
+ try {
+ primitivesInDisguise = Primitives.parseFrom(new byte[0]);
+ } catch (InvalidProtocolBufferException e) {
+ System.out.println("Unexpected exception: " + e);
+ throw new RuntimeException(e);
+ }
+ Primitives primitives = (Primitives) primitivesInDisguise.toBuilder().build();
+ Printer.print(primitives);
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/internal/proto/Proto2BuilderOnlyReferencedFromDynamicMethodTest.java b/src/test/java/com/android/tools/r8/internal/proto/Proto2BuilderOnlyReferencedFromDynamicMethodTest.java
new file mode 100644
index 0000000..6405047
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/internal/proto/Proto2BuilderOnlyReferencedFromDynamicMethodTest.java
@@ -0,0 +1,78 @@
+// Copyright (c) 2020, 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.internal.proto;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.CoreMatchers.equalTo;
+import static org.hamcrest.CoreMatchers.not;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertFalse;
+
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.utils.codeinspector.ClassSubject;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import com.google.common.collect.ImmutableList;
+import java.nio.file.Path;
+import java.util.List;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class Proto2BuilderOnlyReferencedFromDynamicMethodTest extends ProtoShrinkingTestBase {
+
+ private static final String MAIN = "proto2.BuilderOnlyReferencedFromDynamicMethodTestClass";
+
+ private static List<Path> PROGRAM_FILES =
+ ImmutableList.of(PROTO2_EXAMPLES_JAR, PROTO2_PROTO_JAR, PROTOBUF_LITE_JAR);
+
+ private final TestParameters parameters;
+
+ @Parameterized.Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimesAndApiLevels().build();
+ }
+
+ public Proto2BuilderOnlyReferencedFromDynamicMethodTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void test() throws Exception {
+ testForR8(parameters.getBackend())
+ .addProgramFiles(PROGRAM_FILES)
+ .addKeepMainRule(MAIN)
+ .addKeepRuleFiles(PROTOBUF_LITE_PROGUARD_RULES)
+ .allowAccessModification()
+ .allowDiagnosticMessages()
+ .allowUnusedProguardConfigurationRules()
+ .enableProtoShrinking()
+ .setMinApi(parameters.getApiLevel())
+ .compile()
+ .assertAllInfoMessagesMatch(
+ containsString("Proguard configuration rule does not match anything"))
+ .assertAllWarningMessagesMatch(equalTo("Resource 'META-INF/MANIFEST.MF' already exists."))
+ .inspect(this::inspect)
+ .run(parameters.getRuntime(), MAIN)
+ .assertSuccessWithOutputLines(
+ "false", "0", "false", "", "false", "0", "false", "0", "false", "");
+ }
+
+ private void inspect(CodeInspector outputInspector) {
+ verifyBuilderIsAbsent(outputInspector);
+ }
+
+ private void verifyBuilderIsAbsent(CodeInspector outputInspector) {
+ ClassSubject generatedMessageLiteBuilder =
+ outputInspector.clazz("com.google.protobuf.GeneratedMessageLite$Builder");
+ assertThat(generatedMessageLiteBuilder, isPresent());
+ assertFalse(generatedMessageLiteBuilder.isAbstract());
+ assertThat(
+ outputInspector.clazz("com.android.tools.r8.proto2.TestProto$Primitives$Builder"),
+ not(isPresent()));
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/internal/proto/Proto2BuilderShrinkingTest.java b/src/test/java/com/android/tools/r8/internal/proto/Proto2BuilderShrinkingTest.java
index 8c98b46..dc2b296 100644
--- a/src/test/java/com/android/tools/r8/internal/proto/Proto2BuilderShrinkingTest.java
+++ b/src/test/java/com/android/tools/r8/internal/proto/Proto2BuilderShrinkingTest.java
@@ -171,9 +171,7 @@
assertThat(
outputInspector.clazz(
"com.android.tools.r8.proto2.Shrinking$HasFlaggedOffExtension$Builder"),
- mains.equals(ImmutableList.of("proto2.HasFlaggedOffExtensionBuilderTestClass"))
- ? isPresent()
- : not(isPresent()));
+ not(isPresent()));
assertThat(
outputInspector.clazz("com.android.tools.r8.proto2.TestProto$Primitives$Builder"),
not(isPresent()));
