Group equivalent instance initializers in same mergers
This introduces a new (yet to be implemented) InstanceInitializerAnalysis that given an InstanceInitializer returns an InstanceInitializerDescription.
The InstanceInitializerDescription is an abstraction of the instance initializer's code, and is used to group instance initializers into the same mergers, such that they will be mapped to the same constructor in the residual program.
Bug: 189296638
Change-Id: Ib3d804b4ac077b32e9142bd21dc2dc91b9563258
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassMerger.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassMerger.java
index f570f06..89e9f74 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassMerger.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassMerger.java
@@ -70,28 +70,22 @@
private final ClassStaticFieldsMerger classStaticFieldsMerger;
private final ClassInstanceFieldsMerger classInstanceFieldsMerger;
private final Collection<VirtualMethodMerger> virtualMethodMergers;
- private final InstanceInitializerMergerCollection instanceInitializerMergers;
private ClassMerger(
AppView<? extends AppInfoWithClassHierarchy> appView,
Mode mode,
HorizontalClassMergerGraphLens.Builder lensBuilder,
MergeGroup group,
- Collection<VirtualMethodMerger> virtualMethodMergers,
- InstanceInitializerMergerCollection instanceInitializerMergers,
- ClassInitializerMerger classInitializerMerger) {
+ Collection<VirtualMethodMerger> virtualMethodMergers) {
this.appView = appView;
- this.mode = mode;
- this.lensBuilder = lensBuilder;
- this.group = group;
- this.virtualMethodMergers = virtualMethodMergers;
- this.instanceInitializerMergers = instanceInitializerMergers;
-
this.dexItemFactory = appView.dexItemFactory();
- this.classInitializerMerger = classInitializerMerger;
+ this.group = group;
+ this.lensBuilder = lensBuilder;
+ this.mode = mode;
+ this.virtualMethodMergers = virtualMethodMergers;
+ this.classInitializerMerger = ClassInitializerMerger.create(group);
this.classStaticFieldsMerger = new ClassStaticFieldsMerger(appView, lensBuilder, group);
this.classInstanceFieldsMerger = new ClassInstanceFieldsMerger(appView, lensBuilder, group);
-
buildClassIdentifierMap();
}
@@ -103,10 +97,9 @@
void mergeDirectMethods(
SyntheticArgumentClass syntheticArgumentClass,
SyntheticClassInitializerConverter.Builder syntheticClassInitializerConverterBuilder) {
+ mergeInstanceInitializers(syntheticArgumentClass);
mergeStaticClassInitializers(syntheticClassInitializerConverterBuilder);
- mergeDirectMethods(group.getTarget());
- group.forEachSource(this::mergeDirectMethods);
- mergeConstructors(syntheticArgumentClass);
+ group.forEach(this::mergeDirectMethods);
}
void mergeStaticClassInitializers(
@@ -181,13 +174,23 @@
classMethodsBuilder::isFresh);
}
- void mergeConstructors(SyntheticArgumentClass syntheticArgumentClass) {
+ void mergeInstanceInitializers(SyntheticArgumentClass syntheticArgumentClass) {
+ InstanceInitializerMergerCollection instanceInitializerMergers =
+ InstanceInitializerMergerCollection.create(appView, group, lensBuilder, mode);
instanceInitializerMergers.forEach(
merger ->
merger.merge(
classMethodsBuilder, lensBuilder, classIdentifiers, syntheticArgumentClass));
}
+ void mergeMethods(
+ SyntheticArgumentClass syntheticArgumentClass,
+ SyntheticClassInitializerConverter.Builder syntheticClassInitializerConverterBuilder) {
+ mergeVirtualMethods();
+ mergeDirectMethods(syntheticArgumentClass, syntheticClassInitializerConverterBuilder);
+ classMethodsBuilder.setClassMethods(group.getTarget());
+ }
+
void mergeVirtualMethods() {
virtualMethodMergers.forEach(
merger -> merger.merge(classMethodsBuilder, lensBuilder, classIdentifiers));
@@ -223,12 +226,6 @@
classInstanceFieldsMerger.setClassIdField(classIdField);
}
- void mergeStaticFields() {
- group.forEachSource(classStaticFieldsMerger::addFields);
- classStaticFieldsMerger.merge(group.getTarget());
- group.forEachSource(clazz -> clazz.setStaticFields(null));
- }
-
void fixAccessFlags() {
if (Iterables.any(group.getSources(), not(DexProgramClass::isAbstract))) {
group.getTarget().getAccessFlags().demoteFromAbstract();
@@ -289,6 +286,14 @@
group.getTarget().setInterfaces(DexTypeList.create(interfaces));
}
+ void mergeFields() {
+ if (group.hasClassIdField()) {
+ appendClassIdField();
+ }
+ mergeInstanceFields();
+ mergeStaticFields();
+ }
+
void mergeInstanceFields() {
group.forEachSource(
clazz -> {
@@ -298,25 +303,21 @@
group.getTarget().setInstanceFields(classInstanceFieldsMerger.merge());
}
+ void mergeStaticFields() {
+ group.forEachSource(classStaticFieldsMerger::addFields);
+ classStaticFieldsMerger.merge(group.getTarget());
+ group.forEachSource(clazz -> clazz.setStaticFields(null));
+ }
+
public void mergeGroup(
SyntheticArgumentClass syntheticArgumentClass,
SyntheticClassInitializerConverter.Builder syntheticClassInitializerConverterBuilder) {
fixAccessFlags();
fixNestMemberAttributes();
-
- if (group.hasClassIdField()) {
- appendClassIdField();
- }
-
mergeAnnotations();
mergeInterfaces();
-
- mergeVirtualMethods();
- mergeDirectMethods(syntheticArgumentClass, syntheticClassInitializerConverterBuilder);
- classMethodsBuilder.setClassMethods(group.getTarget());
-
- mergeStaticFields();
- mergeInstanceFields();
+ mergeFields();
+ mergeMethods(syntheticArgumentClass, syntheticClassInitializerConverterBuilder);
}
public static class Builder {
@@ -357,17 +358,6 @@
appView.testing().horizontalClassMergingTarget.apply(appView, candidates, target));
}
- private ClassInitializerMerger createClassInitializerMerger() {
- ClassInitializerMerger.Builder builder = new ClassInitializerMerger.Builder();
- group.forEach(
- clazz -> {
- if (clazz.hasClassInitializer()) {
- builder.add(clazz.getProgramClassInitializer());
- }
- });
- return builder.build();
- }
-
private List<VirtualMethodMerger> createVirtualMethodMergers() {
Map<DexMethodSignature, VirtualMethodMerger.Builder> virtualMethodMergerBuilders =
new LinkedHashMap<>();
@@ -410,14 +400,7 @@
createClassIdField();
}
- return new ClassMerger(
- appView,
- mode,
- lensBuilder,
- group,
- virtualMethodMergers,
- InstanceInitializerMergerCollection.create(appView, group, mode),
- createClassInitializerMerger());
+ return new ClassMerger(appView, mode, lensBuilder, group, virtualMethodMergers);
}
}
}
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerAnalysis.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerAnalysis.java
new file mode 100644
index 0000000..febdbe6
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerAnalysis.java
@@ -0,0 +1,17 @@
+// Copyright (c) 2021, 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.horizontalclassmerging;
+
+import com.android.tools.r8.graph.ProgramMethod;
+
+public class InstanceInitializerAnalysis {
+
+ public static InstanceInitializerDescription analyze(
+ ProgramMethod instanceInitializer, HorizontalClassMergerGraphLens.Builder lensBuilder) {
+ // TODO(b/189296638): Return an InstanceInitializerDescription if the given instance initializer
+ // is parent constructor call followed by a sequence of instance field puts.
+ return null;
+ }
+}
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerDescription.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerDescription.java
new file mode 100644
index 0000000..41f2c4e
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerDescription.java
@@ -0,0 +1,91 @@
+// Copyright (c) 2021, 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.horizontalclassmerging;
+
+import com.android.tools.r8.graph.DexField;
+import com.android.tools.r8.graph.DexMethod;
+import com.android.tools.r8.graph.ProgramMethod;
+import com.android.tools.r8.ir.optimize.info.field.InstanceFieldInitializationInfo;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+
+/**
+ * A simple abstraction of an instance initializer's code, which allows a parent constructor call
+ * followed by a sequence of instance-put instructions.
+ */
+public class InstanceInitializerDescription {
+
+ private final int arity;
+ private final Map<DexField, InstanceFieldInitializationInfo> instanceFieldAssignments;
+ private final DexMethod parentConstructor;
+ private final List<InstanceFieldInitializationInfo> parentConstructorArguments;
+
+ InstanceInitializerDescription(
+ int arity,
+ Map<DexField, InstanceFieldInitializationInfo> instanceFieldAssignments,
+ DexMethod parentConstructor,
+ List<InstanceFieldInitializationInfo> parentConstructorArguments) {
+ this.arity = arity;
+ this.instanceFieldAssignments = instanceFieldAssignments;
+ this.parentConstructor = parentConstructor;
+ this.parentConstructorArguments = parentConstructorArguments;
+ }
+
+ public static Builder builder(ProgramMethod instanceInitializer) {
+ return new Builder(instanceInitializer);
+ }
+
+ @Override
+ public boolean equals(Object obj) {
+ if (obj == null || getClass() != obj.getClass()) {
+ return false;
+ }
+ InstanceInitializerDescription description = (InstanceInitializerDescription) obj;
+ return arity == description.arity
+ && instanceFieldAssignments.equals(description.instanceFieldAssignments)
+ && parentConstructor == description.parentConstructor
+ && parentConstructorArguments.equals(description.parentConstructorArguments);
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(
+ arity, instanceFieldAssignments, parentConstructor, parentConstructorArguments);
+ }
+
+ public static class Builder {
+
+ private final int arity;
+
+ private Map<DexField, InstanceFieldInitializationInfo> instanceFieldAssignments =
+ new LinkedHashMap<>();
+ private DexMethod parentConstructor;
+ private List<InstanceFieldInitializationInfo> parentConstructorArguments;
+
+ Builder(ProgramMethod method) {
+ this.arity = method.getReference().getArity();
+ }
+
+ public void addInstancePut(DexField field, InstanceFieldInitializationInfo value) {
+ instanceFieldAssignments.put(field, value);
+ }
+
+ public void addInvokeConstructor(
+ DexMethod method, List<InstanceFieldInitializationInfo> arguments) {
+ assert parentConstructor == null;
+ assert parentConstructorArguments == null;
+ parentConstructor = method;
+ parentConstructorArguments = arguments;
+ }
+
+ public InstanceInitializerDescription build() {
+ assert parentConstructor != null;
+ return new InstanceInitializerDescription(
+ arity, instanceFieldAssignments, parentConstructor, parentConstructorArguments);
+ }
+ }
+}
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerMerger.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerMerger.java
index b1f02a9..c2063f3 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerMerger.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerMerger.java
@@ -43,14 +43,25 @@
private final DexItemFactory dexItemFactory;
private final MergeGroup group;
private final List<ProgramMethod> instanceInitializers;
+ private final InstanceInitializerDescription instanceInitializerDescription;
private final Mode mode;
InstanceInitializerMerger(
AppView<?> appView, MergeGroup group, List<ProgramMethod> instanceInitializers, Mode mode) {
+ this(appView, group, instanceInitializers, mode, null);
+ }
+
+ InstanceInitializerMerger(
+ AppView<?> appView,
+ MergeGroup group,
+ List<ProgramMethod> instanceInitializers,
+ Mode mode,
+ InstanceInitializerDescription instanceInitializerDescription) {
this.appView = appView;
this.dexItemFactory = appView.dexItemFactory();
this.group = group;
this.instanceInitializers = instanceInitializers;
+ this.instanceInitializerDescription = instanceInitializerDescription;
this.mode = mode;
// Constructors should not be empty and all constructors should have the same prototype.
@@ -74,6 +85,14 @@
return instanceInitializers.iterator().next().getReference().getArity();
}
+ public List<ProgramMethod> getInstanceInitializers() {
+ return instanceInitializers;
+ }
+
+ public int size() {
+ return instanceInitializers.size();
+ }
+
public static class Builder {
private final AppView<? extends AppInfoWithClassHierarchy> appView;
@@ -107,6 +126,11 @@
return this;
}
+ public Builder addEquivalent(ProgramMethod instanceInitializer) {
+ ListUtils.last(instanceInitializerGroups).add(instanceInitializer);
+ return this;
+ }
+
public List<InstanceInitializerMerger> build(MergeGroup group) {
assert instanceInitializerGroups.stream().noneMatch(List::isEmpty);
return ListUtils.map(
@@ -114,6 +138,15 @@
instanceInitializers ->
new InstanceInitializerMerger(appView, group, instanceInitializers, mode));
}
+
+ public InstanceInitializerMerger buildSingle(
+ MergeGroup group, InstanceInitializerDescription instanceInitializerDescription) {
+ assert instanceInitializerGroups.stream().noneMatch(List::isEmpty);
+ assert instanceInitializerGroups.size() == 1;
+ List<ProgramMethod> instanceInitializers = ListUtils.first(instanceInitializerGroups);
+ return new InstanceInitializerMerger(
+ appView, group, instanceInitializers, mode, instanceInitializerDescription);
+ }
}
// Returns true if we can simply use an existing constructor as the new constructor.
@@ -198,6 +231,8 @@
HorizontalClassMergerGraphLens.Builder lensBuilder,
Reference2IntMap<DexType> classIdentifiers,
SyntheticArgumentClass syntheticArgumentClass) {
+ // TODO(b/189296638): Handle merging of equivalent constructors when
+ // `instanceInitializerDescription` is set.
if (isTrivialMerge(classMethodsBuilder)) {
mergeTrivial(classMethodsBuilder, lensBuilder);
return;
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerMergerCollection.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerMergerCollection.java
index eec3f96..ed7e099 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerMergerCollection.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerMergerCollection.java
@@ -4,12 +4,15 @@
package com.android.tools.r8.horizontalclassmerging;
+import static com.android.tools.r8.utils.MapUtils.ignoreKey;
+
import com.android.tools.r8.graph.AppInfoWithClassHierarchy;
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexEncodedMethod;
import com.android.tools.r8.graph.DexProto;
import com.android.tools.r8.horizontalclassmerging.HorizontalClassMerger.Mode;
import com.android.tools.r8.horizontalclassmerging.InstanceInitializerMerger.Builder;
+import com.android.tools.r8.utils.collections.ProgramMethodSet;
import java.util.ArrayList;
import java.util.Comparator;
import java.util.LinkedHashMap;
@@ -20,47 +23,89 @@
public class InstanceInitializerMergerCollection {
private final List<InstanceInitializerMerger> instanceInitializerMergers;
+ private final Map<InstanceInitializerDescription, InstanceInitializerMerger>
+ equivalentInstanceInitializerMergers;
private InstanceInitializerMergerCollection(
- List<InstanceInitializerMerger> instanceInitializerMergers) {
+ List<InstanceInitializerMerger> instanceInitializerMergers,
+ Map<InstanceInitializerDescription, InstanceInitializerMerger>
+ equivalentInstanceInitializerMergers) {
+ assert equivalentInstanceInitializerMergers.isEmpty();
this.instanceInitializerMergers = instanceInitializerMergers;
+ this.equivalentInstanceInitializerMergers = equivalentInstanceInitializerMergers;
}
public static InstanceInitializerMergerCollection create(
- AppView<? extends AppInfoWithClassHierarchy> appView, MergeGroup group, Mode mode) {
+ AppView<? extends AppInfoWithClassHierarchy> appView,
+ MergeGroup group,
+ HorizontalClassMergerGraphLens.Builder lensBuilder,
+ Mode mode) {
+ // Create an instance initializer merger for each group of instance initializers that are
+ // equivalent.
+ Map<InstanceInitializerDescription, Builder> buildersByDescription = new LinkedHashMap<>();
+ ProgramMethodSet buildersWithoutDescription = ProgramMethodSet.createLinked();
+ group.forEach(
+ clazz ->
+ clazz.forEachProgramDirectMethodMatching(
+ DexEncodedMethod::isInstanceInitializer,
+ instanceInitializer -> {
+ InstanceInitializerDescription description =
+ InstanceInitializerAnalysis.analyze(instanceInitializer, lensBuilder);
+ if (description != null) {
+ buildersByDescription
+ .computeIfAbsent(
+ description,
+ ignoreKey(() -> new InstanceInitializerMerger.Builder(appView, mode)))
+ .addEquivalent(instanceInitializer);
+ } else {
+ buildersWithoutDescription.add(instanceInitializer);
+ }
+ }));
+
+ Map<InstanceInitializerDescription, InstanceInitializerMerger>
+ equivalentInstanceInitializerMergers = new LinkedHashMap<>();
+ buildersByDescription.forEach(
+ (description, builder) -> {
+ InstanceInitializerMerger instanceInitializerMerger =
+ builder.buildSingle(group, description);
+ if (instanceInitializerMerger.size() == 1) {
+ // If there is only one constructor with a specific behavior, then consider it for
+ // normal instance initializer merging below.
+ buildersWithoutDescription.addAll(instanceInitializerMerger.getInstanceInitializers());
+ } else {
+ equivalentInstanceInitializerMergers.put(description, instanceInitializerMerger);
+ }
+ });
+
+ // Merge instance initializers with different behavior.
List<InstanceInitializerMerger> instanceInitializerMergers = new ArrayList<>();
if (appView.options().horizontalClassMergerOptions().isConstructorMergingEnabled()) {
Map<DexProto, Builder> buildersByProto = new LinkedHashMap<>();
- group.forEach(
- clazz ->
- clazz.forEachProgramDirectMethodMatching(
- DexEncodedMethod::isInstanceInitializer,
- method ->
- buildersByProto
- .computeIfAbsent(
- method.getDefinition().getProto(),
- ignore -> new InstanceInitializerMerger.Builder(appView, mode))
- .add(method)));
+ buildersWithoutDescription.forEach(
+ instanceInitializer ->
+ buildersByProto
+ .computeIfAbsent(
+ instanceInitializer.getDefinition().getProto(),
+ ignore -> new InstanceInitializerMerger.Builder(appView, mode))
+ .add(instanceInitializer));
for (InstanceInitializerMerger.Builder builder : buildersByProto.values()) {
instanceInitializerMergers.addAll(builder.build(group));
}
} else {
- group.forEach(
- clazz ->
- clazz.forEachProgramDirectMethodMatching(
- DexEncodedMethod::isInstanceInitializer,
- method ->
- instanceInitializerMergers.addAll(
- new InstanceInitializerMerger.Builder(appView, mode)
- .add(method)
- .build(group))));
+ buildersWithoutDescription.forEach(
+ instanceInitializer ->
+ instanceInitializerMergers.addAll(
+ new InstanceInitializerMerger.Builder(appView, mode)
+ .add(instanceInitializer)
+ .build(group)));
}
// Try and merge the constructors with the most arguments first, to avoid using synthetic
// arguments if possible.
instanceInitializerMergers.sort(
Comparator.comparing(InstanceInitializerMerger::getArity).reversed());
- return new InstanceInitializerMergerCollection(instanceInitializerMergers);
+ return new InstanceInitializerMergerCollection(
+ instanceInitializerMergers, equivalentInstanceInitializerMergers);
}
public void forEach(Consumer<InstanceInitializerMerger> consumer) {
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/code/ClassInitializerMerger.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/code/ClassInitializerMerger.java
index a720407..11fa11e 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/code/ClassInitializerMerger.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/code/ClassInitializerMerger.java
@@ -23,6 +23,7 @@
import com.android.tools.r8.graph.ProgramMethod;
import com.android.tools.r8.graph.RewrittenPrototypeDescription;
import com.android.tools.r8.graph.UseRegistry;
+import com.android.tools.r8.horizontalclassmerging.MergeGroup;
import com.android.tools.r8.ir.code.BasicBlock;
import com.android.tools.r8.ir.code.IRCode;
import com.android.tools.r8.ir.code.IRMetadata;
@@ -56,6 +57,17 @@
this.classInitializers = classInitializers;
}
+ public static ClassInitializerMerger create(MergeGroup group) {
+ ClassInitializerMerger.Builder builder = new ClassInitializerMerger.Builder();
+ group.forEach(
+ clazz -> {
+ if (clazz.hasClassInitializer()) {
+ builder.add(clazz.getProgramClassInitializer());
+ }
+ });
+ return builder.build();
+ }
+
public boolean isEmpty() {
return classInitializers.isEmpty();
}
