From ad0beb9dbafdad8d4f606b3c3cd29a79fdd08b4d Mon Sep 17 00:00:00 2001 From: Patrick Strawderman Date: Fri, 15 Sep 2023 09:58:26 -0700 Subject: [PATCH] Simplify ArrayTagSet sort method Get rid of unnecessary branches in ArrayTagSet's insertionSort method. --- .../netflix/spectator/api/ArrayTagSet.java | 35 +++++-------------- .../spectator/api/ArrayTagSetTest.java | 22 ++++++++++-- 2 files changed, 29 insertions(+), 28 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 index 2798cc786..9810d94c3 100644 --- a/spectator-api/src/main/java/com/netflix/spectator/api/ArrayTagSet.java +++ b/spectator-api/src/main/java/com/netflix/spectator/api/ArrayTagSet.java @@ -304,33 +304,16 @@ private void checkForNullValues(String[] ts) { * tags. With the small size a simple insertion sort works well. */ private static void insertionSort(String[] ts, int length) { - if (length == 4) { - // Two key/value pairs, swap if needed - if (ts[0].compareTo(ts[2]) > 0) { - // Swap key - String tmp = ts[0]; - ts[0] = ts[2]; - ts[2] = tmp; - - // Swap value - tmp = ts[1]; - ts[1] = ts[3]; - ts[3] = tmp; - } - } else if (length > 4) { - // One entry is already sorted. Two entries handled above, for larger arrays - // use insertion sort. - for (int i = 2; i < length; i += 2) { - String k = ts[i]; - String v = ts[i + 1]; - int j = i - 2; - for (; j >= 0 && ts[j].compareTo(k) > 0; j -= 2) { - ts[j + 2] = ts[j]; - ts[j + 3] = ts[j + 1]; - } - ts[j + 2] = k; - ts[j + 3] = v; + for (int i = 2; i < length; i += 2) { + String k = ts[i]; + String v = ts[i + 1]; + int j = i - 2; + for (; j >= 0 && ts[j].compareTo(k) > 0; j -= 2) { + ts[j + 2] = ts[j]; + ts[j + 3] = ts[j + 1]; } + ts[j + 2] = k; + ts[j + 3] = v; } } 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 index 161ec9091..f86e3319a 100644 --- a/spectator-api/src/test/java/com/netflix/spectator/api/ArrayTagSetTest.java +++ b/spectator-api/src/test/java/com/netflix/spectator/api/ArrayTagSetTest.java @@ -779,7 +779,25 @@ public void addAllDuplicates() { assertDistinct(updated); } - class TagIterator implements Iterable { + @Test + public void testSort() { + Assertions.assertEquals(Arrays.asList("a", "v1"), toList(ArrayTagSet.create("a", "v1"))); + Assertions.assertEquals(Arrays.asList("a", "v1", "b", "v2"), toList(ArrayTagSet.create("a", "v1", "b", "v2"))); + Assertions.assertEquals(Arrays.asList("a", "v1", "b", "v2"), toList(ArrayTagSet.create("b", "v2", "a", "v1"))); + Assertions.assertEquals(Arrays.asList("a", "v1", "b", "v3"), toList(ArrayTagSet.create("b", "v2", "a", "v1", "b", "v3"))); + Assertions.assertEquals(Arrays.asList("a", "v1", "b", "v2", "c", "v3"), toList(ArrayTagSet.create("c", "v3", "b", "v2", "a", "v1"))); + } + + private static List toList(TagList tagList) { + List list = new ArrayList<>(tagList.size() * 2); + tagList.forEach(tag -> { + list.add(tag.key()); + list.add(tag.value()); + }); + return list; + } + + static final class TagIterator implements Iterable { String[] tags; TagIterator(String... tags) { this.tags = tags; @@ -803,7 +821,7 @@ public Tag next() { } } - class BadTagList implements TagList { + static final class BadTagList implements TagList { String[] tags; BadTagList(String... tags) {