Remove unnecessary I/O operations from handling remotely executed actions (fixes performance regression #4749). Also adding Skylark tests for input/output directories.

TESTED=locally
RELNOTES: Fix performance regression
PiperOrigin-RevId: 188346410

3 files changed, 14 insertions, 11 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/remote/AbstractRemoteActionCache.java b/src/main/java/com/google/devtools/build/lib/remote/AbstractRemoteActionCache.java
index d0a05fa473..2828424d83 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/AbstractRemoteActionCache.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/AbstractRemoteActionCache.java
@@ -132,13 +132,7 @@ public abstract class AbstractRemoteActionCache implements AutoCloseable {
         downloadFile(path, file.getDigest(), file.getIsExecutable(), file.getContent());
       }
       for (OutputDirectory dir : result.getOutputDirectoriesList()) {
-        Digest treeDigest = dir.getTreeDigest();
-        byte[] b = downloadBlob(treeDigest);
-        Digest receivedTreeDigest = digestUtil.compute(b);
-        if (!receivedTreeDigest.equals(treeDigest)) {
-          throw new IOException(
-              "Digest does not match " + receivedTreeDigest + " != " + treeDigest);
-        }
+        byte[] b = downloadBlob(dir.getTreeDigest());
         Tree tree = Tree.parseFrom(b);
         Map<Digest, Directory> childrenMap = new HashMap<>();
         for (Directory child : tree.getChildrenList()) {
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java
index fc27127ec7..1b7a0809d6 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java
@@ -19,6 +19,7 @@ import com.google.common.base.Throwables;
 import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.actions.ActionInput;
 import com.google.devtools.build.lib.actions.ActionInputFileCache;
+import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.EnvironmentalExecException;
 import com.google.devtools.build.lib.actions.ExecException;
 import com.google.devtools.build.lib.actions.Spawn;
@@ -296,7 +297,7 @@ class RemoteSpawnRunner implements SpawnRunner {
     ArrayList<String> outputDirectoryPaths = new ArrayList<>();
     for (ActionInput output : outputs) {
       String pathString = output.getExecPathString();
-      if (execRoot.getRelative(pathString).isDirectory()) {
+      if (output instanceof Artifact && ((Artifact) output).isTreeArtifact()) {
         outputDirectoryPaths.add(pathString);
       } else {
         outputPaths.add(pathString);
diff --git a/src/main/java/com/google/devtools/build/lib/remote/TreeNodeRepository.java b/src/main/java/com/google/devtools/build/lib/remote/TreeNodeRepository.java
index 2d2de7e92a..5763b25e51 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/TreeNodeRepository.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/TreeNodeRepository.java
@@ -25,6 +25,7 @@ import com.google.common.collect.TreeTraverser;
 import com.google.devtools.build.lib.actions.ActionInput;
 import com.google.devtools.build.lib.actions.ActionInputFileCache;
 import com.google.devtools.build.lib.actions.ActionInputHelper;
+import com.google.devtools.build.lib.actions.DigestOfDirectoryException;
 import com.google.devtools.build.lib.actions.cache.Metadata;
 import com.google.devtools.build.lib.actions.cache.VirtualActionInput;
 import com.google.devtools.build.lib.concurrent.BlazeInterners;
@@ -311,10 +312,17 @@ public final class TreeNodeRepository extends TreeTraverser<TreeNodeRepository.T
       // Leaf node reached. Must be unique.
       Preconditions.checkArgument(
           inputsStart == inputsEnd - 1, "Encountered two inputs with the same path.");
-      // TODO: check that the actionInput is a single file!
       ActionInput input = inputs.get(inputsStart);
-      Path leafPath = execRoot.getRelative(input.getExecPathString());
-      if (leafPath.isDirectory()) {
+      try {
+        if (!(input instanceof VirtualActionInput)
+            && Preconditions.checkNotNull(inputFileCache.getMetadata(input))
+                .getType()
+                .isDirectory()) {
+          Path leafPath = execRoot.getRelative(input.getExecPathString());
+          return interner.intern(new TreeNode(buildInputDirectoryEntries(leafPath), input));
+        }
+      } catch (DigestOfDirectoryException e) {
+        Path leafPath = execRoot.getRelative(input.getExecPathString());
         return interner.intern(new TreeNode(buildInputDirectoryEntries(leafPath), input));
       }
       return interner.intern(new TreeNode(input));