Skip sorting in ArrayTagSet for SortedMaps

When tags are supplied in a SortedMap, skip the sorting step. Additionally, skip deduping when tags are supplied in a Map in general.
Netflix · Oct 14, 2022 · 5353d70 · 5353d70
1 parent 9e7944f
commit 5353d70
Show file tree

Hide file tree

Showing 2 changed files with 55 additions and 8 deletions.
diff --git a/spectator-api/src/main/java/com/netflix/spectator/api/ArrayTagSet.java b/spectator-api/src/main/java/com/netflix/spectator/api/ArrayTagSet.java
@@ -23,6 +23,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
+import java.util.SortedMap;
 import java.util.Spliterator;
 import java.util.Spliterators;
 import java.util.concurrent.ConcurrentMap;
@@ -184,11 +185,12 @@ ArrayTagSet addAll(Map<String, String> ts) {
       String[] newTags = new String[2 * ts.size()];
       int i = 0;
       for (Map.Entry<String, String> entry : ts.entrySet()) {
-        newTags[i++] = entry.getKey();
-        newTags[i++] = entry.getValue();
+        newTags[i++] = Objects.requireNonNull(entry.getKey(), "tag keys cannot be null");
+        newTags[i++] = Objects.requireNonNull(entry.getValue(), "tag values cannot be null");
       }
-      checkForNullValues(newTags);
-      return addAll(newTags, newTags.length);
+
+      boolean sorted = ts instanceof SortedMap && ((SortedMap<?, ?>) ts).comparator() == null;
+      return addAll(newTags, newTags.length, sorted, true);
     }
   }
 
@@ -202,17 +204,28 @@ ArrayTagSet addAll(String[] ts) {
     return addAll(copy, copy.length);
   }
 
-  /** Add a collection of tags to the set. */
   private ArrayTagSet addAll(String[] ts, int tsLength) {
+    return addAll(ts, tsLength, false, false);
+  }
+
+  /** Add a collection of tags to the set. */
+  private ArrayTagSet addAll(String[] ts, int tsLength, boolean sorted, boolean distinct) {
     if (tsLength == 0) {
       return this;
     } else if (length == 0) {
-      insertionSort(ts, tsLength);
-      int len = dedup(ts, 0, ts, 0, tsLength);
+      if (!sorted) {
+        insertionSort(ts, tsLength);
+      }
+      int len = tsLength;
+      if (!distinct) {
+        len = dedup(ts, 0, ts, 0, tsLength);
+      }
       return new ArrayTagSet(ts, len);
     } else {
       String[] newTags = new String[length + tsLength];
-      insertionSort(ts, tsLength);
+      if (!sorted) {
+        insertionSort(ts, tsLength);
+      }
       int newLength = merge(newTags, tags, length, ts, tsLength);
       return new ArrayTagSet(newTags, newLength);
     }

diff --git a/spectator-api/src/test/java/com/netflix/spectator/api/ArrayTagSetTest.java b/spectator-api/src/test/java/com/netflix/spectator/api/ArrayTagSetTest.java
@@ -19,16 +19,22 @@
 import org.junit.jupiter.api.Test;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.Comparator;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.NoSuchElementException;
+import java.util.SortedMap;
 import java.util.Spliterator;
+import java.util.TreeMap;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.stream.Collectors;
+import java.util.stream.StreamSupport;
 
 public class ArrayTagSetTest {
 
@@ -592,4 +598,32 @@ public void spliteratorSizeAndCharacteristics() {
     Assertions.assertTrue(spliterator.hasCharacteristics(Spliterator.SIZED));
     Assertions.assertEquals(3L, spliterator.getExactSizeIfKnown());
   }
+
+  @Test
+  public void testCreateWithSortedMap() {
+    SortedMap<String, String> tagMap = new TreeMap<>();
+    tagMap.put("a", "v1");
+    tagMap.put("b", "v2");
+    tagMap.put("c", "v3");
+    tagMap.put("d", "v4");
+
+    ArrayTagSet tags = ArrayTagSet.create(tagMap);
+
+    List<Tag> tagList = StreamSupport.stream(tags.spliterator(), false).collect(Collectors.toList());
+    Assertions.assertEquals(Arrays.asList(Tag.of("a", "v1"), Tag.of("b", "v2"), Tag.of("c", "v3"), Tag.of("d", "v4")), tagList);
+  }
+
+  @Test
+  public void testCreateWithSortedMapCustomComparator() {
+    SortedMap<String, String> tagMap = new TreeMap<>(Comparator.reverseOrder());
+    tagMap.put("a", "v1");
+    tagMap.put("b", "v2");
+    tagMap.put("c", "v3");
+    tagMap.put("d", "v4");
+
+    ArrayTagSet tags = ArrayTagSet.create(tagMap);
+
+    List<Tag> tagList = StreamSupport.stream(tags.spliterator(), false).collect(Collectors.toList());
+    Assertions.assertEquals(Arrays.asList(Tag.of("a", "v1"), Tag.of("b", "v2"), Tag.of("c", "v3"), Tag.of("d", "v4")), tagList);
+  }
 }