Skip to content

Commit

Permalink
[6.0.0] Emit Tree objects in topological order (bazelbuild#16904)
Browse files Browse the repository at this point in the history
* Emit Tree objects in topological order

remote-apis PR 230 added a way where producers of Tree messages can    indicate that the directories contained within are stored in topological    order. The advantage of using such an ordering is that it permits    instantiation of such objects onto a local file system in a streaming    fashion. The same holds for lookups of individual paths.

Even though Bazel currently does not gain from this, this change at    least modifies Bazel's REv2 client to emit topologically sorted trees.    This makes it possible for tools such as Buildbarn's bb-browser to    process them more efficiently.

More details:
- bazelbuild/remote-apis#229
- bazelbuild/remote-apis#230

Closes bazelbuild#16463.

PiperOrigin-RevId: 487196375
Change-Id: Iafcfd617fc101fec7bfa943552113ce57ab8041b

* Emit Tree objects in topological order

remote-apis PR 230 added a way where producers of Tree messages can
indicate that the directories contained within are stored in topological
order. The advantage of using such an ordering is that it permits
instantiation of such objects onto a local file system in a streaming
fashion. The same holds for lookups of individual paths.

Even though Bazel currently does not gain from this, this change at
least modifies Bazel's REv2 client to emit topologically sorted trees.
This makes it possible for tools such as Buildbarn's bb-browser to
process them more efficiently.

More details:
- bazelbuild/remote-apis#229
- bazelbuild/remote-apis#230

Partial commit for third_party/*, see bazelbuild#16463.

Signed-off-by: Sunil Gowroji <sgowroji@google.com>

Signed-off-by: Sunil Gowroji <sgowroji@google.com>
Co-authored-by: Ed Schouten <eschouten@apple.com>
  • Loading branch information
ShreeM01 and EdSchouten authored Dec 2, 2022
1 parent f717d6a commit 7ccc661
Show file tree
Hide file tree
Showing 7 changed files with 325 additions and 44 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Lists;
import com.google.devtools.build.lib.actions.ActionExecutionMetadata;
import com.google.devtools.build.lib.actions.ActionUploadFinishedEvent;
import com.google.devtools.build.lib.actions.ActionUploadStartedEvent;
Expand All @@ -54,20 +55,24 @@
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.Symlinks;
import com.google.protobuf.ByteString;
import com.google.protobuf.CodedOutputStream;
import com.google.protobuf.Timestamp;
import io.reactivex.rxjava3.core.Completable;
import io.reactivex.rxjava3.core.Flowable;
import io.reactivex.rxjava3.core.Single;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.time.Duration;
import java.time.Instant;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Comparator;
import java.util.HashMap;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;
import javax.annotation.Nullable;

Expand Down Expand Up @@ -303,25 +308,43 @@ private void addFile(Digest digest, Path file) throws IOException {
digestToFile.put(digest, file);
}

// Field numbers of the 'root' and 'directory' fields in the Tree message.
private static final int TREE_ROOT_FIELD_NUMBER =
Tree.getDescriptor().findFieldByName("root").getNumber();
private static final int TREE_CHILDREN_FIELD_NUMBER =
Tree.getDescriptor().findFieldByName("children").getNumber();

private void addDirectory(Path dir) throws ExecException, IOException {
Tree.Builder tree = Tree.newBuilder();
Directory root = computeDirectory(dir, tree);
tree.setRoot(root);
Set<ByteString> directories = new LinkedHashSet<>();
var ignored = computeDirectory(dir, directories);

// Convert individual Directory messages to a Tree message. As we want the
// records to be topologically sorted (parents before children), we iterate
// over the directories in reverse insertion order.
ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream();
CodedOutputStream codedOutputStream = CodedOutputStream.newInstance(byteArrayOutputStream);
int fieldNumber = TREE_ROOT_FIELD_NUMBER;
for (ByteString directory : Lists.reverse(new ArrayList<ByteString>(directories))) {
codedOutputStream.writeBytes(fieldNumber, directory);
fieldNumber = TREE_CHILDREN_FIELD_NUMBER;
}
codedOutputStream.flush();

ByteString data = tree.build().toByteString();
ByteString data = ByteString.copyFrom(byteArrayOutputStream.toByteArray());
Digest digest = digestUtil.compute(data.toByteArray());

if (result != null) {
result
.addOutputDirectoriesBuilder()
.setPath(remotePathResolver.localPathToOutputPath(dir))
.setTreeDigest(digest);
.setTreeDigest(digest)
.setIsTopologicallySorted(true);
}

digestToBlobs.put(digest, data);
}

private Directory computeDirectory(Path path, Tree.Builder tree)
private ByteString computeDirectory(Path path, Set<ByteString> directories)
throws ExecException, IOException {
Directory.Builder b = Directory.newBuilder();

Expand All @@ -332,9 +355,8 @@ private Directory computeDirectory(Path path, Tree.Builder tree)
String name = dirent.getName();
Path child = path.getRelative(name);
if (dirent.getType() == Dirent.Type.DIRECTORY) {
Directory dir = computeDirectory(child, tree);
b.addDirectoriesBuilder().setName(name).setDigest(digestUtil.compute(dir));
tree.addChildren(dir);
ByteString dir = computeDirectory(child, directories);
b.addDirectoriesBuilder().setName(name).setDigest(digestUtil.compute(dir.toByteArray()));
} else if (dirent.getType() == Dirent.Type.SYMLINK) {
PathFragment target = child.readSymbolicLink();
if (!followSymlinks && !target.isAbsolute()) {
Expand All @@ -353,9 +375,8 @@ private Directory computeDirectory(Path path, Tree.Builder tree)
b.addFilesBuilder().setName(name).setDigest(digest).setIsExecutable(child.isExecutable());
digestToFile.put(digest, child);
} else if (statFollow.isDirectory()) {
Directory dir = computeDirectory(child, tree);
b.addDirectoriesBuilder().setName(name).setDigest(digestUtil.compute(dir));
tree.addChildren(dir);
ByteString dir = computeDirectory(child, directories);
b.addDirectoriesBuilder().setName(name).setDigest(digestUtil.compute(dir.toByteArray()));
} else {
illegalOutput(child);
}
Expand All @@ -368,7 +389,9 @@ private Directory computeDirectory(Path path, Tree.Builder tree)
}
}

return b.build();
ByteString directory = b.build().toByteString();
directories.add(directory);
return directory;
}

private void illegalOutput(Path path) throws ExecException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,11 @@ public void updateActionResult(
.setPath("a/foo")
.setDigest(fooDigest)
.setIsExecutable(true);
expectedResult.addOutputDirectoriesBuilder().setPath("bar").setTreeDigest(barDigest);
expectedResult
.addOutputDirectoriesBuilder()
.setPath("bar")
.setTreeDigest(barDigest)
.setIsTopologicallySorted(true);
assertThat(result).isEqualTo(expectedResult.build());
}

Expand Down Expand Up @@ -409,7 +413,11 @@ public void updateActionResult(

ActionResult result = uploadDirectory(remoteCache, ImmutableList.<Path>of(barDir));
ActionResult.Builder expectedResult = ActionResult.newBuilder();
expectedResult.addOutputDirectoriesBuilder().setPath("bar").setTreeDigest(barDigest);
expectedResult
.addOutputDirectoriesBuilder()
.setPath("bar")
.setTreeDigest(barDigest)
.setIsTopologicallySorted(true);
assertThat(result).isEqualTo(expectedResult.build());
}

Expand Down Expand Up @@ -472,7 +480,11 @@ public void updateActionResult(

ActionResult result = uploadDirectory(remoteCache, ImmutableList.of(barDir));
ActionResult.Builder expectedResult = ActionResult.newBuilder();
expectedResult.addOutputDirectoriesBuilder().setPath("bar").setTreeDigest(barDigest);
expectedResult
.addOutputDirectoriesBuilder()
.setPath("bar")
.setTreeDigest(barDigest)
.setIsTopologicallySorted(true);
assertThat(result).isEqualTo(expectedResult.build());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1314,7 +1314,11 @@ public void uploadOutputs_uploadDirectory_works() throws Exception {
.setPath("outputs/a/foo")
.setDigest(fooDigest)
.setIsExecutable(true);
expectedResult.addOutputDirectoriesBuilder().setPath("outputs/bar").setTreeDigest(barDigest);
expectedResult
.addOutputDirectoriesBuilder()
.setPath("outputs/bar")
.setTreeDigest(barDigest)
.setIsTopologicallySorted(true);
assertThat(manifest.getActionResult()).isEqualTo(expectedResult.build());

ImmutableList<Digest> toQuery = ImmutableList.of(fooDigest, quxDigest, barDigest);
Expand Down Expand Up @@ -1352,7 +1356,11 @@ public void uploadOutputs_uploadEmptyDirectory_works() throws Exception {

// assert
ActionResult.Builder expectedResult = ActionResult.newBuilder();
expectedResult.addOutputDirectoriesBuilder().setPath("outputs/bar").setTreeDigest(barDigest);
expectedResult
.addOutputDirectoriesBuilder()
.setPath("outputs/bar")
.setTreeDigest(barDigest)
.setIsTopologicallySorted(true);
assertThat(manifest.getActionResult()).isEqualTo(expectedResult.build());
assertThat(
getFromFuture(
Expand Down Expand Up @@ -1417,7 +1425,11 @@ public void uploadOutputs_uploadNestedDirectory_works() throws Exception {

// assert
ActionResult.Builder expectedResult = ActionResult.newBuilder();
expectedResult.addOutputDirectoriesBuilder().setPath("outputs/bar").setTreeDigest(barDigest);
expectedResult
.addOutputDirectoriesBuilder()
.setPath("outputs/bar")
.setTreeDigest(barDigest)
.setIsTopologicallySorted(true);
assertThat(manifest.getActionResult()).isEqualTo(expectedResult.build());

ImmutableList<Digest> toQuery = ImmutableList.of(wobbleDigest, quxDigest, barDigest);
Expand Down
Loading

0 comments on commit 7ccc661

Please sign in to comment.