Skip to content

Commit

Permalink
Memoize IncompatibleTargetChecker.createDirectlyIncompatibleTarget.
Browse files Browse the repository at this point in the history 
This uses StateMachine to avoid recomputation on restart.

PiperOrigin-RevId: 513546261
Change-Id: Ice4245a72fe29ccd8dda040910460626d5041512
  • Loading branch information
@aoeui @copybara-github
aoeui authored and copybara-github committed Mar 2, 2023
1 parent 5a72be5 commit 078c14c
Show file tree
Hide file tree
Showing 3 changed files with 142 additions and 73 deletions.
3 changes: 2 additions & 1 deletion src/main/java/com/google/devtools/build/lib/analysis/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2256,20 +2256,21 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/analysis:config/config_conditions",
"//src/main/java/com/google/devtools/build/lib/analysis:configured_target",
"//src/main/java/com/google/devtools/build/lib/analysis:configured_target_value",
"//src/main/java/com/google/devtools/build/lib/analysis:dependency",
"//src/main/java/com/google/devtools/build/lib/analysis:dependency_kind",
"//src/main/java/com/google/devtools/build/lib/analysis:target_and_configuration",
"//src/main/java/com/google/devtools/build/lib/analysis/platform",
"//src/main/java/com/google/devtools/build/lib/analysis/platform:utils",
"//src/main/java/com/google/devtools/build/lib/cmdline",
"//src/main/java/com/google/devtools/build/lib/collect/nestedset",
"//src/main/java/com/google/devtools/build/lib/events",
"//src/main/java/com/google/devtools/build/lib/packages",
"//src/main/java/com/google/devtools/build/lib/packages:configured_attribute_mapper",
"//src/main/java/com/google/devtools/build/lib/skyframe:configured_target_and_data",
"//src/main/java/com/google/devtools/build/lib/skyframe:configured_target_key",
"//src/main/java/com/google/devtools/build/lib/skyframe:rule_configured_target_value",
"//src/main/java/com/google/devtools/build/lib/util",
"//src/main/java/com/google/devtools/build/skyframe",
"//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
"//third_party:guava",
"//third_party:jsr305",
],
Expand Down
151 changes: 87 additions & 64 deletions ...in/java/com/google/devtools/build/lib/analysis/constraints/IncompatibleTargetChecker.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,13 @@

package com.google.devtools.build.lib.analysis.constraints;

import static com.google.common.base.Predicates.notNull;
import static com.google.common.collect.ImmutableList.toImmutableList;

import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.ConfiguredTargetValue;
import com.google.devtools.build.lib.analysis.Dependency;
import com.google.devtools.build.lib.analysis.DependencyKind;
import com.google.devtools.build.lib.analysis.FileProvider;
import com.google.devtools.build.lib.analysis.FilesToRunProvider;
Expand All @@ -44,6 +42,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.collect.nestedset.Order;
import com.google.devtools.build.lib.events.ExtendedEventHandler;
import com.google.devtools.build.lib.packages.BuildType;
import com.google.devtools.build.lib.packages.ConfiguredAttributeMapper;
import com.google.devtools.build.lib.packages.Package;
Expand All @@ -55,10 +54,10 @@
import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey;
import com.google.devtools.build.lib.skyframe.RuleConfiguredTargetValue;
import com.google.devtools.build.lib.util.OrderedSetMultimap;
import com.google.devtools.build.skyframe.SkyFunction.Environment;
import com.google.devtools.build.skyframe.SkyframeLookupResult;
import java.util.List;
import com.google.devtools.build.skyframe.SkyValue;
import com.google.devtools.build.skyframe.state.StateMachine;
import java.util.Optional;
import java.util.function.Consumer;
import javax.annotation.Nullable;

/**
Expand Down Expand Up @@ -87,81 +86,105 @@ public class IncompatibleTargetChecker {
/**
* Creates an incompatible configured target if it is "directly incompatible".
*
* <p>In other words, this function checks if a target is incompatible because of its
* <p>In other words, this state machine checks if a target is incompatible because of its
* "target_compatible_with" attribute.
*
* <p>This function returns a nullable {@code Optional} of a {@link RuleConfiguredTargetValue}.
* This provides three states of return values:
* <p>Outputs an {@code Optional} {@link RuleConfiguredTargetValue} as follows.
*
* <ul>
* <li>{@code null}: A skyframe restart is required.
* <li>{@code Optional.empty()}: The target is not directly incompatible. Analysis can continue.
* <li>{@code !Optional.empty()}: The target is directly incompatible. Analysis should not
* continue.
* </ul>
*/
@Nullable
public static Optional<RuleConfiguredTargetValue> createDirectlyIncompatibleTarget(
TargetAndConfiguration targetAndConfiguration,
ConfigConditions configConditions,
Environment env,
@Nullable PlatformInfo platformInfo,
NestedSetBuilder<Package> transitivePackages)
throws InterruptedException {
Target target = targetAndConfiguration.getTarget();
Rule rule = target.getAssociatedRule();
public static class IncompatibleTargetProducer implements StateMachine, Consumer<SkyValue> {
private final Target target;
@Nullable // Non-null when the target has an associated rule.
private final BuildConfigurationValue configuration;
private final ConfigConditions configConditions;
@Nullable private final PlatformInfo platformInfo;
@Nullable private final NestedSetBuilder<Package> transitivePackages;

if (rule == null || rule.getRuleClass().equals("toolchain") || platformInfo == null) {
return Optional.empty();
private final ResultSink sink;

private final ImmutableList.Builder<ConstraintValueInfo> invalidConstraintValuesBuilder =
new ImmutableList.Builder<>();

/** Sink for the output of this state machine. */
public interface ResultSink {
void accept(Optional<RuleConfiguredTargetValue> incompatibleTarget);
}

// Retrieve the label list for the target_compatible_with attribute.
BuildConfigurationValue configuration = targetAndConfiguration.getConfiguration();
ConfiguredAttributeMapper attrs =
ConfiguredAttributeMapper.of(rule, configConditions.asProviders(), configuration);
if (!attrs.has("target_compatible_with", BuildType.LABEL_LIST)) {
return Optional.empty();
public IncompatibleTargetProducer(
Target target,
@Nullable BuildConfigurationValue configuration,
ConfigConditions configConditions,
@Nullable PlatformInfo platformInfo,
@Nullable NestedSetBuilder<Package> transitivePackages,
ResultSink sink) {
this.target = target;
this.configuration = configuration;
this.configConditions = configConditions;
this.platformInfo = platformInfo;
this.transitivePackages = transitivePackages;
this.sink = sink;
}

// Resolve the constraint labels.
List<Label> labels = attrs.get("target_compatible_with", BuildType.LABEL_LIST);
ImmutableList<ConfiguredTargetKey> constraintKeys =
labels.stream()
.map(
label ->
Dependency.builder()
.setLabel(label)
.setConfiguration(configuration)
.build()
.getConfiguredTargetKey())
.collect(toImmutableList());
SkyframeLookupResult constraintValues = env.getValuesAndExceptions(constraintKeys);
if (env.valuesMissing()) {
return null;
@Override
@Nullable
public StateMachine step(Tasks tasks, ExtendedEventHandler listener) {
Rule rule = target.getAssociatedRule();
if (rule == null || rule.getRuleClass().equals("toolchain") || platformInfo == null) {
sink.accept(Optional.empty());
return null;
}

// Retrieves the label list for the target_compatible_with attribute.
ConfiguredAttributeMapper attrs =
ConfiguredAttributeMapper.of(rule, configConditions.asProviders(), configuration);
if (!attrs.has("target_compatible_with", BuildType.LABEL_LIST)) {
sink.accept(Optional.empty());
return null;
}

// Resolves the constraint labels.
for (Label label : attrs.get("target_compatible_with", BuildType.LABEL_LIST)) {
tasks.lookUp(
ConfiguredTargetKey.builder().setLabel(label).setConfiguration(configuration).build(),
this);
}
return this::processResult;
}

// Find the constraints that don't satisfy the target platform.
ImmutableList<ConstraintValueInfo> invalidConstraintValues =
constraintKeys.stream()
.map(key -> (ConfiguredTargetValue) constraintValues.get(key))
.filter(notNull())
.map(ctv -> PlatformProviderUtils.constraintValue(ctv.getConfiguredTarget()))
.filter(notNull())
.filter(cv -> !platformInfo.constraints().hasConstraintValue(cv))
.collect(toImmutableList());
if (invalidConstraintValues.isEmpty()) {
return Optional.empty();
@Override
public void accept(SkyValue value) {
var configuredTarget = ((ConfiguredTargetValue) value).getConfiguredTarget();
@Nullable ConstraintValueInfo info = PlatformProviderUtils.constraintValue(configuredTarget);
if (info == null || platformInfo.constraints().hasConstraintValue(info)) {
return;
}
invalidConstraintValuesBuilder.add(info);
}

return Optional.of(
createIncompatibleRuleConfiguredTarget(
target,
configuration,
configConditions,
IncompatiblePlatformProvider.incompatibleDueToConstraints(
platformInfo.label(), invalidConstraintValues),
rule.getRuleClass(),
transitivePackages));
@Nullable
private StateMachine processResult(Tasks tasks, ExtendedEventHandler listener) {
var invalidConstraintValues = invalidConstraintValuesBuilder.build();
if (invalidConstraintValues.isEmpty()) {
sink.accept(Optional.empty());
return null;
}
sink.accept(
Optional.of(
createIncompatibleRuleConfiguredTarget(
target,
configuration,
configConditions,
IncompatiblePlatformProvider.incompatibleDueToConstraints(
platformInfo.label(), invalidConstraintValues),
target.getAssociatedRule().getRuleClass(),
transitivePackages)));
return null;
}
}

/**
Expand All @@ -185,7 +208,7 @@ public static Optional<RuleConfiguredTargetValue> createIndirectlyIncompatibleTa
OrderedSetMultimap<DependencyKind, ConfiguredTargetAndData> depValueMap,
ConfigConditions configConditions,
@Nullable PlatformInfo platformInfo,
NestedSetBuilder<Package> transitivePackages) {
@Nullable NestedSetBuilder<Package> transitivePackages) {
Target target = targetAndConfiguration.getTarget();
Rule rule = target.getAssociatedRule();

Expand Down Expand Up @@ -223,7 +246,7 @@ private static RuleConfiguredTargetValue createIncompatibleRuleConfiguredTarget(
ConfigConditions configConditions,
IncompatiblePlatformProvider incompatiblePlatformProvider,
String ruleClassString,
NestedSetBuilder<Package> transitivePackages) {
@Nullable NestedSetBuilder<Package> transitivePackages) {
// Create dummy instances of the necessary data for a configured target. None of this data will
// actually be used because actions associated with incompatible targets must not be evaluated.
NestedSet<Artifact> filesToBuild = NestedSetBuilder.emptySet(Order.STABLE_ORDER);
Expand Down
61 changes: 53 additions & 8 deletions src/main/java/com/google/devtools/build/lib/skyframe/PrerequisiteProducer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import com.google.devtools.build.lib.analysis.config.ToolchainTypeRequirement;
import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition;
import com.google.devtools.build.lib.analysis.constraints.IncompatibleTargetChecker;
import com.google.devtools.build.lib.analysis.constraints.IncompatibleTargetChecker.IncompatibleTargetProducer;
import com.google.devtools.build.lib.analysis.platform.PlatformInfo;
import com.google.devtools.build.lib.bugreport.BugReport;
import com.google.devtools.build.lib.causes.Cause;
Expand Down Expand Up @@ -79,6 +80,7 @@
import com.google.devtools.build.skyframe.SkyFunction.Environment.SkyKeyComputeState;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyframeLookupResult;
import com.google.devtools.build.skyframe.state.Driver;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
Expand Down Expand Up @@ -113,9 +115,22 @@
* <p>See {@link ConfiguredTargetFunction} for more review on analysis implementation.
*/
public final class PrerequisiteProducer {
static class State implements SkyKeyComputeState {
static class State implements SkyKeyComputeState, IncompatibleTargetProducer.ResultSink {
@Nullable TargetAndConfiguration targetAndConfiguration;

/**
* Drives the stateful computation of {@link #incompatibleTarget}.
*
* <p>Non-null only while the computation is in-flight.
*/
@Nullable private Driver incompatibleTargetProducer;
/**
* If a value is present, it means the target was directly incompatible.
*
* <p>Non-null after the {@link #incompatibleTargetProducer} completes.
*/
private Optional<RuleConfiguredTargetValue> incompatibleTarget;

/** Null if not yet computed or if {@link #resolveConfigurationsResult} is non-null. */
@Nullable private OrderedSetMultimap<DependencyKind, DependencyKey> dependentNodeMapResult;

Expand Down Expand Up @@ -154,6 +169,11 @@ static class State implements SkyKeyComputeState {
* thrown away.
*/
@Nullable private StoredEventHandler storedEventHandlerFromResolveConfigurations;

@Override
public void accept(Optional<RuleConfiguredTargetValue> incompatibleTarget) {
this.incompatibleTarget = incompatibleTarget;
}
}

/**
Expand Down Expand Up @@ -341,15 +361,9 @@ public boolean evaluate(
getPrioritizedDetailedExitCode(causes)));
}

Optional<RuleConfiguredTargetValue> incompatibleTarget =
IncompatibleTargetChecker.createDirectlyIncompatibleTarget(
targetAndConfiguration, configConditions, env, platformInfo, transitivePackages);
if (incompatibleTarget == null) {
if (!checkForIncompatibleTarget(env, state, transitivePackages)) {
return false;
}
if (incompatibleTarget.isPresent()) {
throw new IncompatibleTargetException(incompatibleTarget.get());
}

// Calculate the dependencies of this target.
depValueMap =
Expand Down Expand Up @@ -391,6 +405,37 @@ public boolean evaluate(
return true;
}

/**
* Checks if a target is incompatible because of its "target_compatible_with" attribute.
*
* @return false if a {@code Skyframe} restart is needed.
*/
private boolean checkForIncompatibleTarget(
Environment env, State state, @Nullable NestedSetBuilder<Package> transitivePackages)
throws InterruptedException, IncompatibleTargetException {
if (state.incompatibleTarget == null) {
if (state.incompatibleTargetProducer == null) {
state.incompatibleTargetProducer =
new Driver(
new IncompatibleTargetProducer(
targetAndConfiguration.getTarget(),
targetAndConfiguration.getConfiguration(),
configConditions,
platformInfo,
transitivePackages,
state));
}
if (!state.incompatibleTargetProducer.drive(env, env.getListener())) {
return false;
}
state.incompatibleTargetProducer = null;
if (state.incompatibleTarget.isPresent()) {
throw new IncompatibleTargetException(state.incompatibleTarget.get());
}
}
return true;
}

/**
* Handles all exceptions that {@link #evaluate} may throw.
*
Expand Down

0 comments on commit 078c14c

Please sign in to comment.