Change the signature of tree artifact expansion to allow passing a re…

…ference 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

src/main/java/com/google/devtools/build/lib/actions/Artifact.java

@@ -266,12 +266,12 @@ public MissingExpansionException(String message) {
 
   /** 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 ArtifactExpanderImpl(
     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);
       }

src/main/java/com/google/devtools/build/lib/actions/CompletionContext.java

@@ -46,15 +46,15 @@ public class CompletionContext {
 
   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;
   private final boolean fullyResolveFilesetLinks;
 
   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 @@ private CompletionContext(
   }
 
   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 @@ public void visitArtifacts(Iterable<Artifact> artifacts, ArtifactReceiver receiv
           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 ArtifactReceiver {
   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;

src/main/java/com/google/devtools/build/lib/remote/RemoteOutputService.java

@@ -133,7 +133,7 @@ public ArtifactPathResolver createPathResolverForArtifactValues(
       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(

src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java

@@ -941,7 +941,7 @@ private enum DiscoveredState {
 
   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 @@ private static class CheckInputResults {
     /** 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 @@ private static class CheckInputResults {
 
     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 static class CheckInputResults {
   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 @@ private <S extends ActionInputMapSink, R> R accumulateInputs(
     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 @@ private <S extends ActionInputMapSink, R> R accumulateInputsWithNestedSet(
     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 @@ static class ContinuationState {
     /** 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;

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 @@ private ActionInputMapHelper() {}
 
   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(
    */
   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,21 +203,19 @@ static ImmutableList<FilesetOutputSymlink> getFilesets(
   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) {
     if (TreeArtifactValue.OMITTED_TREE_MARKER.equals(value)) {
       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);
 

src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java

@@ -176,7 +176,7 @@ public SkyValue compute(SkyKey skyKey, Environment env)
             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<>();

src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java

@@ -386,7 +386,7 @@ public boolean shouldCreatePathResolverForArtifactValues() {
     @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 {

src/main/java/com/google/devtools/build/lib/vfs/OutputService.java

@@ -203,7 +203,7 @@ default ArtifactPathResolver createPathResolverForArtifactValues(
       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");