Avoid Skyframe restarts in transitive targets due related to conservative aspect deps.
--
MOS_MIGRATED_REVID=100960261
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveBaseTraversalFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveBaseTraversalFunction.java
index fab8842..5bb9c4f 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveBaseTraversalFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveBaseTraversalFunction.java
@@ -14,6 +14,7 @@
package com.google.devtools.build.lib.skyframe;
import com.google.common.base.Preconditions;
+import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.EventHandler;
@@ -111,18 +112,23 @@
TargetAndErrorIfAny targetAndErrorIfAny = (TargetAndErrorIfAny) loadTargetResults;
TProcessedTargets processedTargets = processTarget(label, targetAndErrorIfAny);
- // Process deps from attributes of current target.
+ // Process deps from attributes and conservative aspects of current target.
Iterable<SkyKey> labelDepKeys = getLabelDepKeys(targetAndErrorIfAny.getTarget());
+ Iterable<SkyKey> labelAspectKeys =
+ getConservativeLabelAspectKeys(targetAndErrorIfAny.getTarget());
+ Iterable<SkyKey> depAndAspectKeys = Iterables.concat(labelDepKeys, labelAspectKeys);
+
Set<Entry<SkyKey, ValueOrException2<NoSuchPackageException, NoSuchTargetException>>>
- labelDepEntries = env.getValuesOrThrow(labelDepKeys, NoSuchPackageException.class,
- NoSuchTargetException.class).entrySet();
- processDeps(processedTargets, env.getListener(), targetAndErrorIfAny, labelDepEntries);
+ depsAndAspectEntries = env.getValuesOrThrow(depAndAspectKeys,
+ NoSuchPackageException.class, NoSuchTargetException.class).entrySet();
+ processDeps(processedTargets, env.getListener(), targetAndErrorIfAny, depsAndAspectEntries);
if (env.valuesMissing()) {
return null;
}
- // Process deps from aspects.
- Iterable<SkyKey> labelAspectKeys = getLabelAspectKeys(targetAndErrorIfAny.getTarget(), env);
+
+ // Process deps from strict aspects.
+ labelAspectKeys = getStrictLabelAspectKeys(targetAndErrorIfAny.getTarget(), env);
Set<Entry<SkyKey, ValueOrException2<NoSuchPackageException, NoSuchTargetException>>>
labelAspectEntries = env.getValuesOrThrow(labelAspectKeys, NoSuchPackageException.class,
NoSuchTargetException.class).entrySet();
@@ -142,9 +148,20 @@
/**
* Return an Iterable of SkyKeys corresponding to the Aspect-related dependencies of target.
*
+ * <p>This method may return a precise set of aspect keys, but may need to request additional
+ * dependencies from the env to do so.
+ *
+ * <p>Subclasses should implement only one of #getStrictLabelAspectKeys and
+ * @getConservativeLabelAspectKeys.
+ */
+ protected abstract Iterable<SkyKey> getStrictLabelAspectKeys(Target target, Environment env);
+
+ /**
+ * Return an Iterable of SkyKeys corresponding to the Aspect-related dependencies of target.
+ *
* <p>This method may return a conservative over-approximation of the exact set.
*/
- protected abstract Iterable<SkyKey> getLabelAspectKeys(Target target, Environment env);
+ protected abstract Iterable<SkyKey> getConservativeLabelAspectKeys(Target target);
private Iterable<SkyKey> getLabelDepKeys(Target target) {
List<SkyKey> depKeys = Lists.newArrayList();
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTargetFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTargetFunction.java
index 7ff7117..36d94b9 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTargetFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTargetFunction.java
@@ -154,7 +154,7 @@
}
@Override
- protected Iterable<SkyKey> getLabelAspectKeys(Target target, Environment env) {
+ protected Iterable<SkyKey> getStrictLabelAspectKeys(Target target, Environment env) {
List<SkyKey> depKeys = Lists.newArrayList();
if (target instanceof Rule) {
Multimap<Attribute, Label> transitions =
@@ -181,6 +181,11 @@
return depKeys;
}
+ @Override
+ protected Iterable<SkyKey> getConservativeLabelAspectKeys(Target target) {
+ return ImmutableSet.of();
+ }
+
/**
* Returns every configuration fragment known to the system.
*/
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTraversalFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTraversalFunction.java
index 1be5863..7f54357 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTraversalFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTraversalFunction.java
@@ -86,26 +86,31 @@
: TransitiveTraversalValue.unsuccessfulTransitiveTraversal(errorLoadingTarget);
}
- @Override
- protected Iterable<SkyKey> getLabelAspectKeys(Target target, Environment env) {
- if (!(target instanceof Rule)) {
- return ImmutableSet.of();
- }
- Rule rule = (Rule) target;
- Multimap<Attribute, Label> attibuteMap = LinkedHashMultimap.create();
- for (Attribute attribute : rule.getTransitions(Rule.NO_NODEP_ATTRIBUTES).keys()) {
- for (Class<? extends AspectFactory<?, ?, ?>> aspectFactory : attribute.getAspects()) {
- AspectDefinition.addAllAttributesOfAspect(rule, attibuteMap,
- AspectFactory.Util.create(aspectFactory).getDefinition(), Rule.ALL_DEPS);
- }
+ @Override
+ protected Iterable<SkyKey> getStrictLabelAspectKeys(Target target, Environment env) {
+ return ImmutableSet.of();
}
- ImmutableSet.Builder<SkyKey> depKeys = new ImmutableSet.Builder<>();
- for (Label label : attibuteMap.values()) {
- depKeys.add(getKey(label));
+ @Override
+ protected Iterable<SkyKey> getConservativeLabelAspectKeys(Target target) {
+ if (!(target instanceof Rule)) {
+ return ImmutableSet.of();
+ }
+ Rule rule = (Rule) target;
+ Multimap<Attribute, Label> attibuteMap = LinkedHashMultimap.create();
+ for (Attribute attribute : rule.getTransitions(Rule.NO_NODEP_ATTRIBUTES).keys()) {
+ for (Class<? extends AspectFactory<?, ?, ?>> aspectFactory : attribute.getAspects()) {
+ AspectDefinition.addAllAttributesOfAspect(rule, attibuteMap,
+ AspectFactory.Util.create(aspectFactory).getDefinition(), Rule.ALL_DEPS);
+ }
+ }
+
+ ImmutableSet.Builder<SkyKey> depKeys = new ImmutableSet.Builder<>();
+ for (Label label : attibuteMap.values()) {
+ depKeys.add(getKey(label));
+ }
+ return depKeys.build();
}
- return depKeys.build();
- }
/**
* Because {@link TransitiveTraversalFunction} is invoked only when its side-effects are desired,
