diff --git a/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java index f7fd7b3ea6ca38..01ea75faba166f 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java @@ -185,7 +185,7 @@ public ImmutableList exec( spawnLogContext.logSpawn( spawn, actionExecutionContext.getMetadataProvider(), - context.getInputMapping(PathFragment.EMPTY_FRAGMENT), + context.getInputMapping(PathFragment.EMPTY_FRAGMENT, /* willAccessRepeatedly= */ false), context.getTimeout(), spawnResult); } catch (IOException | ForbiddenActionInputException e) { @@ -246,7 +246,9 @@ public ListenableFuture prefetchInputs() return actionExecutionContext .getActionInputPrefetcher() .prefetchFiles( - getInputMapping(PathFragment.EMPTY_FRAGMENT).values(), getMetadataProvider()); + getInputMapping(PathFragment.EMPTY_FRAGMENT, /* willAccessRepeatedly= */ true) + .values(), + getMetadataProvider()); } return immediateVoidFuture(); @@ -306,22 +308,33 @@ public FileOutErr getFileOutErr() { } @Override - public SortedMap getInputMapping(PathFragment baseDirectory) + public SortedMap getInputMapping( + PathFragment baseDirectory, boolean willAccessRepeatedly) throws IOException, ForbiddenActionInputException { - if (lazyInputMapping == null || !inputMappingBaseDirectory.equals(baseDirectory)) { - try (SilentCloseable c = - Profiler.instance().profile("AbstractSpawnStrategy.getInputMapping")) { - inputMappingBaseDirectory = baseDirectory; - lazyInputMapping = - spawnInputExpander.getInputMapping( - spawn, - actionExecutionContext.getArtifactExpander(), - baseDirectory, - actionExecutionContext.getMetadataProvider()); - } + // Return previously computed copy if present. + if (lazyInputMapping != null && inputMappingBaseDirectory.equals(baseDirectory)) { + return lazyInputMapping; + } + + SortedMap inputMapping; + try (SilentCloseable c = + Profiler.instance().profile("AbstractSpawnStrategy.getInputMapping")) { + inputMapping = + spawnInputExpander.getInputMapping( + spawn, + actionExecutionContext.getArtifactExpander(), + baseDirectory, + actionExecutionContext.getMetadataProvider()); } - return lazyInputMapping; + // Don't cache the input mapping if it is unlikely that it is used again. + // This reduces memory usage in the case where remote caching/execution is + // used, and the expected cache hit rate is high. + if (willAccessRepeatedly) { + inputMappingBaseDirectory = baseDirectory; + lazyInputMapping = inputMapping; + } + return inputMapping; } @Override diff --git a/src/main/java/com/google/devtools/build/lib/exec/SpawnRunner.java b/src/main/java/com/google/devtools/build/lib/exec/SpawnRunner.java index ee87201b1b1f79..ccfc3c8146043a 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/SpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/exec/SpawnRunner.java @@ -251,7 +251,8 @@ void lockOutputFiles(int exitCode, String errorMessage, FileOutErr outErr) * mapping is used in a context where the directory relative to which the keys are interpreted * is not the same as the execroot. */ - SortedMap getInputMapping(PathFragment baseDirectory) + SortedMap getInputMapping( + PathFragment baseDirectory, boolean willAccessRepeatedly) throws IOException, ForbiddenActionInputException; /** Reports a progress update to the Spawn strategy. */ diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteAction.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteAction.java index e680ea484d818c..5e5774832ee04e 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteAction.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteAction.java @@ -28,6 +28,7 @@ import com.google.devtools.build.lib.vfs.PathFragment; import java.io.IOException; import java.util.SortedMap; +import javax.annotation.Nullable; /** A value class representing an action which can be executed remotely. */ public class RemoteAction { @@ -36,7 +37,9 @@ public class RemoteAction { private final SpawnExecutionContext spawnExecutionContext; private final RemoteActionExecutionContext remoteActionExecutionContext; private final RemotePathResolver remotePathResolver; - private final MerkleTree merkleTree; + @Nullable private final MerkleTree merkleTree; + private final long inputBytes; + private final long inputFiles; private final Digest commandHash; private final Command command; private final Action action; @@ -51,12 +54,15 @@ public class RemoteAction { Digest commandHash, Command command, Action action, - ActionKey actionKey) { + ActionKey actionKey, + boolean remoteDiscardMerkleTrees) { this.spawn = spawn; this.spawnExecutionContext = spawnExecutionContext; this.remoteActionExecutionContext = remoteActionExecutionContext; this.remotePathResolver = remotePathResolver; - this.merkleTree = merkleTree; + this.merkleTree = remoteDiscardMerkleTrees ? null : merkleTree; + this.inputBytes = merkleTree.getInputBytes(); + this.inputFiles = merkleTree.getInputFiles(); this.commandHash = commandHash; this.command = command; this.action = action; @@ -80,12 +86,12 @@ public Spawn getSpawn() { * Returns the sum of file sizes plus protobuf sizes used to represent the inputs of this action. */ public long getInputBytes() { - return merkleTree.getInputBytes(); + return inputBytes; } /** Returns the number of input files of this action. */ public long getInputFiles() { - return merkleTree.getInputFiles(); + return inputFiles; } /** Returns the id this is action. */ @@ -111,6 +117,7 @@ public Command getCommand() { return command; } + @Nullable public MerkleTree getMerkleTree() { return merkleTree; } @@ -119,9 +126,9 @@ public MerkleTree getMerkleTree() { * Returns a {@link SortedMap} which maps from input paths for remote action to {@link * ActionInput}. */ - public SortedMap getInputMap() + public SortedMap getInputMap(boolean willAccessRepeatedly) throws IOException, ForbiddenActionInputException { - return remotePathResolver.getInputMapping(spawnExecutionContext); + return remotePathResolver.getInputMapping(spawnExecutionContext, willAccessRepeatedly); } /** diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java index ffa3871a369970..2f7099a1730fe5 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java @@ -142,6 +142,7 @@ import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.Executor; import java.util.concurrent.Phaser; +import java.util.concurrent.Semaphore; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Predicate; import javax.annotation.Nullable; @@ -377,7 +378,9 @@ private MerkleTree buildInputMerkleTree( } return MerkleTree.merge(subMerkleTrees, digestUtil); } else { - SortedMap inputMap = remotePathResolver.getInputMapping(context); + SortedMap inputMap = + remotePathResolver.getInputMapping( + context, /* willAccessRepeatedly= */ !remoteOptions.remoteDiscardMerkleTrees); if (!outputDirMap.isEmpty()) { // The map returned by getInputMapping is mutable, but must not be mutated here as it is // shared with all other strategies. @@ -436,63 +439,90 @@ private static ByteString buildSalt(Spawn spawn) { return null; } + /** + * Semaphore for limiting the concurrent number of Merkle tree input roots we compute and keep in + * memory. + * + *

When --jobs is set to a high value to let the remote execution service runs many actions in + * parallel, there is no point in letting the local system compute Merkle trees of input roots + * with the same amount of parallelism. Not only does this make Bazel feel sluggish and slow to + * respond to being interrupted, it causes it to exhaust memory. + * + *

As there is no point in letting Merkle tree input root computation use a higher concurrency + * than the number of CPUs in the system, use a semaphore to limit the concurrency of + * buildRemoteAction(). + */ + private final Semaphore remoteActionBuildingSemaphore = + new Semaphore(Runtime.getRuntime().availableProcessors(), true); + + @Nullable + private ToolSignature getToolSignature(Spawn spawn, SpawnExecutionContext context) + throws IOException, ExecException, InterruptedException { + return remoteOptions.markToolInputs + && Spawns.supportsWorkers(spawn) + && !spawn.getToolFiles().isEmpty() + ? computePersistentWorkerSignature(spawn, context) + : null; + } + /** Creates a new {@link RemoteAction} instance from spawn. */ public RemoteAction buildRemoteAction(Spawn spawn, SpawnExecutionContext context) throws IOException, ExecException, ForbiddenActionInputException, InterruptedException { - ToolSignature toolSignature = - remoteOptions.markToolInputs - && Spawns.supportsWorkers(spawn) - && !spawn.getToolFiles().isEmpty() - ? computePersistentWorkerSignature(spawn, context) - : null; - final MerkleTree merkleTree = buildInputMerkleTree(spawn, context, toolSignature); - - // Get the remote platform properties. - Platform platform = PlatformUtils.getPlatformProto(spawn, remoteOptions); - if (toolSignature != null) { - platform = - PlatformUtils.getPlatformProto( - spawn, remoteOptions, ImmutableMap.of("persistentWorkerKey", toolSignature.key)); - } else { - platform = PlatformUtils.getPlatformProto(spawn, remoteOptions); - } - - Command command = - buildCommand( - spawn.getOutputFiles(), - spawn.getArguments(), - spawn.getEnvironment(), - platform, - remotePathResolver); - Digest commandHash = digestUtil.compute(command); - Action action = - Utils.buildAction( - commandHash, - merkleTree.getRootDigest(), - platform, - context.getTimeout(), - Spawns.mayBeCachedRemotely(spawn), - buildSalt(spawn)); - - ActionKey actionKey = digestUtil.computeActionKey(action); - - RequestMetadata metadata = - TracingMetadataUtils.buildMetadata( - buildRequestId, commandId, actionKey.getDigest().getHash(), spawn.getResourceOwner()); - RemoteActionExecutionContext remoteActionExecutionContext = - RemoteActionExecutionContext.create( - spawn, metadata, getWriteCachePolicy(spawn), getReadCachePolicy(spawn)); - - return new RemoteAction( - spawn, - context, - remoteActionExecutionContext, - remotePathResolver, - merkleTree, - commandHash, - command, - action, - actionKey); + remoteActionBuildingSemaphore.acquire(); + try { + ToolSignature toolSignature = getToolSignature(spawn, context); + final MerkleTree merkleTree = buildInputMerkleTree(spawn, context, toolSignature); + + // Get the remote platform properties. + Platform platform = PlatformUtils.getPlatformProto(spawn, remoteOptions); + if (toolSignature != null) { + platform = + PlatformUtils.getPlatformProto( + spawn, remoteOptions, ImmutableMap.of("persistentWorkerKey", toolSignature.key)); + } else { + platform = PlatformUtils.getPlatformProto(spawn, remoteOptions); + } + + Command command = + buildCommand( + spawn.getOutputFiles(), + spawn.getArguments(), + spawn.getEnvironment(), + platform, + remotePathResolver); + Digest commandHash = digestUtil.compute(command); + Action action = + Utils.buildAction( + commandHash, + merkleTree.getRootDigest(), + platform, + context.getTimeout(), + Spawns.mayBeCachedRemotely(spawn), + buildSalt(spawn)); + + ActionKey actionKey = digestUtil.computeActionKey(action); + + RequestMetadata metadata = + TracingMetadataUtils.buildMetadata( + buildRequestId, commandId, actionKey.getDigest().getHash(), spawn.getResourceOwner()); + RemoteActionExecutionContext remoteActionExecutionContext = + RemoteActionExecutionContext.create( + spawn, metadata, getWriteCachePolicy(spawn), getReadCachePolicy(spawn)); + + return new RemoteAction( + spawn, + context, + remoteActionExecutionContext, + remotePathResolver, + merkleTree, + commandHash, + command, + action, + actionKey, + remoteOptions.remoteDiscardMerkleTrees); + } finally { + remoteActionBuildingSemaphore.release(); + } } @Nullable @@ -1338,7 +1368,7 @@ private void reportUploadError(Throwable error) { *

Must be called before calling {@link #executeRemotely}. */ public void uploadInputsIfNotPresent(RemoteAction action, boolean force) - throws IOException, InterruptedException { + throws IOException, ExecException, ForbiddenActionInputException, InterruptedException { checkState(!shutdown.get(), "shutdown"); checkState(mayBeExecutedRemotely(action.getSpawn()), "spawn can't be executed remotely"); @@ -1347,13 +1377,33 @@ public void uploadInputsIfNotPresent(RemoteAction action, boolean force) Map additionalInputs = Maps.newHashMapWithExpectedSize(2); additionalInputs.put(action.getActionKey().getDigest(), action.getAction()); additionalInputs.put(action.getCommandHash(), action.getCommand()); - remoteExecutionCache.ensureInputsPresent( - action - .getRemoteActionExecutionContext() - .withWriteCachePolicy(CachePolicy.REMOTE_CACHE_ONLY), // Only upload to remote cache - action.getMerkleTree(), - additionalInputs, - force); + + // As uploading depends on having the full input root in memory, limit + // concurrency. This prevents memory exhaustion. We assume that + // ensureInputsPresent() provides enough parallelism to saturate the + // network connection. + remoteActionBuildingSemaphore.acquire(); + try { + MerkleTree merkleTree = action.getMerkleTree(); + if (merkleTree == null) { + // --experimental_remote_discard_merkle_trees was provided. + // Recompute the input root. + Spawn spawn = action.getSpawn(); + SpawnExecutionContext context = action.getSpawnExecutionContext(); + ToolSignature toolSignature = getToolSignature(spawn, context); + merkleTree = buildInputMerkleTree(spawn, context, toolSignature); + } + + remoteExecutionCache.ensureInputsPresent( + action + .getRemoteActionExecutionContext() + .withWriteCachePolicy(CachePolicy.REMOTE_CACHE_ONLY), // Only upload to remote cache + merkleTree, + additionalInputs, + force); + } finally { + remoteActionBuildingSemaphore.release(); + } } /** diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java index 8da2b2b0455411..09d821a0108e11 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java @@ -202,7 +202,7 @@ public void close() {} private void checkForConcurrentModifications() throws IOException, ForbiddenActionInputException { - for (ActionInput input : action.getInputMap().values()) { + for (ActionInput input : action.getInputMap(true).values()) { if (input instanceof VirtualActionInput) { continue; } diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java index aa2557d3770dd5..49f2b6b73295ae 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java @@ -621,9 +621,9 @@ private Map getInputCtimes(SortedMap inpu SpawnResult execLocallyAndUpload( RemoteAction action, Spawn spawn, SpawnExecutionContext context, boolean uploadLocalResults) throws ExecException, IOException, ForbiddenActionInputException, InterruptedException { - Map ctimesBefore = getInputCtimes(action.getInputMap()); + Map ctimesBefore = getInputCtimes(action.getInputMap(true)); SpawnResult result = execLocally(spawn, context); - Map ctimesAfter = getInputCtimes(action.getInputMap()); + Map ctimesAfter = getInputCtimes(action.getInputMap(true)); uploadLocalResults = uploadLocalResults && Status.SUCCESS.equals(result.status()) && result.exitCode() == 0; if (!uploadLocalResults) { diff --git a/src/main/java/com/google/devtools/build/lib/remote/common/RemotePathResolver.java b/src/main/java/com/google/devtools/build/lib/remote/common/RemotePathResolver.java index d2c1c2c5448a38..5b2bde1377ef60 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/common/RemotePathResolver.java +++ b/src/main/java/com/google/devtools/build/lib/remote/common/RemotePathResolver.java @@ -42,7 +42,8 @@ public interface RemotePathResolver { * Returns a {@link SortedMap} which maps from input paths for remote action to {@link * ActionInput}. */ - SortedMap getInputMapping(SpawnExecutionContext context) + SortedMap getInputMapping( + SpawnExecutionContext context, boolean willAccessRepeatedly) throws IOException, ForbiddenActionInputException; void walkInputs( @@ -100,9 +101,10 @@ public String getWorkingDirectory() { } @Override - public SortedMap getInputMapping(SpawnExecutionContext context) + public SortedMap getInputMapping( + SpawnExecutionContext context, boolean willAccessRepeatedly) throws IOException, ForbiddenActionInputException { - return context.getInputMapping(PathFragment.EMPTY_FRAGMENT); + return context.getInputMapping(PathFragment.EMPTY_FRAGMENT, willAccessRepeatedly); } @Override @@ -171,12 +173,14 @@ public String getWorkingDirectory() { } @Override - public SortedMap getInputMapping(SpawnExecutionContext context) + public SortedMap getInputMapping( + SpawnExecutionContext context, boolean willAccessRepeatedly) throws IOException, ForbiddenActionInputException { // The "root directory" of the action from the point of view of RBE is the parent directory of // the execroot locally. This is so that paths of artifacts in external repositories don't // start with an uplevel reference. - return context.getInputMapping(PathFragment.create(checkNotNull(getWorkingDirectory()))); + return context.getInputMapping( + PathFragment.create(checkNotNull(getWorkingDirectory())), willAccessRepeatedly); } @Override diff --git a/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java b/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java index 0fc54e09557313..c812a423aa3f77 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java +++ b/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java @@ -646,6 +646,18 @@ public RemoteOutputsStrategyConverter() { + "can be used to implement remote persistent workers.") public boolean markToolInputs; + @Option( + name = "experimental_remote_discard_merkle_trees", + defaultValue = "false", + documentationCategory = OptionDocumentationCategory.REMOTE, + effectTags = {OptionEffectTag.UNKNOWN}, + help = + "If set to true, discard in-memory copies of the input root's Merkle tree and associated " + + "input mappings during calls to GetActionResult() and Execute(). This reduces " + + "memory usage significantly, but does require Bazel to recompute them upon remote " + + "cache misses and retries.") + public boolean remoteDiscardMerkleTrees; + // The below options are not configurable by users, only tests. // This is part of the effort to reduce the overall number of flags. diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedSpawnRunner.java index 630a4551485e7f..6145855522621d 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedSpawnRunner.java @@ -232,7 +232,7 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context SandboxInputs inputs = helpers.processInputFiles( - context.getInputMapping(PathFragment.EMPTY_FRAGMENT), + context.getInputMapping(PathFragment.EMPTY_FRAGMENT, /* willAccessRepeatedly= */ true), execRoot); SandboxOutputs outputs = helpers.getOutputs(spawn); diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/DockerSandboxedSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/DockerSandboxedSpawnRunner.java index 1a46186e76c991..68490bf7739ecd 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/DockerSandboxedSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/DockerSandboxedSpawnRunner.java @@ -222,7 +222,7 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context SandboxInputs inputs = helpers.processInputFiles( - context.getInputMapping(PathFragment.EMPTY_FRAGMENT), + context.getInputMapping(PathFragment.EMPTY_FRAGMENT, /* willAccessRepeatedly= */ true), execRoot); SandboxOutputs outputs = helpers.getOutputs(spawn); diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java index 89eb4a70f0668f..a50f4ba89f48be 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java @@ -218,7 +218,9 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context ImmutableSet writableDirs = getWritableDirs(sandboxExecRoot, environment); SandboxInputs inputs = - helpers.processInputFiles(context.getInputMapping(PathFragment.EMPTY_FRAGMENT), execRoot); + helpers.processInputFiles( + context.getInputMapping(PathFragment.EMPTY_FRAGMENT, /* willAccessRepeatedly= */ true), + execRoot); SandboxOutputs outputs = helpers.getOutputs(spawn); Duration timeout = context.getTimeout(); @@ -437,7 +439,10 @@ public void verifyPostCondition( private void checkForConcurrentModifications(SpawnExecutionContext context) throws IOException, ForbiddenActionInputException { - for (ActionInput input : context.getInputMapping(PathFragment.EMPTY_FRAGMENT).values()) { + for (ActionInput input : + context + .getInputMapping(PathFragment.EMPTY_FRAGMENT, /* willAccessRepeatedly= */ true) + .values()) { if (input instanceof VirtualActionInput) { // Virtual inputs are not existing in file system and can't be tampered with via sandbox. No // need to check them. diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedSpawnRunner.java index 6b6835eff2b549..5f2a2442f8b0b4 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedSpawnRunner.java @@ -111,7 +111,7 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context SandboxInputs inputs = helpers.processInputFiles( - context.getInputMapping(PathFragment.EMPTY_FRAGMENT), + context.getInputMapping(PathFragment.EMPTY_FRAGMENT, /* willAccessRepeatedly= */ true), execRoot); SandboxOutputs outputs = helpers.getOutputs(spawn); diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/WindowsSandboxedSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/WindowsSandboxedSpawnRunner.java index 467aa6e5c868ae..7b200365bd972a 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/WindowsSandboxedSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/WindowsSandboxedSpawnRunner.java @@ -71,7 +71,7 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context SandboxInputs readablePaths = helpers.processInputFiles( - context.getInputMapping(PathFragment.EMPTY_FRAGMENT), + context.getInputMapping(PathFragment.EMPTY_FRAGMENT, /* willAccessRepeatedly= */ true), execRoot); ImmutableSet.Builder writablePaths = ImmutableSet.builder(); diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnRunner.java index 7b82c7a94236c8..0f7097993a4745 100644 --- a/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnRunner.java @@ -188,7 +188,9 @@ public SpawnResult exec(Spawn spawn, SpawnExecutionContext context) Profiler.instance().profile(ProfilerTask.WORKER_SETUP, "Setting up inputs")) { inputFiles = helpers.processInputFiles( - context.getInputMapping(PathFragment.EMPTY_FRAGMENT), execRoot); + context.getInputMapping( + PathFragment.EMPTY_FRAGMENT, /* willAccessRepeatedly= */ true), + execRoot); } SandboxOutputs outputs = helpers.getOutputs(spawn); diff --git a/src/test/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunnerTest.java b/src/test/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunnerTest.java index 8dfd86a1ddac49..46f6e704dbfc22 100644 --- a/src/test/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunnerTest.java +++ b/src/test/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunnerTest.java @@ -284,7 +284,8 @@ public FileOutErr getFileOutErr() { } @Override - public SortedMap getInputMapping(PathFragment baseDirectory) { + public SortedMap getInputMapping( + PathFragment baseDirectory, boolean willAccessRepeatedly) { return inputMapping; } diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java index 1808b86d66c5ef..5145d78b216e4c 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java @@ -1698,10 +1698,10 @@ public void uploadInputsIfNotPresent_interrupted_requestCancelled() throws Excep new Thread( () -> { try { - service.uploadInputsIfNotPresent(action, /*force=*/ false); + service.uploadInputsIfNotPresent(action, /* force= */ false); } catch (InterruptedException ignored) { interrupted.countDown(); - } catch (IOException ignored) { + } catch (Exception ignored) { // intentionally ignored } }); diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemotePathResolverTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemotePathResolverTest.java index 3805649a1e0bab..0d36c1ec0bf2fb 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemotePathResolverTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemotePathResolverTest.java @@ -14,6 +14,7 @@ package com.google.devtools.build.lib.remote; import static com.google.common.truth.Truth.assertThat; +import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.Mockito.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -50,7 +51,7 @@ public void setup() throws Exception { input = ActionInputHelper.fromPath("foo"); spawnExecutionContext = mock(SpawnExecutionContext.class); - when(spawnExecutionContext.getInputMapping(any())) + when(spawnExecutionContext.getInputMapping(any(), anyBoolean())) .thenAnswer( invocationOnMock -> { PathFragment baseDirectory = invocationOnMock.getArgument(0); @@ -83,7 +84,7 @@ public void getInputMapping_default_inputsRelativeToExecRoot() throws Exception RemotePathResolver remotePathResolver = RemotePathResolver.createDefault(execRoot); SortedMap inputs = - remotePathResolver.getInputMapping(spawnExecutionContext); + remotePathResolver.getInputMapping(spawnExecutionContext, false); assertThat(inputs).containsExactly(PathFragment.create("foo"), input); } @@ -93,7 +94,7 @@ public void getInputMapping_sibling_inputsRelativeToInputRoot() throws Exception RemotePathResolver remotePathResolver = new SiblingRepositoryLayoutResolver(execRoot); SortedMap inputs = - remotePathResolver.getInputMapping(spawnExecutionContext); + remotePathResolver.getInputMapping(spawnExecutionContext, false); assertThat(inputs).containsExactly(PathFragment.create("main/foo"), input); } diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java index 59f8b8ad4bc1cf..05a635dfde09ae 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java @@ -172,7 +172,8 @@ public FileOutErr getFileOutErr() { } @Override - public SortedMap getInputMapping(PathFragment baseDirectory) + public SortedMap getInputMapping( + PathFragment baseDirectory, boolean willAccessRepeatedly) throws IOException, ForbiddenActionInputException { return getSpawnInputExpander() .getInputMapping(simpleSpawn, SIMPLE_ARTIFACT_EXPANDER, baseDirectory, fakeFileCache); diff --git a/src/test/java/com/google/devtools/build/lib/remote/util/FakeSpawnExecutionContext.java b/src/test/java/com/google/devtools/build/lib/remote/util/FakeSpawnExecutionContext.java index 3c25136a3b5509..46dc2572a29ae9 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/util/FakeSpawnExecutionContext.java +++ b/src/test/java/com/google/devtools/build/lib/remote/util/FakeSpawnExecutionContext.java @@ -133,7 +133,8 @@ public FileOutErr getFileOutErr() { } @Override - public SortedMap getInputMapping(PathFragment baseDirectory) + public SortedMap getInputMapping( + PathFragment baseDirectory, boolean willAccessRepeatedly) throws IOException, ForbiddenActionInputException { return getSpawnInputExpander() .getInputMapping(spawn, this::artifactExpander, baseDirectory, metadataProvider); diff --git a/src/test/java/com/google/devtools/build/lib/sandbox/SpawnRunnerTestUtil.java b/src/test/java/com/google/devtools/build/lib/sandbox/SpawnRunnerTestUtil.java index 562d7eabb90aa0..420721c2b30c31 100644 --- a/src/test/java/com/google/devtools/build/lib/sandbox/SpawnRunnerTestUtil.java +++ b/src/test/java/com/google/devtools/build/lib/sandbox/SpawnRunnerTestUtil.java @@ -131,7 +131,8 @@ public FileOutErr getFileOutErr() { } @Override - public SortedMap getInputMapping(PathFragment baseDirectory) { + public SortedMap getInputMapping( + PathFragment baseDirectory, boolean willAccessRepeatedly) { TreeMap inputMapping = new TreeMap<>(); for (ActionInput actionInput : spawn.getInputFiles().toList()) { inputMapping.put(baseDirectory.getRelative(actionInput.getExecPath()), actionInput); diff --git a/src/test/shell/bazel/remote/remote_execution_test.sh b/src/test/shell/bazel/remote/remote_execution_test.sh index 6d6de17de93479..6f66889aa67c86 100755 --- a/src/test/shell/bazel/remote/remote_execution_test.sh +++ b/src/test/shell/bazel/remote/remote_execution_test.sh @@ -2199,6 +2199,13 @@ function test_empty_tree_artifact_as_inputs() { --experimental_remote_merkle_tree_cache \ //pkg:a &>$TEST_log || fail "expected build to succeed with Merkle tree cache" + bazel clean --expunge + bazel build \ + --spawn_strategy=remote \ + --remote_executor=grpc://localhost:${worker_port} \ + --experimental_remote_discard_merkle_trees \ + //pkg:a &>$TEST_log || fail "expected build to succeed with Merkle tree discarding" + bazel clean --expunge bazel build \ --spawn_strategy=remote \ @@ -2273,6 +2280,15 @@ function test_create_tree_artifact_outputs() { [[ -f bazel-bin/pkg/a/non_empty_dir/out ]] || fail "expected tree artifact to contain a file" [[ -d bazel-bin/pkg/a/empty_dir ]] || fail "expected directory to exist" + bazel clean --expunge + bazel build \ + --spawn_strategy=remote \ + --remote_executor=grpc://localhost:${worker_port} \ + --experimental_remote_discard_merkle_trees \ + //pkg:a &>$TEST_log || fail "expected build to succeed with Merkle tree discarding" + [[ -f bazel-bin/pkg/a/non_empty_dir/out ]] || fail "expected tree artifact to contain a file" + [[ -d bazel-bin/pkg/a/empty_dir ]] || fail "expected directory to exist" + bazel clean --expunge bazel build \ --spawn_strategy=remote \ @@ -3099,6 +3115,15 @@ EOF --experimental_remote_merkle_tree_cache \ //pkg:b &>$TEST_log || fail "expected build to succeed with Merkle tree cache" + bazel clean --expunge + bazel \ + --windows_enable_symlinks \ + build \ + --spawn_strategy=remote \ + --remote_executor=grpc://localhost:${worker_port} \ + --experimental_remote_discard_merkle_trees \ + //pkg:b &>$TEST_log || fail "expected build to succeed with Merkle tree discarding" + if [[ "$(cat bazel-bin/pkg/b.txt)" != "$link_target" ]]; then fail "expected symlink target to be $link_target" fi