@AutoCodec SelectorValue. Rather than making mapMarshaller handle wildcards better, get rid of it, and immutableSetMarshaller for good measure.

PiperOrigin-RevId: 190294922

4 files changed, 124 insertions, 95 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/MapCodec.java b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/MapCodec.java
new file mode 100644
index 0000000000..e01c1dce39
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/MapCodec.java
@@ -0,0 +1,70 @@
+// Copyright 2018 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.build.lib.skyframe.serialization;
+
+import com.google.common.collect.ImmutableList;
+import com.google.protobuf.CodedInputStream;
+import com.google.protobuf.CodedOutputStream;
+import java.io.IOException;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+
+/** {@link ObjectCodec} for {@link HashMap} and {@link java.util.Collections.UnmodifiableMap}. */
+class MapCodec<K, V> implements ObjectCodec<Map<K, V>> {
+  @SuppressWarnings("unchecked")
+  private static final Class<Map<?, ?>> UNMODIFIABLE_TYPE =
+      (Class<Map<?, ?>>) Collections.unmodifiableMap(new HashMap<>()).getClass();
+
+  @SuppressWarnings("unchecked")
+  @Override
+  public Class<Map<K, V>> getEncodedClass() {
+    return cast(HashMap.class);
+  }
+
+  @Override
+  public List<Class<? extends Map<K, V>>> additionalEncodedClasses() {
+    return ImmutableList.of(cast(UNMODIFIABLE_TYPE));
+  }
+
+  @Override
+  public void serialize(SerializationContext context, Map<K, V> obj, CodedOutputStream codedOut)
+      throws SerializationException, IOException {
+    codedOut.writeInt32NoTag(obj.size());
+    for (Map.Entry<K, V> entry : obj.entrySet()) {
+      context.serialize(entry.getKey(), codedOut);
+      context.serialize(entry.getValue(), codedOut);
+    }
+  }
+
+  @Override
+  public Map<K, V> deserialize(DeserializationContext context, CodedInputStream codedIn)
+      throws SerializationException, IOException {
+    int size = codedIn.readInt32();
+    // Load factor is 0.75, so we need an initial capacity of 4/3 actual size to avoid rehashing.
+    LinkedHashMap<K, V> result = new LinkedHashMap<>(4 * size / 3);
+    for (int i = 0; i < size; i++) {
+      result.put(context.deserialize(codedIn), context.deserialize(codedIn));
+    }
+    return Collections.unmodifiableMap(result);
+  }
+
+  @SuppressWarnings({"unchecked", "rawtypes"}) // Raw Map.
+  private Class<Map<K, V>> cast(Class<? extends Map> clazz) {
+    return (Class<Map<K, V>>) clazz;
+  }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec/Marshallers.java b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec/Marshallers.java
index bb3cf31dfa..a0b306b3ce 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec/Marshallers.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec/Marshallers.java
@@ -18,7 +18,6 @@ import com.google.common.base.Supplier;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableListMultimap;
 import com.google.common.collect.ImmutableMultimap;
-import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.ImmutableSortedSet;
 import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.skyframe.serialization.autocodec.SerializationCodeGenerator.Context;
@@ -27,7 +26,6 @@ import com.google.devtools.build.lib.skyframe.serialization.autocodec.Serializat
 import com.google.devtools.build.lib.skyframe.serialization.strings.StringCodecs;
 import com.squareup.javapoet.TypeName;
 import java.util.Comparator;
-import java.util.LinkedHashMap;
 import java.util.Map;
 import javax.annotation.processing.ProcessingEnvironment;
 import javax.lang.model.element.TypeElement;
@@ -344,36 +342,6 @@ class Marshallers {
           context.builder.endControlFlow();
   }
 
-
-  private final Marshaller immutableSetMarshaller =
-      new Marshaller() {
-        @Override
-        public boolean matches(DeclaredType type) {
-          return matchesErased(type, ImmutableSet.class);
-        }
-
-        @Override
-        public void addSerializationCode(Context context) {
-          addSerializationCodeForIterable(context);
-        }
-
-        @Override
-        public void addDeserializationCode(Context context) {
-          Context repeated =
-              context.with(
-                  context.getDeclaredType().getTypeArguments().get(0),
-                  context.makeName("repeated"));
-          String builderName = context.makeName("builder");
-          context.builder.addStatement(
-              "$T<$T> $L = new $T<>()",
-              ImmutableSet.Builder.class,
-              repeated.getTypeName(),
-              builderName,
-              ImmutableSet.Builder.class);
-          writeIterableDeserializationLoopWithoutNullsAndBuild(context, repeated, builderName);
-        }
-      };
-
   private final Marshaller immutableSortedSetMarshaller =
       new Marshaller() {
         @Override
@@ -404,64 +372,6 @@ class Marshallers {
         }
       };
 
-  private final Marshaller mapMarshaller =
-      new Marshaller() {
-        @Override
-        public boolean matches(DeclaredType type) {
-          return matchesErased(type, Map.class);
-        }
-
-        @Override
-        public void addSerializationCode(Context context) {
-          context.builder.addStatement("codedOut.writeInt32NoTag($L.size())", context.name);
-          String entryName = context.makeName("entry");
-          Context key =
-              context.with(
-                  context.getDeclaredType().getTypeArguments().get(0), entryName + ".getKey()");
-          Context value =
-              context.with(
-                  context.getDeclaredType().getTypeArguments().get(1), entryName + ".getValue()");
-          context.builder.beginControlFlow(
-              "for ($T<$T, $T> $L : $L.entrySet())",
-              Map.Entry.class,
-              key.getTypeName(),
-              value.getTypeName(),
-              entryName,
-              context.name);
-          writeSerializationCode(key);
-          writeSerializationCode(value);
-          context.builder.endControlFlow();
-        }
-
-        @Override
-        public void addDeserializationCode(Context context) {
-          String builderName = context.makeName("builder");
-          Context key =
-              context.with(
-                  context.getDeclaredType().getTypeArguments().get(0), context.makeName("key"));
-          Context value =
-              context.with(
-                  context.getDeclaredType().getTypeArguments().get(1), context.makeName("value"));
-          context.builder.addStatement(
-              "$T<$T, $T> $L = new $T<>()",
-              LinkedHashMap.class,
-              key.getTypeName(),
-              value.getTypeName(),
-              builderName,
-              LinkedHashMap.class);
-          String lengthName = context.makeName("length");
-          context.builder.addStatement("int $L = codedIn.readInt32()", lengthName);
-          String indexName = context.makeName("i");
-          context.builder.beginControlFlow(
-              "for (int $L = 0; $L < $L; ++$L)", indexName, indexName, lengthName, indexName);
-          writeDeserializationCode(key);
-          writeDeserializationCode(value);
-          context.builder.addStatement("$L.put($L, $L)", builderName, key.name, value.name);
-          context.builder.endControlFlow();
-          context.builder.addStatement("$L = $L", context.name, builderName);
-        }
-      };
-
   private final Marshaller multimapMarshaller =
       new Marshaller() {
         @Override
@@ -552,9 +462,7 @@ class Marshallers {
       ImmutableList.of(
           charSequenceMarshaller,
           supplierMarshaller,
-          immutableSetMarshaller,
           immutableSortedSetMarshaller,
-          mapMarshaller,
           multimapMarshaller,
           contextMarshaller);
 
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SelectorValue.java b/src/main/java/com/google/devtools/build/lib/syntax/SelectorValue.java
index 545ec58bb4..004e5ef5d9 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SelectorValue.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SelectorValue.java
@@ -15,6 +15,7 @@ package com.google.devtools.build.lib.syntax;
 
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Iterables;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkModule;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkPrinter;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkValue;
@@ -32,9 +33,12 @@ import java.util.Map;
  *       })
  * </pre>
  */
-@SkylarkModule(name = "selector",
-    doc = "A selector between configuration-dependent entities.",
-    documented = false)
+@SkylarkModule(
+  name = "selector",
+  doc = "A selector between configuration-dependent entities.",
+  documented = false
+)
+@AutoCodec
 public final class SelectorValue implements SkylarkValue {
   // TODO(bazel-team): Selectors are currently split between .packages and .syntax . They should
   // really all be in .packages, but then we'd need to figure out a way how to extend binary
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/serialization/MapCodecTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/serialization/MapCodecTest.java
new file mode 100644
index 0000000000..b4d58625db
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/serialization/MapCodecTest.java
@@ -0,0 +1,47 @@
+// Copyright 2018 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.build.lib.skyframe.serialization;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.devtools.build.lib.skyframe.serialization.testutils.SerializationTester;
+import com.google.devtools.build.lib.skyframe.serialization.testutils.SerializationTester.VerificationFunction;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.Map;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/** Tests for {@link MapCodec}. */
+@RunWith(JUnit4.class)
+public class MapCodecTest {
+  @Test
+  public void smoke() throws Exception {
+    HashMap<String, String> map1 = new HashMap<>();
+    map1.put("a", "first");
+    map1.put("b", null);
+    LinkedHashMap<String, String> map2 = new LinkedHashMap<>();
+    map2.put("c", null);
+    map2.put("a", "second");
+    new SerializationTester(map1, map2, Collections.unmodifiableMap(map1))
+        .setVerificationFunction(
+            (VerificationFunction<Map<String, String>>)
+                (original, deserialized) ->
+                    assertThat(deserialized).containsExactlyEntriesIn(original).inOrder())
+        .runTests();
+  }
+}