From 84e315c464ddeb3319af43be89ab965989eba991 Mon Sep 17 00:00:00 2001
From: Mihai Nita <mnita@google.com>
Date: Fri, 2 Aug 2024 10:58:52 -0700
Subject: [PATCH] ICU-22845 Better iterations for the ICU4J UnicodeSet

---
 .../java/com/ibm/icu/text/UnicodeSet.java     | 204 +++++++++++++++++-
 .../ibm/icu/dev/test/lang/UnicodeSetTest.java | 181 ++++++++++++++++
 2 files changed, 378 insertions(+), 7 deletions(-)

diff --git a/icu4j/main/core/src/main/java/com/ibm/icu/text/UnicodeSet.java b/icu4j/main/core/src/main/java/com/ibm/icu/text/UnicodeSet.java
index cc97bbc840b9..adb038adef11 100644
--- a/icu4j/main/core/src/main/java/com/ibm/icu/text/UnicodeSet.java
+++ b/icu4j/main/core/src/main/java/com/ibm/icu/text/UnicodeSet.java
@@ -17,7 +17,12 @@
 import java.util.Iterator;
 import java.util.NoSuchElementException;
 import java.util.SortedSet;
+import java.util.Spliterator;
 import java.util.TreeSet;
+import java.util.function.IntConsumer;
+import java.util.stream.IntStream;
+import java.util.stream.Stream;
+import java.util.stream.StreamSupport;
 
 import com.ibm.icu.impl.BMPSet;
 import com.ibm.icu.impl.CharacterPropertiesImpl;
@@ -278,14 +283,25 @@
  *     </tr>
  *   </table>
  * </blockquote>
- * <p>To iterate over contents of UnicodeSet, the following are available:
- * <ul><li>{@link #ranges()} to iterate through the ranges</li>
- * <li>{@link #strings()} to iterate through the strings</li>
- * <li>{@link #iterator()} to iterate through the entire contents in a single loop.
- * That method is, however, not particularly efficient, since it "boxes" each code point into a String.
+ *
+ * <p>To iterate over contents of {@code UnicodeSet}, the following are available:
+ * <ul>
+ *   <li>to iterate through the ranges: {@link #ranges()}, {@link UnicodeSetIterator#nextRange()},
+ *       {@link #rangeStream()}</li>
+ *   <li>to iterate through the strings: {@link #strings()}, {@link UnicodeSetIterator#next()},
+ *       {@link #stringStream()}</li>
+ *   <li>to iterate through the code points: {@link UnicodeSetIterator#next()},
+ *       {@link #codePoints()}, {@link #codePointStream()}</li>
+ *   <li>to iterate through the entire contents in a single loop: {@link #iterator()},
+ *       {@link UnicodeSetIterator#next()}, {@link #stream()}.<br>
+ *       All of these method are, however, not particularly efficient,
+ *       since they convert each individual code point to a {@code String}.
  * </ul>
- * All of the above can be used in <b>for</b> loops.
- * The {@link com.ibm.icu.text.UnicodeSetIterator UnicodeSetIterator} can also be used, but not in <b>for</b> loops.
+ *
+ * <p>The iterators and streams methods work as expected in current idiomatic Java usage.<br>
+ * The {@link UnicodeSetIterator} cannot be in <b>for</b> loops, and it is not very Java-ideomatic, because it is old.
+ * But it might be faster in certain use cases. We recommend that you measure in performance sensitive code.<br>
+ *
  * <p>To replace, count elements, or delete spans, see {@link com.ibm.icu.text.UnicodeSetSpanner UnicodeSetSpanner}.
  *
  * @author Alan Liu
@@ -5127,5 +5143,179 @@ public static void setDefaultXSymbolTable(XSymbolTable xSymbolTable) {
         CharacterPropertiesImpl.clear();
         XSYMBOL_TABLE = xSymbolTable;
     }
+
+    /**
+     * Returns a {@link Stream} of {@link EntryRange} values from this {@code UnicodeSet}.
+     *
+     * <p><b>Warning:</b> The {@link EntryRange} instance is the same each time; the contents are just reset.
+     *
+     * <p><b>Warning:</b> To iterate over the full contents, you have to also iterate over the strings.
+     *
+     * <p><b>Warning:</b> For speed, {@code UnicodeSet} iteration does not check for concurrent modification.<br>
+     * Do not alter the {@code UnicodeSet} while iterating.
+     *
+     * @return a {@link Stream} of {@link EntryRange}
+     */
+    public Stream<EntryRange> rangeStream() {
+        return StreamSupport.stream(ranges().spliterator(), false);
+    }
+
+    /**
+     * Returns a {@link Stream} of {@code String} values from this {@code UnicodeSet}.
+     *
+     * <p><b>Warning:</b> To iterate over the full contents, you have to also iterate over the strings.
+     *
+     * <p><b>Warning:</b> For speed, {@code UnicodeSet} iteration does not check for concurrent modification.<br>
+     * Do not alter the {@code UnicodeSet} while iterating.
+     *
+     * @return a {@link Stream} of {@code String}
+     */
+    public Stream<String> stringStream() {
+        return StreamSupport.stream(strings().spliterator(), false);
+    }
+
+    /**
+     * Returns an {@link IntStream} of Unicode code point values from this {@code UnicodeSet}.
+     *
+     * <p><b>Warning:</b> To iterate over the full contents, you have to also iterate over the strings.
+     *
+     * <p><b>Warning:</b> For speed, {@code UnicodeSet} iteration does not check for concurrent modification.<br>
+     * Do not alter the {@code UnicodeSet} while iterating.
+     *
+     * @return an {@link IntStream} of Unicode code point values
+     */
+    public IntStream codePointStream() {
+        return StreamSupport.intStream(new CodePointsSpliterator(this), false);
+    }
+
+    /**
+     * Returns a stream of {@code String} values from this {@code UnicodeSet}.
+     *
+     * <p><b>Warning:</b> To iterate over the full contents, you have to also iterate over the strings.
+     *
+     * <p><b>Warning:</b> For speed, {@code UnicodeSet} iteration does not check for concurrent modification.<br>
+     * Do not alter the {@code UnicodeSet} while iterating.
+     *
+     * @return a {@link Stream} of {@code String}
+     */
+    public Stream<String> stream() {
+        return StreamSupport.stream(spliterator(), false);
+    }
+
+    /**
+     * Provide for iteration on code points.
+     *
+     * <p>Returns an {@link Iterable} over all the code points, "expanding" the ranges
+     * by iterating on all code points between the {@link EntryRange#codepoint} and
+     * {@link EntryRange#codepointEnd}.
+     *
+     * <p><b>Warning:</b> This is a convenience method, but comes with a performance penalty
+     * because it boxes {@code int} into {@code Integer}.<br>
+     * For faster iteration use the {@link #codePointStream()} or {@link #charAt(int)}.
+     *
+     * <p><b>Warning:</b> To iterate over the full contents, you have to also iterate over the strings.
+     *
+     * <p><b>Warning:</b> For speed, {@code UnicodeSet} iteration does not check for concurrent modification.<br>
+     * Do not alter the {@code UnicodeSet} while iterating.
+     *
+     * @return an {@link Iterable} over all the code points
+     */
+    public Iterable<Integer> codePoints() {
+        return new CodePointsIterable();
+    }
+
+    private class CodePointsIterable implements Iterable<Integer> {
+        @Override
+        public Iterator<Integer> iterator() {
+            return new CodePointsIterator();
+        }
+    }
+
+    private class CodePointsIterator implements Iterator<Integer> {
+        private int pos = 0;
+        private int lastCp = -1; // don't call `charAt` twice, in `hasNext()` and `next()`
+
+        @Override
+        public boolean hasNext() {
+            lastCp = charAt(pos++);
+            return lastCp != -1;
+        }
+
+        @Override
+        public Integer next() {
+            if (lastCp != -1) {
+                return lastCp;
+            } else {
+                throw new NoSuchElementException();
+            }
+        }
+
+        @Override
+        public void remove() {
+            throw new UnsupportedOperationException();
+        }
+    }
+
+    private static class CodePointsSpliterator implements Spliterator.OfInt {
+        /*
+         * WARNING: although it is tempting to also use `Spliterator.SIZED`, it is not safe,
+         * as `UnicodeSet.size()` does not return the number of code points,
+         * it returns code points + strings
+         *
+         * Quote from the `UnicodeSet.size()` doc:
+         *     > Returns the number of elements in this set (its cardinality)
+         *     > Note than the elements of a set may include both individual
+         *     > codepoints and strings."
+         *
+         * And from the Spliterator.SIZED doc:
+         *     > Characteristic value signifying that the value returned from
+         *     > {@code estimateSize()} prior to traversal or splitting represents a
+         *     > finite size that, in the absence of structural source modification,
+         *     > represents an exact count of the number of elements that would be
+         *     > encountered by a complete traversal.
+         * Note "exact count of the number of elements that would be encountered"
+         */
+        private final static int CHARACTERISTICS = Spliterator.ORDERED | Spliterator.DISTINCT | Spliterator.NONNULL;
+
+        private final UnicodeSet uset;
+        private int index;
+
+        CodePointsSpliterator(UnicodeSet uset) {
+            this.uset = uset;
+            this.index = 0;
+        }
+
+        @Override
+        public long estimateSize() {
+            return uset.size();
+        }
+
+        @Override
+        public int characteristics() {
+            // TODO: consider calling `isFrozen()` and also return `Spliterator.IMMUTABLE` if true?
+            return CHARACTERISTICS;
+        }
+
+        @Override
+        public Spliterator.OfInt trySplit() {
+            /* From the doc:
+             *   > This method may return null for any reason, including emptiness, inability to split after
+             *   > traversal has commenced, data structure constraints, and efficiency considerations.
+             */
+            return null;
+        }
+
+        @Override
+        public boolean tryAdvance(IntConsumer action) {
+            if (action == null)
+                throw new NullPointerException();
+            int cp = uset.charAt(index++);
+            if (cp != -1) {
+                action.accept(cp);
+                return true;
+            }
+            return false;
+        }
+    }
 }
 //eof
diff --git a/icu4j/main/core/src/test/java/com/ibm/icu/dev/test/lang/UnicodeSetTest.java b/icu4j/main/core/src/test/java/com/ibm/icu/dev/test/lang/UnicodeSetTest.java
index e4cbe062ef13..8e5d955f9f67 100644
--- a/icu4j/main/core/src/test/java/com/ibm/icu/dev/test/lang/UnicodeSetTest.java
+++ b/icu4j/main/core/src/test/java/com/ibm/icu/dev/test/lang/UnicodeSetTest.java
@@ -61,6 +61,13 @@ public class UnicodeSetTest extends CoreTestFmwk {
 
     static final String NOT = "%%%%";
 
+    // Used to test iterators and streams
+    final static UnicodeSet[] UNICODE_SETS_TO_ITERATE = {
+            UnicodeSet.EMPTY,
+            // UnicodeSet.ALL_CODE_POINTS, // this works, but the tests are visibly slow
+            populateUnicodeSet(),
+    };
+
     private static final boolean isCccValue(int ccc) {
         switch (ccc) {
         case 0:
@@ -3198,4 +3205,178 @@ public void TestPatternCodePointComplement() {
             assertFalse("[:Basic_Emoji:].complement() --> no bicycle", notBasic.contains("🚲"));
         }
     }
+
+    private static UnicodeSet populateUnicodeSet() {
+        // Trying to cover the most interesting combinations:
+        final UnicodeSet unicodeSet = new UnicodeSet();
+        // Patterns
+        unicodeSet.applyPattern("\\p{sc=Ethi}");
+        unicodeSet.applyPattern("\\p{Number}");
+        // Single code point
+        unicodeSet.add('X'); // adds a code point in the ASCII range
+        unicodeSet.add('Σ'); // adds a code point in the Greek block
+        unicodeSet.add(0x1F600); // adds a code point above BMP (😀)
+        // Code point ranges and from CharSequence
+        unicodeSet.add('A', 'F'); // adds a code point range in the ASCII range
+        unicodeSet.add('α', 'ζ'); // adds a code point range in the Greek block
+        unicodeSet.add(0x1F347, 0x1F353); // adds a code point range above BMP (🍇-🍓)
+        unicodeSet.addAll("hello"); // adds code points from CharSequence
+        // Strings
+        unicodeSet.add("world"); // adds string
+        unicodeSet.addAll("one", "two", "three"); // adds strings
+        return (UnicodeSet) unicodeSet.freeze();
+    }
+
+    @Test
+    public void testIterationMethodsCodepoints() {
+        for (UnicodeSet us : UNICODE_SETS_TO_ITERATE) {
+
+            // Build the reference test result
+            StringJoiner expected = newResult();
+            int i = 0;
+            while (true) {
+                int cp = us.charAt(i++);
+                if (cp == -1) {
+                    break;
+                }
+                expected.add(codePointToString(cp));
+            }
+
+            UnicodeSetIterator usi = new UnicodeSetIterator(us);
+            StringJoiner fromUnicodeSetIterator = newResult();
+            while (usi.next()) {
+                if (usi.codepoint != UnicodeSetIterator.IS_STRING) {
+                    fromUnicodeSetIterator.add(codePointToString(usi.codepoint));
+                }
+            }
+            assertEquals("code points :: UnicodeSetIterator", expected.toString(), fromUnicodeSetIterator.toString());
+
+            StringJoiner fromIterable = newResult();
+            for (Integer cp : us.codePoints()) {
+                fromIterable.add(codePointToString(cp));
+            }
+            assertEquals("code points :: codePoints", expected.toString(), fromIterable.toString());
+
+            StringJoiner fromStream = newResult();
+            us.codePointStream().forEach(cp -> fromStream.add(codePointToString(cp)));
+            assertEquals("code points :: codePointStream", expected.toString(), fromStream.toString());
+        }
+    }
+
+    @Test
+    public void testIterationMethodsRanges() {
+        final UnicodeSet us = populateUnicodeSet();
+
+        // Build the reference test result
+        StringJoiner expected = newResult();
+        for (EntryRange r : us.ranges()) {
+            expected.add(rangeToString(r));
+        }
+
+        UnicodeSetIterator usi = new UnicodeSetIterator(us);
+        StringJoiner fromUnicodeSetIterator = newResult();
+        while (usi.nextRange()) {
+            if (usi.codepoint != UnicodeSetIterator.IS_STRING) {
+                fromUnicodeSetIterator.add(rangeString(usi.codepoint, usi.codepointEnd));
+            }
+        }
+        assertEquals("ranges :: UnicodeSetIterator", expected.toString(), fromUnicodeSetIterator.toString());
+
+        StringJoiner fromStream = newResult();
+        us.rangeStream().forEach(r -> fromStream.add(rangeToString(r)));
+        assertEquals("ranges :: rangeStream", expected.toString(), fromStream.toString());
+    }
+
+    @Test
+    public void testIterationMethodsStrings() {
+        final UnicodeSet us = populateUnicodeSet();
+
+        // Build the reference test result
+        StringJoiner expected = newResult();
+        for (String str : us.strings()) {
+            expected.add(stringToString(str));
+        }
+
+        UnicodeSetIterator usi = new UnicodeSetIterator(us);
+        StringJoiner fromUnicodeSetIterator = newResult();
+        while (usi.nextRange()) {
+            if (usi.codepoint == UnicodeSetIterator.IS_STRING) {
+                fromUnicodeSetIterator.add(stringToString(usi.getString()));
+            }
+        }
+        assertEquals("strings :: UnicodeSetIterator", expected.toString(), fromUnicodeSetIterator.toString());
+
+        StringJoiner fromStream = newResult();
+        us.stringStream().map(UnicodeSetTest::stringToString).forEach(fromStream::add);
+        assertEquals("strings :: stringStream", expected.toString(), fromStream.toString());
+    }
+
+    @Test
+    // Iterates on strings AND on code points (converted to strings)
+    public void testIterationMethodsCodepointsAndStrings() {
+        final UnicodeSet us = populateUnicodeSet();
+
+        // Build the reference test result
+        StringJoiner expect = newResult();
+        for (String str : us) {
+            expect.add(stringToString(str));
+        }
+
+        StringJoiner fromStream = newResult();
+        us.stream().map(UnicodeSetTest::stringToString).forEach(fromStream::add);
+
+        assertEquals("code points + strings :: stream", expect.toString(), fromStream.toString());
+    }
+
+    /*
+     * Helper methods for testing various iterations.
+     * The main goal is to make the results of any possible failures more readable
+     * by formatting code points to something like U+03A3(Σ) and wrapping strings in double quotes.
+     */
+
+    private static String codePointToString(int codepoint) {
+        String fromCodePoint = new String(Character.toChars(codepoint));
+        return String.format("U+%04X(%s)", codepoint, fromCodePoint);
+    }
+
+    private static String rangeToString(EntryRange range) {
+        return rangeString(range.codepoint, range.codepointEnd);
+    }
+
+    private static String rangeString(int codepoint, int codepointEnd) {
+        return String.format("%s-%s", codePointToString(codepoint), codePointToString(codepointEnd));
+    }
+
+    private static String stringToString(String str) {
+        return String.format("\"%s\"", str);
+    }
+
+    private static StringJoiner newResult() {
+        return new StringJoiner(", ");
+    }
+
+    @Test
+    public void testParallelStreams() {
+        if (!isVerbose()) {
+            return;
+        }
+        UnicodeSet us = UnicodeSet.ALL_CODE_POINTS;
+
+        long start = System.nanoTime();
+        int sumNormal = us.codePointStream().sum();
+        long timeNormal = System.nanoTime() - start;
+        System.out.println("codePointStream.normal   : " + timeNormal);
+
+        start = System.nanoTime();
+        int sumParallel = us.codePointStream().parallel().sum();
+        long timeParallel = System.nanoTime() - start;
+        System.out.println("codePointStream.parallel : " + timeParallel);
+
+        // On my machines this is about 1.2, so it is a bit faster.
+        // Unlikely to have any practical benefit, this is more to test that the
+        // parallel stream works the same as the normal one, by comparing the sums.
+        System.out.println("Speed (normal/parallel)  : " + (double) timeNormal / timeParallel);
+
+        assertEquals("normal and parallel give different sum", sumNormal, sumParallel);
+    }
 }