Change the signature of tree artifact expansion to allow passing a reference to
children from `TreeArtifactValue` to `ArtifactExpanderImpl`.
We currently create a copy of the set of children for each of the tree artifacts
when collecting input mappings. That can easily be avoided if we relax the
signature to allow subclasses of `Artifact`. Change the signature so that we
can take a reference to an already existing set of children in
`TreeArtifactValue` rather than copying it.
PiperOrigin-RevId: 373881983
diff --git a/src/main/java/com/google/devtools/build/lib/actions/Artifact.java b/src/main/java/com/google/devtools/build/lib/actions/Artifact.java
index 98a6b58..93497c7 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/Artifact.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/Artifact.java
@@ -266,12 +266,12 @@
/** Implementation of {@link ArtifactExpander} */
public static class ArtifactExpanderImpl implements ArtifactExpander {
- private final Map<Artifact, ImmutableCollection<Artifact>> expandedInputs;
+ private final Map<Artifact, ImmutableCollection<? extends Artifact>> expandedInputs;
private final Map<SpecialArtifact, ArchivedTreeArtifact> archivedTreeArtifacts;
private final Map<Artifact, ImmutableList<FilesetOutputSymlink>> expandedFilesets;
public ArtifactExpanderImpl(
- Map<Artifact, ImmutableCollection<Artifact>> expandedInputs,
+ Map<Artifact, ImmutableCollection<? extends Artifact>> expandedInputs,
Map<SpecialArtifact, ArchivedTreeArtifact> archivedTreeArtifacts,
Map<Artifact, ImmutableList<FilesetOutputSymlink>> expandedFilesets) {
this.expandedInputs = expandedInputs;
@@ -283,7 +283,7 @@
public void expand(Artifact artifact, Collection<? super Artifact> output) {
Preconditions.checkState(
artifact.isMiddlemanArtifact() || artifact.isTreeArtifact(), artifact);
- ImmutableCollection<Artifact> result = expandedInputs.get(artifact);
+ ImmutableCollection<? extends Artifact> result = expandedInputs.get(artifact);
if (result != null) {
output.addAll(result);
}
diff --git a/src/main/java/com/google/devtools/build/lib/actions/CompletionContext.java b/src/main/java/com/google/devtools/build/lib/actions/CompletionContext.java
index cca134d..7fb70e8 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/CompletionContext.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/CompletionContext.java
@@ -46,7 +46,7 @@
private final Path execRoot;
private final ArtifactPathResolver pathResolver;
- private final Map<Artifact, ImmutableCollection<Artifact>> expandedArtifacts;
+ private final Map<Artifact, ImmutableCollection<? extends Artifact>> expandedArtifacts;
private final Map<Artifact, ImmutableList<FilesetOutputSymlink>> expandedFilesets;
private final ActionInputMap inputMap;
private final boolean expandFilesets;
@@ -54,7 +54,7 @@
private CompletionContext(
Path execRoot,
- Map<Artifact, ImmutableCollection<Artifact>> expandedArtifacts,
+ Map<Artifact, ImmutableCollection<? extends Artifact>> expandedArtifacts,
Map<Artifact, ImmutableList<FilesetOutputSymlink>> expandedFilesets,
ArtifactPathResolver pathResolver,
ActionInputMap inputMap,
@@ -70,7 +70,7 @@
}
public static CompletionContext create(
- Map<Artifact, ImmutableCollection<Artifact>> expandedArtifacts,
+ Map<Artifact, ImmutableCollection<? extends Artifact>> expandedArtifacts,
Map<Artifact, ImmutableList<FilesetOutputSymlink>> expandedFilesets,
boolean expandFilesets,
boolean fullyResolveFilesetSymlinks,
@@ -119,7 +119,8 @@
visitFileset(artifact, receiver, fullyResolveFilesetLinks ? RESOLVE_FULLY : RESOLVE);
}
} else if (artifact.isTreeArtifact()) {
- ImmutableCollection<Artifact> expandedArtifacts = this.expandedArtifacts.get(artifact);
+ ImmutableCollection<? extends Artifact> expandedArtifacts =
+ this.expandedArtifacts.get(artifact);
for (Artifact expandedArtifact : expandedArtifacts) {
receiver.accept(expandedArtifact);
}
@@ -162,7 +163,7 @@
public interface PathResolverFactory {
ArtifactPathResolver createPathResolverForArtifactValues(
ActionInputMap actionInputMap,
- Map<Artifact, ImmutableCollection<Artifact>> expandedArtifacts,
+ Map<Artifact, ImmutableCollection<? extends Artifact>> expandedArtifacts,
Map<Artifact, ImmutableList<FilesetOutputSymlink>> filesets,
String workspaceName)
throws IOException;
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteOutputService.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteOutputService.java
index e5101ab..4bdf23a 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteOutputService.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteOutputService.java
@@ -133,7 +133,7 @@
FileSystem fileSystem,
ImmutableList<Root> pathEntries,
ActionInputMap actionInputMap,
- Map<Artifact, ImmutableCollection<Artifact>> expandedArtifacts,
+ Map<Artifact, ImmutableCollection<? extends Artifact>> expandedArtifacts,
Map<Artifact, ImmutableList<FilesetOutputSymlink>> filesets) {
FileSystem remoteFileSystem =
new RemoteActionFileSystem(
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java
index 2870d1c..e14623f 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java
@@ -941,7 +941,7 @@
private DiscoveredState addDiscoveredInputs(
ActionInputMap inputData,
- Map<Artifact, ImmutableCollection<Artifact>> expandedArtifacts,
+ Map<Artifact, ImmutableCollection<? extends Artifact>> expandedArtifacts,
Map<SpecialArtifact, ArchivedTreeArtifact> archivedTreeArtifacts,
Iterable<Artifact> discoveredInputs,
Environment env,
@@ -1051,7 +1051,7 @@
/** Metadata about Artifacts consumed by this Action. */
private final ActionInputMap actionInputMap;
/** Artifact expansion mapping for Runfiles tree and tree artifacts. */
- private final Map<Artifact, ImmutableCollection<Artifact>> expandedArtifacts;
+ private final Map<Artifact, ImmutableCollection<? extends Artifact>> expandedArtifacts;
/** Archived representations for tree artifacts. */
private final Map<SpecialArtifact, ArchivedTreeArtifact> archivedTreeArtifacts;
/** Artifact expansion mapping for Filesets embedded in Runfiles. */
@@ -1062,7 +1062,7 @@
CheckInputResults(
ActionInputMap actionInputMap,
- Map<Artifact, ImmutableCollection<Artifact>> expandedArtifacts,
+ Map<Artifact, ImmutableCollection<? extends Artifact>> expandedArtifacts,
Map<SpecialArtifact, ArchivedTreeArtifact> archivedTreeArtifacts,
Map<Artifact, ImmutableList<FilesetOutputSymlink>> filesetsInsideRunfiles,
Map<Artifact, ImmutableList<FilesetOutputSymlink>> topLevelFilesets) {
@@ -1077,7 +1077,7 @@
private interface AccumulateInputResultsFactory<S extends ActionInputMapSink, R> {
R create(
S actionInputMapSink,
- Map<Artifact, ImmutableCollection<Artifact>> expandedArtifacts,
+ Map<Artifact, ImmutableCollection<? extends Artifact>> expandedArtifacts,
Map<SpecialArtifact, ArchivedTreeArtifact> archivedTreeArtifacts,
Map<Artifact, ImmutableList<FilesetOutputSymlink>> filesetsInsideRunfiles,
Map<Artifact, ImmutableList<FilesetOutputSymlink>> topLevelFilesets);
@@ -1195,7 +1195,7 @@
ImmutableList<Artifact> allInputsList = allInputs.toList();
S inputArtifactData =
actionInputMapSinkFactory.apply(populateInputData ? allInputsList.size() : 0);
- Map<Artifact, ImmutableCollection<Artifact>> expandedArtifacts =
+ Map<Artifact, ImmutableCollection<? extends Artifact>> expandedArtifacts =
Maps.newHashMapWithExpectedSize(populateInputData ? 128 : 0);
Map<SpecialArtifact, ArchivedTreeArtifact> archivedTreeArtifacts =
Maps.newHashMapWithExpectedSize(128);
@@ -1389,7 +1389,7 @@
Map<Artifact, ImmutableList<FilesetOutputSymlink>> topLevelFilesets =
Maps.newHashMapWithExpectedSize(0);
S inputArtifactData = actionInputMapSinkFactory.apply(allInputsList.size());
- Map<Artifact, ImmutableCollection<Artifact>> expandedArtifacts =
+ Map<Artifact, ImmutableCollection<? extends Artifact>> expandedArtifacts =
Maps.newHashMapWithExpectedSize(128);
Map<SpecialArtifact, ArchivedTreeArtifact> archivedTreeArtifacts =
Maps.newHashMapWithExpectedSize(128);
@@ -1526,7 +1526,7 @@
/** Mutable map containing metadata for known artifacts. */
ActionInputMap inputArtifactData = null;
- Map<Artifact, ImmutableCollection<Artifact>> expandedArtifacts = null;
+ Map<Artifact, ImmutableCollection<? extends Artifact>> expandedArtifacts = null;
Map<SpecialArtifact, ArchivedTreeArtifact> archivedTreeArtifacts = null;
ImmutableMap<Artifact, ImmutableList<FilesetOutputSymlink>> filesetsInsideRunfiles = null;
ImmutableMap<Artifact, ImmutableList<FilesetOutputSymlink>> topLevelFilesets = null;
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionInputMapHelper.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionInputMapHelper.java
index b0f3df5..4e6afaf 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionInputMapHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionInputMapHelper.java
@@ -16,7 +16,6 @@
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableCollection;
import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
import com.google.devtools.build.lib.actions.ActionInputMapSink;
import com.google.devtools.build.lib.actions.ActionLookupData;
@@ -41,7 +40,7 @@
static void addToMap(
ActionInputMapSink inputMap,
- Map<Artifact, ImmutableCollection<Artifact>> expandedArtifacts,
+ Map<Artifact, ImmutableCollection<? extends Artifact>> expandedArtifacts,
Map<SpecialArtifact, ArchivedTreeArtifact> archivedTreeArtifacts,
Map<Artifact, ImmutableList<FilesetOutputSymlink>> filesetsInsideRunfiles,
Map<Artifact, ImmutableList<FilesetOutputSymlink>> topLevelFilesets,
@@ -67,7 +66,7 @@
*/
static void addToMap(
ActionInputMapSink inputMap,
- Map<Artifact, ImmutableCollection<Artifact>> expandedArtifacts,
+ Map<Artifact, ImmutableCollection<? extends Artifact>> expandedArtifacts,
Map<SpecialArtifact, ArchivedTreeArtifact> archivedTreeArtifacts,
Map<Artifact, ImmutableList<FilesetOutputSymlink>> filesetsInsideRunfiles,
Map<Artifact, ImmutableList<FilesetOutputSymlink>> topLevelFilesets,
@@ -204,7 +203,7 @@
private static void expandTreeArtifactAndPopulateArtifactData(
Artifact treeArtifact,
TreeArtifactValue value,
- Map<Artifact, ImmutableCollection<Artifact>> expandedArtifacts,
+ Map<Artifact, ImmutableCollection<? extends Artifact>> expandedArtifacts,
Map<SpecialArtifact, ArchivedTreeArtifact> archivedTreeArtifacts,
ActionInputMapSink inputMap,
Artifact depOwner) {
@@ -212,13 +211,11 @@
inputMap.put(treeArtifact, FileArtifactValue.OMITTED_FILE_MARKER, depOwner);
return;
}
- ImmutableSet.Builder<Artifact> children = ImmutableSet.builder();
for (Map.Entry<Artifact.TreeFileArtifact, FileArtifactValue> child :
value.getChildValues().entrySet()) {
- children.add(child.getKey());
inputMap.put(child.getKey(), child.getValue(), depOwner);
}
- expandedArtifacts.put(treeArtifact, children.build());
+ expandedArtifacts.put(treeArtifact, value.getChildren());
// Again, we cache the "digest" of the value for cache checking.
inputMap.put(treeArtifact, value.getMetadata(), depOwner);
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java
index 9361a3c..251f442 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java
@@ -176,7 +176,7 @@
SourceArtifactException.class);
ActionInputMap inputMap = new ActionInputMap(inputDeps.size());
- Map<Artifact, ImmutableCollection<Artifact>> expandedArtifacts = new HashMap<>();
+ Map<Artifact, ImmutableCollection<? extends Artifact>> expandedArtifacts = new HashMap<>();
Map<Artifact, ImmutableList<FilesetOutputSymlink>> expandedFilesets = new HashMap<>();
Map<SpecialArtifact, ArchivedTreeArtifact> archivedTreeArtifacts = new HashMap<>();
Map<Artifact, ImmutableList<FilesetOutputSymlink>> topLevelFilesets = new HashMap<>();
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 78809cb..ea71094 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
@@ -386,7 +386,7 @@
@Override
public ArtifactPathResolver createPathResolverForArtifactValues(
ActionInputMap actionInputMap,
- Map<Artifact, ImmutableCollection<Artifact>> expandedArtifacts,
+ Map<Artifact, ImmutableCollection<? extends Artifact>> expandedArtifacts,
Map<Artifact, ImmutableList<FilesetOutputSymlink>> filesets,
String workspaceName)
throws IOException {
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/OutputService.java b/src/main/java/com/google/devtools/build/lib/vfs/OutputService.java
index dd1cfce..4c9f255 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/OutputService.java
+++ b/src/main/java/com/google/devtools/build/lib/vfs/OutputService.java
@@ -203,7 +203,7 @@
FileSystem fileSystem,
ImmutableList<Root> pathEntries,
ActionInputMap actionInputMap,
- Map<Artifact, ImmutableCollection<Artifact>> expandedArtifacts,
+ Map<Artifact, ImmutableCollection<? extends Artifact>> expandedArtifacts,
Map<Artifact, ImmutableList<FilesetOutputSymlink>> filesets)
throws IOException {
throw new IllegalStateException("Path resolver not supported by this class");
