Skip to content

Commit

Permalink
Wire up PathMapper in SpawnInputsExpander
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 `SpawnInputsExpander`, which takes care of this for inputs
to `Spawn`s executed in a sandbox or remotely.

Constructs specific to Blaze, filesets and archived tree artifacts, are
not covered by this change.

Work towards #6526
  • Loading branch information
fmeum committed Oct 4, 2023
1 parent 9b30bf7 commit b7fdade
Show file tree
Hide file tree
Showing 6 changed files with 203 additions and 47 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,16 @@ default boolean isNoop() {
return this == NOOP;
}

/**
* Returns an opaque object whose equality class encodes the behavior of this mapper for use in
* in-memory cache keys.
*
* <p>The default implementation returns the {@link Class} of the mapper.
*/
default Object cacheKey() {
return this.getClass();
}

/** A {@link PathMapper} that doesn't change paths. */
PathMapper NOOP = execPath -> execPath;
}
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,11 @@ public ExceptionlessMapFn<Object> getMapFn(@Nullable String previousFlag) {
}
return MapFn.DEFAULT;
}

@Override
public Object cacheKey() {
return GUID;
}
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import com.google.devtools.build.lib.actions.FilesetOutputSymlink;
import com.google.devtools.build.lib.actions.ForbiddenActionInputException;
import com.google.devtools.build.lib.actions.InputMetadataProvider;
import com.google.devtools.build.lib.actions.PathMapper;
import com.google.devtools.build.lib.actions.RunfilesSupplier;
import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.actions.cache.VirtualActionInput;
Expand Down Expand Up @@ -125,6 +126,7 @@ void addRunfilesToInputs(
RunfilesSupplier runfilesSupplier,
InputMetadataProvider actionFileCache,
ArtifactExpander artifactExpander,
PathMapper pathMapper,
PathFragment baseDirectory)
throws IOException, ForbiddenActionInputException {
Map<PathFragment, Map<PathFragment, Artifact>> rootsAndMappings =
Expand Down Expand Up @@ -155,7 +157,8 @@ void addRunfilesToInputs(
for (ActionInput input : expandedInputs) {
addMapping(
inputMap,
location.getRelative(((TreeFileArtifact) input).getParentRelativePath()),
mapForRunfiles(pathMapper, root, location).getRelative(
((TreeFileArtifact) input).getParentRelativePath()),
input,
baseDirectory);
}
Expand All @@ -172,10 +175,12 @@ void addRunfilesToInputs(
if (strict) {
failIfDirectory(actionFileCache, localArtifact);
}
addMapping(inputMap, location, localArtifact, baseDirectory);
addMapping(inputMap, mapForRunfiles(pathMapper, root, location), localArtifact,
baseDirectory);
}
} else {
addMapping(inputMap, location, VirtualActionInput.EMPTY_MARKER, baseDirectory);
addMapping(inputMap, mapForRunfiles(pathMapper, root, location),
VirtualActionInput.EMPTY_MARKER, baseDirectory);
}
}
}
Expand All @@ -186,11 +191,12 @@ public Map<PathFragment, ActionInput> addRunfilesToInputs(
RunfilesSupplier runfilesSupplier,
InputMetadataProvider actionFileCache,
ArtifactExpander artifactExpander,
PathMapper pathMapper,
PathFragment baseDirectory)
throws IOException, ForbiddenActionInputException {
Map<PathFragment, ActionInput> inputMap = new HashMap<>();
addRunfilesToInputs(
inputMap, runfilesSupplier, actionFileCache, artifactExpander, baseDirectory);
inputMap, runfilesSupplier, actionFileCache, artifactExpander, pathMapper, baseDirectory);
return inputMap;
}

Expand Down Expand Up @@ -243,6 +249,7 @@ private static void addInputs(
Map<PathFragment, ActionInput> inputMap,
NestedSet<? extends ActionInput> inputFiles,
ArtifactExpander artifactExpander,
PathMapper pathMapper,
PathFragment baseDirectory) {
// Actions that accept TreeArtifacts as inputs generally expect the directory corresponding
// to the artifact to be created, even if it is empty. We explicitly keep empty TreeArtifacts
Expand All @@ -251,7 +258,14 @@ private static void addInputs(
ActionInputHelper.expandArtifacts(
inputFiles, artifactExpander, /* keepEmptyTreeArtifacts= */ true);
for (ActionInput input : inputs) {
addMapping(inputMap, input.getExecPath(), input, baseDirectory);
if (input instanceof TreeFileArtifact) {
addMapping(inputMap,
pathMapper.map(((TreeFileArtifact) input).getParent().getExecPath())
.getRelative(((TreeFileArtifact) input).getParentRelativePath()), input,
baseDirectory);
} else {
addMapping(inputMap, pathMapper.map(input.getExecPath()), input, baseDirectory);
}
}
}

Expand All @@ -273,17 +287,35 @@ public SortedMap<PathFragment, ActionInput> getInputMapping(
InputMetadataProvider actionInputFileCache)
throws IOException, ForbiddenActionInputException {
TreeMap<PathFragment, ActionInput> inputMap = new TreeMap<>();
addInputs(inputMap, spawn.getInputFiles(), artifactExpander, baseDirectory);
addInputs(inputMap, spawn.getInputFiles(), artifactExpander, spawn.getPathMapper(),
baseDirectory);
addRunfilesToInputs(
inputMap,
spawn.getRunfilesSupplier(),
actionInputFileCache,
artifactExpander,
spawn.getPathMapper(),
baseDirectory);
addFilesetManifests(spawn.getFilesetMappings(), inputMap, baseDirectory);
return inputMap;
}

private static PathFragment mapForRunfiles(
PathMapper pathMapper, PathFragment runfilesDir, PathFragment execPath) {
if (pathMapper.isNoop()) {
return execPath;
}
String runfilesDirName = runfilesDir.getBaseName();
Preconditions.checkArgument(runfilesDirName.endsWith(".runfiles"));
// Derive the path of the executable, apply the path mapping to it and then rederive the path
// of the runfiles dir.
PathFragment executable =
runfilesDir.replaceName(
runfilesDirName.substring(0, runfilesDirName.length() - ".runfiles".length()));
return pathMapper.map(executable).replaceName(runfilesDirName)
.getRelative(execPath.relativeTo(runfilesDir));
}

/** The interface for accessing part of the input hierarchy. */
public interface InputWalker {

Expand Down Expand Up @@ -326,19 +358,25 @@ public void walkInputs(
InputMetadataProvider actionInputFileCache,
InputVisitor visitor)
throws IOException, ForbiddenActionInputException {
walkNestedSetInputs(baseDirectory, spawn.getInputFiles(), artifactExpander, visitor);
walkNestedSetInputs(baseDirectory, spawn.getInputFiles(), artifactExpander,
spawn.getPathMapper(), visitor);

RunfilesSupplier runfilesSupplier = spawn.getRunfilesSupplier();
visitor.visit(
// Cache key for the sub-mapping containing the runfiles inputs for this spawn.
ImmutableList.of(runfilesSupplier, baseDirectory),
ImmutableList.of(runfilesSupplier, baseDirectory, spawn.getPathMapper().cacheKey()),
new InputWalker() {
@Override
public SortedMap<PathFragment, ActionInput> getLeavesInputMapping()
throws IOException, ForbiddenActionInputException {
TreeMap<PathFragment, ActionInput> inputMap = new TreeMap<>();
addRunfilesToInputs(
inputMap, runfilesSupplier, actionInputFileCache, artifactExpander, baseDirectory);
inputMap,
runfilesSupplier,
actionInputFileCache,
artifactExpander,
spawn.getPathMapper(),
baseDirectory);
return inputMap;
}
});
Expand All @@ -348,7 +386,7 @@ public SortedMap<PathFragment, ActionInput> getLeavesInputMapping()
// improved runtime.
visitor.visit(
// Cache key for the sub-mapping containing the fileset inputs for this spawn.
ImmutableList.of(filesetMappings, baseDirectory),
ImmutableList.of(filesetMappings, baseDirectory, spawn.getPathMapper().cacheKey()),
new InputWalker() {
@Override
public SortedMap<PathFragment, ActionInput> getLeavesInputMapping()
Expand All @@ -365,11 +403,12 @@ private void walkNestedSetInputs(
PathFragment baseDirectory,
NestedSet<? extends ActionInput> someInputFiles,
ArtifactExpander artifactExpander,
PathMapper pathMapper,
InputVisitor visitor)
throws IOException, ForbiddenActionInputException {
visitor.visit(
// Cache key for the sub-mapping containing the files in this nested set.
ImmutableList.of(someInputFiles.toNode(), baseDirectory),
ImmutableList.of(someInputFiles.toNode(), baseDirectory, pathMapper.cacheKey()),
new InputWalker() {
@Override
public SortedMap<PathFragment, ActionInput> getLeavesInputMapping() {
Expand All @@ -384,6 +423,7 @@ public SortedMap<PathFragment, ActionInput> getLeavesInputMapping() {
inputMap,
NestedSetBuilder.wrap(someInputFiles.getOrder(), leaves),
artifactExpander,
pathMapper,
baseDirectory);
return inputMap;
}
Expand All @@ -394,11 +434,16 @@ public void visitNonLeaves(InputVisitor childVisitor)
for (ActionInput input : someInputFiles.getLeaves()) {
if (isTreeArtifact(input)) {
walkTreeInputs(
baseDirectory, (SpecialArtifact) input, artifactExpander, childVisitor);
baseDirectory,
(SpecialArtifact) input,
artifactExpander,
pathMapper,
childVisitor);
}
}
for (NestedSet<? extends ActionInput> subInputs : someInputFiles.getNonLeaves()) {
walkNestedSetInputs(baseDirectory, subInputs, artifactExpander, childVisitor);
walkNestedSetInputs(
baseDirectory, subInputs, artifactExpander, pathMapper, childVisitor);
}
}
});
Expand All @@ -409,11 +454,12 @@ private void walkTreeInputs(
PathFragment baseDirectory,
SpecialArtifact tree,
ArtifactExpander artifactExpander,
PathMapper pathMapper,
InputVisitor visitor)
throws IOException, ForbiddenActionInputException {
visitor.visit(
// Cache key for the sub-mapping containing the files in this tree artifact.
ImmutableList.of(tree, baseDirectory),
ImmutableList.of(tree, baseDirectory, pathMapper.cacheKey()),
new InputWalker() {
@Override
public SortedMap<PathFragment, ActionInput> getLeavesInputMapping() {
Expand All @@ -422,6 +468,7 @@ public SortedMap<PathFragment, ActionInput> getLeavesInputMapping() {
inputMap,
NestedSetBuilder.create(Order.STABLE_ORDER, tree),
artifactExpander,
pathMapper,
baseDirectory);
return inputMap;
}
Expand Down
Loading

0 comments on commit b7fdade

Please sign in to comment.