Version 1.2.44.
Merge: Workaround massive dex2oat memory usage for self-recursion.
CL: https://r8-review.googlesource.com/c/r8/+/25742
R=sgjesse@google.com, zerny@google.com
Bug: 111960171
Change-Id: I0ce71868a57ae5b799d5ba2288d4322f82ab46dc
diff --git a/src/main/java/com/android/tools/r8/Version.java b/src/main/java/com/android/tools/r8/Version.java
index 67c1ad1..e310190 100644
--- a/src/main/java/com/android/tools/r8/Version.java
+++ b/src/main/java/com/android/tools/r8/Version.java
@@ -11,7 +11,7 @@
// This field is accessed from release scripts using simple pattern matching.
// Therefore, changing this field could break our release scripts.
- public static final String LABEL = "1.2.43";
+ public static final String LABEL = "1.2.44";
private Version() {
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/BasicBlock.java b/src/main/java/com/android/tools/r8/ir/code/BasicBlock.java
index 8771b72..e10f789 100644
--- a/src/main/java/com/android/tools/r8/ir/code/BasicBlock.java
+++ b/src/main/java/com/android/tools/r8/ir/code/BasicBlock.java
@@ -683,6 +683,13 @@
catchHandlers = new CatchHandlers<>(guards, successorIndexes);
}
+ public void addCatchHandler(BasicBlock rethrowBlock, DexType guard) {
+ assert !hasCatchHandlers();
+ successors.add(0, rethrowBlock);
+ rethrowBlock.getPredecessors().add(this);
+ catchHandlers = new CatchHandlers<>(ImmutableList.of(guard), ImmutableList.of(0));
+ }
+
// Due to class merging, it is possible that two exception classes have been merged into one.
// This function renames the guards according to the given graph lense.
public void renameGuardsInCatchHandlers(GraphLense graphLense) {
@@ -1152,6 +1159,20 @@
return block;
}
+ public static BasicBlock createRethrowBlock(IRCode code, Position position) {
+ BasicBlock block = new BasicBlock();
+ MoveException moveException =
+ new MoveException(new Value(code.valueNumberGenerator.next(), ValueType.OBJECT, null));
+ moveException.setPosition(position);
+ Throw throwInstruction = new Throw(moveException.outValue);
+ throwInstruction.setPosition(position);
+ block.add(moveException);
+ block.add(throwInstruction);
+ block.close(null);
+ block.setNumber(code.getHighestBlockNumber() + 1);
+ return block;
+ }
+
public boolean isTrivialGoto() {
return instructions.size() == 1 && exit().isGoto();
}
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 ccbdf67..1252d82 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
@@ -797,6 +797,8 @@
}
private void finalizeToDex(DexEncodedMethod method, IRCode code, OptimizationFeedback feedback) {
+ // Workaround massive dex2oat memory use for self-recursive methods.
+ CodeRewriter.disableDex2OatInliningForSelfRecursiveMethods(code, options);
// Perform register allocation.
RegisterAllocator registerAllocator = performRegisterAllocation(code, method);
method.setCode(code, registerAllocator, options);
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 408041e..985b240 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
@@ -121,6 +121,7 @@
private static final int MAX_FILL_ARRAY_SIZE = 8 * Constants.KILOBYTE;
// This constant was determined by experimentation.
private static final int STOP_SHARED_CONSTANT_THRESHOLD = 50;
+ private static final int SELF_RECURSION_LIMIT = 4;
private final AppInfo appInfo;
private final DexItemFactory dexItemFactory;
@@ -257,6 +258,41 @@
}
}
+ // For method with many self-recursive calls, insert a try-catch to disable inlining.
+ // Marshmallow dex2oat aggressively inlines and eats up all the memory on devices.
+ public static void disableDex2OatInliningForSelfRecursiveMethods(
+ IRCode code, InternalOptions options) {
+ if (!options.canHaveDex2OatInliningIssue() || code.hasCatchHandlers()) {
+ // Catch handlers disables inlining, so if the method already has catch handlers
+ // there is nothing to do.
+ return;
+ }
+ InstructionIterator it = code.instructionIterator();
+ int selfRecursionFanOut = 0;
+ Instruction lastSelfRecursiveCall = null;
+ while (it.hasNext()) {
+ Instruction i = it.next();
+ if (i.isInvokeMethod() && i.asInvokeMethod().getInvokedMethod() == code.method.method) {
+ selfRecursionFanOut++;
+ lastSelfRecursiveCall = i;
+ }
+ }
+ if (selfRecursionFanOut > SELF_RECURSION_LIMIT) {
+ assert lastSelfRecursiveCall != null;
+ // Split out the last recursive call in its own block.
+ InstructionListIterator splitIterator =
+ lastSelfRecursiveCall.getBlock().listIterator(lastSelfRecursiveCall);
+ splitIterator.previous();
+ BasicBlock newBlock = splitIterator.split(code, 1);
+ // Generate rethrow block.
+ BasicBlock rethrowBlock =
+ BasicBlock.createRethrowBlock(code, lastSelfRecursiveCall.getPosition());
+ code.blocks.add(rethrowBlock);
+ // Add catch handler to the block containing the last recursive call.
+ newBlock.addCatchHandler(rethrowBlock, options.itemFactory.throwableType);
+ }
+ }
+
// TODO(sgjesse); Move this somewhere else, and reuse it for some of the other switch rewritings.
public abstract static class InstructionBuilder<T> {
protected int blockNumber;
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 8b34ea7..1818f88 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -622,6 +622,14 @@
return minApiLevel < AndroidApiLevel.N.getLevel();
}
+ // dex2oat on Marshmallow VMs does aggressive inlining which can eat up all the memory on
+ // devices for self-recursive methods.
+ //
+ // See b/111960171
+ public boolean canHaveDex2OatInliningIssue() {
+ return minApiLevel < AndroidApiLevel.N.getLevel();
+ }
+
// Art 7.0.0 and later Art JIT may perform an invalid optimization if a string new-instance does
// not flow directly to the init call.
//
diff --git a/src/test/java/com/android/tools/r8/regress/b111960171/B111960171.java b/src/test/java/com/android/tools/r8/regress/b111960171/B111960171.java
new file mode 100644
index 0000000..f57b1b6
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/regress/b111960171/B111960171.java
@@ -0,0 +1,77 @@
+// 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.regress.b111960171;
+
+import static com.android.tools.r8.utils.DexInspectorMatchers.isPresent;
+import static junit.framework.TestCase.assertTrue;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+import com.android.tools.r8.CompilationFailedException;
+import com.android.tools.r8.D8Command;
+import com.android.tools.r8.ToolHelper;
+import com.android.tools.r8.origin.Origin;
+import com.android.tools.r8.utils.AndroidApiLevel;
+import com.android.tools.r8.utils.AndroidApp;
+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.google.common.collect.ImmutableList;
+import java.io.IOException;
+import java.util.List;
+import java.util.concurrent.ExecutionException;
+import org.junit.Test;
+
+class TestClass {
+ public void f(int x, double y, double u, double v, List<Object> w) {
+ f(x, y, u, v, w);
+ f(x, y, u, v, w);
+ f(x, y, u, v, w);
+ f(x, y, u, v, w);
+ f(x, y, u, v, w);
+ f(x, y, u, v, w);
+ f(x, y, u, v, w);
+ f(x, y, u, v, w);
+ f(x, y, u, v, w);
+ w.add(g(y, u, v));
+ }
+
+ public Object g(double y, double u, double v) {
+ return null;
+ }
+}
+
+public class B111960171 {
+
+ private MethodSubject compileTestClassAndGetMethod(int apiLevel)
+ throws IOException, CompilationFailedException, ExecutionException {
+ AndroidApp app =
+ ToolHelper.runD8(
+ D8Command.builder()
+ .addClassProgramData(ToolHelper.getClassAsBytes(TestClass.class), Origin.unknown())
+ .setMinApiLevel(apiLevel));
+ DexInspector inspector = new DexInspector(app);
+ ClassSubject clazz = inspector.clazz(TestClass.class);
+ assertThat(clazz, isPresent());
+ MethodSubject method =
+ clazz.method(
+ "void", "f", ImmutableList.of("int", "double", "double", "double", "java.util.List"));
+ assertThat(method, isPresent());
+ return method;
+ }
+
+ @Test
+ public void disableDex2OatInliningWithTryCatch()
+ throws IOException, CompilationFailedException, ExecutionException {
+ MethodSubject method = compileTestClassAndGetMethod(AndroidApiLevel.M.getLevel());
+ assertTrue(method.getMethod().getCode().asDexCode().handlers != null);
+ }
+
+ @Test
+ public void dontDisableDex2OatInliningWithTryCatch()
+ throws IOException, CompilationFailedException, ExecutionException {
+ MethodSubject method = compileTestClassAndGetMethod(AndroidApiLevel.N.getLevel());
+ assertTrue(method.getMethod().getCode().asDexCode().handlers == null);
+ }
+}
