Static, horizontal class merging
Change-Id: I8216de0e6aa9531649b88d693ebe75b2d463efc8
diff --git a/src/main/java/com/android/tools/r8/D8.java b/src/main/java/com/android/tools/r8/D8.java
index fd57823..bd63ae5 100644
--- a/src/main/java/com/android/tools/r8/D8.java
+++ b/src/main/java/com/android/tools/r8/D8.java
@@ -166,6 +166,7 @@
options.enableMinification = false;
options.enableInlining = false;
options.enableClassInlining = false;
+ options.enableHorizontalClassMerging = false;
options.enableVerticalClassMerging = false;
options.enableClassStaticizer = false;
options.outline.enabled = false;
diff --git a/src/main/java/com/android/tools/r8/D8Command.java b/src/main/java/com/android/tools/r8/D8Command.java
index 2441034..e0f330d 100644
--- a/src/main/java/com/android/tools/r8/D8Command.java
+++ b/src/main/java/com/android/tools/r8/D8Command.java
@@ -251,6 +251,8 @@
internal.enableInlining = false;
assert internal.enableClassInlining;
internal.enableClassInlining = false;
+ assert internal.enableHorizontalClassMerging;
+ internal.enableHorizontalClassMerging = false;
assert internal.enableVerticalClassMerging;
internal.enableVerticalClassMerging = false;
assert internal.enableClassStaticizer;
diff --git a/src/main/java/com/android/tools/r8/R8.java b/src/main/java/com/android/tools/r8/R8.java
index b7c538f..94921b7 100644
--- a/src/main/java/com/android/tools/r8/R8.java
+++ b/src/main/java/com/android/tools/r8/R8.java
@@ -44,6 +44,7 @@
import com.android.tools.r8.shaking.ReasonPrinter;
import com.android.tools.r8.shaking.RootSetBuilder;
import com.android.tools.r8.shaking.RootSetBuilder.RootSet;
+import com.android.tools.r8.shaking.StaticClassMerger;
import com.android.tools.r8.shaking.TreePruner;
import com.android.tools.r8.shaking.VerticalClassMerger;
import com.android.tools.r8.utils.AndroidApiLevel;
@@ -353,6 +354,14 @@
timing.end();
}
appView.setGraphLense(new MemberRebindingAnalysis(appViewWithLiveness, options).run());
+ if (options.enableHorizontalClassMerging) {
+ StaticClassMerger staticClassMerger = new StaticClassMerger(appViewWithLiveness);
+ appView.setGraphLense(staticClassMerger.run());
+ appViewWithLiveness.setAppInfo(
+ appViewWithLiveness
+ .appInfo()
+ .rewrittenWithLense(application.asDirect(), appView.graphLense()));
+ }
if (options.enableVerticalClassMerging) {
timing.begin("ClassMerger");
VerticalClassMerger verticalClassMerger =
diff --git a/src/main/java/com/android/tools/r8/R8Command.java b/src/main/java/com/android/tools/r8/R8Command.java
index 7b24770..b5eafcd 100644
--- a/src/main/java/com/android/tools/r8/R8Command.java
+++ b/src/main/java/com/android/tools/r8/R8Command.java
@@ -648,6 +648,7 @@
? LineNumberOptimization.ON
: LineNumberOptimization.OFF;
+ assert internal.enableHorizontalClassMerging || !proguardConfiguration.isOptimizing();
assert internal.enableVerticalClassMerging || !proguardConfiguration.isOptimizing();
if (internal.debug) {
internal.proguardConfiguration.getKeepAttributes().lineNumberTable = true;
@@ -656,6 +657,7 @@
// TODO(zerny): Should we support inlining in debug mode? b/62937285
internal.enableInlining = false;
internal.enableClassInlining = false;
+ internal.enableHorizontalClassMerging = false;
internal.enableVerticalClassMerging = false;
internal.enableClassStaticizer = false;
// TODO(zerny): Should we support outlining in debug mode? b/62937285
diff --git a/src/main/java/com/android/tools/r8/graph/AccessFlags.java b/src/main/java/com/android/tools/r8/graph/AccessFlags.java
index 626e5cc..afbb13f 100644
--- a/src/main/java/com/android/tools/r8/graph/AccessFlags.java
+++ b/src/main/java/com/android/tools/r8/graph/AccessFlags.java
@@ -72,6 +72,10 @@
return visibilityOrdinal() > other.visibilityOrdinal();
}
+ public boolean isAtLeastAsVisibleAs(AccessFlags other) {
+ return visibilityOrdinal() >= other.visibilityOrdinal();
+ }
+
private int visibilityOrdinal() {
// public > protected > package > private
if (isPublic()) {
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 22a771b..0ae75c5 100644
--- a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
+++ b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
@@ -157,6 +157,11 @@
return compilationState != CompilationState.NOT_PROCESSED;
}
+ public boolean isInitializer() {
+ checkIfObsolete();
+ return isInstanceInitializer() || isClassInitializer();
+ }
+
public boolean isInstanceInitializer() {
checkIfObsolete();
return accessFlags.isConstructor() && !accessFlags.isStatic();
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 c7cd614..e188f29 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
@@ -34,6 +34,7 @@
import com.android.tools.r8.ir.code.IRCode;
import com.android.tools.r8.ir.code.Instruction;
import com.android.tools.r8.ir.code.InstructionListIterator;
+import com.android.tools.r8.ir.code.InvokeMethod;
import com.android.tools.r8.ir.code.Value;
import com.android.tools.r8.ir.desugar.CovariantReturnTypeAnnotationTransformer;
import com.android.tools.r8.ir.desugar.InterfaceMethodRewriter;
@@ -732,10 +733,8 @@
printC1VisualizerHeader(method);
printMethod(code, "Initial IR (SSA)");
- DexClass holder = definitionFor(method.method.holder);
- if (method.getCode() != null && method.getCode().isJarCode()
- && holder.hasKotlinInfo()) {
- computeKotlinNotNullParamHints(feedback, holder.getKotlinInfo(), method, code);
+ if (method.getCode() != null && method.getCode().isJarCode()) {
+ computeKotlinNotNullParamHints(feedback, method, code);
}
if (options.canHaveArtStringNewInitBug()) {
@@ -907,11 +906,19 @@
}
private void computeKotlinNotNullParamHints(
- OptimizationFeedback feedback, KotlinInfo kotlinInfo, DexEncodedMethod method, IRCode code) {
+ OptimizationFeedback feedback, DexEncodedMethod method, IRCode code) {
+ DexMethod originalSignature = graphLense().getOriginalMethodSignature(method.method);
+ DexClass originalHolder = definitionFor(originalSignature.holder);
+ if (!originalHolder.hasKotlinInfo()) {
+ return;
+ }
+
// Use non-null parameter hints in Kotlin metadata if available.
+ KotlinInfo kotlinInfo = originalHolder.getKotlinInfo();
if (kotlinInfo.hasNonNullParameterHints()) {
- BitSet hintFromMetadata = kotlinInfo.lookupNonNullParameterHint(
- method.method.name.toString(), method.method.proto.toDescriptorString());
+ BitSet hintFromMetadata =
+ kotlinInfo.lookupNonNullParameterHint(
+ originalSignature.name.toString(), originalSignature.proto.toDescriptorString());
if (hintFromMetadata != null) {
if (hintFromMetadata.length() > 0) {
feedback.setKotlinNotNullParamHints(method, hintFromMetadata);
@@ -934,12 +941,15 @@
// 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) {
+ if (!user.isInvokeStatic()) {
continue;
}
- paramsCheckedForNull.set(index);
+ InvokeMethod invoke = user.asInvokeMethod();
+ DexMethod invokedMethod =
+ appView.graphLense().getOriginalMethodSignature(invoke.getInvokedMethod());
+ if (invokedMethod == checkParameterIsNotNull && user.inValues().indexOf(argument) == 0) {
+ paramsCheckedForNull.set(index);
+ }
}
}
if (paramsCheckedForNull.length() > 0) {
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java b/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
index 2a74cad..1660374 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
@@ -75,7 +75,7 @@
}
public boolean isBlackListed(DexEncodedMethod method) {
- return blackList.contains(method.method)
+ return blackList.contains(appView.graphLense().getOriginalMethodSignature(method.method))
|| appView.appInfo().neverInline.contains(method.method)
|| TwrCloseResourceRewriter.isSynthesizedCloseResourceMethod(method.method, converter);
}
diff --git a/src/main/java/com/android/tools/r8/shaking/StaticClassMerger.java b/src/main/java/com/android/tools/r8/shaking/StaticClassMerger.java
new file mode 100644
index 0000000..5d88374
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/shaking/StaticClassMerger.java
@@ -0,0 +1,386 @@
+// 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.shaking;
+
+import com.android.tools.r8.graph.AppView;
+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.DexField;
+import com.android.tools.r8.graph.DexMethod;
+import com.android.tools.r8.graph.DexProgramClass;
+import com.android.tools.r8.graph.DexString;
+import com.android.tools.r8.graph.DexType;
+import com.android.tools.r8.graph.GraphLense;
+import com.android.tools.r8.graph.GraphLense.NestedGraphLense;
+import com.android.tools.r8.logging.Log;
+import com.android.tools.r8.shaking.Enqueuer.AppInfoWithLiveness;
+import com.android.tools.r8.shaking.VerticalClassMerger.IllegalAccessDetector;
+import com.android.tools.r8.utils.FieldSignatureEquivalence;
+import com.android.tools.r8.utils.MethodSignatureEquivalence;
+import com.google.common.base.Equivalence.Wrapper;
+import com.google.common.collect.BiMap;
+import com.google.common.collect.HashBiMap;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Streams;
+import java.util.ArrayDeque;
+import java.util.Arrays;
+import java.util.Deque;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
+
+/**
+ * This optimization merges all classes that only have static members and private virtual methods.
+ *
+ * <p>If a merge candidate does not access any package-private or protected members, then it is
+ * merged into a global representative. Otherwise it is merged into a representative for its
+ * package. If no such representatives exist, then the merge candidate is promoted to be the
+ * representative.
+ *
+ * <p>Note that, when merging a merge candidate X into Y, this optimization merely moves the members
+ * of X into Y -- it does not change all occurrences of X in the program into Y. This makes the
+ * optimization more applicable, because it would otherwise not be possible to merge two classes if
+ * they inherited from, say, X' and Y' (since multiple inheritance is not allowed).
+ */
+public class StaticClassMerger {
+
+ private final AppView<? extends AppInfoWithLiveness> appView;
+
+ private final Map<String, DexProgramClass> representatives = new HashMap<>();
+
+ private DexProgramClass globalRepresentative = null;
+
+ private final BiMap<DexField, DexField> fieldMapping = HashBiMap.create();
+ private final BiMap<DexMethod, DexMethod> methodMapping = HashBiMap.create();
+
+ public StaticClassMerger(AppView<? extends AppInfoWithLiveness> appView) {
+ this.appView = appView;
+ }
+
+ // TODO(christofferqa): Share top-down traversal with vertical class merger.
+ public GraphLense run() {
+ // Visit classes in top-down order.
+ Deque<DexProgramClass> worklist = new ArrayDeque<>();
+ Set<DexProgramClass> seenBefore = new HashSet<>();
+
+ Iterator<DexProgramClass> classIterator =
+ appView.appInfo().app.classesWithDeterministicOrder().iterator();
+
+ // Visit the program classes in a top-down order according to the class hierarchy.
+ while (classIterator.hasNext() || !worklist.isEmpty()) {
+ if (worklist.isEmpty()) {
+ // Add the ancestors of this class (including the class itself) to the worklist in such a
+ // way that all super types of the class come before the class itself.
+ addAncestorsToWorklist(classIterator.next(), worklist, seenBefore);
+ if (worklist.isEmpty()) {
+ continue;
+ }
+ }
+
+ DexProgramClass clazz = worklist.removeFirst();
+ if (!seenBefore.add(clazz)) {
+ continue;
+ }
+
+ if (satisfiesMergeCriteria(clazz)) {
+ merge(clazz);
+ }
+ }
+
+ BiMap<DexField, DexField> originalFieldSignatures = fieldMapping.inverse();
+ BiMap<DexMethod, DexMethod> originalMethodSignatures = methodMapping.inverse();
+ return new NestedGraphLense(
+ ImmutableMap.of(),
+ methodMapping,
+ fieldMapping,
+ originalFieldSignatures,
+ originalMethodSignatures,
+ appView.graphLense(),
+ appView.dexItemFactory());
+ }
+
+ private void addAncestorsToWorklist(
+ DexProgramClass clazz, Deque<DexProgramClass> worklist, Set<DexProgramClass> seenBefore) {
+ if (seenBefore.contains(clazz)) {
+ return;
+ }
+
+ worklist.addFirst(clazz);
+
+ // Add super classes to worklist.
+ if (clazz.superType != null) {
+ DexClass definition = appView.appInfo().definitionFor(clazz.superType);
+ if (definition != null && definition.isProgramClass()) {
+ addAncestorsToWorklist(definition.asProgramClass(), worklist, seenBefore);
+ }
+ }
+
+ // Add super interfaces to worklist.
+ for (DexType interfaceType : clazz.interfaces.values) {
+ DexClass definition = appView.appInfo().definitionFor(interfaceType);
+ if (definition != null && definition.isProgramClass()) {
+ addAncestorsToWorklist(definition.asProgramClass(), worklist, seenBefore);
+ }
+ }
+ }
+
+ public boolean satisfiesMergeCriteria(DexProgramClass clazz) {
+ if (clazz.accessFlags.isInterface()) {
+ return false;
+ }
+ if (clazz.staticFields().length + clazz.directMethods().length + clazz.virtualMethods().length
+ == 0) {
+ return false;
+ }
+ if (clazz.instanceFields().length > 0) {
+ return false;
+ }
+ if (Arrays.stream(clazz.staticFields())
+ .anyMatch(field -> appView.appInfo().isPinned(field.field))) {
+ return false;
+ }
+ if (Arrays.stream(clazz.directMethods()).anyMatch(DexEncodedMethod::isInitializer)) {
+ return false;
+ }
+ if (!Arrays.stream(clazz.virtualMethods()).allMatch(DexEncodedMethod::isPrivateMethod)) {
+ return false;
+ }
+ if (Streams.stream(clazz.methods())
+ .anyMatch(
+ method ->
+ method.accessFlags.isNative()
+ || appView.appInfo().isPinned(method.method)
+ // TODO(christofferqa): Remove the invariant that the graph lense should not
+ // modify any methods from the sets alwaysInline and noSideEffects.
+ || appView.appInfo().alwaysInline.contains(method.method)
+ || appView.appInfo().noSideEffects.keySet().contains(method))) {
+ return false;
+ }
+ return true;
+ }
+
+ public boolean merge(DexProgramClass clazz) {
+ assert satisfiesMergeCriteria(clazz);
+
+ String pkg = clazz.type.getPackageDescriptor();
+ DexProgramClass representativeForPackage = representatives.get(pkg);
+
+ if (mayMergeAcrossPackageBoundaries(clazz)) {
+ if (globalRepresentative != null) {
+ // Merge this class into the global representative.
+ moveMembersFromSourceToTarget(clazz, globalRepresentative);
+ return true;
+ }
+
+ // Make the current class the global representative as well as the representative for this
+ // package.
+ if (Log.ENABLED) {
+ Log.info(getClass(), "Making %s a global representative", clazz.type.toSourceString(), pkg);
+ Log.info(getClass(), "Making %s a representative for %s", clazz.type.toSourceString(), pkg);
+ }
+ globalRepresentative = clazz;
+ representatives.put(pkg, clazz);
+
+ if (representativeForPackage != null) {
+ // If there was a previous representative for this package, we can merge it into the current
+ // class that has just become the global representative.
+ moveMembersFromSourceToTarget(representativeForPackage, clazz);
+ return true;
+ }
+
+ // No merge.
+ return false;
+ }
+
+ if (representativeForPackage != null) {
+ if (clazz.accessFlags.isMoreVisibleThan(representativeForPackage.accessFlags)) {
+ // Use `clazz` as a representative for this package instead.
+ representatives.put(pkg, clazz);
+ moveMembersFromSourceToTarget(representativeForPackage, clazz);
+ return true;
+ }
+
+ // Merge current class into the representative for this package.
+ moveMembersFromSourceToTarget(clazz, representativeForPackage);
+ return true;
+ }
+
+ // No merge.
+ if (Log.ENABLED) {
+ Log.info(getClass(), "Making %s a representative for %s", clazz.type.toSourceString(), pkg);
+ }
+ representatives.put(pkg, clazz);
+ return false;
+ }
+
+ private boolean mayMergeAcrossPackageBoundaries(DexProgramClass clazz) {
+ // Check that the class is public. Otherwise, accesses to `clazz` from within its current
+ // package may become illegal.
+ if (!clazz.accessFlags.isPublic()) {
+ return false;
+ }
+ // Check that all of the members are private or public.
+ if (!Arrays.stream(clazz.directMethods())
+ .allMatch(method -> method.accessFlags.isPrivate() || method.accessFlags.isPublic())) {
+ return false;
+ }
+ if (!Arrays.stream(clazz.staticFields())
+ .allMatch(method -> method.accessFlags.isPrivate() || method.accessFlags.isPublic())) {
+ return false;
+ }
+
+ // Note that a class is only considered a candidate if it has no instance fields and all of its
+ // virtual methods are private. Therefore, we don't need to consider check if there are any
+ // package-private or protected instance fields or virtual methods here.
+ assert Arrays.stream(clazz.instanceFields()).count() == 0;
+ assert Arrays.stream(clazz.virtualMethods()).allMatch(method -> method.accessFlags.isPrivate());
+
+ // Check that no methods access package-private or protected members.
+ IllegalAccessDetector registry = new IllegalAccessDetector(appView.appInfo(), clazz);
+ for (DexEncodedMethod method : clazz.methods()) {
+ method.registerCodeReferences(registry);
+ if (registry.foundIllegalAccess()) {
+ return false;
+ }
+ }
+ return true;
+ }
+
+ private void moveMembersFromSourceToTarget(
+ DexProgramClass sourceClass, DexProgramClass targetClass) {
+ if (Log.ENABLED) {
+ Log.info(
+ getClass(),
+ "Merging %s into %s",
+ sourceClass.type.toSourceString(),
+ targetClass.type.toSourceString());
+ }
+
+ assert targetClass.accessFlags.isAtLeastAsVisibleAs(sourceClass.accessFlags);
+ assert sourceClass.instanceFields().length == 0;
+ assert targetClass.instanceFields().length == 0;
+
+ // Move members from source to target.
+ targetClass.setDirectMethods(
+ mergeMethods(sourceClass.directMethods(), targetClass.directMethods(), targetClass));
+ targetClass.setVirtualMethods(
+ mergeMethods(sourceClass.virtualMethods(), targetClass.virtualMethods(), targetClass));
+ targetClass.setStaticFields(
+ mergeFields(sourceClass.staticFields(), targetClass.staticFields(), targetClass));
+
+ // Cleanup source.
+ sourceClass.setDirectMethods(DexEncodedMethod.EMPTY_ARRAY);
+ sourceClass.setVirtualMethods(DexEncodedMethod.EMPTY_ARRAY);
+ sourceClass.setStaticFields(DexEncodedField.EMPTY_ARRAY);
+ }
+
+ private DexEncodedMethod[] mergeMethods(
+ DexEncodedMethod[] sourceMethods,
+ DexEncodedMethod[] targetMethods,
+ DexProgramClass targetClass) {
+ DexEncodedMethod[] result = new DexEncodedMethod[sourceMethods.length + targetMethods.length];
+
+ // Move all target methods to result.
+ System.arraycopy(targetMethods, 0, result, 0, targetMethods.length);
+
+ // Move source methods to result one by one, renaming them if needed.
+ MethodSignatureEquivalence equivalence = MethodSignatureEquivalence.get();
+ Set<Wrapper<DexMethod>> existingMethods =
+ Arrays.stream(targetMethods)
+ .map(targetMethod -> equivalence.wrap(targetMethod.method))
+ .collect(Collectors.toSet());
+
+ Predicate<DexMethod> availableMethodSignatures =
+ method -> !existingMethods.contains(equivalence.wrap(method));
+
+ int i = targetMethods.length;
+ for (DexEncodedMethod sourceMethod : sourceMethods) {
+ DexEncodedMethod sourceMethodAfterMove =
+ renameMethodIfNeeded(sourceMethod, targetClass, availableMethodSignatures);
+ result[i++] = sourceMethodAfterMove;
+
+ DexMethod originalMethod =
+ methodMapping.inverse().getOrDefault(sourceMethod.method, sourceMethod.method);
+ methodMapping.put(originalMethod, sourceMethodAfterMove.method);
+ }
+
+ return result;
+ }
+
+ private DexEncodedField[] mergeFields(
+ DexEncodedField[] sourceFields, DexEncodedField[] targetFields, DexProgramClass targetClass) {
+ DexEncodedField[] result = new DexEncodedField[sourceFields.length + targetFields.length];
+
+ // Move all target fields to result.
+ System.arraycopy(targetFields, 0, result, 0, targetFields.length);
+
+ // Move source fields to result one by one, renaming them if needed.
+ FieldSignatureEquivalence equivalence = FieldSignatureEquivalence.get();
+ Set<Wrapper<DexField>> existingFields =
+ Arrays.stream(targetFields)
+ .map(targetField -> equivalence.wrap(targetField.field))
+ .collect(Collectors.toSet());
+
+ Predicate<DexField> availableFieldSignatures =
+ field -> !existingFields.contains(equivalence.wrap(field));
+
+ int i = targetFields.length;
+ for (DexEncodedField sourceField : sourceFields) {
+ DexEncodedField sourceFieldAfterMove =
+ renameFieldIfNeeded(sourceField, targetClass, availableFieldSignatures);
+ result[i++] = sourceFieldAfterMove;
+
+ DexField originalField =
+ fieldMapping.inverse().getOrDefault(sourceField.field, sourceField.field);
+ fieldMapping.put(originalField, sourceFieldAfterMove.field);
+ }
+
+ return result;
+ }
+
+ private DexEncodedMethod renameMethodIfNeeded(
+ DexEncodedMethod method,
+ DexProgramClass targetClass,
+ Predicate<DexMethod> availableMethodSignatures) {
+ assert !method.accessFlags.isConstructor();
+ DexString oldName = method.method.name;
+ DexMethod newSignature =
+ appView.dexItemFactory().createMethod(targetClass.type, method.method.proto, oldName);
+ if (!availableMethodSignatures.test(newSignature)) {
+ int count = 1;
+ do {
+ DexString newName = appView.dexItemFactory().createString(oldName.toSourceString() + count);
+ newSignature =
+ appView.dexItemFactory().createMethod(targetClass.type, method.method.proto, newName);
+ count++;
+ } while (!availableMethodSignatures.test(newSignature));
+ }
+ return method.toTypeSubstitutedMethod(newSignature);
+ }
+
+ private DexEncodedField renameFieldIfNeeded(
+ DexEncodedField field,
+ DexProgramClass targetClass,
+ Predicate<DexField> availableFieldSignatures) {
+ DexString oldName = field.field.name;
+ DexField newSignature =
+ appView.dexItemFactory().createField(targetClass.type, field.field.type, oldName);
+ if (!availableFieldSignatures.test(newSignature)) {
+ int count = 1;
+ do {
+ DexString newName = appView.dexItemFactory().createString(oldName.toSourceString() + count);
+ newSignature =
+ appView.dexItemFactory().createField(targetClass.type, field.field.type, newName);
+ count++;
+ } while (!availableFieldSignatures.test(newSignature));
+ }
+ return field.toTypeSubstitutedField(newSignature);
+ }
+}
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 d41bd69..2824718 100644
--- a/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
+++ b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
@@ -8,6 +8,7 @@
import com.android.tools.r8.errors.CompilationError;
import com.android.tools.r8.errors.Unreachable;
+import com.android.tools.r8.graph.AppInfo;
import com.android.tools.r8.graph.AppInfo.ResolutionResult;
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.Code;
@@ -18,7 +19,6 @@
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.DexItemFactory;
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexProgramClass;
import com.android.tools.r8.graph.DexProto;
@@ -477,7 +477,7 @@
// Check that all accesses from [source] to classes or members from the current package of
// [source] will continue to work. This is guaranteed if the methods of [source] do not access
// any private or protected classes or members from the current package of [source].
- IllegalAccessDetector registry = new IllegalAccessDetector(options.itemFactory, source);
+ IllegalAccessDetector registry = new IllegalAccessDetector(appInfo, source);
for (DexEncodedMethod method : source.methods()) {
method.registerCodeReferences(registry);
if (registry.foundIllegalAccess()) {
@@ -1334,7 +1334,7 @@
private class TreeFixer {
private final Builder lense = GraphLense.builder();
- Map<DexProto, DexProto> protoFixupCache = new IdentityHashMap<>();
+ private final Map<DexProto, DexProto> protoFixupCache = new IdentityHashMap<>();
private GraphLense fixupTypeReferences(GraphLense graphLense) {
// Globally substitute merged class types in protos and holders.
@@ -1345,7 +1345,7 @@
clazz.setStaticFields(substituteTypesIn(clazz.staticFields()));
clazz.setInstanceFields(substituteTypesIn(clazz.instanceFields()));
}
- // Record type renamings so instanceof and checkcast checks are also fixed.
+ // Record type renamings so check-cast and instance-of checks are also fixed.
for (DexType type : mergedClasses.keySet()) {
DexType fixed = fixupType(type);
lense.map(type, fixed);
@@ -1649,12 +1649,16 @@
// Searches for a reference to a non-public class, field or method declared in the same package
// as [source].
- private class IllegalAccessDetector extends UseRegistry {
- private boolean foundIllegalAccess = false;
- private DexClass source;
+ public static class IllegalAccessDetector extends UseRegistry {
- public IllegalAccessDetector(DexItemFactory factory, DexClass source) {
- super(factory);
+ private boolean foundIllegalAccess = false;
+
+ private final AppInfo appInfo;
+ private final DexClass source;
+
+ public IllegalAccessDetector(AppInfo appInfo, DexClass source) {
+ super(appInfo.dexItemFactory);
+ this.appInfo = appInfo;
this.source = source;
}
@@ -1664,7 +1668,8 @@
private boolean checkFieldReference(DexField field) {
if (!foundIllegalAccess) {
- if (field.clazz.isSamePackage(source.type)) {
+ DexType baseType = field.clazz.toBaseType(appInfo.dexItemFactory);
+ if (baseType.isClassType() && baseType.isSamePackage(source.type)) {
checkTypeReference(field.clazz);
checkTypeReference(field.type);
@@ -1679,7 +1684,8 @@
private boolean checkMethodReference(DexMethod method) {
if (!foundIllegalAccess) {
- if (method.holder.isSamePackage(source.type)) {
+ DexType baseType = method.holder.toBaseType(appInfo.dexItemFactory);
+ if (baseType.isClassType() && baseType.isSamePackage(source.type)) {
checkTypeReference(method.holder);
checkTypeReference(method.proto.returnType);
for (DexType type : method.proto.parameters.values) {
@@ -1696,8 +1702,9 @@
private boolean checkTypeReference(DexType type) {
if (!foundIllegalAccess) {
- if (type.isClassType() && type.isSamePackage(source.type)) {
- DexClass clazz = appInfo.definitionFor(type);
+ DexType baseType = type.toBaseType(appInfo.dexItemFactory);
+ if (baseType.isClassType() && baseType.isSamePackage(source.type)) {
+ DexClass clazz = appInfo.definitionFor(baseType);
if (clazz == null || !clazz.accessFlags.isPublic()) {
foundIllegalAccess = true;
}
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 65d1e3a..c76841d 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -79,6 +79,7 @@
itemFactory = proguardConfiguration.getDexItemFactory();
// -dontoptimize disables optimizations by flipping related flags.
if (!proguardConfiguration.isOptimizing()) {
+ enableHorizontalClassMerging = false;
enableVerticalClassMerging = false;
enableDevirtualization = false;
enableNonNullTracking = false;
@@ -97,6 +98,7 @@
public boolean passthroughDexCode = false;
// Optimization-related flags. These should conform to -dontoptimize.
+ public boolean enableHorizontalClassMerging = true;
public boolean enableVerticalClassMerging = true;
public boolean enableDevirtualization = true;
public boolean enableNonNullTracking = true;
diff --git a/src/test/java/com/android/tools/r8/TestBuilder.java b/src/test/java/com/android/tools/r8/TestBuilder.java
index 6d5a158..7397c39 100644
--- a/src/test/java/com/android/tools/r8/TestBuilder.java
+++ b/src/test/java/com/android/tools/r8/TestBuilder.java
@@ -53,7 +53,15 @@
}
public T addProgramClassesAndInnerClasses(Collection<Class<?>> classes) throws IOException {
- return addProgramFiles(getFilesForClassesAndInnerClasses(classes));
+ return addProgramClasses(classes).addInnerClasses(classes);
+ }
+
+ public T addInnerClasses(Class<?>... classes) throws IOException {
+ return addInnerClasses(Arrays.asList(classes));
+ }
+
+ public T addInnerClasses(Collection<Class<?>> classes) throws IOException {
+ return addProgramFiles(getFilesForInnerClasses(classes));
}
public abstract T addLibraryFiles(Collection<Path> files);
@@ -74,15 +82,14 @@
return ListUtils.map(classes, ToolHelper::getClassFileForTestClass);
}
- static Collection<Path> getFilesForClassesAndInnerClasses(Collection<Class<?>> classes)
- throws IOException {
+ static Collection<Path> getFilesForInnerClasses(Collection<Class<?>> classes) throws IOException {
Set<Path> paths = new HashSet<>();
for (Class clazz : classes) {
Path path = ToolHelper.getClassFileForTestClass(clazz);
String prefix = path.toString().replace(CLASS_EXTENSION, "$");
paths.addAll(
ToolHelper.getClassFilesForTestDirectory(
- path.getParent(), p -> p.equals(path) || p.toString().startsWith(prefix)));
+ path.getParent(), p -> p.toString().startsWith(prefix)));
}
return paths;
}
diff --git a/src/test/java/com/android/tools/r8/classmerging/StaticClassMergerTest.java b/src/test/java/com/android/tools/r8/classmerging/StaticClassMergerTest.java
new file mode 100644
index 0000000..2d2788c
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/classmerging/StaticClassMergerTest.java
@@ -0,0 +1,87 @@
+// 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.classmerging;
+
+import static org.junit.Assert.assertEquals;
+
+import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.utils.StringUtils;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import com.android.tools.r8.utils.codeinspector.FoundClassSubject;
+import java.util.List;
+import java.util.stream.Collectors;
+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 StaticClassMergerTest extends TestBase {
+
+ static class StaticMergeCandidateA {
+
+ @NeverInline
+ public static String m() {
+ return "StaticMergeCandidateA.m()";
+ }
+ }
+
+ static class StaticMergeCandidateB {
+
+ @NeverInline
+ public static String m() {
+ return "StaticMergeCandidateB.m()";
+ }
+ }
+
+ static class TestClass {
+
+ public static void main(String[] args) {
+ System.out.println(StaticMergeCandidateA.m());
+ System.out.print(StaticMergeCandidateB.m());
+ }
+ }
+
+ private Backend backend;
+
+ public StaticClassMergerTest(Backend backend) {
+ this.backend = backend;
+ }
+
+ @Parameters(name = "Backend: {0}")
+ public static Backend[] data() {
+ return Backend.values();
+ }
+
+ @Test
+ public void testStaticClassIsRemoved() throws Exception {
+ String expected =
+ StringUtils.joinLines("StaticMergeCandidateA.m()", "StaticMergeCandidateB.m()");
+
+ testForJvm().addTestClasspath().run(TestClass.class).assertSuccessWithOutput(expected);
+
+ CodeInspector inspector =
+ testForR8(backend)
+ .addInnerClasses(StaticClassMergerTest.class)
+ .addKeepMainRule(TestClass.class)
+ .addOptionsModification(options -> options.enableMinification = false)
+ .enableInliningAnnotations()
+ .run(TestClass.class)
+ .assertSuccessWithOutput(expected)
+ .inspector();
+
+ // Check that one of the two static merge candidates has been removed
+ List<FoundClassSubject> classes =
+ inspector.allClasses().stream()
+ .filter(clazz -> clazz.getOriginalName().contains("StaticMergeCandidate"))
+ .collect(Collectors.toList());
+ assertEquals(1, classes.size());
+
+ // Check that the remaining static merge candidate has two methods.
+ FoundClassSubject remaining = classes.get(0);
+ assertEquals(2, remaining.allMethods().size());
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/classmerging/ClassMergingTest.java b/src/test/java/com/android/tools/r8/classmerging/VerticalClassMergerTest.java
similarity index 99%
rename from src/test/java/com/android/tools/r8/classmerging/ClassMergingTest.java
rename to src/test/java/com/android/tools/r8/classmerging/VerticalClassMergerTest.java
index 7a1f831..29f4605 100644
--- a/src/test/java/com/android/tools/r8/classmerging/ClassMergingTest.java
+++ b/src/test/java/com/android/tools/r8/classmerging/VerticalClassMergerTest.java
@@ -61,7 +61,7 @@
// TODO(christofferqa): Add tests to check that statically typed invocations on method handles
// continue to work after class merging.
@RunWith(VmTestRunner.class)
-public class ClassMergingTest extends TestBase {
+public class VerticalClassMergerTest extends TestBase {
private static final Path CF_DIR =
Paths.get(ToolHelper.BUILD_DIR).resolve("test/examples/classes/classmerging");
@@ -1154,7 +1154,7 @@
public void test(AndroidApp app, Path proguardMapPath) throws Throwable {
Path appPath =
- File.createTempFile("app", ".zip", ClassMergingTest.this.temp.getRoot()).toPath();
+ File.createTempFile("app", ".zip", VerticalClassMergerTest.this.temp.getRoot()).toPath();
app.writeToZip(appPath, OutputMode.DexIndexed);
DexDebugTestConfig config = new DexDebugTestConfig(appPath);
diff --git a/src/test/java/com/android/tools/r8/naming/b116840216/ReserveOuterClassNameTest.java b/src/test/java/com/android/tools/r8/naming/b116840216/ReserveOuterClassNameTest.java
index b1881ee..b694949 100644
--- a/src/test/java/com/android/tools/r8/naming/b116840216/ReserveOuterClassNameTest.java
+++ b/src/test/java/com/android/tools/r8/naming/b116840216/ReserveOuterClassNameTest.java
@@ -15,6 +15,7 @@
import com.android.tools.r8.ToolHelper;
import com.android.tools.r8.origin.Origin;
import com.android.tools.r8.utils.AndroidApp;
+import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.codeinspector.ClassSubject;
import com.android.tools.r8.utils.codeinspector.CodeInspector;
import com.android.tools.r8.utils.codeinspector.MethodSubject;
@@ -84,7 +85,7 @@
Origin.unknown());
ToolHelper.allowTestProguardOptions(builder);
- AndroidApp processedApp = ToolHelper.runR8(builder.build());
+ AndroidApp processedApp = ToolHelper.runR8(builder.build(), this::configure);
CodeInspector inspector = new CodeInspector(processedApp);
ClassSubject mainSubject = inspector.clazz(mainClass);
@@ -109,6 +110,11 @@
assertThat(foo, isRenamed());
}
+ private void configure(InternalOptions options) {
+ // Disable horizontal class merging to avoid that all members of Outer are moved to Outer$Inner
+ // or vice versa (both Outer and Outer$Inner are merge candidates for the static class merger).
+ options.enableHorizontalClassMerging = false;
+ }
@Test
public void test_keepOuterName() throws Exception {
diff --git a/src/test/java/com/android/tools/r8/shaking/testrules/ForceInlineTest.java b/src/test/java/com/android/tools/r8/shaking/testrules/ForceInlineTest.java
index f37d6c6..1b7ebe9 100644
--- a/src/test/java/com/android/tools/r8/shaking/testrules/ForceInlineTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/testrules/ForceInlineTest.java
@@ -20,6 +20,7 @@
import com.android.tools.r8.TestBase;
import com.android.tools.r8.ToolHelper;
import com.android.tools.r8.origin.Origin;
+import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.codeinspector.ClassSubject;
import com.android.tools.r8.utils.codeinspector.CodeInspector;
import com.google.common.collect.ImmutableList;
@@ -61,7 +62,13 @@
.addLibraryFiles(library);
ToolHelper.allowTestProguardOptions(builder);
builder.addProguardConfiguration(proguardConfiguration, Origin.unknown());
- return new CodeInspector(ToolHelper.runR8(builder.build()));
+ return new CodeInspector(ToolHelper.runR8(builder.build(), this::configure));
+ }
+
+ private void configure(InternalOptions options) {
+ // Disable horizontal class merging to prevent that A and C are merged (both classes are
+ // candidates for the static class merger).
+ options.enableHorizontalClassMerging = false;
}
@Test
