Skip to content

Commit

Permalink
Wire up PathMapper in RemoteExecutionService
Browse files Browse the repository at this point in the history
`PathMapper`s rewrite paths in command lines to make them more cache
friendly, which requires executor support to stage files at the
rewritten paths. This commit wires up the `PathMapper` used by a given
`Spawn` in `RemoteExecutionService`, which ensures that paths of inputs
and outputs are correctly mapped before being sent off to the remote
executor and mapped back to the correct local paths when downloading the
results.

Work towards #6526
  • Loading branch information
fmeum committed Oct 7, 2023
1 parent 1e02f45 commit a223adb
Show file tree
Hide file tree
Showing 3 changed files with 109 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,10 @@ public Command getCommand() {
return command;
}

public RemotePathResolver getRemotePathResolver() {
return remotePathResolver;
}

@Nullable
public MerkleTree getMerkleTree() {
return merkleTree;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,13 @@ public class RemoteExecutionService {
private final Reporter reporter;
private final boolean verboseFailures;
private final Path execRoot;
private final RemotePathResolver remotePathResolver;

/**
* Do not use directly, instead use the per-spawn resolver created in {@link
* #buildRemoteAction(Spawn, SpawnExecutionContext)}.
*/
private final RemotePathResolver baseRemotePathResolver;

private final String buildRequestId;
private final String commandId;
private final DigestUtil digestUtil;
Expand Down Expand Up @@ -197,7 +203,7 @@ public RemoteExecutionService(
this.reporter = reporter;
this.verboseFailures = verboseFailures;
this.execRoot = execRoot;
this.remotePathResolver = remotePathResolver;
this.baseRemotePathResolver = remotePathResolver;
this.buildRequestId = buildRequestId;
this.commandId = commandId;
this.digestUtil = digestUtil;
Expand Down Expand Up @@ -335,7 +341,8 @@ Cache<Object, CompletableFuture<MerkleTree>> getMerkleTreeCache() {
return merkleTreeCache;
}

private SortedMap<PathFragment, ActionInput> buildOutputDirMap(Spawn spawn) {
private SortedMap<PathFragment, ActionInput> buildOutputDirMap(
Spawn spawn, RemotePathResolver remotePathResolver) {
TreeMap<PathFragment, ActionInput> outputDirMap = new TreeMap<>();
for (ActionInput output : spawn.getOutputFiles()) {
if (output instanceof Artifact && ((Artifact) output).isTreeArtifact()) {
Expand All @@ -349,12 +356,16 @@ private SortedMap<PathFragment, ActionInput> buildOutputDirMap(Spawn spawn) {
}

private MerkleTree buildInputMerkleTree(
Spawn spawn, SpawnExecutionContext context, ToolSignature toolSignature)
Spawn spawn,
SpawnExecutionContext context,
ToolSignature toolSignature,
RemotePathResolver remotePathResolver)
throws IOException, ForbiddenActionInputException {
// Add output directories to inputs so that they are created as empty directories by the
// executor. The spec only requires the executor to create the parent directory of an output
// directory, which differs from the behavior of both local and sandboxed execution.
SortedMap<PathFragment, ActionInput> outputDirMap = buildOutputDirMap(spawn);
SortedMap<PathFragment, ActionInput> outputDirMap =
buildOutputDirMap(spawn, remotePathResolver);
boolean useMerkleTreeCache = remoteOptions.remoteMerkleTreeCache;
if (toolSignature != null) {
// Marking tool files is not yet supported in conjunction with the merkle tree cache.
Expand Down Expand Up @@ -507,8 +518,15 @@ public RemoteAction buildRemoteAction(Spawn spawn, SpawnExecutionContext context
throws IOException, ExecException, ForbiddenActionInputException, InterruptedException {
remoteActionBuildingSemaphore.acquire();
try {
// Create a remote path resolver that is aware of the spawn's path mapper, which rewrites
// the paths of the inputs and outputs as well as paths appearing in the command line for
// execution. This is necessary to ensure that artifacts are correctly emitted into and staged
// from the unmapped location locally.
RemotePathResolver remotePathResolver =
RemotePathResolver.createMapped(baseRemotePathResolver, execRoot, spawn.getPathMapper());
ToolSignature toolSignature = getToolSignature(spawn, context);
final MerkleTree merkleTree = buildInputMerkleTree(spawn, context, toolSignature);
final MerkleTree merkleTree =
buildInputMerkleTree(spawn, context, toolSignature, remotePathResolver);

// Get the remote platform properties.
Platform platform;
Expand Down Expand Up @@ -724,7 +742,8 @@ private ListenableFuture<FileMetadata> downloadFile(
RemoteActionExecutionContext context,
ProgressStatusListener progressStatusListener,
FileMetadata file,
Path tmpPath) {
Path tmpPath,
RemotePathResolver remotePathResolver) {
checkNotNull(remoteCache, "remoteCache can't be null");

try {
Expand Down Expand Up @@ -965,7 +984,9 @@ private DirectoryMetadata parseDirectory(
}

ActionResultMetadata parseActionResultMetadata(
RemoteActionExecutionContext context, RemoteActionResult result)
RemoteActionExecutionContext context,
RemoteActionResult result,
RemotePathResolver remotePathResolver)
throws IOException, InterruptedException {
checkNotNull(remoteCache, "remoteCache can't be null");

Expand Down Expand Up @@ -1070,7 +1091,7 @@ public InMemoryOutput downloadOutputs(RemoteAction action, RemoteActionResult re

ActionResultMetadata metadata;
try (SilentCloseable c = Profiler.instance().profile("Remote.parseActionResultMetadata")) {
metadata = parseActionResultMetadata(context, result);
metadata = parseActionResultMetadata(context, result, action.getRemotePathResolver());
}

// The expiration time for remote cache entries.
Expand Down Expand Up @@ -1107,7 +1128,9 @@ public InMemoryOutput downloadOutputs(RemoteAction action, RemoteActionResult re
if (!isInMemoryOutputFile && shouldDownload(result, execPath)) {
Path tmpPath = tempPathGenerator.generateTempPath();
realToTmpPath.put(file.path, tmpPath);
downloadsBuilder.add(downloadFile(context, progressStatusListener, file, tmpPath));
downloadsBuilder.add(
downloadFile(
context, progressStatusListener, file, tmpPath, action.getRemotePathResolver()));
} else {
remoteActionFileSystem.injectRemoteFile(
file.path().asFragment(),
Expand Down Expand Up @@ -1137,7 +1160,9 @@ public InMemoryOutput downloadOutputs(RemoteAction action, RemoteActionResult re
if (shouldDownload(result, file.path.relativeTo(execRoot))) {
Path tmpPath = tempPathGenerator.generateTempPath();
realToTmpPath.put(file.path, tmpPath);
downloadsBuilder.add(downloadFile(context, progressStatusListener, file, tmpPath));
downloadsBuilder.add(
downloadFile(
context, progressStatusListener, file, tmpPath, action.getRemotePathResolver()));
} else {
remoteActionFileSystem.injectRemoteFile(
file.path().asFragment(),
Expand Down Expand Up @@ -1295,7 +1320,7 @@ private Single<UploadManifest> buildUploadManifestAsync(
remoteOptions,
remoteCache.getCacheCapabilities(),
digestUtil,
remotePathResolver,
action.getRemotePathResolver(),
action.getActionKey(),
action.getAction(),
action.getCommand(),
Expand Down Expand Up @@ -1414,7 +1439,8 @@ public void uploadInputsIfNotPresent(RemoteAction action, boolean force)
Spawn spawn = action.getSpawn();
SpawnExecutionContext context = action.getSpawnExecutionContext();
ToolSignature toolSignature = getToolSignature(spawn, context);
merkleTree = buildInputMerkleTree(spawn, context, toolSignature);
merkleTree =
buildInputMerkleTree(spawn, context, toolSignature, action.getRemotePathResolver());
}

remoteExecutionCache.ensureInputsPresent(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,20 @@

import static com.google.common.base.Preconditions.checkNotNull;

import com.google.common.base.Preconditions;
import com.google.devtools.build.lib.actions.ActionInput;
import com.google.devtools.build.lib.actions.ActionInputHelper;
import com.google.devtools.build.lib.actions.ForbiddenActionInputException;
import com.google.devtools.build.lib.actions.PathMapper;
import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.exec.SpawnInputExpander;
import com.google.devtools.build.lib.exec.SpawnInputExpander.InputVisitor;
import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionContext;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.io.IOException;
import java.util.SortedMap;
import java.util.concurrent.ConcurrentHashMap;

/**
* A {@link RemotePathResolver} is used to resolve input/output paths for remote execution from
Expand Down Expand Up @@ -225,4 +229,66 @@ public Path outputPathToLocalPath(ActionInput actionInput) {
return ActionInputHelper.toInputPath(actionInput, execRoot);
}
}

/**
* Adapts a given base {@link RemotePathResolver} to also apply a {@link PathMapper} to map (and
* inverse map) paths.
*/
static RemotePathResolver createMapped(
RemotePathResolver base, Path execRoot, PathMapper pathMapper) {
if (pathMapper.isNoop()) {
return base;
}
return new RemotePathResolver() {
private final ConcurrentHashMap<PathFragment, PathFragment> inverse =
new ConcurrentHashMap<>();

@Override
public String getWorkingDirectory() {
return base.getWorkingDirectory();
}

@Override
public SortedMap<PathFragment, ActionInput> getInputMapping(
SpawnExecutionContext context, boolean willAccessRepeatedly)
throws IOException, ForbiddenActionInputException {
return base.getInputMapping(context, willAccessRepeatedly);
}

@Override
public void walkInputs(Spawn spawn, SpawnExecutionContext context, InputVisitor visitor)
throws IOException, ForbiddenActionInputException {
base.walkInputs(spawn, context, visitor);
}

@Override
public String localPathToOutputPath(Path path) {
return localPathToOutputPath(path.relativeTo(execRoot));
}

@Override
public String localPathToOutputPath(PathFragment execPath) {
return base.localPathToOutputPath(map(execPath));
}

@Override
public Path outputPathToLocalPath(String outputPath) {
return execRoot.getRelative(
inverseMap(base.outputPathToLocalPath(outputPath).relativeTo(execRoot)));
}

private PathFragment map(PathFragment path) {
PathFragment mappedPath = pathMapper.map(path);
inverse.put(mappedPath, path);
return mappedPath;
}

private PathFragment inverseMap(PathFragment path) {
PathFragment originalPath = inverse.get(path);
Preconditions.checkState(
originalPath != null, "Failed to find original path for mapped path %s", path);
return originalPath;
}
};
}
}

0 comments on commit a223adb

Please sign in to comment.