Refactor "determine all dependency edges from all aspects" logic to be a streaming visitation.
In doing so, we use more efficient algorithm that notably doesn't have several large sources of garbage. The inefficiencies and garbage churn of the old algorithm are more easily noticed when you consider the pseudo-code.
The old algorithm for was:
res <- []
for each attribute A of T that entails a label dep
for each label dep L entailed by A
aspectDeps <- {}
for each aspect S of A
if S is satisfied by T
for each attribute A' of S
for each label dep L' entailed by A'
// Note that L' has nothing directly to do with L.
aspectDeps <- aspectDeps U {A', L')
for each {a, l} in aspectDeps
res <- res + [l]
return res
And the new algorithm is:
res <- []
for each attribute A of T that entails a label dep
for each aspect S of A
if there exists a label dep L entailed by A that satisfies S
for each label dep L' entailed by S
res <- res + [L']
return res
This refactor exposes an existing inefficiency in LabelVisitor. I added a TODO comment explaining it.
RELNOTES: None
PiperOrigin-RevId: 237063485
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 4366a0c..5f0be6d 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
@@ -13,7 +13,6 @@
// limitations under the License.
package com.google.devtools.build.lib.skyframe;
-import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
@@ -24,11 +23,9 @@
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.EventHandler;
-import com.google.devtools.build.lib.packages.Aspect;
-import com.google.devtools.build.lib.packages.AspectDefinition;
+import com.google.devtools.build.lib.packages.AdvertisedProviderSet;
import com.google.devtools.build.lib.packages.Attribute;
import com.google.devtools.build.lib.packages.ConfigurationFragmentPolicy;
-import com.google.devtools.build.lib.packages.DependencyFilter;
import com.google.devtools.build.lib.packages.NoSuchPackageException;
import com.google.devtools.build.lib.packages.NoSuchTargetException;
import com.google.devtools.build.lib.packages.NoSuchThingException;
@@ -219,36 +216,38 @@
}
}
+ @Nullable
@Override
- protected Collection<Label> getAspectLabels(
- Rule fromRule,
- ImmutableList<Aspect> aspectsOfAttribute,
+ protected AdvertisedProviderSet getAdvertisedProviderSet(
Label toLabel,
- ValueOrException2<NoSuchPackageException, NoSuchTargetException> toVal,
- final Environment env)
+ @Nullable ValueOrException2<NoSuchPackageException, NoSuchTargetException> toVal,
+ Environment env)
throws InterruptedException {
SkyKey packageKey = PackageValue.key(toLabel.getPackageIdentifier());
+ Target toTarget;
try {
PackageValue pkgValue =
(PackageValue) env.getValueOrThrow(packageKey, NoSuchPackageException.class);
if (pkgValue == null) {
- return ImmutableList.of();
+ return null;
}
Package pkg = pkgValue.getPackage();
if (pkg.containsErrors()) {
- // Do nothing. This error was handled when we computed the corresponding
+ // Do nothing interesting. This error was handled when we computed the corresponding
// TransitiveTargetValue.
- return ImmutableList.of();
+ return null;
}
- Target dependedTarget = pkgValue.getPackage().getTarget(toLabel.getName());
- return AspectDefinition.visitAspectsIfRequired(
- fromRule, aspectsOfAttribute, dependedTarget, DependencyFilter.ALL_DEPS)
- .values();
+ toTarget = pkgValue.getPackage().getTarget(toLabel.getName());
} catch (NoSuchThingException e) {
- // Do nothing. This error was handled when we computed the corresponding
+ // Do nothing interesting. This error was handled when we computed the corresponding
// TransitiveTargetValue.
- return ImmutableList.of();
+ return null;
}
+ if (!(toTarget instanceof Rule)) {
+ // Aspect can be declared only for Rules.
+ return null;
+ }
+ return ((Rule) toTarget).getRuleClassObject().getAdvertisedProviders();
}
@Override
