diff --git a/src/main/java/com/google/devtools/build/lib/actions/FileArtifactValue.java b/src/main/java/com/google/devtools/build/lib/actions/FileArtifactValue.java index f08bdb949f834a..ebcc2900994b0b 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/FileArtifactValue.java +++ b/src/main/java/com/google/devtools/build/lib/actions/FileArtifactValue.java @@ -36,6 +36,7 @@ import java.io.IOException; import java.util.Arrays; import java.util.Objects; +import java.util.Optional; import javax.annotation.Nullable; /** @@ -172,6 +173,19 @@ protected boolean couldBeModifiedByMetadata(FileArtifactValue lastKnown) { return true; } + /** + * Optional materialization path. + * + *

If present, this artifact is a copy of another artifact. It is still tracked as a + * non-symlink by Bazel, but materialized in the local filesystem as a symlink to the original + * artifact, whose contents live at this location. This is used by {@link + * com.google.devtools.build.lib.remote.AbstractActionInputPrefetcher} to implement zero-cost + * copies of remotely stored artifacts. + */ + public Optional getMaterializationExecPath() { + return Optional.empty(); + } + /** * Marker interface for singleton implementations of this class. * @@ -514,9 +528,7 @@ public long getModifiedTime() { @Override public String toString() { return MoreObjects.toStringHelper(this) - .add( - "digest", - digest == null ? "(null)" : BaseEncoding.base16().lowerCase().encode(digest)) + .add("digest", BaseEncoding.base16().lowerCase().encode(digest)) .add("size", size) .add("proxy", proxy) .toString(); @@ -535,10 +547,10 @@ protected boolean couldBeModifiedByMetadata(FileArtifactValue o) { /** Metadata for remotely stored files. */ public static class RemoteFileArtifactValue extends FileArtifactValue { - private final byte[] digest; - private final long size; - private final int locationIndex; - private final String actionId; + protected final byte[] digest; + protected final long size; + protected final int locationIndex; + protected final String actionId; private RemoteFileArtifactValue(byte[] digest, long size, int locationIndex, String actionId) { this.digest = Preconditions.checkNotNull(digest, actionId); @@ -556,6 +568,19 @@ public static RemoteFileArtifactValue create(byte[] digest, long size, int locat return new RemoteFileArtifactValue(digest, size, locationIndex, /* actionId= */ ""); } + public static RemoteFileArtifactValue create( + byte[] digest, + long size, + int locationIndex, + String actionId, + @Nullable PathFragment materializationExecPath) { + if (materializationExecPath != null) { + return new RemoteFileArtifactValueWithMaterializationPath( + digest, size, locationIndex, actionId, materializationExecPath); + } + return new RemoteFileArtifactValue(digest, size, locationIndex, actionId); + } + @Override public boolean equals(Object o) { if (!(o instanceof RemoteFileArtifactValue)) { @@ -602,7 +627,7 @@ public String getActionId() { @Override public long getModifiedTime() { throw new UnsupportedOperationException( - "RemoteFileArifactValue doesn't support getModifiedTime"); + "RemoteFileArtifactValue doesn't support getModifiedTime"); } @Override @@ -626,6 +651,65 @@ public String toString() { .add("digest", bytesToString(digest)) .add("size", size) .add("locationIndex", locationIndex) + .add("actionId", actionId) + .toString(); + } + } + + /** + * A remote artifact that should be materialized in the local filesystem as a symlink to another + * location. + * + *

See the documentation for {@link FileArtifactValue#getMaterializationExecPath}. + */ + public static final class RemoteFileArtifactValueWithMaterializationPath + extends RemoteFileArtifactValue { + private PathFragment materializationExecPath; + + private RemoteFileArtifactValueWithMaterializationPath( + byte[] digest, + long size, + int locationIndex, + String actionId, + PathFragment materializationExecPath) { + super(digest, size, locationIndex, actionId); + this.materializationExecPath = Preconditions.checkNotNull(materializationExecPath); + } + + @Override + public Optional getMaterializationExecPath() { + return Optional.ofNullable(materializationExecPath); + } + + @Override + public boolean equals(Object o) { + if (!(o instanceof RemoteFileArtifactValueWithMaterializationPath)) { + return false; + } + + RemoteFileArtifactValueWithMaterializationPath that = + (RemoteFileArtifactValueWithMaterializationPath) o; + return Arrays.equals(digest, that.digest) + && size == that.size + && locationIndex == that.locationIndex + && Objects.equals(actionId, that.actionId) + && Objects.equals(materializationExecPath, that.materializationExecPath); + } + + @Override + public int hashCode() { + return Objects.hash( + Arrays.hashCode(digest), size, locationIndex, actionId, materializationExecPath); + } + + @Override + public String toString() { + return MoreObjects.toStringHelper(this) + .add("digest", bytesToString(digest)) + .add("size", size) + .add("locationIndex", locationIndex) + .add("actionId", actionId) + .add("materializationExecPath", materializationExecPath) .toString(); } } diff --git a/src/main/java/com/google/devtools/build/lib/actions/cache/ActionCache.java b/src/main/java/com/google/devtools/build/lib/actions/cache/ActionCache.java index ff1b13c85be8e7..6204e6f06038f1 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/cache/ActionCache.java +++ b/src/main/java/com/google/devtools/build/lib/actions/cache/ActionCache.java @@ -109,9 +109,10 @@ final class Entry { public abstract static class SerializableTreeArtifactValue { public static SerializableTreeArtifactValue create( ImmutableMap childValues, - Optional archivedFileValue) { + Optional archivedFileValue, + Optional materializationExecPath) { return new AutoValue_ActionCache_Entry_SerializableTreeArtifactValue( - childValues, archivedFileValue); + childValues, archivedFileValue, materializationExecPath); } /** @@ -138,17 +139,25 @@ public static Optional createSerializable( .filter(ar -> ar.archivedFileValue().isRemote()) .map(ar -> (RemoteFileArtifactValue) ar.archivedFileValue()); - if (childValues.isEmpty() && archivedFileValue.isEmpty()) { + Optional materializationExecPath = treeMetadata.getMaterializationExecPath(); + + if (childValues.isEmpty() + && archivedFileValue.isEmpty() + && materializationExecPath.isEmpty()) { return Optional.empty(); } - return Optional.of(SerializableTreeArtifactValue.create(childValues, archivedFileValue)); + return Optional.of( + SerializableTreeArtifactValue.create( + childValues, archivedFileValue, materializationExecPath)); } // A map from parentRelativePath to the file metadata public abstract ImmutableMap childValues(); public abstract Optional archivedFileValue(); + + public abstract Optional materializationExecPath(); } public Entry(String key, Map usedClientEnv, boolean discoversInputs) { diff --git a/src/main/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCache.java b/src/main/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCache.java index 6fc3d4956d65ca..635e8127922d08 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCache.java +++ b/src/main/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCache.java @@ -36,6 +36,7 @@ import com.google.devtools.build.lib.util.VarInt; import com.google.devtools.build.lib.vfs.DigestUtils; import com.google.devtools.build.lib.vfs.Path; +import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.SyscallCache; import com.google.devtools.build.lib.vfs.UnixGlob; import java.io.ByteArrayOutputStream; @@ -75,7 +76,7 @@ public class CompactPersistentActionCache implements ActionCache { private static final int NO_INPUT_DISCOVERY_COUNT = -1; - private static final int VERSION = 13; + private static final int VERSION = 14; private static final class ActionMap extends PersistentMap { private final Clock clock; @@ -466,6 +467,14 @@ private static void encodeRemoteMetadata( VarInt.putVarInt(value.getLocationIndex(), sink); VarInt.putVarInt(indexer.getOrCreateIndex(value.getActionId()), sink); + + Optional materializationExecPath = value.getMaterializationExecPath(); + if (materializationExecPath.isPresent()) { + VarInt.putVarInt(1, sink); + VarInt.putVarInt(indexer.getOrCreateIndex(materializationExecPath.get().toString()), sink); + } else { + VarInt.putVarInt(0, sink); + } } private static final int MAX_REMOTE_METADATA_SIZE = @@ -484,7 +493,18 @@ private static RemoteFileArtifactValue decodeRemoteMetadata( String actionId = getStringForIndex(indexer, VarInt.getVarInt(source)); - return RemoteFileArtifactValue.create(digest, size, locationIndex, actionId); + PathFragment materializationExecPath = null; + int nummaterializationExecPath = VarInt.getVarInt(source); + if (nummaterializationExecPath > 0) { + if (nummaterializationExecPath != 1) { + throw new IOException("Invalid number of symlink target paths"); + } + materializationExecPath = + PathFragment.create(getStringForIndex(indexer, VarInt.getVarInt(source))); + } + + return RemoteFileArtifactValue.create( + digest, size, locationIndex, actionId, materializationExecPath); } /** @return action data encoded as a byte[] array. */ @@ -513,9 +533,12 @@ private static byte[] encode(StringIndexer indexer, ActionCache.Entry entry) thr + MAX_REMOTE_METADATA_SIZE) * value.childValues().size(); - maxOutputTreesSize += VarInt.MAX_VARINT_SIZE; // value.archivedFileValue() optional maxOutputTreesSize += - value.archivedFileValue().map(ignored -> MAX_REMOTE_METADATA_SIZE).orElse(0); + (1 + VarInt.MAX_VARINT_SIZE) // value.archivedFileValue() optional + + value.archivedFileValue().map(ignored -> MAX_REMOTE_METADATA_SIZE).orElse(0); + maxOutputTreesSize += + (1 + VarInt.MAX_VARINT_SIZE) // value.materializationExecPath() optional + + value.materializationExecPath().map(ignored -> MAX_REMOTE_METADATA_SIZE).orElse(0); } // Estimate the size of the buffer: @@ -578,6 +601,15 @@ private static byte[] encode(StringIndexer indexer, ActionCache.Entry entry) thr } else { VarInt.putVarInt(0, sink); } + + Optional materializationExecPath = + serializableTreeArtifactValue.materializationExecPath(); + if (materializationExecPath.isPresent()) { + VarInt.putVarInt(1, sink); + VarInt.putVarInt(indexer.getOrCreateIndex(materializationExecPath.get().toString()), sink); + } else { + VarInt.putVarInt(0, sink); + } } return sink.toByteArray(); @@ -649,13 +681,25 @@ private static ActionCache.Entry decode(StringIndexer indexer, byte[] data) thro int numArchivedFileValue = VarInt.getVarInt(source); if (numArchivedFileValue > 0) { if (numArchivedFileValue != 1) { - throw new IOException("Invalid number of archived artifacts"); + throw new IOException("Invalid presence marker for archived representation"); } archivedFileValue = Optional.of(decodeRemoteMetadata(indexer, source)); } + Optional symlinkTargetPath = Optional.empty(); + int nummaterializationExecPath = VarInt.getVarInt(source); + if (nummaterializationExecPath > 0) { + if (nummaterializationExecPath != 1) { + throw new IOException("Invalid presence marker for symlink target"); + } + symlinkTargetPath = + Optional.of( + PathFragment.create(getStringForIndex(indexer, VarInt.getVarInt(source)))); + } + SerializableTreeArtifactValue value = - SerializableTreeArtifactValue.create(childValues.buildOrThrow(), archivedFileValue); + SerializableTreeArtifactValue.create( + childValues.buildOrThrow(), archivedFileValue, symlinkTargetPath); outputTrees.put(treeKey, value); } diff --git a/src/main/java/com/google/devtools/build/lib/remote/AbstractActionInputPrefetcher.java b/src/main/java/com/google/devtools/build/lib/remote/AbstractActionInputPrefetcher.java index 8db09901ac7ab6..a5ea0e46a785f7 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/AbstractActionInputPrefetcher.java +++ b/src/main/java/com/google/devtools/build/lib/remote/AbstractActionInputPrefetcher.java @@ -45,7 +45,6 @@ import com.google.devtools.build.lib.vfs.PathFragment; import io.reactivex.rxjava3.core.Completable; import io.reactivex.rxjava3.core.Flowable; -import io.reactivex.rxjava3.core.Single; import java.io.IOException; import java.util.ArrayList; import java.util.HashMap; @@ -142,6 +141,7 @@ protected ListenableFuture prefetchFiles( Priority priority) { Map> trees = new HashMap<>(); List files = new ArrayList<>(); + for (ActionInput input : inputs) { if (input instanceof Artifact && ((Artifact) input).isSourceArtifact()) { continue; @@ -162,34 +162,75 @@ protected ListenableFuture prefetchFiles( .flatMapSingle( entry -> toTransferResult( - prefetchInputTree( + prefetchInputTreeOrSymlink( metadataProvider, entry.getKey(), entry.getValue(), priority))); + Flowable fileDownloads = Flowable.fromIterable(files) .flatMapSingle( - input -> toTransferResult(prefetchInputFile(metadataProvider, input, priority))); + input -> + toTransferResult( + prefetchInputFileOrSymlink(metadataProvider, input, priority))); + Flowable transfers = Flowable.merge(treeDownloads, fileDownloads); Completable prefetch = mergeBulkTransfer(transfers).onErrorResumeNext(this::onErrorResumeNext); return toListenableFuture(prefetch); } - private Completable prefetchInputTree( + private Completable prefetchInputTreeOrSymlink( MetadataProvider provider, SpecialArtifact tree, List treeFiles, + Priority priority) + throws IOException { + + PathFragment execPath = tree.getExecPath(); + + FileArtifactValue treeMetadata = provider.getMetadata(tree); + // TODO(tjgq): Only download individual files that were requested within the tree. + // This isn't straightforward because multiple tree artifacts may share the same output tree + // when a ctx.actions.symlink is involved. + if (treeMetadata == null || !shouldDownloadAnyTreeFiles(treeFiles, treeMetadata)) { + return Completable.complete(); + } + + PathFragment prefetchExecPath = treeMetadata.getMaterializationExecPath().orElse(execPath); + + Completable prefetch = prefetchInputTree(provider, prefetchExecPath, treeFiles, priority); + + // If prefetching to a different path, plant a symlink into it. + if (!prefetchExecPath.equals(execPath)) { + Completable prefetchAndSymlink = + prefetch.doOnComplete(() -> createSymlink(execPath, prefetchExecPath)); + return downloadCache.executeIfNot(execRoot.getRelative(execPath), prefetchAndSymlink); + } + + return prefetch; + } + + private boolean shouldDownloadAnyTreeFiles( + Iterable treeFiles, FileArtifactValue metadata) { + for (TreeFileArtifact treeFile : treeFiles) { + if (shouldDownloadFile(treeFile.getPath(), metadata)) { + return true; + } + } + return false; + } + + private Completable prefetchInputTree( + MetadataProvider provider, + PathFragment execPath, + List treeFiles, Priority priority) { - Path treeRoot = execRoot.getRelative(tree.getExecPath()); + Path treeRoot = execRoot.getRelative(execPath); HashMap treeFileTmpPathMap = new HashMap<>(); Flowable transfers = Flowable.fromIterable(treeFiles) .flatMapSingle( treeFile -> { - Path path = treeRoot.getRelative(treeFile.getParentRelativePath()); FileArtifactValue metadata = provider.getMetadata(treeFile); - if (!shouldDownloadFile(path, metadata)) { - return Single.just(TransferResult.ok()); - } Path tempPath = tempPathGenerator.generateTempPath(); treeFileTmpPathMap.put(treeFile, tempPath); @@ -198,7 +239,7 @@ private Completable prefetchInputTree( toCompletable( () -> doDownloadFile( - tempPath, path.relativeTo(execRoot), metadata, priority), + tempPath, treeFile.getExecPath(), metadata, priority), directExecutor())); }); @@ -209,10 +250,11 @@ private Completable prefetchInputTree( () -> { HashSet dirs = new HashSet<>(); - // Tree root is created by Bazel before action execution, but the permission is - // changed to 0555 afterwards. We need to set it as writable in order to move - // files into it. - treeRoot.setWritable(true); + // Even though the root directory for a tree artifact is created prior to action + // execution, we might be prefetching to a different directory that doesn't yet + // exist (when FileArtifactValue#getMaterializationExecPath() is present). + // In any case, we need to make it writable to move files into it. + createWritableDirectory(treeRoot); dirs.add(treeRoot); for (Map.Entry entry : treeFileTmpPathMap.entrySet()) { @@ -227,8 +269,7 @@ private Completable prefetchInputTree( break; } if (dirs.add(dir)) { - dir.createDirectory(); - dir.setWritable(true); + createWritableDirectory(dir); } } checkState(dir.equals(path)); @@ -257,20 +298,33 @@ private Completable prefetchInputTree( return downloadCache.executeIfNot(treeRoot, download); } - private Completable prefetchInputFile( + private Completable prefetchInputFileOrSymlink( MetadataProvider metadataProvider, ActionInput input, Priority priority) throws IOException { if (input instanceof VirtualActionInput) { prefetchVirtualActionInput((VirtualActionInput) input); return Completable.complete(); } + PathFragment execPath = input.getExecPath(); + FileArtifactValue metadata = metadataProvider.getMetadata(input); - if (metadata == null) { + if (metadata == null || !shouldDownloadFile(execRoot.getRelative(execPath), metadata)) { return Completable.complete(); } - Path path = execRoot.getRelative(input.getExecPath()); - return downloadFileRx(path, metadata, priority); + PathFragment prefetchExecPath = metadata.getMaterializationExecPath().orElse(execPath); + + Completable prefetch = + downloadFileNoCheckRx(execRoot.getRelative(prefetchExecPath), metadata, priority); + + // If prefetching to a different path, plant a symlink into it. + if (!prefetchExecPath.equals(execPath)) { + Completable prefetchAndSymlink = + prefetch.doOnComplete(() -> createSymlink(execPath, prefetchExecPath)); + return downloadCache.executeIfNot(execRoot.getRelative(execPath), prefetchAndSymlink); + } + + return prefetch; } /** @@ -283,7 +337,11 @@ private Completable downloadFileRx(Path path, FileArtifactValue metadata, Priori if (!shouldDownloadFile(path, metadata)) { return Completable.complete(); } + return downloadFileNoCheckRx(path, metadata, priority); + } + private Completable downloadFileNoCheckRx( + Path path, FileArtifactValue metadata, Priority priority) { if (path.isSymbolicLink()) { try { path = path.getRelative(path.readSymbolicLink()); @@ -348,6 +406,20 @@ private void deletePartialDownload(Path path) { } } + private void createWritableDirectory(Path dir) throws IOException { + dir.createDirectory(); + dir.setWritable(true); + } + + private void createSymlink(PathFragment linkPath, PathFragment targetPath) throws IOException { + Path link = execRoot.getRelative(linkPath); + Path target = execRoot.getRelative(targetPath); + // Delete the link path if it already exists. + // This will happen for output directories, which get created before the action runs. + link.delete(); + link.createSymbolicLink(target); + } + public ImmutableSet downloadedFiles() { return downloadCache.getFinishedTasks(); } diff --git a/src/main/java/com/google/devtools/build/lib/remote/BUILD b/src/main/java/com/google/devtools/build/lib/remote/BUILD index fff15853a558cf..6b5e96b9209100 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/BUILD +++ b/src/main/java/com/google/devtools/build/lib/remote/BUILD @@ -179,6 +179,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/actions:artifacts", "//src/main/java/com/google/devtools/build/lib/actions:file_metadata", "//src/main/java/com/google/devtools/build/lib/remote/util", + "//src/main/java/com/google/devtools/build/lib/skyframe:tree_artifact_value", "//src/main/java/com/google/devtools/build/lib/vfs", "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", "//third_party:flogger", diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java index efbc146394b88f..ddf86d35f5b6d7 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java @@ -17,6 +17,7 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; +import static com.google.common.base.Preconditions.checkState; import static com.google.common.collect.ImmutableMap.toImmutableMap; import static com.google.common.collect.Streams.stream; @@ -38,6 +39,7 @@ import com.google.devtools.build.lib.vfs.FileSystem; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; +import com.google.devtools.build.lib.vfs.Symlinks; import com.google.devtools.build.lib.vfs.inmemoryfs.FileInfo; import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryContentInfo; import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem; @@ -53,7 +55,7 @@ /** * This is a basic implementation and incomplete implementation of an action file system that's been - * tuned to what native (non-spawn) actions in Bazel currently use. + * tuned to what internal (non-spawn) actions in Bazel currently use. * *

The implementation mostly delegates to the local file system except for the case where an * action input is a remotely stored action output. Most notably {@link @@ -62,7 +64,6 @@ *

This implementation only supports creating local action outputs. */ public class RemoteActionFileSystem extends DelegateFileSystem { - private final PathFragment execRoot; private final PathFragment outputBase; private final ActionInputMap inputArtifactData; @@ -101,7 +102,7 @@ protected FileSystem getLocalFileSystem() { /** Returns true if {@code path} is a file that's stored remotely. */ boolean isRemote(Path path) { - return getRemoteInputMetadata(path.asFragment()) != null; + return getRemoteMetadata(path.asFragment()) != null; } public void updateContext(MetadataInjector metadataInjector) { @@ -120,9 +121,13 @@ void flush() throws IOException { checkNotNull(metadataInjector, "metadataInjector is null"); for (Map.Entry entry : outputMapping.entrySet()) { - PathFragment execPath = entry.getKey(); - PathFragment path = execRoot.getRelative(execPath); + PathFragment path = execRoot.getRelative(entry.getKey()); Artifact output = entry.getValue(); + + if (maybeInjectMetadataForSymlink(path, output)) { + continue; + } + if (output.isTreeArtifact()) { if (remoteOutputTree.exists(path)) { SpecialArtifact parent = (SpecialArtifact) output; @@ -157,6 +162,91 @@ void flush() throws IOException { } } + /** + * Inject metadata for non-symlink outputs that were materialized as a symlink to a remote + * artifact. + * + *

If a non-symlink output is materialized as a symlink, the symlink has "copy" semantics, + * i.e., the output metadata is identical to that of the symlink target. For these artifacts, we + * inject their metadata instead of collecting it from the filesystem. This is done for two + * reasons: + * + *