Improve performance of if rules with fixed class name
This improves the build speed of an app that contains thousands of Moshi generated -if rules by 10-20%.
Bug: b/378464445
Change-Id: I8c84281c6d2ff3d08ccf81962a0541ed87077571
diff --git a/src/main/java/com/android/tools/r8/shaking/IfRuleEvaluator.java b/src/main/java/com/android/tools/r8/shaking/IfRuleEvaluator.java
index 583380c..4dcaed1 100644
--- a/src/main/java/com/android/tools/r8/shaking/IfRuleEvaluator.java
+++ b/src/main/java/com/android/tools/r8/shaking/IfRuleEvaluator.java
@@ -12,6 +12,7 @@
import com.android.tools.r8.graph.DexEncodedField;
import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.graph.DexProgramClass;
+import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.graph.SubtypingInfo;
import com.android.tools.r8.shaking.RootSetUtils.ConsequentRootSetBuilder;
import com.android.tools.r8.shaking.RootSetUtils.RootSetBuilder;
@@ -19,6 +20,7 @@
import com.android.tools.r8.threading.TaskCollection;
import com.android.tools.r8.utils.MapUtils;
import com.android.tools.r8.utils.Pair;
+import com.android.tools.r8.utils.Timing;
import com.android.tools.r8.utils.UncheckedExecutionException;
import com.android.tools.r8.utils.collections.ProgramMethodSet;
import com.google.common.base.Equivalence.Wrapper;
@@ -122,8 +124,10 @@
public void processActiveIfRulesWithoutMembers(
Map<Wrapper<ProguardIfRule>, Set<ProguardIfRule>> ifRules,
- Iterable<DexProgramClass> newlyLiveClasses)
+ Map<DexType, DexProgramClass> newlyLiveClasses,
+ Timing timing)
throws ExecutionException {
+ List<DexProgramClass> classMatches = new ArrayList<>(newlyLiveClasses.size());
MapUtils.removeIf(
ifRules,
(ifRuleWrapper, ifRulesInEquivalence) -> {
@@ -131,15 +135,11 @@
// -keep rule may vary (due to back references). So, we need to try all pairs of -if
// rule and live types.
ProguardIfRule ifRuleKey = ifRuleWrapper.get();
- List<DexProgramClass> classMatches = new ArrayList<>();
- for (DexProgramClass clazz : newlyLiveClasses) {
- if (evaluateClassForIfRule(ifRuleKey, clazz)) {
- classMatches.add(clazz);
- }
- }
+ evaluateClassesForIfRule(ifRuleKey, newlyLiveClasses, classMatches, timing);
if (!classMatches.isEmpty()) {
ifRulesInEquivalence.removeIf(
ifRule -> processActiveIfRuleWithWithoutMembers(ifRule, classMatches));
+ classMatches.clear();
}
return ifRulesInEquivalence.isEmpty();
});
@@ -206,6 +206,32 @@
}
}
+ private void evaluateClassesForIfRule(
+ ProguardIfRule rule,
+ Map<DexType, DexProgramClass> newlyLiveClasses,
+ List<DexProgramClass> classMatches,
+ Timing timing) {
+ timing.begin("Evaluate precondition");
+ if (rule.getClassNames().hasSingleSpecificType()) {
+ timing.begin("Evaluate single class");
+ DexType singleSpecificType = rule.getClassNames().getSingleSpecificType();
+ DexProgramClass singleSpecificClass = newlyLiveClasses.get(singleSpecificType);
+ if (singleSpecificClass != null && evaluateClassForIfRule(rule, singleSpecificClass)) {
+ classMatches.add(singleSpecificClass);
+ }
+ timing.end();
+ } else {
+ for (DexProgramClass clazz : newlyLiveClasses.values()) {
+ timing.begin("Evaluate class");
+ if (evaluateClassForIfRule(rule, clazz)) {
+ classMatches.add(clazz);
+ }
+ timing.end();
+ }
+ }
+ timing.end();
+ }
+
/** Determines if {@param clazz} satisfies the given if-rule class specification. */
private boolean evaluateClassForIfRule(ProguardIfRule rule, DexProgramClass clazz) {
incrementNumberOfProguardIfRuleClassEvaluations();
diff --git a/src/main/java/com/android/tools/r8/shaking/IfRuleEvaluatorFactory.java b/src/main/java/com/android/tools/r8/shaking/IfRuleEvaluatorFactory.java
index 497f691..2e58f76 100644
--- a/src/main/java/com/android/tools/r8/shaking/IfRuleEvaluatorFactory.java
+++ b/src/main/java/com/android/tools/r8/shaking/IfRuleEvaluatorFactory.java
@@ -10,6 +10,7 @@
import com.android.tools.r8.graph.DexEncodedField;
import com.android.tools.r8.graph.DexEncodedMethod;
import com.android.tools.r8.graph.DexProgramClass;
+import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.graph.ProgramDefinition;
import com.android.tools.r8.graph.ProgramField;
import com.android.tools.r8.graph.ProgramMethod;
@@ -30,6 +31,7 @@
import com.google.common.collect.Sets;
import java.util.Collections;
import java.util.HashMap;
+import java.util.IdentityHashMap;
import java.util.LinkedHashSet;
import java.util.Map;
import java.util.Set;
@@ -151,7 +153,7 @@
ConsequentRootSet consequentRootSet =
timing.time(
"Find consequent items for -if rules...",
- () -> processActiveIfRules(enqueuer, isFirstFixpoint));
+ () -> processActiveIfRules(enqueuer, isFirstFixpoint, timing));
enqueuer.addConsequentRootSet(consequentRootSet);
assert enqueuer.getNumberOfLiveItems() == numberOfLiveItemsAtStart;
}
@@ -166,19 +168,23 @@
return !classesWithNewlyLiveMembers.isEmpty();
}
- private ConsequentRootSet processActiveIfRules(Enqueuer enqueuer, boolean isFirstFixpoint)
- throws ExecutionException {
+ private ConsequentRootSet processActiveIfRules(
+ Enqueuer enqueuer, boolean isFirstFixpoint, Timing timing) throws ExecutionException {
SubtypingInfo subtypingInfo = enqueuer.getSubtypingInfo();
ConsequentRootSetBuilder consequentRootSetBuilder =
ConsequentRootSet.builder(appView, enqueuer, subtypingInfo);
IfRuleEvaluator evaluator =
new IfRuleEvaluator(appView, subtypingInfo, enqueuer, consequentRootSetBuilder, tasks);
+ timing.begin("If rules with members");
if (shouldProcessActiveIfRulesWithMembers(isFirstFixpoint)) {
processActiveIfRulesWithMembers(evaluator, isFirstFixpoint);
}
+ timing.end();
+ timing.begin("If rules without members");
if (shouldProcessActiveIfRulesWithoutMembers(isFirstFixpoint)) {
- processActiveIfRulesWithoutMembers(evaluator, isFirstFixpoint);
+ processActiveIfRulesWithoutMembers(evaluator, isFirstFixpoint, timing);
}
+ timing.end();
return consequentRootSetBuilder.buildConsequentRootSet();
}
@@ -211,13 +217,20 @@
}
private void processActiveIfRulesWithoutMembers(
- IfRuleEvaluator evaluator, boolean isFirstFixpoint) throws ExecutionException {
+ IfRuleEvaluator evaluator, boolean isFirstFixpoint, Timing timing) throws ExecutionException {
if (isFirstFixpoint && !effectivelyFakeLiveClasses.isEmpty()) {
+ Map<DexType, DexProgramClass> newlyLiveClassesMap =
+ new IdentityHashMap<>(effectivelyFakeLiveClasses.size() + newlyLiveClasses.size());
+ effectivelyFakeLiveClasses.forEach(clazz -> newlyLiveClassesMap.put(clazz.getType(), clazz));
+ newlyLiveClasses.forEach(clazz -> newlyLiveClassesMap.put(clazz.getType(), clazz));
evaluator.processActiveIfRulesWithoutMembers(
- activeIfRulesWithoutMembers,
- Iterables.concat(effectivelyFakeLiveClasses, newlyLiveClasses));
+ activeIfRulesWithoutMembers, newlyLiveClassesMap, timing);
} else {
- evaluator.processActiveIfRulesWithoutMembers(activeIfRulesWithoutMembers, newlyLiveClasses);
+ Map<DexType, DexProgramClass> newlyLiveClassesMap =
+ new IdentityHashMap<>(newlyLiveClasses.size());
+ newlyLiveClasses.forEach(clazz -> newlyLiveClassesMap.put(clazz.getType(), clazz));
+ evaluator.processActiveIfRulesWithoutMembers(
+ activeIfRulesWithoutMembers, newlyLiveClassesMap, timing);
}
newlyLiveClasses.clear();
}
diff --git a/src/main/java/com/android/tools/r8/shaking/ProguardClassNameList.java b/src/main/java/com/android/tools/r8/shaking/ProguardClassNameList.java
index 37c0cf3..58d855f 100644
--- a/src/main/java/com/android/tools/r8/shaking/ProguardClassNameList.java
+++ b/src/main/java/com/android/tools/r8/shaking/ProguardClassNameList.java
@@ -85,9 +85,17 @@
@Override
public abstract int hashCode();
+ public abstract boolean hasSingleSpecificType();
+
+ public DexType getSingleSpecificType() {
+ return null;
+ }
+
public abstract boolean hasSpecificTypes();
- public abstract Set<DexType> getSpecificTypes();
+ public Set<DexType> getSpecificTypes() {
+ return null;
+ }
public abstract boolean matches(DexType type);
@@ -165,13 +173,13 @@
}
@Override
- public boolean hasSpecificTypes() {
+ public boolean hasSingleSpecificType() {
return false;
}
@Override
- public Set<DexType> getSpecificTypes() {
- return null;
+ public boolean hasSpecificTypes() {
+ return false;
}
@Override
@@ -227,6 +235,16 @@
}
@Override
+ public boolean hasSingleSpecificType() {
+ return className.hasSpecificType();
+ }
+
+ @Override
+ public DexType getSingleSpecificType() {
+ return className.getSpecificType();
+ }
+
+ @Override
public boolean hasSpecificTypes() {
return className.hasSpecificType() || className.hasSpecificTypes();
}
@@ -278,6 +296,7 @@
private PositiveClassNameList(Collection<ProguardTypeMatcher> classNames) {
this.classNames = ImmutableList.copyOf(classNames);
+ assert !hasSpecificTypes() || getSpecificTypes().size() > 1;
}
@Override
@@ -316,6 +335,11 @@
}
@Override
+ public boolean hasSingleSpecificType() {
+ return false;
+ }
+
+ @Override
public boolean hasSpecificTypes() {
return getSpecificTypes() != null;
}
@@ -426,13 +450,13 @@
}
@Override
- public boolean hasSpecificTypes() {
+ public boolean hasSingleSpecificType() {
return false;
}
@Override
- public Set<DexType> getSpecificTypes() {
- return null;
+ public boolean hasSpecificTypes() {
+ return false;
}
@Override
