Remove ValueConstants. They're not pulling their weight in CPU overhead.

PiperOrigin-RevId: 197465288

3 files changed, 1 insertions, 80 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/AutoRegistry.java b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/AutoRegistry.java
index 33c8d670db..87be12e504 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/AutoRegistry.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/AutoRegistry.java
@@ -21,10 +21,7 @@ import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Ordering;
 import java.io.IOException;
-import java.util.ArrayList;
-import java.util.Collections;
 import java.util.Comparator;
-import java.util.HashMap;
 
 /**
  * A lazy, automatically populated registry.
@@ -62,14 +59,6 @@ public class AutoRegistry {
           Comparator.naturalOrder(),
           Ordering.natural());
 
-  private static final ImmutableList<Object> VALUE_CONSTANTS_TO_REGISTER =
-      ImmutableList.of(
-          "",
-          Boolean.FALSE,
-          Boolean.TRUE,
-          Collections.unmodifiableMap(new HashMap<>()),
-          Collections.unmodifiableList(new ArrayList<>()));
-
   public static ObjectCodecRegistry get() {
     return SUPPLIER.get();
   }
@@ -83,9 +72,6 @@ public class AutoRegistry {
       for (Object constant : REFERENCE_CONSTANTS_TO_REGISTER) {
         registry.addReferenceConstant(constant);
       }
-      for (Object constant : VALUE_CONSTANTS_TO_REGISTER) {
-        registry.addValueConstant(constant);
-      }
       for (String classNamePrefix : CLASS_NAME_PREFIX_BLACKLIST) {
         registry.blacklistClassNamePrefix(classNamePrefix);
       }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/ObjectCodecRegistry.java b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/ObjectCodecRegistry.java
index 211a67e53d..1ad3e1f270 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/ObjectCodecRegistry.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/ObjectCodecRegistry.java
@@ -52,10 +52,6 @@ public class ObjectCodecRegistry {
   private final IdentityHashMap<Object, Integer> referenceConstantsMap;
   private final ImmutableList<Object> referenceConstants;
 
-  private final int valueConstantsStartTag;
-  private final ImmutableMap<Class<?>, ImmutableMap<Object, Integer>> valueConstantsMap;
-  private final ImmutableList<Object> valueConstants;
-
   /** This is sorted, but we need index-based access. */
   private final ImmutableList<String> classNames;
 
@@ -64,7 +60,6 @@ public class ObjectCodecRegistry {
   private ObjectCodecRegistry(
       ImmutableSet<ObjectCodec<?>> memoizingCodecs,
       ImmutableList<Object> referenceConstants,
-      ImmutableList<Object> valueConstants,
       ImmutableSortedSet<String> classNames,
       ImmutableList<String> blacklistedClassNamePrefixes,
       boolean allowDefaultCodec) {
@@ -89,22 +84,6 @@ public class ObjectCodecRegistry {
     }
     this.referenceConstants = referenceConstants;
 
-    valueConstantsStartTag = nextTag;
-
-    HashMap<Class<?>, HashMap<Object, Integer>> valuesBuilder = new HashMap<>();
-    for (Object constant : valueConstants) {
-      valuesBuilder
-          .computeIfAbsent(constant.getClass(), k -> new HashMap<>())
-          .put(constant, nextTag++);
-    }
-    this.valueConstantsMap =
-        valuesBuilder
-            .entrySet()
-            .stream()
-            .collect(
-                ImmutableMap.toImmutableMap(
-                    Map.Entry::getKey, e -> ImmutableMap.copyOf(e.getValue())));
-    this.valueConstants = valueConstants;
     this.classNames =
         classNames
             .stream()
@@ -153,23 +132,12 @@ public class ObjectCodecRegistry {
         && tag < referenceConstantsStartTag + referenceConstants.size()) {
       return referenceConstants.get(tag - referenceConstantsStartTag);
     }
-    if (valueConstantsStartTag <= tag && tag < valueConstantsStartTag + valueConstants.size()) {
-      return valueConstants.get(tag - valueConstantsStartTag);
-    }
     return null;
   }
 
   @Nullable
   Integer maybeGetTagForConstant(Object object) {
-    Integer result = referenceConstantsMap.get(object);
-    if (result != null) {
-      return result;
-    }
-    ImmutableMap<Object, Integer> valueConstantsForClass = valueConstantsMap.get(object.getClass());
-    if (valueConstantsForClass == null) {
-      return null;
-    }
-    return valueConstantsForClass.get(object);
+    return referenceConstantsMap.get(object);
   }
 
   /** Returns the {@link CodecDescriptor} associated with the supplied tag. */
@@ -185,7 +153,6 @@ public class ObjectCodecRegistry {
 
     tagOffset -= tagMappedCodecs.size();
     tagOffset -= referenceConstants.size();
-    tagOffset -= valueConstants.size();
     if (!allowDefaultCodec || tagOffset < 0 || tagOffset >= classNames.size()) {
       throw new SerializationException.NoCodecException("No codec available for tag " + tag);
     }
@@ -209,10 +176,6 @@ public class ObjectCodecRegistry {
       builder.addReferenceConstant(constant);
     }
 
-    for (Object constant : valueConstants) {
-      builder.addValueConstant(constant);
-    }
-
     for (String className : classNames) {
       builder.addClassName(className);
     }
@@ -289,7 +252,6 @@ public class ObjectCodecRegistry {
   public static class Builder {
     private final Map<Class<?>, ObjectCodec<?>> codecs = new HashMap<>();
     private final ImmutableList.Builder<Object> referenceConstantsBuilder = ImmutableList.builder();
-    private final ImmutableList.Builder<Object> valueConstantsBuilder = ImmutableList.builder();
     private final ImmutableSortedSet.Builder<String> classNames = ImmutableSortedSet.naturalOrder();
     private final ImmutableList.Builder<String> blacklistedClassNamePrefixes =
         ImmutableList.builder();
@@ -339,19 +301,6 @@ public class ObjectCodecRegistry {
       return this;
     }
 
-    /**
-     * Adds a constant value. Any value encountered during serialization which has the same class as
-     * {@code object} and {@link Object#equals} {@code object} will be replaced by {@code object}
-     * upon deserialization. These objects should therefore be indistinguishable, and unequal
-     * objects should quickly compare unequal (it is ok for equal objects to be relatively expensive
-     * to compare equal, if that is still less expensive than the cost of serializing the object).
-     * Short {@link String} objects are ideal for value constants.
-     */
-    public Builder addValueConstant(Object object) {
-      valueConstantsBuilder.add(object);
-      return this;
-    }
-
     public Builder addClassName(String className) {
       classNames.add(className);
       return this;
@@ -366,7 +315,6 @@ public class ObjectCodecRegistry {
       return new ObjectCodecRegistry(
           ImmutableSet.copyOf(codecs.values()),
           referenceConstantsBuilder.build(),
-          valueConstantsBuilder.build(),
           classNames.build(),
           blacklistedClassNamePrefixes.build(),
           allowDefaultCodec);
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/serialization/ObjectCodecRegistryTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/serialization/ObjectCodecRegistryTest.java
index 42daccb6b2..a0974ab118 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/serialization/ObjectCodecRegistryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/serialization/ObjectCodecRegistryTest.java
@@ -134,19 +134,6 @@ public class ObjectCodecRegistryTest {
     assertThat(underTest2.maybeGetTagForConstant(constant2)).isEqualTo(2);
   }
 
-  @Test
-  public void valueConstant() {
-    String valueConstant = "ab";
-    ObjectCodecRegistry underTest =
-        ObjectCodecRegistry.newBuilder()
-            .setAllowDefaultCodec(false)
-            .addValueConstant(valueConstant)
-            .build();
-    assertThat(underTest.maybeGetTagForConstant("a")).isNull();
-    assertThat(underTest.maybeGetTagForConstant("ab")).isNotNull();
-    assertThat(underTest.maybeGetTagForConstant("cab".substring(1))).isNotNull();
-  }
-
   private static ObjectCodecRegistry.Builder builderWithThisClass() {
     return ObjectCodecRegistry.newBuilder().addClassName(ObjectCodecRegistryTest.class.getName());
   }