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