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())) {
