Avoid allocating extra space when overriding tag (#1000)

Change ArrayTagSet::add to avoid allocating extra space when an
existing tag is being overridden, and to avoid creating a new
ArrayTagSet at all if the supplied key/value pair already exists in
the target ArrayTagSet.

Additionally, switch to System.arraycopy instead of manually
copying to the newly created array.

spectator-api/src/main/java/com/netflix/spectator/api/ArrayTagSet.java

@@ -104,34 +104,40 @@ boolean isEmpty() {
   }
 
   /** Add a new tag to the set. */
-  @SuppressWarnings("PMD.AvoidArrayLoops")
   ArrayTagSet add(String k, String v) {
     Preconditions.checkNotNull(k, "key");
     Preconditions.checkNotNull(v, "value");
     if (length == 0) {
       return new ArrayTagSet(new String[] {k, v});
-    } else {
-      String[] newTags = new String[length + 2];
-      int i = 0;
-      for (; i < length && tags[i].compareTo(k) < 0; i += 2) {
-        newTags[i] = tags[i];
-        newTags[i + 1] = tags[i + 1];
+    }
+
+    int idx = 0;
+    int cmp = -1;
+    while (idx < length) {
+      cmp = tags[idx].compareTo(k);
+      if (cmp >= 0) {
+        break;
       }
-      if (i < length && tags[i].equals(k)) {
-        // Override
-        newTags[i++] = k;
-        newTags[i++] = v;
-        System.arraycopy(tags, i, newTags, i, length - i);
-        i = length;
-      } else {
-        // Insert
-        newTags[i] = k;
-        newTags[i + 1] = v;
-        System.arraycopy(tags, i, newTags, i + 2, length - i);
-        i = newTags.length;
+      idx += 2;
+    }
+
+    // Update an existing tag
+    if (cmp == 0) {
+      if (tags[idx + 1].equals(v)) {
+        return this;
       }
-      return new ArrayTagSet(newTags, i);
+      String[] newTags = Arrays.copyOf(tags, length);
+      newTags[idx + 1] = v;
+      return new ArrayTagSet(newTags);
     }
+
+    String[] newTags = new String[length + 2];
+    newTags[idx] = k;
+    newTags[idx + 1] = v;
+
+    System.arraycopy(tags, 0, newTags, 0, idx);
+    System.arraycopy(tags, idx, newTags, idx + 2, length - idx);
+    return new ArrayTagSet(newTags);
   }
 
   /** Add a new tag to the set. */

spectator-api/src/test/java/com/netflix/spectator/api/ArrayTagSetTest.java

@@ -640,4 +640,39 @@ public void testCreateWithSortedMapCustomComparatorNaturalOrder() {
     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 testAddBeginning() {
+    ArrayTagSet tags = ArrayTagSet.create("a", "v1", "b", "v2", "c", "v3");
+    ArrayTagSet updated = tags.add("0", "v0");
+    Assertions.assertEquals(ArrayTagSet.create("0", "v0", "a", "v1", "b", "v2", "c", "v3"), updated);
+  }
+
+  @Test
+  public void testAddEnd() {
+    ArrayTagSet tags = ArrayTagSet.create("a", "v1", "b", "v2", "c", "v3");
+    ArrayTagSet updated = tags.add("d", "v4");
+    Assertions.assertEquals(ArrayTagSet.create("a", "v1", "b", "v2", "c", "v3", "d", "v4"), updated);
+  }
+
+  @Test
+  public void testAddMiddle() {
+    ArrayTagSet tags = ArrayTagSet.create("a", "v1", "b", "v2", "d", "v4");
+    ArrayTagSet updated = tags.add("c", "v3");
+    Assertions.assertEquals(ArrayTagSet.create("a", "v1", "b", "v2", "c", "v3", "d", "v4"), updated);
+  }
+
+  @Test
+  public void testAddUpdatesExisting() {
+    ArrayTagSet tags = ArrayTagSet.create("a", "v1", "b", "v2", "c", "v3");
+    ArrayTagSet updated = tags.add("c", "v3-updated");
+    Assertions.assertEquals(ArrayTagSet.create("a", "v1", "b", "v2", "c", "v3-updated"), updated);
+  }
+
+  @Test
+  public void testAddUpdatesExistingWithSameTag() {
+    ArrayTagSet tags = ArrayTagSet.create("a", "v1", "b", "v2", "c", "v3");
+    ArrayTagSet updated = tags.add("c", "v3");
+    Assertions.assertSame(tags, updated);
+  }
 }