Merge "Rewrite method handles via lense lookup results."
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 362fc2c..83cb824 100644
--- a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
+++ b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
@@ -47,6 +47,7 @@
import com.android.tools.r8.utils.InternalOptions;
import com.google.common.collect.ImmutableList;
import java.util.Arrays;
+import java.util.BitSet;
import java.util.Collections;
import java.util.List;
import java.util.function.Consumer;
@@ -677,6 +678,7 @@
private ClassInlinerEligibility classInlinerEligibility = null;
private TrivialInitializer trivialInitializerInfo = null;
private ParameterUsagesInfo parametersUsages = null;
+ private BitSet kotlinNotNullParamHints = null;
private OptimizationInfo() {
// Intentionally left empty.
@@ -700,6 +702,14 @@
return parametersUsages == null ? null : parametersUsages.getParameterUsage(parameter);
}
+ public BitSet getKotlinNotNullParamHints() {
+ return kotlinNotNullParamHints;
+ }
+
+ public void setKotlinNotNullParamHints(BitSet hints) {
+ this.kotlinNotNullParamHints = hints;
+ }
+
public boolean returnsArgument() {
return returnedArgument != -1;
}
@@ -843,6 +853,10 @@
ensureMutableOI().setParameterUsages(parametersUsages);
}
+ synchronized public void setKotlinNotNullParamHints(BitSet hints) {
+ ensureMutableOI().setKotlinNotNullParamHints(hints);
+ }
+
synchronized public void setTrivialInitializer(TrivialInitializer info) {
ensureMutableOI().setTrivialInitializer(info);
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/CheckCast.java b/src/main/java/com/android/tools/r8/ir/code/CheckCast.java
index 1a43128..51e1891 100644
--- a/src/main/java/com/android/tools/r8/ir/code/CheckCast.java
+++ b/src/main/java/com/android/tools/r8/ir/code/CheckCast.java
@@ -35,6 +35,11 @@
this.type = type;
}
+ @Override
+ boolean computeNeverNull() {
+ return object().isNeverNull();
+ }
+
public DexType getType() {
return type;
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/Instruction.java b/src/main/java/com/android/tools/r8/ir/code/Instruction.java
index 5d37ea6..62e518d 100644
--- a/src/main/java/com/android/tools/r8/ir/code/Instruction.java
+++ b/src/main/java/com/android/tools/r8/ir/code/Instruction.java
@@ -68,6 +68,12 @@
this.position = position;
}
+ boolean computeNeverNull() {
+ // By default just return the flag from the out value since it never changes.
+ assert outValue != null;
+ return outValue.isNeverNull();
+ }
+
public final void forceSetPosition(Position position) {
this.position = position;
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/Move.java b/src/main/java/com/android/tools/r8/ir/code/Move.java
index f27153d..b095bb4 100644
--- a/src/main/java/com/android/tools/r8/ir/code/Move.java
+++ b/src/main/java/com/android/tools/r8/ir/code/Move.java
@@ -29,6 +29,11 @@
}
}
+ @Override
+ boolean computeNeverNull() {
+ return src().isNeverNull();
+ }
+
public Value dest() {
return outValue;
}
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 51850a0..ed09ee7 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
@@ -57,7 +57,6 @@
// period of time to break cycles. When the cycle has been resolved they are completed
// exactly once by adding the operands.
assert operands.isEmpty();
- boolean canBeNull = false;
if (block.getPredecessors().size() == 0) {
throwUndefinedValueError();
}
@@ -66,13 +65,10 @@
// Since this read has been delayed we must provide the local info for the value.
Value operand = builder.readRegister(register, type, pred, edgeType, getLocalInfo());
operand.constrainType(type);
- canBeNull |= operand.canBeNull();
appendOperand(operand);
}
- if (!canBeNull) {
- markNeverNull();
- }
removeTrivialPhi();
+ recomputeNeverNull();
}
public void addOperands(List<Value> operands) {
@@ -84,20 +80,24 @@
// period of time to break cycles. When the cycle has been resolved they are completed
// exactly once by adding the operands.
assert this.operands.isEmpty();
- boolean canBeNull = false;
if (operands.size() == 0) {
throwUndefinedValueError();
}
for (Value operand : operands) {
- canBeNull |= operand.canBeNull();
appendOperand(operand);
}
- if (!canBeNull) {
- markNeverNull();
- }
if (removeTrivialPhi) {
removeTrivialPhi();
}
+ recomputeNeverNull();
+ }
+
+ // Implementation assumes that canBeNull may change to neverNull, but
+ // not other way around. This will need to be revised later.
+ void recomputeNeverNull() {
+ if (canBeNull() && operands.stream().allMatch(Value::isNeverNull)) {
+ markNeverNull();
+ }
}
public void addDebugValue(Value value) {
@@ -131,7 +131,10 @@
public void removeOperand(int index) {
operands.get(index).removePhiUser(this);
- operands.remove(index);
+ Value value = operands.remove(index);
+ if (value.canBeNull()) {
+ recomputeNeverNull();
+ }
}
public void removeOperandsByIndex(List<Integer> operandsToRemove) {
@@ -147,6 +150,7 @@
current = i + 1;
}
operands.addAll(copy.subList(current, copy.size()));
+ recomputeNeverNull();
}
public void replaceOperandAt(int predIndex, Value newValue) {
@@ -154,6 +158,9 @@
operands.set(predIndex, newValue);
newValue.addPhiUser(this);
current.removePhiUser(this);
+ if (current.canBeNull() && newValue.isNeverNull()) {
+ recomputeNeverNull();
+ }
}
void replaceOperand(Value current, Value newValue) {
@@ -163,6 +170,9 @@
newValue.addPhiUser(this);
}
}
+ if (current.canBeNull() && newValue.isNeverNull()) {
+ recomputeNeverNull();
+ }
}
void replaceDebugValue(Value current, Value newValue) {
diff --git a/src/main/java/com/android/tools/r8/ir/code/Value.java b/src/main/java/com/android/tools/r8/ir/code/Value.java
index 05dd801..815a38f 100644
--- a/src/main/java/com/android/tools/r8/ir/code/Value.java
+++ b/src/main/java/com/android/tools/r8/ir/code/Value.java
@@ -628,6 +628,17 @@
public void markNeverNull() {
assert !neverNull;
neverNull = true;
+
+ // Propagate never null flag change.
+ for (Instruction user : users) {
+ Value value = user.outValue;
+ if (value != null && value.canBeNull() && user.computeNeverNull()) {
+ value.markNeverNull();
+ }
+ }
+ for (Phi phi : phiUsers) {
+ phi.recomputeNeverNull();
+ }
}
/**
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 479af89..dce8aa6 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
@@ -65,6 +65,7 @@
import com.google.common.collect.ArrayListMultimap;
import com.google.common.collect.ListMultimap;
import java.util.ArrayList;
+import java.util.BitSet;
import java.util.Collection;
import java.util.HashSet;
import java.util.List;
@@ -630,6 +631,11 @@
printC1VisualizerHeader(method);
printMethod(code, "Initial IR (SSA)");
+ if (method.getCode() != null && method.getCode().isJarCode() &&
+ appInfo.definitionFor(method.method.holder).hasKotlinInfo()) {
+ computeKotlinNotNullParamHints(feedback, method, code);
+ }
+
if (options.canHaveArtStringNewInitBug()) {
CodeRewriter.ensureDirectStringNewToInit(code);
}
@@ -655,7 +661,8 @@
}
if (memberValuePropagation != null) {
- memberValuePropagation.rewriteWithConstantValues(code, method.method.holder);
+ memberValuePropagation.rewriteWithConstantValues(
+ code, method.method.holder, isProcessedConcurrently);
}
if (options.enableSwitchMapRemoval && appInfo.hasLiveness()) {
codeRewriter.removeSwitchMaps(code);
@@ -779,6 +786,36 @@
finalizeIR(method, code, feedback);
}
+ private void computeKotlinNotNullParamHints(
+ OptimizationFeedback feedback, DexEncodedMethod method, IRCode code) {
+ // Try to infer Kotlin non-null parameter check to use it as a hint.
+ List<Value> arguments = code.collectArguments(true);
+ BitSet paramsCheckedForNull = new BitSet();
+ DexMethod checkParameterIsNotNull =
+ code.options.itemFactory.kotlin.intrinsics.checkParameterIsNotNull;
+ for (int index = 0; index < arguments.size(); index++) {
+ Value argument = arguments.get(index);
+ for (Instruction user : argument.uniqueUsers()) {
+ // To enforce parameter non-null requirement Kotlin uses intrinsic:
+ // kotlin.jvm.internal.Intrinsics.checkParameterIsNotNull(param, message)
+ //
+ // with the following we simply look if the parameter is ever passed
+ // to the mentioned intrinsic as the first argument. We do it only for
+ // code coming from Java classfile, since after the method is rewritten
+ // by R8 this call gets inlined.
+ if (!user.isInvokeStatic() ||
+ user.asInvokeMethod().getInvokedMethod() != checkParameterIsNotNull ||
+ user.inValues().indexOf(argument) != 0) {
+ continue;
+ }
+ paramsCheckedForNull.set(index);
+ }
+ }
+ if (paramsCheckedForNull.length() > 0) {
+ feedback.setKotlinNotNullParamHints(method, paramsCheckedForNull);
+ }
+ }
+
private void finalizeIR(DexEncodedMethod method, IRCode code, OptimizationFeedback feedback) {
code.traceBlocks();
if (options.isGeneratingClassFiles()) {
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 7ed34ba..ad6d66d 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
@@ -9,6 +9,7 @@
import com.android.tools.r8.graph.DexEncodedMethod.ParameterUsagesInfo;
import com.android.tools.r8.graph.DexEncodedMethod.TrivialInitializer;
import com.android.tools.r8.ir.optimize.Inliner.Constraint;
+import java.util.BitSet;
public interface OptimizationFeedback {
void methodReturnsArgument(DexEncodedMethod method, int argument);
@@ -21,4 +22,5 @@
void setClassInlinerEligibility(DexEncodedMethod method, ClassInlinerEligibility eligibility);
void setTrivialInitializer(DexEncodedMethod method, TrivialInitializer info);
void setParameterUsages(DexEncodedMethod method, ParameterUsagesInfo parameterUsagesInfo);
+ void setKotlinNotNullParamHints(DexEncodedMethod method, BitSet hints);
}
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 44588ab..326fa03 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
@@ -9,6 +9,7 @@
import com.android.tools.r8.graph.DexEncodedMethod.ParameterUsagesInfo;
import com.android.tools.r8.graph.DexEncodedMethod.TrivialInitializer;
import com.android.tools.r8.ir.optimize.Inliner.Constraint;
+import java.util.BitSet;
public class OptimizationFeedbackDirect implements OptimizationFeedback {
@@ -62,4 +63,9 @@
public void setParameterUsages(DexEncodedMethod method, ParameterUsagesInfo parameterUsagesInfo) {
method.setParameterUsages(parameterUsagesInfo);
}
+
+ @Override
+ public void setKotlinNotNullParamHints(DexEncodedMethod method, BitSet hints) {
+ method.setKotlinNotNullParamHints(hints);
+ }
}
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 c2d6ede..359c90d 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
@@ -9,6 +9,7 @@
import com.android.tools.r8.graph.DexEncodedMethod.ParameterUsagesInfo;
import com.android.tools.r8.graph.DexEncodedMethod.TrivialInitializer;
import com.android.tools.r8.ir.optimize.Inliner.Constraint;
+import java.util.BitSet;
public class OptimizationFeedbackIgnore implements OptimizationFeedback {
@@ -45,4 +46,8 @@
@Override
public void setParameterUsages(DexEncodedMethod method, ParameterUsagesInfo parameterUsagesInfo) {
}
+
+ @Override
+ public void setKotlinNotNullParamHints(DexEncodedMethod method, BitSet hints) {
+ }
}
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 7ea7f57..22e3c32 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
@@ -755,7 +755,9 @@
//
// (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
+ // (3) as a receiver of a call to the superclass initializer. Note that we don't
+ // check what is passed to superclass initializer as arguments, only check
+ // that it is not the instance being initialized.
//
boolean instanceInitializer = method.isInstanceInitializer();
if (method.accessFlags.isNative() ||
@@ -787,23 +789,25 @@
(insn.isInstancePut() && insn.asInstancePut().object() == receiver)) {
DexField field = insn.asFieldInstruction().getField();
if (field.clazz == clazz.type && clazz.lookupInstanceField(field) != null) {
- // Since class inliner currently only supports classes directly extending
- // java.lang.Object, we don't need to worry about fields defined in superclasses.
+ // Require only accessing instance fields of the *current* class.
continue;
}
return;
}
- // If this is an instance initializer allow one call
- // to java.lang.Object.<init>() on 'this'.
- if (instanceInitializer && insn.isInvokeDirect()) {
+ // If this is an instance initializer allow one call to superclass instance initializer.
+ if (insn.isInvokeDirect()) {
InvokeDirect invokedDirect = insn.asInvokeDirect();
- if (invokedDirect.getInvokedMethod() == dexItemFactory.objectMethods.constructor &&
- invokedDirect.getReceiver() == receiver &&
- !seenSuperInitCall) {
+ DexMethod invokedMethod = invokedDirect.getInvokedMethod();
+ if (dexItemFactory.isConstructor(invokedMethod) &&
+ invokedMethod.holder == clazz.superType &&
+ invokedDirect.inValues().lastIndexOf(receiver) == 0 &&
+ !seenSuperInitCall &&
+ instanceInitializer) {
seenSuperInitCall = true;
continue;
}
+ // We don't support other direct calls yet.
return;
}
@@ -870,7 +874,7 @@
// This method defines trivial instance initializer as follows:
//
- // ** The initializer may only call the initializer of the base class, which
+ // ** The initializer may call the initializer of the base class, which
// itself must be trivial.
//
// ** java.lang.Object.<init>() is considered trivial.
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 b05fd4f..cc7f571 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
@@ -14,11 +14,14 @@
import com.android.tools.r8.ir.code.InvokeMethodWithReceiver;
import com.android.tools.r8.ir.code.InvokePolymorphic;
import com.android.tools.r8.ir.code.InvokeStatic;
+import com.android.tools.r8.ir.code.Value;
import com.android.tools.r8.ir.conversion.CallSiteInformation;
import com.android.tools.r8.ir.optimize.Inliner.InlineAction;
import com.android.tools.r8.ir.optimize.Inliner.Reason;
import com.android.tools.r8.logging.Log;
import com.android.tools.r8.utils.InternalOptions;
+import java.util.BitSet;
+import java.util.List;
import java.util.ListIterator;
import java.util.function.Predicate;
@@ -218,13 +221,33 @@
if (reason == Reason.SIMPLE) {
// If we are looking for a simple method, only inline if actually simple.
Code code = candidate.getCode();
- if (!code.estimatedSizeForInliningAtMost(inliningInstructionLimit)) {
+ int instructionLimit = computeInstructionLimit(invoke, candidate);
+ if (!code.estimatedSizeForInliningAtMost(instructionLimit)) {
return false;
}
}
return true;
}
+ private int computeInstructionLimit(InvokeMethod invoke, DexEncodedMethod candidate) {
+ int instructionLimit = inliningInstructionLimit;
+ BitSet hints = candidate.getOptimizationInfo().getKotlinNotNullParamHints();
+ if (hints != null) {
+ List<Value> arguments = invoke.inValues();
+ if (invoke.isInvokeMethodWithReceiver()) {
+ arguments = arguments.subList(1, arguments.size());
+ }
+ for (int index = 0; index < arguments.size(); index++) {
+ Value argument = arguments.get(index);
+ if (argument.isNeverNull() && hints.get(index)) {
+ // 5-4 instructions per parameter check are expected to be removed.
+ instructionLimit += 4;
+ }
+ }
+ }
+ return instructionLimit;
+ }
+
@Override
public InlineAction computeForInvokeWithReceiver(
InvokeMethodWithReceiver invoke, DexType invocationContext) {
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/MemberValuePropagation.java b/src/main/java/com/android/tools/r8/ir/optimize/MemberValuePropagation.java
index cdfa2b1..f92eb80 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/MemberValuePropagation.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/MemberValuePropagation.java
@@ -7,6 +7,8 @@
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.TrivialInitializer;
+import com.android.tools.r8.graph.DexEncodedMethod.TrivialInitializer.TrivialClassInitializer;
import com.android.tools.r8.graph.DexField;
import com.android.tools.r8.graph.DexItem;
import com.android.tools.r8.graph.DexMethod;
@@ -23,6 +25,7 @@
import com.android.tools.r8.ir.code.ValueType;
import com.android.tools.r8.shaking.Enqueuer.AppInfoWithLiveness;
import com.android.tools.r8.shaking.ProguardMemberRule;
+import java.util.function.Predicate;
public class MemberValuePropagation {
@@ -113,7 +116,8 @@
* <p>
* Also assigns value ranges to values where possible.
*/
- public void rewriteWithConstantValues(IRCode code, DexType callingContext) {
+ public void rewriteWithConstantValues(
+ IRCode code, DexType callingContext, Predicate<DexEncodedMethod> isProcessedConcurrently) {
InstructionIterator iterator = code.instructionIterator();
while (iterator.hasNext()) {
Instruction current = iterator.next();
@@ -155,7 +159,7 @@
if (!invokeReplaced && invoke.outValue() != null) {
DexEncodedMethod target = invoke.computeSingleTarget(appInfo);
if (target != null) {
- if (target.getOptimizationInfo().neverReturnsNull()) {
+ if (target.getOptimizationInfo().neverReturnsNull() && invoke.outValue().canBeNull()) {
invoke.outValue().markNeverNull();
}
if (target.getOptimizationInfo().returnsConstant()) {
@@ -182,12 +186,11 @@
} else if (current.isStaticGet()) {
StaticGet staticGet = current.asStaticGet();
DexField field = staticGet.getField();
- Instruction replacement = null;
DexEncodedField target = appInfo.lookupStaticTarget(field.getHolder(), field);
ProguardMemberRuleLookup lookup = null;
if (target != null) {
// Check if a this value is known const.
- replacement = target.valueAsConstInstruction(appInfo, staticGet.dest());
+ Instruction replacement = target.valueAsConstInstruction(appInfo, staticGet.dest());
if (replacement == null) {
lookup = lookupMemberRule(target);
if (lookup != null) {
@@ -207,6 +210,37 @@
} else {
iterator.replaceCurrentInstruction(replacement);
}
+ } else if (staticGet.dest() != null) {
+ // In case the class holder of this static field satisfying following criteria:
+ // -- cannot trigger other static initializer except for its own
+ // -- is final
+ // -- has a class initializer which is classified as trivial
+ // (see CodeRewriter::computeClassInitializerInfo) and
+ // initializes the field being accessed
+ //
+ // ... and the field itself is not pinned by keep rules (in which case it might
+ // be updated outside the class constructor, e.g. via reflections), it is safe
+ // to assume that the static-get instruction reads the value it initialized value
+ // in class initializer and is never null.
+ //
+ DexClass holderDefinition = appInfo.definitionFor(field.getHolder());
+ if (holderDefinition != null &&
+ holderDefinition.accessFlags.isFinal() &&
+ !appInfo.canTriggerStaticInitializer(field.getHolder(), true)) {
+
+ Value outValue = staticGet.dest();
+ DexEncodedMethod classInitializer = holderDefinition.getClassInitializer();
+ if (classInitializer != null && !isProcessedConcurrently.test(classInitializer)) {
+ TrivialInitializer info =
+ classInitializer.getOptimizationInfo().getTrivialInitializerInfo();
+ if (info != null &&
+ ((TrivialClassInitializer) info).field == field &&
+ !appInfo.isPinned(field) &&
+ outValue.canBeNull()) {
+ outValue.markNeverNull();
+ }
+ }
+ }
}
}
} else if (current.isStaticPut()) {
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 b28158a..219b5b1 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
@@ -16,6 +16,7 @@
import com.android.tools.r8.ir.optimize.InliningOracle;
import com.android.tools.r8.shaking.Enqueuer.AppInfoWithLiveness;
import com.google.common.collect.Streams;
+import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
@@ -127,31 +128,51 @@
.filter(insn -> insn.isNewInstance() || insn.isStaticGet())
.collect(Collectors.toList());
- for (Instruction root : roots) {
- InlineCandidateProcessor processor =
- new InlineCandidateProcessor(factory, appInfo,
- type -> isClassEligible(appInfo, type),
- isProcessedConcurrently, method, root);
+ // We loop inlining iterations until there was no inlining, but still use same set
+ // of roots to avoid infinite inlining. Looping makes possible for some roots to
+ // become eligible after other roots are inlined.
- // Assess eligibility and compute inlining of direct calls and extra methods needed.
- if (!processor.isInstanceEligible() ||
- !processor.isClassAndUsageEligible() ||
- !processor.areInstanceUsersEligible(defaultOracle)) {
- continue;
+ boolean repeat;
+ do {
+ repeat = false;
+
+ Iterator<Instruction> rootsIterator = roots.iterator();
+ while (rootsIterator.hasNext()) {
+ Instruction root = rootsIterator.next();
+ InlineCandidateProcessor processor =
+ new InlineCandidateProcessor(factory, appInfo,
+ type -> isClassEligible(appInfo, type),
+ isProcessedConcurrently, method, root);
+
+ // Assess eligibility of instance and class.
+ if (!processor.isInstanceEligible() ||
+ !processor.isClassAndUsageEligible()) {
+ // This root will never be inlined.
+ rootsIterator.remove();
+ continue;
+ }
+
+ // Assess users eligibility and compute inlining of direct calls and extra methods needed.
+ if (!processor.areInstanceUsersEligible(defaultOracle)) {
+ // This root may succeed if users change in future.
+ continue;
+ }
+
+ // Is inlining allowed.
+ if (processor.getEstimatedCombinedSizeForInlining() >= totalMethodInstructionLimit) {
+ continue;
+ }
+
+ // Inline the class instance.
+ processor.processInlining(code, inliner);
+
+ // Restore normality.
+ code.removeAllTrivialPhis();
+ assert code.isConsistentSSA();
+ rootsIterator.remove();
+ repeat = true;
}
-
- // Is inlining allowed.
- if (processor.getEstimatedCombinedSizeForInlining() >= totalMethodInstructionLimit) {
- continue;
- }
-
- // Inline the class instance.
- processor.processInlining(code, inliner);
-
- // Restore normality.
- code.removeAllTrivialPhis();
- assert code.isConsistentSSA();
- }
+ } while (repeat);
}
private boolean isClassEligible(AppInfo appInfo, DexType clazz) {
@@ -167,7 +188,6 @@
// Class is eligible for this optimization. Eligibility implementation:
// - is not an abstract class or interface
- // - directly extends java.lang.Object
// - does not declare finalizer
// - does not trigger any static initializers except for its own
private boolean computeClassEligible(AppInfo appInfo, DexType clazz) {
@@ -177,11 +197,6 @@
return false;
}
- // Must directly extend Object.
- if (definition.superType != factory.objectType) {
- return false;
- }
-
// Class must not define finalizer.
for (DexEncodedMethod method : definition.virtualMethods()) {
if (method.method.name == factory.finalizeMethodName &&
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java
index 9c553bd..4d6e3b1 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java
@@ -14,6 +14,7 @@
import com.android.tools.r8.graph.DexEncodedMethod.ParameterUsagesInfo.SingleCallOfArgumentMethod;
import com.android.tools.r8.graph.DexEncodedMethod.TrivialInitializer;
import com.android.tools.r8.graph.DexEncodedMethod.TrivialInitializer.TrivialClassInitializer;
+import com.android.tools.r8.graph.DexEncodedMethod.TrivialInitializer.TrivialInstanceInitializer;
import com.android.tools.r8.graph.DexField;
import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.graph.DexMethod;
@@ -21,6 +22,7 @@
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.If;
import com.android.tools.r8.ir.code.InstanceGet;
import com.android.tools.r8.ir.code.Instruction;
import com.android.tools.r8.ir.code.Invoke.Type;
@@ -114,6 +116,10 @@
}
if (isNewInstance()) {
+ // NOTE: if the eligible class does not directly extend java.lang.Object,
+ // we also have to guarantee that it is initialized with initializer classified as
+ // TrivialInstanceInitializer. This will be checked in areInstanceUsersEligible(...).
+
// There must be no static initializer on the class itself.
return !eligibleClassDefinition.hasClassInitializer();
}
@@ -238,6 +244,16 @@
}
}
+ // Allow some IF instructions.
+ if (user.isIf()) {
+ If ifInsn = user.asIf();
+ If.Type type = ifInsn.getType();
+ if (ifInsn.isZeroTest() && (type == If.Type.EQ || type == If.Type.NE)) {
+ // Allow ==/!= null tests, we know that the instance is a non-null value.
+ continue;
+ }
+ }
+
return false; // Not eligible.
}
return true;
@@ -256,7 +272,8 @@
replaceUsagesAsUnusedArgument(code);
forceInlineExtraMethodInvocations(inliner);
forceInlineDirectMethodInvocations(inliner);
- removeSuperClassInitializerAndFieldReads(code);
+ removeMiscUsages(code);
+ removeFieldReads(code);
removeFieldWrites();
removeInstruction(root);
}
@@ -308,19 +325,58 @@
}
}
- // Remove call to superclass initializer, replace field reads with appropriate
- // values, insert phis when needed.
- private void removeSuperClassInitializerAndFieldReads(IRCode code) {
- Map<DexField, FieldValueHelper> fieldHelpers = new IdentityHashMap<>();
+ // Remove miscellaneous users before handling field reads.
+ private void removeMiscUsages(IRCode code) {
+ boolean needToRemoveUnreachableBlocks = false;
for (Instruction user : eligibleInstance.uniqueUsers()) {
// Remove the call to superclass constructor.
if (root.isNewInstance() &&
user.isInvokeDirect() &&
- user.asInvokeDirect().getInvokedMethod() == factory.objectMethods.constructor) {
+ factory.isConstructor(user.asInvokeDirect().getInvokedMethod()) &&
+ user.asInvokeDirect().getInvokedMethod().holder == eligibleClassDefinition.superType) {
removeInstruction(user);
continue;
}
+ if (user.isIf()) {
+ If ifInsn = user.asIf();
+ assert ifInsn.isZeroTest()
+ : "Unexpected usage in non-zero-test IF instruction: " + user;
+ BasicBlock block = user.getBlock();
+ If.Type type = ifInsn.getType();
+ assert type == If.Type.EQ || type == If.Type.NE
+ : "Unexpected type in zero-test IF instruction: " + user;
+ BasicBlock newBlock = type == If.Type.EQ
+ ? ifInsn.fallthroughBlock() : ifInsn.getTrueTarget();
+ BasicBlock blockToRemove = type == If.Type.EQ
+ ? ifInsn.getTrueTarget() : ifInsn.fallthroughBlock();
+ assert newBlock != blockToRemove;
+
+ block.replaceSuccessor(blockToRemove, newBlock);
+ blockToRemove.removePredecessor(block);
+ assert block.exit().isGoto();
+ assert block.exit().asGoto().getTarget() == newBlock;
+ needToRemoveUnreachableBlocks = true;
+ continue;
+ }
+
+ if (user.isInstanceGet() || user.isInstancePut()) {
+ // Leave field reads/writes until next steps.
+ continue;
+ }
+
+ throw new Unreachable("Unexpected usage left after method inlining: " + user);
+ }
+
+ if (needToRemoveUnreachableBlocks) {
+ code.removeUnreachableBlocks();
+ }
+ }
+
+ // Replace field reads with appropriate values, insert phis when needed.
+ private void removeFieldReads(IRCode code) {
+ Map<DexField, FieldValueHelper> fieldHelpers = new IdentityHashMap<>();
+ for (Instruction user : eligibleInstance.uniqueUsers()) {
if (user.isInstanceGet()) {
// Replace a field read with appropriate value.
replaceFieldRead(code, user.asInstanceGet(), fieldHelpers);
@@ -385,6 +441,19 @@
return null;
}
+ // If the superclass of the initializer is NOT java.lang.Object, the super class
+ // initializer being called must be classified as TrivialInstanceInitializer.
+ //
+ // NOTE: since we already classified the class as eligible, it does not have
+ // any class initializers in superclass chain or in superinterfaces, see
+ // details in ClassInliner::computeClassEligible(...).
+ if (eligibleClassDefinition.superType != factory.objectType) {
+ TrivialInitializer info = definition.getOptimizationInfo().getTrivialInitializerInfo();
+ if (!(info instanceof TrivialInstanceInitializer)) {
+ return null;
+ }
+ }
+
if (!definition.isInliningCandidate(method, Reason.SIMPLE, appInfo)) {
// We won't be able to inline it here.
diff --git a/src/main/java/com/android/tools/r8/ir/regalloc/RegisterMoveScheduler.java b/src/main/java/com/android/tools/r8/ir/regalloc/RegisterMoveScheduler.java
index d1fe04c..031d161 100644
--- a/src/main/java/com/android/tools/r8/ir/regalloc/RegisterMoveScheduler.java
+++ b/src/main/java/com/android/tools/r8/ir/regalloc/RegisterMoveScheduler.java
@@ -14,6 +14,8 @@
import com.android.tools.r8.ir.code.Move;
import com.android.tools.r8.ir.code.Position;
import com.android.tools.r8.ir.code.Value;
+import it.unimi.dsi.fastutil.ints.IntArraySet;
+import it.unimi.dsi.fastutil.ints.IntSet;
import java.util.ArrayList;
import java.util.Deque;
import java.util.HashMap;
@@ -59,6 +61,8 @@
}
public void schedule() {
+ assert everyDestinationOnlyWrittenOnce();
+
// Worklist of moves that are ready to be inserted.
Deque<RegisterMove> worklist = new LinkedList<>();
@@ -196,4 +200,13 @@
iterator.remove();
return move;
}
+
+ private boolean everyDestinationOnlyWrittenOnce() {
+ IntSet destinations = new IntArraySet(moveSet.size());
+ for (RegisterMove move : moveSet) {
+ boolean changed = destinations.add(move.dst);
+ assert changed;
+ }
+ return true;
+ }
}
diff --git a/src/main/java/com/android/tools/r8/ir/regalloc/SpillMoveSet.java b/src/main/java/com/android/tools/r8/ir/regalloc/SpillMoveSet.java
index 1ea788d..cebcc8c 100644
--- a/src/main/java/com/android/tools/r8/ir/regalloc/SpillMoveSet.java
+++ b/src/main/java/com/android/tools/r8/ir/regalloc/SpillMoveSet.java
@@ -61,7 +61,46 @@
assert to.getSplitParent() == from.getSplitParent();
BasicBlock atEntryToBlock = blockStartMap.get(i + 1);
if (atEntryToBlock == null) {
- addInMove(i, to, from);
+ Value value = from.getValue();
+ if (value.definition != null
+ && value.definition.isMoveException()
+ && to.getStart() == value.definition.asMoveException().getNumber() + 1) {
+ // Consider the following IR code.
+ // 40: ...
+ // 42: v0 <- move-exception
+ // 44: ...
+ // ...
+ // 50: ... v0 ... // 4-bit constrained use of v0
+ //
+ // Initially, the liveness interval of v0 is [42; 50[. If the method has overlapping move-
+ // exception intervals (and the register allocator needs more than 16 registers), then the
+ // intervals of v0 will be split immediately after its definition. Therefore, v0 will have
+ // two liveness intervals: I1=[42; 43[ and I2=[43;50[.
+ //
+ // When allocating a register for the interval I2, we may need to spill an existing value v1
+ // that is currently active. As a result, we will split the liveness interval of v1 before
+ // the start of I2 (i.e., at position 43). The live intervals of v1 after the split
+ // therefore becomes J1=[x, y[, J2=[41, 43[, J3=[43, z[.
+ //
+ // If the registers assigned to J1 and J2 are different, we will create an in-move at
+ // position 43 in resolveControlFlow() (not position 41, since the first instruction of the
+ // target block is a move-exception instruction). If the the registers assigned to I1 and I2
+ // are also different, then an in-move will also be created at position 43 by the call to
+ // addSpillOrRestoreMove() in insertMoves().
+ //
+ // If the registers of I2 and J2 are the same (which is a valid assignment), then we will
+ // do parallel move scheduling for the following two in-moves:
+ // move X, reg(I2)
+ // move X, reg(J2)
+ //
+ // Therefore, there is a risk that we end up with the value that has been spilled instead of
+ // the exception object in register X at position 44. To avoid this situation, we schedule
+ // the move of the exception object (in this case, "move X, reg(I2)") as an out-move, such
+ // that it always gets inserted *after* the resolution moves of the current block.
+ addOutMove(i, to, from);
+ } else {
+ addInMove(i, to, from);
+ }
}
}
diff --git a/src/main/java/com/android/tools/r8/kotlin/Kotlin.java b/src/main/java/com/android/tools/r8/kotlin/Kotlin.java
index 0af41b1..05c7b92 100644
--- a/src/main/java/com/android/tools/r8/kotlin/Kotlin.java
+++ b/src/main/java/com/android/tools/r8/kotlin/Kotlin.java
@@ -80,6 +80,9 @@
public final DexType type = factory.createType("Lkotlin/jvm/internal/Intrinsics;");
public final DexMethod throwParameterIsNullException = factory.createMethod(type,
factory.createProto(factory.voidType, factory.stringType), "throwParameterIsNullException");
+ public final DexMethod checkParameterIsNotNull = factory.createMethod(type,
+ factory.createProto(factory.voidType, factory.objectType, factory.stringType),
+ "checkParameterIsNotNull");
public final DexMethod throwNpe = factory.createMethod(
type, factory.createProto(factory.voidType), "throwNpe");
}
diff --git a/src/test/examples/classmerging/PinnedParameterTypesTest.java b/src/test/examples/classmerging/PinnedParameterTypesTest.java
new file mode 100644
index 0000000..bdfe3b7
--- /dev/null
+++ b/src/test/examples/classmerging/PinnedParameterTypesTest.java
@@ -0,0 +1,47 @@
+// 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 classmerging;
+
+import java.lang.reflect.Method;
+
+public class PinnedParameterTypesTest {
+
+ public static void main(String[] args) throws Exception {
+ for (Method method : TestClass.class.getMethods()) {
+ if (method.getName().equals("method")) {
+ Class<?> parameterType = method.getParameterTypes()[0];
+
+ // Should print classmerging.PinnedParameterTypesTest$Interface when -keepparameternames is
+ // used.
+ System.out.println(parameterType.getName());
+
+ method.invoke(null, new InterfaceImpl());
+ break;
+ }
+ }
+ }
+
+ public interface Interface {
+
+ void foo();
+ }
+
+ public static class InterfaceImpl implements Interface {
+
+ @Override
+ public void foo() {
+ System.out.println("In InterfaceImpl.foo()");
+ }
+ }
+
+ public static class TestClass {
+
+ // This method has been kept explicitly by a keep rule. Therefore, since -keepparameternames is
+ // used, Interface must not be merged into InterfaceImpl.
+ public static void method(Interface obj) {
+ obj.foo();
+ }
+ }
+}
diff --git a/src/test/examples/classmerging/keep-rules.txt b/src/test/examples/classmerging/keep-rules.txt
index fa48747..516e39d 100644
--- a/src/test/examples/classmerging/keep-rules.txt
+++ b/src/test/examples/classmerging/keep-rules.txt
@@ -19,6 +19,12 @@
-keep public class classmerging.RewritePinnedMethodTest {
public static void main(...);
}
+-keep public class classmerging.PinnedParameterTypesTest {
+ public static void main(...);
+}
+-keep public class classmerging.PinnedParameterTypesTest$TestClass {
+ public static void method(...);
+}
-keep public class classmerging.SimpleInterfaceAccessTest {
public static void main(...);
}
diff --git a/src/test/java/com/android/tools/r8/classmerging/ClassMergingTest.java b/src/test/java/com/android/tools/r8/classmerging/ClassMergingTest.java
index 341e259..59accb3 100644
--- a/src/test/java/com/android/tools/r8/classmerging/ClassMergingTest.java
+++ b/src/test/java/com/android/tools/r8/classmerging/ClassMergingTest.java
@@ -206,6 +206,29 @@
}
@Test
+ public void testPinnedParameterTypes() throws Exception {
+ String main = "classmerging.PinnedParameterTypesTest";
+ Path[] programFiles =
+ new Path[] {
+ CF_DIR.resolve("PinnedParameterTypesTest.class"),
+ CF_DIR.resolve("PinnedParameterTypesTest$Interface.class"),
+ CF_DIR.resolve("PinnedParameterTypesTest$InterfaceImpl.class"),
+ CF_DIR.resolve("PinnedParameterTypesTest$TestClass.class")
+ };
+ Set<String> preservedClassNames =
+ ImmutableSet.of(
+ "classmerging.PinnedParameterTypesTest",
+ "classmerging.PinnedParameterTypesTest$Interface",
+ "classmerging.PinnedParameterTypesTest$InterfaceImpl",
+ "classmerging.PinnedParameterTypesTest$TestClass");
+ runTest(
+ main,
+ programFiles,
+ preservedClassNames::contains,
+ getProguardConfig(EXAMPLE_KEEP, "-keepparameternames"));
+ }
+
+ @Test
public void testSuperCallWasDetected() throws Exception {
String main = "classmerging.SuperCallRewritingTest";
Path[] programFiles =
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/CheckCastRemovalTest.java b/src/test/java/com/android/tools/r8/ir/optimize/CheckCastRemovalTest.java
index be7ee92..f894322 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/CheckCastRemovalTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/CheckCastRemovalTest.java
@@ -89,7 +89,7 @@
"-keep class B { *; }",
"-keep class C { *; }",
"-dontshrink");
- AndroidApp app = compileWithR8(builder, pgConfigs, null);
+ AndroidApp app = compileWithR8(builder, pgConfigs, opts -> opts.enableClassInlining = false);
DexEncodedMethod method = getMethod(app, CLASS_NAME, main);
assertNotNull(method);
diff --git a/src/test/java/com/android/tools/r8/kotlin/AbstractR8KotlinTestBase.java b/src/test/java/com/android/tools/r8/kotlin/AbstractR8KotlinTestBase.java
index f99934e..fd78cb7 100644
--- a/src/test/java/com/android/tools/r8/kotlin/AbstractR8KotlinTestBase.java
+++ b/src/test/java/com/android/tools/r8/kotlin/AbstractR8KotlinTestBase.java
@@ -231,7 +231,7 @@
}
// Build classpath for compilation (and java execution)
- assert classpath.isEmpty();
+ classpath.clear();
classpath.add(getKotlinJarFile(folder));
classpath.add(getJavaJarFile(folder));
classpath.addAll(extraClasspath);
diff --git a/src/test/java/com/android/tools/r8/kotlin/KotlinClassInlinerTest.java b/src/test/java/com/android/tools/r8/kotlin/KotlinClassInlinerTest.java
index 46af30d..a690429 100644
--- a/src/test/java/com/android/tools/r8/kotlin/KotlinClassInlinerTest.java
+++ b/src/test/java/com/android/tools/r8/kotlin/KotlinClassInlinerTest.java
@@ -7,7 +7,9 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+import com.android.tools.r8.ToolHelper.KotlinTargetVersion;
import com.android.tools.r8.code.NewInstance;
import com.android.tools.r8.code.SgetObject;
import com.android.tools.r8.graph.DexClass;
@@ -16,14 +18,26 @@
import com.android.tools.r8.naming.MemberNaming.MethodSignature;
import com.android.tools.r8.utils.DexInspector;
import com.android.tools.r8.utils.DexInspector.ClassSubject;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.Sets;
+import java.util.Collection;
import java.util.Set;
import java.util.function.Predicate;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import org.junit.Test;
+import org.junit.runners.Parameterized.Parameters;
public class KotlinClassInlinerTest extends AbstractR8KotlinTestBase {
+ @Parameters(name = "allowAccessModification: {0} target: {1}")
+ public static Collection<Object[]> data() {
+ ImmutableList.Builder<Object[]> builder = new ImmutableList.Builder<>();
+ for (KotlinTargetVersion targetVersion : KotlinTargetVersion.values()) {
+ builder.add(new Object[]{Boolean.TRUE, targetVersion});
+ }
+ return builder.build();
+ }
+
private static boolean isLambda(DexClass clazz) {
return !clazz.type.getPackageDescriptor().startsWith("kotlin") &&
(isKStyleLambdaOrGroup(clazz) || isJStyleLambdaOrGroup(clazz));
@@ -49,7 +63,17 @@
@Test
public void testJStyleLambdas() throws Exception {
final String mainClassName = "class_inliner_lambda_j_style.MainKt";
- runTest("class_inliner_lambda_j_style", mainClassName, (app) -> {
+ runTest("class_inliner_lambda_j_style", mainClassName, false, (app) -> {
+ DexInspector inspector = new DexInspector(app);
+ assertTrue(
+ inspector.clazz("class_inliner_lambda_j_style.MainKt$testStateful$1").isPresent());
+ assertTrue(
+ inspector.clazz("class_inliner_lambda_j_style.MainKt$testStateful2$1").isPresent());
+ assertTrue(
+ inspector.clazz("class_inliner_lambda_j_style.MainKt$testStateful3$1").isPresent());
+ });
+
+ runTest("class_inliner_lambda_j_style", mainClassName, true, (app) -> {
DexInspector inspector = new DexInspector(app);
Predicate<DexType> lambdaCheck = createLambdaCheck(inspector);
ClassSubject clazz = inspector.clazz(mainClassName);
@@ -81,6 +105,40 @@
});
}
+ @Test
+ public void testKStyleLambdas() throws Exception {
+ final String mainClassName = "class_inliner_lambda_k_style.MainKt";
+ runTest("class_inliner_lambda_k_style", mainClassName, false, (app) -> {
+ DexInspector inspector = new DexInspector(app);
+ assertTrue(inspector.clazz(
+ "class_inliner_lambda_k_style.MainKt$testKotlinSequencesStateless$1").isPresent());
+ assertTrue(inspector.clazz(
+ "class_inliner_lambda_k_style.MainKt$testKotlinSequencesStateful$1").isPresent());
+ });
+
+ runTest("class_inliner_lambda_k_style", mainClassName, true, (app) -> {
+ DexInspector inspector = new DexInspector(app);
+ Predicate<DexType> lambdaCheck = createLambdaCheck(inspector);
+ ClassSubject clazz = inspector.clazz(mainClassName);
+
+ assertEquals(
+ Sets.newHashSet(),
+ collectAccessedLambdaTypes(lambdaCheck, clazz,
+ "testKotlinSequencesStateless", "kotlin.sequences.Sequence"));
+
+ assertFalse(inspector.clazz(
+ "class_inliner_lambda_k_style.MainKt$testKotlinSequencesStateless$1").isPresent());
+
+ assertEquals(
+ Sets.newHashSet(),
+ collectAccessedLambdaTypes(lambdaCheck, clazz,
+ "testKotlinSequencesStateful", "int", "int", "kotlin.sequences.Sequence"));
+
+ assertFalse(inspector.clazz(
+ "class_inliner_lambda_k_style.MainKt$testKotlinSequencesStateful$1").isPresent());
+ });
+ }
+
private Set<String> collectAccessedLambdaTypes(
Predicate<DexType> isLambdaType, ClassSubject clazz, String methodName, String... params) {
assertNotNull(clazz);
@@ -97,14 +155,13 @@
.collect(Collectors.toSet());
}
- @Override
protected void runTest(String folder, String mainClass,
- AndroidAppInspector inspector) throws Exception {
+ boolean enabled, AndroidAppInspector inspector) throws Exception {
runTest(
folder, mainClass, null,
options -> {
options.enableInlining = true;
- options.enableClassInlining = true;
+ options.enableClassInlining = enabled;
options.enableLambdaMerging = false;
}, inspector);
}
diff --git a/src/test/java/com/android/tools/r8/kotlin/KotlinLambdaMergingTest.java b/src/test/java/com/android/tools/r8/kotlin/KotlinLambdaMergingTest.java
index ee9065f..08d43ae 100644
--- a/src/test/java/com/android/tools/r8/kotlin/KotlinLambdaMergingTest.java
+++ b/src/test/java/com/android/tools/r8/kotlin/KotlinLambdaMergingTest.java
@@ -14,12 +14,14 @@
import com.android.tools.r8.ir.optimize.lambda.CaptureSignature;
import com.android.tools.r8.utils.AndroidApp;
import com.android.tools.r8.utils.DexInspector;
+import com.android.tools.r8.utils.InternalOptions;
import com.google.common.collect.Lists;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.concurrent.ExecutionException;
+import java.util.function.Consumer;
import org.junit.Test;
public class KotlinLambdaMergingTest extends AbstractR8KotlinTestBase {
@@ -29,6 +31,7 @@
"-keepattributes InnerClasses,EnclosingMethod\n";
private static final String KEEP_SIGNATURE_INNER_ENCLOSING =
"-keepattributes Signature,InnerClasses,EnclosingMethod\n";
+ private Consumer<InternalOptions> optionsModifier = opts -> opts.enableClassInlining = false;
abstract static class LambdaOrGroup {
abstract boolean match(DexClass clazz);
@@ -252,7 +255,7 @@
@Test
public void testTrivialKs() throws Exception {
final String mainClassName = "lambdas_kstyle_trivial.MainKt";
- runTest("lambdas_kstyle_trivial", mainClassName, (app) -> {
+ runTest("lambdas_kstyle_trivial", mainClassName, optionsModifier, (app) -> {
Verifier verifier = new Verifier(app);
String pkg = "lambdas_kstyle_trivial";
@@ -293,7 +296,7 @@
@Test
public void testCapturesKs() throws Exception {
final String mainClassName = "lambdas_kstyle_captures.MainKt";
- runTest("lambdas_kstyle_captures", mainClassName, (app) -> {
+ runTest("lambdas_kstyle_captures", mainClassName, optionsModifier, (app) -> {
Verifier verifier = new Verifier(app);
String pkg = "lambdas_kstyle_captures";
String grpPkg = allowAccessModification ? "" : pkg;
@@ -318,7 +321,7 @@
@Test
public void testGenericsNoSignatureKs() throws Exception {
final String mainClassName = "lambdas_kstyle_generics.MainKt";
- runTest("lambdas_kstyle_generics", mainClassName, (app) -> {
+ runTest("lambdas_kstyle_generics", mainClassName, optionsModifier, (app) -> {
Verifier verifier = new Verifier(app);
String pkg = "lambdas_kstyle_generics";
String grpPkg = allowAccessModification ? "" : pkg;
@@ -339,55 +342,57 @@
@Test
public void testInnerClassesAndEnclosingMethodsKs() throws Exception {
final String mainClassName = "lambdas_kstyle_generics.MainKt";
- runTest("lambdas_kstyle_generics", mainClassName, KEEP_INNER_AND_ENCLOSING, (app) -> {
- Verifier verifier = new Verifier(app);
- String pkg = "lambdas_kstyle_generics";
- String grpPkg = allowAccessModification ? "" : pkg;
+ runTest("lambdas_kstyle_generics", mainClassName,
+ KEEP_INNER_AND_ENCLOSING, optionsModifier, (app) -> {
+ Verifier verifier = new Verifier(app);
+ String pkg = "lambdas_kstyle_generics";
+ String grpPkg = allowAccessModification ? "" : pkg;
- verifier.assertLambdaGroups(
- kstyle(grpPkg, 1, 3), // Group for Any
- kstyle(grpPkg, "L", 1), // Group for Beta // First
- kstyle(grpPkg, "L", 1), // Group for Beta // Second
- kstyle(grpPkg, "LS", 1), // Group for Gamma // First
- kstyle(grpPkg, "LS", 1), // Group for Gamma // Second
- kstyle(grpPkg, 1, 2) // Group for int
- );
+ verifier.assertLambdaGroups(
+ kstyle(grpPkg, 1, 3), // Group for Any
+ kstyle(grpPkg, "L", 1), // Group for Beta // First
+ kstyle(grpPkg, "L", 1), // Group for Beta // Second
+ kstyle(grpPkg, "LS", 1), // Group for Gamma // First
+ kstyle(grpPkg, "LS", 1), // Group for Gamma // Second
+ kstyle(grpPkg, 1, 2) // Group for int
+ );
- verifier.assertLambdas(
- new Lambda(pkg, "MainKt$main$4", 1)
- );
- });
+ verifier.assertLambdas(
+ new Lambda(pkg, "MainKt$main$4", 1)
+ );
+ });
}
@Test
public void testGenericsSignatureInnerEnclosingKs() throws Exception {
final String mainClassName = "lambdas_kstyle_generics.MainKt";
- runTest("lambdas_kstyle_generics", mainClassName, KEEP_SIGNATURE_INNER_ENCLOSING, (app) -> {
- Verifier verifier = new Verifier(app);
- String pkg = "lambdas_kstyle_generics";
- String grpPkg = allowAccessModification ? "" : pkg;
+ runTest("lambdas_kstyle_generics", mainClassName,
+ KEEP_SIGNATURE_INNER_ENCLOSING, optionsModifier, (app) -> {
+ Verifier verifier = new Verifier(app);
+ String pkg = "lambdas_kstyle_generics";
+ String grpPkg = allowAccessModification ? "" : pkg;
- verifier.assertLambdaGroups(
- kstyle(grpPkg, 1, 3), // Group for Any
- kstyle(grpPkg, "L", 1), // Group for Beta in First
- kstyle(grpPkg, "L", 1), // Group for Beta in Second
- kstyle(grpPkg, "LS", 1), // Group for Gamma<String> in First
- kstyle(grpPkg, "LS", 1), // Group for Gamma<Integer> in First
- kstyle(grpPkg, "LS", 1), // Group for Gamma<String> in Second
- kstyle(grpPkg, "LS", 1), // Group for Gamma<Integer> in Second
- kstyle(grpPkg, 1, 2) // Group for int
- );
+ verifier.assertLambdaGroups(
+ kstyle(grpPkg, 1, 3), // Group for Any
+ kstyle(grpPkg, "L", 1), // Group for Beta in First
+ kstyle(grpPkg, "L", 1), // Group for Beta in Second
+ kstyle(grpPkg, "LS", 1), // Group for Gamma<String> in First
+ kstyle(grpPkg, "LS", 1), // Group for Gamma<Integer> in First
+ kstyle(grpPkg, "LS", 1), // Group for Gamma<String> in Second
+ kstyle(grpPkg, "LS", 1), // Group for Gamma<Integer> in Second
+ kstyle(grpPkg, 1, 2) // Group for int
+ );
- verifier.assertLambdas(
- new Lambda(pkg, "MainKt$main$4", 1)
- );
- });
+ verifier.assertLambdas(
+ new Lambda(pkg, "MainKt$main$4", 1)
+ );
+ });
}
@Test
public void testTrivialJs() throws Exception {
final String mainClassName = "lambdas_jstyle_trivial.MainKt";
- runTest("lambdas_jstyle_trivial", mainClassName, (app) -> {
+ runTest("lambdas_jstyle_trivial", mainClassName, optionsModifier, (app) -> {
Verifier verifier = new Verifier(app);
String pkg = "lambdas_jstyle_trivial";
String grp = allowAccessModification ? "" : pkg;
@@ -435,7 +440,7 @@
@Test
public void testSingleton() throws Exception {
final String mainClassName = "lambdas_singleton.MainKt";
- runTest("lambdas_singleton", mainClassName, (app) -> {
+ runTest("lambdas_singleton", mainClassName, optionsModifier, (app) -> {
Verifier verifier = new Verifier(app);
String pkg = "lambdas_singleton";
String grp = allowAccessModification ? "" : pkg;
diff --git a/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/defaultctor/ImplicitlyKeptDefaultConstructorTest.java b/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/defaultctor/ImplicitlyKeptDefaultConstructorTest.java
index 61b1454..d8615e0 100644
--- a/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/defaultctor/ImplicitlyKeptDefaultConstructorTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/defaultctor/ImplicitlyKeptDefaultConstructorTest.java
@@ -155,8 +155,10 @@
runTest(
mainClass,
ImmutableList.of(mainClass, SuperClass.class, SubClass.class),
- // Prevent SuperClass from being merged into SubClass.
- keepMainProguardConfiguration(mainClass, ImmutableList.of("-keep class **.SuperClass")),
+ // Prevent SuperClass from being merged into SubClass and keep both
+ // SuperClass and SubClass after instantiation is inlined.
+ keepMainProguardConfiguration(mainClass, ImmutableList.of(
+ "-keep class **.SuperClass", "-keep class **.SubClass")),
this::checkAllClassesPresentWithDefaultConstructor);
}
diff --git a/src/test/kotlinR8TestResources/class_inliner_lambda_k_style/main.kt b/src/test/kotlinR8TestResources/class_inliner_lambda_k_style/main.kt
new file mode 100644
index 0000000..136f72d
--- /dev/null
+++ b/src/test/kotlinR8TestResources/class_inliner_lambda_k_style/main.kt
@@ -0,0 +1,49 @@
+// 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 class_inliner_lambda_k_style
+
+private var COUNT = 0
+
+fun next() = "${COUNT++}".padStart(3, '0')
+
+fun main(args: Array<String>) {
+ testKotlinSequencesStateless(produceSequence(10))
+ testKotlinSequencesStateful(5, 2, produceSequence(10))
+}
+
+data class Record(val foo: String, val good: Boolean)
+
+@Synchronized
+fun testKotlinSequencesStateless(strings: Sequence<String>) {
+ useRecord()
+ // Stateless k-style lambda
+ strings.map { Record(it, false) }.forEach { println(it) }
+}
+
+@Synchronized
+fun testKotlinSequencesStateful(a: Int, b: Int, strings: Sequence<String>) {
+ useRecord()
+ // Big stateful k-style lambda
+ val capture = next()
+ strings.map {
+ val x = it.toInt()
+ val y = a + x
+ val z = capture.toInt() + b
+ println("logging $x/$y/$z") // Intentional
+ Record(it, y % z == 0)
+ }.forEach {
+ println(it)
+ }
+}
+
+private fun produceSequence(size: Int): Sequence<String> {
+ var count = size
+ return generateSequence { if (count-- > 0) next() else null }
+}
+
+// Need this to make sure testKotlinSequenceXXX is not processed
+// concurrently with invoke() on lambdas.
+fun useRecord() = useRecord2()
+fun useRecord2() = Record("", true)
diff --git a/src/test/sampleApks/split/AndroidManifest.xml b/src/test/sampleApks/split/AndroidManifest.xml
new file mode 100644
index 0000000..e46cd1e
--- /dev/null
+++ b/src/test/sampleApks/split/AndroidManifest.xml
@@ -0,0 +1,26 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!--
+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.
+-->
+<manifest xmlns:android="http://schemas.android.com/apk/res/android"
+ package="com.android.tools.r8.sample.split"
+ android:versionCode="1"
+ android:versionName="0.1" >
+
+ <uses-sdk android:minSdkVersion="21" />
+
+ <application
+ android:icon="@drawable/icon"
+ android:label="@string/app_name" >
+ <activity
+ android:name=".R8Activity"
+ android:label="@string/app_name" >
+ <intent-filter>
+ <action android:name="android.intent.action.MAIN" />
+ <category android:name="android.intent.category.LAUNCHER" />
+ </intent-filter>
+ </activity>
+ </application>
+</manifest>
diff --git a/src/test/sampleApks/split/assets/README.txt b/src/test/sampleApks/split/assets/README.txt
new file mode 100644
index 0000000..ecb060c
--- /dev/null
+++ b/src/test/sampleApks/split/assets/README.txt
@@ -0,0 +1 @@
+Sample split app from R8 project
diff --git a/src/test/sampleApks/split/res/drawable-mdpi/icon.png b/src/test/sampleApks/split/res/drawable-mdpi/icon.png
new file mode 100644
index 0000000..0799d58
--- /dev/null
+++ b/src/test/sampleApks/split/res/drawable-mdpi/icon.png
Binary files differ
diff --git a/src/test/sampleApks/split/res/layout/main.xml b/src/test/sampleApks/split/res/layout/main.xml
new file mode 100644
index 0000000..7859435
--- /dev/null
+++ b/src/test/sampleApks/split/res/layout/main.xml
@@ -0,0 +1,18 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!--
+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.
+-->
+<LinearLayout
+ xmlns:android="http://schemas.android.com/apk/res/android"
+ android:orientation="vertical" android:layout_width="fill_parent"
+ android:layout_height="fill_parent" android:id="@+id/MainLayout"
+ android:background="@android:color/background_light">
+
+ <Button
+ android:layout_height="wrap_content"
+ android:layout_width="match_parent" android:id="@+id/PressButton"
+ android:layout_margin="2dip"
+ android:text="Do something"/>
+</LinearLayout>
diff --git a/src/test/sampleApks/split/res/values/strings.xml b/src/test/sampleApks/split/res/values/strings.xml
new file mode 100644
index 0000000..ecac20f
--- /dev/null
+++ b/src/test/sampleApks/split/res/values/strings.xml
@@ -0,0 +1,9 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!--
+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.
+-->
+<resources>
+ <string name="app_name">R8 split app</string>
+</resources>
diff --git a/src/test/sampleApks/split/split.spec b/src/test/sampleApks/split/split.spec
new file mode 100644
index 0000000..a06e259
--- /dev/null
+++ b/src/test/sampleApks/split/split.spec
@@ -0,0 +1 @@
+com.android.tools.r8.sample.split.SplitClass:split
diff --git a/src/test/sampleApks/split/split_manifest/AndroidManifest.xml b/src/test/sampleApks/split/split_manifest/AndroidManifest.xml
new file mode 100644
index 0000000..21ea9f3
--- /dev/null
+++ b/src/test/sampleApks/split/split_manifest/AndroidManifest.xml
@@ -0,0 +1,15 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!--
+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.
+-->
+<manifest xmlns:android="http://schemas.android.com/apk/res/android"
+ package="com.android.tools.r8.sample.split"
+ split="featuresplit"
+ android:versionCode="1"
+ android:versionName="0.1" >
+
+ <uses-sdk android:minSdkVersion="21" />
+
+</manifest>
diff --git a/src/test/sampleApks/split/src/com/android/tools/r8/sample/split/R8Activity.java b/src/test/sampleApks/split/src/com/android/tools/r8/sample/split/R8Activity.java
new file mode 100644
index 0000000..a80cf55
--- /dev/null
+++ b/src/test/sampleApks/split/src/com/android/tools/r8/sample/split/R8Activity.java
@@ -0,0 +1,40 @@
+// 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.sample.split;
+
+import android.app.Activity;
+import android.os.Bundle;
+import com.android.tools.r8.sample.split.R;
+import com.android.tools.r8.sample.split.SplitClass;
+
+public class R8Activity extends Activity {
+ private int res = 0;
+
+ public void onCreate(Bundle savedInstanceState) {
+ super.onCreate(savedInstanceState);
+ setTheme(android.R.style.Theme_Light);
+ setContentView(R.layout.main);
+ // Currently this is split up into 100 iterations to be able to better see
+ // the impact of the jit on later versions of art.
+ long total = 0;
+ for (int i = 0; i < 100; i++) {
+ total += benchmarkCall();
+ }
+ System.out.println("Total: " + total);
+ }
+
+ public long benchmarkCall() {
+ SplitClass split = new SplitClass(3);
+ long start = System.nanoTime();
+ for (int i = 0; i < 1000; i++) {
+ // Ensure no dead code elimination.
+ res = split.calculate(i);
+ }
+ long finish = System.nanoTime();
+ long timeElapsed = finish - start;
+ System.out.println("Took: " + timeElapsed);
+ return timeElapsed;
+ }
+}
diff --git a/src/test/sampleApks/split/src/com/android/tools/r8/sample/split/SplitClass.java b/src/test/sampleApks/split/src/com/android/tools/r8/sample/split/SplitClass.java
new file mode 100644
index 0000000..9b6990d
--- /dev/null
+++ b/src/test/sampleApks/split/src/com/android/tools/r8/sample/split/SplitClass.java
@@ -0,0 +1,21 @@
+// 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.sample.split;
+
+public class SplitClass {
+ final int initialValue;
+
+ public SplitClass(int initialValue) {
+ this.initialValue = initialValue;
+ }
+
+ public int calculate(int x) {
+ int result = 2;
+ for (int i = 0; i < 42; i++) {
+ result += initialValue + x;
+ }
+ return result;
+ }
+}
diff --git a/tools/build_sample_apk.py b/tools/build_sample_apk.py
index e1b2deb..b31ade4 100755
--- a/tools/build_sample_apk.py
+++ b/tools/build_sample_apk.py
@@ -18,12 +18,14 @@
ANDROID_JAR = 'third_party/android_jar/lib-v{api}/android.jar'
DEFAULT_AAPT = 'aapt' # Assume in path.
DEFAULT_D8 = os.path.join(utils.REPO_ROOT, 'tools', 'd8.py')
+DEFAULT_DEXSPLITTER = os.path.join(utils.REPO_ROOT, 'tools', 'dexsplitter.py')
DEFAULT_JAVAC = 'javac'
SRC_LOCATION = 'src/com/android/tools/r8/sample/{app}/*.java'
DEFAULT_KEYSTORE = os.path.join(os.getenv('HOME'), '.android', 'debug.keystore')
SAMPLE_APKS = [
- 'simple'
+ 'simple',
+ 'split'
]
def parse_options():
@@ -38,6 +40,12 @@
result.add_option('--keystore',
help='Keystore used for signing',
default=DEFAULT_KEYSTORE)
+ result.add_option('--split',
+ help='Split the app using the split.spec file',
+ default=False, action='store_true')
+ result.add_option('--install',
+ help='Install the app (including featuresplit)',
+ default=False, action='store_true')
result.add_option('--app',
help='Which app to build',
default='simple',
@@ -72,6 +80,12 @@
def get_src_path(app):
return os.path.join(get_sample_dir(app), 'src')
+def get_dex_path(app):
+ return os.path.join(get_bin_path(app), 'classes.dex')
+
+def get_split_path(app, split):
+ return os.path.join(get_bin_path(app), split, 'classes.dex')
+
def run_aapt_pack(aapt, api, app):
with utils.ChangedWorkingDirectory(get_sample_dir(app)):
args = ['package',
@@ -86,6 +100,16 @@
'-G', os.path.join(get_build_dir(app), 'proguard_options')]
run_aapt(aapt, args)
+def run_aapt_split_pack(aapt, api, app):
+ with utils.ChangedWorkingDirectory(get_sample_dir(app)):
+ args = ['package',
+ '-v', '-f',
+ '-I', get_android_jar(api),
+ '-M', 'split_manifest/AndroidManifest.xml',
+ '-S', 'res',
+ '-F', os.path.join(get_bin_path(app), 'split_resources.ap_')]
+ run_aapt(aapt, args)
+
def compile_with_javac(api, app):
with utils.ChangedWorkingDirectory(get_sample_dir(app)):
files = glob.glob(SRC_LOCATION.format(app=app))
@@ -110,28 +134,75 @@
utils.PrintCmd(command)
subprocess.check_call(command)
-def create_temp_apk(app):
+def split(app):
+ split_spec = os.path.join(get_sample_dir(app), 'split.spec')
+ command = [DEFAULT_DEXSPLITTER,
+ '--input', get_dex_path(app),
+ '--output', get_bin_path(app),
+ '--feature-splits', split_spec]
+ utils.PrintCmd(command)
+ subprocess.check_call(command)
+
+def run_adb(args):
+ command = ['adb']
+ command.extend(args)
+ utils.PrintCmd(command)
+ subprocess.check_call(command)
+
+def adb_install(apks):
+ args = [
+ 'install-multiple' if len(apks) > 1 else 'install',
+ '-r',
+ '-d']
+ args.extend(apks)
+ run_adb(args)
+
+def create_temp_apk(app, prefix):
temp_apk_path = os.path.join(get_bin_path(app), '%s.ap_' % app)
- shutil.move(os.path.join(get_bin_path(app), 'resources.ap_'),
- temp_apk_path)
+ shutil.copyfile(os.path.join(get_bin_path(app), '%sresources.ap_' % prefix),
+ temp_apk_path)
return temp_apk_path
-def aapt_add_dex(aapt, app, temp_apk_path):
+def aapt_add_dex(aapt, dex, temp_apk_path):
args = ['add',
'-k', temp_apk_path,
- os.path.join(get_bin_path(app), 'classes.dex')]
+ dex]
run_aapt(aapt, args)
def Main():
(options, args) = parse_options()
+ apks = []
+ is_split = options.split
run_aapt_pack(options.aapt, options.api, options.app)
+ if is_split:
+ run_aapt_split_pack(options.aapt, options.api, options.app)
compile_with_javac(options.api, options.app)
dex(options.app, options.api)
- temp_apk_path = create_temp_apk(options.app)
- aapt_add_dex(options.aapt, options.app, temp_apk_path)
+ dex_files = { options.app: get_dex_path(options.app)}
+ dex_path = get_dex_path(options.app)
+ if is_split:
+ split(options.app)
+ dex_path = get_split_path(options.app, 'base')
+
+ temp_apk_path = create_temp_apk(options.app, '')
+ aapt_add_dex(options.aapt, dex_path, temp_apk_path)
apk_path = os.path.join(get_bin_path(options.app), '%s.apk' % options.app)
apk_utils.sign(temp_apk_path, apk_path, options.keystore)
- print('Apk available at: %s' % apk_path)
+ apks.append(apk_path)
+
+ if is_split:
+ split_temp_apk_path = create_temp_apk(options.app, 'split_')
+ aapt_add_dex(options.aapt,
+ get_split_path(options.app, 'split'),
+ temp_apk_path)
+ split_apk_path = os.path.join(get_bin_path(options.app), 'featuresplit.apk')
+ apk_utils.sign(temp_apk_path, split_apk_path, options.keystore)
+ apks.append(split_apk_path)
+
+ print('Generated apks available at: %s' % ' '.join(apks))
+ if options.install:
+ adb_install(apks)
+
if __name__ == '__main__':
sys.exit(Main())
