Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inject metadata when creating a filesystem symlink for a non-symlink artifact. #16283

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import java.io.IOException;
import java.util.Arrays;
import java.util.Objects;
import java.util.Optional;
import javax.annotation.Nullable;

/**
Expand Down Expand Up @@ -172,6 +173,19 @@ protected boolean couldBeModifiedByMetadata(FileArtifactValue lastKnown) {
return true;
}

/**
* Optional materialization path.
*
* <p>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<PathFragment> getMaterializationExecPath() {
return Optional.empty();
}

/**
* Marker interface for singleton implementations of this class.
*
Expand Down Expand Up @@ -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();
Expand All @@ -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);
Expand All @@ -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)) {
Expand Down Expand Up @@ -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
Expand All @@ -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.
*
* <p>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<PathFragment> 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();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,10 @@ final class Entry {
public abstract static class SerializableTreeArtifactValue {
public static SerializableTreeArtifactValue create(
ImmutableMap<String, RemoteFileArtifactValue> childValues,
Optional<RemoteFileArtifactValue> archivedFileValue) {
Optional<RemoteFileArtifactValue> archivedFileValue,
Optional<PathFragment> materializationExecPath) {
return new AutoValue_ActionCache_Entry_SerializableTreeArtifactValue(
childValues, archivedFileValue);
childValues, archivedFileValue, materializationExecPath);
}

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

if (childValues.isEmpty() && archivedFileValue.isEmpty()) {
Optional<PathFragment> 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<String, RemoteFileArtifactValue> childValues();

public abstract Optional<RemoteFileArtifactValue> archivedFileValue();

public abstract Optional<PathFragment> materializationExecPath();
}

public Entry(String key, Map<String, String> usedClientEnv, boolean discoversInputs) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<Integer, byte[]> {
private final Clock clock;
Expand Down Expand Up @@ -466,6 +467,14 @@ private static void encodeRemoteMetadata(
VarInt.putVarInt(value.getLocationIndex(), sink);

VarInt.putVarInt(indexer.getOrCreateIndex(value.getActionId()), sink);

Optional<PathFragment> 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 =
Expand All @@ -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 presence marker for materialization 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. */
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -578,6 +601,15 @@ private static byte[] encode(StringIndexer indexer, ActionCache.Entry entry) thr
} else {
VarInt.putVarInt(0, sink);
}

Optional<PathFragment> 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();
Expand Down Expand Up @@ -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<PathFragment> materializationExecPath = Optional.empty();
int numMaterializationExecPath = VarInt.getVarInt(source);
if (numMaterializationExecPath > 0) {
if (numMaterializationExecPath != 1) {
throw new IOException("Invalid presence marker for materialization path");
}
materializationExecPath =
Optional.of(
PathFragment.create(getStringForIndex(indexer, VarInt.getVarInt(source))));
}

SerializableTreeArtifactValue value =
SerializableTreeArtifactValue.create(childValues.buildOrThrow(), archivedFileValue);
SerializableTreeArtifactValue.create(
childValues.buildOrThrow(), archivedFileValue, materializationExecPath);
outputTrees.put(treeKey, value);
}

Expand Down
Loading