Add base tests illustrating merging/inlining issues with DexSplitter
Extracted a new test base that we can use for testing the R8Splitter
Bug: 122902374
Change-Id: I549b113fb35a042857630fb21b73d63c77820989
diff --git a/src/main/java/com/android/tools/r8/graph/DexApplication.java b/src/main/java/com/android/tools/r8/graph/DexApplication.java
index e87ab25..28c5878 100644
--- a/src/main/java/com/android/tools/r8/graph/DexApplication.java
+++ b/src/main/java/com/android/tools/r8/graph/DexApplication.java
@@ -102,8 +102,12 @@
public Iterable<DexProgramClass> classesWithDeterministicOrder() {
List<DexProgramClass> classes = new ArrayList<>(programClasses());
- // To keep the order deterministic, we sort the classes by their type, which is a unique key.
- classes.sort((a, b) -> a.type.slowCompareTo(b.type));
+ // We never actually sort by anything but the DexType, this is just here in case we ever change
+ // that.
+ if (options.testing.deterministicSortingBasedOnDexType) {
+ // To keep the order deterministic, we sort the classes by their type, which is a unique key.
+ classes.sort((a, b) -> a.type.slowCompareTo(b.type));
+ }
return classes;
}
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 51ea8a0..f459ced 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -981,6 +981,7 @@
public boolean enableStatefulLambdaCreateInstanceMethod = false;
public int basicBlockMuncherIterationLimit = NO_LIMIT;
public boolean dontReportFailingCheckDiscarded = false;
+ public boolean deterministicSortingBasedOnDexType = true;
// Flag to turn on/off JDK11+ nest-access control even when not required (Cf backend)
public boolean enableForceNestBasedAccessDesugaringForTest = false;
diff --git a/src/test/java/com/android/tools/r8/TestBase.java b/src/test/java/com/android/tools/r8/TestBase.java
index 9d90bf7..9b8abe6 100644
--- a/src/test/java/com/android/tools/r8/TestBase.java
+++ b/src/test/java/com/android/tools/r8/TestBase.java
@@ -394,6 +394,11 @@
return jarTestClasses(Arrays.asList(classes), null);
}
+ /** Create a temporary JAR file containing the specified test classes. */
+ protected Path jarTestClasses(Iterable<Class<?>> classes) throws IOException {
+ return jarTestClasses(classes, null);
+ }
+
/** Create a temporary JAR file containing the specified test classes and data resources. */
protected Path jarTestClasses(Iterable<Class<?>> classes, List<DataResource> dataResources)
throws IOException {
diff --git a/src/test/java/com/android/tools/r8/dexsplitter/DexSplitterInlineRegression.java b/src/test/java/com/android/tools/r8/dexsplitter/DexSplitterInlineRegression.java
new file mode 100644
index 0000000..1d26f28
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/dexsplitter/DexSplitterInlineRegression.java
@@ -0,0 +1,97 @@
+// 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.dexsplitter;
+
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertTrue;
+
+import com.android.tools.r8.NeverMerge;
+import com.android.tools.r8.R8FullTestBuilder;
+import com.android.tools.r8.R8TestCompileResult;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.ToolHelper.ProcessResult;
+import com.android.tools.r8.utils.StringUtils;
+import com.android.tools.r8.utils.codeinspector.ClassSubject;
+import com.google.common.collect.ImmutableSet;
+import java.io.IOException;
+import java.util.concurrent.ExecutionException;
+import java.util.function.Consumer;
+import java.util.function.Predicate;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class DexSplitterInlineRegression extends SplitterTestBase {
+
+ public static final String EXPECTED = StringUtils.lines("42");
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection params() {
+ return getTestParameters().withAllRuntimes().build();
+ }
+
+ private final TestParameters parameters;
+
+ public DexSplitterInlineRegression(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void testInliningFromFeature() throws Exception {
+ Predicate<R8TestCompileResult> ensureGetFromFeatureGone =
+ r8TestCompileResult -> {
+ // Ensure that getFromFeature from FeatureClass is inlined into the run method.
+ try {
+ ClassSubject clazz = r8TestCompileResult.inspector().clazz(FeatureClass.class);
+ return clazz.uniqueMethodWithName("getFromFeature").isAbsent();
+ } catch (IOException | ExecutionException ex) {
+ throw new RuntimeException("Found getFromFeature in FeatureClass");
+ }
+ };
+ Consumer<R8FullTestBuilder> configurator =
+ r8FullTestBuilder -> r8FullTestBuilder.enableMergeAnnotations().noMinification();
+ ProcessResult processResult =
+ testDexSplitter(
+ parameters,
+ ImmutableSet.of(BaseSuperClass.class),
+ ImmutableSet.of(FeatureClass.class),
+ FeatureClass.class,
+ EXPECTED,
+ ensureGetFromFeatureGone,
+ configurator);
+ // We expect art to fail on this with the dex splitter, see b/122902374
+ assertNotEquals(processResult.exitCode, 0);
+ assertTrue(processResult.stderr.contains("NoClassDefFoundError"));
+ }
+
+ @NeverMerge
+ public abstract static class BaseSuperClass implements RunInterface {
+ public void run() {
+ System.out.println(getFromFeature());
+ }
+
+ public abstract String getFromFeature();
+ }
+
+ public static class FeatureClass extends BaseSuperClass {
+ String s;
+
+ public FeatureClass() {
+ if (System.currentTimeMillis() < 2) {
+ s = "43";
+ } else {
+ s = "42";
+ }
+ }
+
+ @Override
+ public String getFromFeature() {
+ return s;
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/dexsplitter/DexSplitterMergeRegression.java b/src/test/java/com/android/tools/r8/dexsplitter/DexSplitterMergeRegression.java
new file mode 100644
index 0000000..9ea52d0
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/dexsplitter/DexSplitterMergeRegression.java
@@ -0,0 +1,121 @@
+// 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.dexsplitter;
+
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertTrue;
+
+import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.NeverMerge;
+import com.android.tools.r8.R8FullTestBuilder;
+import com.android.tools.r8.R8TestCompileResult;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.ToolHelper.ProcessResult;
+import com.android.tools.r8.utils.StringUtils;
+import com.android.tools.r8.utils.codeinspector.ClassSubject;
+import com.google.common.collect.ImmutableSet;
+import java.io.IOException;
+import java.util.concurrent.ExecutionException;
+import java.util.function.Consumer;
+import java.util.function.Predicate;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class DexSplitterMergeRegression extends SplitterTestBase {
+
+ public static final String EXPECTED = StringUtils.lines("42", "foobar");
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection params() {
+ return getTestParameters().withAllRuntimes().build();
+ }
+
+ private final TestParameters parameters;
+
+ public DexSplitterMergeRegression(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void testInliningFromFeature() throws Exception {
+ // Static merging is based on sorting order, we assert that we merged to the feature.
+ Predicate<R8TestCompileResult> ensureMergingToFeature =
+ r8TestCompileResult -> {
+ try {
+ ClassSubject clazz = r8TestCompileResult.inspector().clazz(AFeatureWithStatic.class);
+ return clazz.allMethods().size() == 2
+ && clazz.uniqueMethodWithName("getBase42").isPresent()
+ && clazz.uniqueMethodWithName("getFoobar").isPresent();
+ } catch (IOException | ExecutionException ex) {
+ throw new RuntimeException("Failed lookup up AFeatureWithStatic");
+ }
+ };
+ Consumer<R8FullTestBuilder> configurator =
+ r8FullTestBuilder ->
+ r8FullTestBuilder
+ .enableMergeAnnotations()
+ .enableInliningAnnotations()
+ .noMinification()
+ .addOptionsModification(o -> o.testing.deterministicSortingBasedOnDexType = true);
+ ProcessResult processResult =
+ testDexSplitter(
+ parameters,
+ ImmutableSet.of(BaseClass.class, BaseWithStatic.class),
+ ImmutableSet.of(FeatureClass.class, AFeatureWithStatic.class),
+ FeatureClass.class,
+ EXPECTED,
+ ensureMergingToFeature,
+ configurator);
+ // We expect art to fail on this with the dex splitter.
+ assertNotEquals(processResult.exitCode, 0);
+ assertTrue(processResult.stderr.contains("NoClassDefFoundError"));
+ }
+
+ @NeverMerge
+ public static class BaseClass implements RunInterface {
+
+ @NeverInline
+ public void run() {
+ System.out.println(BaseWithStatic.getBase42());
+ }
+ }
+
+ public static class BaseWithStatic {
+
+ @NeverInline
+ public static int getBase42() {
+ if (System.currentTimeMillis() < 2) {
+ return 43;
+ } else {
+ return 42;
+ }
+ }
+ }
+
+ public static class FeatureClass extends BaseClass {
+
+ public void run() {
+ super.run();
+ System.out.println(AFeatureWithStatic.getFoobar());
+ }
+ }
+
+ // Name is important, see predicate in tests/
+ public static class AFeatureWithStatic {
+
+ @NeverInline
+ public static String getFoobar() {
+ if (System.currentTimeMillis() < 2) {
+ return "barFoo";
+ } else {
+ return "foobar";
+ }
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/dexsplitter/SplitterTestBase.java b/src/test/java/com/android/tools/r8/dexsplitter/SplitterTestBase.java
new file mode 100644
index 0000000..05b8a84
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/dexsplitter/SplitterTestBase.java
@@ -0,0 +1,155 @@
+package com.android.tools.r8.dexsplitter;
+
+import static junit.framework.TestCase.assertTrue;
+
+import com.android.tools.r8.R8FullTestBuilder;
+import com.android.tools.r8.R8TestCompileResult;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.ToolHelper;
+import com.android.tools.r8.ToolHelper.ArtCommandBuilder;
+import com.android.tools.r8.ToolHelper.ProcessResult;
+import com.android.tools.r8.dexsplitter.DexSplitter.Options;
+import com.google.common.collect.ImmutableList;
+import dalvik.system.PathClassLoader;
+import java.net.MalformedURLException;
+import java.nio.file.Path;
+import java.util.List;
+import java.util.Set;
+import java.util.function.Consumer;
+import java.util.function.Predicate;
+
+public class SplitterTestBase extends TestBase {
+
+ // Compile the passed in classes plus RunInterface and SplitRunner using R8, then split
+ // based on the base/feature sets. toRun must implement the BaseRunInterface
+ protected ProcessResult testDexSplitter(
+ TestParameters parameters,
+ Set<Class<?>> baseClasses,
+ Set<Class<?>> featureClasses,
+ Class toRun,
+ String expectedOutput,
+ Predicate<R8TestCompileResult> predicate,
+ Consumer<R8FullTestBuilder> r8TestConfigurator)
+ throws Exception {
+ List<Class<?>> baseClassesWithRunner =
+ ImmutableList.<Class<?>>builder()
+ .add(RunInterface.class, SplitRunner.class)
+ .addAll(baseClasses)
+ .build();
+
+ Path baseJar = jarTestClasses(baseClassesWithRunner);
+ Path featureJar = jarTestClasses(featureClasses);
+
+ Path featureOnly =
+ testForR8(parameters.getBackend())
+ .addProgramClasses(featureClasses)
+ .addClasspathClasses(baseClasses)
+ .addClasspathClasses(RunInterface.class)
+ .addKeepAllClassesRule()
+ .setMinApi(parameters.getRuntime())
+ .compile()
+ .writeToZip();
+ if (parameters.isDexRuntime()) {
+ // With D8 this should just work. We compile all of the base classes, then run with the
+ // feature loaded at runtime. Since there is no inlining/class merging we don't
+ // have any issues.
+ testForD8()
+ .addProgramClasses(SplitRunner.class, RunInterface.class)
+ .addProgramClasses(baseClasses)
+ .setMinApi(parameters.getRuntime())
+ .compile()
+ .run(
+ parameters.getRuntime(),
+ SplitRunner.class,
+ toRun.getName(),
+ featureOnly.toAbsolutePath().toString())
+ .assertSuccessWithOutput(expectedOutput);
+ }
+
+ R8FullTestBuilder builder = testForR8(parameters.getBackend());
+ if (parameters.isCfRuntime()) {
+ // Compiling to jar we need to support the same way of loading code at runtime as
+ // android supports.
+ builder
+ .addProgramClasses(PathClassLoader.class)
+ .addKeepClassAndMembersRules(PathClassLoader.class);
+ }
+
+ R8FullTestBuilder r8FullTestBuilder =
+ builder
+ .setMinApi(parameters.getRuntime())
+ .addProgramClasses(SplitRunner.class, RunInterface.class)
+ .addProgramClasses(baseClasses)
+ .addProgramClasses(featureClasses)
+ .addKeepMainRule(SplitRunner.class)
+ .addKeepClassRules(toRun);
+ r8TestConfigurator.accept(r8FullTestBuilder);
+ R8TestCompileResult r8TestCompileResult = r8FullTestBuilder.compile();
+ assertTrue(predicate.test(r8TestCompileResult));
+ Path fullFiles = r8TestCompileResult.writeToZip();
+
+ // Ensure that we can run the program as a unit (i.e., without splitting)
+ r8TestCompileResult
+ .run(parameters.getRuntime(), SplitRunner.class, toRun.getName())
+ .assertSuccessWithOutput(expectedOutput);
+
+ Path splitterOutput = temp.newFolder().toPath();
+ Path splitterBaseDexFile = splitterOutput.resolve("base").resolve("classes.dex");
+ Path splitterFeatureDexFile = splitterOutput.resolve("feature").resolve("classes.dex");
+
+ Options options = new Options();
+ options.setOutput(splitterOutput.toString());
+ options.addBaseJar(baseJar.toString());
+ options.addFeatureJar(featureJar.toString(), "feature");
+
+ options.addInputArchive(fullFiles.toString());
+ DexSplitter.run(options);
+
+ ArtCommandBuilder commandBuilder = new ArtCommandBuilder();
+ commandBuilder.appendClasspath(splitterBaseDexFile.toString());
+ commandBuilder.appendProgramArgument(toRun.getName());
+ commandBuilder.appendProgramArgument(splitterFeatureDexFile.toString());
+ commandBuilder.setMainClass(SplitRunner.class.getName());
+ ProcessResult processResult = ToolHelper.runArtRaw(commandBuilder);
+ return processResult;
+ }
+
+ public interface RunInterface {
+ void run();
+ }
+
+ static class SplitRunner {
+ /* We support two different modes:
+ * - One argument to main:
+ * Pass in the class to be loaded, must implement RunInterface, run will be called
+ * - Two arguments to main:
+ * Pass in the class to be loaded, must implement RunInterface, run will be called
+ * Pass in the feature split that we class load
+ *
+ */
+ public static void main(String[] args) {
+ if (args.length < 1 || args.length > 2) {
+ throw new RuntimeException("Unsupported number of arguments");
+ }
+ String classToRun = args[0];
+ ClassLoader loader = SplitRunner.class.getClassLoader();
+ // In the case where we simulate splits, we pass in the feature as the second argument
+ if (args.length == 2) {
+ try {
+ loader = new PathClassLoader(args[1], SplitRunner.class.getClassLoader());
+ } catch (MalformedURLException e) {
+ throw new RuntimeException("Failed reading input URL");
+ }
+ }
+
+ try {
+ Class<?> aClass = loader.loadClass(classToRun);
+ RunInterface b = (RunInterface) aClass.newInstance();
+ b.run();
+ } catch (InstantiationException | IllegalAccessException | ClassNotFoundException e) {
+ throw new RuntimeException("Failed loading class");
+ }
+ }
+ }
+}
diff --git a/src/test/java/dalvik/system/PathClassLoader.java b/src/test/java/dalvik/system/PathClassLoader.java
new file mode 100644
index 0000000..e629fae
--- /dev/null
+++ b/src/test/java/dalvik/system/PathClassLoader.java
@@ -0,0 +1,19 @@
+package dalvik.system;
+
+import java.io.File;
+import java.net.MalformedURLException;
+import java.net.URL;
+import java.net.URLClassLoader;
+
+// Implement the equivalent of the PathClassLoader for CF backend. This simply forwards to the
+// URLClassLoader.
+public class PathClassLoader extends URLClassLoader {
+
+ public PathClassLoader(String dexPath, ClassLoader parent) throws MalformedURLException {
+ super(new URL[] {new File(dexPath).toURI().toURL()}, parent);
+ }
+
+ public PathClassLoader(URL[] urls, ClassLoader parent) {
+ super(urls, parent);
+ }
+}
