[7.1.0] Avoid unnecessary overhead when determining whether an action…

… input is a directory.

When building Bazel against a fully populated disk cache, this halves the CPU overhead incurred by the compact log (4% to 2%), mostly due to avoiding symlink resolution in the RemoteActionFileSystem.

PiperOrigin-RevId: 596938086
Change-Id: I6d548894fad5bf97f6a54516c5dca87d0b8d7a8f

src/main/java/com/google/devtools/build/lib/exec/CompactSpawnLogContext.java

@@ -18,6 +18,7 @@
 import static com.google.devtools.build.lib.exec.SpawnLogContext.getEnvironmentVariables;
 import static com.google.devtools.build.lib.exec.SpawnLogContext.getPlatform;
 import static com.google.devtools.build.lib.exec.SpawnLogContext.getSpawnMetricsProto;
+import static com.google.devtools.build.lib.exec.SpawnLogContext.isInputDirectory;
 
 import com.github.luben.zstd.ZstdOutputStream;
 import com.google.common.collect.ImmutableList;
@@ -274,7 +275,7 @@ private int logNestedSet(
               continue;
             }
             Path path = fileSystem.getPath(execRoot.getRelative(input.getExecPath()));
-            if (path.isDirectory()) {
+            if (isInputDirectory(input, inputMetadataProvider)) {
               builder.addDirectoryIds(logDirectory(input, path, inputMetadataProvider));
             } else {
               builder.addFileIds(logFile(input, path, inputMetadataProvider));
@@ -373,7 +374,7 @@ private int logRunfilesDirectory(
 
             Path path = fileSystem.getPath(execRoot.getRelative(input.getExecPath()));
 
-            if (path.isDirectory()) {
+            if (isInputDirectory(input, inputMetadataProvider)) {
               builder.addAllFiles(expandDirectory(path, runfilesPath, inputMetadataProvider));
               continue;
             }

src/main/java/com/google/devtools/build/lib/exec/ExpandedSpawnLogContext.java

@@ -17,6 +17,7 @@
 import static com.google.devtools.build.lib.exec.SpawnLogContext.getEnvironmentVariables;
 import static com.google.devtools.build.lib.exec.SpawnLogContext.getPlatform;
 import static com.google.devtools.build.lib.exec.SpawnLogContext.getSpawnMetricsProto;
+import static com.google.devtools.build.lib.exec.SpawnLogContext.isInputDirectory;
 
 import com.google.common.collect.ImmutableSet;
 import com.google.common.flogger.GoogleLogger;
@@ -168,7 +169,7 @@ public void logSpawn(
 
           Path contentPath = fileSystem.getPath(execRoot.getRelative(input.getExecPathString()));
 
-          if (contentPath.isDirectory()) {
+          if (isInputDirectory(input, inputMetadataProvider)) {
             listDirectoryContents(
                 displayPath, contentPath, builder::addInputs, inputMetadataProvider, isTool);
             continue;

src/main/java/com/google/devtools/build/lib/exec/SpawnLogContext.java

@@ -13,13 +13,16 @@
 // limitations under the License.
 package com.google.devtools.build.lib.exec;
 
+import static com.google.common.base.Preconditions.checkNotNull;
+
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSortedMap;
 import com.google.common.hash.HashCode;
 import com.google.devtools.build.lib.actions.ActionContext;
 import com.google.devtools.build.lib.actions.ActionInput;
+import com.google.devtools.build.lib.actions.Artifact.SourceArtifact;
 import com.google.devtools.build.lib.actions.ExecException;
 import com.google.devtools.build.lib.actions.FileArtifactValue;
 import com.google.devtools.build.lib.actions.InputMetadataProvider;
@@ -102,6 +105,29 @@ static Platform getPlatform(Spawn spawn, RemoteOptions remoteOptions) throws Use
     return builder.build();
   }
 
+  /**
+   * Determines whether an action input is a directory, avoiding I/O if possible.
+   *
+   * <p>Do not call for action outputs.
+   */
+  static boolean isInputDirectory(ActionInput input, InputMetadataProvider inputMetadataProvider)
+      throws IOException {
+    if (input.isDirectory()) {
+      return true;
+    }
+    if (!(input instanceof SourceArtifact)) {
+      return false;
+    }
+    // A source artifact may be a directory in spite of claiming to be a file. Avoid unnecessary I/O
+    // by inspecting its metadata, which should have already been collected and cached.
+    FileArtifactValue metadata =
+        checkNotNull(
+            inputMetadataProvider.getInputMetadata(input),
+            "missing metadata for %s",
+            input.getExecPath());
+    return metadata.getType().isDirectory();
+  }
+
   /**
    * Computes the digest of an ActionInput or its path.
    *