Use `SkyKeyComputeState` to address a major inefficiency in `ConfiguredTargetFunction` and `AspectFunction`.
In my benchmarks of a diverse set of targets, this reduces the analysis phase wall time by between 3% and 6.5%.
See the description of https://github.com/bazelbuild/bazel/commit/ed279ab4fa2d4356be00b54266f56edcc5aeae78 for some background on the inefficiency. And see the description of https://github.com/bazelbuild/bazel/commit/343ba438a93f8c56a7b524ac7a54666c57a969d9 for how this CL here is able to be a strict performance win even when Blaze is memory constrained.
PiperOrigin-RevId: 419918993
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
index fe141a9..dcb2189 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
@@ -315,8 +315,11 @@
}
SkyframeDependencyResolver resolver = new SkyframeDependencyResolver(env);
- NestedSetBuilder<Package> transitivePackagesForPackageRootResolution =
- storeTransitivePackagesForPackageRootResolution ? NestedSetBuilder.stableOrder() : null;
+ ConfiguredTargetFunction.State state =
+ env.getState(
+ () ->
+ new ConfiguredTargetFunction.State(
+ storeTransitivePackagesForPackageRootResolution));
TargetAndConfiguration originalTargetAndConfiguration =
new TargetAndConfiguration(target, configuration);
@@ -327,16 +330,14 @@
return null;
}
- NestedSetBuilder<Cause> transitiveRootCauses = NestedSetBuilder.stableOrder();
-
// Get the configuration targets that trigger this rule's configurable attributes.
ConfigConditions configConditions =
ConfiguredTargetFunction.getConfigConditions(
env,
originalTargetAndConfiguration,
- transitivePackagesForPackageRootResolution,
+ state.transitivePackagesForPackageRootResolution,
unloadedToolchainContext == null ? null : unloadedToolchainContext.targetPlatform(),
- transitiveRootCauses);
+ state.transitiveRootCauses);
if (configConditions == null) {
// Those targets haven't yet been resolved.
return null;
@@ -346,6 +347,7 @@
try {
depValueMap =
ConfiguredTargetFunction.computeDependencies(
+ state,
env,
resolver,
originalTargetAndConfiguration,
@@ -358,9 +360,7 @@
.build(),
shouldUseToolchainTransition(configuration, aspect.getDefinition()),
ruleClassProvider,
- buildViewProvider.getSkyframeBuildView().getHostConfiguration(),
- transitivePackagesForPackageRootResolution,
- transitiveRootCauses);
+ buildViewProvider.getSkyframeBuildView().getHostConfiguration());
} catch (ConfiguredValueCreationException e) {
throw new AspectCreationException(
e.getMessage(), key.getLabel(), configuration, e.getDetailedExitCode());
@@ -368,8 +368,8 @@
if (depValueMap == null) {
return null;
}
- if (!transitiveRootCauses.isEmpty()) {
- NestedSet<Cause> causes = transitiveRootCauses.build();
+ if (!state.transitiveRootCauses.isEmpty()) {
+ NestedSet<Cause> causes = state.transitiveRootCauses.build();
throw new AspectFunctionException(
new AspectCreationException(
"Loading failed",
@@ -403,7 +403,7 @@
configConditions,
toolchainContext,
depValueMap,
- transitivePackagesForPackageRootResolution);
+ state.transitivePackagesForPackageRootResolution);
} catch (DependencyEvaluationException e) {
// TODO(bazel-team): consolidate all env.getListener().handle() calls in this method, like in
// ConfiguredTargetFunction. This encourages clear, consistent user messages (ideally without
@@ -636,7 +636,8 @@
originalTargetAndAspectConfiguration,
hostConfiguration,
configConditions.asProviders());
- ImmutableList<Dependency> deps = resolver.resolveConfiguration(depKind, depKey);
+ ImmutableList<Dependency> deps =
+ resolver.resolveConfiguration(depKind, depKey, env.getListener());
if (deps == null) {
return null;
}