Skip to content

Commit

Permalink
Avoid downloading the empty Tree message
Browse files Browse the repository at this point in the history
Valid empty `Tree` messages, which correspond to empty output directories, serialize to two bytes since they always contain an empty but present `Tree` message in the `root` field. Since such directories are very common (for example, as undeclared test output directories), it pays off to avoid downloading them.
  • Loading branch information
fmeum committed Feb 25, 2025
1 parent 6da4d12 commit c237626
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1158,6 +1158,15 @@ private static DirectoryMetadata parseDirectory(
return new DirectoryMetadata(filesBuilder.build(), symlinksBuilder.build());
}

// The Tree message representing an empty directory.
private static final Tree EMPTY_DIRECTORY =
Tree.newBuilder().setRoot(Directory.getDefaultInstance()).build();

static {
// See logic in parseActionResultMetadata below.
Preconditions.checkState(EMPTY_DIRECTORY.toByteString().size() == 2);
}

static ActionResultMetadata parseActionResultMetadata(
CombinedCache combinedCache,
DigestUtil digestUtil,
Expand All @@ -1171,13 +1180,25 @@ static ActionResultMetadata parseActionResultMetadata(
Maps.newHashMapWithExpectedSize(result.getOutputDirectoriesCount());
for (OutputDirectory dir : result.getOutputDirectoriesList()) {
var outputPath = dir.getPath();
dirMetadataDownloads.put(
remotePathResolver.outputPathToLocalPath(unicodeToInternal(outputPath)),
Futures.transformAsync(
combinedCache.downloadBlob(context, outputPath, dir.getTreeDigest()),
(treeBytes) ->
immediateFuture(Tree.parseFrom(treeBytes, ExtensionRegistry.getEmptyRegistry())),
directExecutor()));
var localPath = remotePathResolver.outputPathToLocalPath(unicodeToInternal(outputPath));
if (dir.getTreeDigest().getSizeBytes() == 2) {
// A valid Tree message contains at least a non-empty root field. The only way for a Tree
// message to have a size of 2 bytes is if the root field is the only non-empty field and
// the Directory message in the root field is empty, which corresponds to one byte for the
// LEN tag and field number and one byte for the zero-length varint. Since empty tree
// artifacts are relatively common (e.g., as the undeclared test output directory), we avoid
// downloading these messages here.
dirMetadataDownloads.put(localPath, immediateFuture(EMPTY_DIRECTORY));
} else {
dirMetadataDownloads.put(
localPath,
Futures.transformAsync(
combinedCache.downloadBlob(context, outputPath, dir.getTreeDigest()),
(treeBytes) ->
immediateFuture(
Tree.parseFrom(treeBytes, ExtensionRegistry.getEmptyRegistry())),
directExecutor()));
}
}

waitForBulkTransfer(dirMetadataDownloads.values(), /* cancelRemainingOnInterrupt= */ true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -589,8 +589,9 @@ public void downloadOutputs_emptyOutputDirectories_works() throws Exception {

// arrange
Tree barTreeMessage = Tree.newBuilder().setRoot(Directory.getDefaultInstance()).build();
Digest barTreeDigest =
cache.addContents(remoteActionExecutionContext, barTreeMessage.toByteArray());
// Don't add barTreeMessage to the cache, the Tree proto for an empty output directory is
// recognized by its digest.
Digest barTreeDigest = digestUtil.compute(barTreeMessage);
ActionResult.Builder builder = ActionResult.newBuilder();
builder.addOutputDirectoriesBuilder().setPath("outputs/a/bar").setTreeDigest(barTreeDigest);
RemoteActionResult result =
Expand Down

0 comments on commit c237626

Please # to comment.