Deduplicate locally executed path mapped spawns

When path mapping is enabled, different `Spawn`s in the same build can have identical `RemoteAction.ActionKey`s and can thus provide remote cache hits for each other. However, cache hits are only possible after the first local execution has concluded and uploaded its result to the cache.

To avoid unnecessary duplication of local work, the first `Spawn` for each `RemoteAction.ActionKey` is tracked until its results have been uploaded and all other concurrently scheduled `Spawn`s wait for it and then copy over its local outputs.

Fixes bazelbuild#21043

Closes bazelbuild#22556.

PiperOrigin-RevId: 655097996
Change-Id: I4368f9210c67a306775164d252aae122d8b46f9b

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

@@ -488,6 +488,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 InputStream 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/exec/AbstractSpawnStrategy.java

@@ -148,8 +148,7 @@ public ImmutableList<SpawnResult> exec(
  }
  SpawnResult spawnResult;
  ExecException ex = null;
- try {
- CacheHandle cacheHandle = cache.lookup(spawn, context);
+ try (CacheHandle cacheHandle = cache.lookup(spawn, context)) {
  if (cacheHandle.hasResult()) {
  spawnResult = Preconditions.checkNotNull(cacheHandle.getResult());
  } else {
@@ -227,7 +226,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

@@ -231,6 +231,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,7 +18,9 @@
 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 com.google.devtools.build.lib.profiler.SilentCloseable;
 import java.io.IOException;
 import java.util.NoSuchElementException;
 
@@ -51,6 +53,9 @@ public boolean willStore() {
  public void store(SpawnResult result) throws InterruptedException, IOException {
  // Do nothing.
  }
+
+ @Override
+ public void close() {}
  };
 
  /**
@@ -77,6 +82,9 @@ public boolean willStore() {
  public void store(SpawnResult result) throws InterruptedException, IOException {
  throw new IllegalStateException();
  }
+
+ @Override
+ public void close() {}
  };
  }
 
@@ -104,7 +112,7 @@ private NoSpawnCache() {}
  * to the cache after successful execution. Otherwise, if {@link #willStore} returns false, then
  * {@link #store} throws an {@link IllegalStateException}.
  */
- interface CacheHandle {
+ interface CacheHandle extends SilentCloseable {
  /** Returns whether the cache lookup was successful. */
  boolean hasResult();