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.

An end-to-end test will be added in #18155, but requires #19718, #19719, and #19721.

Work towards #6526

Closes #19718.

PiperOrigin-RevId: 571109361
Change-Id: Ia38464011f658178ab2a1981a3ddaf5aead7c8fa
  • Loading branch information
@fmeum
fmeum authored and Copybara-Service committed Oct 5, 2023
1 parent 05787f3 commit d262c7d
Show file tree
Hide file tree
Showing 5 changed files with 287 additions and 43 deletions.
21 changes: 20 additions & 1 deletion src/main/java/com/google/devtools/build/lib/actions/PathMapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,15 @@
* part (e.g. "k8-fastbuild") from exec paths to allow for cross-configuration cache hits.
*/
public interface PathMapper {
/** Returns the exec path with the path mapping applied. */
/**
* Returns the exec path with the path mapping applied.
*
* <p>Path mappers may return paths with different roots for two paths that have the same root
* (e.g., they may map an artifact at {@code bazel-out/k8-fastbuild/bin/pkg/foo} to {@code
* bazel-out/<hash of the file>/bin/pkg/foo}). Paths of artifacts that should share the same
* parent directory, such as runfiles or tree artifact files, should thus be derived from the
* mapped path of their parent.
*/
PathFragment map(PathFragment execPath);

/** Returns the exec path of the input with the path mapping applied. */
Expand Down Expand Up @@ -80,6 +88,17 @@ default boolean isNoop() {
return this == NOOP;
}

/**
* Returns an opaque object whose equality class should encode all information that goes into the
* behavior of the {@link #map(PathFragment)} function of this path mapper. This is used as a key
* for in-memory caches.
*
* <p>The default implementation returns the {@link Class} of the path mapper.
*/
default Object cacheKey() {
return this.getClass();
}

/** A {@link PathMapper} that doesn't change paths. */
PathMapper NOOP = execPath -> execPath;
}
94 changes: 76 additions & 18 deletions src/main/java/com/google/devtools/build/lib/exec/SpawnInputExpander.java
View file
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 All @@ -145,6 +147,7 @@ void addRunfilesToInputs(
? null
: artifactExpander.getArchivedTreeArtifact((SpecialArtifact) localArtifact);
if (archivedTreeArtifact != null) {
// TODO(bazel-team): Add path mapping support for archived tree artifacts.
addMapping(inputMap, location, localArtifact, baseDirectory);
} else {
List<ActionInput> expandedInputs =
Expand All @@ -153,11 +156,12 @@ void addRunfilesToInputs(
artifactExpander,
/* keepEmptyTreeArtifacts= */ false);
for (ActionInput input : expandedInputs) {
addMapping(
inputMap,
location.getRelative(((TreeFileArtifact) input).getParentRelativePath()),
input,
baseDirectory);
addMapping(
inputMap,
mapForRunfiles(pathMapper, root, location)
.getRelative(((TreeFileArtifact) input).getParentRelativePath()),
input,
baseDirectory);
}
}
} else if (localArtifact.isFileset()) {
Expand All @@ -167,15 +171,21 @@ void addRunfilesToInputs(
} catch (MissingExpansionException e) {
throw new IllegalStateException(e);
}
// TODO(bazel-team): Add path mapping support for filesets.
addFilesetManifest(location, localArtifact, filesetLinks, inputMap, baseDirectory);
} else {
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 +196,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 @@ -235,6 +246,7 @@ void addFilesetManifest(
value == null
? VirtualActionInput.EMPTY_MARKER
: ActionInputHelper.fromPath(execRoot.getRelative(value).asFragment());
// TODO(bazel-team): Add path mapping support for filesets.
addMapping(inputMappings, mapping.getKey(), artifact, baseDirectory);
}
}
Expand All @@ -243,6 +255,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 +264,17 @@ 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 +296,37 @@ 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 +369,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 +397,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 +414,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 +434,7 @@ public SortedMap<PathFragment, ActionInput> getLeavesInputMapping() {
inputMap,
NestedSetBuilder.wrap(someInputFiles.getOrder(), leaves),
artifactExpander,
pathMapper,
baseDirectory);
return inputMap;
}
Expand All @@ -394,11 +445,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 +465,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 +479,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 d262c7d

Please sign in to comment.