Precomputes and stores split transition results for null configuration dependencies in ConfigurationResolver#resolveConfigurations so that RuleContext#getSplitPrerequisiteConfiguredTargetAndTargets does not need to execute split transitions in any cases.
PiperOrigin-RevId: 309053703
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/Dependency.java b/src/main/java/com/google/devtools/build/lib/analysis/Dependency.java
index 7dc403c..0b7e227 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/Dependency.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/Dependency.java
@@ -14,6 +14,7 @@
package com.google.devtools.build.lib.analysis;
import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.analysis.config.transitions.ConfigurationTransition;
@@ -30,8 +31,8 @@
*
* <p>Explicit configurations: includes the target and the configuration of the dependency
* configured target and any aspects that may be required, as well as the configurations for these
- * aspects and an optional transition key. {@link Dependency#getTransitionKey} provides some more
- * context on transition keys.
+ * aspects and transition keys. {@link Dependency#getTransitionKeys} provides some more context on
+ * transition keys.
*
* <p>Configuration transitions: includes the target and the desired configuration transitions that
* should be applied to produce the dependency's configuration. It's the caller's responsibility to
@@ -51,11 +52,21 @@
* configuration.
*/
public static Dependency withNullConfiguration(Label label) {
- return new NullConfigurationDependency(label);
+ return new NullConfigurationDependency(label, ImmutableList.of());
}
/**
- * Creates a new {@link Dependency} with the given explicit configuration.
+ * Creates a new {@link Dependency} with a null configuration and transition keys, used when edges
+ * with a split transition were resolved to null configuration targets.
+ */
+ public static Dependency withNullConfigurationAndTransitionKeys(
+ Label label, ImmutableList<String> transitionKeys) {
+ return new NullConfigurationDependency(label, transitionKeys);
+ }
+
+ /**
+ * Creates a new {@link Dependency} with the given explicit configuration. Should only be used for
+ * dependency with no configuration changes.
*
* <p>The configuration must not be {@code null}.
*
@@ -67,12 +78,12 @@
configuration,
AspectCollection.EMPTY,
ImmutableMap.<AspectDescriptor, BuildConfiguration>of(),
- null);
+ ImmutableList.of());
}
/**
- * Creates a new {@link Dependency} with the given configuration and aspects. The configuration
- * is also applied to all aspects.
+ * Creates a new {@link Dependency} with the given configuration and aspects. The configuration is
+ * also applied to all aspects. Should only be used for dependency with no configuration changes.
*
* <p>The configuration and aspects must not be {@code null}.
*/
@@ -84,7 +95,7 @@
aspectBuilder.put(aspect, configuration);
}
return new ExplicitConfigurationDependency(
- label, configuration, aspects, aspectBuilder.build(), null);
+ label, configuration, aspects, aspectBuilder.build(), ImmutableList.of());
}
/**
@@ -106,22 +117,31 @@
aspectBuilder.put(aspect, configuration);
}
return new ExplicitConfigurationDependency(
- label, configuration, aspects, aspectBuilder.build(), transitionKey);
+ label,
+ configuration,
+ aspects,
+ aspectBuilder.build(),
+ transitionKey == null ? ImmutableList.of() : ImmutableList.of(transitionKey));
}
/**
- * Creates a new {@link Dependency} with the given configuration and aspects, suitable for
- * storing the output of a configuration trimming step. The aspects each have their own
- * configuration.
+ * Creates a new {@link Dependency} with the given configuration and aspects, suitable for storing
+ * the output of a configuration trimming step. The aspects each have their own configuration.
+ * Should only be used for dependency with no configuration changes.
*
* <p>The aspects and configurations must not be {@code null}.
*/
public static Dependency withConfiguredAspects(
- Label label, BuildConfiguration configuration,
+ Label label,
+ BuildConfiguration configuration,
AspectCollection aspects,
Map<AspectDescriptor, BuildConfiguration> aspectConfigurations) {
return new ExplicitConfigurationDependency(
- label, configuration, aspects, ImmutableMap.copyOf(aspectConfigurations), null);
+ label,
+ configuration,
+ aspects,
+ ImmutableMap.copyOf(aspectConfigurations),
+ ImmutableList.of());
}
/**
@@ -184,20 +204,28 @@
public abstract BuildConfiguration getAspectConfiguration(AspectDescriptor aspect);
/**
- * Returns the key of a configuration transition, if exists, associated with this dependency. See
- * {@link ConfigurationTransition#apply} for more details.
+ * Returns the keys of a configuration transition, if exist, associated with this dependency. See
+ * {@link ConfigurationTransition#apply} for more details. Normally, this returns an empty list,
+ * when there was no configuration transition in effect, or one with a single entry, when there
+ * was a specific configuration transition result that led to this. It may also return a list with
+ * multiple entries if the dependency has a null configuration, yet the outgoing edge has a split
+ * transition. In such cases all transition keys returned by the transition are tagged to the
+ * dependency.
*
* @throws IllegalStateException if {@link #hasExplicitConfiguration()} returns false.
*/
- public abstract String getTransitionKey();
+ public abstract ImmutableList<String> getTransitionKeys();
/**
* Implementation of a dependency with no configuration, suitable for, e.g., source files or
* visibility.
*/
private static final class NullConfigurationDependency extends Dependency {
- public NullConfigurationDependency(Label label) {
+ private final ImmutableList<String> transitionKeys;
+
+ public NullConfigurationDependency(Label label, ImmutableList<String> transitionKeys) {
super(label);
+ this.transitionKeys = Preconditions.checkNotNull(transitionKeys);
}
@Override
@@ -227,10 +255,9 @@
return null;
}
- @Nullable
@Override
- public String getTransitionKey() {
- return null;
+ public ImmutableList<String> getTransitionKeys() {
+ return transitionKeys;
}
@Override
@@ -260,19 +287,19 @@
private final BuildConfiguration configuration;
private final AspectCollection aspects;
private final ImmutableMap<AspectDescriptor, BuildConfiguration> aspectConfigurations;
- @Nullable private final String transitionKey;
+ private final ImmutableList<String> transitionKeys;
public ExplicitConfigurationDependency(
Label label,
BuildConfiguration configuration,
AspectCollection aspects,
ImmutableMap<AspectDescriptor, BuildConfiguration> aspectConfigurations,
- @Nullable String transitionKey) {
+ ImmutableList<String> transitionKeys) {
super(label);
this.configuration = Preconditions.checkNotNull(configuration);
this.aspects = Preconditions.checkNotNull(aspects);
this.aspectConfigurations = Preconditions.checkNotNull(aspectConfigurations);
- this.transitionKey = transitionKey;
+ this.transitionKeys = Preconditions.checkNotNull(transitionKeys);
}
@Override
@@ -301,15 +328,14 @@
return aspectConfigurations.get(aspect);
}
- @Nullable
@Override
- public String getTransitionKey() {
- return transitionKey;
+ public ImmutableList<String> getTransitionKeys() {
+ return transitionKeys;
}
@Override
public int hashCode() {
- return Objects.hash(label, configuration, aspectConfigurations, transitionKey);
+ return Objects.hash(label, configuration, aspectConfigurations, transitionKeys);
}
@Override
@@ -374,7 +400,7 @@
}
@Override
- public String getTransitionKey() {
+ public ImmutableList<String> getTransitionKeys() {
throw new IllegalStateException(
"This dependency has a transition, not an explicit configuration.");
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
index a68be1d..9e53631 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
@@ -44,13 +44,11 @@
import com.google.devtools.build.lib.analysis.actions.ActionConstructionContext;
import com.google.devtools.build.lib.analysis.buildinfo.BuildInfoKey;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
-import com.google.devtools.build.lib.analysis.config.BuildOptions;
import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider;
import com.google.devtools.build.lib.analysis.config.Fragment;
import com.google.devtools.build.lib.analysis.config.FragmentCollection;
import com.google.devtools.build.lib.analysis.config.transitions.ConfigurationTransition;
import com.google.devtools.build.lib.analysis.config.transitions.NoTransition;
-import com.google.devtools.build.lib.analysis.config.transitions.SplitTransition;
import com.google.devtools.build.lib.analysis.config.transitions.TransitionFactory;
import com.google.devtools.build.lib.analysis.constraints.ConstraintSemantics;
import com.google.devtools.build.lib.analysis.platform.ConstraintValueInfo;
@@ -905,40 +903,19 @@
public Map<Optional<String>, List<ConfiguredTargetAndData>>
getSplitPrerequisiteConfiguredTargetAndTargets(String attributeName) {
checkAttribute(attributeName, TransitionMode.SPLIT);
- Attribute attributeDefinition = attributes().getAttributeDefinition(attributeName);
- Preconditions.checkState(attributeDefinition.getTransitionFactory().isSplit());
- SplitTransition transition =
- (SplitTransition)
- attributeDefinition
- .getTransitionFactory()
- .create(
- AttributeTransitionData.builder()
- .attributes(ConfiguredAttributeMapper.of(rule, configConditions))
- .executionPlatform(getToolchainContext().executionPlatform().label())
- .build());
- BuildOptions fromOptions = getConfiguration().getOptions();
- Map<String, BuildOptions> splitOptions =
- transition.split(fromOptions, getAnalysisEnvironment().getEventHandler());
- List<ConfiguredTargetAndData> deps = getConfiguredTargetAndTargetDeps(attributeName);
-
- if (SplitTransition.equals(fromOptions, splitOptions.values())) {
- // The split transition is not active.
- return ImmutableMap.of(Optional.<String>absent(), deps);
- }
-
// Use an ImmutableListMultimap.Builder here to preserve ordering.
ImmutableListMultimap.Builder<Optional<String>, ConfiguredTargetAndData> result =
ImmutableListMultimap.builder();
+ List<ConfiguredTargetAndData> deps = getConfiguredTargetAndTargetDeps(attributeName);
for (ConfiguredTargetAndData t : deps) {
- if (t.getTransitionKey() == null
- || t.getTransitionKey().equals(ConfigurationTransition.PATCH_TRANSITION_KEY)) {
- // The target doesn't have a specific transition key associated. This likely means it's a
- // non-configurable target, e.g. files, package groups. Pan out to all available keys.
- for (String key : splitOptions.keySet()) {
- result.put(Optional.of(key), t);
- }
- } else {
- result.put(Optional.of(t.getTransitionKey()), t);
+ ImmutableList<String> transitionKeys = t.getTransitionKeys();
+ if (transitionKeys.isEmpty()) {
+ // The split transition is not active, i.e. does not change build configurations.
+ // TODO(jungjw): Investigate if we need to do a sanity check here.
+ return ImmutableMap.of(Optional.absent(), deps);
+ }
+ for (String key : transitionKeys) {
+ result.put(Optional.of(key), t);
}
}
return Multimaps.asMap(result.build());
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationResolver.java b/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationResolver.java
index 0696ef0..3dc86ce 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationResolver.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationResolver.java
@@ -19,6 +19,7 @@
import com.google.common.base.Preconditions;
import com.google.common.base.Verify;
import com.google.common.base.VerifyException;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
import com.google.common.collect.LinkedHashMultimap;
@@ -34,6 +35,8 @@
import com.google.devtools.build.lib.analysis.config.transitions.ConfigurationTransition;
import com.google.devtools.build.lib.analysis.config.transitions.NoTransition;
import com.google.devtools.build.lib.analysis.config.transitions.NullTransition;
+import com.google.devtools.build.lib.analysis.config.transitions.SplitTransition;
+import com.google.devtools.build.lib.analysis.config.transitions.TransitionFactory;
import com.google.devtools.build.lib.analysis.skylark.StarlarkTransition;
import com.google.devtools.build.lib.analysis.skylark.StarlarkTransition.TransitionException;
import com.google.devtools.build.lib.cmdline.Label;
@@ -42,6 +45,8 @@
import com.google.devtools.build.lib.events.ExtendedEventHandler;
import com.google.devtools.build.lib.events.StoredEventHandler;
import com.google.devtools.build.lib.packages.Attribute;
+import com.google.devtools.build.lib.packages.AttributeTransitionData;
+import com.google.devtools.build.lib.packages.ConfiguredAttributeMapper;
import com.google.devtools.build.lib.packages.RuleClassProvider;
import com.google.devtools.build.lib.packages.Target;
import com.google.devtools.build.lib.packages.TargetUtils;
@@ -105,6 +110,8 @@
* @param originalDeps the transition requests for each dep and each dependency kind
* @param hostConfiguration the host configuration
* @param ruleClassProvider provider for determining the right configuration fragments for deps
+ * @param defaultBuildOptions default build options to diff options against for optimization
+ * @param configConditions {@link ConfigMatchingProvider} map for the rule
* @return a mapping from each dependency kind in the source target to the {@link
* BuildConfiguration}s and {@link Label}s for the deps under that dependency kind . Returns
* null if not all Skyframe dependencies are available.
@@ -116,7 +123,8 @@
OrderedSetMultimap<DependencyKind, Dependency> originalDeps,
BuildConfiguration hostConfiguration,
RuleClassProvider ruleClassProvider,
- BuildOptions defaultBuildOptions)
+ BuildOptions defaultBuildOptions,
+ ImmutableMap<Label, ConfigMatchingProvider> configConditions)
throws DependencyEvaluationException, InterruptedException {
// Maps each Skyframe-evaluated BuildConfiguration to the dependencies that need that
@@ -162,7 +170,8 @@
for (Map.Entry<DependencyKind, Dependency> depsEntry : originalDeps.entries()) {
Dependency dep = depsEntry.getValue();
- DependencyEdge dependencyEdge = new DependencyEdge(depsEntry.getKey(), dep.getLabel());
+ DependencyKind depKind = depsEntry.getKey();
+ DependencyEdge dependencyEdge = new DependencyEdge(depKind, dep.getLabel());
attributesAndLabels.add(dependencyEdge);
// DependencyResolver should never emit a Dependency with an explicit configuration
Preconditions.checkState(!dep.hasExplicitConfiguration());
@@ -173,8 +182,48 @@
// total analysis phase time.
ConfigurationTransition transition = dep.getTransition();
if (transition == NullTransition.INSTANCE) {
- putOnlyEntry(
- resolvedDeps, dependencyEdge, Dependency.withNullConfiguration(dep.getLabel()));
+ Dependency finalDependency = Dependency.withNullConfiguration(dep.getLabel());
+ // If the base transition is a split transition, execute the transition and store returned
+ // transition keys along with the null configuration dependency, so that other code relying
+ // on stored transition keys doesn't have to implement special handling logic just for this
+ // kind of cases.
+ if (depKind.getAttribute() != null) {
+ TransitionFactory<AttributeTransitionData> transitionFactory =
+ depKind.getAttribute().getTransitionFactory();
+ if (transitionFactory.isSplit()) {
+ AttributeTransitionData transitionData =
+ AttributeTransitionData.builder()
+ .attributes(
+ ConfiguredAttributeMapper.of(
+ ctgValue.getTarget().getAssociatedRule(), configConditions))
+ .build();
+ ConfigurationTransition baseTransition = transitionFactory.create(transitionData);
+ Map<String, BuildOptions> toOptions;
+ try {
+ // TODO(jungjw): See if we can dedup getBuildSettingPackages implementations and put
+ // this in applyTransition.
+ HashMap<PackageValue.Key, PackageValue> buildSettingPackages =
+ StarlarkTransition.getBuildSettingPackages(env, baseTransition);
+ if (buildSettingPackages == null) {
+ return null;
+ }
+ toOptions =
+ applyTransition(
+ currentConfiguration.getOptions(),
+ baseTransition,
+ buildSettingPackages,
+ env.getListener());
+ } catch (TransitionException e) {
+ throw new DependencyEvaluationException(e);
+ }
+ if (!SplitTransition.equals(currentConfiguration.getOptions(), toOptions.values())) {
+ finalDependency =
+ Dependency.withNullConfigurationAndTransitionKeys(
+ dep.getLabel(), ImmutableList.copyOf(toOptions.keySet()));
+ }
+ }
+ }
+ putOnlyEntry(resolvedDeps, dependencyEdge, finalDependency);
continue;
}
@@ -592,7 +641,7 @@
}
Set<String> depFragmentNames = new HashSet<>();
for (Class<? extends Fragment> fragmentClass : expectedDepFragments) {
- depFragmentNames.add(fragmentClass.getSimpleName());
+ depFragmentNames.add(fragmentClass.getSimpleName());
}
Set<String> missing = Sets.difference(depFragmentNames, ctgFragmentNames);
if (!missing.isEmpty()) {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetAndData.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetAndData.java
index a553da1..1d06228 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetAndData.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetAndData.java
@@ -16,6 +16,7 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
@@ -29,10 +30,9 @@
/**
* A container class for a {@link ConfiguredTarget} and associated data, {@link Target}, {@link
- * BuildConfiguration}, and an optional transition key. In the future, {@link ConfiguredTarget}
- * objects will no longer contain their associated {@link BuildConfiguration}. Consumers that need
- * the {@link Target} or {@link BuildConfiguration} must therefore have access to one of these
- * objects.
+ * BuildConfiguration}, and transition keys. In the future, {@link ConfiguredTarget} objects will no
+ * longer contain their associated {@link BuildConfiguration}. Consumers that need the {@link
+ * Target} or {@link BuildConfiguration} must therefore have access to one of these objects.
*
* <p>These objects are intended to be short-lived, never stored in Skyframe, since they pair three
* heavyweight objects, a {@link ConfiguredTarget}, a {@link Target} (which holds a {@link
@@ -42,18 +42,18 @@
private final ConfiguredTarget configuredTarget;
private final Target target;
private final BuildConfiguration configuration;
- @Nullable private final String transitionKey;
+ private final ImmutableList<String> transitionKeys;
@VisibleForTesting
public ConfiguredTargetAndData(
ConfiguredTarget configuredTarget,
Target target,
BuildConfiguration configuration,
- String transitionKey) {
+ ImmutableList<String> transitionKeys) {
this.configuredTarget = configuredTarget;
this.target = target;
this.configuration = configuration;
- this.transitionKey = transitionKey;
+ this.transitionKeys = transitionKeys;
Preconditions.checkState(
configuredTarget.getLabel().equals(target.getLabel()),
"Unable to construct ConfiguredTargetAndData:"
@@ -123,7 +123,7 @@
if (configuredTarget.equals(maybeNew)) {
return this;
}
- return new ConfiguredTargetAndData(maybeNew, target, configuration, transitionKey);
+ return new ConfiguredTargetAndData(maybeNew, target, configuration, transitionKeys);
}
public Target getTarget() {
@@ -138,8 +138,7 @@
return configuredTarget;
}
- @Nullable
- public String getTransitionKey() {
- return transitionKey;
+ public ImmutableList<String> getTransitionKeys() {
+ return transitionKeys;
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
index 79e3f49..0e5594e 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
@@ -620,7 +620,8 @@
depValueNames,
hostConfiguration,
ruleClassProvider,
- defaultBuildOptions);
+ defaultBuildOptions,
+ configConditions);
// Return early in case packages were not loaded yet. In theory, we could start configuring
// dependent targets in loaded packages. However, that creates an artificial sync boundary
@@ -872,7 +873,7 @@
depValue.getConfiguredTarget(),
pkgValue.getPackage().getTarget(depLabel.getName()),
depConfiguration,
- dep.getTransitionKey()));
+ dep.getTransitionKeys()));
} catch (NoSuchTargetException e) {
throw new IllegalStateException("Target already verified for " + dep, e);
}
