diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ExecGroupCollection.java b/src/main/java/com/google/devtools/build/lib/analysis/ExecGroupCollection.java
index f70a7cbd61e447..0c43b6d50bfcfd 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/ExecGroupCollection.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ExecGroupCollection.java
@@ -22,6 +22,7 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableTable;
+import com.google.common.collect.Maps;
import com.google.common.collect.Table;
import com.google.devtools.build.lib.analysis.config.ToolchainTypeRequirement;
import com.google.devtools.build.lib.analysis.platform.PlatformInfo;
@@ -44,17 +45,22 @@
*/
@AutoValue
public abstract class ExecGroupCollection {
-
- public static Builder emptyBuilder() {
- return new AutoValue_ExecGroupCollection_Builder(ImmutableMap.of());
- }
-
- public static Builder builder(
+ /**
+ * Prepares the input exec groups to serve as {@link Builder#execGroups}.
+ *
+ *
Applies any inheritance specified via {@link ExecGroup#copyFrom} and adds auto exec groups
+ * when {@code useAutoExecGroups} is true.
+ */
+ public static ImmutableMap process(
ImmutableMap execGroups,
ImmutableSet defaultExecWith,
- ImmutableSet defaultToolchainTypes) {
- // Post-process the groups to handle inheritance.
- Map processedGroups = new LinkedHashMap<>();
+ ImmutableSet defaultToolchainTypes,
+ boolean useAutoExecGroups) {
+ var processedGroups =
+ Maps.newHashMapWithExpectedSize(
+ useAutoExecGroups
+ ? (execGroups.size() + defaultToolchainTypes.size())
+ : execGroups.size());
for (Map.Entry entry : execGroups.entrySet()) {
String name = entry.getKey();
ExecGroup execGroup = entry.getValue();
@@ -74,13 +80,20 @@ public static Builder builder(
processedGroups.put(name, execGroup);
}
- return new AutoValue_ExecGroupCollection_Builder(ImmutableMap.copyOf(processedGroups));
+ if (useAutoExecGroups) {
+ // Creates one exec group for each toolchain (automatic exec groups).
+ for (ToolchainTypeRequirement toolchainType : defaultToolchainTypes) {
+ processedGroups.put(
+ toolchainType.toolchainType().toString(),
+ ExecGroup.builder().addToolchainType(toolchainType).copyFrom(null).build());
+ }
+ }
+ return ImmutableMap.copyOf(processedGroups);
}
/** Builder class for correctly constructing ExecGroupCollection instances. */
// Note that this is _not_ an actual @AutoValue.Builder: it provides more logic and has different
// fields.
- @AutoValue
public abstract static class Builder {
public abstract ImmutableMap execGroups();
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ToolchainCollection.java b/src/main/java/com/google/devtools/build/lib/analysis/ToolchainCollection.java
index 572d077d6c17f3..5f7f95d2ca88f9 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/ToolchainCollection.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ToolchainCollection.java
@@ -14,6 +14,7 @@
package com.google.devtools.build.lib.analysis;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
+import static com.google.common.collect.Maps.newHashMapWithExpectedSize;
import com.google.auto.value.AutoValue;
import com.google.common.base.Preconditions;
@@ -78,10 +79,22 @@ public static Builder builder() {
return new Builder();
}
+ public static Builder builderWithExpectedSize(int expectedSize) {
+ return new Builder(expectedSize);
+ }
+
/** Builder for ToolchainCollection. */
public static final class Builder {
// This is not immutable so that we can check for duplicate keys easily.
- private final Map toolchainContexts = new HashMap<>();
+ private final Map toolchainContexts;
+
+ private Builder() {
+ this.toolchainContexts = new HashMap<>();
+ }
+
+ private Builder(int expectedSize) {
+ this.toolchainContexts = newHashMapWithExpectedSize(expectedSize);
+ }
public ToolchainCollection build() {
Preconditions.checkArgument(toolchainContexts.containsKey(ExecGroup.DEFAULT_EXEC_GROUP_NAME));
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 82ffc8c477978c..8f59b0967f83e8 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
@@ -15,9 +15,9 @@
package com.google.devtools.build.lib.skyframe;
import static com.google.common.collect.ImmutableList.toImmutableList;
+import static com.google.devtools.build.lib.skyframe.PrerequisiteProducer.createDefaultToolchainContextKey;
import com.google.common.base.Preconditions;
-import com.google.common.base.Predicates;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
@@ -64,6 +64,7 @@
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.StoredEventHandler;
import com.google.devtools.build.lib.packages.Aspect;
+import com.google.devtools.build.lib.packages.AspectDefinition;
import com.google.devtools.build.lib.packages.Attribute;
import com.google.devtools.build.lib.packages.BuildFileContainsErrorsException;
import com.google.devtools.build.lib.packages.NativeAspectClass;
@@ -81,7 +82,7 @@
import com.google.devtools.build.lib.profiler.memory.CurrentRuleTracker;
import com.google.devtools.build.lib.skyframe.AspectKeyCreator.AspectKey;
import com.google.devtools.build.lib.skyframe.BzlLoadFunction.BzlLoadFailedException;
-import com.google.devtools.build.lib.skyframe.PrerequisiteProducer.ComputedToolchainContexts;
+import com.google.devtools.build.lib.skyframe.PrerequisiteProducer.UnloadedToolchainContextsInputs;
import com.google.devtools.build.lib.skyframe.SkyframeExecutor.BuildViewProvider;
import com.google.devtools.build.lib.util.OrderedSetMultimap;
import com.google.devtools.build.skyframe.SkyFunction;
@@ -94,6 +95,7 @@
import java.util.List;
import java.util.Map;
import java.util.Objects;
+import java.util.Optional;
import javax.annotation.Nullable;
import net.starlark.java.eval.StarlarkSemantics;
@@ -275,27 +277,25 @@ public SkyValue compute(SkyKey skyKey, Environment env)
}
}
- TargetAndConfiguration originalTargetAndConfiguration =
- new TargetAndConfiguration(target, configuration);
try {
- ComputedToolchainContexts computedToolchainContexts =
- getUnloadedToolchainContexts(env, key, aspect, configuration);
- if (env.valuesMissing()) {
+ var unloadedToolchainContextsInputs =
+ getUnloadedToolchainContextsInputs(
+ aspect.getDefinition(), key.getConfigurationKey(), configuration);
+ Optional> computedToolchainContexts =
+ PrerequisiteProducer.computeUnloadedToolchainContexts(
+ env, unloadedToolchainContextsInputs);
+ if (computedToolchainContexts == null) {
return null;
}
- ToolchainCollection unloadedToolchainContexts = null;
- ExecGroupCollection.Builder execGroupCollectionBuilder = null;
- if (computedToolchainContexts != null) {
- unloadedToolchainContexts = computedToolchainContexts.toolchainCollection;
- execGroupCollectionBuilder = computedToolchainContexts.execGroupCollectionBuilder;
- }
+ ToolchainCollection unloadedToolchainContexts =
+ computedToolchainContexts.orElse(null);
// Get the configuration targets that trigger this rule's configurable attributes.
ConfigConditions configConditions =
PrerequisiteProducer.computeConfigConditions(
env,
- originalTargetAndConfiguration,
+ targetAndConfiguration,
state.transitivePackages,
unloadedToolchainContexts == null
? null
@@ -368,7 +368,7 @@ public SkyValue compute(SkyKey skyKey, Environment env)
configuration,
configConditions,
toolchainContexts,
- execGroupCollectionBuilder,
+ unloadedToolchainContextsInputs,
depValueMap,
state.transitivePackages);
} catch (DependencyEvaluationException e) {
@@ -559,46 +559,42 @@ static StarlarkDefinedAspect loadAspectFromBzl(
}
@Nullable
- private static ComputedToolchainContexts getUnloadedToolchainContexts(
- Environment env,
- AspectKey key,
- Aspect aspect,
- @Nullable BuildConfigurationValue configuration)
- throws InterruptedException, AspectCreationException {
+ private static UnloadedToolchainContextsInputs getUnloadedToolchainContextsInputs(
+ AspectDefinition aspectDefinition,
+ @Nullable BuildConfigurationKey configurationKey,
+ @Nullable BuildConfigurationValue configuration) {
if (configuration == null) {
// Configuration can be null in the case of aspects applied to input files. In this case,
- // there are no chances of toolchains being used, so skip it.
- return null;
- }
-
- ImmutableMap aspectAttributes = aspect.getDefinition().getAttributes();
+ // there are no toolchains being used.
+ return UnloadedToolchainContextsInputs.empty();
+ }
+
+ boolean useAutoExecGroups = shouldUseAutoExecGroups(aspectDefinition, configuration);
+ var processedExecGroups =
+ ExecGroupCollection.process(
+ aspectDefinition.execGroups(),
+ aspectDefinition.execCompatibleWith(),
+ aspectDefinition.getToolchainTypes(),
+ useAutoExecGroups);
+ // Note: `configuration.getOptions().hasNoConfig()` is handled early in #compute.
+ return UnloadedToolchainContextsInputs.create(
+ processedExecGroups,
+ createDefaultToolchainContextKey(
+ configurationKey,
+ aspectDefinition.execCompatibleWith(),
+ /* debugTarget= */ false,
+ /* useAutoExecGroups= */ useAutoExecGroups,
+ aspectDefinition.getToolchainTypes(),
+ /* parentExecutionPlatformLabel= */ null));
+ }
- boolean useAutoExecGroups;
+ private static boolean shouldUseAutoExecGroups(
+ AspectDefinition aspectDefinition, BuildConfigurationValue configuration) {
+ ImmutableMap aspectAttributes = aspectDefinition.getAttributes();
if (aspectAttributes.containsKey("$use_auto_exec_groups")) {
- useAutoExecGroups =
- (boolean) aspectAttributes.get("$use_auto_exec_groups").getDefaultValueUnchecked();
- } else {
- useAutoExecGroups = configuration.useAutoExecGroups();
- }
-
- // Determine what toolchains are needed by this target.
- try {
- return PrerequisiteProducer.computeUnloadedToolchainContexts(
- env,
- key.getLabel(),
- !configuration.getOptions().hasNoConfig(),
- Predicates.alwaysFalse(),
- useAutoExecGroups,
- configuration.getKey(),
- aspect.getDefinition().getToolchainTypes(),
- aspect.getDefinition().execCompatibleWith(),
- aspect.getDefinition().execGroups(),
- null);
- } catch (ToolchainException e) {
- // TODO(katre): better error handling
- throw new AspectCreationException(
- e.getMessage(), new LabelCause(key.getLabel(), e.getDetailedExitCode()));
+ return (boolean) aspectAttributes.get("$use_auto_exec_groups").getDefaultValueUnchecked();
}
+ return configuration.useAutoExecGroups();
}
/**
@@ -643,14 +639,20 @@ private AspectValue createAliasAspect(
// Compute the Dependency from the original target to aliasedLabel.
Dependency dep;
try {
- ComputedToolchainContexts computedToolchainContexts =
- getUnloadedToolchainContexts(env, originalKey, aspect, originalTarget.getConfiguration());
- if (env.valuesMissing()) {
+ var configuration = originalTarget.getConfiguration();
+ // Determine what toolchains are needed by this target.
+ var unloadedToolchainContextsInputs =
+ getUnloadedToolchainContextsInputs(
+ aspect.getDefinition(), originalKey.getConfigurationKey(), configuration);
+ Optional> computedToolchainContexts =
+ PrerequisiteProducer.computeUnloadedToolchainContexts(
+ env, unloadedToolchainContextsInputs);
+ if (computedToolchainContexts == null) {
return null;
}
ToolchainCollection unloadedToolchainContexts =
- computedToolchainContexts.toolchainCollection;
+ computedToolchainContexts.orElse(null);
// Get the configuration targets that trigger this rule's configurable attributes.
ConfigConditions configConditions =
@@ -673,7 +675,7 @@ private AspectValue createAliasAspect(
}
ConfigurationTransition transition =
TransitionResolver.evaluateTransition(
- originalTarget.getConfiguration(),
+ configuration,
NoTransition.INSTANCE,
aliasedTarget,
((ConfiguredRuleClassProvider) ruleClassProvider).getTrimmingTransitionFactory());
@@ -705,8 +707,10 @@ private AspectValue createAliasAspect(
throw new AspectFunctionException(e);
} catch (ConfiguredValueCreationException e) {
throw new AspectFunctionException(e);
- } catch (AspectCreationException e) {
- throw new AspectFunctionException(e);
+ } catch (ToolchainException e) {
+ throw new AspectFunctionException(
+ new AspectCreationException(
+ e.getMessage(), new LabelCause(originalKey.getLabel(), e.getDetailedExitCode())));
}
if (!transitiveRootCauses.isEmpty()) {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PrerequisiteProducer.java b/src/main/java/com/google/devtools/build/lib/skyframe/PrerequisiteProducer.java
index 5a7223839c167b..b40a4942d0b6a3 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PrerequisiteProducer.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PrerequisiteProducer.java
@@ -13,6 +13,9 @@
// limitations under the License.
package com.google.devtools.build.lib.skyframe;
+import static com.google.devtools.build.lib.packages.ExecGroup.DEFAULT_EXEC_GROUP_NAME;
+
+import com.google.auto.value.AutoValue;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
@@ -56,6 +59,7 @@
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.events.Event;
+import com.google.devtools.build.lib.events.ExtendedEventHandler;
import com.google.devtools.build.lib.events.StoredEventHandler;
import com.google.devtools.build.lib.packages.Aspect;
import com.google.devtools.build.lib.packages.BuildType;
@@ -90,7 +94,6 @@
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
-import java.util.function.Predicate;
import javax.annotation.Nullable;
import net.starlark.java.syntax.Location;
@@ -153,7 +156,6 @@ static class State implements SkyKeyComputeState, IncompatibleTargetProducer.Res
@Nullable
private Map resolveConfiguredTargetDependenciesResult;
-
/**
* Non-null if all the work in {@link #computeDependencies} is already done. This field contains
* the result.
@@ -290,7 +292,7 @@ public boolean evaluate(
InconsistentNullConfigException,
IncompatibleTargetException,
InterruptedException {
- targetAndConfiguration =
+ this.targetAndConfiguration =
computeTargetAndConfiguration(
configuredTargetKey, state, transitiveRootCauses, transitivePackages, env);
if (targetAndConfiguration == null) {
@@ -347,21 +349,21 @@ public boolean evaluate(
return false;
}
}
-
- // Determine what toolchains are needed by this target.
- ComputedToolchainContexts result =
- computeUnloadedToolchainContexts(
- env,
- ruleClassProvider,
+ var unloadedToolchainContextsInputs =
+ getUnloadedToolchainContextsInputs(
targetAndConfiguration,
- configuredTargetKey.getExecutionPlatformLabel());
- if (env.valuesMissing()) {
- // computeUnloadedToolchainContexts may return non-null even when deps are missing.
+ configuredTargetKey.getExecutionPlatformLabel(),
+ ruleClassProvider,
+ env.getListener());
+ // Determine what toolchains are needed by this target.
+ Optional> result =
+ computeUnloadedToolchainContexts(env, unloadedToolchainContextsInputs);
+ if (result == null) {
return false;
}
- unloadedToolchainContexts = result.toolchainCollection;
- execGroupCollectionBuilder = result.execGroupCollectionBuilder;
- platformInfo =
+ this.unloadedToolchainContexts = result.orElse(null);
+ this.execGroupCollectionBuilder = unloadedToolchainContextsInputs;
+ this.platformInfo =
unloadedToolchainContexts != null ? unloadedToolchainContexts.getTargetPlatform() : null;
// Get the configuration targets that trigger this rule's configurable attributes.
@@ -653,188 +655,65 @@ private static TargetAndConfiguration computeTargetAndConfiguration(
return state.targetAndConfiguration;
}
- /**
- * Simple wrapper to allow returning two variables from {@link #computeUnloadedToolchainContexts}.
- */
- @VisibleForTesting
- public static class ComputedToolchainContexts {
- @Nullable public ToolchainCollection toolchainCollection = null;
- public ExecGroupCollection.Builder execGroupCollectionBuilder =
- ExecGroupCollection.emptyBuilder();
- }
+ @AutoValue
+ abstract static class UnloadedToolchainContextsInputs extends ExecGroupCollection.Builder {
+ @Nullable // Null if no toolchain resolution is required.
+ abstract ToolchainContextKey targetToolchainContextKey();
- @VisibleForTesting
- @Nullable
- public static ComputedToolchainContexts computeUnloadedToolchainContexts(
- Environment env,
- RuleClassProvider ruleClassProvider,
- TargetAndConfiguration targetAndConfig,
- @Nullable Label parentExecutionPlatformLabel)
- throws InterruptedException, ToolchainException {
-
- // We can only perform toolchain resolution on Targets and Aspects.
- if (!(targetAndConfig.getTarget() instanceof Rule)) {
- return new ComputedToolchainContexts();
+ static UnloadedToolchainContextsInputs create(
+ ImmutableMap processedExecGroups,
+ @Nullable ToolchainContextKey targetToolchainContextKey) {
+ return new AutoValue_PrerequisiteProducer_UnloadedToolchainContextsInputs(
+ processedExecGroups, targetToolchainContextKey);
}
- Label label = targetAndConfig.getLabel();
- Rule rule = ((Rule) targetAndConfig.getTarget());
- BuildConfigurationValue configuration = targetAndConfig.getConfiguration();
-
- ImmutableSet toolchainTypes =
- rule.getRuleClassObject().getToolchainTypes();
- // Collect local (target, rule) constraints for filtering out execution platforms.
- ImmutableSet defaultExecConstraintLabels =
- getExecutionPlatformConstraints(
- rule, configuration.getFragment(PlatformConfiguration.class));
- ImmutableMap execGroups = rule.getRuleClassObject().getExecGroups();
-
- // The toolchain context's options are the parent rule's options with manual trimming
- // auto-applied. This means toolchains don't inherit feature flags. This helps build
- // performance: if the toolchain context had the exact same configuration of its parent and that
- // included feature flags, all the toolchain's dependencies would apply this transition
- // individually. That creates a lot more potentially expensive applications of that transition
- // (especially since manual trimming applies to every configured target in the build).
- //
- // In other words: without this modification:
- // parent rule -> toolchain context -> toolchain
- // -> toolchain dep 1 # applies manual trimming to remove feature flags
- // -> toolchain dep 2 # applies manual trimming to remove feature flags
- // ...
- //
- // With this modification:
- // parent rule -> toolchain context # applies manual trimming to remove feature flags
- // -> toolchain
- // -> toolchain dep 1
- // -> toolchain dep 2
- // ...
- //
- // None of this has any effect on rules that don't utilize manual trimming.
- PatchTransition toolchainTaggedTrimmingTransition =
- ((ConfiguredRuleClassProvider) ruleClassProvider).getToolchainTaggedTrimmingTransition();
- BuildOptions toolchainOptions =
- toolchainTaggedTrimmingTransition.patch(
- new BuildOptionsView(
- configuration.getOptions(),
- toolchainTaggedTrimmingTransition.requiresOptionFragments()),
- env.getListener());
-
- BuildConfigurationKey toolchainConfig =
- BuildConfigurationKey.withoutPlatformMapping(toolchainOptions);
-
- PlatformConfiguration platformConfig = configuration.getFragment(PlatformConfiguration.class);
-
- boolean useAutoExecGroups;
- if (rule.isAttrDefined("$use_auto_exec_groups", Type.BOOLEAN)) {
- useAutoExecGroups = (boolean) rule.getAttr("$use_auto_exec_groups");
- } else {
- useAutoExecGroups = configuration.useAutoExecGroups();
+ static UnloadedToolchainContextsInputs empty() {
+ return new AutoValue_PrerequisiteProducer_UnloadedToolchainContextsInputs(
+ ImmutableMap.of(), /* targetToolchainContextKey= */ null);
}
-
- return computeUnloadedToolchainContexts(
- env,
- label,
- platformConfig != null && rule.useToolchainResolution(),
- l -> platformConfig != null && platformConfig.debugToolchainResolution(l),
- useAutoExecGroups,
- toolchainConfig,
- toolchainTypes,
- defaultExecConstraintLabels,
- execGroups,
- parentExecutionPlatformLabel);
}
/**
- * Returns the toolchain context and exec group collection for this target. The toolchain context
- * may be {@code null} if the target doesn't use toolchains.
+ * Computes the unloaded toolchain contexts.
*
- * This involves Skyframe evaluation: callers should check {@link Environment#valuesMissing()
- * to check the result is valid.
+ * @return null if a Skyframe restart is needed, {@link Optional#empty} if there is no toolchain
+ * resolution and the unloaded toolchain contexts otherwise.
*/
+ @VisibleForTesting // package private
@Nullable
- static ComputedToolchainContexts computeUnloadedToolchainContexts(
- Environment env,
- Label label,
- boolean useToolchainResolution,
- Predicate debugResolution,
- boolean useAutoExecGroups,
- BuildConfigurationKey configurationKey,
- ImmutableSet toolchainTypes,
- ImmutableSet defaultExecConstraintLabels,
- ImmutableMap execGroups,
- @Nullable Label parentExecutionPlatformLabel)
- throws InterruptedException, ToolchainException {
-
- Map allExecGroups = new HashMap<>();
-
- // Add exec groups that the rule itself has defined (custom exec groups).
- allExecGroups.putAll(execGroups);
-
- if (useAutoExecGroups) {
- // Create one exec group for each toolchain (automatic exec groups).
- for (ToolchainTypeRequirement toolchainType : toolchainTypes) {
- allExecGroups.put(
- toolchainType.toolchainType().toString(),
- ExecGroup.builder().addToolchainType(toolchainType).copyFrom(null).build());
- }
- }
-
- ExecGroupCollection.Builder execGroupCollectionBuilder =
- ExecGroupCollection.builder(
- /* execGroups= */ ImmutableMap.copyOf(allExecGroups),
- /* defaultExecWith= */ defaultExecConstraintLabels,
- /* defaultToolchainTypes= */ toolchainTypes);
-
+ public static Optional>
+ computeUnloadedToolchainContexts(Environment env, UnloadedToolchainContextsInputs inputs)
+ throws InterruptedException, ToolchainException {
+ var targetToolchainContextKey = inputs.targetToolchainContextKey();
// Short circuit and end now if this target doesn't require toolchain resolution.
- if (!useToolchainResolution) {
- ComputedToolchainContexts result = new ComputedToolchainContexts();
- result.execGroupCollectionBuilder = execGroupCollectionBuilder;
- return result;
+ if (targetToolchainContextKey == null) {
+ return Optional.empty();
}
- Map toolchainContextKeys = new HashMap<>();
- String targetUnloadedToolchainContext = "target-unloaded-toolchain-context";
-
- // Check if this specific target should be debugged for toolchain resolution.
- boolean debugTarget = debugResolution.test(label);
+ var toolchainContextKeys = new HashMap();
+ toolchainContextKeys.put(DEFAULT_EXEC_GROUP_NAME, targetToolchainContextKey);
- ToolchainContextKey.Builder toolchainContextKeyBuilder =
+ var keyBuilder =
ToolchainContextKey.key()
- .configurationKey(configurationKey)
- .execConstraintLabels(defaultExecConstraintLabels)
- .debugTarget(debugTarget);
-
- // Add toolchain types only if automatic exec groups are not created for this target.
- if (!useAutoExecGroups) {
- toolchainContextKeyBuilder.toolchainTypes(toolchainTypes);
- }
-
- if (parentExecutionPlatformLabel != null) {
- // Find out what execution platform the parent used, and force that.
- // This should only be set for direct toolchain dependencies.
- toolchainContextKeyBuilder.forceExecutionPlatform(parentExecutionPlatformLabel);
- }
-
- ToolchainContextKey toolchainContextKey = toolchainContextKeyBuilder.build();
- toolchainContextKeys.put(targetUnloadedToolchainContext, toolchainContextKey);
- for (String name : execGroupCollectionBuilder.getExecGroupNames()) {
- ExecGroup execGroup = execGroupCollectionBuilder.getExecGroup(name);
+ .configurationKey(targetToolchainContextKey.configurationKey())
+ .debugTarget(targetToolchainContextKey.debugTarget());
+ for (Map.Entry entry : inputs.execGroups().entrySet()) {
+ var execGroup = entry.getValue();
toolchainContextKeys.put(
- name,
- ToolchainContextKey.key()
- .configurationKey(configurationKey)
+ entry.getKey(),
+ keyBuilder
.toolchainTypes(execGroup.toolchainTypes())
.execConstraintLabels(execGroup.execCompatibleWith())
- .debugTarget(debugTarget)
.build());
}
SkyframeLookupResult values = env.getValuesAndExceptions(toolchainContextKeys.values());
-
boolean valuesMissing = env.valuesMissing();
ToolchainCollection.Builder toolchainContexts =
- valuesMissing ? null : ToolchainCollection.builder();
+ valuesMissing
+ ? null
+ : ToolchainCollection.builderWithExpectedSize(toolchainContextKeys.size());
for (Map.Entry unloadedToolchainContextKey :
toolchainContextKeys.entrySet()) {
UnloadedToolchainContext unloadedToolchainContext =
@@ -850,19 +729,16 @@ static ComputedToolchainContexts computeUnloadedToolchainContexts(
throw new NoMatchingPlatformException(unloadedToolchainContext.errorData());
}
if (!valuesMissing) {
- String execGroup = unloadedToolchainContextKey.getKey();
- if (execGroup.equals(targetUnloadedToolchainContext)) {
- toolchainContexts.addDefaultContext(unloadedToolchainContext);
- } else {
- toolchainContexts.addContext(execGroup, unloadedToolchainContext);
- }
+ toolchainContexts.addContext(
+ unloadedToolchainContextKey.getKey(), unloadedToolchainContext);
}
}
- ComputedToolchainContexts result = new ComputedToolchainContexts();
- result.toolchainCollection = env.valuesMissing() ? null : toolchainContexts.build();
- result.execGroupCollectionBuilder = execGroupCollectionBuilder;
- return result;
+ if (valuesMissing) {
+ return null;
+ }
+
+ return Optional.of(toolchainContexts.build());
}
/**
@@ -1144,6 +1020,115 @@ static ConfigConditions computeConfigConditions(
asConfiguredTargets.buildOrThrow(), asConfigConditions.buildOrThrow());
}
+ static ToolchainContextKey createDefaultToolchainContextKey(
+ BuildConfigurationKey configurationKey,
+ ImmutableSet defaultExecConstraintLabels,
+ boolean debugTarget,
+ boolean useAutoExecGroups,
+ ImmutableSet toolchainTypes,
+ @Nullable Label parentExecutionPlatformLabel) {
+ ToolchainContextKey.Builder toolchainContextKeyBuilder =
+ ToolchainContextKey.key()
+ .configurationKey(configurationKey)
+ .execConstraintLabels(defaultExecConstraintLabels)
+ .debugTarget(debugTarget);
+
+ // Add toolchain types only if automatic exec groups are not created for this target.
+ if (!useAutoExecGroups) {
+ toolchainContextKeyBuilder.toolchainTypes(toolchainTypes);
+ }
+
+ if (parentExecutionPlatformLabel != null) {
+ // Find out what execution platform the parent used, and force that.
+ // This should only be set for direct toolchain dependencies.
+ toolchainContextKeyBuilder.forceExecutionPlatform(parentExecutionPlatformLabel);
+ }
+ return toolchainContextKeyBuilder.build();
+ }
+
+ @VisibleForTesting // private
+ public static UnloadedToolchainContextsInputs getUnloadedToolchainContextsInputs(
+ TargetAndConfiguration targetAndConfiguration,
+ @Nullable Label parentExecutionPlatformLabel,
+ RuleClassProvider ruleClassProvider,
+ ExtendedEventHandler listener)
+ throws InterruptedException {
+ var target = targetAndConfiguration.getTarget();
+ if (!(target instanceof Rule)) {
+ return UnloadedToolchainContextsInputs.empty();
+ }
+
+ Rule rule = (Rule) target;
+ var configuration = targetAndConfiguration.getConfiguration();
+ boolean useAutoExecGroups =
+ rule.isAttrDefined("$use_auto_exec_groups", Type.BOOLEAN)
+ ? (boolean) rule.getAttr("$use_auto_exec_groups")
+ : configuration.useAutoExecGroups();
+ var platformConfig = configuration.getFragment(PlatformConfiguration.class);
+ var defaultExecConstraintLabels = getExecutionPlatformConstraints(rule, platformConfig);
+ var ruleClass = rule.getRuleClassObject();
+ var processedExecGroups =
+ ExecGroupCollection.process(
+ ruleClass.getExecGroups(),
+ defaultExecConstraintLabels,
+ ruleClass.getToolchainTypes(),
+ useAutoExecGroups);
+
+ if (platformConfig == null || !rule.useToolchainResolution()) {
+ return UnloadedToolchainContextsInputs.create(
+ processedExecGroups, /* targetToolchainContextKey= */ null);
+ }
+
+ return UnloadedToolchainContextsInputs.create(
+ processedExecGroups,
+ createDefaultToolchainContextKey(
+ computeToolchainConfigurationKey(
+ configuration,
+ ((ConfiguredRuleClassProvider) ruleClassProvider)
+ .getToolchainTaggedTrimmingTransition(),
+ listener),
+ defaultExecConstraintLabels,
+ /* debugTarget= */ platformConfig.debugToolchainResolution(rule.getLabel()),
+ /* useAutoExecGroups= */ useAutoExecGroups,
+ ruleClass.getToolchainTypes(),
+ parentExecutionPlatformLabel));
+ }
+
+ private static BuildConfigurationKey computeToolchainConfigurationKey(
+ BuildConfigurationValue configuration,
+ PatchTransition toolchainTaggedTrimmingTransition,
+ ExtendedEventHandler listener)
+ throws InterruptedException {
+ // The toolchain context's options are the parent rule's options with manual trimming
+ // auto-applied. This means toolchains don't inherit feature flags. This helps build
+ // performance: if the toolchain context had the exact same configuration of its parent and that
+ // included feature flags, all the toolchain's dependencies would apply this transition
+ // individually. That creates a lot more potentially expensive applications of that transition
+ // (especially since manual trimming applies to every configured target in the build).
+ //
+ // In other words: without this modification:
+ // parent rule -> toolchain context -> toolchain
+ // -> toolchain dep 1 # applies manual trimming to remove feature flags
+ // -> toolchain dep 2 # applies manual trimming to remove feature flags
+ // ...
+ //
+ // With this modification:
+ // parent rule -> toolchain context # applies manual trimming to remove feature flags
+ // -> toolchain
+ // -> toolchain dep 1
+ // -> toolchain dep 2
+ // ...
+ //
+ // None of this has any effect on rules that don't utilize manual trimming.
+ BuildOptions toolchainOptions =
+ toolchainTaggedTrimmingTransition.patch(
+ new BuildOptionsView(
+ configuration.getOptions(),
+ toolchainTaggedTrimmingTransition.requiresOptionFragments()),
+ listener);
+ return BuildConfigurationKey.withoutPlatformMapping(toolchainOptions);
+ }
+
/**
* Resolves the targets referenced in depValueNames and returns their {@link
* ConfiguredTargetAndData} instances.
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewForTesting.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewForTesting.java
index 5dc750f85282ea..39d184e11b60c6 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewForTesting.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewForTesting.java
@@ -54,7 +54,6 @@
import com.google.devtools.build.lib.analysis.DependencyKind;
import com.google.devtools.build.lib.analysis.DependencyResolver;
import com.google.devtools.build.lib.analysis.DuplicateException;
-import com.google.devtools.build.lib.analysis.ExecGroupCollection;
import com.google.devtools.build.lib.analysis.ExecGroupCollection.InvalidExecGroupException;
import com.google.devtools.build.lib.analysis.InconsistentAspectOrderException;
import com.google.devtools.build.lib.analysis.ResolvedToolchainContext;
@@ -104,7 +103,6 @@
import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey;
import com.google.devtools.build.lib.skyframe.PackageValue;
import com.google.devtools.build.lib.skyframe.PrerequisiteProducer;
-import com.google.devtools.build.lib.skyframe.PrerequisiteProducer.ComputedToolchainContexts;
import com.google.devtools.build.lib.skyframe.SkyFunctionEnvironmentForTesting;
import com.google.devtools.build.lib.skyframe.SkyframeBuildView;
import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
@@ -125,6 +123,7 @@
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
+import java.util.Optional;
import java.util.Set;
import java.util.function.Function;
import java.util.stream.Collectors;
@@ -638,16 +637,17 @@ public RuleContext getRuleContextForTesting(
SkyFunctionEnvironmentForTesting skyfunctionEnvironment =
new SkyFunctionEnvironmentForTesting(eventHandler, skyframeExecutor);
- ComputedToolchainContexts result =
- PrerequisiteProducer.computeUnloadedToolchainContexts(
- skyfunctionEnvironment,
- ruleClassProvider,
+ var unloadedToolchainContextsInputs =
+ PrerequisiteProducer.getUnloadedToolchainContextsInputs(
new TargetAndConfiguration(target.getAssociatedRule(), targetConfig),
- null);
+ /* parentExecutionPlatformLabel= */ null,
+ ruleClassProvider,
+ eventHandler);
+ Optional> result =
+ PrerequisiteProducer.computeUnloadedToolchainContexts(
+ skyfunctionEnvironment, unloadedToolchainContextsInputs);
- ToolchainCollection unloadedToolchainCollection =
- result.toolchainCollection;
- ExecGroupCollection.Builder execGroupCollectionBuilder = result.execGroupCollectionBuilder;
+ ToolchainCollection unloadedToolchainCollection = result.orElse(null);
OrderedSetMultimap prerequisiteMap =
getPrerequisiteMapForTesting(
@@ -668,7 +668,7 @@ public RuleContext getRuleContextForTesting(
resolvedToolchainContext.addContext(unloadedToolchainContext.getKey(), toolchainContext);
}
- return new RuleContext.Builder(env, target, /*aspects=*/ ImmutableList.of(), targetConfig)
+ return new RuleContext.Builder(env, target, /* aspects= */ ImmutableList.of(), targetConfig)
.setRuleClassProvider(ruleClassProvider)
.setConfigurationFragmentPolicy(
target.getAssociatedRule().getRuleClassObject().getConfigurationFragmentPolicy())
@@ -681,7 +681,7 @@ public RuleContext getRuleContextForTesting(
.setPrerequisites(ConfiguredTargetFactory.transformPrerequisiteMap(prerequisiteMap))
.setConfigConditions(ConfigConditions.EMPTY)
.setToolchainContexts(resolvedToolchainContext.build())
- .setExecGroupCollectionBuilder(execGroupCollectionBuilder)
+ .setExecGroupCollectionBuilder(unloadedToolchainContextsInputs)
.unsafeBuild();
}
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ToolchainsForTargetsTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ToolchainsForTargetsTest.java
index 69ca6da80e2ae7..ac28444c3d45d2 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/ToolchainsForTargetsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/ToolchainsForTargetsTest.java
@@ -37,7 +37,6 @@
import com.google.devtools.build.lib.analysis.util.AnalysisTestCase;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.packages.RuleClassProvider;
-import com.google.devtools.build.lib.skyframe.PrerequisiteProducer.ComputedToolchainContexts;
import com.google.devtools.build.lib.skyframe.util.SkyframeExecutorTestUtils;
import com.google.devtools.build.skyframe.EvaluationResult;
import com.google.devtools.build.skyframe.SkyFunction;
@@ -46,6 +45,7 @@
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
+import java.util.Optional;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -91,7 +91,7 @@ public SkyFunctionName functionName() {
/**
* Returns a {@link ToolchainCollection} as the result of {@link
- * ConfiguredTargetFunction#computeUnloadedToolchainContexts}.
+ * PrerequisiteProducer#computeUnloadedToolchainContexts}.
*/
@AutoValue
abstract static class Value implements SkyValue {
@@ -104,7 +104,7 @@ static Value create(ToolchainCollection toolchainColle
/**
* A mock {@link SkyFunction} that just calls {@link
- * ConfiguredTargetFunction#computeUnloadedToolchainContexts} and returns its results.
+ * PrerequisiteProducer#computeUnloadedToolchainContexts} and returns its results.
*/
static class ComputeUnloadedToolchainContextsFunction implements SkyFunction {
static final SkyFunctionName SKYFUNCTION_NAME =
@@ -122,17 +122,19 @@ public SkyValue compute(SkyKey skyKey, Environment env)
throws ComputeUnloadedToolchainContextsException, InterruptedException {
try {
Key key = (Key) skyKey.argument();
- ComputedToolchainContexts result =
- PrerequisiteProducer.computeUnloadedToolchainContexts(
- env,
- stateProvider.lateBoundRuleClassProvider(),
+ var unloadedToolchainContextsInputs =
+ PrerequisiteProducer.getUnloadedToolchainContextsInputs(
key.targetAndConfiguration(),
- key.configuredTargetKey().getExecutionPlatformLabel());
- if (env.valuesMissing()) {
+ key.configuredTargetKey().getExecutionPlatformLabel(),
+ stateProvider.lateBoundRuleClassProvider(),
+ env.getListener());
+ Optional> result =
+ PrerequisiteProducer.computeUnloadedToolchainContexts(
+ env, unloadedToolchainContextsInputs);
+ if (result == null) {
return null;
}
- ToolchainCollection toolchainCollection =
- result.toolchainCollection;
+ ToolchainCollection toolchainCollection = result.orElse(null);
return Value.create(toolchainCollection);
} catch (ToolchainException e) {
throw new ComputeUnloadedToolchainContextsException(e);