[7.4.0] Output reuse

src/main/java/com/google/devtools/build/lib/actions/ActionExecutionMetadata.java

@@ -119,4 +119,14 @@ default String getProgressMessage(RepositoryMapping mainRepositoryMapping) {
   default boolean mayInsensitivelyPropagateInputs() {
     return false;
   }
+
+  /**
+   * Returns true if the action may modify spawn outputs after the spawn has executed.
+   *
+   * <p>If this returns true, any kind of spawn output caching or reuse needs to happen
+   * synchronously directly after the spawn execution.
+   */
+  default boolean mayModifySpawnOutputsAfterExecution() {
+    return false;
+  }
 }

src/main/java/com/google/devtools/build/lib/actions/SpawnResult.java

@@ -29,7 +29,6 @@
 import com.google.errorprone.annotations.CanIgnoreReturnValue;
 import com.google.protobuf.ByteString;
 import java.io.IOException;
-import java.io.InputStream;
 import java.time.Instant;
 import java.util.Locale;
 import javax.annotation.Nullable;
@@ -38,7 +37,7 @@
 @SuppressWarnings("GoodTime") // Use ints instead of Durations to improve build time (cl/505728570)
 public interface SpawnResult {
 
-  int POSIX_TIMEOUT_EXIT_CODE = /*SIGNAL_BASE=*/ 128 + /*SIGALRM=*/ 14;
+  int POSIX_TIMEOUT_EXIT_CODE = /* SIGNAL_BASE= */ 128 + /* SIGALRM= */ 14;
 
   /** The status of the attempted Spawn execution. */
   enum Status {
@@ -262,14 +261,11 @@ default String getFailureMessage() {
    * ExecutionRequirements#REMOTE_EXECUTION_INLINE_OUTPUTS}.
    */
   @Nullable
-  default InputStream getInMemoryOutput(ActionInput output) {
+  default ByteString getInMemoryOutput(ActionInput output) {
     return null;
   }
 
-  String getDetailMessage(
-      String message,
-      boolean catastrophe,
-      boolean forciblyRunRemotely);
+  String getDetailMessage(String message, boolean catastrophe, boolean forciblyRunRemotely);
 
   /** Returns a file path to the action metadata log. */
   @Nullable
@@ -434,11 +430,8 @@ public String getFailureMessage() {
 
     @Override
     public String getDetailMessage(
-        String message,
-        boolean catastrophe,
-        boolean forciblyRunRemotely) {
-      TerminationStatus status = new TerminationStatus(
-          exitCode(), status() == Status.TIMEOUT);
+        String message, boolean catastrophe, boolean forciblyRunRemotely) {
+      TerminationStatus status = new TerminationStatus(exitCode(), status() == Status.TIMEOUT);
       String reason = "(" + status.toShortString() + ")"; // e.g. "(Exit 1)"
       String explanation = Strings.isNullOrEmpty(message) ? "" : ": " + message;
 
@@ -457,17 +450,18 @@ public String getDetailMessage(
         explanation += " (Remote action was terminated due to Out of Memory.)";
       }
       if (status() != Status.TIMEOUT && forciblyRunRemotely) {
-        explanation += " Action tagged as local was forcibly run remotely and failed - it's "
-            + "possible that the action simply doesn't work remotely";
+        explanation +=
+            " Action tagged as local was forcibly run remotely and failed - it's "
+                + "possible that the action simply doesn't work remotely";
       }
       return reason + explanation;
     }
 
     @Nullable
     @Override
-    public InputStream getInMemoryOutput(ActionInput output) {
+    public ByteString getInMemoryOutput(ActionInput output) {
       if (inMemoryOutputFile != null && inMemoryOutputFile.equals(output)) {
-        return inMemoryContents.newInput();
+        return inMemoryContents;
       }
       return null;
     }
@@ -488,6 +482,149 @@ public Digest getDigest() {
     }
   }
 
+  /**
+   * A helper class for wrapping an existing {@link SpawnResult} and modifying a subset of its
+   * methods.
+   */
+  class DelegateSpawnResult implements SpawnResult {
+    private final SpawnResult delegate;
+
+    public DelegateSpawnResult(SpawnResult delegate) {
+      this.delegate = delegate;
+    }
+
+    @Override
+    public boolean setupSuccess() {
+      return delegate.setupSuccess();
+    }
+
+    @Override
+    public boolean isCatastrophe() {
+      return delegate.isCatastrophe();
+    }
+
+    @Override
+    public Status status() {
+      return delegate.status();
+    }
+
+    @Override
+    public int exitCode() {
+      return delegate.exitCode();
+    }
+
+    @Override
+    @Nullable
+    public FailureDetail failureDetail() {
+      return delegate.failureDetail();
+    }
+
+    @Override
+    @Nullable
+    public String getExecutorHostName() {
+      return delegate.getExecutorHostName();
+    }
+
+    @Override
+    public String getRunnerName() {
+      return delegate.getRunnerName();
+    }
+
+    @Override
+    public String getRunnerSubtype() {
+      return delegate.getRunnerSubtype();
+    }
+
+    @Override
+    @Nullable
+    public Instant getStartTime() {
+      return delegate.getStartTime();
+    }
+
+    @Override
+    public int getWallTimeInMs() {
+      return delegate.getWallTimeInMs();
+    }
+
+    @Override
+    public int getUserTimeInMs() {
+      return delegate.getUserTimeInMs();
+    }
+
+    @Override
+    public int getSystemTimeInMs() {
+      return delegate.getSystemTimeInMs();
+    }
+
+    @Override
+    @Nullable
+    public Long getNumBlockOutputOperations() {
+      return delegate.getNumBlockOutputOperations();
+    }
+
+    @Override
+    @Nullable
+    public Long getNumBlockInputOperations() {
+      return delegate.getNumBlockInputOperations();
+    }
+
+    @Override
+    @Nullable
+    public Long getNumInvoluntaryContextSwitches() {
+      return delegate.getNumInvoluntaryContextSwitches();
+    }
+
+    @Override
+    @Nullable
+    public Long getMemoryInKb() {
+      return delegate.getMemoryInKb();
+    }
+
+    @Override
+    public SpawnMetrics getMetrics() {
+      return delegate.getMetrics();
+    }
+
+    @Override
+    public boolean isCacheHit() {
+      return delegate.isCacheHit();
+    }
+
+    @Override
+    public String getFailureMessage() {
+      return delegate.getFailureMessage();
+    }
+
+    @Override
+    @Nullable
+    public ByteString getInMemoryOutput(ActionInput output) {
+      return delegate.getInMemoryOutput(output);
+    }
+
+    @Override
+    public String getDetailMessage(
+        String message, boolean catastrophe, boolean forciblyRunRemotely) {
+      return delegate.getDetailMessage(message, catastrophe, forciblyRunRemotely);
+    }
+
+    @Override
+    @Nullable
+    public MetadataLog getActionMetadataLog() {
+      return delegate.getActionMetadataLog();
+    }
+
+    @Override
+    public boolean wasRemote() {
+      return delegate.wasRemote();
+    }
+
+    @Override
+    @Nullable
+    public Digest getDigest() {
+      return delegate.getDigest();
+    }
+  }
+
   /** Builder class for {@link SpawnResult}. */
   final class Builder {
     private int exitCode;

src/main/java/com/google/devtools/build/lib/analysis/actions/StarlarkAction.java

@@ -47,6 +47,7 @@
 import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
 import com.google.devtools.build.lib.server.FailureDetails.StarlarkAction.Code;
 import com.google.errorprone.annotations.CanIgnoreReturnValue;
+import com.google.protobuf.ByteString;
 import java.io.BufferedReader;
 import java.io.FileNotFoundException;
 import java.io.IOException;
@@ -315,9 +316,9 @@ private InputStream getUnusedInputListInputStream(
       // Note: SpawnActionContext guarantees that the first list entry exists and corresponds to the
       // executed spawn.
       Artifact unusedInputsListArtifact = unusedInputsList.get();
-      InputStream inputStream = spawnResults.get(0).getInMemoryOutput(unusedInputsListArtifact);
-      if (inputStream != null) {
-        return inputStream;
+      ByteString content = spawnResults.get(0).getInMemoryOutput(unusedInputsListArtifact);
+      if (content != null) {
+        return content.newInput();
       }
       // Fallback to reading from disk.
       try {

src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java

@@ -309,6 +309,16 @@ private static ImmutableSet<Artifact> nonNullAsSet(Artifact... artifacts) {
     this.isExecutedOnWindows = isExecutedOnWindows;
   }
 
+  @Override
+  public boolean mayModifySpawnOutputsAfterExecution() {
+    // Test actions modify test spawn outputs after execution:
+    // - if there are multiple attempts (unavoidable);
+    // - in all cases due to appending any stray stderr output to the test log in
+    //   StandaloneTestStrategy.
+    // TODO: Get rid of the second case and only return true if there are multiple attempts.
+    return true;
+  }
+
   public boolean isExecutedOnWindows() {
     return isExecutedOnWindows;
   }

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

@@ -221,7 +221,7 @@ public ImmutableList<SpawnResult> exec(
               ? resultMessage
               : CommandFailureUtils.describeCommandFailure(
                   executionOptions.verboseFailures, cwd, spawn);
-      throw new SpawnExecException(message, spawnResult, /*forciblyRunRemotely=*/ false);
+      throw new SpawnExecException(message, spawnResult, /* forciblyRunRemotely= */ false);
     }
     return ImmutableList.of(spawnResult);
   }

src/main/java/com/google/devtools/build/lib/exec/BUILD

@@ -225,6 +225,7 @@ java_library(
     deps = [
         ":spawn_runner",
         "//src/main/java/com/google/devtools/build/lib/actions",
+        "//src/main/java/com/google/devtools/build/lib/profiler",
     ],
 )
 

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

@@ -18,6 +18,7 @@
 import com.google.devtools.build.lib.actions.ForbiddenActionInputException;
 import com.google.devtools.build.lib.actions.Spawn;
 import com.google.devtools.build.lib.actions.SpawnResult;
+import com.google.devtools.build.lib.actions.Spawns;
 import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionContext;
 import java.io.Closeable;
 import java.io.IOException;

src/main/java/com/google/devtools/build/lib/includescanning/SpawnIncludeScanner.java

@@ -52,6 +52,7 @@
 import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.build.lib.vfs.Symlinks;
 import com.google.devtools.build.lib.vfs.SyscallCache;
+import com.google.protobuf.ByteString;
 import java.io.IOException;
 import java.io.InputStream;
 import java.util.Collection;
@@ -351,6 +352,7 @@ public Collection<Inclusion> extractInclusions(
    *     Otherwise "null"
    * @throws ExecException if scanning fails
    */
+  @Nullable
   private static InputStream spawnGrep(
       Artifact input,
       PathFragment outputExecPath,
@@ -412,7 +414,11 @@ private static InputStream spawnGrep(
     }
 
     SpawnResult result = Iterables.getLast(results);
-    return result.getInMemoryOutput(output);
+    ByteString includesContent = result.getInMemoryOutput(output);
+    if (includesContent != null) {
+      return includesContent.newInput();
+    }
+    return null;
   }
 
   private static void dump(ActionExecutionContext fromContext, ActionExecutionContext toContext) {