Disable nested set sharing across multiple nested set deserialization sessions. This is incorrect in the presence of memoization: a single element may be serialized as just a pair of integers (type + memoization index). Lots of different nested sets may contain elements that are serialized this way, so they will have the same digests. We could consider doing a parallel hash computation, but for now just disable.

This is not a full rollback of https://github.com/bazelbuild/bazel/commit/39cef6d6a4a9e3ae80b11a9ccc0f35325852777c since there was a refactoring in it that it doesn't seem worth it to roll back.

PiperOrigin-RevId: 191509089

2 files changed, 15 insertions, 27 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/collect/nestedset/BUILD b/src/main/java/com/google/devtools/build/lib/collect/nestedset/BUILD
index 09be2fbb75..dbf0d0684a 100644
--- a/src/main/java/com/google/devtools/build/lib/collect/nestedset/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/collect/nestedset/BUILD
@@ -19,9 +19,7 @@ java_library(
     ],
     deps = [
         "//src/main/java/com/google/devtools/build/lib/collect/compacthashset",
-        "//src/main/java/com/google/devtools/build/lib/concurrent",
         "//src/main/java/com/google/devtools/build/lib/skyframe/serialization",
-        "//src/main/java/com/google/devtools/build/lib/skyframe/serialization:constants",
         "//third_party:guava",
         "//third_party:jsr305",
         "//third_party/protobuf:protobuf_java",
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 4932b3474c..14369814af 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
@@ -14,7 +14,7 @@
 package com.google.devtools.build.lib.collect.nestedset;
 
 import com.google.common.base.Preconditions;
-import com.google.common.cache.CacheBuilder;
+import com.google.common.collect.Maps;
 import com.google.common.hash.Hashing;
 import com.google.common.hash.HashingOutputStream;
 import com.google.devtools.build.lib.skyframe.serialization.DeserializationContext;
@@ -33,7 +33,6 @@ import java.util.Collection;
 import java.util.IdentityHashMap;
 import java.util.LinkedHashSet;
 import java.util.Map;
-import java.util.concurrent.ConcurrentMap;
 
 /**
  * Codec for {@link NestedSet}.
@@ -46,15 +45,6 @@ public class NestedSetCodec<T> implements ObjectCodec<NestedSet<T>> {
 
   private static final EnumCodec<Order> orderCodec = new EnumCodec<>(Order.class);
 
-  private static final ConcurrentMap<ByteString, Object> digestToChild =
-      CacheBuilder.newBuilder()
-          .concurrencyLevel(SerializationConstants.DESERIALIZATION_POOL_SIZE)
-          .weakValues()
-          .<ByteString, Object>build()
-          .asMap();
-
-  public NestedSetCodec() {}
-
   @SuppressWarnings("unchecked")
   @Override
   public Class<NestedSet<T>> getEncodedClass() {
@@ -90,6 +80,7 @@ public class NestedSetCodec<T> implements ObjectCodec<NestedSet<T>> {
     }
 
     int nestedSetCount = codedIn.readInt32();
+    Map<ByteString, Object> digestToChild = Maps.newHashMapWithExpectedSize(nestedSetCount);
     Preconditions.checkState(
         nestedSetCount >= 1,
         "Should have at least serialized one nested set, got: %s",
@@ -99,7 +90,7 @@ public class NestedSetCodec<T> implements ObjectCodec<NestedSet<T>> {
     for (int i = 0; i < nestedSetCount; ++i) {
       // Maintain pointers to all children in this NestedSet so that their entries in the
       // digestToChild map are not GCed.
-      childrenForThisNestedSet.add(deserializeOneNestedSet(context, codedIn));
+      childrenForThisNestedSet.add(deserializeOneNestedSet(context, codedIn, digestToChild));
     }
     // The last element of childrenForThisNestedSet is the top-level NestedSet
     return createNestedSet(order, childrenForThisNestedSet.get(nestedSetCount - 1));
@@ -162,7 +153,10 @@ public class NestedSetCodec<T> implements ObjectCodec<NestedSet<T>> {
     context.serialize(singleChild, childCodedOut);
   }
 
-  private Object deserializeOneNestedSet(DeserializationContext context, CodedInputStream codedIn)
+  private Object deserializeOneNestedSet(
+      DeserializationContext context,
+      CodedInputStream codedIn,
+      Map<ByteString, Object> digestToChild)
       throws SerializationException, IOException {
     ByteString digest = codedIn.readBytes();
     CodedInputStream childCodedIn = codedIn.readBytes().newCodedInput();
@@ -170,28 +164,24 @@ public class NestedSetCodec<T> implements ObjectCodec<NestedSet<T>> {
     int childCount = childCodedIn.readInt32();
     final Object result;
     if (childCount > 1) {
-      result = deserializeMultipleItemChildArray(context, childCodedIn, childCount);
+      result = deserializeMultipleItemChildArray(context, childCodedIn, childCount, digestToChild);
     } else if (childCount == 1) {
       result = context.deserialize(childCodedIn);
     } else {
       result = NestedSet.EMPTY_CHILDREN;
     }
-    // If this member has been deserialized already, use it, so that NestedSets that share members
-    // will point at the same object in memory instead of duplicating them.  Unfortunately, we had
-    // to do the work of deserializing the member that we will now throw away, in order to ensure
-    // that the pointer in the codedIn buffer was incremented appropriately.
-    Object oldValue =
-        digestToChild.putIfAbsent(
-            // Copy the ByteString to avoid keeping the full buffer from codedIn alive due to
-            // aliasing.
-            ByteString.copyFrom(digest.asReadOnlyByteBuffer()), result);
-    return oldValue == null ? result : oldValue;
+    digestToChild.put(
+        // Copy the ByteString to avoid keeping the full buffer from codedIn alive due to
+        // aliasing.
+        ByteString.copyFrom(digest.asReadOnlyByteBuffer()), result);
+    return result;
   }
 
   private Object deserializeMultipleItemChildArray(
       DeserializationContext context,
       CodedInputStream childCodedIn,
-      int childCount)
+      int childCount,
+      Map<ByteString, Object> digestToChild)
       throws IOException, SerializationException {
     Object[] children = new Object[childCount];
     for (int i = 0; i < childCount; ++i) {