diff options
author | 2018-04-03 16:50:37 -0700 | |
---|---|---|
committer | 2018-04-03 16:52:32 -0700 | |
commit | 94164097c112b41b94ddf45d7f74b9092b74656b (patch) | |
tree | 6f5d2d4e76989bceea76e2e5e5c1f41b162caa6b /src/main/java/com/google | |
parent | 7e8110b8dedfef1562e7337ad67b63525c9ae8b1 (diff) |
Don't do serialization of empty/singleton nested sets into a child CodedOutputStream. It creates immense amounts of garbage and we don't ever use the result: it's only used for Object[] children anyway.
We can consider removing the child CodedOutputStream entirely and relying on normal serialization memoization, but for now, let's just do the simple thing.
Also fix a weird code-only bug that had been there since NestedSetCodec was written (I think): NestedSet.EMPTY_CHILDREN is an Object[], and therefore we never took the fast path of just writing 0 and moving on. While the code as written was misleading, the bits written to the output stream were the same, until this change, when there was a divergence.
PiperOrigin-RevId: 191520712
Diffstat (limited to 'src/main/java/com/google')
-rw-r--r-- | src/main/java/com/google/devtools/build/lib/collect/nestedset/NestedSetCodec.java | 59 |
1 files changed, 30 insertions, 29 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/collect/nestedset/NestedSetCodec.java b/src/main/java/com/google/devtools/build/lib/collect/nestedset/NestedSetCodec.java index 14369814af..c564f32f19 100644 --- a/src/main/java/com/google/devtools/build/lib/collect/nestedset/NestedSetCodec.java +++ b/src/main/java/com/google/devtools/build/lib/collect/nestedset/NestedSetCodec.java @@ -102,25 +102,26 @@ public class NestedSetCodec<T> implements ObjectCodec<NestedSet<T>> { CodedOutputStream codedOut, Map<Object, byte[]> childToDigest) throws IOException, SerializationException { - // Serialize nested set into an inner byte array so we can take its digest - ByteArrayOutputStream childOutputStream = new ByteArrayOutputStream(); - HashingOutputStream hashingOutputStream = - new HashingOutputStream(Hashing.md5(), childOutputStream); - CodedOutputStream childCodedOut = CodedOutputStream.newInstance(hashingOutputStream); - if (children instanceof Object[]) { + if (children == NestedSet.EMPTY_CHILDREN) { + // Empty set + codedOut.writeInt32NoTag(0); + } else if (children instanceof Object[]) { + // Serialize nested set into an inner byte array so we can take its digest. + ByteArrayOutputStream childOutputStream = new ByteArrayOutputStream(); + HashingOutputStream hashingOutputStream = + new HashingOutputStream(Hashing.md5(), childOutputStream); + CodedOutputStream childCodedOut = CodedOutputStream.newInstance(hashingOutputStream); serializeMultiItemChildArray(context, (Object[]) children, childToDigest, childCodedOut); - } else if (children != NestedSet.EMPTY_CHILDREN) { - serializeSingleItemChildArray(context, children, childCodedOut); + childCodedOut.flush(); + byte[] digest = hashingOutputStream.hash().asBytes(); + codedOut.writeInt32NoTag(((Object[]) children).length); + codedOut.writeByteArrayNoTag(digest); + byte[] childBytes = childOutputStream.toByteArray(); + codedOut.writeByteArrayNoTag(childBytes); + childToDigest.put(children, digest); } else { - // Empty set - childCodedOut.writeInt32NoTag(0); + serializeSingleItemChildArray(context, children, codedOut); } - childCodedOut.flush(); - byte[] digest = hashingOutputStream.hash().asBytes(); - codedOut.writeByteArrayNoTag(digest); - byte[] childBytes = childOutputStream.toByteArray(); - codedOut.writeByteArrayNoTag(childBytes); - childToDigest.put(children, digest); } private void serializeMultiItemChildArray( @@ -129,7 +130,6 @@ public class NestedSetCodec<T> implements ObjectCodec<NestedSet<T>> { Map<Object, byte[]> childToDigest, CodedOutputStream childCodedOut) throws IOException, SerializationException { - childCodedOut.writeInt32NoTag(children.length); for (Object child : children) { if (child instanceof Object[]) { byte[] digest = @@ -146,11 +146,11 @@ public class NestedSetCodec<T> implements ObjectCodec<NestedSet<T>> { } private void serializeSingleItemChildArray( - SerializationContext context, Object children, CodedOutputStream childCodedOut) + SerializationContext context, Object children, CodedOutputStream codedOut) throws IOException, SerializationException { - childCodedOut.writeInt32NoTag(1); + codedOut.writeInt32NoTag(1); T singleChild = cast(children); - context.serialize(singleChild, childCodedOut); + context.serialize(singleChild, codedOut); } private Object deserializeOneNestedSet( @@ -158,22 +158,23 @@ public class NestedSetCodec<T> implements ObjectCodec<NestedSet<T>> { CodedInputStream codedIn, Map<ByteString, Object> digestToChild) throws SerializationException, IOException { - ByteString digest = codedIn.readBytes(); - CodedInputStream childCodedIn = codedIn.readBytes().newCodedInput(); - childCodedIn.enableAliasing(true); // Allow efficient views of byte slices when reading digests - int childCount = childCodedIn.readInt32(); + int childCount = codedIn.readInt32(); final Object result; if (childCount > 1) { + ByteString digest = codedIn.readBytes(); + CodedInputStream childCodedIn = codedIn.readBytes().newCodedInput(); + childCodedIn.enableAliasing( + true); // Allow efficient views of byte slices when reading digests result = deserializeMultipleItemChildArray(context, childCodedIn, childCount, digestToChild); + digestToChild.put( + // Copy the ByteString to avoid keeping the full buffer from codedIn alive due to + // aliasing. + ByteString.copyFrom(digest.asReadOnlyByteBuffer()), result); } else if (childCount == 1) { - result = context.deserialize(childCodedIn); + result = context.deserialize(codedIn); } else { result = NestedSet.EMPTY_CHILDREN; } - digestToChild.put( - // Copy the ByteString to avoid keeping the full buffer from codedIn alive due to - // aliasing. - ByteString.copyFrom(digest.asReadOnlyByteBuffer()), result); return result; } |