[7.1.0] Optimize RemoteActionFileSystem#readdir for the tree artifact…

… input case.

As explained in the comment, we don't need to fall back to other sources when the directory is determined to belong to an input tree artifact.

(This remains a valid optimization even when cherry-picked to 7.x or earlier, since two nested artifacts are always produced by the same action, so they're either both inputs or both outputs.)

PiperOrigin-RevId: 604571427
Change-Id: I7e28df8baed543e85a26041aa10c027dcd28d9ad

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

@@ -782,7 +782,7 @@ private <T extends Comparable<T>> ImmutableSortedSet<T> getDirectoryContents(
     path = resolveSymbolicLinks(path).asFragment();
 
     HashMap<String, Dirent> entries = new HashMap<>();
-    boolean found = false;
+    boolean exists = false;
 
     if (path.startsWith(execRoot)) {
       var execPath = path.relativeTo(execRoot);
@@ -791,31 +791,38 @@ private <T extends Comparable<T>> ImmutableSortedSet<T> getDirectoryContents(
         for (var entry : treeEntries) {
           entries.put(entry.getName(), entry);
         }
-        found = true;
+        exists = true;
       }
     }
 
-    if (isOutput(path)) {
+    // Since actions are assumed not to modify their inputs, a directory belonging to an input tree
+    // artifact cannot also contain an output, so we can safely skip the other sources.
+    if (!exists) {
+      if (isOutput(path)) {
+        try {
+          for (var entry : remoteOutputTree.getPath(path).readdir(Symlinks.NOFOLLOW)) {
+            entry = maybeFollowSymlinkForDirent(path, entry, followSymlinks);
+            entries.put(entry.getName(), entry);
+          }
+          exists = true;
+        } catch (FileNotFoundException ignored) {
+          // Will be rethrown below if directory does not exist in any of the sources.
+        }
+      }
+
       try {
-        for (var entry : remoteOutputTree.getPath(path).readdir(Symlinks.NOFOLLOW)) {
+        for (var entry : localFs.getPath(path).readdir(Symlinks.NOFOLLOW)) {
           entry = maybeFollowSymlinkForDirent(path, entry, followSymlinks);
           entries.put(entry.getName(), entry);
         }
-        found = true;
+        exists = true;
       } catch (FileNotFoundException ignored) {
-        // Will be rethrown below if directory exists on neither side.
+        // Will be rethrown below if directory does not exist in any of the sources.
       }
     }
 
-    try {
-      for (var entry : localFs.getPath(path).readdir(Symlinks.NOFOLLOW)) {
-        entry = maybeFollowSymlinkForDirent(path, entry, followSymlinks);
-        entries.put(entry.getName(), entry);
-      }
-    } catch (FileNotFoundException e) {
-      if (!found) {
-        throw e;
-      }
+    if (!exists) {
+      throw new FileNotFoundException(path.getPathString() + " (No such file or directory)");
     }
 
     // Sort entries to get a deterministic order.

src/test/java/com/google/devtools/build/lib/remote/RemoteActionFileSystemTest.java

@@ -658,9 +658,17 @@ public void readdir_fromInputArtifactData() throws Exception {
   }
 
   @Test
-  public void readdir_fromMultipleSources() throws Exception {
+  public void readdir_fromInputArtifactData_emptyDir() throws Exception {
+    ActionInputMap inputs = new ActionInputMap(1);
+    createLocalTreeArtifact("tree", ImmutableMap.of(), inputs);
+    RemoteActionFileSystem actionFs = (RemoteActionFileSystem) createActionFileSystem(inputs);
+
+    assertReaddir(actionFs, getOutputPath("tree"), /* followSymlinks= */ true);
+  }
+
+  @Test
+  public void readdir_fromRemoteOutputTreeAndLocalFilesystem() throws Exception {
     ActionInputMap inputs = new ActionInputMap(1);
-    createLocalTreeArtifact("dir", ImmutableMap.of("subdir/subfile", ""), inputs);
     RemoteActionFileSystem actionFs = (RemoteActionFileSystem) createActionFileSystem(inputs);
     writeLocalFile(actionFs, getOutputPath("dir/out1"), "contents1");
     injectRemoteFile(actionFs, getOutputPath("dir/out2"), "contents2");
@@ -671,8 +679,18 @@ public void readdir_fromMultipleSources() throws Exception {
         dirPath,
         /* followSymlinks= */ true,
         new Dirent("out1", Dirent.Type.FILE),
-        new Dirent("out2", Dirent.Type.FILE),
-        new Dirent("subdir", Dirent.Type.DIRECTORY));
+        new Dirent("out2", Dirent.Type.FILE));
+  }
+
+  @Test
+  public void readdir_fromRemoteOutputTreeAndLocalFilesystem_emptyDir() throws Exception {
+    ActionInputMap inputs = new ActionInputMap(1);
+    RemoteActionFileSystem actionFs = (RemoteActionFileSystem) createActionFileSystem(inputs);
+    PathFragment dirPath = getOutputPath("dir");
+    actionFs.getRemoteOutputTree().getPath(dirPath).createDirectoryAndParents();
+    actionFs.getLocalFileSystem().getPath(dirPath).createDirectoryAndParents();
+
+    assertReaddir(actionFs, dirPath, /* followSymlinks= */ true);
   }
 
   @Test