Pass only the top-level artifacts to `ExecutorLifecycleListener` and make the computation more efficient.
Delete the now unused `ArtifactsToOwnerLabels`.
PiperOrigin-RevId: 372152937
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/AnalysisResult.java b/src/main/java/com/google/devtools/build/lib/analysis/AnalysisResult.java
index 3c63b34..bd6d48e 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/AnalysisResult.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/AnalysisResult.java
@@ -20,6 +20,7 @@
import com.google.common.collect.ImmutableSortedSet;
import com.google.common.collect.Sets;
import com.google.devtools.build.lib.actions.ActionGraph;
+import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.PackageRoots;
import com.google.devtools.build.lib.analysis.config.BuildConfigurationCollection;
import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
@@ -37,7 +38,7 @@
private final ImmutableSet<ConfiguredTarget> targetsToSkip;
@Nullable private final FailureDetail failureDetail;
private final ActionGraph actionGraph;
- private final ArtifactsToOwnerLabels topLevelArtifactsToOwnerLabels;
+ private final ImmutableSet<Artifact> artifactsToBuild;
private final ImmutableSet<ConfiguredTarget> parallelTests;
private final ImmutableSet<ConfiguredTarget> exclusiveTests;
@Nullable private final TopLevelArtifactContext topLevelContext;
@@ -55,7 +56,7 @@
ImmutableSet<ConfiguredTarget> targetsToSkip,
@Nullable FailureDetail failureDetail,
ActionGraph actionGraph,
- ArtifactsToOwnerLabels topLevelArtifactsToOwnerLabels,
+ ImmutableSet<Artifact> artifactsToBuild,
ImmutableSet<ConfiguredTarget> parallelTests,
ImmutableSet<ConfiguredTarget> exclusiveTests,
TopLevelArtifactContext topLevelContext,
@@ -70,7 +71,7 @@
this.targetsToSkip = targetsToSkip;
this.failureDetail = failureDetail;
this.actionGraph = actionGraph;
- this.topLevelArtifactsToOwnerLabels = topLevelArtifactsToOwnerLabels;
+ this.artifactsToBuild = artifactsToBuild;
this.parallelTests = parallelTests;
this.exclusiveTests = exclusiveTests;
this.topLevelContext = topLevelContext;
@@ -120,8 +121,8 @@
return targetsToSkip;
}
- public ArtifactsToOwnerLabels getTopLevelArtifactsToOwnerLabels() {
- return topLevelArtifactsToOwnerLabels;
+ public ImmutableSet<Artifact> getArtifactsToBuild() {
+ return artifactsToBuild;
}
public ImmutableSet<ConfiguredTarget> getExclusiveTests() {
@@ -178,7 +179,7 @@
targetsToSkip,
failureDetail,
actionGraph,
- topLevelArtifactsToOwnerLabels,
+ artifactsToBuild,
Sets.union(parallelTests, exclusiveTests).immutableCopy(),
/*exclusiveTests=*/ ImmutableSet.of(),
topLevelContext,
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ArtifactsToOwnerLabels.java b/src/main/java/com/google/devtools/build/lib/analysis/ArtifactsToOwnerLabels.java
deleted file mode 100644
index 12fbb16..0000000
--- a/src/main/java/com/google/devtools/build/lib/analysis/ArtifactsToOwnerLabels.java
+++ /dev/null
@@ -1,125 +0,0 @@
-// Copyright 2018 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.common.base.Preconditions;
-import com.google.common.collect.HashMultimap;
-import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.SetMultimap;
-import com.google.common.collect.Sets;
-import com.google.devtools.build.lib.actions.Artifact;
-import com.google.devtools.build.lib.cmdline.Label;
-import java.util.HashSet;
-import java.util.Set;
-import java.util.function.Predicate;
-
-/**
- * Morally a {@link SetMultimap} from artifacts to the labels that own them, which may not be just
- * {@link Artifact#getOwnerLabel} depending on the build. Optimizes for artifacts that are owned by
- * their {@link Artifact#getOwnerLabel} by storing them separately.
- */
-public class ArtifactsToOwnerLabels {
- private final SetMultimap<Artifact, Label> artifactToMultipleOrDifferentOwnerLabels;
- private final Set<Artifact> artifactsOwnedOnlyByTheirLabels;
-
- private ArtifactsToOwnerLabels(
- SetMultimap<Artifact, Label> artifactToMultipleOrDifferentOwnerLabels,
- Set<Artifact> artifactsOwnedOnlyByTheirLabels) {
- this.artifactToMultipleOrDifferentOwnerLabels = artifactToMultipleOrDifferentOwnerLabels;
- this.artifactsOwnedOnlyByTheirLabels = artifactsOwnedOnlyByTheirLabels;
- }
-
- public Set<Label> getOwners(Artifact artifact) {
- if (artifactsOwnedOnlyByTheirLabels.contains(artifact)) {
- Preconditions.checkState(
- !artifactToMultipleOrDifferentOwnerLabels.containsKey(artifact),
- "Artifact %s incorrectly in multiple categories",
- artifact);
- Label ownerLabel = artifact.getOwnerLabel();
- if (ownerLabel == null) {
- return ImmutableSet.of();
- }
- return ImmutableSet.of(ownerLabel);
- }
- return Preconditions.checkNotNull(
- artifactToMultipleOrDifferentOwnerLabels.get(artifact), artifact);
- }
-
- public Set<Artifact> getArtifacts() {
- return Sets.union(
- artifactsOwnedOnlyByTheirLabels, artifactToMultipleOrDifferentOwnerLabels.keySet());
- }
-
- public Builder toBuilder() {
- return new Builder(
- HashMultimap.create(artifactToMultipleOrDifferentOwnerLabels),
- new HashSet<>(artifactsOwnedOnlyByTheirLabels));
- }
-
- /** Builder for {@link ArtifactsToOwnerLabels}. */
- public static class Builder {
- private final SetMultimap<Artifact, Label> artifactToMultipleOrDifferentOwnerLabels;
- private final Set<Artifact> artifactsOwnedOnlyByTheirLabels;
-
- public Builder() {
- this(HashMultimap.create(), new HashSet<>());
- }
-
- private Builder(
- SetMultimap<Artifact, Label> artifactToMultipleOrDifferentOwnerLabels,
- Set<Artifact> artifactsOwnedOnlyByTheirLabels) {
- this.artifactToMultipleOrDifferentOwnerLabels = artifactToMultipleOrDifferentOwnerLabels;
- this.artifactsOwnedOnlyByTheirLabels = artifactsOwnedOnlyByTheirLabels;
- }
-
- public Builder addArtifact(Artifact artifact) {
- if (artifactToMultipleOrDifferentOwnerLabels.containsKey(artifact)) {
- Label ownerLabel = artifact.getOwnerLabel();
- if (ownerLabel != null) {
- artifactToMultipleOrDifferentOwnerLabels.put(artifact, artifact.getOwnerLabel());
- }
- } else {
- artifactsOwnedOnlyByTheirLabels.add(artifact);
- }
- return this;
- }
-
- public Builder addArtifact(Artifact artifact, Label label) {
- Preconditions.checkNotNull(label, artifact);
- if (label.equals(artifact.getOwnerLabel())) {
- addArtifact(artifact);
- } else {
- artifactToMultipleOrDifferentOwnerLabels.put(artifact, label);
- if (artifactsOwnedOnlyByTheirLabels.remove(artifact)) {
- // Redoing this call now that we have a mismatched label will force addition into the
- // multimap.
- addArtifact(artifact);
- }
- }
- return this;
- }
-
- public Builder filterArtifacts(Predicate<Artifact> artifactsToKeep) {
- Predicate<Artifact> artifactsToRemove = artifactsToKeep.negate();
- artifactToMultipleOrDifferentOwnerLabels.keySet().removeIf(artifactsToRemove);
- artifactsOwnedOnlyByTheirLabels.removeIf(artifactsToRemove);
- return this;
- }
-
- ArtifactsToOwnerLabels build() {
- return new ArtifactsToOwnerLabels(
- artifactToMultipleOrDifferentOwnerLabels, artifactsOwnedOnlyByTheirLabels);
- }
- }
-}
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 407ea48..30dc53d 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BUILD
@@ -38,7 +38,6 @@
":analysis_options",
":analysis_phase_complete_event",
":analysis_phase_started_event",
- ":artifacts_to_owner_labels",
":aspect_aware_attribute_mapper",
":aspect_collection",
":aspect_configured_event",
@@ -282,7 +281,6 @@
":actions/symlink_action",
":actions/template_expansion_action",
":actions_provider",
- ":artifacts_to_owner_labels",
":aspect_aware_attribute_mapper",
":aspect_collection",
":build_setting_provider",
@@ -498,16 +496,6 @@
)
java_library(
- name = "artifacts_to_owner_labels",
- srcs = ["ArtifactsToOwnerLabels.java"],
- deps = [
- "//src/main/java/com/google/devtools/build/lib/actions:artifacts",
- "//src/main/java/com/google/devtools/build/lib/cmdline",
- "//third_party:guava",
- ],
-)
-
-java_library(
name = "aspect_aware_attribute_mapper",
srcs = ["AspectAwareAttributeMapper.java"],
deps = [
@@ -599,7 +587,6 @@
":analysis_cluster",
":analysis_options",
":analysis_phase_started_event",
- ":artifacts_to_owner_labels",
":aspect_configured_event",
":blaze_directories",
":config/build_configuration",
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
index ee61f22..bec43a1 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
@@ -470,8 +470,7 @@
allTargetsToTest = filterTestsByTargets(configuredTargets, testsToRun);
}
- ArtifactsToOwnerLabels.Builder artifactsToOwnerLabelsBuilder =
- new ArtifactsToOwnerLabels.Builder();
+ ImmutableSet.Builder<Artifact> artifactsToBuild = ImmutableSet.builder();
// build-info and build-changelist.
Collection<Artifact> buildInfoArtifacts =
@@ -480,11 +479,11 @@
// Extra actions
addExtraActionsIfRequested(
- viewOptions, configuredTargets, aspects, artifactsToOwnerLabelsBuilder, eventHandler);
+ viewOptions, configuredTargets, aspects, artifactsToBuild, eventHandler);
// Coverage
NestedSet<Artifact> baselineCoverageArtifacts =
- getBaselineCoverageArtifacts(configuredTargets, artifactsToOwnerLabelsBuilder);
+ getBaselineCoverageArtifacts(configuredTargets, artifactsToBuild);
if (coverageReportActionFactory != null) {
CoverageReportActionsWrapper actionsWrapper;
actionsWrapper =
@@ -501,23 +500,24 @@
if (actionsWrapper != null) {
Actions.GeneratingActions actions = actionsWrapper.getActions();
skyframeExecutor.injectCoverageReportData(actions);
- actionsWrapper.getCoverageOutputs().forEach(artifactsToOwnerLabelsBuilder::addArtifact);
+ actionsWrapper.getCoverageOutputs().forEach(artifactsToBuild::add);
}
}
// TODO(cparsons): If extra actions are ever removed, this filtering step can probably be
// removed as well: the only concern would be action conflicts involving coverage artifacts,
// which seems far-fetched.
if (skyframeAnalysisResult.hasActionConflicts()) {
- ArtifactsToOwnerLabels tempOwners = artifactsToOwnerLabelsBuilder.build();
// We don't remove the (hopefully unnecessary) guard in SkyframeBuildView that enables/
// disables analysis, since no new targets should actually be analyzed.
- Set<Artifact> artifacts = tempOwners.getArtifacts();
+ ImmutableSet<Artifact> artifacts = artifactsToBuild.build();
Predicate<Artifact> errorFreeArtifacts =
skyframeExecutor.filterActionConflictsForTopLevelArtifacts(eventHandler, artifacts);
- artifactsToOwnerLabelsBuilder = tempOwners.toBuilder().filterArtifacts(errorFreeArtifacts);
+
+ artifactsToBuild = ImmutableSet.builder();
+ artifacts.stream().filter(errorFreeArtifacts).forEach(artifactsToBuild::add);
}
// Build-info artifacts are always conflict-free, and can't be checked easily.
- buildInfoArtifacts.forEach(artifactsToOwnerLabelsBuilder::addArtifact);
+ buildInfoArtifacts.forEach(artifactsToBuild::add);
// Tests.
Pair<ImmutableSet<ConfiguredTarget>, ImmutableSet<ConfiguredTarget>> testsPair =
@@ -529,8 +529,8 @@
FailureDetail failureDetail =
createFailureDetail(loadingResult, skyframeAnalysisResult, topLevelTargetsWithConfigs);
- final WalkableGraph graph = skyframeAnalysisResult.getWalkableGraph();
- final ActionGraph actionGraph =
+ WalkableGraph graph = skyframeAnalysisResult.getWalkableGraph();
+ ActionGraph actionGraph =
new ActionGraph() {
@Nullable
@Override
@@ -563,7 +563,7 @@
ImmutableSet.copyOf(targetsToSkip),
failureDetail,
actionGraph,
- artifactsToOwnerLabelsBuilder.build(),
+ artifactsToBuild.build(),
parallelTests,
exclusiveTests,
topLevelOptions,
@@ -623,16 +623,12 @@
private static NestedSet<Artifact> getBaselineCoverageArtifacts(
Collection<ConfiguredTarget> configuredTargets,
- ArtifactsToOwnerLabels.Builder topLevelArtifactsToOwnerLabels) {
+ ImmutableSet.Builder<Artifact> artifactsToBuild) {
NestedSetBuilder<Artifact> baselineCoverageArtifacts = NestedSetBuilder.stableOrder();
for (ConfiguredTarget target : configuredTargets) {
InstrumentedFilesInfo provider = target.get(InstrumentedFilesInfo.STARLARK_CONSTRUCTOR);
if (provider != null) {
- TopLevelArtifactHelper.addArtifactsWithOwnerLabel(
- provider.getBaselineCoverageArtifacts(),
- null,
- target.getLabel(),
- topLevelArtifactsToOwnerLabels);
+ artifactsToBuild.addAll(provider.getBaselineCoverageArtifacts().toList());
baselineCoverageArtifacts.addTransitive(provider.getBaselineCoverageArtifacts());
}
}
@@ -643,7 +639,7 @@
AnalysisOptions viewOptions,
Collection<ConfiguredTarget> configuredTargets,
ImmutableMap<AspectKey, ConfiguredAspect> aspects,
- ArtifactsToOwnerLabels.Builder artifactsToTopLevelLabelsMap,
+ ImmutableSet.Builder<Artifact> artifactsToBuild,
ExtendedEventHandler eventHandler) {
RegexFilter filter = viewOptions.extraActionFilter;
for (ConfiguredTarget target : configuredTargets) {
@@ -663,24 +659,15 @@
for (Attribute attr : actualTarget.getAssociatedRule().getAttributes()) {
aspectClasses.addAll(attr.getAspectClasses());
}
- TopLevelArtifactHelper.addArtifactsWithOwnerLabel(
- provider.getExtraActionArtifacts(),
- filter,
- target.getLabel(),
- artifactsToTopLevelLabelsMap);
+ addArtifactsToBuilder(
+ provider.getExtraActionArtifacts().toList(), artifactsToBuild, filter);
if (!aspectClasses.isEmpty()) {
- TopLevelArtifactHelper.addArtifactsWithOwnerLabel(
- filterTransitiveExtraActions(provider, aspectClasses),
- filter,
- target.getLabel(),
- artifactsToTopLevelLabelsMap);
+ addArtifactsToBuilder(
+ filterTransitiveExtraActions(provider, aspectClasses), artifactsToBuild, filter);
}
} else {
- TopLevelArtifactHelper.addArtifactsWithOwnerLabel(
- provider.getTransitiveExtraActionArtifacts(),
- filter,
- target.getLabel(),
- artifactsToTopLevelLabelsMap);
+ addArtifactsToBuilder(
+ provider.getTransitiveExtraActionArtifacts().toList(), artifactsToBuild, filter);
}
}
}
@@ -689,22 +676,27 @@
aspectEntry.getValue().getProvider(ExtraActionArtifactsProvider.class);
if (provider != null) {
if (viewOptions.extraActionTopLevelOnly) {
- TopLevelArtifactHelper.addArtifactsWithOwnerLabel(
- provider.getExtraActionArtifacts(),
- filter,
- aspectEntry.getKey().getLabel(),
- artifactsToTopLevelLabelsMap);
+ addArtifactsToBuilder(
+ provider.getExtraActionArtifacts().toList(), artifactsToBuild, filter);
} else {
- TopLevelArtifactHelper.addArtifactsWithOwnerLabel(
- provider.getTransitiveExtraActionArtifacts(),
- filter,
- aspectEntry.getKey().getLabel(),
- artifactsToTopLevelLabelsMap);
+ addArtifactsToBuilder(
+ provider.getTransitiveExtraActionArtifacts().toList(), artifactsToBuild, filter);
}
}
}
}
+ private static void addArtifactsToBuilder(
+ List<? extends Artifact> artifacts,
+ ImmutableSet.Builder<Artifact> builder,
+ RegexFilter filter) {
+ for (Artifact artifact : artifacts) {
+ if (filter.isIncluded(artifact.getOwnerLabel().toString())) {
+ builder.add(artifact);
+ }
+ }
+ }
+
/**
* Returns a list of artifacts from 'provider' that were registered by an aspect from
* 'aspectClasses'. All artifacts in 'provider' are considered - both direct and transitive.
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/OutputGroupInfo.java b/src/main/java/com/google/devtools/build/lib/analysis/OutputGroupInfo.java
index 1c1fc4c..9f39590 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/OutputGroupInfo.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/OutputGroupInfo.java
@@ -151,7 +151,7 @@
* Validation outputs built by {@code ValidateTarget} aspect "promoting" {@link #VALIDATION}
* output group Blaze core collects to {@link #VALIDATION_TOP_LEVEL} and requesting the latter.
*/
- ASPECT;
+ ASPECT
}
private final ImmutableMap<String, NestedSet<Artifact>> outputGroups;
@@ -182,9 +182,8 @@
* If the specified output group is not present, the empty set is returned.
*/
public NestedSet<Artifact> getOutputGroup(String outputGroupName) {
- return outputGroups.containsKey(outputGroupName)
- ? outputGroups.get(outputGroupName)
- : NestedSetBuilder.<Artifact>emptySet(Order.STABLE_ORDER);
+ return outputGroups.getOrDefault(
+ outputGroupName, NestedSetBuilder.emptySet(Order.STABLE_ORDER));
}
/**
@@ -195,7 +194,7 @@
@Nullable
public static OutputGroupInfo merge(List<OutputGroupInfo> providers)
throws DuplicateException {
- if (providers.size() == 0) {
+ if (providers.isEmpty()) {
return null;
}
if (providers.size() == 1) {
@@ -293,7 +292,7 @@
}
@Override
- public boolean containsKey(StarlarkSemantics semantics, Object key) throws EvalException {
+ public boolean containsKey(StarlarkSemantics semantics, Object key) {
return outputGroups.containsKey(key);
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/TopLevelArtifactHelper.java b/src/main/java/com/google/devtools/build/lib/analysis/TopLevelArtifactHelper.java
index 060d5e9..6c02cf3 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/TopLevelArtifactHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/TopLevelArtifactHelper.java
@@ -14,26 +14,22 @@
package com.google.devtools.build.lib.analysis;
+import static com.google.common.base.MoreObjects.firstNonNull;
import static com.google.common.base.Preconditions.checkNotNull;
-import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.actions.Artifact;
-import com.google.devtools.build.lib.analysis.test.TestProvider;
-import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSet.Node;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.profiler.AutoProfiler;
import com.google.devtools.build.lib.profiler.GoogleAutoProfilerUtils;
-import com.google.devtools.build.lib.skyframe.AspectValueKey.AspectKey;
-import com.google.devtools.build.lib.util.RegexFilter;
import java.time.Duration;
import java.util.ArrayList;
-import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
@@ -127,61 +123,48 @@
private static final Duration MIN_LOGGING = Duration.ofMillis(10);
- @VisibleForTesting
- public static ArtifactsToOwnerLabels makeTopLevelArtifactsToOwnerLabels(
- AnalysisResult analysisResult) {
+ /**
+ * Returns the set of all top-level output artifacts.
+ *
+ * <p>In contrast with {@link AnalysisResult#getArtifactsToBuild}, which only returns artifacts to
+ * request from the build tool, this method returns <em>all</em> artifacts produced by top-level
+ * targets (including tests) and aspects.
+ */
+ public static ImmutableSet<Artifact> findAllTopLevelArtifacts(AnalysisResult analysisResult) {
try (AutoProfiler ignored =
- GoogleAutoProfilerUtils.logged("assigning owner labels", MIN_LOGGING)) {
- ArtifactsToOwnerLabels.Builder artifactsToOwnerLabelsBuilder =
- analysisResult.getTopLevelArtifactsToOwnerLabels().toBuilder();
- TopLevelArtifactContext artifactContext = analysisResult.getTopLevelContext();
- for (ConfiguredTarget target : analysisResult.getTargetsToBuild()) {
- addArtifactsWithOwnerLabel(
- getAllArtifactsToBuild(target, artifactContext).getAllArtifacts(),
- null,
- target.getLabel(),
- artifactsToOwnerLabelsBuilder);
- }
- for (Map.Entry<AspectKey, ConfiguredAspect> aspectEntry :
- analysisResult.getAspectsMap().entrySet()) {
- addArtifactsWithOwnerLabel(
- getAllArtifactsToBuild(aspectEntry.getValue(), artifactContext).getAllArtifacts(),
- null,
- aspectEntry.getKey().getLabel(),
- artifactsToOwnerLabelsBuilder);
- }
- if (analysisResult.getTargetsToTest() != null) {
- for (ConfiguredTarget target : analysisResult.getTargetsToTest()) {
- addArtifactsWithOwnerLabel(
- TestProvider.getTestStatusArtifacts(target),
- null,
- target.getLabel(),
- artifactsToOwnerLabelsBuilder);
+ GoogleAutoProfilerUtils.logged("finding top level artifacts", MIN_LOGGING)) {
+
+ ImmutableSet.Builder<Artifact> artifacts = ImmutableSet.builder();
+ artifacts.addAll(analysisResult.getArtifactsToBuild());
+
+ Iterable<ProviderCollection> providers =
+ Iterables.concat(
+ analysisResult.getTargetsToBuild(),
+ analysisResult.getAspectsMap().values(),
+ firstNonNull(analysisResult.getTargetsToTest(), ImmutableList.of()));
+ TopLevelArtifactContext ctx = analysisResult.getTopLevelContext();
+ Set<NestedSet.Node> visited = new HashSet<>();
+
+ for (ProviderCollection provider : providers) {
+ for (ArtifactsInOutputGroup group :
+ getAllArtifactsToBuild(provider, ctx).getAllArtifactsByOutputGroup().values()) {
+ memoizedAddAll(group.getArtifacts(), artifacts, visited);
}
}
- // TODO(dslomov): Artifacts to test from aspects?
- return artifactsToOwnerLabelsBuilder.build();
+ return artifacts.build();
}
}
- public static void addArtifactsWithOwnerLabel(
- NestedSet<? extends Artifact> artifacts,
- @Nullable RegexFilter filter,
- Label ownerLabel,
- ArtifactsToOwnerLabels.Builder artifactsToOwnerLabelsBuilder) {
- addArtifactsWithOwnerLabel(
- artifacts.toList(), filter, ownerLabel, artifactsToOwnerLabelsBuilder);
- }
-
- public static void addArtifactsWithOwnerLabel(
- Collection<? extends Artifact> artifacts,
- @Nullable RegexFilter filter,
- Label ownerLabel,
- ArtifactsToOwnerLabels.Builder artifactsToOwnerLabelsBuilder) {
- for (Artifact artifact : artifacts) {
- if (filter == null || filter.isIncluded(artifact.getOwnerLabel().toString())) {
- artifactsToOwnerLabelsBuilder.addArtifact(artifact, ownerLabel);
- }
+ private static void memoizedAddAll(
+ NestedSet<Artifact> current,
+ ImmutableSet.Builder<Artifact> artifacts,
+ Set<NestedSet.Node> visited) {
+ if (!visited.add(current.toNode())) {
+ return;
+ }
+ artifacts.addAll(current.getLeaves());
+ for (NestedSet<Artifact> child : current.getNonLeaves()) {
+ memoizedAddAll(child, artifacts, visited);
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java b/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java
index 50a6d96..a7bf2aa 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java
@@ -103,6 +103,7 @@
import java.util.Collection;
import java.util.HashSet;
import java.util.LinkedHashSet;
+import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.UUID;
@@ -192,9 +193,7 @@
actionContextRegistryBuilder.register(DynamicStrategyRegistry.class, spawnStrategyRegistry);
actionContextRegistryBuilder.register(RemoteLocalFallbackRegistry.class, spawnStrategyRegistry);
- ModuleActionContextRegistry moduleActionContextRegistry = actionContextRegistryBuilder.build();
-
- this.actionContextRegistry = moduleActionContextRegistry;
+ this.actionContextRegistry = actionContextRegistryBuilder.build();
this.spawnStrategyRegistry = spawnStrategyRegistry;
}
@@ -336,7 +335,7 @@
actionGraph,
// If this supplier is ever consumed by more than one ActionContextProvider, it can be
// pulled out of the loop and made a memoizing supplier.
- () -> TopLevelArtifactHelper.makeTopLevelArtifactsToOwnerLabels(analysisResult));
+ () -> TopLevelArtifactHelper.findAllTopLevelArtifacts(analysisResult));
}
}
skyframeExecutor.drainChangedFiles();
@@ -347,12 +346,10 @@
Profiler.instance().markPhase(ProfilePhase.EXECUTE);
boolean shouldTrustRemoteArtifacts =
- env.getOutputService() == null
- ? false
- : env.getOutputService().shouldTrustRemoteArtifacts();
+ env.getOutputService() != null && env.getOutputService().shouldTrustRemoteArtifacts();
builder.buildArtifacts(
env.getReporter(),
- analysisResult.getTopLevelArtifactsToOwnerLabels().getArtifacts(),
+ analysisResult.getArtifactsToBuild(),
analysisResult.getParallelTests(),
analysisResult.getExclusiveTests(),
analysisResult.getTargetsToBuild(),
@@ -598,7 +595,7 @@
buildRequestOptions.useTopLevelTargetsForSymlinks()
? analysisResult.getTargetsToBuild().stream()
.map(ConfiguredTarget::getConfigurationKey)
- .filter(configuration -> configuration != null)
+ .filter(Objects::nonNull)
.distinct()
.map((key) -> executor.getConfiguration(reporter, key))
.collect(toImmutableSet())
@@ -713,7 +710,7 @@
*
* @param configuredTargets The configured targets whose artifacts are to be built.
*/
- private Collection<ConfiguredTarget> determineSuccessfulTargets(
+ private static Collection<ConfiguredTarget> determineSuccessfulTargets(
Collection<ConfiguredTarget> configuredTargets, Set<ConfiguredTargetKey> builtTargets) {
// Maintain the ordering by copying builtTargets into a LinkedHashSet in the same iteration
// order as configuredTargets.
diff --git a/src/main/java/com/google/devtools/build/lib/exec/BUILD b/src/main/java/com/google/devtools/build/lib/exec/BUILD
index 0a85847..6156860 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/exec/BUILD
@@ -123,7 +123,7 @@
srcs = ["ExecutorLifecycleListener.java"],
deps = [
"//src/main/java/com/google/devtools/build/lib/actions",
- "//src/main/java/com/google/devtools/build/lib/analysis:artifacts_to_owner_labels",
+ "//src/main/java/com/google/devtools/build/lib/actions:artifacts",
"//src/main/java/com/google/devtools/build/lib/util:abrupt_exit_exception",
"//third_party:guava",
],
diff --git a/src/main/java/com/google/devtools/build/lib/exec/ExecutorLifecycleListener.java b/src/main/java/com/google/devtools/build/lib/exec/ExecutorLifecycleListener.java
index 14dc308..0e50691 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/ExecutorLifecycleListener.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/ExecutorLifecycleListener.java
@@ -13,10 +13,11 @@
// limitations under the License.
package com.google.devtools.build.lib.exec;
-import com.google.common.base.Supplier;
+import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.actions.ActionGraph;
-import com.google.devtools.build.lib.analysis.ArtifactsToOwnerLabels;
+import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.util.AbruptExitException;
+import java.util.function.Supplier;
/**
* Type that can get informed about executor lifecycle events.
@@ -38,11 +39,10 @@
* Handles the start of the execution phase.
*
* @param actionGraph actions as calcuated in the analysis phase
- * @param topLevelArtifactsToOwnerLabels supplier of all output artifacts from top-level targets
- * and aspects, mapped to their owners
+ * @param topLevelArtifacts supplies all output artifacts from top-level targets and aspects
*/
void executionPhaseStarting(
- ActionGraph actionGraph, Supplier<ArtifactsToOwnerLabels> topLevelArtifactsToOwnerLabels)
+ ActionGraph actionGraph, Supplier<ImmutableSet<Artifact>> topLevelArtifacts)
throws AbruptExitException, InterruptedException;
/** Handles the end of the execution phase. */
diff --git a/src/main/java/com/google/devtools/build/lib/includescanning/BUILD b/src/main/java/com/google/devtools/build/lib/includescanning/BUILD
index 2bf6673..f5dce07 100644
--- a/src/main/java/com/google/devtools/build/lib/includescanning/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/includescanning/BUILD
@@ -21,7 +21,6 @@
"//src/main/java/com/google/devtools/build/lib/actions:file_metadata",
"//src/main/java/com/google/devtools/build/lib/actions:localhost_capacity",
"//src/main/java/com/google/devtools/build/lib/actions:middleman_type",
- "//src/main/java/com/google/devtools/build/lib/analysis:artifacts_to_owner_labels",
"//src/main/java/com/google/devtools/build/lib/analysis:blaze_directories",
"//src/main/java/com/google/devtools/build/lib/analysis/platform",
"//src/main/java/com/google/devtools/build/lib/cmdline",
@@ -43,7 +42,6 @@
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
"//src/main/java/com/google/devtools/build/lib/util:abrupt_exit_exception",
"//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code",
- "//src/main/java/com/google/devtools/build/lib/util:exit_code",
"//src/main/java/com/google/devtools/build/lib/util:resource_converter",
"//src/main/java/com/google/devtools/build/lib/util/io",
"//src/main/java/com/google/devtools/build/lib/vfs",
diff --git a/src/main/java/com/google/devtools/build/lib/includescanning/IncludeScannerSupplier.java b/src/main/java/com/google/devtools/build/lib/includescanning/IncludeScannerSupplier.java
index dd56ff4..729825b 100644
--- a/src/main/java/com/google/devtools/build/lib/includescanning/IncludeScannerSupplier.java
+++ b/src/main/java/com/google/devtools/build/lib/includescanning/IncludeScannerSupplier.java
@@ -14,7 +14,6 @@
package com.google.devtools.build.lib.includescanning;
import com.google.common.base.Preconditions;
-import com.google.common.base.Supplier;
import com.google.common.cache.CacheBuilder;
import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;
@@ -32,6 +31,7 @@
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.ExecutorService;
+import java.util.function.Supplier;
/**
* Creates include scanner instances.
diff --git a/src/main/java/com/google/devtools/build/lib/includescanning/IncludeScanningModule.java b/src/main/java/com/google/devtools/build/lib/includescanning/IncludeScanningModule.java
index f0e5ec3..7b25140 100644
--- a/src/main/java/com/google/devtools/build/lib/includescanning/IncludeScanningModule.java
+++ b/src/main/java/com/google/devtools/build/lib/includescanning/IncludeScanningModule.java
@@ -14,7 +14,6 @@
package com.google.devtools.build.lib.includescanning;
import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Supplier;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
@@ -29,7 +28,6 @@
import com.google.devtools.build.lib.actions.ArtifactFactory;
import com.google.devtools.build.lib.actions.ArtifactResolver;
import com.google.devtools.build.lib.actions.ExecException;
-import com.google.devtools.build.lib.analysis.ArtifactsToOwnerLabels;
import com.google.devtools.build.lib.analysis.BlazeDirectories;
import com.google.devtools.build.lib.buildtool.BuildRequest;
import com.google.devtools.build.lib.concurrent.ExecutorUtil;
@@ -63,6 +61,7 @@
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.ExecutorService;
+import java.util.function.Supplier;
/**
* Module that provides implementations of {@link CppIncludeExtractionContext},
@@ -111,7 +110,7 @@
public Iterable<Class<? extends OptionsBase>> getCommandOptions(Command command) {
return "build".equals(command.name())
? ImmutableList.of(IncludeScanningOptions.class)
- : ImmutableList.<Class<? extends OptionsBase>>of();
+ : ImmutableList.of();
}
@Override
@@ -219,7 +218,7 @@
* supplier} which can be used to access the (potentially shared) scanners and exposes {@linkplain
* #getSwigActionContext() action} {@linkplain #getCppActionContext() contexts} based on them.
*/
- private static class IncludeScannerLifecycleManager implements ExecutorLifecycleListener {
+ private static final class IncludeScannerLifecycleManager implements ExecutorLifecycleListener {
private final CommandEnvironment env;
private final BuildRequest buildRequest;
@@ -227,7 +226,7 @@
private IncludeScannerSupplier includeScannerSupplier;
private ExecutorService includePool;
- public IncludeScannerLifecycleManager(
+ IncludeScannerLifecycleManager(
CommandEnvironment env,
BuildRequest buildRequest,
MutableSupplier<SpawnIncludeScanner> spawnScannerSupplier) {
@@ -254,8 +253,7 @@
@Override
public void executionPhaseStarting(
- ActionGraph actionGraph,
- Supplier<ArtifactsToOwnerLabels> topLevelArtifactsToAccountingGroups)
+ ActionGraph actionGraph, Supplier<ImmutableSet<Artifact>> topLevelArtifacts)
throws AbruptExitException, InterruptedException {
try {
includeScannerSupplier.init(
diff --git a/src/main/java/com/google/devtools/build/lib/includescanning/LegacyIncludeScanner.java b/src/main/java/com/google/devtools/build/lib/includescanning/LegacyIncludeScanner.java
index 51d7740..1aeb9c6 100644
--- a/src/main/java/com/google/devtools/build/lib/includescanning/LegacyIncludeScanner.java
+++ b/src/main/java/com/google/devtools/build/lib/includescanning/LegacyIncludeScanner.java
@@ -14,7 +14,6 @@
package com.google.devtools.build.lib.includescanning;
import com.google.common.base.Preconditions;
-import com.google.common.base.Supplier;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
@@ -55,6 +54,7 @@
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Future;
+import java.util.function.Supplier;
/**
* C include scanner. Quickly scans C/C++ source files to determine the bounding set of transitively
@@ -254,7 +254,7 @@
}
// Look for header in path/to/foo.framework/Headers/
- PathFragment foundHeaderPath = null;
+ PathFragment foundHeaderPath;
PathFragment fullHeaderPath =
fullFrameworkPath.getRelative("Headers").getRelative(relHeaderPath);
@@ -938,10 +938,10 @@
}
}
- private static class ExecRuntimeException extends RuntimeException {
+ private static final class ExecRuntimeException extends RuntimeException {
private final ExecException cause;
- public ExecRuntimeException(ExecException e) {
+ ExecRuntimeException(ExecException e) {
super(e);
this.cause = e;
}
@@ -951,10 +951,10 @@
}
}
- private static class InterruptedRuntimeException extends RuntimeException {
+ private static final class InterruptedRuntimeException extends RuntimeException {
private final InterruptedException cause;
- public InterruptedRuntimeException(InterruptedException e) {
+ InterruptedRuntimeException(InterruptedException e) {
super(e);
this.cause = e;
}
diff --git a/src/main/java/com/google/devtools/build/lib/remote/BUILD b/src/main/java/com/google/devtools/build/lib/remote/BUILD
index ad5159d..56bbf5d 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/remote/BUILD
@@ -48,7 +48,6 @@
"//src/main/java/com/google/devtools/build/lib/actions:file_metadata",
"//src/main/java/com/google/devtools/build/lib/actions:fileset_output_symlink",
"//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster",
- "//src/main/java/com/google/devtools/build/lib/analysis:artifacts_to_owner_labels",
"//src/main/java/com/google/devtools/build/lib/analysis:config/build_options",
"//src/main/java/com/google/devtools/build/lib/analysis:configured_target",
"//src/main/java/com/google/devtools/build/lib/analysis:top_level_artifact_context",
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java
index c1f8462..be7de3c 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java
@@ -16,12 +16,11 @@
import static com.google.common.base.Preconditions.checkNotNull;
import com.google.common.base.Preconditions;
-import com.google.common.base.Supplier;
import com.google.common.collect.ImmutableSet;
import com.google.common.util.concurrent.ListeningScheduledExecutorService;
import com.google.devtools.build.lib.actions.ActionGraph;
import com.google.devtools.build.lib.actions.ActionInput;
-import com.google.devtools.build.lib.analysis.ArtifactsToOwnerLabels;
+import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.exec.ExecutionOptions;
import com.google.devtools.build.lib.exec.ExecutorLifecycleListener;
import com.google.devtools.build.lib.exec.ModuleActionContextRegistry;
@@ -36,9 +35,10 @@
import com.google.devtools.build.lib.remote.util.DigestUtil;
import com.google.devtools.build.lib.runtime.CommandEnvironment;
import com.google.devtools.build.lib.vfs.Path;
+import java.util.function.Supplier;
import javax.annotation.Nullable;
-/** Provide a remote execution context. */
+/** Provides a remote execution context. */
final class RemoteActionContextProvider implements ExecutorLifecycleListener {
private final CommandEnvironment env;
@@ -85,7 +85,7 @@
env, cache, executor, retryScheduler, digestUtil, logDir);
}
- RemotePathResolver createRemotePathResolver() {
+ private RemotePathResolver createRemotePathResolver() {
Path execRoot = env.getExecRoot();
BuildLanguageOptions buildLanguageOptions =
env.getOptions().getOptions(BuildLanguageOptions.class);
@@ -101,7 +101,7 @@
return remotePathResolver;
}
- RemoteExecutionService getRemoteExecutionService() {
+ private RemoteExecutionService getRemoteExecutionService() {
if (remoteExecutionService == null) {
remoteExecutionService =
new RemoteExecutionService(
@@ -177,7 +177,7 @@
@Override
public void executionPhaseStarting(
- ActionGraph actionGraph, Supplier<ArtifactsToOwnerLabels> topLevelArtifactsToOwnerLabels) {}
+ ActionGraph actionGraph, Supplier<ImmutableSet<Artifact>> topLevelArtifacts) {}
@Override
public void executionPhaseEnding() {
