Refactor the ActionLookupValue/ConfiguredObjectValue hierarchy to reflect the reality that some configured targets will never have associated actions (all non-rule configured targets, including input/output files, aliases, and package groups).
This allows us to more explicitly track the different categories of configured targets for metrics. Because non-rule configured targets can depend on rule configured targets (an output file depends on its generating rule, an alias depends on a target), the traversal of the analysis graph that powers these metrics must still visit these non-rule configured targets, allowing them to be tracked with little additional overhead.
As a small optimization, we can delete from the Skyframe graph any nodes corresponding to non-rule configured targets if dropping the analysis phase.
PiperOrigin-RevId: 368457602
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionLookupKey.java b/src/main/java/com/google/devtools/build/lib/actions/ActionLookupKey.java
index 62db95b..fa81239 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionLookupKey.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionLookupKey.java
@@ -16,7 +16,12 @@
import com.google.devtools.build.skyframe.SkyKey;
/**
- * {@link SkyKey} for an {@link ActionLookupValue}.
+ * {@link SkyKey} for an "analysis object": either an {@link ActionLookupValue} or a {@link
+ * com.google.devtools.build.lib.analysis.ConfiguredTargetValue}.
+ *
+ * <p>Whether a configured target creates actions cannot be inferred from its {@link
+ * com.google.devtools.build.lib.cmdline.Label} without performing analysis, so this class is used
+ * for both types. Non-{@link ActionLookupValue} nodes are not accessed during the execution phase.
*
* <p>All subclasses of {@link ActionLookupValue} "own" artifacts with {@link ArtifactOwner}s that
* are subclasses of {@link ActionLookupKey}. This allows callers to easily find the value key,
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionLookupValue.java b/src/main/java/com/google/devtools/build/lib/actions/ActionLookupValue.java
index 6dbc68a..ef536c8 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionLookupValue.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionLookupValue.java
@@ -14,9 +14,7 @@
package com.google.devtools.build.lib.actions;
import com.google.common.collect.ImmutableList;
-import com.google.devtools.build.lib.actions.Artifact.SourceArtifact;
import com.google.devtools.build.skyframe.SkyValue;
-import javax.annotation.Nullable;
/** Base interface for all values which can provide the generating action of an artifact. */
public interface ActionLookupValue extends SkyValue {
@@ -52,11 +50,4 @@
default int getNumActions() {
return getActions().size();
}
-
- /** Returns a source artifact if the underlying configured target is an input file. */
- @Nullable
- default SourceArtifact getSourceArtifact() {
- return null;
- }
-
}
diff --git a/src/main/java/com/google/devtools/build/lib/actions/AnalysisGraphStatsEvent.java b/src/main/java/com/google/devtools/build/lib/actions/AnalysisGraphStatsEvent.java
index 45a4039..4d6b82b 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/AnalysisGraphStatsEvent.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/AnalysisGraphStatsEvent.java
@@ -13,34 +13,22 @@
// limitations under the License.
package com.google.devtools.build.lib.actions;
+import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos;
+
/**
* Event transporting data about the size/shape of the analysis graph. Only emitted when Bazel is
* forced to visit the entire analysis graph (for action/artifact conflict checking). See {@link
* com.google.devtools.build.lib.skyframe.SkyframeBuildView#shouldCheckForConflicts}.
*/
public final class AnalysisGraphStatsEvent {
- private final TotalAndConfiguredTargetOnlyMetric actionLookupValueCount;
- private final TotalAndConfiguredTargetOnlyMetric actionCount;
- private final int outputArtifactCount;
+ private final BuildEventStreamProtos.BuildMetrics.BuildGraphMetrics buildGraphMetrics;
public AnalysisGraphStatsEvent(
- TotalAndConfiguredTargetOnlyMetric actionLookupValueCount,
- TotalAndConfiguredTargetOnlyMetric actionCount,
- int outputArtifactCount) {
- this.actionLookupValueCount = actionLookupValueCount;
- this.actionCount = actionCount;
- this.outputArtifactCount = outputArtifactCount;
+ BuildEventStreamProtos.BuildMetrics.BuildGraphMetrics buildGraphMetrics) {
+ this.buildGraphMetrics = buildGraphMetrics;
}
- public TotalAndConfiguredTargetOnlyMetric getActionLookupValueCount() {
- return actionLookupValueCount;
- }
-
- public TotalAndConfiguredTargetOnlyMetric getActionCount() {
- return actionCount;
- }
-
- public int getOutputArtifactCount() {
- return outputArtifactCount;
+ public BuildEventStreamProtos.BuildMetrics.BuildGraphMetrics getBuildGraphMetrics() {
+ return buildGraphMetrics;
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/actions/BUILD b/src/main/java/com/google/devtools/build/lib/actions/BUILD
index 9e9551d..153b81e 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/actions/BUILD
@@ -155,7 +155,7 @@
java_library(
name = "analysis_graph_stats_event",
srcs = ["AnalysisGraphStatsEvent.java"],
- deps = [":total_and_configured_target_only_metric"],
+ deps = ["//src/main/java/com/google/devtools/build/lib/buildeventstream/proto:build_event_stream_java_proto"],
)
java_library(
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/AspectValue.java b/src/main/java/com/google/devtools/build/lib/analysis/AspectValue.java
index 58a84d0..0c18c7f 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/AspectValue.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/AspectValue.java
@@ -24,7 +24,7 @@
import net.starlark.java.syntax.Location;
/** An aspect in the context of the Skyframe graph. */
-public final class AspectValue extends BasicActionLookupValue implements ConfiguredObjectValue {
+public final class AspectValue extends BasicActionLookupValue implements RuleConfiguredObjectValue {
// These variables are only non-final because they may be clear()ed to save memory. They are null
// only after they are cleared except for transitivePackagesForPackageRootResolution.
@Nullable private Aspect aspect;
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BUILD b/src/main/java/com/google/devtools/build/lib/analysis/BUILD
index 4a3d8c1..e41e289 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BUILD
@@ -313,7 +313,6 @@
":config/transitions/split_transition",
":config/transitions/transition_factory",
":configurations_collector",
- ":configured_object_value",
":configured_target",
":constraints/constraint_constants",
":constraints/constraint_semantics",
@@ -341,6 +340,7 @@
":provider_collection",
":required_config_fragments_provider",
":resolved_toolchain_context",
+ ":rule_configured_object_value",
":rule_definition_context",
":rule_definition_environment",
":starlark/args",
@@ -689,7 +689,6 @@
srcs = ["ConfiguredObjectValue.java"],
deps = [
":provider_collection",
- "//src/main/java/com/google/devtools/build/lib/actions",
"//src/main/java/com/google/devtools/build/lib/collect/nestedset",
"//src/main/java/com/google/devtools/build/lib/packages",
"//src/main/java/com/google/devtools/build/skyframe",
@@ -697,12 +696,20 @@
)
java_library(
+ name = "rule_configured_object_value",
+ srcs = ["RuleConfiguredObjectValue.java"],
+ deps = [
+ ":configured_object_value",
+ "//src/main/java/com/google/devtools/build/lib/actions",
+ ],
+)
+
+java_library(
name = "configured_target",
srcs = ["ConfiguredTarget.java"],
deps = [
":config/config_matching_provider",
":transitive_info_collection",
- "//src/main/java/com/google/devtools/build/lib/actions:artifacts",
"//src/main/java/com/google/devtools/build/lib/cmdline",
"//src/main/java/com/google/devtools/build/lib/skyframe:build_configuration_value",
"//src/main/java/net/starlark/java/eval",
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredObjectValue.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredObjectValue.java
index 353cbd5..b06000a 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredObjectValue.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredObjectValue.java
@@ -13,13 +13,15 @@
// limitations under the License.
package com.google.devtools.build.lib.analysis;
-import com.google.devtools.build.lib.actions.ActionLookupValue;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.packages.Package;
import com.google.devtools.build.skyframe.NotComparableSkyValue;
-/** Super-interface for {@link ConfiguredTargetValue} and {@link AspectValue}. */
-public interface ConfiguredObjectValue extends ActionLookupValue, NotComparableSkyValue {
+/**
+ * Super-interface for {@link ConfiguredTargetValue} and {@link RuleConfiguredObjectValue}
+ * (transitively including {@link AspectValue}).
+ */
+public interface ConfiguredObjectValue extends NotComparableSkyValue {
/** Returns the configured target/aspect for this value. */
ProviderCollection getConfiguredObject();
@@ -34,11 +36,10 @@
NestedSet<Package> getTransitivePackagesForPackageRootResolution();
/**
- * Clears provider data from this value, leaving only the artifact->generating action map.
+ * Clears data from this value.
*
* <p>Should only be used when user specifies --discard_analysis_cache. Must be called at most
- * once per value, after which {@link #getConfiguredObject} and {@link #getActions} cannot be
- * called.
+ * once per value, after which this object's other methods cannot be called.
*/
void clear(boolean clearEverything);
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTarget.java
index 4404f56..735276c 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTarget.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTarget.java
@@ -16,7 +16,6 @@
import com.google.common.collect.ImmutableCollection;
import com.google.common.collect.ImmutableMap;
-import com.google.devtools.build.lib.actions.Artifact.SourceArtifact;
import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.skyframe.BuildConfigurationValue;
@@ -71,12 +70,6 @@
@Override
Object getValue(String name);
- /** Returns a source artifact if this is an input file. */
- @Nullable
- default SourceArtifact getSourceArtifact() {
- return null;
- }
-
/**
* If the configured target is an alias, return the actual target, otherwise return the current
* target. This follows alias chains.
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredObjectValue.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredObjectValue.java
new file mode 100644
index 0000000..74e1333
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredObjectValue.java
@@ -0,0 +1,23 @@
+// Copyright 2021 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.build.lib.analysis;
+
+import com.google.devtools.build.lib.actions.ActionLookupValue;
+
+/**
+ * Common interface for {@link AspectValue} and {@link
+ * com.google.devtools.build.lib.skyframe.RuleConfiguredTargetValue}
+ */
+public interface RuleConfiguredObjectValue extends ConfiguredObjectValue, ActionLookupValue {}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/InputFileConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/InputFileConfiguredTarget.java
index d980d27..e397337 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/InputFileConfiguredTarget.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/InputFileConfiguredTarget.java
@@ -101,7 +101,6 @@
printer.append("<input file target " + getLabel() + ">");
}
- @Override
public SourceArtifact getSourceArtifact() {
return artifact;
}
diff --git a/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/build_event_stream.proto b/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/build_event_stream.proto
index 75c6b78..21c93a6 100644
--- a/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/build_event_stream.proto
+++ b/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/build_event_stream.proto
@@ -865,11 +865,14 @@
message BuildGraphMetrics {
// How many configured targets/aspects were in this build, including any
// that were analyzed on a prior build and are still valid. May not be
- // populated if analysis phase was fully cached.
+ // populated if analysis phase was fully cached. Note: for historical
+ // reasons this includes input/output files and other configured targets
+ // that do not actually have associated actions.
int32 action_lookup_value_count = 1;
// How many configured targets alone were in this build: always at most
// action_lookup_value_count. Useful mainly for historical comparisons to
- // TargetMetrics.targets_configured, which used to not count aspects.
+ // TargetMetrics.targets_configured, which used to not count aspects. This
+ // also includes configured targets that do not have associated actions.
int32 action_lookup_value_count_not_including_aspects = 5;
// How many actions belonged to the configured targets/aspects above. It may
// not be necessary to execute all of these actions to build the requested
@@ -879,6 +882,15 @@
// action_count. Useful mainly for historical comparisons to
// ActionMetrics.actions_created, which used to not count aspects' actions.
int32 action_count_not_including_aspects = 6;
+ // How many "input file" configured targets there were: one per source file.
+ // Should agree with artifact_metrics.source_artifacts_read.count above,
+ int32 input_file_configured_target_count = 7;
+ // How many "output file" configured targets there were: output files that
+ // are targets (not implicit outputs).
+ int32 output_file_configured_target_count = 8;
+ // How many "other" configured targets there were (like alias,
+ // package_group, and other non-rule non-file configured targets).
+ int32 other_configured_target_count = 9;
// How many artifacts are outputs of the above actions. May not be populated
// if analysis phase was fully cached.
int32 output_artifact_count = 3;
diff --git a/src/main/java/com/google/devtools/build/lib/metrics/BUILD b/src/main/java/com/google/devtools/build/lib/metrics/BUILD
index 45af86c..dcb1be5 100644
--- a/src/main/java/com/google/devtools/build/lib/metrics/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/metrics/BUILD
@@ -37,6 +37,7 @@
"//src/main/java/com/google/devtools/build/lib/actions:analysis_graph_stats_event",
"//src/main/java/com/google/devtools/build/lib/analysis:analysis_phase_complete_event",
"//src/main/java/com/google/devtools/build/lib/analysis:analysis_phase_started_event",
+ "//src/main/java/com/google/devtools/build/lib/bugreport",
"//src/main/java/com/google/devtools/build/lib/buildeventstream/proto:build_event_stream_java_proto",
"//src/main/java/com/google/devtools/build/lib/profiler",
"//src/main/java/com/google/devtools/build/lib/skyframe:execution_finished_event",
diff --git a/src/main/java/com/google/devtools/build/lib/metrics/MetricsCollector.java b/src/main/java/com/google/devtools/build/lib/metrics/MetricsCollector.java
index 18d2b2d..4666d56 100644
--- a/src/main/java/com/google/devtools/build/lib/metrics/MetricsCollector.java
+++ b/src/main/java/com/google/devtools/build/lib/metrics/MetricsCollector.java
@@ -20,6 +20,7 @@
import com.google.devtools.build.lib.actions.TotalAndConfiguredTargetOnlyMetric;
import com.google.devtools.build.lib.analysis.AnalysisPhaseCompleteEvent;
import com.google.devtools.build.lib.analysis.AnalysisPhaseStartedEvent;
+import com.google.devtools.build.lib.bugreport.BugReport;
import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos.BuildMetrics;
import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos.BuildMetrics.ActionSummary;
import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos.BuildMetrics.ArtifactMetrics;
@@ -100,15 +101,16 @@
@SuppressWarnings("unused")
@Subscribe
public synchronized void logAnalysisGraphStats(AnalysisGraphStatsEvent event) {
- TotalAndConfiguredTargetOnlyMetric actionLookupValueCount = event.getActionLookupValueCount();
- TotalAndConfiguredTargetOnlyMetric actionCount = event.getActionCount();
- buildGraphMetrics
- .setActionLookupValueCount(actionLookupValueCount.total())
- .setActionLookupValueCountNotIncludingAspects(
- actionLookupValueCount.configuredTargetsOnly())
- .setActionCount(actionCount.total())
- .setActionCountNotIncludingAspects(actionCount.configuredTargetsOnly())
- .setOutputArtifactCount(event.getOutputArtifactCount());
+ // Check only one event per build. No proto3 check for presence, so check for not-default value.
+ if (buildGraphMetrics.getActionLookupValueCount() > 0) {
+ BugReport.sendBugReport(
+ new IllegalStateException(
+ "Already initialized build graph metrics builder: "
+ + buildGraphMetrics
+ + ", "
+ + event.getBuildGraphMetrics()));
+ }
+ buildGraphMetrics.mergeFrom(event.getBuildGraphMetrics());
}
@SuppressWarnings("unused")
diff --git a/src/main/java/com/google/devtools/build/lib/query2/aquery/ActionGraphProtoOutputFormatterCallback.java b/src/main/java/com/google/devtools/build/lib/query2/aquery/ActionGraphProtoOutputFormatterCallback.java
index 945a240..e29e5b2 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/aquery/ActionGraphProtoOutputFormatterCallback.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/aquery/ActionGraphProtoOutputFormatterCallback.java
@@ -16,9 +16,9 @@
import com.google.devtools.build.lib.actions.CommandLineExpansionException;
import com.google.devtools.build.lib.analysis.AspectValue;
import com.google.devtools.build.lib.analysis.ConfiguredTargetValue;
-import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget;
import com.google.devtools.build.lib.events.ExtendedEventHandler;
import com.google.devtools.build.lib.query2.engine.QueryEnvironment.TargetAccessor;
+import com.google.devtools.build.lib.skyframe.RuleConfiguredTargetValue;
import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
import com.google.devtools.build.lib.skyframe.actiongraph.v2.ActionGraphDump;
import com.google.devtools.build.lib.skyframe.actiongraph.v2.AqueryOutputHandler;
@@ -92,12 +92,15 @@
options.includeCommandline |= options.includeParamFiles;
for (ConfiguredTargetValue configuredTargetValue : partialResult) {
- actionGraphDump.dumpConfiguredTarget(configuredTargetValue);
+ if (!(configuredTargetValue instanceof RuleConfiguredTargetValue)) {
+ // We have to include non-rule values in the graph to visit their dependencies, but they
+ // don't have any actions to print out.
+ continue;
+ }
+ actionGraphDump.dumpConfiguredTarget((RuleConfiguredTargetValue) configuredTargetValue);
if (options.useAspects) {
- if (configuredTargetValue.getConfiguredTarget() instanceof RuleConfiguredTarget) {
- for (AspectValue aspectValue : accessor.getAspectValues(configuredTargetValue)) {
- actionGraphDump.dumpAspect(aspectValue, configuredTargetValue);
- }
+ for (AspectValue aspectValue : accessor.getAspectValues(configuredTargetValue)) {
+ actionGraphDump.dumpAspect(aspectValue, configuredTargetValue);
}
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/query2/aquery/ActionGraphTextOutputFormatterCallback.java b/src/main/java/com/google/devtools/build/lib/query2/aquery/ActionGraphTextOutputFormatterCallback.java
index fccabc4..29ac05c 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/aquery/ActionGraphTextOutputFormatterCallback.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/aquery/ActionGraphTextOutputFormatterCallback.java
@@ -29,12 +29,12 @@
import com.google.devtools.build.lib.analysis.AspectValue;
import com.google.devtools.build.lib.analysis.ConfiguredTargetValue;
import com.google.devtools.build.lib.analysis.actions.ParameterFileWriteAction;
-import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget;
import com.google.devtools.build.lib.buildeventstream.BuildEvent;
import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos;
import com.google.devtools.build.lib.events.ExtendedEventHandler;
import com.google.devtools.build.lib.packages.AspectDescriptor;
import com.google.devtools.build.lib.query2.engine.QueryEnvironment.TargetAccessor;
+import com.google.devtools.build.lib.skyframe.RuleConfiguredTargetValue;
import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
import com.google.devtools.build.lib.util.CommandDescriptionForm;
import com.google.devtools.build.lib.util.CommandFailureUtils;
@@ -79,15 +79,19 @@
options.includeCommandline |= options.includeParamFiles;
for (ConfiguredTargetValue configuredTargetValue : partialResult) {
- for (ActionAnalysisMetadata action : configuredTargetValue.getActions()) {
+ if (!(configuredTargetValue instanceof RuleConfiguredTargetValue)) {
+ // We have to include non-rule values in the graph to visit their dependencies, but they
+ // don't have any actions to print out.
+ continue;
+ }
+ for (ActionAnalysisMetadata action :
+ ((RuleConfiguredTargetValue) configuredTargetValue).getActions()) {
writeAction(action, printStream);
}
if (options.useAspects) {
- if (configuredTargetValue.getConfiguredTarget() instanceof RuleConfiguredTarget) {
- for (AspectValue aspectValue : accessor.getAspectValues(configuredTargetValue)) {
- for (ActionAnalysisMetadata action : aspectValue.getActions()) {
- writeAction(action, printStream);
- }
+ for (AspectValue aspectValue : accessor.getAspectValues(configuredTargetValue)) {
+ for (ActionAnalysisMetadata action : aspectValue.getActions()) {
+ writeAction(action, printStream);
}
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactConflictFinder.java b/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactConflictFinder.java
index 8eb9a8f..611eae5 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactConflictFinder.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactConflictFinder.java
@@ -23,19 +23,15 @@
import com.google.devtools.build.lib.actions.ActionKeyContext;
import com.google.devtools.build.lib.actions.ActionLookupValue;
import com.google.devtools.build.lib.actions.Actions;
-import com.google.devtools.build.lib.actions.AnalysisGraphStatsEvent;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.ArtifactPrefixConflictException;
import com.google.devtools.build.lib.actions.MapBasedActionGraph;
import com.google.devtools.build.lib.actions.MutableActionGraph;
import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
-import com.google.devtools.build.lib.actions.TotalAndConfiguredTargetOnlyMetric;
-import com.google.devtools.build.lib.analysis.ConfiguredTargetValue;
import com.google.devtools.build.lib.concurrent.ExecutorUtil;
import com.google.devtools.build.lib.concurrent.Sharder;
import com.google.devtools.build.lib.concurrent.ThrowableRecordingRunnableWrapper;
import com.google.devtools.build.lib.skyframe.PrecomputedValue.Precomputed;
-import com.google.devtools.build.lib.util.Pair;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.util.List;
import java.util.Map;
@@ -50,6 +46,8 @@
class ArtifactConflictFinder {
static final Precomputed<ImmutableMap<ActionAnalysisMetadata, ConflictException>>
ACTION_CONFLICTS = new Precomputed<>("action_conflicts");
+ // Action graph construction is CPU-bound.
+ static final int NUM_JOBS = Runtime.getRuntime().availableProcessors();
private ArtifactConflictFinder() {}
@@ -65,7 +63,7 @@
* build as of 2014), so it should only be called when necessary.
*/
static ActionConflictsAndStats findAndStoreArtifactConflicts(
- Iterable<ActionLookupValue> actionLookupValues,
+ Sharder<ActionLookupValue> actionLookupValues,
boolean strictConflictChecks,
ActionKeyContext actionKeyContext)
throws InterruptedException {
@@ -74,9 +72,8 @@
MapBasedActionGraph actionGraph = new MapBasedActionGraph(actionKeyContext);
ConcurrentNavigableMap<PathFragment, Artifact> artifactPathMap =
new ConcurrentSkipListMap<>(Actions.comparatorForPrefixConflicts());
- Pair<TotalAndConfiguredTargetOnlyMetric, TotalAndConfiguredTargetOnlyMetric> counts =
- constructActionGraphAndPathMap(
- actionGraph, artifactPathMap, actionLookupValues, temporaryBadActionMap);
+ constructActionGraphAndPathMap(
+ actionGraph, artifactPathMap, actionLookupValues, temporaryBadActionMap);
Map<ActionAnalysisMetadata, ArtifactPrefixConflictException> actionsWithArtifactPrefixConflict =
Actions.findArtifactPrefixConflicts(actionGraph, artifactPathMap, strictConflictChecks);
@@ -87,8 +84,6 @@
}
return ActionConflictsAndStats.create(
ImmutableMap.copyOf(temporaryBadActionMap),
- counts.first,
- counts.second,
actionGraph.getSize());
}
@@ -97,53 +92,33 @@
* PathFragment}s to their respective {@link Artifact}s. We do this in a threadpool to save around
* 1.5 seconds on a mid-sized build versus a single-threaded operation.
*/
- private static Pair<TotalAndConfiguredTargetOnlyMetric, TotalAndConfiguredTargetOnlyMetric>
- constructActionGraphAndPathMap(
- MutableActionGraph actionGraph,
- ConcurrentNavigableMap<PathFragment, Artifact> artifactPathMap,
- Iterable<ActionLookupValue> values,
- ConcurrentMap<ActionAnalysisMetadata, ConflictException> badActionMap)
- throws InterruptedException {
- // Action graph construction is CPU-bound.
- int numJobs = Runtime.getRuntime().availableProcessors();
- // No great reason for expecting 5000 action lookup values, but not worth counting size of
- // values.
- Sharder<ActionLookupValue> actionShards = new Sharder<>(numJobs, 5000);
- int actionLookupValueCount = 0;
- int configuredTargetValueCount = 0;
- int actionCount = 0;
- int configuredTargetActionCount = 0;
- for (ActionLookupValue value : values) {
- actionShards.add(value);
- actionLookupValueCount++;
- actionCount += value.getNumActions();
- if (value instanceof ConfiguredTargetValue) {
- configuredTargetValueCount++;
- configuredTargetActionCount += value.getNumActions();
- }
- }
-
+ private static void constructActionGraphAndPathMap(
+ MutableActionGraph actionGraph,
+ ConcurrentNavigableMap<PathFragment, Artifact> artifactPathMap,
+ Sharder<ActionLookupValue> actionShards,
+ ConcurrentMap<ActionAnalysisMetadata, ConflictException> badActionMap)
+ throws InterruptedException {
ThrowableRecordingRunnableWrapper wrapper =
new ThrowableRecordingRunnableWrapper(
"ArtifactConflictFinder#constructActionGraphAndPathMap");
ExecutorService executor =
Executors.newFixedThreadPool(
- numJobs,
+ NUM_JOBS,
new ThreadFactoryBuilder().setNameFormat("ActionLookupValue Processor %d").build());
for (List<ActionLookupValue> shard : actionShards) {
executor.execute(
wrapper.wrap(actionRegistration(shard, actionGraph, artifactPathMap, badActionMap)));
}
boolean interrupted = ExecutorUtil.interruptibleShutdown(executor);
- Throwables.propagateIfPossible(wrapper.getFirstThrownError());
+ Throwable firstThrownError = wrapper.getFirstThrownError();
+ if (firstThrownError != null) {
+ Throwables.throwIfUnchecked(firstThrownError);
+ throw new IllegalStateException(firstThrownError);
+ }
if (interrupted) {
throw new InterruptedException();
}
- return Pair.of(
- TotalAndConfiguredTargetOnlyMetric.create(
- actionLookupValueCount, configuredTargetValueCount),
- TotalAndConfiguredTargetOnlyMetric.create(actionCount, configuredTargetActionCount));
}
private static Runnable actionRegistration(
@@ -215,15 +190,12 @@
abstract static class ActionConflictsAndStats {
abstract ImmutableMap<ActionAnalysisMetadata, ConflictException> getConflicts();
- abstract AnalysisGraphStatsEvent getStats();
+ abstract int getOutputArtifactCount();
private static ActionConflictsAndStats create(
ImmutableMap<ActionAnalysisMetadata, ConflictException> conflicts,
- TotalAndConfiguredTargetOnlyMetric actionValueCount,
- TotalAndConfiguredTargetOnlyMetric actionCount,
int artifactCount) {
- return new AutoValue_ArtifactConflictFinder_ActionConflictsAndStats(
- conflicts, new AnalysisGraphStatsEvent(actionValueCount, actionCount, artifactCount));
+ return new AutoValue_ArtifactConflictFinder_ActionConflictsAndStats(conflicts, artifactCount);
}
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
index 2f7a38d..6ff4409 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
@@ -219,6 +219,7 @@
"//src/main/java/com/google/devtools/build/lib:keep-going-option",
"//src/main/java/com/google/devtools/build/lib/actions",
"//src/main/java/com/google/devtools/build/lib/actions:action_lookup_key",
+ "//src/main/java/com/google/devtools/build/lib/actions:analysis_graph_stats_event",
"//src/main/java/com/google/devtools/build/lib/actions:artifacts",
"//src/main/java/com/google/devtools/build/lib/actions:commandline_item",
"//src/main/java/com/google/devtools/build/lib/actions:file_metadata",
@@ -255,6 +256,7 @@
"//src/main/java/com/google/devtools/build/lib/analysis:platform_configuration",
"//src/main/java/com/google/devtools/build/lib/analysis:platform_options",
"//src/main/java/com/google/devtools/build/lib/analysis:resolved_toolchain_context",
+ "//src/main/java/com/google/devtools/build/lib/analysis:rule_configured_object_value",
"//src/main/java/com/google/devtools/build/lib/analysis:toolchain_collection",
"//src/main/java/com/google/devtools/build/lib/analysis:toolchain_context",
"//src/main/java/com/google/devtools/build/lib/analysis:top_level_artifact_context",
@@ -760,11 +762,8 @@
deps = [
":precomputed_value",
"//src/main/java/com/google/devtools/build/lib/actions",
- "//src/main/java/com/google/devtools/build/lib/actions:analysis_graph_stats_event",
"//src/main/java/com/google/devtools/build/lib/actions:artifacts",
- "//src/main/java/com/google/devtools/build/lib/analysis:configured_target_value",
"//src/main/java/com/google/devtools/build/lib/concurrent",
- "//src/main/java/com/google/devtools/build/lib/util",
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
"//third_party:auto_value",
"//third_party:guava",
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 e33b6a9..950693a 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
@@ -22,8 +22,6 @@
import com.google.common.collect.Iterables;
import com.google.common.collect.Maps;
import com.google.common.flogger.GoogleLogger;
-import com.google.devtools.build.lib.actions.Actions;
-import com.google.devtools.build.lib.actions.Actions.GeneratingActions;
import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
import com.google.devtools.build.lib.analysis.AnalysisRootCauseEvent;
import com.google.devtools.build.lib.analysis.AspectResolver;
@@ -242,7 +240,6 @@
// not prepared for it.
return new NonRuleConfiguredTargetValue(
new EmptyConfiguredTarget(target.getLabel(), configuredTargetKey.getConfigurationKey()),
- GeneratingActions.EMPTY,
transitivePackagesForPackageRootResolution == null
? null
: transitivePackagesForPackageRootResolution.build());
@@ -1059,22 +1056,14 @@
? null
: transitivePackagesForPackageRootResolution.build());
} else {
- GeneratingActions generatingActions;
- // Check for conflicting actions within this configured target (that indicates a bug in the
- // rule implementation).
- try {
- generatingActions =
- Actions.assignOwnersAndFilterSharedActionsAndThrowActionConflict(
- analysisEnvironment.getActionKeyContext(),
- analysisEnvironment.getRegisteredActions(),
- configuredTargetKey,
- /*outputFiles=*/ null);
- } catch (ActionConflictException e) {
- throw new ConfiguredTargetFunctionException(e);
- }
+ Preconditions.checkState(
+ analysisEnvironment.getRegisteredActions().isEmpty(),
+ "Non-rule can't have actions: %s %s %s %s",
+ configuredTargetKey,
+ analysisEnvironment.getRegisteredActions(),
+ configuredTarget);
return new NonRuleConfiguredTargetValue(
configuredTarget,
- generatingActions,
transitivePackagesForPackageRootResolution == null
? null
: transitivePackagesForPackageRootResolution.build());
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/NonRuleConfiguredTargetValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/NonRuleConfiguredTargetValue.java
index 3826ef9..e1c96d2 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/NonRuleConfiguredTargetValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/NonRuleConfiguredTargetValue.java
@@ -13,13 +13,8 @@
// limitations under the License.
package com.google.devtools.build.lib.skyframe;
-import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.MoreObjects;
import com.google.common.base.Preconditions;
-import com.google.common.collect.ImmutableList;
-import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
-import com.google.devtools.build.lib.actions.Actions.GeneratingActions;
-import com.google.devtools.build.lib.actions.Artifact.SourceArtifact;
-import com.google.devtools.build.lib.actions.BasicActionLookupValue;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.ConfiguredTargetValue;
import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget;
@@ -36,10 +31,7 @@
@ThreadSafe
// Reached via OutputFileConfiguredTarget.
@AutoCodec(explicitlyAllowClass = RuleConfiguredTarget.class)
-@VisibleForTesting
-public final class NonRuleConfiguredTargetValue extends BasicActionLookupValue
- implements ConfiguredTargetValue {
-
+public final class NonRuleConfiguredTargetValue implements ConfiguredTargetValue {
// These variables are only non-final because they may be clear()ed to save memory.
// configuredTarget is null only after it is cleared.
@Nullable private ConfiguredTarget configuredTarget;
@@ -50,20 +42,15 @@
@AutoCodec.Instantiator
@VisibleForSerialization
NonRuleConfiguredTargetValue(
- ImmutableList<ActionAnalysisMetadata> actions,
ConfiguredTarget configuredTarget) {
- super(actions);
- this.configuredTarget = configuredTarget;
// Transitive packages are not serialized.
- this.transitivePackagesForPackageRootResolution = null;
+ this(configuredTarget, null);
}
NonRuleConfiguredTargetValue(
ConfiguredTarget configuredTarget,
- GeneratingActions generatingActions,
@Nullable NestedSet<Package> transitivePackagesForPackageRootResolution) {
- super(generatingActions);
- this.configuredTarget = Preconditions.checkNotNull(configuredTarget, generatingActions);
+ this.configuredTarget = Preconditions.checkNotNull(configuredTarget);
this.transitivePackagesForPackageRootResolution = transitivePackagesForPackageRootResolution;
}
@@ -74,12 +61,6 @@
}
@Override
- public ImmutableList<ActionAnalysisMetadata> getActions() {
- Preconditions.checkNotNull(configuredTarget, this);
- return actions;
- }
-
- @Override
public NestedSet<Package> getTransitivePackagesForPackageRootResolution() {
return Preconditions.checkNotNull(transitivePackagesForPackageRootResolution);
}
@@ -95,11 +76,6 @@
@Override
public String toString() {
- return getStringHelper().add("configuredTarget", configuredTarget).toString();
- }
-
- @Override
- public SourceArtifact getSourceArtifact() {
- return configuredTarget == null ? null : configuredTarget.getSourceArtifact();
+ return MoreObjects.toStringHelper(this).add("configuredTarget", configuredTarget).toString();
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RuleConfiguredTargetValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/RuleConfiguredTargetValue.java
index e94b49f..a1ca50c 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/RuleConfiguredTargetValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/RuleConfiguredTargetValue.java
@@ -17,9 +17,9 @@
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
-import com.google.devtools.build.lib.actions.ActionLookupValue;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.ConfiguredTargetValue;
+import com.google.devtools.build.lib.analysis.RuleConfiguredObjectValue;
import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
@@ -32,7 +32,8 @@
@Immutable
@ThreadSafe
@AutoCodec(explicitlyAllowClass = RuleConfiguredTarget.class)
-public final class RuleConfiguredTargetValue implements ActionLookupValue, ConfiguredTargetValue {
+public final class RuleConfiguredTargetValue
+ implements RuleConfiguredObjectValue, ConfiguredTargetValue {
// This variable is non-final because it may be clear()ed to save memory. It is null only after
// clear(true) is called.
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java
index 8e29c08..9e9638b 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java
@@ -762,8 +762,8 @@
SkyValue value = skyKeyAndValue.getValue();
SkyKey key = skyKeyAndValue.getKey();
SkyFunctionName functionName = key.functionName();
- if (functionName.equals(SkyFunctions.CONFIGURED_TARGET)) {
- ConfiguredTargetValue ctValue = (ConfiguredTargetValue) value;
+ if (value instanceof RuleConfiguredTargetValue) {
+ RuleConfiguredTargetValue ctValue = (RuleConfiguredTargetValue) value;
ConfiguredTarget configuredTarget = ctValue.getConfiguredTarget();
if (configuredTarget instanceof RuleConfiguredTarget) {
@@ -808,20 +808,20 @@
memoizingEvaluator.getDoneValues().entrySet()) {
SkyKey key = skyKeyAndValue.getKey();
SkyValue skyValue = skyKeyAndValue.getValue();
- SkyFunctionName functionName = key.functionName();
- try {
+ if (skyValue == null) {
// The skyValue may be null in case analysis of the previous build failed.
- if (skyValue != null) {
- if (functionName.equals(SkyFunctions.CONFIGURED_TARGET)) {
- actionGraphDump.dumpConfiguredTarget((ConfiguredTargetValue) skyValue);
- } else if (functionName.equals(SkyFunctions.ASPECT)) {
- AspectValue aspectValue = (AspectValue) skyValue;
- AspectKey aspectKey = (AspectKey) key;
- ConfiguredTargetValue configuredTargetValue =
- (ConfiguredTargetValue)
- memoizingEvaluator.getExistingValue(aspectKey.getBaseConfiguredTargetKey());
- actionGraphDump.dumpAspect(aspectValue, configuredTargetValue);
- }
+ continue;
+ }
+ try {
+ if (skyValue instanceof RuleConfiguredTargetValue) {
+ actionGraphDump.dumpConfiguredTarget((RuleConfiguredTargetValue) skyValue);
+ } else if (key.functionName().equals(SkyFunctions.ASPECT)) {
+ AspectValue aspectValue = (AspectValue) skyValue;
+ AspectKey aspectKey = (AspectKey) key;
+ ConfiguredTargetValue configuredTargetValue =
+ (ConfiguredTargetValue)
+ memoizingEvaluator.getExistingValue(aspectKey.getBaseConfiguredTargetKey());
+ actionGraphDump.dumpAspect(aspectValue, configuredTargetValue);
}
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
@@ -840,20 +840,20 @@
memoizingEvaluator.getDoneValues().entrySet()) {
SkyKey key = skyKeyAndValue.getKey();
SkyValue skyValue = skyKeyAndValue.getValue();
- SkyFunctionName functionName = key.functionName();
- try {
+ if (skyValue == null) {
// The skyValue may be null in case analysis of the previous build failed.
- if (skyValue != null) {
- if (functionName.equals(SkyFunctions.CONFIGURED_TARGET)) {
- actionGraphDump.dumpConfiguredTarget((ConfiguredTargetValue) skyValue);
- } else if (functionName.equals(SkyFunctions.ASPECT)) {
- AspectValue aspectValue = (AspectValue) skyValue;
- AspectKey aspectKey = (AspectKey) key;
- ConfiguredTargetValue configuredTargetValue =
- (ConfiguredTargetValue)
- memoizingEvaluator.getExistingValue(aspectKey.getBaseConfiguredTargetKey());
- actionGraphDump.dumpAspect(aspectValue, configuredTargetValue);
- }
+ continue;
+ }
+ try {
+ if (skyValue instanceof RuleConfiguredTargetValue) {
+ actionGraphDump.dumpConfiguredTarget((RuleConfiguredTargetValue) skyValue);
+ } else if (key.functionName().equals(SkyFunctions.ASPECT)) {
+ AspectValue aspectValue = (AspectValue) skyValue;
+ AspectKey aspectKey = (AspectKey) key;
+ ConfiguredTargetValue configuredTargetValue =
+ (ConfiguredTargetValue)
+ memoizingEvaluator.getExistingValue(aspectKey.getBaseConfiguredTargetKey());
+ actionGraphDump.dumpAspect(aspectValue, configuredTargetValue);
}
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
index 1a90a8a..2cce56f 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
@@ -32,6 +32,7 @@
import com.google.devtools.build.lib.actions.ActionKeyContext;
import com.google.devtools.build.lib.actions.ActionLookupKey;
import com.google.devtools.build.lib.actions.ActionLookupValue;
+import com.google.devtools.build.lib.actions.AnalysisGraphStatsEvent;
import com.google.devtools.build.lib.actions.ArtifactFactory;
import com.google.devtools.build.lib.actions.ArtifactPrefixConflictException;
import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
@@ -59,6 +60,7 @@
import com.google.devtools.build.lib.analysis.config.CoreOptions;
import com.google.devtools.build.lib.analysis.config.FragmentClassSet;
import com.google.devtools.build.lib.bugreport.BugReport;
+import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos;
import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos.BuildEventId.ConfigurationId;
import com.google.devtools.build.lib.buildtool.BuildRequestOptions;
import com.google.devtools.build.lib.causes.AnalysisFailedCause;
@@ -142,7 +144,7 @@
private BuildConfiguration topLevelHostConfiguration;
// Fragment-limited versions of the host configuration. It's faster to create/cache these here
// than to store them in Skyframe.
- private Map<BuildConfiguration, BuildConfiguration> hostConfigurationCache =
+ private final Map<BuildConfiguration, BuildConfiguration> hostConfigurationCache =
Maps.newConcurrentMap();
private BuildConfigurationCollection configurations;
@@ -174,8 +176,7 @@
public TotalAndConfiguredTargetOnlyMetric getEvaluatedCounts() {
return TotalAndConfiguredTargetOnlyMetric.create(
- progressReceiver.actionLookupValueCount.get(),
- progressReceiver.configuredTargetCount.get());
+ progressReceiver.configuredObjectCount.get(), progressReceiver.configuredTargetCount.get());
}
ConfiguredTargetFactory getConfiguredTargetFactory() {
@@ -448,12 +449,17 @@
// This operation is somewhat expensive, so we only do it if the graph might have changed in
// some way -- either we analyzed a new target or we invalidated an old one or are building
// targets together that haven't been built before.
+ SkyframeExecutor.AnalysisTraversalResult analysisTraversalResult =
+ skyframeExecutor.getActionLookupValuesInBuild(ctKeys, aspectKeys);
ArtifactConflictFinder.ActionConflictsAndStats conflictsAndStats =
ArtifactConflictFinder.findAndStoreArtifactConflicts(
- skyframeExecutor.getActionLookupValuesInBuild(ctKeys, aspectKeys),
- strictConflictChecks,
- actionKeyContext);
- eventBus.post(conflictsAndStats.getStats());
+ analysisTraversalResult.getActionShards(), strictConflictChecks, actionKeyContext);
+ BuildEventStreamProtos.BuildMetrics.BuildGraphMetrics buildGraphMetrics =
+ analysisTraversalResult
+ .getMetrics()
+ .setOutputArtifactCount(conflictsAndStats.getOutputArtifactCount())
+ .build();
+ eventBus.post(new AnalysisGraphStatsEvent(buildGraphMetrics));
actionConflicts = conflictsAndStats.getConflicts();
someActionLookupValueEvaluated = false;
}
@@ -1135,7 +1141,7 @@
}
private final class ActionLookupValueProgressReceiver implements EvaluationProgressReceiver {
- private final AtomicInteger actionLookupValueCount = new AtomicInteger();
+ private final AtomicInteger configuredObjectCount = new AtomicInteger();
private final AtomicInteger actionCount = new AtomicInteger();
private final AtomicInteger configuredTargetCount = new AtomicInteger();
private final AtomicInteger configuredTargetActionCount = new AtomicInteger();
@@ -1165,16 +1171,17 @@
}
switch (state) {
case BUILT:
+ if (!evaluationSuccessState.get().succeeded()) {
+ return;
+ }
+ configuredObjectCount.incrementAndGet();
boolean isConfiguredTarget = skyKey.functionName().equals(SkyFunctions.CONFIGURED_TARGET);
- if (evaluationSuccessState.get().succeeded()) {
- actionLookupValueCount.incrementAndGet();
- if (isConfiguredTarget) {
- configuredTargetCount.incrementAndGet();
- }
- // During multithreaded operation, this is only set to true, so no concurrency issues.
- someActionLookupValueEvaluated = true;
+ if (isConfiguredTarget) {
+ configuredTargetCount.incrementAndGet();
}
if (newValue instanceof ActionLookupValue) {
+ // During multithreaded operation, this is only set to true, so no concurrency issues.
+ someActionLookupValueEvaluated = true;
int numActions = ((ActionLookupValue) newValue).getNumActions();
actionCount.addAndGet(numActions);
if (isConfiguredTarget) {
@@ -1190,7 +1197,7 @@
}
public void reset() {
- actionLookupValueCount.set(0);
+ configuredObjectCount.set(0);
actionCount.set(0);
configuredTargetCount.set(0);
configuredTargetActionCount.set(0);
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
index 0965be7..74c0d5b 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
@@ -15,6 +15,7 @@
import static com.google.devtools.build.lib.concurrent.Uninterruptibles.callUninterruptibly;
import static com.google.devtools.build.lib.skyframe.ArtifactConflictFinder.ACTION_CONFLICTS;
+import static com.google.devtools.build.lib.skyframe.ArtifactConflictFinder.NUM_JOBS;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
@@ -97,10 +98,14 @@
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.configuredtargets.InputFileConfiguredTarget;
import com.google.devtools.build.lib.analysis.configuredtargets.MergedConfiguredTarget;
+import com.google.devtools.build.lib.analysis.configuredtargets.OutputFileConfiguredTarget;
import com.google.devtools.build.lib.analysis.starlark.StarlarkTransition;
import com.google.devtools.build.lib.analysis.starlark.StarlarkTransition.TransitionException;
+import com.google.devtools.build.lib.bugreport.BugReport;
import com.google.devtools.build.lib.bugreport.BugReporter;
+import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos;
import com.google.devtools.build.lib.buildtool.BuildRequestOptions;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
@@ -108,6 +113,7 @@
import com.google.devtools.build.lib.collect.compacthashset.CompactHashSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSetExpander;
import com.google.devtools.build.lib.concurrent.NamedForkJoinPool;
+import com.google.devtools.build.lib.concurrent.Sharder;
import com.google.devtools.build.lib.concurrent.ThreadSafety;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadCompatible;
import com.google.devtools.build.lib.events.ErrorSensingEventHandler;
@@ -1090,7 +1096,17 @@
}
// ctValue may be null if target was not successfully analyzed.
if (ctValue != null) {
- ctValue.clear(!topLevelTargets.contains(ctValue.getConfiguredTarget()));
+ if (!(ctValue instanceof ActionLookupValue)
+ && discardType.discardsLoading()
+ && !topLevelTargets.contains(ctValue.getConfiguredTarget())) {
+ // If loading is already being deleted, deleting these nodes doesn't hurt. Morally
+ // we should always be able to delete these, since they're not used for execution,
+ // but it leaves the graph inconsistent, and the --discard_analysis_cache with
+ // --track_incremental_state case isn't worth optimizing for.
+ it.remove();
+ } else {
+ ctValue.clear(!topLevelTargets.contains(ctValue.getConfiguredTarget()));
+ }
}
} else if (functionName.equals(SkyFunctions.ASPECT)) {
AspectValue aspectValue;
@@ -3142,18 +3158,24 @@
return ExecutionFinishedEvent.builderWithDefaults();
}
- final Iterable<ActionLookupValue> getActionLookupValuesInBuild(
+ final AnalysisTraversalResult getActionLookupValuesInBuild(
List<ConfiguredTargetKey> topLevelCtKeys, List<AspectValueKey> aspectKeys)
throws InterruptedException {
+ AnalysisTraversalResult result = new AnalysisTraversalResult();
if (!isAnalysisIncremental()) {
- // If we do not track incremental state we do not have graph edges, so we cannot traverse the
- // graph and find only actions in the current build. In this case we can simply return all
- // ActionLookupValues in the graph, since the graph's lifetime is a single build anyway.
- return Iterables.filter(memoizingEvaluator.getDoneValues().values(), ActionLookupValue.class);
+ // If we do not have incremental analysis state we do not have graph edges, so we cannot
+ // traverse the graph and find only actions in the current build. In this case we can simply
+ // return all ActionLookupValues in the graph, since the graph's lifetime is a single build
+ // anyway.
+ for (Map.Entry<SkyKey, SkyValue> entry : memoizingEvaluator.getDoneValues().entrySet()) {
+ if ((entry.getKey() instanceof ActionLookupKey) && entry.getValue() != null) {
+ result.accumulate(entry.getValue(), (ActionLookupKey) entry.getKey());
+ }
+ }
+ return result;
}
WalkableGraph walkableGraph = SkyframeExecutorWrappingWalkableGraph.of(this);
Set<SkyKey> seen = CompactHashSet.create();
- List<ActionLookupValue> result = new ArrayList<>();
for (ConfiguredTargetKey key : topLevelCtKeys) {
findActionsRecursively(walkableGraph, key, seen, result);
}
@@ -3163,22 +3185,95 @@
return result;
}
+ static class AnalysisTraversalResult {
+ // Some metrics indicate this is a rough average # of ALVs in a build.
+ private final Sharder<ActionLookupValue> actionShards = new Sharder<>(NUM_JOBS, 200_000);
+
+ // Metrics.
+ private int configuredObjectCount = 0;
+ private int configuredTargetCount = 0;
+ private int actionCount = 0;
+ private int actionCountNotIncludingAspects = 0;
+ private int inputFileConfiguredTargetCount = 0;
+ private int outputFileConfiguredTargetCount = 0;
+ private int otherConfiguredTargetCount = 0;
+
+ private AnalysisTraversalResult() {}
+
+ private void accumulate(SkyValue value, ActionLookupKey keyForDebugging) {
+ boolean isConfiguredTarget = value instanceof ConfiguredTargetValue;
+ boolean isActionLookupValue = value instanceof ActionLookupValue;
+ if (!isConfiguredTarget && !isActionLookupValue) {
+ BugReport.sendBugReport(
+ new IllegalStateException(
+ String.format(
+ "Should only be called with ConfiguredTargetValue or ActionLookupValue: %s %s"
+ + " %s",
+ value.getClass(), keyForDebugging, value)));
+ return;
+ }
+ configuredObjectCount++;
+ if (isConfiguredTarget) {
+ configuredTargetCount++;
+ }
+ if (isActionLookupValue) {
+ ActionLookupValue alv = (ActionLookupValue) value;
+ int numActions = alv.getNumActions();
+ actionCount += numActions;
+ if (isConfiguredTarget) {
+ actionCountNotIncludingAspects += numActions;
+ }
+ actionShards.add(alv);
+ return;
+ }
+ if (!(value instanceof NonRuleConfiguredTargetValue)) {
+ BugReport.sendBugReport(
+ new IllegalStateException(
+ String.format(
+ "Unexpected value type: %s %s %s", value.getClass(), keyForDebugging, value)));
+ return;
+ }
+ ConfiguredTarget configuredTarget =
+ ((NonRuleConfiguredTargetValue) value).getConfiguredTarget();
+ if (configuredTarget instanceof InputFileConfiguredTarget) {
+ inputFileConfiguredTargetCount++;
+ } else if (configuredTarget instanceof OutputFileConfiguredTarget) {
+ outputFileConfiguredTargetCount++;
+ } else {
+ otherConfiguredTargetCount++;
+ }
+ }
+
+ Sharder<ActionLookupValue> getActionShards() {
+ return actionShards;
+ }
+
+ BuildEventStreamProtos.BuildMetrics.BuildGraphMetrics.Builder getMetrics() {
+ return BuildEventStreamProtos.BuildMetrics.BuildGraphMetrics.newBuilder()
+ .setActionLookupValueCount(configuredObjectCount)
+ .setActionLookupValueCountNotIncludingAspects(configuredTargetCount)
+ .setActionCount(actionCount)
+ .setActionCountNotIncludingAspects(actionCountNotIncludingAspects)
+ .setInputFileConfiguredTargetCount(inputFileConfiguredTargetCount)
+ .setOutputFileConfiguredTargetCount(outputFileConfiguredTargetCount)
+ .setOtherConfiguredTargetCount(otherConfiguredTargetCount);
+ }
+ }
+
private static void findActionsRecursively(
- WalkableGraph walkableGraph, SkyKey key, Set<SkyKey> seen, List<ActionLookupValue> result)
+ WalkableGraph walkableGraph, SkyKey key, Set<SkyKey> seen, AnalysisTraversalResult result)
throws InterruptedException {
if (!(key instanceof ActionLookupKey) || !seen.add(key)) {
- // The subgraph of dependencies of ActionLookupValues never has a non-ActionLookupValue
- // depending on an ActionLookupValue. So we can skip any non-ActionLookupValues in the
- // traversal as an optimization.
+ // The subgraph of dependencies of ActionLookupKeys never has a non-ActionLookupKey depending
+ // on an ActionLookupKey. So we can skip any non-ActionLookupKeys in the traversal as an
+ // optimization.
return;
}
SkyValue value = walkableGraph.getValue(key);
if (value == null) {
return; // The value failed to evaluate.
}
- if (value instanceof ActionLookupValue) {
- result.add((ActionLookupValue) value);
- }
+ result.accumulate(value, (ActionLookupKey) key);
for (SkyKey dep : walkableGraph.getDirectDeps(key)) {
findActionsRecursively(walkableGraph, dep, seen, result);
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/ActionGraphDump.java b/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/ActionGraphDump.java
index 05b46ba..3ec30db 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/ActionGraphDump.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/ActionGraphDump.java
@@ -36,6 +36,7 @@
import com.google.devtools.build.lib.packages.AspectDescriptor;
import com.google.devtools.build.lib.query2.aquery.AqueryActionFilter;
import com.google.devtools.build.lib.query2.aquery.AqueryUtils;
+import com.google.devtools.build.lib.skyframe.RuleConfiguredTargetValue;
import com.google.devtools.build.lib.util.Pair;
import java.util.HashMap;
import java.util.List;
@@ -234,7 +235,7 @@
}
}
- public void dumpConfiguredTarget(ConfiguredTargetValue configuredTargetValue)
+ public void dumpConfiguredTarget(RuleConfiguredTargetValue configuredTargetValue)
throws CommandLineExpansionException, InterruptedException {
ConfiguredTarget configuredTarget = configuredTargetValue.getConfiguredTarget();
if (!includeInActionGraph(configuredTarget.getLabel().toString())) {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/ActionGraphDump.java b/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/ActionGraphDump.java
index f43bead..7dd45a1 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/ActionGraphDump.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/ActionGraphDump.java
@@ -36,6 +36,7 @@
import com.google.devtools.build.lib.packages.AspectDescriptor;
import com.google.devtools.build.lib.query2.aquery.AqueryActionFilter;
import com.google.devtools.build.lib.query2.aquery.AqueryUtils;
+import com.google.devtools.build.lib.skyframe.RuleConfiguredTargetValue;
import com.google.devtools.build.lib.util.Pair;
import java.io.IOException;
import java.util.HashMap;
@@ -49,11 +50,8 @@
* proto format.
*/
public class ActionGraphDump {
-
private final ActionKeyContext actionKeyContext = new ActionKeyContext();
private final Set<String> actionGraphTargets;
-
- private final KnownRuleClassStrings knownRuleClassStrings;
private final KnownArtifacts knownArtifacts;
private final KnownConfigurations knownConfigurations;
private final KnownNestedSets knownNestedSets;
@@ -96,7 +94,7 @@
this.includeParamFiles = includeParamFiles;
this.aqueryOutputHandler = aqueryOutputHandler;
- knownRuleClassStrings = new KnownRuleClassStrings(aqueryOutputHandler);
+ KnownRuleClassStrings knownRuleClassStrings = new KnownRuleClassStrings(aqueryOutputHandler);
knownArtifacts = new KnownArtifacts(aqueryOutputHandler);
knownConfigurations = new KnownConfigurations(aqueryOutputHandler);
knownNestedSets = new KnownNestedSets(aqueryOutputHandler, knownArtifacts);
@@ -245,7 +243,7 @@
}
}
- public void dumpConfiguredTarget(ConfiguredTargetValue configuredTargetValue)
+ public void dumpConfiguredTarget(RuleConfiguredTargetValue configuredTargetValue)
throws CommandLineExpansionException, InterruptedException, IOException {
ConfiguredTarget configuredTarget = configuredTargetValue.getConfiguredTarget();
if (!includeInActionGraph(configuredTarget.getLabel().toString())) {
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/AnalysisCachingTest.java b/src/test/java/com/google/devtools/build/lib/analysis/AnalysisCachingTest.java
index a5b6778..c683731 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/AnalysisCachingTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/AnalysisCachingTest.java
@@ -313,6 +313,27 @@
assertThat(successfulAnalyses).isEqualTo(1);
}
+ @Test
+ public void aliasConflict() throws Exception {
+ scratch.file(
+ "conflict/conflict.bzl",
+ "def _conflict(ctx):",
+ " file = ctx.actions.declare_file('single_file')",
+ " ctx.actions.write(output = file, content = ctx.attr.name)",
+ " return [DefaultInfo(files = depset([file]))]",
+ "my_rule = rule(implementation = _conflict)");
+ scratch.file(
+ "conflict/BUILD",
+ "load(':conflict.bzl', 'my_rule')",
+ "my_rule(name = 'conflict1')",
+ "my_rule(name = 'conflict2')",
+ "alias(name = 'aliased', actual = ':conflict2')");
+ reporter.removeHandler(failFastHandler);
+ assertThrows(
+ ViewCreationFailedException.class,
+ () -> update("//conflict:conflict1", "//conflict:aliased"));
+ }
+
/** BUILD file involved in BUILD-file cycle is changed */
@Test
public void testBuildFileInCycleChanged() throws Exception {
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
index 35e0a22..9728117 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
@@ -175,6 +175,7 @@
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyFunctionName;
import com.google.devtools.build.skyframe.SkyKey;
+import com.google.devtools.build.skyframe.SkyValue;
import com.google.devtools.common.options.Options;
import com.google.devtools.common.options.OptionsParser;
import com.google.devtools.common.options.OptionsParsingException;
@@ -1346,16 +1347,14 @@
protected final Artifact.DerivedArtifact getDerivedArtifact(
PathFragment rootRelativePath, ArtifactRoot root, ArtifactOwner owner) {
if ((owner instanceof ActionLookupKey)) {
- ActionLookupValue actionLookupValue;
+ SkyValue skyValue;
try {
- actionLookupValue =
- (ActionLookupValue)
- skyframeExecutor.getEvaluatorForTesting().getExistingValue((SkyKey) owner);
+ skyValue = skyframeExecutor.getEvaluatorForTesting().getExistingValue((SkyKey) owner);
} catch (InterruptedException e) {
throw new IllegalStateException(e);
}
- if (actionLookupValue != null) {
- for (ActionAnalysisMetadata action : actionLookupValue.getActions()) {
+ if (skyValue instanceof ActionLookupValue) {
+ for (ActionAnalysisMetadata action : ((ActionLookupValue) skyValue).getActions()) {
for (Artifact output : action.getOutputs()) {
if (output.getRootRelativePath().equals(rootRelativePath)
&& output.getRoot().equals(root)) {
@@ -1366,6 +1365,7 @@
}
}
// Fall back: some tests don't actually need an artifact with an owner.
+ // TODO(janakr): the tests that are passing in nonsense here should be changed.
return view.getArtifactFactory().getDerivedArtifact(rootRelativePath, root, owner);
}
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/BUILD b/src/test/java/com/google/devtools/build/lib/rules/cpp/BUILD
index 96f0d70..b998dd6 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/BUILD
@@ -93,10 +93,10 @@
"//src/main/java/com/google/devtools/build/lib/actions:artifacts",
"//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster",
"//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/cmdline",
"//src/main/java/com/google/devtools/build/lib/rules/cpp",
"//src/main/java/com/google/devtools/build/lib/skyframe:configured_target_key",
+ "//src/main/java/com/google/devtools/build/lib/skyframe:skyframe_cluster",
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
"//src/test/java/com/google/devtools/build/lib/actions/util",
"//src/test/java/com/google/devtools/build/lib/analysis/util",
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcBinaryThinLtoTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcBinaryThinLtoTest.java
index edc6186..a21f311 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcBinaryThinLtoTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcBinaryThinLtoTest.java
@@ -25,7 +25,6 @@
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.util.ActionsTestUtil;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
-import com.google.devtools.build.lib.analysis.ConfiguredTargetValue;
import com.google.devtools.build.lib.analysis.RuleContext;
import com.google.devtools.build.lib.analysis.actions.SpawnAction;
import com.google.devtools.build.lib.analysis.util.AnalysisMock;
@@ -35,6 +34,7 @@
import com.google.devtools.build.lib.packages.util.MockCcSupport;
import com.google.devtools.build.lib.rules.cpp.CppConfiguration.Tool;
import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey;
+import com.google.devtools.build.lib.skyframe.RuleConfiguredTargetValue;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.io.IOException;
import java.util.List;
@@ -192,8 +192,8 @@
CppLinkAction indexAction = getIndexAction(backendAction);
- ConfiguredTargetValue configuredTargetValue =
- (ConfiguredTargetValue)
+ RuleConfiguredTargetValue configuredTargetValue =
+ (RuleConfiguredTargetValue)
getSkyframeExecutor()
.getEvaluatorForTesting()
.getExistingEntryAtCurrentlyEvaluatingVersion(
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunctionTest.java
index a3f9dcf..6673acd 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunctionTest.java
@@ -27,6 +27,7 @@
import com.google.devtools.build.lib.actions.ActionKeyContext;
import com.google.devtools.build.lib.actions.ActionLookupData;
import com.google.devtools.build.lib.actions.ActionLookupKey;
+import com.google.devtools.build.lib.actions.ActionLookupValue;
import com.google.devtools.build.lib.actions.ActionOwner;
import com.google.devtools.build.lib.actions.ActionTemplate;
import com.google.devtools.build.lib.actions.Actions;
@@ -39,14 +40,13 @@
import com.google.devtools.build.lib.actions.ArtifactPrefixConflictException;
import com.google.devtools.build.lib.actions.ArtifactRoot;
import com.google.devtools.build.lib.actions.ArtifactRoot.RootType;
+import com.google.devtools.build.lib.actions.BasicActionLookupValue;
import com.google.devtools.build.lib.actions.FileArtifactValue;
import com.google.devtools.build.lib.actions.MiddlemanType;
import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
import com.google.devtools.build.lib.actions.util.ActionsTestUtil;
import com.google.devtools.build.lib.actions.util.InjectedActionLookupKey;
import com.google.devtools.build.lib.actions.util.TestAction.DummyAction;
-import com.google.devtools.build.lib.analysis.ConfiguredTarget;
-import com.google.devtools.build.lib.analysis.ConfiguredTargetValue;
import com.google.devtools.build.lib.analysis.actions.CustomCommandLine;
import com.google.devtools.build.lib.analysis.actions.SpawnActionTemplate;
import com.google.devtools.build.lib.analysis.actions.SpawnActionTemplate.OutputPathMapper;
@@ -77,7 +77,6 @@
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
-import org.mockito.Mockito;
/** Tests for {@link ActionTemplateExpansionFunction}. */
@RunWith(JUnit4.class)
@@ -342,7 +341,7 @@
private static final ActionLookupKey CTKEY = new InjectedActionLookupKey("key");
private ImmutableList<Action> evaluate(ActionTemplate<?> actionTemplate) throws Exception {
- ConfiguredTargetValue ctValue = createConfiguredTargetValue(actionTemplate);
+ ActionLookupValue ctValue = createActionLookupValue(actionTemplate);
differencer.inject(CTKEY, ctValue);
ActionTemplateExpansionKey templateKey = ActionTemplateExpansionValue.key(CTKEY, 0);
@@ -365,12 +364,9 @@
return actionList.build();
}
- private static ConfiguredTargetValue createConfiguredTargetValue(
- ActionTemplate<?> actionTemplate) {
- return new NonRuleConfiguredTargetValue(
- Mockito.mock(ConfiguredTarget.class),
- Actions.GeneratingActions.fromSingleAction(actionTemplate, CTKEY),
- NestedSetBuilder.emptySet(Order.STABLE_ORDER));
+ private static ActionLookupValue createActionLookupValue(ActionTemplate<?> actionTemplate) {
+ return new BasicActionLookupValue(
+ Actions.GeneratingActions.fromSingleAction(actionTemplate, CTKEY));
}
private SpecialArtifact createTreeArtifact(String path) {
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorTest.java
index 0ad0a5e..c8b91d6 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorTest.java
@@ -47,11 +47,11 @@
import com.google.devtools.build.lib.actions.ActionKeyContext;
import com.google.devtools.build.lib.actions.ActionLookupData;
import com.google.devtools.build.lib.actions.ActionLookupKey;
+import com.google.devtools.build.lib.actions.ActionLookupValue;
import com.google.devtools.build.lib.actions.ActionOwner;
import com.google.devtools.build.lib.actions.ActionResult;
import com.google.devtools.build.lib.actions.ActionTemplate;
import com.google.devtools.build.lib.actions.Actions;
-import com.google.devtools.build.lib.actions.Actions.GeneratingActions;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact;
import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
@@ -59,6 +59,7 @@
import com.google.devtools.build.lib.actions.ArtifactResolver;
import com.google.devtools.build.lib.actions.ArtifactRoot;
import com.google.devtools.build.lib.actions.ArtifactRoot.RootType;
+import com.google.devtools.build.lib.actions.BasicActionLookupValue;
import com.google.devtools.build.lib.actions.BuildFailedException;
import com.google.devtools.build.lib.actions.FileArtifactValue;
import com.google.devtools.build.lib.actions.FileStateValue;
@@ -73,7 +74,6 @@
import com.google.devtools.build.lib.analysis.AnalysisProtos;
import com.google.devtools.build.lib.analysis.AnalysisProtos.ActionGraphContainer;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
-import com.google.devtools.build.lib.analysis.ConfiguredTargetValue;
import com.google.devtools.build.lib.analysis.OutputGroupInfo;
import com.google.devtools.build.lib.analysis.TopLevelArtifactContext;
import com.google.devtools.build.lib.analysis.TransitiveInfoProvider;
@@ -672,7 +672,7 @@
Action action1 =
new MissingOutputAction(
NestedSetBuilder.emptySet(Order.STABLE_ORDER), output1, MiddlemanType.NORMAL);
- ConfiguredTargetValue ctValue1 = createConfiguredTargetValue(action1, lc1);
+ ActionLookupValue ctValue1 = createActionLookupValue(action1, lc1);
ActionLookupKey lc2 = new InjectedActionLookupKey("lc2");
Artifact output2 =
new Artifact.DerivedArtifact(
@@ -680,7 +680,7 @@
Action action2 =
new MissingOutputAction(
NestedSetBuilder.emptySet(Order.STABLE_ORDER), output2, MiddlemanType.NORMAL);
- ConfiguredTargetValue ctValue2 = createConfiguredTargetValue(action2, lc2);
+ ActionLookupValue ctValue2 = createActionLookupValue(action2, lc2);
// Inject the "configured targets" into the graph.
skyframeExecutor
.getDifferencerForTesting()
@@ -736,7 +736,7 @@
inputKey);
Action baseAction =
new DummyAction(NestedSetBuilder.emptySet(Order.STABLE_ORDER), input, MiddlemanType.NORMAL);
- ConfiguredTargetValue ctBase = createConfiguredTargetValue(baseAction, inputKey);
+ ActionLookupValue ctBase = createActionLookupValue(baseAction, inputKey);
ActionLookupKey lc1 = new InjectedActionLookupKey("lc1");
Artifact output1 =
new Artifact.DerivedArtifact(
@@ -744,7 +744,7 @@
Action action1 =
new DummyAction(
NestedSetBuilder.create(Order.STABLE_ORDER, input), output1, MiddlemanType.NORMAL);
- ConfiguredTargetValue ctValue1 = createConfiguredTargetValue(action1, lc1);
+ ActionLookupValue ctValue1 = createActionLookupValue(action1, lc1);
ActionLookupKey lc2 = new InjectedActionLookupKey("lc2");
Artifact output2 =
new Artifact.DerivedArtifact(
@@ -752,7 +752,7 @@
Action action2 =
new DummyAction(
NestedSetBuilder.create(Order.STABLE_ORDER, input), output2, MiddlemanType.NORMAL);
- ConfiguredTargetValue ctValue2 = createConfiguredTargetValue(action2, lc2);
+ ActionLookupValue ctValue2 = createActionLookupValue(action2, lc2);
// Stall both actions during the "checking inputs" phase so that neither will enter
// SkyframeActionExecutor before both have asked SkyframeActionExecutor if another shared action
@@ -850,7 +850,7 @@
},
NestedSetBuilder.emptySet(Order.STABLE_ORDER),
ImmutableSet.of(outputA));
- ConfiguredTargetValue ctA = createConfiguredTargetValue(actionA, lcA);
+ ActionLookupValue ctA = createActionLookupValue(actionA, lcA);
// Shared actions: they look the same from the point of view of Blaze data.
ActionLookupKey lcB = new InjectedActionLookupKey("lcB");
@@ -860,7 +860,7 @@
Action actionB =
new DummyAction(
NestedSetBuilder.emptySet(Order.STABLE_ORDER), outputB, MiddlemanType.NORMAL);
- ConfiguredTargetValue ctB = createConfiguredTargetValue(actionB, lcB);
+ ActionLookupValue ctB = createActionLookupValue(actionB, lcB);
ActionLookupKey lcC = new InjectedActionLookupKey("lcC");
Artifact outputC =
new Artifact.DerivedArtifact(
@@ -868,7 +868,7 @@
Action actionC =
new DummyAction(
NestedSetBuilder.emptySet(Order.STABLE_ORDER), outputC, MiddlemanType.NORMAL);
- ConfiguredTargetValue ctC = createConfiguredTargetValue(actionC, lcC);
+ ActionLookupValue ctC = createActionLookupValue(actionC, lcC);
// Both shared actions wait for A to start executing. We do that by stalling their dep requests
// on their configured targets. We then let B proceed. Once B finishes its SkyFunction run, it
@@ -995,7 +995,7 @@
ImmutableList<PathFragment> children = ImmutableList.of(PathFragment.create("child"));
Action action1 =
new TreeArtifactAction(NestedSetBuilder.emptySet(Order.STABLE_ORDER), output1, children);
- ConfiguredTargetValue ctValue1 = createConfiguredTargetValue(action1, lc1);
+ ActionLookupValue ctValue1 = createActionLookupValue(action1, lc1);
ActionLookupKey lc2 = new InjectedActionLookupKey("lc2");
SpecialArtifact output2 =
new SpecialArtifact(
@@ -1005,7 +1005,7 @@
Artifact.SpecialArtifactType.TREE);
Action action2 =
new TreeArtifactAction(NestedSetBuilder.emptySet(Order.STABLE_ORDER), output2, children);
- ConfiguredTargetValue ctValue2 = createConfiguredTargetValue(action2, lc2);
+ ActionLookupValue ctValue2 = createActionLookupValue(action2, lc2);
// Inject the "configured targets" into the graph.
skyframeExecutor
.getDifferencerForTesting()
@@ -1092,7 +1092,7 @@
ImmutableList<PathFragment> children = ImmutableList.of(PathFragment.create("child"));
Action action1 =
new TreeArtifactAction(NestedSetBuilder.emptySet(Order.STABLE_ORDER), baseOutput, children);
- ConfiguredTargetValue baseCt = createConfiguredTargetValue(action1, baseKey);
+ ActionLookupValue baseCt = createActionLookupValue(action1, baseKey);
ActionLookupKey shared1 = new InjectedActionLookupKey("shared1");
PathFragment execPath2 = PathFragment.create("out").getRelative("treesShared");
SpecialArtifact sharedOutput1 =
@@ -1103,7 +1103,7 @@
Artifact.SpecialArtifactType.TREE);
ActionTemplate<DummyAction> template1 =
new DummyActionTemplate(baseOutput, sharedOutput1, ActionOwner.SYSTEM_ACTION_OWNER);
- ConfiguredTargetValue shared1Ct = createConfiguredTargetValue(template1, shared1);
+ ActionLookupValue shared1Ct = createActionLookupValue(template1, shared1);
ActionLookupKey shared2 = new InjectedActionLookupKey("shared2");
SpecialArtifact sharedOutput2 =
new SpecialArtifact(
@@ -1113,7 +1113,7 @@
Artifact.SpecialArtifactType.TREE);
ActionTemplate<DummyAction> template2 =
new DummyActionTemplate(baseOutput, sharedOutput2, ActionOwner.SYSTEM_ACTION_OWNER);
- ConfiguredTargetValue shared2Ct = createConfiguredTargetValue(template2, shared2);
+ ActionLookupValue shared2Ct = createActionLookupValue(template2, shared2);
// Inject the "configured targets" into the graph.
skyframeExecutor
.getDifferencerForTesting()
@@ -1283,7 +1283,7 @@
Action action1 =
new DummyAction(
NestedSetBuilder.emptySet(Order.STABLE_ORDER), output1, MiddlemanType.NORMAL);
- ConfiguredTargetValue ctValue1 = createConfiguredTargetValue(action1, lc1);
+ ActionLookupValue ctValue1 = createActionLookupValue(action1, lc1);
ActionLookupKey lc2 = new InjectedActionLookupKey("lc2");
Artifact output2 =
new Artifact.DerivedArtifact(
@@ -1301,7 +1301,7 @@
},
NestedSetBuilder.emptySet(Order.STABLE_ORDER),
ImmutableSet.of(output2));
- ConfiguredTargetValue ctValue2 = createConfiguredTargetValue(action2, lc2);
+ ActionLookupValue ctValue2 = createActionLookupValue(action2, lc2);
ActionLookupKey topLc = new InjectedActionLookupKey("top");
Artifact top =
@@ -1324,7 +1324,7 @@
},
NestedSetBuilder.create(Order.STABLE_ORDER, output1),
ImmutableSet.of(top));
- ConfiguredTargetValue ctTop = createConfiguredTargetValue(topAction, topLc);
+ ActionLookupValue ctTop = createActionLookupValue(topAction, topLc);
// Inject the "configured targets" and artifact into the graph.
skyframeExecutor
@@ -1386,7 +1386,7 @@
Action action1 = new WarningAction(ImmutableList.of(), output, "action 1");
SkyValue ctValue1 =
ValueWithMetadata.normal(
- createConfiguredTargetValue(action1, lc1),
+ createActionLookupValue(action1, lc1),
null,
NestedSetBuilder.create(
Order.STABLE_ORDER,
@@ -1401,7 +1401,7 @@
Action action2 = new WarningAction(ImmutableList.of(output), output2, "action 2");
SkyValue ctValue2 =
ValueWithMetadata.normal(
- createConfiguredTargetValue(action2, lc2),
+ createActionLookupValue(action2, lc2),
null,
NestedSetBuilder.create(
Order.STABLE_ORDER,
@@ -1561,7 +1561,7 @@
execPath.getRelative("foo"),
lc1);
Action action1 = new CatastrophicAction(output);
- ConfiguredTargetValue ctValue1 = createConfiguredTargetValue(action1, lc1);
+ ActionLookupValue ctValue1 = createActionLookupValue(action1, lc1);
ActionLookupKey lc2 = new InjectedActionLookupKey("lc2");
Artifact output2 =
new Artifact.DerivedArtifact(
@@ -1570,7 +1570,7 @@
lc2);
AtomicBoolean markerRan = new AtomicBoolean(false);
Action action2 = new MarkerAction(output2, markerRan);
- ConfiguredTargetValue ctValue2 = createConfiguredTargetValue(action2, lc2);
+ ActionLookupValue ctValue2 = createActionLookupValue(action2, lc2);
// Perform testing-related setup.
skyframeExecutor
@@ -1625,12 +1625,10 @@
assertThat(markerRan.get()).isFalse();
}
- private static NonRuleConfiguredTargetValue createConfiguredTargetValue(
+ private static ActionLookupValue createActionLookupValue(
ActionAnalysisMetadata generatingAction, ActionLookupKey actionLookupKey) {
- return new NonRuleConfiguredTargetValue(
- new SerializableConfiguredTarget(),
- GeneratingActions.fromSingleAction(generatingAction, actionLookupKey),
- NestedSetBuilder.<Package>stableOrder().build());
+ return new BasicActionLookupValue(
+ Actions.GeneratingActions.fromSingleAction(generatingAction, actionLookupKey));
}
@Test
@@ -1672,8 +1670,7 @@
return super.execute(actionExecutionContext);
}
};
- ConfiguredTargetValue catastropheCTV =
- createConfiguredTargetValue(catastrophicAction, catastropheCTK);
+ ActionLookupValue catastropheALV = createActionLookupValue(catastrophicAction, catastropheCTK);
ActionLookupKey failureCTK = new InjectedActionLookupKey("failure");
Artifact failureArtifact =
new Artifact.DerivedArtifact(
@@ -1681,7 +1678,7 @@
execPath.getRelative("fail"),
failureCTK);
Action failureAction = new FailedExecAction(failureArtifact, USER_DETAILED_EXIT_CODE);
- ConfiguredTargetValue failureCTV = createConfiguredTargetValue(failureAction, failureCTK);
+ ActionLookupValue failureALV = createActionLookupValue(failureAction, failureCTK);
ActionLookupKey topCTK = new InjectedActionLookupKey("top");
Artifact topArtifact =
new Artifact.DerivedArtifact(
@@ -1692,15 +1689,15 @@
new DummyAction(
NestedSetBuilder.create(Order.STABLE_ORDER, failureArtifact, catastropheArtifact),
topArtifact);
- ConfiguredTargetValue topCTV = createConfiguredTargetValue(topAction, topCTK);
+ ActionLookupValue topALV = createActionLookupValue(topAction, topCTK);
// Perform testing-related setup.
skyframeExecutor
.getDifferencerForTesting()
.inject(
ImmutableMap.of(
- catastropheCTK, catastropheCTV,
- failureCTK, failureCTV,
- topCTK, topCTV));
+ catastropheCTK, catastropheALV,
+ failureCTK, failureALV,
+ topCTK, topALV));
skyframeExecutor
.getDriver()
.getGraphForTesting()
@@ -1802,9 +1799,8 @@
failedArtifacts.add(failureArtifact);
failedActions.add(new FailedExecAction(failureArtifact, USER_DETAILED_EXIT_CODE));
}
- NonRuleConfiguredTargetValue nonRuleConfiguredTargetValue =
- new NonRuleConfiguredTargetValue(
- new SerializableConfiguredTarget(),
+ ActionLookupValue nonRuleActionLookupValue =
+ new BasicActionLookupValue(
Actions.assignOwnersAndFilterSharedActionsAndThrowActionConflict(
new ActionKeyContext(),
ImmutableList.<ActionAnalysisMetadata>builder()
@@ -1812,8 +1808,7 @@
.addAll(failedActions)
.build(),
configuredTargetKey,
- /*outputFiles=*/ null),
- NestedSetBuilder.<Package>stableOrder().build());
+ /*outputFiles=*/ null));
HashSet<ActionLookupData> failedActionKeys = new HashSet<>();
for (Action failedAction : failedActions) {
failedActionKeys.add(
@@ -1823,7 +1818,7 @@
// Perform testing-related setup.
skyframeExecutor
.getDifferencerForTesting()
- .inject(ImmutableMap.of(configuredTargetKey, nonRuleConfiguredTargetValue));
+ .inject(ImmutableMap.of(configuredTargetKey, nonRuleActionLookupValue));
skyframeExecutor
.getDriver()
.getGraphForTesting()
@@ -1934,7 +1929,7 @@
NestedSetBuilder.emptySet(Order.STABLE_ORDER),
ImmutableSet.of(failedOutput));
failedActionReference.set(failedAction);
- ConfiguredTargetValue failedTarget = createConfiguredTargetValue(failedAction, failedKey);
+ ActionLookupValue failedTarget = createActionLookupValue(failedAction, failedKey);
// And an action that throws a catastrophic exception when it is executed,
ActionLookupKey catastrophicKey = new InjectedActionLookupKey("catastrophic");
@@ -1944,8 +1939,8 @@
execPath.getRelative("catastrophic"),
catastrophicKey);
Action catastrophicAction = new CatastrophicAction(catastrophicOutput);
- ConfiguredTargetValue catastrophicTarget =
- createConfiguredTargetValue(catastrophicAction, catastrophicKey);
+ ActionLookupValue catastrophicTarget =
+ createActionLookupValue(catastrophicAction, catastrophicKey);
// And the relevant configured targets have been injected into the graph,
skyframeExecutor
@@ -2051,10 +2046,9 @@
// Create 1 succeeded key and 1 failed key with user error
Action succeededAction =
new DummyAction(NestedSetBuilder.emptySet(Order.STABLE_ORDER), succeededOutput);
- ConfiguredTargetValue succeededTarget =
- createConfiguredTargetValue(succeededAction, succeededKey);
+ ActionLookupValue succeededTarget = createActionLookupValue(succeededAction, succeededKey);
Action failedAction = new FailedExecAction(failedOutput, USER_DETAILED_EXIT_CODE);
- ConfiguredTargetValue failedTarget = createConfiguredTargetValue(failedAction, failedKey);
+ ActionLookupValue failedTarget = createActionLookupValue(failedAction, failedKey);
// Inject the targets into the graph,
skyframeExecutor
@@ -2147,12 +2141,11 @@
Action succeededAction =
new DummyAction(NestedSetBuilder.emptySet(Order.STABLE_ORDER), succeededOutput);
- ConfiguredTargetValue succeededTarget =
- createConfiguredTargetValue(succeededAction, succeededKey);
+ ActionLookupValue succeededTarget = createActionLookupValue(succeededAction, succeededKey);
Action failedAction1 = new FailedExecAction(failedOutput1, USER_DETAILED_EXIT_CODE);
- ConfiguredTargetValue failedTarget1 = createConfiguredTargetValue(failedAction1, failedKey1);
+ ActionLookupValue failedTarget1 = createActionLookupValue(failedAction1, failedKey1);
Action failedAction2 = new FailedExecAction(failedOutput2, INFRA_DETAILED_EXIT_CODE);
- ConfiguredTargetValue failedTarget2 = createConfiguredTargetValue(failedAction2, failedKey2);
+ ActionLookupValue failedTarget2 = createActionLookupValue(failedAction2, failedKey2);
// Inject the targets into the graph,
skyframeExecutor
@@ -2248,7 +2241,7 @@
}
};
- ConfiguredTargetValue topTarget = createConfiguredTargetValue(inputDiscoveringAction, topKey);
+ ActionLookupValue topTarget = createActionLookupValue(inputDiscoveringAction, topKey);
skyframeExecutor.getDifferencerForTesting().inject(ImmutableMap.of(topKey, topTarget));
// Collect all events.
eventCollector = new EventCollector();
