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.

Sign up for GitHub

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

[7.1.0] Include the digest hash function in the compact execution log. #21174

Merged
merged 1 commit into from
Feb 1, 2024
Merged

[7.1.0] Include the digest hash function in the compact execution log. #21174

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
54 changes: 37 additions & 17 deletions src/main/java/com/google/devtools/build/lib/exec/CompactSpawnLogContext.java
View file
Open in desktop
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
17 changes: 14 additions & 3 deletions src/main/java/com/google/devtools/build/lib/exec/ExpandedSpawnLogContext.java
View file
Open in desktop
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
9 changes: 7 additions & 2 deletions src/main/java/com/google/devtools/build/lib/exec/SpawnLogContext.java
View file
Open in desktop
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
View file
Open in desktop
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
51 changes: 30 additions & 21 deletions src/test/java/com/google/devtools/build/lib/exec/CompactSpawnLogContextTest.java
View file
Open in desktop
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();
}
}
2 changes: 1 addition & 1 deletion src/test/java/com/google/devtools/build/lib/exec/SpawnLogContextTestBase.java
View file
Open in desktop
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
Loading