Reapply "Move if rule evalation logic to own class"
This reverts commit ff10459b2d120cea74d59190ebc3f772ff13bed5.
Change-Id: I0998aa8fd59a53800f81369cf79e2ff0004ad1c1
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 fa22d3a..6b70524 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -147,7 +147,6 @@
import com.android.tools.r8.shaking.KeepMethodInfo.Joiner;
import com.android.tools.r8.shaking.KeepReason.ReflectiveUseFromXml;
import com.android.tools.r8.shaking.RootSetUtils.ConsequentRootSet;
-import com.android.tools.r8.shaking.RootSetUtils.ConsequentRootSetBuilder;
import com.android.tools.r8.shaking.RootSetUtils.RootSet;
import com.android.tools.r8.shaking.RootSetUtils.RootSetBase;
import com.android.tools.r8.shaking.RootSetUtils.RootSetBuilder;
@@ -172,7 +171,6 @@
import com.android.tools.r8.utils.collections.ProgramFieldSet;
import com.android.tools.r8.utils.collections.ProgramMethodMap;
import com.android.tools.r8.utils.collections.ProgramMethodSet;
-import com.google.common.base.Equivalence.Wrapper;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
@@ -272,6 +270,7 @@
// Don't hold a direct pointer to app info (use appView).
private AppInfoWithClassHierarchy appInfo;
private final AppView<AppInfoWithClassHierarchy> appView;
+ private final IfRuleEvaluatorFactory ifRuleEvaluatorFactory;
private final EnqueuerDeferredTracing deferredTracing;
private final ExecutorService executorService;
private SubtypingInfo subtypingInfo;
@@ -460,9 +459,6 @@
private final Map<DexType, Map<DexAnnotation, List<ProgramDefinition>>>
deferredParameterAnnotations = new IdentityHashMap<>();
- /** Map of active if rules to speed up aapt2 generated keep rules. */
- private Map<Wrapper<ProguardIfRule>, Set<ProguardIfRule>> activeIfRules;
-
/**
* A cache of ScopedDexMethodSet for each live type used for determining that virtual methods that
* cannot be removed because they are widening access for another virtual method defined earlier
@@ -504,6 +500,8 @@
InternalOptions options = appView.options();
this.appInfo = appView.appInfo();
this.appView = appView.withClassHierarchy();
+ this.ifRuleEvaluatorFactory =
+ new IfRuleEvaluatorFactory(appView.withClassHierarchy(), this, executorService);
this.profileCollectionAdditions = profileCollectionAdditions;
this.deferredTracing = EnqueuerDeferredTracing.create(appView, this, mode);
this.executorService = executorService;
@@ -4756,28 +4754,7 @@
long numberOfLiveItemsAfterProcessing = getNumberOfLiveItems();
if (numberOfLiveItemsAfterProcessing > numberOfLiveItems) {
timing.time("Conditional rules", () -> applicableRules.evaluateConditionalRules(this));
-
- // Build the mapping of active if rules. We use a single collection of if-rules to allow
- // removing if rules that have a constant sequent keep rule when they materialize.
- if (activeIfRules == null) {
- activeIfRules = new HashMap<>();
- IfRuleClassPartEquivalence equivalence = new IfRuleClassPartEquivalence();
- for (ProguardIfRule ifRule : rootSet.ifRules) {
- Wrapper<ProguardIfRule> wrap = equivalence.wrap(ifRule);
- activeIfRules.computeIfAbsent(wrap, ignore -> new LinkedHashSet<>()).add(ifRule);
- }
- }
- ConsequentRootSetBuilder consequentSetBuilder =
- ConsequentRootSet.builder(appView, this, subtypingInfo);
- IfRuleEvaluator ifRuleEvaluator =
- new IfRuleEvaluator(
- appView,
- subtypingInfo,
- this,
- executorService,
- activeIfRules,
- consequentSetBuilder);
- addConsequentRootSet(ifRuleEvaluator.run());
+ ifRuleEvaluatorFactory.run(subtypingInfo);
assert getNumberOfLiveItems() == numberOfLiveItemsAfterProcessing;
if (!worklist.isEmpty()) {
continue;
@@ -4905,7 +4882,7 @@
return result;
}
- private void addConsequentRootSet(ConsequentRootSet consequentRootSet) {
+ void addConsequentRootSet(ConsequentRootSet consequentRootSet) {
// TODO(b/132600955): This modifies the root set, but the consequent should not be persistent.
// Instead, the consequent root set should be added to collections that are owned by the
// enqueuer, similar to Enqueuer#dependentMinimumKeepClassInfo.
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 44ba57d..34bf3c1 100644
--- a/src/main/java/com/android/tools/r8/shaking/IfRuleEvaluator.java
+++ b/src/main/java/com/android/tools/r8/shaking/IfRuleEvaluator.java
@@ -14,7 +14,6 @@
import com.android.tools.r8.graph.DexProgramClass;
import com.android.tools.r8.graph.SubtypingInfo;
import com.android.tools.r8.shaking.InlineRule.InlineRuleType;
-import com.android.tools.r8.shaking.RootSetUtils.ConsequentRootSet;
import com.android.tools.r8.shaking.RootSetUtils.ConsequentRootSetBuilder;
import com.android.tools.r8.shaking.RootSetUtils.RootSetBuilder;
import com.android.tools.r8.threading.TaskCollection;
@@ -29,7 +28,6 @@
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ExecutionException;
-import java.util.concurrent.ExecutorService;
import java.util.stream.Collectors;
public class IfRuleEvaluator {
@@ -45,21 +43,21 @@
AppView<? extends AppInfoWithClassHierarchy> appView,
SubtypingInfo subtypingInfo,
Enqueuer enqueuer,
- ExecutorService executorService,
Map<Wrapper<ProguardIfRule>, Set<ProguardIfRule>> ifRules,
- ConsequentRootSetBuilder rootSetBuilder) {
+ ConsequentRootSetBuilder rootSetBuilder,
+ TaskCollection<?> tasks) {
+ assert tasks.isEmpty();
this.appView = appView;
this.subtypingInfo = subtypingInfo;
this.enqueuer = enqueuer;
this.ifRules = ifRules;
this.rootSetBuilder = rootSetBuilder;
- this.tasks = new TaskCollection<>(appView.options(), executorService);
+ this.tasks = tasks;
}
- public ConsequentRootSet run() throws ExecutionException {
+ public void run() throws ExecutionException {
appView.appInfo().app().timing.begin("Find consequent items for -if rules...");
try {
- if (ifRules != null && !ifRules.isEmpty()) {
Iterator<Map.Entry<Wrapper<ProguardIfRule>, Set<ProguardIfRule>>> it =
ifRules.entrySet().iterator();
while (it.hasNext()) {
@@ -107,11 +105,9 @@
}
}
tasks.await();
- }
} finally {
appView.appInfo().app().timing.end();
}
- return rootSetBuilder.buildConsequentRootSet();
}
private boolean canRemoveSubsequentKeepRule(ProguardIfRule rule) {
diff --git a/src/main/java/com/android/tools/r8/shaking/IfRuleEvaluatorFactory.java b/src/main/java/com/android/tools/r8/shaking/IfRuleEvaluatorFactory.java
new file mode 100644
index 0000000..b85f182
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/shaking/IfRuleEvaluatorFactory.java
@@ -0,0 +1,73 @@
+// Copyright (c) 2024, 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 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.SubtypingInfo;
+import com.android.tools.r8.shaking.RootSetUtils.ConsequentRootSet;
+import com.android.tools.r8.shaking.RootSetUtils.ConsequentRootSetBuilder;
+import com.android.tools.r8.threading.TaskCollection;
+import com.google.common.base.Equivalence.Wrapper;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.LinkedHashSet;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+
+public class IfRuleEvaluatorFactory {
+
+ private final AppView<? extends AppInfoWithClassHierarchy> appView;
+ private final Enqueuer enqueuer;
+
+ /** Map of active if rules. This is important for speeding up aapt2 generated keep rules. */
+ private final Map<Wrapper<ProguardIfRule>, Set<ProguardIfRule>> activeIfRules;
+
+ private final TaskCollection<?> tasks;
+
+ public IfRuleEvaluatorFactory(
+ AppView<? extends AppInfoWithClassHierarchy> appView,
+ Enqueuer enqueuer,
+ ExecutorService executorService) {
+ this.appView = appView;
+ this.enqueuer = enqueuer;
+ this.activeIfRules =
+ createActiveIfRules(
+ appView.hasRootSet() ? appView.rootSet().ifRules : Collections.emptySet());
+ this.tasks = new TaskCollection<>(appView.options(), executorService);
+ }
+
+ @SuppressWarnings("MixedMutabilityReturnType")
+ private static Map<Wrapper<ProguardIfRule>, Set<ProguardIfRule>> createActiveIfRules(
+ Set<ProguardIfRule> ifRules) {
+ if (ifRules == null || ifRules.isEmpty()) {
+ return Collections.emptyMap();
+ }
+ // Build the mapping of active if rules. We use a single collection of if-rules to allow
+ // removing if rules that have a constant sequent keep rule when they materialize.
+ Map<Wrapper<ProguardIfRule>, Set<ProguardIfRule>> activeIfRules = new HashMap<>(ifRules.size());
+ IfRuleClassPartEquivalence equivalence = new IfRuleClassPartEquivalence();
+ for (ProguardIfRule ifRule : ifRules) {
+ Wrapper<ProguardIfRule> wrap = equivalence.wrap(ifRule);
+ activeIfRules.computeIfAbsent(wrap, ignoreKey(LinkedHashSet::new)).add(ifRule);
+ }
+ return activeIfRules;
+ }
+
+ public void run(SubtypingInfo subtypingInfo) throws ExecutionException {
+ if (activeIfRules.isEmpty()) {
+ return;
+ }
+ ConsequentRootSetBuilder consequentRootSetBuilder =
+ ConsequentRootSet.builder(appView, enqueuer, subtypingInfo);
+ new IfRuleEvaluator(
+ appView, subtypingInfo, enqueuer, activeIfRules, consequentRootSetBuilder, tasks)
+ .run();
+ enqueuer.addConsequentRootSet(consequentRootSetBuilder.buildConsequentRootSet());
+ }
+}
