Skip to content

Commit 32b0f5a

Browse files
tjgqcopybara-github
authored andcommitted
Inject metadata when creating a filesystem symlink for a non-symlink artifact.
A ctx.actions.symlink whose output is a declare_file or declare_directory (as opposed to a declare_symlink) has "copy" semantics, i.e., the output artifact is indistinguishable from the referent except for its name; the symlink is just a filesystem-level optimization to avoid an actual copy, and is transparently resolved when collecting the action output metadata. When the symlink points to an artifact that was built remotely and without the bytes, we currently must download it before executing the symlink action, so that the output metadata can be constructed from the local filesystem. This change short-circuits the filesystem traversal by injecting output metadata, which is identical to the input plus a pointer to the original path. This is used by the prefetcher to avoid downloading the same files multiple times when they're symlinked more than once. Fixes #15678. Closes #16283. PiperOrigin-RevId: 482479466 Change-Id: I3d10bef315b76a99a9e97077f07cc467e084984a
1 parent 633348c commit 32b0f5a

16 files changed

+1310
-167
lines changed

src/main/java/com/google/devtools/build/lib/actions/FileArtifactValue.java

Lines changed: 92 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import java.io.IOException;
3737
import java.util.Arrays;
3838
import java.util.Objects;
39+
import java.util.Optional;
3940
import javax.annotation.Nullable;
4041

4142
/**
@@ -172,6 +173,19 @@ protected boolean couldBeModifiedByMetadata(FileArtifactValue lastKnown) {
172173
return true;
173174
}
174175

176+
/**
177+
* Optional materialization path.
178+
*
179+
* <p>If present, this artifact is a copy of another artifact. It is still tracked as a
180+
* non-symlink by Bazel, but materialized in the local filesystem as a symlink to the original
181+
* artifact, whose contents live at this location. This is used by {@link
182+
* com.google.devtools.build.lib.remote.AbstractActionInputPrefetcher} to implement zero-cost
183+
* copies of remotely stored artifacts.
184+
*/
185+
public Optional<PathFragment> getMaterializationExecPath() {
186+
return Optional.empty();
187+
}
188+
175189
/**
176190
* Marker interface for singleton implementations of this class.
177191
*
@@ -514,9 +528,7 @@ public long getModifiedTime() {
514528
@Override
515529
public String toString() {
516530
return MoreObjects.toStringHelper(this)
517-
.add(
518-
"digest",
519-
digest == null ? "(null)" : BaseEncoding.base16().lowerCase().encode(digest))
531+
.add("digest", BaseEncoding.base16().lowerCase().encode(digest))
520532
.add("size", size)
521533
.add("proxy", proxy)
522534
.toString();
@@ -535,10 +547,10 @@ protected boolean couldBeModifiedByMetadata(FileArtifactValue o) {
535547

536548
/** Metadata for remotely stored files. */
537549
public static class RemoteFileArtifactValue extends FileArtifactValue {
538-
private final byte[] digest;
539-
private final long size;
540-
private final int locationIndex;
541-
private final String actionId;
550+
protected final byte[] digest;
551+
protected final long size;
552+
protected final int locationIndex;
553+
protected final String actionId;
542554

543555
private RemoteFileArtifactValue(byte[] digest, long size, int locationIndex, String actionId) {
544556
this.digest = Preconditions.checkNotNull(digest, actionId);
@@ -556,6 +568,19 @@ public static RemoteFileArtifactValue create(byte[] digest, long size, int locat
556568
return new RemoteFileArtifactValue(digest, size, locationIndex, /* actionId= */ "");
557569
}
558570

571+
public static RemoteFileArtifactValue create(
572+
byte[] digest,
573+
long size,
574+
int locationIndex,
575+
String actionId,
576+
@Nullable PathFragment materializationExecPath) {
577+
if (materializationExecPath != null) {
578+
return new RemoteFileArtifactValueWithMaterializationPath(
579+
digest, size, locationIndex, actionId, materializationExecPath);
580+
}
581+
return new RemoteFileArtifactValue(digest, size, locationIndex, actionId);
582+
}
583+
559584
@Override
560585
public boolean equals(Object o) {
561586
if (!(o instanceof RemoteFileArtifactValue)) {
@@ -602,7 +627,7 @@ public String getActionId() {
602627
@Override
603628
public long getModifiedTime() {
604629
throw new UnsupportedOperationException(
605-
"RemoteFileArifactValue doesn't support getModifiedTime");
630+
"RemoteFileArtifactValue doesn't support getModifiedTime");
606631
}
607632

608633
@Override
@@ -626,6 +651,65 @@ public String toString() {
626651
.add("digest", bytesToString(digest))
627652
.add("size", size)
628653
.add("locationIndex", locationIndex)
654+
.add("actionId", actionId)
655+
.toString();
656+
}
657+
}
658+
659+
/**
660+
* A remote artifact that should be materialized in the local filesystem as a symlink to another
661+
* location.
662+
*
663+
* <p>See the documentation for {@link FileArtifactValue#getMaterializationExecPath}.
664+
*/
665+
public static final class RemoteFileArtifactValueWithMaterializationPath
666+
extends RemoteFileArtifactValue {
667+
private final PathFragment materializationExecPath;
668+
669+
private RemoteFileArtifactValueWithMaterializationPath(
670+
byte[] digest,
671+
long size,
672+
int locationIndex,
673+
String actionId,
674+
PathFragment materializationExecPath) {
675+
super(digest, size, locationIndex, actionId);
676+
this.materializationExecPath = Preconditions.checkNotNull(materializationExecPath);
677+
}
678+
679+
@Override
680+
public Optional<PathFragment> getMaterializationExecPath() {
681+
return Optional.ofNullable(materializationExecPath);
682+
}
683+
684+
@Override
685+
public boolean equals(Object o) {
686+
if (!(o instanceof RemoteFileArtifactValueWithMaterializationPath)) {
687+
return false;
688+
}
689+
690+
RemoteFileArtifactValueWithMaterializationPath that =
691+
(RemoteFileArtifactValueWithMaterializationPath) o;
692+
return Arrays.equals(digest, that.digest)
693+
&& size == that.size
694+
&& locationIndex == that.locationIndex
695+
&& Objects.equals(actionId, that.actionId)
696+
&& Objects.equals(materializationExecPath, that.materializationExecPath);
697+
}
698+
699+
@Override
700+
public int hashCode() {
701+
return Objects.hash(
702+
Arrays.hashCode(digest), size, locationIndex, actionId, materializationExecPath);
703+
}
704+
705+
@Override
706+
public String toString() {
707+
return MoreObjects.toStringHelper(this)
708+
.add("digest", bytesToString(digest))
709+
.add("size", size)
710+
.add("locationIndex", locationIndex)
711+
.add("actionId", actionId)
712+
.add("materializationExecPath", materializationExecPath)
629713
.toString();
630714
}
631715
}

src/main/java/com/google/devtools/build/lib/actions/cache/ActionCache.java

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -109,9 +109,10 @@ final class Entry {
109109
public abstract static class SerializableTreeArtifactValue {
110110
public static SerializableTreeArtifactValue create(
111111
ImmutableMap<String, RemoteFileArtifactValue> childValues,
112-
Optional<RemoteFileArtifactValue> archivedFileValue) {
112+
Optional<RemoteFileArtifactValue> archivedFileValue,
113+
Optional<PathFragment> materializationExecPath) {
113114
return new AutoValue_ActionCache_Entry_SerializableTreeArtifactValue(
114-
childValues, archivedFileValue);
115+
childValues, archivedFileValue, materializationExecPath);
115116
}
116117

117118
/**
@@ -138,17 +139,25 @@ public static Optional<SerializableTreeArtifactValue> createSerializable(
138139
.filter(ar -> ar.archivedFileValue().isRemote())
139140
.map(ar -> (RemoteFileArtifactValue) ar.archivedFileValue());
140141

141-
if (childValues.isEmpty() && archivedFileValue.isEmpty()) {
142+
Optional<PathFragment> materializationExecPath = treeMetadata.getMaterializationExecPath();
143+
144+
if (childValues.isEmpty()
145+
&& archivedFileValue.isEmpty()
146+
&& materializationExecPath.isEmpty()) {
142147
return Optional.empty();
143148
}
144149

145-
return Optional.of(SerializableTreeArtifactValue.create(childValues, archivedFileValue));
150+
return Optional.of(
151+
SerializableTreeArtifactValue.create(
152+
childValues, archivedFileValue, materializationExecPath));
146153
}
147154

148155
// A map from parentRelativePath to the file metadata
149156
public abstract ImmutableMap<String, RemoteFileArtifactValue> childValues();
150157

151158
public abstract Optional<RemoteFileArtifactValue> archivedFileValue();
159+
160+
public abstract Optional<PathFragment> materializationExecPath();
152161
}
153162

154163
public Entry(String key, Map<String, String> usedClientEnv, boolean discoversInputs) {

src/main/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCache.java

Lines changed: 50 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import com.google.devtools.build.lib.util.VarInt;
3737
import com.google.devtools.build.lib.vfs.DigestUtils;
3838
import com.google.devtools.build.lib.vfs.Path;
39+
import com.google.devtools.build.lib.vfs.PathFragment;
3940
import com.google.devtools.build.lib.vfs.SyscallCache;
4041
import com.google.devtools.build.lib.vfs.UnixGlob;
4142
import java.io.ByteArrayOutputStream;
@@ -75,7 +76,7 @@ public class CompactPersistentActionCache implements ActionCache {
7576

7677
private static final int NO_INPUT_DISCOVERY_COUNT = -1;
7778

78-
private static final int VERSION = 13;
79+
private static final int VERSION = 14;
7980

8081
private static final class ActionMap extends PersistentMap<Integer, byte[]> {
8182
private final Clock clock;
@@ -466,6 +467,14 @@ private static void encodeRemoteMetadata(
466467
VarInt.putVarInt(value.getLocationIndex(), sink);
467468

468469
VarInt.putVarInt(indexer.getOrCreateIndex(value.getActionId()), sink);
470+
471+
Optional<PathFragment> materializationExecPath = value.getMaterializationExecPath();
472+
if (materializationExecPath.isPresent()) {
473+
VarInt.putVarInt(1, sink);
474+
VarInt.putVarInt(indexer.getOrCreateIndex(materializationExecPath.get().toString()), sink);
475+
} else {
476+
VarInt.putVarInt(0, sink);
477+
}
469478
}
470479

471480
private static final int MAX_REMOTE_METADATA_SIZE =
@@ -484,7 +493,18 @@ private static RemoteFileArtifactValue decodeRemoteMetadata(
484493

485494
String actionId = getStringForIndex(indexer, VarInt.getVarInt(source));
486495

487-
return RemoteFileArtifactValue.create(digest, size, locationIndex, actionId);
496+
PathFragment materializationExecPath = null;
497+
int numMaterializationExecPath = VarInt.getVarInt(source);
498+
if (numMaterializationExecPath > 0) {
499+
if (numMaterializationExecPath != 1) {
500+
throw new IOException("Invalid presence marker for materialization path");
501+
}
502+
materializationExecPath =
503+
PathFragment.create(getStringForIndex(indexer, VarInt.getVarInt(source)));
504+
}
505+
506+
return RemoteFileArtifactValue.create(
507+
digest, size, locationIndex, actionId, materializationExecPath);
488508
}
489509

490510
/** @return action data encoded as a byte[] array. */
@@ -513,9 +533,12 @@ private static byte[] encode(StringIndexer indexer, ActionCache.Entry entry) thr
513533
+ MAX_REMOTE_METADATA_SIZE)
514534
* value.childValues().size();
515535

516-
maxOutputTreesSize += VarInt.MAX_VARINT_SIZE; // value.archivedFileValue() optional
517536
maxOutputTreesSize +=
518-
value.archivedFileValue().map(ignored -> MAX_REMOTE_METADATA_SIZE).orElse(0);
537+
(1 + VarInt.MAX_VARINT_SIZE) // value.archivedFileValue() optional
538+
+ value.archivedFileValue().map(ignored -> MAX_REMOTE_METADATA_SIZE).orElse(0);
539+
maxOutputTreesSize +=
540+
(1 + VarInt.MAX_VARINT_SIZE) // value.materializationExecPath() optional
541+
+ value.materializationExecPath().map(ignored -> MAX_REMOTE_METADATA_SIZE).orElse(0);
519542
}
520543

521544
// Estimate the size of the buffer:
@@ -578,6 +601,15 @@ private static byte[] encode(StringIndexer indexer, ActionCache.Entry entry) thr
578601
} else {
579602
VarInt.putVarInt(0, sink);
580603
}
604+
605+
Optional<PathFragment> materializationExecPath =
606+
serializableTreeArtifactValue.materializationExecPath();
607+
if (materializationExecPath.isPresent()) {
608+
VarInt.putVarInt(1, sink);
609+
VarInt.putVarInt(indexer.getOrCreateIndex(materializationExecPath.get().toString()), sink);
610+
} else {
611+
VarInt.putVarInt(0, sink);
612+
}
581613
}
582614

583615
return sink.toByteArray();
@@ -649,13 +681,25 @@ private static ActionCache.Entry decode(StringIndexer indexer, byte[] data) thro
649681
int numArchivedFileValue = VarInt.getVarInt(source);
650682
if (numArchivedFileValue > 0) {
651683
if (numArchivedFileValue != 1) {
652-
throw new IOException("Invalid number of archived artifacts");
684+
throw new IOException("Invalid presence marker for archived representation");
653685
}
654686
archivedFileValue = Optional.of(decodeRemoteMetadata(indexer, source));
655687
}
656688

689+
Optional<PathFragment> materializationExecPath = Optional.empty();
690+
int numMaterializationExecPath = VarInt.getVarInt(source);
691+
if (numMaterializationExecPath > 0) {
692+
if (numMaterializationExecPath != 1) {
693+
throw new IOException("Invalid presence marker for materialization path");
694+
}
695+
materializationExecPath =
696+
Optional.of(
697+
PathFragment.create(getStringForIndex(indexer, VarInt.getVarInt(source))));
698+
}
699+
657700
SerializableTreeArtifactValue value =
658-
SerializableTreeArtifactValue.create(childValues.buildOrThrow(), archivedFileValue);
701+
SerializableTreeArtifactValue.create(
702+
childValues.buildOrThrow(), archivedFileValue, materializationExecPath);
659703
outputTrees.put(treeKey, value);
660704
}
661705

0 commit comments

Comments
 (0)