Improve ImmutableKeyValuePairs dedup, do everything inplace (#2787)

This change will reduce the number of allocations by 2 for the case where all entries are valid,
and by 1 when entries are filtered in the dedup.

Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>

api/all/src/main/java/io/opentelemetry/api/baggage/ImmutableBaggage.java

@@ -64,7 +64,7 @@ final class ImmutableBaggage extends ImmutableKeyValuePairs<String, BaggageEntry
     }
 
     Builder(List<Object> data) {
-      this.data = new ArrayList<>(data);
+      this.data = data;
     }
 
     @Override

api/all/src/main/java/io/opentelemetry/api/internal/ImmutableKeyValuePairs.java

@@ -7,8 +7,7 @@ package io.opentelemetry.api.internal;
 
 import static io.opentelemetry.api.internal.Utils.checkArgument;
 
-import java.util.ArrayList;
+import java.util.Arrays;
-import java.util.Collections;
 import java.util.Comparator;
 import java.util.List;
 import java.util.Map;
@@ -29,18 +28,14 @@ import javax.annotation.concurrent.Immutable;
  */
 @Immutable
 public abstract class ImmutableKeyValuePairs<K, V> {
-  private final List<Object> data;
+  private final Object[] data;
-
-  private ImmutableKeyValuePairs(List<Object> data) {
-    this.data = data;
-  }
 
   /**
    * Sorts and dedupes the key/value pairs in {@code data}. {@code null} values will be removed.
    * Keys must be {@link Comparable}.
    */
   protected ImmutableKeyValuePairs(Object[] data) {
-    this(sortAndFilter(data, Comparator.naturalOrder()));
+    this(data, Comparator.naturalOrder());
   }
 
   /**
@@ -48,19 +43,21 @@ public abstract class ImmutableKeyValuePairs<K, V> {
    * Keys will be compared with the given {@link Comparator}.
    */
   protected ImmutableKeyValuePairs(Object[] data, Comparator<?> keyComparator) {
-    this(sortAndFilter(data, keyComparator));
+    this.data = sortAndFilter(data, keyComparator);
   }
 
+  // TODO: Improve this to avoid one allocation, for the moment only some Builders and the asMap
+  //  calls this.
   protected final List<Object> data() {
-    return data;
+    return Arrays.asList(data);
   }
 
   public final int size() {
-    return data().size() / 2;
+    return data.length / 2;
   }
 
   public final boolean isEmpty() {
-    return data().isEmpty();
+    return data.length == 0;
   }
 
   public final Map<K, V> asMap() {
@@ -71,9 +68,9 @@ public abstract class ImmutableKeyValuePairs<K, V> {
   @Nullable
   @SuppressWarnings("unchecked")
   public final V get(K key) {
-    for (int i = 0; i < data().size(); i += 2) {
+    for (int i = 0; i < data.length; i += 2) {
-      if (key.equals(data().get(i))) {
+      if (key.equals(data[i])) {
-        return (V) data().get(i + 1);
+        return (V) data[i + 1];
       }
     }
     return null;
@@ -82,8 +79,8 @@ public abstract class ImmutableKeyValuePairs<K, V> {
   /** Iterates over all the key-value pairs of labels contained by this instance. */
   @SuppressWarnings("unchecked")
   public final void forEach(BiConsumer<K, V> consumer) {
-    for (int i = 0; i < data().size(); i += 2) {
+    for (int i = 0; i < data.length; i += 2) {
-      consumer.accept((K) data().get(i), (V) data().get(i + 1));
+      consumer.accept((K) data[i], (V) data[i + 1]);
     }
   }
 
@@ -91,10 +88,14 @@ public abstract class ImmutableKeyValuePairs<K, V> {
    * Sorts and dedupes the key/value pairs in {@code data}. {@code null} values will be removed.
    * Keys will be compared with the given {@link Comparator}.
    */
-  private static List<Object> sortAndFilter(Object[] data, Comparator<?> keyComparator) {
+  private static Object[] sortAndFilter(Object[] data, Comparator<?> keyComparator) {
     checkArgument(
         data.length % 2 == 0, "You must provide an even number of key/value pair arguments.");
 
+    if (data.length == 0) {
+      return data;
+    }
+
     mergeSort(data, keyComparator);
     return dedupe(data, keyComparator);
   }
@@ -181,32 +182,40 @@ public abstract class ImmutableKeyValuePairs<K, V> {
   }
 
   @SuppressWarnings("unchecked")
-  private static <K> List<Object> dedupe(Object[] data, Comparator<K> keyComparator) {
+  private static <K> Object[] dedupe(Object[] data, Comparator<K> keyComparator) {
-    List<Object> result = new ArrayList<>(data.length);
     Object previousKey = null;
+    int size = 0;
 
-    // iterate in reverse, to implement the "last one in wins" behavior.
+    // Implement the "last one in wins" behavior.
-    for (int i = data.length - 2; i >= 0; i -= 2) {
+    for (int i = 0; i < data.length; i += 2) {
       Object key = data[i];
       Object value = data[i + 1];
+      // Skip entries with key null.
       if (key == null) {
         continue;
       }
+      // If the previously added key is equal with the current key, we overwrite what we have.
       if (previousKey != null && keyComparator.compare((K) key, (K) previousKey) == 0) {
-        continue;
+        size -= 2;
       }
-      previousKey = key;
+      // Skip entries with null value, we do it here because we want them to overwrite and remove
+      // entries with same key that we already added.
       if (value == null) {
         continue;
       }
-      // add them in reverse order, because we'll reverse the list before returning,
+      previousKey = key;
-      // to preserve insertion order.
+      data[size++] = key;
-      result.add(value);
+      data[size++] = value;
-      result.add(key);
     }
-    Collections.reverse(result);
+    // Elements removed from the array, copy the array. We optimize for the case where we don't have
+    // duplicates or invalid entries.
+    if (data.length != size) {
+      Object[] result = new Object[size];
+      System.arraycopy(data, 0, result, 0, size);
       return result;
     }
+    return data;
+  }
 
   @Override
   public boolean equals(Object o) {
@@ -217,26 +226,25 @@ public abstract class ImmutableKeyValuePairs<K, V> {
       return false;
     }
     ImmutableKeyValuePairs<?, ?> that = (ImmutableKeyValuePairs<?, ?>) o;
-    return this.data.equals(that.data());
+    return Arrays.equals(this.data, that.data);
   }
 
   @Override
   public int hashCode() {
     int result = 1;
     result *= 1000003;
-    result ^= data.hashCode();
+    result ^= Arrays.hashCode(data);
     return result;
   }
 
   @Override
   public String toString() {
     final StringBuilder sb = new StringBuilder("{");
-    List<Object> data = data();
+    for (int i = 0; i < data.length; i += 2) {
-    for (int i = 0; i < data.size(); i += 2) {
       // Quote string values
-      Object value = data.get(i + 1);
+      Object value = data[i + 1];
       String valueStr = value instanceof String ? '"' + (String) value + '"' : value.toString();
-      sb.append(data.get(i)).append("=").append(valueStr).append(", ");
+      sb.append(data[i]).append("=").append(valueStr).append(", ");
     }
     // get rid of that last pesky comma
     if (sb.length() > 1) {