Merge branch 'main' into fix-knn-scroll

Author: mayya-sharipova

docs/changelog/126889.yaml

@@ -0,0 +1,6 @@
+pr: 126889
+summary: Rework uniquify to not use iterators
+area: Infra/Core
+type: bug
+issues:
+ - 126883

server/src/main/java/org/elasticsearch/common/util/CollectionUtils.java

@@ -51,20 +51,24 @@ public static <T> void uniquify(List<T> list, Comparator<T> cmp) {
             return;
         }
 
-        ListIterator<T> uniqueItr = list.listIterator();
-        ListIterator<T> existingItr = list.listIterator();
-        T uniqueValue = uniqueItr.next(); // get first element to compare with
-        existingItr.next(); // advance the existing iterator to the second element, where we will begin comparing
+        ListIterator<T> resultItr = list.listIterator();
+        ListIterator<T> currentItr = list.listIterator(1); // start at second element to compare
+        T lastValue = resultItr.next(); // result always includes first element, so advance it and grab first
         do {
-            T existingValue = existingItr.next();
-            if (cmp.compare(existingValue, uniqueValue) != 0 && (uniqueValue = uniqueItr.next()) != existingValue) {
-                uniqueItr.set(existingValue);
+            T currentValue = currentItr.next(); // each iter we check if the next element is different from the last we put in the result
+            if (cmp.compare(lastValue, currentValue) != 0) {
+                lastValue = currentValue;
+                resultItr.next(); // advance result so the current position is where we want to set
+                if (resultItr.previousIndex() != currentItr.previousIndex()) {
+                    // optimization: only need to set if different
+                    resultItr.set(currentValue);
+                }
             }
-        } while (existingItr.hasNext());
+        } while (currentItr.hasNext());
 
         // Lop off the rest of the list. Note with LinkedList this requires advancing back to this index,
         // but Java provides no way to efficiently remove from the end of a non random-access list.
-        list.subList(uniqueItr.nextIndex(), list.size()).clear();
+        list.subList(resultItr.nextIndex(), list.size()).clear();
     }
 
     /**

server/src/test/java/org/elasticsearch/common/util/CollectionUtilsTests.java

@@ -65,7 +65,7 @@ private <T> void assertUniquify(List<T> list, Comparator<T> cmp, int size) {
         for (List<T> listCopy : List.of(new ArrayList<T>(list), new LinkedList<T>(list))) {
             CollectionUtils.uniquify(listCopy, cmp);
             for (int i = 0; i < listCopy.size() - 1; ++i) {
-                assertThat(cmp.compare(listCopy.get(i), listCopy.get(i + 1)), lessThan(0));
+                assertThat(listCopy.get(i) + " < " + listCopy.get(i + 1), cmp.compare(listCopy.get(i), listCopy.get(i + 1)), lessThan(0));
             }
             assertThat(listCopy.size(), equalTo(size));
         }
@@ -75,9 +75,25 @@ public void testUniquify() {
         assertUniquify(List.<Integer>of(), Comparator.naturalOrder(), 0);
         assertUniquify(List.of(1), Comparator.naturalOrder(), 1);
         assertUniquify(List.of(1, 2, 3), Comparator.naturalOrder(), 3);
+        assertUniquify(List.of(1, 1, 3), Comparator.naturalOrder(), 2);
         assertUniquify(List.of(1, 1, 1), Comparator.naturalOrder(), 1);
         assertUniquify(List.of(1, 2, 2, 3), Comparator.naturalOrder(), 3);
         assertUniquify(List.of(1, 2, 2, 2), Comparator.naturalOrder(), 2);
+        assertUniquify(List.of(1, 2, 2, 3, 3, 5), Comparator.naturalOrder(), 4);
+
+        for (int i = 0; i < 10; ++i) {
+            int uniqueItems = randomIntBetween(1, 10);
+            var list = new ArrayList<Integer>();
+            int next = 1;
+            for (int j = 0; j < uniqueItems; ++j) {
+                int occurences = randomIntBetween(1, 10);
+                while (occurences-- > 0) {
+                    list.add(next);
+                }
+                next++;
+            }
+            assertUniquify(list, Comparator.naturalOrder(), uniqueItems);
+        }
     }
 
     public void testEmptyPartition() {