Support for simple builder pattern in class inliner.
CL generalizes ClassInliner to properly support arbitrary writes
to class fields. Actually, this change makes some things simpler
(like method eligibility analysis).
Stat (comparing to numbers with class inliner disabled)
gmscore-v9: -43K
gmscore-v10: -103K
tachiyomi: -22K
Bug: 80392161
Change-Id: I85db2fbec3655c69ffd267b7ecb0ab07cb6e7e82
diff --git a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
index 9f97f88..831edbe 100644
--- a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
+++ b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
@@ -43,12 +43,9 @@
import com.android.tools.r8.naming.NamingLens;
import com.android.tools.r8.origin.Origin;
import com.android.tools.r8.utils.InternalOptions;
-import com.google.common.collect.ImmutableMap;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
-import java.util.Map;
-import java.util.Set;
import java.util.function.Consumer;
public class DexEncodedMethod extends KeyedDexItem<DexMethod> implements ResolutionResult {
@@ -582,6 +579,14 @@
return m1.method.slowCompareTo(m2.method);
}
+ public static class ClassInlinerEligibility {
+ public final boolean returnsReceiver;
+
+ public ClassInlinerEligibility(boolean returnsReceiver) {
+ this.returnsReceiver = returnsReceiver;
+ }
+ }
+
public static class OptimizationInfo {
private int returnedArgument = -1;
@@ -593,8 +598,9 @@
private boolean useIdentifierNameString = false;
private boolean checksNullReceiverBeforeAnySideEffect = false;
private boolean triggersClassInitBeforeAnySideEffect = false;
- private Set<DexField> receiverOnlyUsedForReadingFields = null;
- private Map<DexField, Integer> onlyInitializesFieldsWithNoOtherSideEffects = null;
+ // Stores information about instance methods and constructors for
+ // class inliner, null value indicates that the method is not eligible.
+ private ClassInlinerEligibility classInlinerEligibility = null;
private OptimizationInfo() {
// Intentionally left empty.
@@ -631,13 +637,12 @@
return returnsConstant;
}
- public boolean isReceiverOnlyUsedForReadingFields(Set<DexField> fields) {
- return receiverOnlyUsedForReadingFields != null &&
- fields.containsAll(receiverOnlyUsedForReadingFields);
+ private void setClassInlinerEligibility(ClassInlinerEligibility eligibility) {
+ this.classInlinerEligibility = eligibility;
}
- public Map<DexField, Integer> onlyInitializesFieldsWithNoOtherSideEffects() {
- return onlyInitializesFieldsWithNoOtherSideEffects;
+ public ClassInlinerEligibility getClassInlinerEligibility() {
+ return this.classInlinerEligibility;
}
public long getReturnedConstant() {
@@ -681,19 +686,6 @@
returnedConstant = value;
}
- private void markReceiverOnlyUsedForReadingFields(Set<DexField> fields) {
- receiverOnlyUsedForReadingFields = fields;
- }
-
- private void markOnlyInitializesFieldsWithNoOtherSideEffects(Map<DexField, Integer> mapping) {
- if (mapping == null) {
- onlyInitializesFieldsWithNoOtherSideEffects = null;
- } else {
- onlyInitializesFieldsWithNoOtherSideEffects = mapping.isEmpty()
- ? Collections.emptyMap() : ImmutableMap.copyOf(mapping);
- }
- }
-
private void markForceInline() {
forceInline = true;
}
@@ -751,13 +743,8 @@
ensureMutableOI().markReturnsConstant(value);
}
- synchronized public void markReceiverOnlyUsedForReadingFields(Set<DexField> fields) {
- ensureMutableOI().markReceiverOnlyUsedForReadingFields(fields);
- }
-
- synchronized public void markOnlyInitializesFieldsWithNoOtherSideEffects(
- Map<DexField, Integer> mapping) {
- ensureMutableOI().markOnlyInitializesFieldsWithNoOtherSideEffects(mapping);
+ synchronized public void setClassInlinerEligibility(ClassInlinerEligibility eligibility) {
+ ensureMutableOI().setClassInlinerEligibility(eligibility);
}
synchronized public void markForceInline() {
diff --git a/src/main/java/com/android/tools/r8/ir/code/Phi.java b/src/main/java/com/android/tools/r8/ir/code/Phi.java
index 186839b..51850a0 100644
--- a/src/main/java/com/android/tools/r8/ir/code/Phi.java
+++ b/src/main/java/com/android/tools/r8/ir/code/Phi.java
@@ -76,6 +76,10 @@
}
public void addOperands(List<Value> operands) {
+ addOperands(operands, true);
+ }
+
+ public void addOperands(List<Value> operands, boolean removeTrivialPhi) {
// Phi operands are only filled in once to complete the phi. Some phis are incomplete for a
// period of time to break cycles. When the cycle has been resolved they are completed
// exactly once by adding the operands.
@@ -91,7 +95,9 @@
if (!canBeNull) {
markNeverNull();
}
- removeTrivialPhi();
+ if (removeTrivialPhi) {
+ removeTrivialPhi();
+ }
}
public void addDebugValue(Value value) {
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 90a9898..ac7edba 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
@@ -169,7 +169,8 @@
}
this.classInliner =
(options.enableClassInlining && options.enableInlining && inliner != null)
- ? new ClassInliner(appInfo.dexItemFactory) : null;
+ ? new ClassInliner(appInfo.dexItemFactory, options.classInliningInstructionLimit)
+ : null;
}
/**
@@ -752,10 +753,7 @@
}
// Analysis must be done after method is rewritten by logArgumentTypes()
- codeRewriter.identifyReceiverOnlyUsedForReadingFields(method, code, feedback);
- if (method.isInstanceInitializer()) {
- codeRewriter.identifyOnlyInitializesFieldsWithNoOtherSideEffects(method, code, feedback);
- }
+ codeRewriter.identifyClassInlinerEligibility(method, code, feedback);
printMethod(code, "Optimized IR (SSA)");
finalizeIR(method, code, feedback);
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/OptimizationFeedback.java b/src/main/java/com/android/tools/r8/ir/conversion/OptimizationFeedback.java
index 26ca6b2..16a77c8 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/OptimizationFeedback.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/OptimizationFeedback.java
@@ -5,10 +5,8 @@
package com.android.tools.r8.ir.conversion;
import com.android.tools.r8.graph.DexEncodedMethod;
-import com.android.tools.r8.graph.DexField;
+import com.android.tools.r8.graph.DexEncodedMethod.ClassInlinerEligibility;
import com.android.tools.r8.ir.optimize.Inliner.Constraint;
-import java.util.Map;
-import java.util.Set;
public interface OptimizationFeedback {
void methodReturnsArgument(DexEncodedMethod method, int argument);
@@ -18,7 +16,5 @@
void markProcessed(DexEncodedMethod method, Constraint state);
void markCheckNullReceiverBeforeAnySideEffect(DexEncodedMethod method, boolean mark);
void markTriggerClassInitBeforeAnySideEffect(DexEncodedMethod method, boolean mark);
- void markReceiverOnlyUsedForReadingFields(DexEncodedMethod method, Set<DexField> fields);
- void markOnlyInitializesFieldsWithNoOtherSideEffects(
- DexEncodedMethod method, Map<DexField, Integer> mapping);
+ void setClassInlinerEligibility(DexEncodedMethod method, ClassInlinerEligibility eligibility);
}
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/OptimizationFeedbackDirect.java b/src/main/java/com/android/tools/r8/ir/conversion/OptimizationFeedbackDirect.java
index 645df64..7303b6b 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/OptimizationFeedbackDirect.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/OptimizationFeedbackDirect.java
@@ -5,10 +5,8 @@
package com.android.tools.r8.ir.conversion;
import com.android.tools.r8.graph.DexEncodedMethod;
-import com.android.tools.r8.graph.DexField;
+import com.android.tools.r8.graph.DexEncodedMethod.ClassInlinerEligibility;
import com.android.tools.r8.ir.optimize.Inliner.Constraint;
-import java.util.Map;
-import java.util.Set;
public class OptimizationFeedbackDirect implements OptimizationFeedback {
@@ -48,13 +46,8 @@
}
@Override
- public void markReceiverOnlyUsedForReadingFields(DexEncodedMethod method, Set<DexField> fields) {
- method.markReceiverOnlyUsedForReadingFields(fields);
- }
-
- @Override
- public void markOnlyInitializesFieldsWithNoOtherSideEffects(DexEncodedMethod method,
- Map<DexField, Integer> mapping) {
- method.markOnlyInitializesFieldsWithNoOtherSideEffects(mapping);
+ public void setClassInlinerEligibility(
+ DexEncodedMethod method, ClassInlinerEligibility eligibility) {
+ method.setClassInlinerEligibility(eligibility);
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/OptimizationFeedbackIgnore.java b/src/main/java/com/android/tools/r8/ir/conversion/OptimizationFeedbackIgnore.java
index 97fe052..6915a2f 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/OptimizationFeedbackIgnore.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/OptimizationFeedbackIgnore.java
@@ -5,10 +5,8 @@
package com.android.tools.r8.ir.conversion;
import com.android.tools.r8.graph.DexEncodedMethod;
-import com.android.tools.r8.graph.DexField;
+import com.android.tools.r8.graph.DexEncodedMethod.ClassInlinerEligibility;
import com.android.tools.r8.ir.optimize.Inliner.Constraint;
-import java.util.Map;
-import java.util.Set;
public class OptimizationFeedbackIgnore implements OptimizationFeedback {
@@ -34,11 +32,7 @@
public void markTriggerClassInitBeforeAnySideEffect(DexEncodedMethod method, boolean mark) {}
@Override
- public void markReceiverOnlyUsedForReadingFields(DexEncodedMethod method, Set<DexField> fields) {
- }
-
- @Override
- public void markOnlyInitializesFieldsWithNoOtherSideEffects(DexEncodedMethod method,
- Map<DexField, Integer> mapping) {
+ public void setClassInlinerEligibility(
+ DexEncodedMethod method, ClassInlinerEligibility eligibility) {
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java b/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
index 3ea434a..cdcd29c 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
@@ -12,6 +12,7 @@
import com.android.tools.r8.graph.DexClass;
import com.android.tools.r8.graph.DexEncodedField;
import com.android.tools.r8.graph.DexEncodedMethod;
+import com.android.tools.r8.graph.DexEncodedMethod.ClassInlinerEligibility;
import com.android.tools.r8.graph.DexField;
import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.graph.DexMethod;
@@ -47,8 +48,6 @@
import com.android.tools.r8.ir.code.IRCode;
import com.android.tools.r8.ir.code.If;
import com.android.tools.r8.ir.code.If.Type;
-import com.android.tools.r8.ir.code.InstanceGet;
-import com.android.tools.r8.ir.code.InstancePut;
import com.android.tools.r8.ir.code.Instruction;
import com.android.tools.r8.ir.code.InstructionIterator;
import com.android.tools.r8.ir.code.InstructionListIterator;
@@ -736,120 +735,72 @@
}
}
- public void identifyReceiverOnlyUsedForReadingFields(
+ public void identifyClassInlinerEligibility(
DexEncodedMethod method, IRCode code, OptimizationFeedback feedback) {
- if (!method.isNonAbstractVirtualMethod()) {
+ // Method eligibility is calculated in similar way for regular method
+ // and for the constructor. To be eligible method should only be using its
+ // receiver in the following ways:
+ //
+ // (1) as a receiver of reads/writes of instance fields of the holder class
+ // (2) as a return value
+ // (3) as a receiver of a call to the superclass initializer
+ //
+ boolean instanceInitializer = method.isInstanceInitializer();
+ if (method.accessFlags.isNative() ||
+ (!method.isNonAbstractVirtualMethod() && !instanceInitializer)) {
return;
}
- feedback.markReceiverOnlyUsedForReadingFields(method, null);
+ feedback.setClassInlinerEligibility(method, null); // To allow returns below.
- Value instance = code.getThis();
- if (instance.numberOfPhiUsers() > 0) {
- return;
- }
-
- Set<DexField> fields = Sets.newIdentityHashSet();
- for (Instruction insn : instance.uniqueUsers()) {
- if (!insn.isInstanceGet()) {
- return;
- }
- InstanceGet get = insn.asInstanceGet();
- if (get.dest() == instance || get.object() != instance) {
- return;
- }
- fields.add(get.getField());
- }
- feedback.markReceiverOnlyUsedForReadingFields(method, fields);
- }
-
- public void identifyOnlyInitializesFieldsWithNoOtherSideEffects(
- DexEncodedMethod method, IRCode code, OptimizationFeedback feedback) {
- assert method.isInstanceInitializer();
-
- feedback.markOnlyInitializesFieldsWithNoOtherSideEffects(method, null);
-
- if (code.hasCatchHandlers()) {
- return;
- }
-
- List<Value> args = code.collectArguments(true /* not interested in receiver */);
- Map<DexField, Integer> mapping = new IdentityHashMap<>();
Value receiver = code.getThis();
-
- InstructionIterator it = code.instructionIterator();
- while (it.hasNext()) {
- Instruction instruction = it.next();
-
- // Mark an argument.
- if (instruction.isArgument()) {
- continue;
- }
-
- // Allow super call to java.lang.Object.<init>() on 'this'.
- if (instruction.isInvokeDirect()) {
- InvokeDirect invokedDirect = instruction.asInvokeDirect();
- if (invokedDirect.getInvokedMethod() != dexItemFactory.objectMethods.constructor ||
- invokedDirect.getReceiver() != receiver) {
- return;
- }
- continue;
- }
-
- // Allow final return.
- if (instruction.isReturn()) {
- continue;
- }
-
- // Allow assignment to this class' fields. If the assigned value is an argument
- // reference update the mep. Otherwise just allow assigning any value, since all
- // invalid values should be filtered out at the definitions.
- if (instruction.isInstancePut()) {
- InstancePut instancePut = instruction.asInstancePut();
- DexField field = instancePut.getField();
- if (instancePut.object() != receiver) {
- return;
- }
-
- Value value = instancePut.value();
- if (value.isArgument() && !value.isThis()) {
- assert (args.contains(value));
- mapping.put(field, args.indexOf(value));
- } else {
- mapping.remove(field);
- }
- continue;
- }
-
- // Allow non-throwing constants.
- if (instruction.isConstInstruction()) {
- if (instruction.instructionTypeCanThrow()) {
- return;
- }
- continue;
- }
-
- // Allow goto instructions jumping to the next block.
- if (instruction.isGoto()) {
- if (instruction.getBlock().getNumber() + 1 !=
- instruction.asGoto().getTarget().getNumber()) {
- return;
- }
- continue;
- }
-
- // Allow binary and unary instructions if they don't throw.
- if (instruction.isBinop() || instruction.isUnop()) {
- if (instruction.instructionTypeCanThrow()) {
- return;
- }
- continue;
- }
-
+ if (receiver.numberOfPhiUsers() > 0) {
return;
}
- feedback.markOnlyInitializesFieldsWithNoOtherSideEffects(method, mapping);
+ boolean receiverUsedAsReturnValue = false;
+ boolean seenSuperInitCall = false;
+ for (Instruction insn : receiver.uniqueUsers()) {
+ if (insn.isReturn()) {
+ receiverUsedAsReturnValue = true;
+ continue;
+ }
+
+ if (insn.isInstanceGet() ||
+ (insn.isInstancePut() && insn.asInstancePut().object() == receiver)) {
+ DexField field = insn.asFieldInstruction().getField();
+ if (field.clazz == method.method.holder) {
+ // Since class inliner currently only supports classes directly extending
+ // java.lang.Object, we don't need to worry about fields defined in superclasses.
+ continue;
+ }
+ return;
+ }
+
+ // If this is an instance initializer allow one call
+ // to java.lang.Object.<init>() on 'this'.
+ if (instanceInitializer && insn.isInvokeDirect()) {
+ InvokeDirect invokedDirect = insn.asInvokeDirect();
+ if (invokedDirect.getInvokedMethod() == dexItemFactory.objectMethods.constructor &&
+ invokedDirect.getReceiver() == receiver &&
+ !seenSuperInitCall) {
+ seenSuperInitCall = true;
+ continue;
+ }
+ return;
+ }
+
+ // Other receiver usages make the method not eligible.
+ return;
+ }
+
+ if (instanceInitializer && !seenSuperInitCall) {
+ // Call to super constructor not found?
+ return;
+ }
+
+ feedback.setClassInlinerEligibility(
+ method, new ClassInlinerEligibility(receiverUsedAsReturnValue));
}
/**
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/ConstantCanonicalizer.java b/src/main/java/com/android/tools/r8/ir/optimize/ConstantCanonicalizer.java
index 7a6a846..92bda1d 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/ConstantCanonicalizer.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/ConstantCanonicalizer.java
@@ -86,6 +86,7 @@
});
}
+ code.removeAllTrivialPhis();
assert code.isConsistentSSA();
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java b/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java
index 845e17b..01dcf0d 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java
@@ -315,6 +315,12 @@
}
@Override
+ public boolean isValidTarget(InvokeMethod invoke, DexEncodedMethod target, IRCode inlinee) {
+ return !target.isInstanceInitializer()
+ || inliner.legalConstructorInline(method, invoke, inlinee);
+ }
+
+ @Override
public boolean exceededAllowance() {
return instructionAllowance < 0;
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/ForcedInliningOracle.java b/src/main/java/com/android/tools/r8/ir/optimize/ForcedInliningOracle.java
index 86d40d9..b56dc21 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/ForcedInliningOracle.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/ForcedInliningOracle.java
@@ -61,6 +61,11 @@
}
@Override
+ public boolean isValidTarget(InvokeMethod invoke, DexEncodedMethod target, IRCode inlinee) {
+ return true;
+ }
+
+ @Override
public ListIterator<BasicBlock> updateTypeInformationIfNeeded(IRCode inlinee,
ListIterator<BasicBlock> blockIterator, BasicBlock block, BasicBlock invokeSuccessor) {
return blockIterator;
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java b/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
index b75597d..25cd969 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
@@ -285,7 +285,7 @@
return numOfInstructions;
}
- private boolean legalConstructorInline(DexEncodedMethod method,
+ boolean legalConstructorInline(DexEncodedMethod method,
InvokeMethod invoke, IRCode code) {
// In the Java VM Specification section "4.10.2.4. Instance Initialization Methods and
@@ -447,8 +447,7 @@
// Make sure constructor inlining is legal.
assert !target.isClassInitializer();
- if (target.isInstanceInitializer()
- && !legalConstructorInline(method, invoke, inlinee)) {
+ if (!strategy.isValidTarget(invoke, target, inlinee)) {
continue;
}
DexType downcast = createDowncastIfNeeded(strategy, invoke, target);
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/InliningStrategy.java b/src/main/java/com/android/tools/r8/ir/optimize/InliningStrategy.java
index 7b88b38..2443d11 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/InliningStrategy.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/InliningStrategy.java
@@ -18,6 +18,8 @@
void ensureMethodProcessed(DexEncodedMethod target, IRCode inlinee);
+ boolean isValidTarget(InvokeMethod invoke, DexEncodedMethod target, IRCode inlinee);
+
ListIterator<BasicBlock> updateTypeInformationIfNeeded(IRCode inlinee,
ListIterator<BasicBlock> blockIterator, BasicBlock block, BasicBlock invokeSuccessor);
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInliner.java b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInliner.java
index 9ff44eb..7bdff33 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInliner.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInliner.java
@@ -9,68 +9,75 @@
import com.android.tools.r8.graph.AppInfoWithSubtyping;
import com.android.tools.r8.graph.DexClass;
import com.android.tools.r8.graph.DexEncodedMethod;
+import com.android.tools.r8.graph.DexEncodedMethod.ClassInlinerEligibility;
import com.android.tools.r8.graph.DexField;
import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexType;
+import com.android.tools.r8.ir.code.BasicBlock;
+import com.android.tools.r8.ir.code.ConstNumber;
import com.android.tools.r8.ir.code.IRCode;
import com.android.tools.r8.ir.code.InstanceGet;
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.InvokeMethodWithReceiver;
import com.android.tools.r8.ir.code.NewInstance;
+import com.android.tools.r8.ir.code.Phi;
import com.android.tools.r8.ir.code.Value;
+import com.android.tools.r8.ir.code.ValueType;
import com.android.tools.r8.ir.optimize.Inliner.InliningInfo;
import com.android.tools.r8.ir.optimize.Inliner.Reason;
import com.google.common.collect.Streams;
-import java.util.Collections;
+import java.util.ArrayList;
import java.util.IdentityHashMap;
+import java.util.LinkedList;
import java.util.List;
import java.util.Map;
-import java.util.Set;
+import java.util.Map.Entry;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Predicate;
import java.util.stream.Collectors;
public final class ClassInliner {
private final DexItemFactory factory;
+ private final int totalMethodInstructionLimit;
private final ConcurrentHashMap<DexType, Boolean> knownClasses = new ConcurrentHashMap<>();
- private static final Map<DexField, Integer> NO_MAPPING = new IdentityHashMap<>();
-
public interface InlinerAction {
void inline(Map<InvokeMethodWithReceiver, InliningInfo> methods);
}
- public ClassInliner(DexItemFactory factory) {
+ public ClassInliner(DexItemFactory factory, int totalMethodInstructionLimit) {
this.factory = factory;
+ this.totalMethodInstructionLimit = totalMethodInstructionLimit;
}
// Process method code and inline eligible class instantiations, in short:
//
// - collect all 'new-instance' instructions in the original code. Note that class
- // inlining, if happens, mutates code and can add 'new-instance' instructions.
- // Processing them as well is possible, but does not seem to have much value.
+ // inlining, if happens, mutates code and may add new 'new-instance' instructions.
+ // Processing them as well is possible, but does not seem to bring much value.
//
// - for each 'new-instance' we check if it is eligible for inlining, i.e:
// -> the class of the new instance is 'eligible' (see computeClassEligible(...))
- // -> the instance is initialized with 'eligible' constructor (see
- // onlyInitializesFieldsWithNoOtherSideEffects flag in method's optimization
- // info); eligible constructor also defines a set of instance fields directly
- // initialized with parameter values, called field initialization mapping below
+ // -> the instance is initialized with 'eligible' constructor (see comments in
+ // CodeRewriter::identifyClassInlinerEligibility(...))
// -> has only 'eligible' uses, i.e:
- // * it is a receiver of a field read if the field is present in the
- // field initialization mapping
- // * it is a receiver of virtual or interface call with single target being
- // a method only reading fields in the current field initialization mapping
+ // * as a receiver of a field read/write for a field defined in same class
+ // as method.
+ // * as a receiver of virtual or interface call with single target being
+ // an eligible method according to identifyClassInlinerEligibility(...);
+ // NOTE: if method receiver is used as a return value, the method call
+ // should ignore return value
//
// - inline eligible 'new-instance' instructions, i.e:
- // -> force inline methods called on the instance (which may introduce additional
- // instance field reads, but only for fields present in the current field
- // initialization mapping)
- // -> replace instance field reads with appropriate values passed to the constructor
- // according to field initialization mapping
- // -> remove constructor call
+ // -> force inline methods called on the instance (including the initializer);
+ // (this may introduce additional instance field reads/writes on the receiver)
+ // -> replace instance field reads with appropriate values calculated based on
+ // fields writes
+ // -> remove the call to superclass initializer
+ // -> remove all field writes
// -> remove 'new-instance' instructions
//
// For example:
@@ -117,7 +124,6 @@
.map(Instruction::asNewInstance)
.collect(Collectors.toList());
- nextNewInstance:
for (NewInstance newInstance : newInstances) {
Value eligibleInstance = newInstance.outValue();
if (eligibleInstance == null) {
@@ -129,177 +135,281 @@
continue;
}
- // No Phi users.
- if (eligibleInstance.numberOfPhiUsers() > 0) {
+ Map<InvokeMethodWithReceiver, InliningInfo> methodCalls = checkInstanceUsersEligibility(
+ appInfo, method, isProcessedConcurrently, newInstance, eligibleInstance, eligibleClass);
+ if (methodCalls == null) {
continue;
}
- Set<Instruction> uniqueUsers = eligibleInstance.uniqueUsers();
+ if (getTotalEstimatedMethodSize(methodCalls) >= totalMethodInstructionLimit) {
+ continue;
+ }
- // Find an initializer invocation.
- InvokeDirect eligibleInitCall = null;
- Map<DexField, Integer> mappings = null;
- for (Instruction user : uniqueUsers) {
- if (!user.isInvokeDirect()) {
+ // Inline the class instance.
+ forceInlineAllMethodInvocations(inliner, methodCalls);
+ removeSuperClassInitializerAndFieldReads(code, newInstance, eligibleInstance);
+ removeFieldWrites(eligibleInstance, eligibleClass);
+ removeInstruction(newInstance);
+
+ // Restore normality.
+ code.removeAllTrivialPhis();
+ assert code.isConsistentSSA();
+ }
+ }
+
+ private Map<InvokeMethodWithReceiver, InliningInfo> checkInstanceUsersEligibility(
+ AppInfoWithSubtyping appInfo, DexEncodedMethod method,
+ Predicate<DexEncodedMethod> isProcessedConcurrently,
+ NewInstance newInstanceInsn, Value receiver, DexType clazz) {
+
+ // No Phi users.
+ if (receiver.numberOfPhiUsers() > 0) {
+ return null; // Not eligible.
+ }
+
+ Map<InvokeMethodWithReceiver, InliningInfo> methodCalls = new IdentityHashMap<>();
+
+ for (Instruction user : receiver.uniqueUsers()) {
+ // Field read/write.
+ if (user.isInstanceGet() ||
+ (user.isInstancePut() && user.asInstancePut().value() != receiver)) {
+ if (user.asFieldInstruction().getField().clazz == newInstanceInsn.clazz) {
+ // Eligible field read or write. Note: as long as we consider only classes eligible
+ // if they directly extend java.lang.Object we don't need to check if the field
+ // really exists in the class.
continue;
}
+ return null; // Not eligible.
+ }
- InvokeDirect candidate = user.asInvokeDirect();
- DexMethod candidateInit = candidate.getInvokedMethod();
- if (factory.isConstructor(candidateInit) &&
- candidate.inValues().lastIndexOf(eligibleInstance) == 0) {
+ // Eligible constructor call.
+ if (user.isInvokeDirect()) {
+ InliningInfo inliningInfo = isEligibleConstructorCall(appInfo, method,
+ user.asInvokeDirect(), receiver, clazz, isProcessedConcurrently);
+ if (inliningInfo != null) {
+ methodCalls.put(user.asInvokeDirect(), inliningInfo);
+ continue;
+ }
+ return null; // Not eligible.
+ }
- if (candidateInit.holder != eligibleClass) {
- // Inlined constructor call? We won't get field initialization mapping in this
- // case, but since we only support eligible classes extending java.lang.Object,
- // it's safe to assume an empty mapping.
- if (candidateInit.holder == factory.objectType) {
- mappings = Collections.emptyMap();
- }
+ // Eligible virtual method call.
+ if (user.isInvokeVirtual() || user.isInvokeInterface()) {
+ InliningInfo inliningInfo = isEligibleMethodCall(
+ appInfo, method, user.asInvokeMethodWithReceiver(),
+ receiver, clazz, isProcessedConcurrently);
+ if (inliningInfo != null) {
+ methodCalls.put(user.asInvokeMethodWithReceiver(), inliningInfo);
+ continue;
+ }
+ return null; // Not eligible.
+ }
- } else {
- // Is it a call to an *eligible* constructor?
- mappings = getConstructorFieldMappings(appInfo, candidateInit, isProcessedConcurrently);
- }
+ return null; // Not eligible.
+ }
+ return methodCalls;
+ }
- eligibleInitCall = candidate;
+ // Remove call to superclass initializer, replace field reads with appropriate
+ // values, insert phis when needed.
+ private void removeSuperClassInitializerAndFieldReads(
+ IRCode code, NewInstance newInstance, Value eligibleInstance) {
+ Map<DexField, FieldValueHelper> fieldHelpers = new IdentityHashMap<>();
+ for (Instruction user : eligibleInstance.uniqueUsers()) {
+ // Remove the call to superclass constructor.
+ if (user.isInvokeDirect() &&
+ user.asInvokeDirect().getInvokedMethod() == factory.objectMethods.constructor) {
+ removeInstruction(user);
+ continue;
+ }
+
+ if (user.isInstanceGet()) {
+ // Replace a field read with appropriate value.
+ replaceFieldRead(code, newInstance, user.asInstanceGet(), fieldHelpers);
+ continue;
+ }
+
+ if (user.isInstancePut()) {
+ // Skip in this iteration since these instructions are needed to
+ // properly calculate what value should field reads be replaced with.
+ continue;
+ }
+
+ throw new Unreachable("Unexpected usage left after method inlining: " + user);
+ }
+ }
+
+ private void removeFieldWrites(Value receiver, DexType clazz) {
+ for (Instruction user : receiver.uniqueUsers()) {
+ if (!user.isInstancePut()) {
+ throw new Unreachable("Unexpected usage left after field reads removed: " + user);
+ }
+ if (user.asInstancePut().getField().clazz != clazz) {
+ throw new Unreachable("Unexpected field write left after field reads removed: " + user);
+ }
+ removeInstruction(user);
+ }
+ }
+
+ private int getTotalEstimatedMethodSize(Map<InvokeMethodWithReceiver, InliningInfo> methodCalls) {
+ int totalSize = 0;
+ for (InliningInfo info : methodCalls.values()) {
+ totalSize += info.target.getCode().estimatedSizeForInlining();
+ }
+ return totalSize;
+ }
+
+ private void replaceFieldRead(IRCode code, NewInstance newInstance,
+ InstanceGet fieldRead, Map<DexField, FieldValueHelper> fieldHelpers) {
+
+ Value value = fieldRead.outValue();
+ if (value != null) {
+ FieldValueHelper helper = fieldHelpers.computeIfAbsent(
+ fieldRead.getField(), field -> new FieldValueHelper(field, code, newInstance));
+ Value newValue = helper.getValueForFieldRead(fieldRead.getBlock(), fieldRead);
+ value.replaceUsers(newValue);
+ for (FieldValueHelper fieldValueHelper : fieldHelpers.values()) {
+ fieldValueHelper.replaceValue(value, newValue);
+ }
+ assert value.numberOfAllUsers() == 0;
+ }
+ removeInstruction(fieldRead);
+ }
+
+ // Describes and caches what values are supposed to be used instead of field reads.
+ private static final class FieldValueHelper {
+ final DexField field;
+ final IRCode code;
+ final NewInstance newInstance;
+
+ private Value defaultValue = null;
+ private final Map<BasicBlock, Value> ins = new IdentityHashMap<>();
+ private final Map<BasicBlock, Value> outs = new IdentityHashMap<>();
+
+ private FieldValueHelper(DexField field, IRCode code, NewInstance newInstance) {
+ this.field = field;
+ this.code = code;
+ this.newInstance = newInstance;
+ }
+
+ void replaceValue(Value oldValue, Value newValue) {
+ for (Entry<BasicBlock, Value> entry : ins.entrySet()) {
+ if (entry.getValue() == oldValue) {
+ entry.setValue(newValue);
+ }
+ }
+ for (Entry<BasicBlock, Value> entry : outs.entrySet()) {
+ if (entry.getValue() == oldValue) {
+ entry.setValue(newValue);
+ }
+ }
+ }
+
+ Value getValueForFieldRead(BasicBlock block, Instruction valueUser) {
+ assert valueUser != null;
+ Value value = getValueDefinedInTheBlock(block, valueUser);
+ return value != null ? value : getOrCreateInValue(block);
+ }
+
+ private Value getOrCreateOutValue(BasicBlock block) {
+ Value value = outs.get(block);
+ if (value != null) {
+ return value;
+ }
+
+ value = getValueDefinedInTheBlock(block, null);
+ if (value == null) {
+ // No value defined in the block.
+ value = getOrCreateInValue(block);
+ }
+
+ assert value != null;
+ outs.put(block, value);
+ return value;
+ }
+
+ private Value getOrCreateInValue(BasicBlock block) {
+ Value value = ins.get(block);
+ if (value != null) {
+ return value;
+ }
+
+ List<BasicBlock> predecessors = block.getPredecessors();
+ if (predecessors.size() == 1) {
+ value = getOrCreateOutValue(predecessors.get(0));
+ ins.put(block, value);
+ } else {
+ // Create phi, add it to the block, cache in ins map for future use.
+ Phi phi = new Phi(code.valueNumberGenerator.next(),
+ block, ValueType.fromDexType(field.type), null);
+ ins.put(block, phi);
+
+ List<Value> operands = new ArrayList<>();
+ for (BasicBlock predecessor : block.getPredecessors()) {
+ operands.add(getOrCreateOutValue(predecessor));
+ }
+ // Add phi, but don't remove trivial phis; since we cache the phi
+ // we just created for future use we should delay removing trivial
+ // phis until we are done with replacing fields reads.
+ phi.addOperands(operands, false);
+ value = phi;
+ }
+
+ assert value != null;
+ return value;
+ }
+
+ private Value getValueDefinedInTheBlock(BasicBlock block, Instruction stopAt) {
+ InstructionListIterator iterator = stopAt == null ?
+ block.listIterator(block.getInstructions().size()) : block.listIterator(stopAt);
+
+ Instruction valueProducingInsn = null;
+ while (iterator.hasPrevious()) {
+ Instruction instruction = iterator.previous();
+ assert instruction != null;
+
+ if (instruction == newInstance ||
+ (instruction.isInstancePut() &&
+ instruction.asInstancePut().getField() == field &&
+ instruction.asInstancePut().object() == newInstance.outValue())) {
+ valueProducingInsn = instruction;
break;
}
}
- if (mappings == null) {
- continue;
+ if (valueProducingInsn == null) {
+ return null;
+ }
+ if (valueProducingInsn.isInstancePut()) {
+ return valueProducingInsn.asInstancePut().value();
}
- // Check all regular users.
- Map<InvokeMethodWithReceiver, InliningInfo> methodCalls = new IdentityHashMap<>();
-
- for (Instruction user : uniqueUsers) {
- if (user == eligibleInitCall) {
- continue /* next user */;
- }
-
- if (user.isInstanceGet()) {
- InstanceGet instanceGet = user.asInstanceGet();
- if (mappings.containsKey(instanceGet.getField())) {
- continue /* next user */;
- }
-
- // Not replaceable field read.
- continue nextNewInstance;
- }
-
- if (user.isInvokeVirtual() || user.isInvokeInterface()) {
- InvokeMethodWithReceiver invoke = user.asInvokeMethodWithReceiver();
- if (invoke.inValues().lastIndexOf(eligibleInstance) > 0) {
- continue nextNewInstance; // Instance must only be passes as a receiver.
- }
-
- DexEncodedMethod singleTarget =
- findSingleTarget(appInfo, invoke, eligibleClass);
- if (singleTarget == null) {
- continue nextNewInstance;
- }
- if (isProcessedConcurrently.test(singleTarget)) {
- continue nextNewInstance;
- }
- if (method == singleTarget) {
- continue nextNewInstance; // Don't inline itself.
- }
-
- if (!singleTarget.getOptimizationInfo()
- .isReceiverOnlyUsedForReadingFields(mappings.keySet())) {
- continue nextNewInstance; // Target must be trivial.
- }
-
- if (!singleTarget.isInliningCandidate(method, Reason.SIMPLE, appInfo)) {
- // We won't be able to inline it here.
-
- // Note that there may be some false negatives here since the method may
- // reference private fields of its class which are supposed to be replaced
- // with arguments after inlining. We should try and improve it later.
-
- // Using -allowaccessmodification mitigates this.
- continue nextNewInstance;
- }
-
- methodCalls.put(invoke, new InliningInfo(singleTarget, eligibleClass));
- continue /* next user */;
- }
-
- continue nextNewInstance; // Unsupported user.
+ assert newInstance == valueProducingInsn;
+ if (defaultValue == null) {
+ // If we met newInstance it means that default value is supposed to be used.
+ defaultValue = code.createValue(ValueType.fromDexType(field.type));
+ ConstNumber defaultValueInsn = new ConstNumber(defaultValue, 0);
+ defaultValueInsn.setPosition(newInstance.getPosition());
+ LinkedList<Instruction> instructions = block.getInstructions();
+ instructions.add(instructions.indexOf(newInstance) + 1, defaultValueInsn);
+ defaultValueInsn.setBlock(block);
}
-
- // Force-inline of method invocation if any.
- inlineAllCalls(inliner, methodCalls);
- assert assertOnlyConstructorAndFieldReads(eligibleInstance, eligibleInitCall, mappings);
-
- // Replace all field reads with arguments passed to the constructor.
- patchFieldReads(eligibleInstance, eligibleInitCall, mappings);
- assert assertOnlyConstructor(eligibleInstance, eligibleInitCall);
-
- // Remove constructor call and new-instance instructions.
- removeInstruction(eligibleInitCall);
- removeInstruction(newInstance);
- code.removeAllTrivialPhis();
+ return defaultValue;
}
}
- private void inlineAllCalls(
+ private void forceInlineAllMethodInvocations(
InlinerAction inliner, Map<InvokeMethodWithReceiver, InliningInfo> methodCalls) {
if (!methodCalls.isEmpty()) {
inliner.inline(methodCalls);
}
}
- private void patchFieldReads(
- Value instance, InvokeDirect invokeMethod, Map<DexField, Integer> mappings) {
- for (Instruction user : instance.uniqueUsers()) {
- if (!user.isInstanceGet()) {
- continue;
- }
- InstanceGet fieldRead = user.asInstanceGet();
-
- // Replace the field read with
- assert mappings.containsKey(fieldRead.getField());
- Value arg = invokeMethod.inValues().get(1 + mappings.get(fieldRead.getField()));
- assert arg != null;
- Value value = fieldRead.outValue();
- if (value != null) {
- value.replaceUsers(arg);
- assert value.numberOfAllUsers() == 0;
- }
-
- // Remove instruction.
- removeInstruction(fieldRead);
- }
- }
-
private void removeInstruction(Instruction instruction) {
instruction.inValues().forEach(v -> v.removeUser(instruction));
instruction.getBlock().removeInstruction(instruction);
}
- private boolean assertOnlyConstructorAndFieldReads(
- Value instance, InvokeDirect invokeMethod, Map<DexField, Integer> mappings) {
- for (Instruction user : instance.uniqueUsers()) {
- if (user != invokeMethod &&
- !(user.isInstanceGet() && mappings.containsKey(user.asFieldInstruction().getField()))) {
- throw new Unreachable("Not all calls are inlined!");
- }
- }
- return true;
- }
-
- private boolean assertOnlyConstructor(Value instance, InvokeDirect invokeMethod) {
- for (Instruction user : instance.uniqueUsers()) {
- if (user != invokeMethod) {
- throw new Unreachable("Not all field reads are substituted!");
- }
- }
- return true;
- }
-
private DexEncodedMethod findSingleTarget(
AppInfo appInfo, InvokeMethodWithReceiver invoke, DexType instanceType) {
@@ -322,24 +432,94 @@
return null;
}
- private Map<DexField, Integer> getConstructorFieldMappings(
- AppInfo appInfo, DexMethod init, Predicate<DexEncodedMethod> isProcessedConcurrently) {
- assert isClassEligible(appInfo, init.holder);
+ private InliningInfo isEligibleConstructorCall(
+ AppInfoWithSubtyping appInfo,
+ DexEncodedMethod method,
+ InvokeDirect initInvoke,
+ Value receiver,
+ DexType inlinedClass,
+ Predicate<DexEncodedMethod> isProcessedConcurrently) {
+
+ // Must be a constructor of the exact same class.
+ DexMethod init = initInvoke.getInvokedMethod();
+ if (!factory.isConstructor(init)) {
+ return null;
+ }
+ // Must be a constructor called on the receiver.
+ if (initInvoke.inValues().lastIndexOf(receiver) != 0) {
+ return null;
+ }
+
+ assert init.holder == inlinedClass
+ : "Inlined constructor? [invoke: " + initInvoke +
+ ", expected class: " + inlinedClass + "]";
DexEncodedMethod definition = appInfo.definitionFor(init);
- if (definition == null) {
- return NO_MAPPING;
+ if (definition == null || isProcessedConcurrently.test(definition)) {
+ return null;
}
- if (isProcessedConcurrently.test(definition)) {
- return NO_MAPPING;
+ if (!definition.isInliningCandidate(method, Reason.SIMPLE, appInfo)) {
+ // We won't be able to inline it here.
+
+ // Note that there may be some false negatives here since the method may
+ // reference private fields of its class which are supposed to be replaced
+ // with arguments after inlining. We should try and improve it later.
+
+ // Using -allowaccessmodification mitigates this.
+ return null;
}
- if (definition.accessFlags.isAbstract() || definition.accessFlags.isNative()) {
- return NO_MAPPING;
+ return definition.getOptimizationInfo().getClassInlinerEligibility() != null
+ ? new InliningInfo(definition, inlinedClass) : null;
+ }
+
+ private InliningInfo isEligibleMethodCall(
+ AppInfoWithSubtyping appInfo,
+ DexEncodedMethod method,
+ InvokeMethodWithReceiver invoke,
+ Value receiver,
+ DexType inlinedClass,
+ Predicate<DexEncodedMethod> isProcessedConcurrently) {
+
+ if (invoke.inValues().lastIndexOf(receiver) > 0) {
+ return null; // Instance passed as an argument.
}
- return definition.getOptimizationInfo().onlyInitializesFieldsWithNoOtherSideEffects();
+ DexEncodedMethod singleTarget =
+ findSingleTarget(appInfo, invoke, inlinedClass);
+ if (singleTarget == null || isProcessedConcurrently.test(singleTarget)) {
+ return null;
+ }
+ if (method == singleTarget) {
+ return null; // Don't inline itself.
+ }
+
+ ClassInlinerEligibility eligibility =
+ singleTarget.getOptimizationInfo().getClassInlinerEligibility();
+ if (eligibility == null) {
+ return null;
+ }
+
+ // If the method returns receiver and the return value is actually
+ // used in the code the method is not eligible.
+ if (eligibility.returnsReceiver &&
+ invoke.outValue() != null && invoke.outValue().numberOfAllUsers() > 0) {
+ return null;
+ }
+
+ if (!singleTarget.isInliningCandidate(method, Reason.SIMPLE, appInfo)) {
+ // We won't be able to inline it here.
+
+ // Note that there may be some false negatives here since the method may
+ // reference private fields of its class which are supposed to be replaced
+ // with arguments after inlining. We should try and improve it later.
+
+ // Using -allowaccessmodification mitigates this.
+ return null;
+ }
+
+ return new InliningInfo(singleTarget, inlinedClass);
}
private boolean isClassEligible(AppInfo appInfo, DexType clazz) {
@@ -354,7 +534,7 @@
}
// Class is eligible for this optimization. Eligibility implementation:
- // - not an abstract or interface
+ // - is not an abstract class or interface
// - directly extends java.lang.Object
// - does not declare finalizer
// - does not trigger any static initializers
diff --git a/src/main/java/com/android/tools/r8/utils/InternalOptions.java b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
index 68796ac..9a21fb4 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -101,6 +101,7 @@
public boolean enableInlining =
!Version.isDev() || System.getProperty("com.android.tools.r8.disableinlining") == null;
public boolean enableClassInlining = true;
+ public int classInliningInstructionLimit = 50;
public int inliningInstructionLimit = 5;
public boolean enableSwitchMapRemoval = true;
public final OutlineOptions outline = new OutlineOptions();
diff --git a/src/test/java/com/android/tools/r8/R8RunArtTestsTest.java b/src/test/java/com/android/tools/r8/R8RunArtTestsTest.java
index ca6feed..44eb344 100644
--- a/src/test/java/com/android/tools/r8/R8RunArtTestsTest.java
+++ b/src/test/java/com/android/tools/r8/R8RunArtTestsTest.java
@@ -1010,8 +1010,7 @@
// Test depends on exception produced for missing method or similar cases, but
// after class inlining removes class instantiations and references the exception
// is not produced.
- "042-new-instance",
- "075-verification-error"
+ "435-new-instance"
);
private static List<String> hasMissingClasses = ImmutableList.of(
@@ -1554,6 +1553,8 @@
if (disableClassInlining) {
options.enableClassInlining = false;
}
+ // Make sure we don't depend on this settings.
+ options.classInliningInstructionLimit = 10000;
options.lineNumberOptimization = LineNumberOptimization.OFF;
// Some tests actually rely on missing classes for what they test.
options.ignoreMissingClasses = hasMissingClasses;
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerTest.java b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerTest.java
index e9319d2..d36e6ad 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerTest.java
@@ -16,6 +16,11 @@
import com.android.tools.r8.VmTestRunner;
import com.android.tools.r8.code.NewInstance;
import com.android.tools.r8.graph.DexCode;
+import com.android.tools.r8.ir.optimize.classinliner.builders.BuildersTestClass;
+import com.android.tools.r8.ir.optimize.classinliner.builders.ControlFlow;
+import com.android.tools.r8.ir.optimize.classinliner.builders.Pair;
+import com.android.tools.r8.ir.optimize.classinliner.builders.PairBuilder;
+import com.android.tools.r8.ir.optimize.classinliner.builders.Tuple;
import com.android.tools.r8.ir.optimize.classinliner.trivial.ClassWithFinal;
import com.android.tools.r8.ir.optimize.classinliner.trivial.CycleReferenceAB;
import com.android.tools.r8.ir.optimize.classinliner.trivial.CycleReferenceBA;
@@ -75,8 +80,7 @@
collectNewInstanceTypes(clazz, "testConstructorMapping1"));
assertEquals(
- Collections.singleton(
- "com.android.tools.r8.ir.optimize.classinliner.trivial.ReferencedFields"),
+ Collections.emptySet(),
collectNewInstanceTypes(clazz, "testConstructorMapping2"));
assertEquals(
@@ -120,6 +124,60 @@
assertFalse(inspector.clazz(CycleReferenceBA.class).isPresent());
}
+ @Test
+ public void testBuilders() throws Exception {
+ byte[][] classes = {
+ ToolHelper.getClassAsBytes(BuildersTestClass.class),
+ ToolHelper.getClassAsBytes(BuildersTestClass.Pos.class),
+ ToolHelper.getClassAsBytes(Tuple.class),
+ ToolHelper.getClassAsBytes(Pair.class),
+ ToolHelper.getClassAsBytes(PairBuilder.class),
+ ToolHelper.getClassAsBytes(ControlFlow.class),
+ };
+ String main = BuildersTestClass.class.getCanonicalName();
+ ProcessResult javaOutput = runOnJava(main, classes);
+ assertEquals(0, javaOutput.exitCode);
+
+ AndroidApp app = runR8(buildAndroidApp(classes), BuildersTestClass.class);
+
+ DexInspector inspector = new DexInspector(app);
+ ClassSubject clazz = inspector.clazz(BuildersTestClass.class);
+
+ assertEquals(
+ Sets.newHashSet(
+ "com.android.tools.r8.ir.optimize.classinliner.builders.Pair",
+ "java.lang.StringBuilder"),
+ collectNewInstanceTypes(clazz, "testSimpleBuilder"));
+
+ // Note that Pair created instances were also inlined in the following method since
+ // we use 'System.out.println(pX.toString())', if we used 'System.out.println(pX)'
+ // as in the above method, the instance of pair would be passed to println() which
+ // would make it not eligible for inlining.
+ assertEquals(
+ Collections.singleton("java.lang.StringBuilder"),
+ collectNewInstanceTypes(clazz, "testSimpleBuilderWithMultipleBuilds"));
+
+ assertFalse(inspector.clazz(PairBuilder.class).isPresent());
+
+ assertEquals(
+ Collections.singleton("java.lang.StringBuilder"),
+ collectNewInstanceTypes(clazz, "testBuilderConstructors"));
+
+ assertFalse(inspector.clazz(Tuple.class).isPresent());
+
+ assertEquals(
+ Collections.singleton("java.lang.StringBuilder"),
+ collectNewInstanceTypes(clazz, "testWithControlFlow"));
+
+ assertFalse(inspector.clazz(ControlFlow.class).isPresent());
+
+ assertEquals(
+ Collections.emptySet(),
+ collectNewInstanceTypes(clazz, "testWithMoreControlFlow"));
+
+ assertFalse(inspector.clazz(BuildersTestClass.Pos.class).isPresent());
+ }
+
private Set<String> collectNewInstanceTypes(
ClassSubject clazz, String methodName, String... params) {
assertNotNull(clazz);
@@ -135,7 +193,11 @@
+ "-dontobfuscate\n"
+ "-allowaccessmodification";
- AndroidApp compiled = compileWithR8(app, config, o -> o.enableClassInlining = true);
+ AndroidApp compiled = compileWithR8(app, config,
+ o -> {
+ o.enableClassInlining = true;
+ o.classInliningInstructionLimit = 10000;
+ });
// Materialize file for execution.
Path generatedDexFile = temp.getRoot().toPath().resolve("classes.jar");
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/classinliner/builders/BuildersTestClass.java b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/builders/BuildersTestClass.java
new file mode 100644
index 0000000..1516e14
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/builders/BuildersTestClass.java
@@ -0,0 +1,95 @@
+// Copyright (c) 2018, 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.ir.optimize.classinliner.builders;
+
+public class BuildersTestClass {
+ private static int ID = 0;
+
+ private static int nextInt() {
+ return ID++;
+ }
+
+ private static String next() {
+ return Integer.toString(nextInt());
+ }
+
+ public static void main(String[] args) {
+ BuildersTestClass test = new BuildersTestClass();
+ test.testSimpleBuilder();
+ test.testSimpleBuilderWithMultipleBuilds();
+ test.testBuilderConstructors();
+ test.testWithControlFlow();
+ test.testWithMoreControlFlow();
+ }
+
+ private synchronized void testSimpleBuilder() {
+ System.out.println(
+ new PairBuilder<String, String>().setFirst("f-" + next()).build().toString());
+ testSimpleBuilder2();
+ testSimpleBuilder3();
+ }
+
+ private synchronized void testSimpleBuilder2() {
+ System.out.println(
+ new PairBuilder<String, String>().setSecond("s-" + next()).build().toString());
+ }
+
+ private synchronized void testSimpleBuilder3() {
+ System.out.println(new PairBuilder<String, String>()
+ .setFirst("f-" + next()).setSecond("s-" + next()).build().toString());
+ }
+
+ private synchronized void testSimpleBuilderWithMultipleBuilds() {
+ PairBuilder<String, String> builder = new PairBuilder<>();
+ Pair p1 = builder.build();
+ System.out.println(p1.toString());
+ builder.setFirst("f-" + next());
+ Pair p2 = builder.build();
+ System.out.println(p2.toString());
+ builder.setSecond("s-" + next());
+ Pair p3 = builder.build();
+ System.out.println(p3.toString());
+ }
+
+ private synchronized void testBuilderConstructors() {
+ System.out.println(new Tuple().toString());
+ System.out.println(new Tuple(true, (byte) 77, (short) 9977, '#', 42,
+ 987654321123456789L, -12.34f, 43210.98765, "s-" + next() + "-s").toString());
+ }
+
+ private synchronized void testWithControlFlow() {
+ ControlFlow flow = new ControlFlow(-1, 2, 7);
+ for (int k = 0; k < 25; k++) {
+ if (k % 3 == 0) {
+ flow.foo(k);
+ } else if (k % 3 == 1) {
+ flow.bar(nextInt(), nextInt(), nextInt(), nextInt());
+ }
+ }
+ System.out.println("flow = " + flow.toString());
+ }
+
+ private synchronized void testWithMoreControlFlow() {
+ String str = "1234567890";
+ Pos pos = new Pos();
+ while (pos.y < str.length()) {
+ pos.x = pos.y;
+ pos.y = pos.x;
+
+ if (str.charAt(pos.x) != '*') {
+ if ('0' <= str.charAt(pos.y) && str.charAt(pos.y) <= '9') {
+ while (pos.y < str.length() && '0' <= str.charAt(pos.y) && str.charAt(pos.y) <= '9') {
+ pos.y++;
+ }
+ }
+ }
+ }
+ }
+
+ public static class Pos {
+ public int x = 0;
+ public int y = 0;
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/classinliner/builders/ControlFlow.java b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/builders/ControlFlow.java
new file mode 100644
index 0000000..7b95dc1
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/builders/ControlFlow.java
@@ -0,0 +1,54 @@
+// Copyright (c) 2018, 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.ir.optimize.classinliner.builders;
+
+public class ControlFlow {
+ int a;
+ int b;
+ int c = 1234;
+ int d;
+ String s = ">";
+
+ ControlFlow(int b, int c, int d) {
+ this.s += this.a++ + ">";
+ this.s += this.b + ">";
+ this.b = b;
+ this.s += this.b + ">";
+ this.s += this.c + ">";
+ this.c += c;
+ this.s += this.c + ">";
+ this.s += (this.d = d) + ">";
+ }
+
+ public void foo(int count) {
+ for (int i = 0; i < count; i++) {
+ switch (i % 4) {
+ case 0:
+ this.s += ++this.a + ">";
+ break;
+ case 1:
+ this.c += this.b;
+ this.s += this.c + ">";
+ break;
+ case 2:
+ this.d += this.d++ + this.c++ + this.b++ + this.a++;
+ this.s += this.d + ">";
+ break;
+ }
+ }
+ }
+
+ public void bar(int a, int b, int c, int d) {
+ this.a += a;
+ this.b += b;
+ this.c += c;
+ this.d += d;
+ }
+
+ @Override
+ public String toString() {
+ return s;
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/classinliner/builders/Pair.java b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/builders/Pair.java
new file mode 100644
index 0000000..af45cab
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/builders/Pair.java
@@ -0,0 +1,22 @@
+// Copyright (c) 2018, 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.ir.optimize.classinliner.builders;
+
+public class Pair<F, S> {
+ public final F first;
+ public final S second;
+
+ public Pair(F first, S second) {
+ this.first = first;
+ this.second = second;
+ }
+
+ @Override
+ public String toString() {
+ return "Pair(" +
+ (first == null ? "<null>" : first) + ", " +
+ (second == null ? "<null>" : second) + ")";
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/classinliner/builders/PairBuilder.java b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/builders/PairBuilder.java
new file mode 100644
index 0000000..0c80c53
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/builders/PairBuilder.java
@@ -0,0 +1,29 @@
+// Copyright (c) 2018, 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.ir.optimize.classinliner.builders;
+
+public class PairBuilder<F, S> {
+ public F first;
+ public S second = null;
+
+ public PairBuilder<F, S> setFirst(F first) {
+ System.out.println("[before] first = " + this.first);
+ this.first = first;
+ System.out.println("[after] first = " + this.first);
+ return this;
+ }
+
+ public PairBuilder<F, S> setSecond(S second) {
+ System.out.println("[before] second = " + this.second);
+ this.second = second;
+ System.out.println("[after] second = " + this.second);
+ return this;
+ }
+
+ public Pair<F, S> build() {
+ return new Pair<>(first, second);
+ }
+}
+
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/classinliner/builders/Tuple.java b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/builders/Tuple.java
new file mode 100644
index 0000000..3a77cc2
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/builders/Tuple.java
@@ -0,0 +1,39 @@
+// Copyright (c) 2018, 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.ir.optimize.classinliner.builders;
+
+public class Tuple {
+ public boolean z;
+ public byte b;
+ public short s;
+ public char c;
+ public int i;
+ public long l;
+ public float f;
+ public double d;
+ public Object o;
+
+ Tuple() {
+ }
+
+ Tuple(boolean z, byte b, short s, char c, int i, long l, float f, double d, Object o) {
+ this.z = z;
+ this.b = b;
+ this.s = s;
+ this.c = c;
+ this.i = i;
+ this.l = l;
+ this.f = f;
+ this.d = d;
+ this.o = o;
+ }
+
+ @Override
+ public String toString() {
+ return "Tuple1(" + z + ", " + b + ", " + s + ", " +
+ ((int) c) + ", " + i + ", " + l + ", " + f + ", " +
+ d + ", " + (o == null ? "<null>" : o) + ")";
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/kotlin/R8KotlinDataClassTest.java b/src/test/java/com/android/tools/r8/kotlin/R8KotlinDataClassTest.java
index c8c8aec..73d1e50 100644
--- a/src/test/java/com/android/tools/r8/kotlin/R8KotlinDataClassTest.java
+++ b/src/test/java/com/android/tools/r8/kotlin/R8KotlinDataClassTest.java
@@ -10,7 +10,9 @@
import com.android.tools.r8.utils.DexInspector;
import com.android.tools.r8.utils.DexInspector.ClassSubject;
import com.android.tools.r8.utils.DexInspector.MethodSubject;
+import com.android.tools.r8.utils.InternalOptions;
import java.util.Collections;
+import java.util.function.Consumer;
import org.junit.Test;
public class R8KotlinDataClassTest extends AbstractR8KotlinTestBase {
@@ -33,13 +35,15 @@
private static final MethodSignature COPY_DEFAULT_METHOD =
TEST_DATA_CLASS.getCopyDefaultSignature();
+ private Consumer<InternalOptions> disableClassInliner = o -> o.enableClassInlining = false;
+
@Test
public void test_dataclass_gettersOnly() throws Exception {
final String mainClassName = "dataclass.MainGettersOnlyKt";
final MethodSignature testMethodSignature =
new MethodSignature("testDataClassGetters", "void", Collections.emptyList());
final String extraRules = keepClassMethod(mainClassName, testMethodSignature);
- runTest("dataclass", mainClassName, extraRules, (app) -> {
+ runTest("dataclass", mainClassName, extraRules, disableClassInliner, (app) -> {
DexInspector dexInspector = new DexInspector(app);
ClassSubject dataClass = checkClassIsKept(dexInspector, TEST_DATA_CLASS.getClassName());
@@ -74,7 +78,7 @@
final MethodSignature testMethodSignature =
new MethodSignature("testAllDataClassComponentFunctions", "void", Collections.emptyList());
final String extraRules = keepClassMethod(mainClassName, testMethodSignature);
- runTest("dataclass", mainClassName, extraRules, (app) -> {
+ runTest("dataclass", mainClassName, extraRules, disableClassInliner, (app) -> {
DexInspector dexInspector = new DexInspector(app);
ClassSubject dataClass = checkClassIsKept(dexInspector, TEST_DATA_CLASS.getClassName());
@@ -109,7 +113,7 @@
final MethodSignature testMethodSignature =
new MethodSignature("testSomeDataClassComponentFunctions", "void", Collections.emptyList());
final String extraRules = keepClassMethod(mainClassName, testMethodSignature);
- runTest("dataclass", mainClassName, extraRules, (app) -> {
+ runTest("dataclass", mainClassName, extraRules, disableClassInliner, (app) -> {
DexInspector dexInspector = new DexInspector(app);
ClassSubject dataClass = checkClassIsKept(dexInspector, TEST_DATA_CLASS.getClassName());
@@ -144,7 +148,7 @@
final MethodSignature testMethodSignature =
new MethodSignature("testDataClassCopy", "void", Collections.emptyList());
final String extraRules = keepClassMethod(mainClassName, testMethodSignature);
- runTest("dataclass", mainClassName, extraRules, (app) -> {
+ runTest("dataclass", mainClassName, extraRules, disableClassInliner, (app) -> {
DexInspector dexInspector = new DexInspector(app);
ClassSubject dataClass = checkClassIsKept(dexInspector, TEST_DATA_CLASS.getClassName());
@@ -159,7 +163,7 @@
final MethodSignature testMethodSignature =
new MethodSignature("testDataClassCopyWithDefault", "void", Collections.emptyList());
final String extraRules = keepClassMethod(mainClassName, testMethodSignature);
- runTest("dataclass", mainClassName, extraRules, (app) -> {
+ runTest("dataclass", mainClassName, extraRules, disableClassInliner, (app) -> {
DexInspector dexInspector = new DexInspector(app);
ClassSubject dataClass = checkClassIsKept(dexInspector, TEST_DATA_CLASS.getClassName());
