Per action file caching for input TreeArtifact.

--
MOS_MIGRATED_REVID=136475556

3 files changed, 19 insertions, 31 deletions

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 d3ffe1a7c7..9d9cafd5e3 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
@@ -24,7 +24,6 @@ import com.google.devtools.build.lib.actions.Action;
 import com.google.devtools.build.lib.actions.ActionCacheChecker.Token;
 import com.google.devtools.build.lib.actions.ActionExecutionContext;
 import com.google.devtools.build.lib.actions.ActionExecutionException;
-import com.google.devtools.build.lib.actions.ActionInputHelper;
 import com.google.devtools.build.lib.actions.AlreadyReportedActionExecutionException;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.MissingInputFileException;
@@ -609,13 +608,12 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver
             }
             expandedArtifacts.put(input, expansionBuilder.build());
           } else if (value instanceof TreeArtifactValue) {
-            TreeArtifactValue setValue = (TreeArtifactValue) value;
-            ImmutableSet<Artifact> expandedTreeArtifacts = ImmutableSet.<Artifact>copyOf(
-                ActionInputHelper.asTreeFileArtifacts(input, setValue.getChildPaths()));
+            TreeArtifactValue treeValue = (TreeArtifactValue) value;
+            expandedArtifacts.put(input, ImmutableSet.copyOf(treeValue.getChildren()));
+            inputArtifactData.putAll(treeValue.getChildValues());
 
-            expandedArtifacts.put(input, expandedTreeArtifacts);
             // Again, we cache the "digest" of the value for cache checking.
-            inputArtifactData.put(input, setValue.getSelfData());
+            inputArtifactData.put(input, treeValue.getSelfData());
           } else {
             Preconditions.checkState(value instanceof FileArtifactValue, depsEntry);
             inputArtifactData.put(input, (FileArtifactValue) value);
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PerActionFileCache.java b/src/main/java/com/google/devtools/build/lib/skyframe/PerActionFileCache.java
index c5576d9dfb..ff01f38690 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PerActionFileCache.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PerActionFileCache.java
@@ -20,12 +20,10 @@ import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.util.Preconditions;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.protobuf.ByteString;
-
 import java.io.IOException;
 import java.nio.charset.StandardCharsets;
 import java.util.HashMap;
 import java.util.Map;
-
 import javax.annotation.Nullable;
 
 /**
@@ -52,12 +50,6 @@ class PerActionFileCache implements ActionInputFileCache {
     if (!(input instanceof Artifact)) {
       return null;
     }
-
-    // TODO(rduan): Implement action file caching for TreeFileArtifacts.
-    if (((Artifact) input).hasParent()) {
-      return null;
-    }
-
     return Preconditions.checkNotNull(inputArtifactData.get(input), input);
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactValue.java
index b4fe3ee6dd..e37be07a5c 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactValue.java
@@ -13,7 +13,6 @@
 // limitations under the License.
 package com.google.devtools.build.lib.skyframe;
 
-import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Function;
 import com.google.common.base.MoreObjects;
 import com.google.common.collect.ImmutableMap;
@@ -37,7 +36,7 @@ import javax.annotation.Nullable;
  * Value for TreeArtifacts, which contains a digest and the {@link FileArtifactValue}s of its child
  * {@link TreeFileArtifact}s.
  */
-public class TreeArtifactValue implements SkyValue {
+class TreeArtifactValue implements SkyValue {
   private static final Function<Artifact, PathFragment> PARENT_RELATIVE_PATHS =
       new Function<Artifact, PathFragment>() {
         @Override
@@ -58,8 +57,7 @@ public class TreeArtifactValue implements SkyValue {
    * Returns a TreeArtifactValue out of the given Artifact-relative path fragments
    * and their corresponding FileArtifactValues.
    */
-  @VisibleForTesting
-  public static TreeArtifactValue create(Map<TreeFileArtifact, FileArtifactValue> childFileValues) {
+  static TreeArtifactValue create(Map<TreeFileArtifact, FileArtifactValue> childFileValues) {
     Map<String, Metadata> digestBuilder =
         Maps.newHashMapWithExpectedSize(childFileValues.size());
     for (Map.Entry<TreeFileArtifact, FileArtifactValue> e : childFileValues.entrySet()) {
@@ -73,29 +71,29 @@ public class TreeArtifactValue implements SkyValue {
         ImmutableMap.copyOf(childFileValues));
   }
 
-  public FileArtifactValue getSelfData() {
+  FileArtifactValue getSelfData() {
     return FileArtifactValue.createProxy(digest);
   }
 
-  public Metadata getMetadata() {
+  Metadata getMetadata() {
     return new Metadata(digest.clone());
   }
 
-  public Set<PathFragment> getChildPaths() {
+  Set<PathFragment> getChildPaths() {
     return ImmutableSet.copyOf(Iterables.transform(childData.keySet(), PARENT_RELATIVE_PATHS));
   }
 
   @Nullable
-  public byte[] getDigest() {
+  byte[] getDigest() {
     return digest.clone();
   }
 
-  public Iterable<TreeFileArtifact> getChildren() {
+  Iterable<TreeFileArtifact> getChildren() {
     return childData.keySet();
   }
 
-  public FileArtifactValue getChildValue(TreeFileArtifact artifact) {
-    return childData.get(artifact);
+  Map<TreeFileArtifact, FileArtifactValue> getChildValues() {
+    return childData;
   }
 
   @Override
@@ -136,33 +134,33 @@ public class TreeArtifactValue implements SkyValue {
   static final TreeArtifactValue MISSING_TREE_ARTIFACT = new TreeArtifactValue(null,
       ImmutableMap.<TreeFileArtifact, FileArtifactValue>of()) {
     @Override
-    public FileArtifactValue getSelfData() {
+    FileArtifactValue getSelfData() {
       throw new UnsupportedOperationException();
     }
 
     @Override
-    public Iterable<TreeFileArtifact> getChildren() {
+    Iterable<TreeFileArtifact> getChildren() {
       throw new UnsupportedOperationException();
     }
 
     @Override
-    public FileArtifactValue getChildValue(TreeFileArtifact artifact) {
+    Map<TreeFileArtifact, FileArtifactValue> getChildValues() {
       throw new UnsupportedOperationException();
     }
 
     @Override
-    public Metadata getMetadata() {
+    Metadata getMetadata() {
       throw new UnsupportedOperationException();
     }
 
     @Override
-    public Set<PathFragment> getChildPaths() {
+    Set<PathFragment> getChildPaths() {
       throw new UnsupportedOperationException();
     }
 
     @Nullable
     @Override
-    public byte[] getDigest() {
+    byte[] getDigest() {
       throw new UnsupportedOperationException();
     }