Save space when serializing ArtifactRoot: serialize the base exec root as a likely constant after https://github.com/bazelbuild/bazel/commit/f93c72c34e1d1d3211c834f2bec432eed47b763a and reconstruct this root from the execPath on deserialization.
PiperOrigin-RevId: 303997590
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ArtifactRoot.java b/src/main/java/com/google/devtools/build/lib/actions/ArtifactRoot.java
index 6eba8cf..4ad8ae6 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ArtifactRoot.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ArtifactRoot.java
@@ -89,8 +89,13 @@
@AutoCodec.VisibleForSerialization
@AutoCodec.Instantiator
- static ArtifactRoot createForSerialization(Root root, PathFragment execPath, RootType rootType) {
- return INTERNER.intern(new ArtifactRoot(root, execPath, rootType));
+ static ArtifactRoot createForSerialization(
+ Root rootForSerialization, PathFragment execPath, RootType rootType) {
+ if (rootType != RootType.Output) {
+ return INTERNER.intern(new ArtifactRoot(rootForSerialization, execPath, rootType));
+ }
+ return asDerivedRoot(
+ rootForSerialization.asPath(), execPath.getSegments().toArray(new String[0]));
}
@AutoCodec.VisibleForSerialization
@@ -149,6 +154,31 @@
return Objects.hash(root, execPath, rootType);
}
+ /**
+ * The Root of a derived ArtifactRoot contains the exec path. In order to avoid duplicating that
+ * path, and enable the Root to be serialized as a constant, we return the "exec root" Root here,
+ * by stripping the exec path. That Root is likely to be serialized as a constant by {@link
+ * Root.RootCodec}, saving a lot of serialized bytes on the wire.
+ */
+ @SuppressWarnings("unused") // Used by @AutoCodec.
+ Root getRootForSerialization() {
+ if (rootType != RootType.Output) {
+ return root;
+ }
+ // Find fragment of root that does not include execPath and return just that root. It is likely
+ // to be serialized as a constant by RootCodec. For instance, if the original exec root was
+ // /execroot, and this root was /execroot/bazel-out/bin, with execPath bazel-out/bin, then we
+ // just serialize /execroot and bazel-out/bin separately.
+ // We just want to strip execPath from root, but I don't know a trivial way to do that.
+ PathFragment rootFragment = root.asPath().asFragment();
+ return Root.fromPath(
+ root.asPath()
+ .getFileSystem()
+ .getPath(
+ rootFragment.subFragment(
+ 0, rootFragment.segmentCount() - execPath.segmentCount())));
+ }
+
@Override
public boolean equals(Object o) {
if (o == this) {
diff --git a/src/test/java/com/google/devtools/build/lib/actions/ArtifactRootTest.java b/src/test/java/com/google/devtools/build/lib/actions/ArtifactRootTest.java
index 6ecb16c..6df05ff 100644
--- a/src/test/java/com/google/devtools/build/lib/actions/ArtifactRootTest.java
+++ b/src/test/java/com/google/devtools/build/lib/actions/ArtifactRootTest.java
@@ -16,11 +16,18 @@
import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.assertThrows;
+import com.google.common.collect.ImmutableMap;
import com.google.common.testing.EqualsTester;
+import com.google.devtools.build.lib.skyframe.serialization.AutoRegistry;
+import com.google.devtools.build.lib.skyframe.serialization.ObjectCodecRegistry;
+import com.google.devtools.build.lib.skyframe.serialization.ObjectCodecs;
+import com.google.devtools.build.lib.skyframe.serialization.SerializationException;
import com.google.devtools.build.lib.testutil.Scratch;
+import com.google.devtools.build.lib.vfs.FileSystem;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.Root;
+import com.google.protobuf.ByteString;
import java.io.IOException;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -89,6 +96,28 @@
}
@Test
+ public void derivedSerialization() throws IOException, SerializationException {
+ Path execRoot = scratch.dir("/thisisaveryverylongexecrootthatwedontwanttoserialize");
+ ArtifactRoot derivedRoot = ArtifactRoot.asDerivedRoot(execRoot, "first", "second", "third");
+ ObjectCodecRegistry registry = AutoRegistry.get();
+ ImmutableMap<Class<?>, Object> dependencies =
+ ImmutableMap.<Class<?>, Object>builder()
+ .put(FileSystem.class, scratch.getFileSystem())
+ .put(
+ Root.RootCodecDependencies.class,
+ new Root.RootCodecDependencies(/*likelyPopularRoot=*/ Root.fromPath(execRoot)))
+ .build();
+ ObjectCodecRegistry.Builder registryBuilder = registry.getBuilder();
+ for (Object val : dependencies.values()) {
+ registryBuilder.addReferenceConstant(val);
+ }
+ ObjectCodecs objectCodecs = new ObjectCodecs(registryBuilder.build(), dependencies);
+ ByteString serialized = objectCodecs.serialize(derivedRoot);
+ // 28 bytes as of 2020/03/31.
+ assertThat(serialized.size()).isLessThan(30);
+ }
+
+ @Test
public void testEquals() throws IOException {
Path execRoot = scratch.dir("/exec");
String rootSegment = "root";
@@ -104,7 +133,7 @@
false, rootA, ArtifactRoot.asDerivedRoot(otherRootDir, "exec", rootSegment));
}
- public void assertEqualsAndHashCode(boolean expected, Object a, Object b) {
+ private void assertEqualsAndHashCode(boolean expected, Object a, Object b) {
if (expected) {
new EqualsTester().addEqualityGroup(b, a).testEquals();
} else {
