Merge "Add reproduction test for b/120164595"
diff --git a/build.gradle b/build.gradle
index 977ebd7..d2df8f8 100644
--- a/build.gradle
+++ b/build.gradle
@@ -257,6 +257,7 @@
def r8LibPath = "$buildDir/libs/r8lib.jar"
def r8LibGeneratedKeepRulesPath = "$buildDir/generated/keep.txt"
def r8LibTestPath = "$buildDir/classes/r8libtest"
+def r8TestsJarPath = "$buildDir/libs/r8tests.jar"
def osString = OperatingSystem.current().isLinux() ? "linux" :
OperatingSystem.current().isMacOsX() ? "mac" : "windows"
@@ -565,9 +566,7 @@
}
from repackageSources.outputs.files
from repackageDeps.outputs.files
- doLast {
- configureRelocations(it)
- }
+ configureRelocations(it)
}
task r8WithoutDeps(type: ShadowJar) {
@@ -599,6 +598,14 @@
}
}
+task R8NoManifestNoDeps(type: ShadowJar) {
+ from consolidatedLicense.outputs.files
+ baseName 'r8nomanifest-exclude-deps'
+ classifier = null
+ version = null
+ from sourceSets.main.output
+}
+
task R8NoManifest(type: ShadowJar) {
from consolidatedLicense.outputs.files
baseName 'r8nomanifest'
@@ -639,7 +646,8 @@
}
def baseR8CommandLine(args = []) {
- return ["java", "-ea", "-jar", R8.outputs.files[0]] + args
+ // Execute r8 commands against a stable r8 with relocated dependencies.
+ return ["java", "-ea", "-jar", r8WithRelocatedDeps.outputs.files[0]] + args
}
def r8CfCommandLine(input, output, pgconf, args = [], libs = []) {
@@ -655,34 +663,42 @@
def r8LibCreateTask(name, pgConf, r8Task, output, args = [], libs = []) {
return tasks.create("r8Lib${name}", Exec) {
- inputs.files ([pgConf, R8.outputs, r8Task.outputs])
+ inputs.files ([pgConf, r8WithRelocatedDeps.outputs, r8Task.outputs])
outputs.file output
dependsOn downloadOpenJDKrt
+ dependsOn r8WithRelocatedDeps
+ dependsOn r8Task
commandLine r8CfCommandLine(r8Task.outputs.files[0], output, pgConf, args, libs)
workingDir = projectDir
}
}
-task testJar(type: Jar, dependsOn: testClasses) {
+task testJar(type: ShadowJar, dependsOn: testClasses) {
+ outputs.upToDateWhen { false }
baseName = "r8tests"
from sourceSets.test.output
+ if (!project.hasProperty('exclude_deps')) {
+ relocate('com.google.common', 'com.android.tools.r8.com.google.common')
+ relocate('org.objectweb.asm', 'com.android.tools.r8.org.objectweb.asm')
+ }
}
task generateR8LibKeepRules(type: Exec) {
doFirst {
standardOutput new FileOutputStream(r8LibGeneratedKeepRulesPath)
}
- dependsOn R8
- dependsOn r8WithoutDeps
+ def libSourceTask = project.hasProperty('exclude_deps') ? R8NoManifestNoDeps : R8NoManifest
+ dependsOn r8WithRelocatedDeps
+ dependsOn libSourceTask
dependsOn testJar
dependsOn downloadOpenJDKrt
- inputs.files ([R8.outputs, r8WithoutDeps.outputs, testJar.outputs])
+ inputs.files ([r8WithRelocatedDeps.outputs, libSourceTask.outputs, testJar.outputs])
outputs.file r8LibGeneratedKeepRulesPath
commandLine baseR8CommandLine([
"printuses",
"--keeprules",
"third_party/openjdk/openjdk-rt-1.8/rt.jar",
- r8WithoutDeps.outputs.files[0],
+ libSourceTask.outputs.files[0],
testJar.outputs.files[0]])
workingDir = projectDir
}
@@ -691,15 +707,22 @@
dependsOn r8LibCreateTask(
"NoDeps",
"src/main/keep.txt",
- r8WithoutDeps,
+ R8NoManifestNoDeps,
r8LibPath,
["--pg-conf", generateR8LibKeepRules.outputs.files[0]],
repackageDepsNoRelocate.outputs.files
- ).dependsOn(repackageDepsNoRelocate, r8WithoutDeps, generateR8LibKeepRules)
+ ).dependsOn(repackageDepsNoRelocate, generateR8LibKeepRules)
+ outputs.file r8LibPath
}
task R8Lib {
- dependsOn r8LibCreateTask("Main", "src/main/keep.txt", R8NoManifest, r8LibPath)
+ dependsOn r8LibCreateTask(
+ "Main",
+ "src/main/keep.txt",
+ R8NoManifest,
+ r8LibPath,
+ ["--pg-conf", generateR8LibKeepRules.outputs.files[0]]
+ ).dependsOn(generateR8LibKeepRules)
outputs.file r8LibPath
}
@@ -1385,19 +1408,15 @@
}
task configureTestForR8Lib(type: Copy) {
- dependsOn testClasses
- // Setting classpath triggers a scan for test files in $buildDir/classes/test that finds all
- // tests and not just the ones under $test/com/android/tools/r8. That is generally not
- // something we want so we just copy the desired test files to $r8LibTestPath.
- // Note, we cannot use sourceSets.test.output here since it will copy all tests.
- def sourcePath = "$buildDir/classes/test/com/android/tools/r8"
- inputs.file sourcePath
+ dependsOn testJar
+ inputs.file r8TestsJarPath
if (getR8LibTask() != null) {
dependsOn getR8LibTask()
- def destPath = r8LibTestPath + "/com/android/tools/r8"
- delete destPath
- from(sourcePath)
- into(destPath)
+ delete r8LibTestPath
+ from zipTree(testJar.outputs.files[0])
+ into r8LibTestPath
+ include "com/android/tools/r8/**"
+ include "dalvik/**"
}
outputs.dir r8LibTestPath
}
@@ -1499,7 +1518,12 @@
}
if (project.hasProperty('r8lib') || project.hasProperty('r8lib_no_deps')) {
dependsOn configureTestForR8Lib
- classpath = files([r8LibPath]) + (sourceSets.test.runtimeClasspath - sourceSets.main.output)
+ // We remove build/classes/test from classpath and rely on configureTestForR8Lib to provide
+ // all needed tests in r8LibTestPath.
+ classpath = files([r8LibPath, r8LibTestPath]) +
+ sourceSets.test.runtimeClasspath -
+ sourceSets.main.output -
+ files(['build/classes/test'])
testClassesDir = new File(r8LibTestPath)
}
if (OperatingSystem.current().isLinux()
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/type/TypeLatticeElement.java b/src/main/java/com/android/tools/r8/ir/analysis/type/TypeLatticeElement.java
index ad8edd3..14aad66 100644
--- a/src/main/java/com/android/tools/r8/ir/analysis/type/TypeLatticeElement.java
+++ b/src/main/java/com/android/tools/r8/ir/analysis/type/TypeLatticeElement.java
@@ -7,7 +7,6 @@
import com.android.tools.r8.graph.AppInfo;
import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.graph.DexType;
-import com.android.tools.r8.ir.code.NumericType;
import com.android.tools.r8.ir.code.Value;
/**
@@ -346,24 +345,6 @@
return appInfo.dexItemFactory.createReferenceTypeLatticeElement(type, isNullable, appInfo);
}
- public static TypeLatticeElement fromNumericType(NumericType type) {
- switch (type) {
- case BYTE:
- case CHAR:
- case SHORT:
- case INT:
- return INT;
- case LONG:
- return LONG;
- case FLOAT:
- return FLOAT;
- case DOUBLE:
- return DOUBLE;
- default:
- throw new Unreachable("Unexpected numeric type: " + type);
- }
- }
-
public boolean isValueTypeCompatible(TypeLatticeElement other) {
return (isReference() && other.isReference())
|| (isSingle() && other.isSingle())
diff --git a/src/main/java/com/android/tools/r8/ir/code/BasicBlockInstructionIterator.java b/src/main/java/com/android/tools/r8/ir/code/BasicBlockInstructionIterator.java
index 7dbb1f2..af2dae6 100644
--- a/src/main/java/com/android/tools/r8/ir/code/BasicBlockInstructionIterator.java
+++ b/src/main/java/com/android/tools/r8/ir/code/BasicBlockInstructionIterator.java
@@ -29,17 +29,17 @@
protected Instruction current;
protected Position position = null;
- protected BasicBlockInstructionIterator(BasicBlock block) {
+ BasicBlockInstructionIterator(BasicBlock block) {
this.block = block;
this.listIterator = block.getInstructions().listIterator();
}
- protected BasicBlockInstructionIterator(BasicBlock block, int index) {
+ BasicBlockInstructionIterator(BasicBlock block, int index) {
this.block = block;
this.listIterator = block.getInstructions().listIterator(index);
}
- protected BasicBlockInstructionIterator(BasicBlock block, Instruction instruction) {
+ BasicBlockInstructionIterator(BasicBlock block, Instruction instruction) {
this(block);
nextUntil((x) -> x == instruction);
}
@@ -134,6 +134,8 @@
Value value = current.inValues().get(i);
value.removeUser(current);
}
+ // These needs to stay to ensure that an optimization incorrectly not taking debug info into
+ // account still produces valid code when run without enabled assertions.
for (Value value : current.getDebugValues()) {
value.removeDebugUser(current);
}
@@ -302,11 +304,8 @@
blocksIterator.previous();
}
assert IteratorUtils.peekNext(blocksIterator) == inlinee.blocks.getFirst();
-
// Iterate through the inlined blocks.
- Iterator<BasicBlock> inlinedBlocksIterator = inlinee.blocks.iterator();
- while (inlinedBlocksIterator.hasNext()) {
- BasicBlock inlinedBlock = inlinedBlocksIterator.next();
+ for (BasicBlock inlinedBlock : inlinee.blocks) {
BasicBlock expected = blocksIterator.next();
assert inlinedBlock == expected; // Iterators must be in sync.
if (inlinedBlock.hasCatchHandlers()) {
diff --git a/src/main/java/com/android/tools/r8/ir/code/Binop.java b/src/main/java/com/android/tools/r8/ir/code/Binop.java
index 931a584..54567b5 100644
--- a/src/main/java/com/android/tools/r8/ir/code/Binop.java
+++ b/src/main/java/com/android/tools/r8/ir/code/Binop.java
@@ -10,6 +10,7 @@
import com.android.tools.r8.errors.Unreachable;
import com.android.tools.r8.graph.AppInfo;
import com.android.tools.r8.graph.DexType;
+import com.android.tools.r8.ir.analysis.type.PrimitiveTypeLatticeElement;
import com.android.tools.r8.ir.analysis.type.TypeLatticeElement;
import com.android.tools.r8.ir.optimize.Inliner.ConstraintWithTarget;
import com.android.tools.r8.ir.optimize.InliningConstraints;
@@ -135,7 +136,7 @@
@Override
public TypeLatticeElement evaluate(AppInfo appInfo) {
- return TypeLatticeElement.fromNumericType(type);
+ return PrimitiveTypeLatticeElement.fromNumericType(type);
}
@Override
diff --git a/src/main/java/com/android/tools/r8/ir/code/InstructionListIterator.java b/src/main/java/com/android/tools/r8/ir/code/InstructionListIterator.java
index 48b6c9b..a5e5d78 100644
--- a/src/main/java/com/android/tools/r8/ir/code/InstructionListIterator.java
+++ b/src/main/java/com/android/tools/r8/ir/code/InstructionListIterator.java
@@ -9,7 +9,6 @@
import java.util.ArrayList;
import java.util.List;
import java.util.ListIterator;
-import java.util.function.Predicate;
public interface InstructionListIterator
extends ListIterator<Instruction>,
@@ -46,23 +45,6 @@
return next;
}
- /**
- * Continue to call {@link #next} while {@code predicate} tests {@code false}.
- *
- * @return the instruction that matched the predicate or {@code null} if all instructions fails
- * the predicate test
- */
- @Override
- default Instruction nextUntil(Predicate<Instruction> predicate) {
- while (hasNext()) {
- Instruction instruction = next();
- if (predicate.test(instruction)) {
- return instruction;
- }
- }
- return null;
- }
-
default void setInsertionPosition(Position position) {
// Intentionally empty.
}
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/CfBuilder.java b/src/main/java/com/android/tools/r8/ir/conversion/CfBuilder.java
index 8f32081..6e5e5d7 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/CfBuilder.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/CfBuilder.java
@@ -155,12 +155,14 @@
LoadStoreHelper loadStoreHelper = new LoadStoreHelper(code, typeVerificationHelper, appInfo);
loadStoreHelper.insertLoadsAndStores();
// Run optimizations on phis and basic blocks in a fixpoint.
- PhiOptimizations phiOptimizations = new PhiOptimizations();
- boolean reachedFixpoint = false;
- phiOptimizations.optimize(code);
- while (!reachedFixpoint) {
- BasicBlockMuncher.optimize(code);
- reachedFixpoint = !phiOptimizations.optimize(code);
+ if (!options.testing.disallowLoadStoreOptimization) {
+ PhiOptimizations phiOptimizations = new PhiOptimizations();
+ boolean reachedFixpoint = false;
+ phiOptimizations.optimize(code);
+ while (!reachedFixpoint) {
+ BasicBlockMuncher.optimize(code);
+ reachedFixpoint = !phiOptimizations.optimize(code);
+ }
}
registerAllocator = new CfRegisterAllocator(code, options, typeVerificationHelper);
registerAllocator.allocateRegisters();
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 e08e9c4..41faa04 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
@@ -95,15 +95,11 @@
private static ConstNumber createConstNumberReplacement(
IRCode code, long constant, TypeLatticeElement typeLattice, DebugLocalInfo debugLocalInfo) {
- ConstNumber replacement;
- if (typeLattice.isReference()) {
- assert constant == 0;
- replacement = code.createConstNull();
- } else {
- Value returnedValue = code.createValue(typeLattice, debugLocalInfo);
- replacement = new ConstNumber(returnedValue, constant);
- }
- return replacement;
+ assert !typeLattice.isReference() || constant == 0;
+ Value returnedValue =
+ code.createValue(
+ typeLattice.isReference() ? TypeLatticeElement.NULL : typeLattice, debugLocalInfo);
+ return new ConstNumber(returnedValue, constant);
}
private void setValueRangeFromProguardRule(ProguardMemberRule rule, Value value) {
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/Outliner.java b/src/main/java/com/android/tools/r8/ir/optimize/Outliner.java
index f9740c6..7966d3a 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/Outliner.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/Outliner.java
@@ -24,6 +24,7 @@
import com.android.tools.r8.graph.MethodAccessFlags;
import com.android.tools.r8.graph.ParameterAnnotationsList;
import com.android.tools.r8.graph.UseRegistry;
+import com.android.tools.r8.ir.analysis.type.PrimitiveTypeLatticeElement;
import com.android.tools.r8.ir.analysis.type.TypeLatticeElement;
import com.android.tools.r8.ir.code.Add;
import com.android.tools.r8.ir.code.BasicBlock;
@@ -273,7 +274,7 @@
inValues.add(
builder.readRegister(register, ValueTypeConstraint.fromNumericType(numericType)));
}
- TypeLatticeElement latticeElement = TypeLatticeElement.fromNumericType(numericType);
+ TypeLatticeElement latticeElement = PrimitiveTypeLatticeElement.fromNumericType(numericType);
Value outValue =
builder.writeRegister(outline.argumentCount(), latticeElement, ThrowingInfo.CAN_THROW);
Instruction newInstruction = null;
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 243a0ee..c0c6e59 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -529,6 +529,7 @@
public boolean allowFailureOnInnerClassErrors = false;
public boolean noLocalsTableOnInput = false;
public boolean forceNameReflectionOptimization = false;
+ public boolean disallowLoadStoreOptimization = false;
}
private boolean hasMinApi(AndroidApiLevel level) {
diff --git a/src/test/java/com/android/tools/r8/ExternalR8TestBuilder.java b/src/test/java/com/android/tools/r8/ExternalR8TestBuilder.java
index 3b5ddd8..e9d85be 100644
--- a/src/test/java/com/android/tools/r8/ExternalR8TestBuilder.java
+++ b/src/test/java/com/android/tools/r8/ExternalR8TestBuilder.java
@@ -76,6 +76,7 @@
Collections.addAll(
command,
getJavaExecutable(),
+ "-ea",
"-cp",
r8jar.toAbsolutePath().toString(),
R8.class.getTypeName(),
diff --git a/src/test/java/com/android/tools/r8/cf/TryRangeTest.java b/src/test/java/com/android/tools/r8/cf/TryRangeTest.java
new file mode 100644
index 0000000..08e01ec
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/cf/TryRangeTest.java
@@ -0,0 +1,41 @@
+// Copyright (c) 2019, 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.cf;
+
+import com.android.tools.r8.NeverInline;
+
+public class TryRangeTest {
+
+ @NeverInline
+ public static float doSomething(int x) throws Exception {
+ if (x == 42) {
+ throw new Exception("is 42");
+ } else {
+ return 1;
+ }
+ }
+
+ @NeverInline
+ public static void test(int count) {
+ int x = count;
+ float y;
+ if (x == 7) {
+ try {
+ y = doSomething(x);
+ } catch (Exception e) {
+ System.out.println(x);
+ return;
+ }
+ } else {
+ System.out.println(x);
+ y = 7;
+ }
+ System.out.println(y);
+ }
+
+ public static void main(String[] args) {
+ test(10);
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/cf/TryRangeTestRunner.java b/src/test/java/com/android/tools/r8/cf/TryRangeTestRunner.java
new file mode 100644
index 0000000..98542ff
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/cf/TryRangeTestRunner.java
@@ -0,0 +1,33 @@
+// Copyright (c) 2019, 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.cf;
+
+import com.android.tools.r8.CompilationMode;
+import com.android.tools.r8.TestBase;
+import org.junit.Test;
+
+/**
+ * This tests that we produce valid code when having normal-flow with exceptional edges in blocks.
+ * We might perform optimizations that add operations (dup, swap, etc.) before and after
+ * instructions that lie on the boundary of the exception table that is generated for a basic block.
+ * If live-ranges are minimized this could produce VerifyErrors. TODO(b/119771771) Will fail if
+ * shorten live ranges without shorten exception table range.
+ */
+public class TryRangeTestRunner extends TestBase {
+
+ @Test
+ public void test() throws Exception {
+ testForR8(Backend.CF)
+ .addProgramClasses(TryRangeTest.class)
+ .addKeepMainRule(TryRangeTest.class)
+ .setMode(CompilationMode.RELEASE)
+ .minification(false)
+ .noTreeShaking()
+ .enableInliningAnnotations()
+ .addOptionsModification(o -> o.testing.disallowLoadStoreOptimization = true)
+ .run(TryRangeTest.class)
+ .assertSuccess();
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/debug/DebugStreamComparator.java b/src/test/java/com/android/tools/r8/debug/DebugStreamComparator.java
index 83dbd9e..04477bd 100644
--- a/src/test/java/com/android/tools/r8/debug/DebugStreamComparator.java
+++ b/src/test/java/com/android/tools/r8/debug/DebugStreamComparator.java
@@ -9,6 +9,7 @@
import com.android.tools.r8.debug.DebugTestBase.JUnit3Wrapper.DebuggeeState;
import com.android.tools.r8.debug.DebugTestBase.JUnit3Wrapper.FrameInspector;
+import com.android.tools.r8.debug.DebugTestConfig.RuntimeKind;
import com.android.tools.r8.utils.ListUtils;
import com.android.tools.r8.utils.Pair;
import com.android.tools.r8.utils.StringUtils;
@@ -288,8 +289,10 @@
String sig = reference.getMethodSignature();
List<Variable> variables = reference.getVisibleVariables();
int frameDepth = reference.getFrameDepth();
+ RuntimeKind referenceRuntime = reference.getConfig().getRuntimeKind();
for (int i = 1; i < states.size(); i++) {
DebuggeeState state = states.get(i);
+ RuntimeKind stateRuntime = state.getConfig().getRuntimeKind();
if (verifyFiles) {
assertEquals("source file mismatch", file, state.getSourceFile());
}
@@ -304,7 +307,11 @@
"method mismatch", method + sig, state.getMethodName() + state.getMethodSignature());
}
if (verifyVariables) {
- verifyVariablesEqual(variables, state.getVisibleVariables());
+ verifyVariablesEqual(
+ referenceRuntime,
+ reference.getVisibleVariables(),
+ stateRuntime,
+ state.getVisibleVariables());
}
if (verifyStack) {
assertEquals(frameDepth, state.getFrameDepth());
@@ -312,20 +319,33 @@
FrameInspector referenceInspector = reference.getFrame(j);
FrameInspector stateInspector = state.getFrame(j);
verifyVariablesEqual(
- referenceInspector.getVisibleVariables(), stateInspector.getVisibleVariables());
+ referenceRuntime,
+ referenceInspector.getVisibleVariables(),
+ stateRuntime,
+ stateInspector.getVisibleVariables());
}
}
}
}
- private static void verifyVariablesEqual(List<Variable> xs, List<Variable> ys) {
+ private static boolean shouldIgnoreVariable(Variable variable, RuntimeKind runtime) {
+ return runtime == RuntimeKind.DEX && variable.getName().isEmpty();
+ }
+
+ private static void verifyVariablesEqual(
+ RuntimeKind xRuntime, List<Variable> xs, RuntimeKind yRuntime, List<Variable> ys) {
Map<String, Variable> map = new HashMap<>(xs.size());
for (Variable x : xs) {
- map.put(x.getName(), x);
+ if (!shouldIgnoreVariable(x, xRuntime)) {
+ map.put(x.getName(), x);
+ }
}
List<Variable> unexpected = new ArrayList<>(ys.size());
List<Pair<Variable, Variable>> different = new ArrayList<>(Math.min(xs.size(), ys.size()));
for (Variable y : ys) {
+ if (shouldIgnoreVariable(y, yRuntime)) {
+ continue;
+ }
Variable x = map.remove(y.getName());
if (x == null) {
unexpected.add(y);
diff --git a/src/test/java/com/android/tools/r8/debug/ExamplesDebugTest.java b/src/test/java/com/android/tools/r8/debug/ExamplesDebugTest.java
index 099f55b..17d5077 100644
--- a/src/test/java/com/android/tools/r8/debug/ExamplesDebugTest.java
+++ b/src/test/java/com/android/tools/r8/debug/ExamplesDebugTest.java
@@ -81,8 +81,7 @@
@Test
public void testBridgeMethod() throws Exception {
- // TODO(b/79671093): D8 has local variables with empty names.
- testDebuggingJvmOnly("bridge", "BridgeMethod");
+ testDebugging("bridge", "BridgeMethod");
}
@Test
@@ -107,8 +106,7 @@
@Test
public void testFloatingPointValuedAnnotation() throws Exception {
- // D8 has no source file.
- testDebuggingJvmOnly("floating_point_annotations", "FloatingPointValuedAnnotationTest");
+ testDebugging("floating_point_annotations", "FloatingPointValuedAnnotationTest");
}
@Test
@@ -190,14 +188,14 @@
@Test
public void testSync() throws Exception {
- // D8 has two local variables with empty names.
+ // TODO(b/79671093): Line number mismatch in D8.
testDebuggingJvmOnly("sync", "Sync");
}
@Test
public void testThrowing() throws Exception {
- // TODO(b/79671093): We don't match JVM's behavior on this example.
- testDebuggingJvmOutputOnly("throwing", "Throwing");
+ // TODO(b/79671093): D8 has unexpected variables (this in throwing.c <init>).
+ testDebuggingJvmOnly("throwing", "Throwing");
}
@Test
@@ -205,11 +203,9 @@
testDebugging("trivial", "Trivial");
}
- @Ignore("TODO(mathiasr): InvalidDebugInfoException in CfSourceCode")
@Test
public void testTryCatch() throws Exception {
- // TODO(b/79671093): We don't match JVM's behavior on this example.
- testDebuggingJvmOutputOnly("trycatch", "TryCatch");
+ testDebugging("trycatch", "TryCatch");
}
@Test
@@ -259,8 +255,7 @@
@Test
public void testRegress62300145() throws Exception {
- // D8 has no source file.
- testDebuggingJvmOnly("regress_62300145", "Regress");
+ testDebugging("regress_62300145", "Regress");
}
@Test
@@ -281,21 +276,17 @@
@Test
public void testRegress70736958() throws Exception {
- // D8 has a local variable with empty name.
- testDebuggingJvmOnly("regress_70736958", "Test");
+ testDebugging("regress_70736958", "Test");
}
- @Ignore("TODO(mathiasr): Different behavior CfSourceCode vs JarSourceCode")
@Test
public void testRegress70737019() throws Exception {
- // TODO(b/79671093): We don't match JVM's behavior on this example.
- testDebuggingJvmOutputOnly("regress_70737019", "Test");
+ testDebugging("regress_70737019", "Test");
}
@Test
public void testRegress72361252() throws Exception {
- // D8 output has variable with empty name.
- testDebuggingJvmOnly("regress_72361252", "Test");
+ testDebugging("regress_72361252", "Test");
}
@Test
@@ -341,6 +332,8 @@
.add("R8/CfSourceCode", r8cf())
.add("R8/JarSourceCode", r8jar())
.add("D8", d8())
+ // When running on CF and DEX runtimes, filter down to states within the test package.
+ .setFilter(state -> state.getClassName().startsWith(pkg))
.compare();
}
diff --git a/tools/archive.py b/tools/archive.py
index 78a8189..ce04494 100755
--- a/tools/archive.py
+++ b/tools/archive.py
@@ -92,9 +92,10 @@
raise Exception('You are not a bot, don\'t archive builds')
# Generate an r8-ed build without dependencies.
- # Note: build_r8lib does a gradle-clean, this must be the first command.
- build_r8lib('r8nomanifest', True, True, utils.R8LIB_KEEP_RULES,
- utils.R8LIB_EXCLUDE_DEPS_JAR)
+ # The '-Pno_internal' flag is important because we generate the lib based on uses in tests.
+ gradle.RunGradleExcludeDeps([utils.R8LIB_NO_DEPS, '-Pno_internal'])
+ shutil.copyfile(utils.R8LIB_JAR, utils.R8LIB_EXCLUDE_DEPS_JAR)
+ shutil.copyfile(utils.R8LIB_JAR + '.map', utils.R8LIB_EXCLUDE_DEPS_JAR + '.map')
# Create maven release which uses a build that exclude dependencies.
create_maven_release.main(["--out", utils.LIBS])
@@ -106,6 +107,7 @@
# Ensure all archived artifacts has been built before archiving.
# The target tasks postfixed by 'lib' depend on the actual target task so
# building it invokes the original task first.
+ # The '-Pno_internal' flag is important because we generate the lib based on uses in tests.
gradle.RunGradle([
utils.R8,
utils.D8,
@@ -114,6 +116,7 @@
utils.R8LIB,
utils.COMPATDXLIB,
utils.COMPATPROGUARDLIB,
+ '-Pno_internal'
])
version = GetVersion()
is_master = IsMaster(version)
diff --git a/tools/as_utils.py b/tools/as_utils.py
index 1eaa206..7b373f4 100644
--- a/tools/as_utils.py
+++ b/tools/as_utils.py
@@ -8,7 +8,7 @@
import utils
-def add_r8_dependency(checkout_dir):
+def add_r8_dependency(checkout_dir, minified):
build_file = os.path.join(checkout_dir, 'build.gradle')
assert os.path.isfile(build_file), 'Expected a file to be present at {}'.format(build_file)
@@ -27,12 +27,21 @@
is_inside_dependencies = True
if is_inside_dependencies:
if utils.R8_JAR in stripped:
+ if minified:
+ # Skip line to avoid dependency on r8.jar
+ continue
+ added_r8_dependency = True
+ elif utils.R8LIB_JAR in stripped:
+ if not minified:
+ # Skip line to avoid dependency on r8lib.jar
+ continue
added_r8_dependency = True
elif 'com.android.tools.build:gradle:' in stripped:
gradle_version = stripped[stripped.rindex(':')+1:-1]
if not added_r8_dependency:
indent = ''.ljust(line.index('classpath'))
- f.write('{}classpath files(\'{}\')\n'.format(indent, utils.R8_JAR))
+ jar = utils.R8LIB_JAR if minified else utils.R8_JAR
+ f.write('{}classpath files(\'{}\')\n'.format(indent, jar))
added_r8_dependency = True
elif stripped == '}':
is_inside_dependencies = False
@@ -51,5 +60,5 @@
lines = f.readlines()
with open(build_file, 'w') as f:
for line in lines:
- if utils.R8_JAR not in line:
+ if (utils.R8_JAR not in line) and (utils.R8LIB_JAR not in line):
f.write(line)
diff --git a/tools/historic_memory_usage.py b/tools/historic_memory_usage.py
new file mode 100755
index 0000000..fec7b31
--- /dev/null
+++ b/tools/historic_memory_usage.py
@@ -0,0 +1,155 @@
+#!/usr/bin/env python
+# Copyright (c) 2019, 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.
+
+# Convenience script for running run_on_app.py finding minimum memory need for
+# compiling a given app back in time. This utilizes the prebuilt r8 jars on
+# cloud storage.
+# The script find all commits that exists on cloud storage in the given range.
+# It will then run the oldest and newest such commit, and gradually fill in
+# the commits in between.
+
+import optparse
+import os
+import subprocess
+import sys
+import utils
+
+APPS = ['gmscore', 'nest', 'youtube', 'gmail', 'chrome']
+COMPILERS = ['d8', 'r8']
+
+def ParseOptions(argv):
+ result = optparse.OptionParser()
+ result.add_option('--compiler',
+ help='The compiler to use',
+ default='d8',
+ choices=COMPILERS)
+ result.add_option('--app',
+ help='What app to run on',
+ default='gmail',
+ choices=APPS)
+ result.add_option('--top',
+ default=utils.get_HEAD_sha1(),
+ help='The most recent commit to test')
+ result.add_option('--bottom',
+ help='The oldest commit to test')
+ result.add_option('--output',
+ default='build',
+ help='Directory where to output results')
+ return result.parse_args(argv)
+
+
+class GitCommit(object):
+ def __init__(self, git_hash, destination, timestamp):
+ self.git_hash = git_hash
+ self.destination = destination
+ self.timestamp = timestamp
+
+ def __str__(self):
+ return '%s : %s (%s)' % (self.git_hash, self.destination, self.timestamp)
+
+ def __repr__(self):
+ return self.__str__()
+
+def git_commit_from_hash(hash):
+ commit_timestamp = subprocess.check_output(['git', 'show', '--no-patch',
+ '--no-notes', '--pretty=\'%ct\'',
+ hash]).strip().strip('\'')
+ destination = 'gs://r8-releases/raw/master/%s/r8.jar' % hash
+ commit = GitCommit(hash, destination, commit_timestamp)
+ return commit
+
+def enumerate_git_commits(options):
+ top = options.top if options.top else utils.get_HEAD_sha1()
+ # TODO(ricow): if not set, search back 1000
+ if not options.bottom:
+ raise Exception('No bottom specified')
+ bottom = options.bottom
+ output = subprocess.check_output(['git', 'rev-list', '--first-parent', top])
+ found_bottom = False
+ commits = []
+ for c in output.splitlines():
+ commits.append(git_commit_from_hash(c.strip()))
+ if c.strip() == bottom:
+ found_bottom = True
+ break
+ if not found_bottom:
+ raise Exception('Bottom not found, did you not use a merge commit')
+ return commits
+
+def get_available_commits(commits):
+ available_commits = []
+ for commit in commits:
+ if utils.cloud_storage_exists(commit.destination):
+ available_commits.append(commit)
+ return available_commits
+
+def print_commits(commits):
+ for commit in commits:
+ print(commit)
+
+def permutate_range(start, end):
+ diff = end - start
+ assert diff >= 0
+ if diff == 1:
+ return [start, end]
+ if diff == 0:
+ return [start]
+ half = end - (diff / 2)
+ numbers = [half]
+ first_half = permutate_range(start, half - 1)
+ second_half = permutate_range(half + 1, end)
+ for index in range(len(first_half)):
+ numbers.append(first_half[index])
+ if index < len(second_half):
+ numbers.append(second_half[index])
+ return numbers
+
+def permutate(number_of_commits):
+ assert number_of_commits > 0
+ numbers = permutate_range(0, number_of_commits - 1)
+ assert all(n in numbers for n in range(number_of_commits))
+ return numbers
+
+def pull_r8_from_cloud(commit):
+ utils.download_file_from_cloud_storage(commit.destination, utils.R8_JAR)
+
+def run_on_app(options, commit):
+ app = options.app
+ compiler = options.compiler
+ cmd = ['tools/run_on_app.py', '--app', app, '--compiler', compiler,
+ '--find-min-xmx']
+ stdout = subprocess.check_output(cmd)
+ output_path = options.output or 'build'
+ time_commit = '%s_%s' % (commit.timestamp, commit.git_hash)
+ time_commit_path = os.path.join(output_path, time_commit)
+ if not os.path.exists(time_commit_path):
+ os.makedirs(time_commit_path)
+ stdout_path = os.path.join(time_commit_path, 'stdout')
+ with open(stdout_path, 'w') as f:
+ f.write(stdout)
+ print('Wrote stdout to: %s' % stdout_path)
+
+
+def benchmark(commits, options):
+ commit_permutations = permutate(len(commits))
+ for index in commit_permutations:
+ commit = commits[index]
+ pull_r8_from_cloud(commit)
+ print('Running for commit: %s' % commit.git_hash)
+ run_on_app(options, commit)
+
+def main(argv):
+ (options, args) = ParseOptions(argv)
+ if not options.app:
+ raise Exception('Please specify an app')
+ commits = enumerate_git_commits(options)
+ available_commits = get_available_commits(commits)
+ print('Running for:')
+ print_commits(available_commits)
+ print('')
+ benchmark(available_commits, options)
+
+if __name__ == '__main__':
+ sys.exit(main(sys.argv[1:]))
diff --git a/tools/run_on_app.py b/tools/run_on_app.py
index cba1d6e..41d4578 100755
--- a/tools/run_on_app.py
+++ b/tools/run_on_app.py
@@ -220,7 +220,8 @@
extra_args = []
app_provided_pg_conf = False;
# todo(121018500): remove when memory is under control
- extra_args.append('-Xmx8G')
+ if not any('-Xmx' in arg for arg in extra_args):
+ extra_args.append('-Xmx8G')
if options.golem:
golem.link_third_party()
options.out = os.getcwd()
diff --git a/tools/run_on_as_app.py b/tools/run_on_as_app.py
index e2f70f0..8f37f43 100755
--- a/tools/run_on_as_app.py
+++ b/tools/run_on_as_app.py
@@ -14,7 +14,7 @@
import as_utils
-SHRINKERS = ['r8', 'r8full', 'proguard']
+SHRINKERS = ['r8', 'r8full', 'r8-minified', 'r8full-minified', 'proguard']
WORKING_DIR = utils.BUILD
if 'R8_BENCHMARK_DIR' in os.environ and os.path.isdir(os.environ['R8_BENCHMARK_DIR']):
@@ -107,6 +107,9 @@
script = os.path.join(utils.TOOLS_DIR, 'extractmarker.py')
return '~~R8' in subprocess.check_output(['python', script, apk]).strip()
+def IsMinifiedR8(shrinker):
+ return shrinker == 'r8-minified' or shrinker == 'r8full-minified'
+
def IsTrackedByGit(file):
return subprocess.check_output(['git', 'ls-files', file]).strip() != ''
@@ -163,11 +166,6 @@
with utils.ChangedWorkingDirectory(checkout_dir):
GitPull()
- if options.use_tot:
- as_utils.add_r8_dependency(checkout_dir)
- else:
- as_utils.remove_r8_dependency(checkout_dir)
-
result_per_shrinker = {}
with utils.ChangedWorkingDirectory(checkout_dir):
@@ -203,6 +201,11 @@
def BuildAppWithShrinker(app, config, shrinker, checkout_dir, options):
print('Building {} with {}'.format(app, shrinker))
+ if options.disable_tot:
+ as_utils.remove_r8_dependency(checkout_dir)
+ else:
+ as_utils.add_r8_dependency(checkout_dir, IsMinifiedR8(shrinker))
+
app_module = config.get('app_module', 'app')
archives_base_name = config.get(' archives_base_name', app_module)
flavor = config.get('flavor')
@@ -214,7 +217,7 @@
with open("gradle.properties", "a") as gradle_properties:
if 'r8' in shrinker:
gradle_properties.write('\nandroid.enableR8=true\n')
- if shrinker == 'r8full':
+ if shrinker == 'r8full' or shrinker == 'r8full-minified':
gradle_properties.write('android.enableR8.fullMode=true\n')
else:
assert shrinker == 'proguard'
@@ -328,16 +331,26 @@
result.add_option('--shrinker',
help='The shrinker to use (by default, all are run)',
choices=SHRINKERS)
- result.add_option('--use_tot',
+ result.add_option('--disable_tot',
help='Whether to disable the use of the ToT version of R8',
- default=True,
- action='store_false')
+ default=False,
+ action='store_true')
return result.parse_args(argv)
def main(argv):
+ global SHRINKERS
+
(options, args) = ParseOptions(argv)
- assert not options.use_tot or os.path.isfile(utils.R8_JAR), (
+ assert options.disable_tot or os.path.isfile(utils.R8_JAR), (
'Cannot build from ToT without r8.jar')
+ assert options.disable_tot or os.path.isfile(utils.R8LIB_JAR), (
+ 'Cannot build from ToT without r8lib.jar')
+
+ if options.disable_tot:
+ # Cannot run r8 lib without adding r8lib.jar as an dependency
+ SHRINKERS = [
+ shrinker for shrinker in SHRINKERS
+ if 'minified' not in shrinker]
result_per_shrinker_per_app = {}
diff --git a/tools/test.py b/tools/test.py
index 043487c..6b24433 100755
--- a/tools/test.py
+++ b/tools/test.py
@@ -185,6 +185,7 @@
gradle_args.append('-Pr8lib')
if options.r8lib_no_deps:
gradle_args.append('-Pr8lib_no_deps')
+ gradle_args.append('-Pexclude_deps')
# Add Gradle tasks
gradle_args.append('cleanTest')
diff --git a/tools/utils.py b/tools/utils.py
index 9278459..e0ffa5c 100644
--- a/tools/utils.py
+++ b/tools/utils.py
@@ -30,6 +30,7 @@
D8 = 'd8'
R8 = 'r8'
R8LIB = 'r8lib'
+R8LIB_NO_DEPS = 'r8LibNoDeps'
R8_SRC = 'sourceJar'
COMPATDX = 'compatdx'
COMPATDXLIB = 'compatdxlib'
