Merge "ClassInliner should not remove IncompatibleClassChangeError"
diff --git a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
index 2b51720..dae98ad 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -1651,7 +1651,7 @@
rewriteKeysWhileMergingValues(previous.staticFieldWrites, lense::lookupField);
this.fieldsRead = rewriteItems(previous.fieldsRead, lense::lookupField);
this.fieldsWritten = rewriteItems(previous.fieldsWritten, lense::lookupField);
- this.pinnedItems = rewriteMixedItems(previous.pinnedItems, lense);
+ this.pinnedItems = rewriteMixedItemsConservatively(previous.pinnedItems, lense);
this.virtualInvokes = rewriteMethodsConservatively(previous.virtualInvokes, lense);
this.interfaceInvokes = rewriteMethodsConservatively(previous.interfaceInvokes, lense);
this.superInvokes = rewriteMethodsConservatively(previous.superInvokes, lense);
@@ -1665,7 +1665,8 @@
this.assumedValues = previous.assumedValues;
assert assertNotModifiedByLense(previous.alwaysInline, lense);
this.alwaysInline = previous.alwaysInline;
- this.identifierNameStrings = rewriteMixedItems(previous.identifierNameStrings, lense);
+ this.identifierNameStrings =
+ rewriteMixedItemsConservatively(previous.identifierNameStrings, lense);
// Switchmap classes should never be affected by renaming.
assert assertNotModifiedByLense(
previous.switchMaps.keySet().stream().map(this::definitionFor).filter(Objects::nonNull)
@@ -1846,7 +1847,7 @@
return Collections.unmodifiableMap(result);
}
- private static ImmutableSet<DexItem> rewriteMixedItems(
+ private static ImmutableSet<DexItem> rewriteMixedItemsConservatively(
Set<DexItem> original, GraphLense lense) {
ImmutableSet.Builder<DexItem> builder = ImmutableSet.builder();
for (DexItem item : original) {
@@ -1854,7 +1855,12 @@
if (item instanceof DexType) {
builder.add(lense.lookupType((DexType) item));
} else if (item instanceof DexMethod) {
- builder.add(lense.lookupMethod((DexMethod) item));
+ DexMethod method = (DexMethod) item;
+ if (lense.isContextFreeForMethod(method)) {
+ builder.add(lense.lookupMethod(method));
+ } else {
+ builder.addAll(lense.lookupMethodInAllContexts(method));
+ }
} else if (item instanceof DexField) {
builder.add(lense.lookupField((DexField) item));
} else {
diff --git a/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
index 4b57209..d73b826 100644
--- a/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
+++ b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
@@ -11,6 +11,7 @@
import com.android.tools.r8.graph.DexEncodedField;
import com.android.tools.r8.graph.DexEncodedMethod;
import com.android.tools.r8.graph.DexField;
+import com.android.tools.r8.graph.DexItem;
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexProgramClass;
import com.android.tools.r8.graph.DexProto;
@@ -87,28 +88,20 @@
// Returns a set of types that must not be merged into other types.
private Set<DexType> getPinnedTypes(Iterable<DexProgramClass> classes) {
Set<DexType> pinnedTypes = new HashSet<>();
+
+ // For all pinned fields, also pin the type of the field (because changing the type of the field
+ // implicitly changes the signature of the field). Similarly, for all pinned methods, also pin
+ // the return type and the parameter types of the method.
+ extractPinnedItems(appInfo.pinnedItems, pinnedTypes);
+
+ // TODO(christofferqa): Remove the invariant that the graph lense should not modify any
+ // methods from the sets alwaysInline and noSideEffects (see use of assertNotModifiedBy-
+ // Lense).
+ extractPinnedItems(appInfo.alwaysInline, pinnedTypes);
+ extractPinnedItems(appInfo.noSideEffects.keySet(), pinnedTypes);
+
for (DexProgramClass clazz : classes) {
for (DexEncodedMethod method : clazz.methods()) {
- // TODO(christofferqa): Remove the invariant that the graph lense should not modify any
- // methods from the sets alwaysInline and noSideEffects (see use of assertNotModifiedBy-
- // Lense).
- if (appInfo.alwaysInline.contains(method) || appInfo.noSideEffects.containsKey(method)) {
- DexClass other = appInfo.definitionFor(method.method.proto.returnType);
- if (other != null && other.isProgramClass()) {
- // If we were to merge [other] into its sub class, then we would implicitly change the
- // signature of this method, and therefore break the invariant.
- pinnedTypes.add(other.type);
- }
- for (DexType parameterType : method.method.proto.parameters.values) {
- other = appInfo.definitionFor(parameterType);
- if (other != null && other.isProgramClass()) {
- // If we were to merge [other] into its sub class, then we would implicitly change the
- // signature of this method, and therefore break the invariant.
- pinnedTypes.add(other.type);
- }
- }
- }
-
// Avoid merging two types if this could remove a NoSuchMethodError, as illustrated by the
// following example. (Alternatively, it would be possible to merge A and B and rewrite the
// "invoke-super A.m" instruction into "invoke-super Object.m" to preserve the error. This
@@ -143,6 +136,38 @@
return pinnedTypes;
}
+ private void extractPinnedItems(Iterable<DexItem> items, Set<DexType> pinnedTypes) {
+ for (DexItem item : items) {
+ // Note: Nothing to do for the case where item is a DexType, since we check for this case
+ // using appInfo.isPinned.
+ if (item instanceof DexField) {
+ // Pin the type of the field.
+ DexField field = (DexField) item;
+ DexClass clazz = appInfo.definitionFor(field.type);
+ if (clazz != null && clazz.isProgramClass()) {
+ pinnedTypes.add(clazz.type);
+ }
+ } else if (item instanceof DexMethod) {
+ // Pin the return type and the parameter types of the method.
+ DexMethod method = (DexMethod) item;
+ DexClass clazz = appInfo.definitionFor(method.proto.returnType);
+ if (clazz != null && clazz.isProgramClass()) {
+ // If we were to merge [other] into its sub class, then we would implicitly change the
+ // signature of this method, and therefore break the invariant.
+ pinnedTypes.add(clazz.type);
+ }
+ for (DexType parameterType : method.proto.parameters.values) {
+ clazz = appInfo.definitionFor(parameterType);
+ if (clazz != null && clazz.isProgramClass()) {
+ // If we were to merge [other] into its sub class, then we would implicitly change the
+ // signature of this method, and therefore break the invariant.
+ pinnedTypes.add(clazz.type);
+ }
+ }
+ }
+ }
+ }
+
private boolean isMergeCandidate(DexProgramClass clazz, Set<DexType> pinnedTypes) {
if (appInfo.instantiatedTypes.contains(clazz.type)
|| appInfo.isPinned(clazz.type)
diff --git a/src/test/examples/classmerging/RewritePinnedMethodTest.java b/src/test/examples/classmerging/RewritePinnedMethodTest.java
new file mode 100644
index 0000000..3d25805
--- /dev/null
+++ b/src/test/examples/classmerging/RewritePinnedMethodTest.java
@@ -0,0 +1,41 @@
+// 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;
+
+public class RewritePinnedMethodTest {
+
+ public static void main(String[] args) {
+ C obj = new C();
+ obj.m();
+ }
+
+ // A.m is pinned by a keep rule, to prevent it from being merged into B.
+ public static class A {
+ public void m() {
+ System.out.println("In A.m");
+ }
+ }
+
+ // B is merged into C.
+ public static class B extends A {
+ @Override
+ public void m() {
+ System.out.println("In B.m");
+ super.m();
+ }
+ }
+
+ public static class C extends B {
+ @Override
+ public void m() {
+ System.out.println("In C.m");
+ // This invocation is changed from invoke-super to invoke-direct. It would be valid for this
+ // instruction to be on the form "invoke-super A.m". Therefore, the graph lense contains a
+ // mapping for "A.m" from (context: "C.m", type: SUPER) to C.m$B, where the method C.m$B is
+ // the direct method that gets created for B.m during vertical class merging.
+ super.m();
+ }
+ }
+}
diff --git a/src/test/examples/classmerging/keep-rules.txt b/src/test/examples/classmerging/keep-rules.txt
index 01e35f3..1a20ce3 100644
--- a/src/test/examples/classmerging/keep-rules.txt
+++ b/src/test/examples/classmerging/keep-rules.txt
@@ -13,6 +13,9 @@
-keep public class classmerging.ExceptionTest {
public static void main(...);
}
+-keep public class classmerging.RewritePinnedMethodTest {
+ public static void main(...);
+}
-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 7016163..58b87c0 100644
--- a/src/test/java/com/android/tools/r8/classmerging/ClassMergingTest.java
+++ b/src/test/java/com/android/tools/r8/classmerging/ClassMergingTest.java
@@ -289,6 +289,30 @@
"-keep public class classmerging.pkg.SimpleInterfaceImplRetriever"));
}
+ @Ignore("b/73958515")
+ @Test
+ public void testRewritePinnedMethod() throws Exception {
+ String main = "classmerging.RewritePinnedMethodTest";
+ Path[] programFiles =
+ new Path[] {
+ CF_DIR.resolve("RewritePinnedMethodTest.class"),
+ CF_DIR.resolve("RewritePinnedMethodTest$A.class"),
+ CF_DIR.resolve("RewritePinnedMethodTest$B.class"),
+ CF_DIR.resolve("RewritePinnedMethodTest$C.class")
+ };
+ Set<String> preservedClassNames =
+ ImmutableSet.of(
+ "classmerging.RewritePinnedMethodTest",
+ "classmerging.RewritePinnedMethodTest$A",
+ "classmerging.RewritePinnedMethodTest$C");
+ runTest(
+ main,
+ programFiles,
+ preservedClassNames,
+ getProguardConfig(
+ EXAMPLE_KEEP, "-keep class classmerging.RewritePinnedMethodTest$A { *; }"));
+ }
+
@Test
public void testTemplateMethodPattern() throws Exception {
String main = "classmerging.TemplateMethodTest";
diff --git a/third_party/r8.tar.gz.sha1 b/third_party/r8.tar.gz.sha1
index fbb2aa8..06f1922 100644
--- a/third_party/r8.tar.gz.sha1
+++ b/third_party/r8.tar.gz.sha1
@@ -1 +1 @@
-38b85dcea75f12c37332a5425c87733e78754ba6
\ No newline at end of file
+ad3858bd8a8597f674d1af6bb762c16672dc6436
\ No newline at end of file
diff --git a/tools/minify_tool.py b/tools/minify_tool.py
index 8f440dd..8b086e5 100755
--- a/tools/minify_tool.py
+++ b/tools/minify_tool.py
@@ -19,9 +19,10 @@
import re
import sys
import time
+import zipfile
+
import toolhelper
import utils
-import zipfile
KEEP = '-keep public class %s { public static void main(...); }\n'
MANIFEST_PATH = 'META-INF/MANIFEST.MF'
@@ -85,7 +86,8 @@
return mo.group(1)
def minify_tool(mainclass=None, input_jar=utils.R8_JAR, output_jar=None,
- lib=utils.RT_JAR, debug=True, build=True, benchmark_name=None):
+ lib=utils.RT_JAR, debug=True, build=True, benchmark_name=None,
+ track_memory_file=None):
if output_jar is None:
output_jar = generate_output_name(input_jar, mainclass)
with utils.TempDir() as path:
@@ -105,10 +107,15 @@
'--release',
tmp_input_path)
start_time = time.time()
- return_code = toolhelper.run('r8', args, debug=debug, build=build)
+ return_code = toolhelper.run('r8', args, debug=debug, build=build,
+ track_memory_file=track_memory_file)
if benchmark_name:
elapsed_ms = 1000 * (time.time() - start_time)
print('%s(RunTimeRaw): %s ms' % (benchmark_name, elapsed_ms))
+ if track_memory_file:
+ print('%s(MemoryUse): %s' %
+ (benchmark_name, utils.grep_memoryuse(track_memory_file)))
+
return return_code
if __name__ == '__main__':
diff --git a/tools/run_bootstrap_benchmark.py b/tools/run_bootstrap_benchmark.py
index 2ae97b2..8230c71 100755
--- a/tools/run_bootstrap_benchmark.py
+++ b/tools/run_bootstrap_benchmark.py
@@ -3,21 +3,53 @@
# 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.
-import argparse
-import minify_tool
import os
import sys
+
+import minify_tool
+import toolhelper
import utils
-
PINNED_R8_JAR = os.path.join(utils.REPO_ROOT, 'third_party/r8/r8.jar')
+PINNED_PGR8_JAR = os.path.join(utils.REPO_ROOT, 'third_party/r8/r8-pg6.0.1.jar')
-parser = argparse.ArgumentParser()
-parser.add_argument(
- '--name', metavar='NAME', dest='benchmark_name',
- help='Print "<NAME>(RunTimeRaw): <elapsed> ms" at the end')
-
+def dex(input, output):
+ return_code = toolhelper.run(
+ 'd8', [
+ input,
+ '--output', output,
+ '--lib', utils.RT_JAR,
+ '--min-api', '10000',
+ '--no-desugaring',
+ ],
+ debug=False,
+ build=False)
+ if return_code != 0:
+ sys.exit(return_code)
if __name__ == '__main__':
- sys.exit(minify_tool.minify_tool(input_jar=PINNED_R8_JAR, debug=False,
- build=False, **vars(parser.parse_args())))
+ with utils.TempDir() as temp:
+ memory_file = os.path.join(temp, 'memory.dump')
+ r8_output = os.path.join(temp, 'r8.zip')
+ d8_r8_output = os.path.join(temp, 'd8r8.zip')
+ d8_pg_output = os.path.join(temp, 'd8pg.zip')
+
+ return_code = minify_tool.minify_tool(
+ input_jar=PINNED_R8_JAR,
+ output_jar=r8_output,
+ debug=False,
+ build=False,
+ track_memory_file=memory_file,
+ benchmark_name="BootstrapR8")
+ if return_code != 0:
+ sys.exit(return_code)
+
+ dex(r8_output, d8_r8_output)
+ print "BootstrapR8(CodeSize):", os.path.getsize(r8_output)
+ print "BootstrapR8Dex(CodeSize):", os.path.getsize(d8_r8_output)
+
+ dex(PINNED_PGR8_JAR, d8_pg_output)
+ print "BootstrapPG(CodeSize):", os.path.getsize(PINNED_PGR8_JAR)
+ print "BootstrapPGDex(CodeSize):", os.path.getsize(d8_pg_output)
+
+ sys.exit(0)
\ No newline at end of file
