spawn log optimize

Author: sluongng

src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java

@@ -141,6 +141,17 @@ public RunfilesTreeImpl(PathFragment execPath, Runfiles runfiles) {
           RunfileSymlinksMode.CREATE);
     }
 
+    @VisibleForTesting
+    public RunfilesTreeImpl(PathFragment execPath, Runfiles runfiles, boolean cacheMapping) {
+      this(
+          execPath,
+          runfiles,
+          /* repoMappingManifest= */ null,
+          /* buildRunfileLinks= */ false,
+          cacheMapping,
+          RunfileSymlinksMode.CREATE);
+    }
+
     @Override
     public PathFragment getExecPath() {
       return execPath;

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

@@ -324,11 +324,6 @@ public void logSymlinkAction(AbstractAction action) throws IOException, Interrup
     }
   }
 
-  private static boolean isTestRunnerSpawnMnemonic(String mnemonic) {
-    return TEST_RUNNER_MNEMONIC.equals(mnemonic)
-        || TEST_COVERAGE_POST_PROCESSING_MNEMONIC.equals(mnemonic);
-  }
-
   /**
    * Logs the inputs.
    *
@@ -340,11 +335,7 @@ private int logInputs(
       throws IOException, InterruptedException {
 
     return logInputSet(
-        spawn.getInputFiles(),
-        inputMetadataProvider,
-        fileSystem,
-        /* shared= */ false,
-        isTestRunnerSpawnMnemonic(spawn.getMnemonic()));
+        spawn.getInputFiles(), inputMetadataProvider, fileSystem, /* shared= */ false);
   }
 
   /**
@@ -356,29 +347,22 @@ private int logInputs(
   private int logTools(
       Spawn spawn, InputMetadataProvider inputMetadataProvider, FileSystem fileSystem)
       throws IOException, InterruptedException {
-    return logInputSet(
-        spawn.getToolFiles(),
-        inputMetadataProvider,
-        fileSystem,
-        /* shared= */ true,
-        isTestRunnerSpawnMnemonic(spawn.getMnemonic()));
+    return logInputSet(spawn.getToolFiles(), inputMetadataProvider, fileSystem, /* shared= */ true);
   }
 
   /**
    * Logs a nested set.
    *
    * @param set the nested set
    * @param shared whether this nested set is likely to be a transitive member of other sets
-   * @param isTestRunnerSpawn whether this nested set is logged for a test runner spawn
    * @return the entry ID of the {@link ExecLogEntry.InputSet} describing the nested set, or 0 if
    *     the nested set is empty.
    */
   private int logInputSet(
       NestedSet<? extends ActionInput> set,
       InputMetadataProvider inputMetadataProvider,
       FileSystem fileSystem,
-      boolean shared,
-      boolean isTestRunnerSpawn)
+      boolean shared)
       throws IOException, InterruptedException {
     if (set.isEmpty()) {
       return 0;
@@ -392,12 +376,7 @@ private int logInputSet(
           for (NestedSet<? extends ActionInput> transitive : set.getNonLeaves()) {
             checkState(!transitive.isEmpty());
             builder.addTransitiveSetIds(
-                logInputSet(
-                    transitive,
-                    inputMetadataProvider,
-                    fileSystem,
-                    /* shared= */ true,
-                    isTestRunnerSpawn));
+                logInputSet(transitive, inputMetadataProvider, fileSystem, /* shared= */ true));
           }
 
           for (ActionInput input : set.getLeaves()) {
@@ -409,11 +388,9 @@ private int logInputSet(
                       runfilesTree,
                       inputMetadataProvider,
                       fileSystem,
-                      // Runfiles of non-test spawns are tool inputs and thus potentially reused
-                      // between spawns. Runfiles of test spawns are reused if the test is attempted
-                      // multiple times in the same build; in this case, the runfiles tree caches
-                      // its mapping.
-                      !isTestRunnerSpawn || runfilesTree.isMappingCached()));
+                      // Only share runfiles trees when their mapping is cached; otherwise the
+                      // mapping can vary per spawn and should be treated as unique.
+                      runfilesTree.isMappingCached()));
               continue;
             }
 
@@ -583,10 +560,7 @@ private int logRunfilesTree(
                   fileSystem,
                   // The runfiles tree itself is shared, but the nested set is unique to the tree as
                   // it contains the executable.
-                  /* shared= */ false,
-                  // This value only matters for nested sets that may contain runfiles trees, but
-                  // these are never nested.
-                  /* isTestRunnerSpawn= */ false));
+                  /* shared= */ false));
           builder.setSymlinksId(
               logSymlinkEntries(
                   runfilesTree.getSymlinksForLogging(), inputMetadataProvider, fileSystem));

src/test/java/com/google/devtools/build/lib/exec/CompactSpawnLogContextTest.java

@@ -166,7 +166,7 @@ public void testRunfilesTreeReusedForTool() throws Exception {
     Artifact toolRunfiles = ActionsTestUtil.createRunfilesArtifact(outputDir, "tool.runfiles");
 
     PathFragment runfilesRoot = outputDir.getExecPath().getRelative("foo.runfiles");
-    RunfilesTree runfilesTree = createRunfilesTree(runfilesRoot, tool);
+    RunfilesTree runfilesTree = createCachedRunfilesTree(runfilesRoot, tool);
 
     Artifact firstInput = ActionsTestUtil.createArtifact(rootDir, "first_input");
     writeFile(firstInput, "def");

src/test/java/com/google/devtools/build/lib/exec/SpawnLogContextTestBase.java

@@ -2168,10 +2168,29 @@ protected static RunfilesTree createRunfilesTree(PathFragment root, Artifact...
     return createRunfilesTree(root, ImmutableMap.of(), ImmutableMap.of(), artifacts);
   }
 
+  protected static RunfilesTree createCachedRunfilesTree(PathFragment root, Artifact... artifacts) {
+    return createRunfilesTree(
+        root, ImmutableMap.of(), ImmutableMap.of(), /* cacheMapping= */ true, artifacts);
+  }
+
+  protected static RunfilesTree createRunfilesTree(
+      PathFragment root, boolean cacheMapping, Artifact... artifacts) {
+    return createRunfilesTree(root, ImmutableMap.of(), ImmutableMap.of(), cacheMapping, artifacts);
+  }
+
+  protected static RunfilesTree createRunfilesTree(
+      PathFragment root,
+      Map<String, Artifact> symlinks,
+      Map<String, Artifact> rootSymlinks,
+      NestedSet<Artifact> artifacts) {
+    return createRunfilesTree(root, symlinks, rootSymlinks, /* cacheMapping= */ false, artifacts);
+  }
+
   protected static RunfilesTree createRunfilesTree(
       PathFragment root,
       Map<String, Artifact> symlinks,
       Map<String, Artifact> rootSymlinks,
+      boolean cacheMapping,
       NestedSet<Artifact> artifacts) {
     Runfiles.Builder runfiles = new Runfiles.Builder(WORKSPACE_NAME);
     runfiles.addTransitiveArtifacts(artifacts);
@@ -2182,18 +2201,33 @@ protected static RunfilesTree createRunfilesTree(
       runfiles.addRootSymlink(PathFragment.create(entry.getKey()), entry.getValue());
     }
     runfiles.setEmptyFilesSupplier(BazelPyBuiltins.GET_INIT_PY_FILES);
-    return new RunfilesSupport.RunfilesTreeImpl(root, runfiles.build());
+    return new RunfilesSupport.RunfilesTreeImpl(root, runfiles.build(), cacheMapping);
+  }
+
+  protected static RunfilesTree createRunfilesTree(
+      PathFragment root,
+      Map<String, Artifact> symlinks,
+      Map<String, Artifact> rootSymlinks,
+      Artifact... artifacts) {
+    return createRunfilesTree(
+        root,
+        symlinks,
+        rootSymlinks,
+        /* cacheMapping= */ false,
+        NestedSetBuilder.wrap(Order.COMPILE_ORDER, Arrays.asList(artifacts)));
   }
 
   protected static RunfilesTree createRunfilesTree(
       PathFragment root,
       Map<String, Artifact> symlinks,
       Map<String, Artifact> rootSymlinks,
+      boolean cacheMapping,
       Artifact... artifacts) {
     return createRunfilesTree(
         root,
         symlinks,
         rootSymlinks,
+        cacheMapping,
         NestedSetBuilder.wrap(Order.COMPILE_ORDER, Arrays.asList(artifacts)));
   }