Skip to content

Commit

Permalink
[7.1.0] Include the digest hash function in the compact execution log. (
Browse files Browse the repository at this point in the history
#21174)

The digest function is the same for every digest computed by an
invocation. To save space, it is stored once in the Invocation log
entry.

Also embed a Digest proto in File and Spawn messages, with the proviso
that the hash_function_name field will be empty. The size is always
required because the hash alone might not be sufficient to retrieve an
entry from a remote cache.

PiperOrigin-RevId: 602684865
Change-Id: I6f4882051aad9ea72bea0232f8ad4b2cd7b8e335
  • Loading branch information
tjgq authored Feb 1, 2024
1 parent 2f4bad0 commit 3f11b34
Show file tree
Hide file tree
Showing 6 changed files with 103 additions and 55 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,8 @@ public CompactSpawnLogContext(
this.digestHashFunction = digestHashFunction;
this.xattrProvider = xattrProvider;
this.outputStream = getOutputStream(outputPath);

logInvocation();
}

private static MessageOutputStream<ExecLogEntry> getOutputStream(Path path) throws IOException {
Expand All @@ -104,6 +106,16 @@ private static MessageOutputStream<ExecLogEntry> getOutputStream(Path path) thro
path.toString(), new ZstdOutputStream(new BufferedOutputStream(path.getOutputStream())));
}

private void logInvocation() throws IOException {
logEntry(
null,
() ->
ExecLogEntry.newBuilder()
.setInvocation(
ExecLogEntry.Invocation.newBuilder()
.setHashFunctionName(digestHashFunction.toString())));
}

@Override
public void logSpawn(
Spawn spawn,
Expand Down Expand Up @@ -156,7 +168,7 @@ public void logSpawn(
builder.setRemoteCacheable(Spawns.mayBeCachedRemotely(spawn));

if (result.getDigest() != null) {
builder.setDigest(result.getDigest().getHash());
builder.setDigest(result.getDigest().toBuilder().clearHashFunctionName().build());
}

builder.setTimeoutMillis(timeout.toMillis());
Expand Down Expand Up @@ -306,9 +318,15 @@ private int logFile(ActionInput input, Path path, InputMetadataProvider inputMet
builder.setPath(input.getExecPathString());

Digest digest =
computeDigest(input, path, inputMetadataProvider, xattrProvider, digestHashFunction);
builder.setDigest(digest.getHash());
builder.setSizeBytes(digest.getSizeBytes());
computeDigest(
input,
path,
inputMetadataProvider,
xattrProvider,
digestHashFunction,
/* includeHashFunctionName= */ false);

builder.setDigest(digest);

return ExecLogEntry.newBuilder().setFile(builder);
});
Expand Down Expand Up @@ -381,13 +399,15 @@ private int logRunfilesDirectory(

Digest digest =
computeDigest(
input, path, inputMetadataProvider, xattrProvider, digestHashFunction);
input,
path,
inputMetadataProvider,
xattrProvider,
digestHashFunction,
/* includeHashFunctionName= */ false);

builder.addFiles(
ExecLogEntry.File.newBuilder()
.setPath(runfilesPath)
.setSizeBytes(digest.getSizeBytes())
.setDigest(digest.getHash()));
ExecLogEntry.File.newBuilder().setPath(runfilesPath).setDigest(digest));
}

return ExecLogEntry.newBuilder().setDirectory(builder);
Expand Down Expand Up @@ -422,14 +442,14 @@ private ImmutableList<ExecLogEntry.File> expandDirectory(

Digest digest =
computeDigest(
/* input= */ null, child, inputMetadataProvider, xattrProvider, digestHashFunction);

builder.add(
ExecLogEntry.File.newBuilder()
.setPath(childPath)
.setSizeBytes(digest.getSizeBytes())
.setDigest(digest.getHash())
.build());
/* input= */ null,
child,
inputMetadataProvider,
xattrProvider,
digestHashFunction,
/* includeHashFunctionName= */ false);

builder.add(ExecLogEntry.File.newBuilder().setPath(childPath).setDigest(digest).build());
}
}
return builder.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,12 @@ public void logSpawn(

Digest digest =
computeDigest(
input, contentPath, inputMetadataProvider, xattrProvider, digestHashFunction);
input,
contentPath,
inputMetadataProvider,
xattrProvider,
digestHashFunction,
/* includeHashFunctionName= */ true);

builder
.addInputsBuilder()
Expand Down Expand Up @@ -213,7 +218,12 @@ public void logSpawn(
.setPath(output.getExecPathString())
.setDigest(
computeDigest(
output, path, inputMetadataProvider, xattrProvider, digestHashFunction));
output,
path,
inputMetadataProvider,
xattrProvider,
digestHashFunction,
/* includeHashFunctionName= */ true));
}
} catch (IOException ex) {
logger.atWarning().withCause(ex).log("Error computing spawn output properties");
Expand Down Expand Up @@ -332,7 +342,8 @@ private void listDirectoryContents(
childContentPath,
inputMetadataProvider,
xattrProvider,
digestHashFunction))
digestHashFunction,
/* includeHashFunctionName= */ true))
.setIsTool(isTool)
.build());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,9 +139,14 @@ static Digest computeDigest(
Path path,
InputMetadataProvider inputMetadataProvider,
XattrProvider xattrProvider,
DigestHashFunction digestHashFunction)
DigestHashFunction digestHashFunction,
boolean includeHashFunctionName)
throws IOException {
Digest.Builder builder = Digest.newBuilder().setHashFunctionName(digestHashFunction.toString());
Digest.Builder builder = Digest.newBuilder();

if (includeHashFunctionName) {
builder.setHashFunctionName(digestHashFunction.toString());
}

if (input != null) {
if (input instanceof VirtualActionInput) {
Expand Down
25 changes: 14 additions & 11 deletions src/main/protobuf/spawn.proto
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,13 @@ option java_outer_classname = "Protos";

// Digest of a file or action cache entry.
message Digest {
// The content hash.
// The content hash as a lowercase hex string including any leading zeroes.
string hash = 1;

// The original content size in bytes.
int64 size_bytes = 2;

// The digest function that was used to generate the hash.
// This is not an enum for compatibility reasons, and also because the
// purpose of these logs is to enable analysis by comparison of multiple
// builds. So, from the programmatic perspective, this is an opaque field.
// The name of the digest function used to compute the hash.
string hash_function_name = 3;
}

Expand All @@ -46,6 +43,7 @@ message File {
string path = 1;

// File digest.
// May be omitted for empty files.
Digest digest = 2;

// Whether the file is a tool.
Expand Down Expand Up @@ -211,16 +209,19 @@ message SpawnExec {
message ExecLogEntry {
// Information pertaining to the entire invocation.
// May appear at most once in the initial position.
message Invocation {}
message Invocation {
// The hash function used to compute digests.
string hash_function_name = 1;
}

// An input or output file.
message File {
// The file path.
string path = 1;
// The file size in bytes.
int64 size_bytes = 2;
// A digest of the file contents. Unset if the file is empty.
string digest = 3;
// A digest of the file contents.
// The hash function name is omitted. It can be obtained from Invocation.
// May be omitted for empty files.
Digest digest = 2;
}

// An input or output directory.
Expand Down Expand Up @@ -305,7 +306,9 @@ message ExecLogEntry {
bool remote_cacheable = 15;

// See SpawnExec.digest.
string digest = 16;
// The hash function name is omitted. It can be obtained from Invocation.
// Unset if the file is empty.
Digest digest = 16;

// See SpawnExec.timeout_millis.
int64 timeout_millis = 17;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import com.google.devtools.build.lib.actions.util.ActionsTestUtil;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.exec.Protos.Digest;
import com.google.devtools.build.lib.exec.Protos.ExecLogEntry;
import com.google.devtools.build.lib.exec.Protos.File;
import com.google.devtools.build.lib.exec.Protos.SpawnExec;
Expand Down Expand Up @@ -128,14 +127,20 @@ protected void closeAndAssertLog(SpawnLogContext context, SpawnExec... expected)
context.close();

HashMap<Integer, ExecLogEntry> entryMap = new HashMap<>();
String hashFunctionName = "";

ArrayList<SpawnExec> actual = new ArrayList<>();
try (InputStream in = new ZstdInputStream(logPath.getInputStream())) {
ExecLogEntry e;
while ((e = ExecLogEntry.parseDelimitedFrom(in)) != null) {
entryMap.put(e.getId(), e);

if (e.hasInvocation()) {
hashFunctionName = e.getInvocation().getHashFunctionName();
}

if (e.hasSpawn()) {
actual.add(reconstructSpawnExec(e.getSpawn(), entryMap));
actual.add(reconstructSpawnExec(e.getSpawn(), hashFunctionName, entryMap));
}
}
}
Expand All @@ -144,7 +149,7 @@ protected void closeAndAssertLog(SpawnLogContext context, SpawnExec... expected)
}

private SpawnExec reconstructSpawnExec(
ExecLogEntry.Spawn entry, Map<Integer, ExecLogEntry> entryMap) {
ExecLogEntry.Spawn entry, String hashFunctionName, Map<Integer, ExecLogEntry> entryMap) {
SpawnExec.Builder builder =
SpawnExec.newBuilder()
.addAllCommandArgs(entry.getArgsList())
Expand All @@ -165,8 +170,10 @@ private SpawnExec reconstructSpawnExec(
builder.setPlatform(entry.getPlatform());
}

SortedMap<String, File> inputs = reconstructInputs(entry.getInputSetId(), entryMap);
SortedMap<String, File> toolInputs = reconstructInputs(entry.getToolSetId(), entryMap);
SortedMap<String, File> inputs =
reconstructInputs(entry.getInputSetId(), hashFunctionName, entryMap);
SortedMap<String, File> toolInputs =
reconstructInputs(entry.getToolSetId(), hashFunctionName, entryMap);

for (Map.Entry<String, File> e : inputs.entrySet()) {
File file = e.getValue();
Expand All @@ -181,14 +188,14 @@ private SpawnExec reconstructSpawnExec(
case FILE_ID:
ExecLogEntry.File file = checkNotNull(entryMap.get(output.getFileId())).getFile();
builder.addListedOutputs(file.getPath());
builder.addActualOutputs(reconstructFile(/* parentDir= */ null, file));
builder.addActualOutputs(reconstructFile(/* parentDir= */ null, file, hashFunctionName));
break;
case DIRECTORY_ID:
ExecLogEntry.Directory dir =
checkNotNull(entryMap.get(output.getDirectoryId())).getDirectory();
builder.addListedOutputs(dir.getPath());
for (ExecLogEntry.File dirFile : dir.getFilesList()) {
builder.addActualOutputs(reconstructFile(dir, dirFile));
builder.addActualOutputs(reconstructFile(dir, dirFile, hashFunctionName));
}
break;
case MISSING_PATH:
Expand All @@ -199,15 +206,16 @@ private SpawnExec reconstructSpawnExec(
}
}

if (!entry.getDigest().isEmpty()) {
builder.setDigest(Digest.newBuilder().setHash(entry.getDigest()).build());
if (entry.hasDigest()) {
builder.setDigest(
entry.getDigest().toBuilder().setHashFunctionName(hashFunctionName).build());
}

return builder.build();
}

private SortedMap<String, File> reconstructInputs(
int setId, Map<Integer, ExecLogEntry> entryMap) {
int setId, String hashFunctionName, Map<Integer, ExecLogEntry> entryMap) {
TreeMap<String, File> inputs = new TreeMap<>();
ArrayDeque<Integer> setsToVisit = new ArrayDeque<>();
if (setId != 0) {
Expand All @@ -218,29 +226,30 @@ private SortedMap<String, File> reconstructInputs(
checkNotNull(entryMap.get(setsToVisit.removeFirst())).getInputSet();
for (int fileId : set.getFileIdsList()) {
ExecLogEntry.File file = checkNotNull(entryMap.get(fileId)).getFile();
inputs.put(file.getPath(), reconstructFile(null, file));
inputs.put(file.getPath(), reconstructFile(null, file, hashFunctionName));
}
for (int dirId : set.getDirectoryIdsList()) {
ExecLogEntry.Directory dir = checkNotNull(entryMap.get(dirId)).getDirectory();
for (ExecLogEntry.File dirFile : dir.getFilesList()) {
inputs.put(dirFile.getPath(), reconstructFile(dir, dirFile));
inputs.put(dirFile.getPath(), reconstructFile(dir, dirFile, hashFunctionName));
}
}
setsToVisit.addAll(set.getTransitiveSetIdsList());
}
return inputs;
}

private File reconstructFile(@Nullable ExecLogEntry.Directory parentDir, ExecLogEntry.File file) {
String path = parentDir != null ? parentDir.getPath() + "/" + file.getPath() : file.getPath();
File.Builder builder = File.newBuilder().setPath(path);
if (file.getSizeBytes() > 0) {
builder.setDigest(
Digest.newBuilder()
.setSizeBytes(file.getSizeBytes())
.setHash(file.getDigest())
.setHashFunctionName(digestHashFunction.toString()));
private File reconstructFile(
@Nullable ExecLogEntry.Directory parentDir, ExecLogEntry.File file, String hashFunctionName) {
File.Builder builder = File.newBuilder();

builder.setPath(
parentDir != null ? parentDir.getPath() + "/" + file.getPath() : file.getPath());

if (file.hasDigest()) {
builder.setDigest(file.getDigest().toBuilder().setHashFunctionName(hashFunctionName).build());
}

return builder.build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -630,7 +630,7 @@ public void testCacheHit() throws Exception {
public void testDigest() throws Exception {
SpawnLogContext context = createSpawnLogContext();

Digest digest = Digest.newBuilder().setHash("deadbeef").build();
Digest digest = getDigest("something");

SpawnResult result = defaultSpawnResultBuilder().setDigest(digest).build();

Expand Down

0 comments on commit 3f11b34

Please sign in to comment.