GROOVY-12036 and GROOVY-12037: DGM collectors#2558
Open
paulk-asert wants to merge 2 commits into
Open
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #2558 +/- ##
==================================================
- Coverage 68.1904% 68.1838% -0.0066%
- Complexity 33103 33118 +15
==================================================
Files 1508 1508
Lines 126157 126200 +43
Branches 22891 22893 +2
==================================================
+ Hits 86027 86048 +21
- Misses 32490 32513 +23
+ Partials 7640 7639 -1
🚀 New features to boost your workflow:
|
…and ParallelCollectionExtensions
…ariants and deprecate toList(Stream) shim
✅ All tests passed ✅🏷️ Commit: 5bbbd58 Learn more about TestLens at testlens.app. |
There was a problem hiding this comment.
Pull request overview
This PR updates Groovy’s stream/collection GDK helpers in response to GROOVY-12036/12037 by reducing collector allocation overhead and by adding explicit mutable/immutable collection conversion methods (notably around JDK 16+ Stream#toList() shadowing the prior GDK toList(Stream) extension).
Changes:
- Cache commonly used
Collectorinstances and reuse them across calls. - Deprecate
StreamGroovyMethods.toList(Stream)and introduce explicittoMutable*/toImmutable*alternatives for streams. - Add
toImmutableList/toImmutableSetconversions for iterators/iterables and arrays.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 11 comments.
| File | Description |
|---|---|
src/main/java/org/codehaus/groovy/runtime/StreamGroovyMethods.java |
Cache collectors, deprecate toList(Stream), add explicit mutable/immutable stream collectors. |
src/main/java/org/codehaus/groovy/runtime/ParallelCollectionExtensions.java |
Reuse a cached list collector in parallel stream-based operations. |
src/main/java/org/codehaus/groovy/runtime/DefaultGroovyMethods.java |
Add toImmutableList/toImmutableSet for Iterator/Iterable. |
src/main/java/org/codehaus/groovy/runtime/ArrayGroovyMethods.java |
Add toImmutableList and toImmutableSet overloads for primitive and object arrays. |
Comment on lines
+60
to
+75
| @SuppressWarnings({"unchecked", "rawtypes"}) | ||
| private static final Collector TO_LIST = Collectors.toList(); | ||
| @SuppressWarnings({"unchecked", "rawtypes"}) | ||
| private static final Collector TO_SET = Collectors.toSet(); | ||
| @SuppressWarnings({"unchecked", "rawtypes"}) | ||
| private static final Collector TO_UNMODIFIABLE_SET = Collectors.toUnmodifiableSet(); | ||
|
|
||
| @SuppressWarnings({"unchecked", "rawtypes"}) | ||
| private static <T> Collector<T, ?, List<T>> toListCollector() { | ||
| return (Collector) TO_LIST; | ||
| } | ||
|
|
||
| @SuppressWarnings({"unchecked", "rawtypes"}) | ||
| private static <T> Collector<T, ?, Set<T>> toSetCollector() { | ||
| return (Collector) TO_SET; | ||
| } |
Comment on lines
+725
to
+733
| * Accumulates the elements of stream into a new mutable List. | ||
| * <p> | ||
| * <strong>Note:</strong> since JDK 16, {@link Stream} has a native | ||
| * {@code toList()} method that returns an <em>unmodifiable</em> list. | ||
| * Java instance methods take precedence over GDK extensions in both | ||
| * {@code @CompileStatic} and dynamic dispatch, so {@code stream.toList()} | ||
| * now resolves to the native call and yields an immutable result — | ||
| * <em>not</em> a fresh {@code ArrayList} as it did pre-JDK 16. | ||
| * Direct callers of this extension method are pointed at the explicit |
Comment on lines
+56
to
+65
| // Cached Collector: stateless config object; the per-call mutable ArrayList | ||
| // is produced fresh by the Supplier inside collect(), so sharing this | ||
| // instance across threads is safe and saves a per-call allocation. | ||
| @SuppressWarnings({"unchecked", "rawtypes"}) | ||
| private static final Collector TO_LIST = Collectors.toList(); | ||
|
|
||
| @SuppressWarnings({"unchecked", "rawtypes"}) | ||
| private static <T> Collector<T, ?, List<T>> toListCollector() { | ||
| return (Collector) TO_LIST; | ||
| } |
Comment on lines
+15692
to
+15706
| /** | ||
| * Convert an iterator to an immutable List. The iterator will become | ||
| * exhausted of elements after making this conversion. Returns the | ||
| * canonical empty list ({@code List.of()}) when the iterator is empty. | ||
| * | ||
| * @param self an iterator | ||
| * @return an immutable List of the elements from the iterator | ||
| * @since 6.0.0 | ||
| */ | ||
| public static <T> List<T> toImmutableList(Iterator<T> self) { | ||
| List<T> answer = new ArrayList<>(); | ||
| while (self.hasNext()) { | ||
| answer.add(self.next()); | ||
| } | ||
| return List.copyOf(answer); |
Comment on lines
+15701
to
+15707
| public static <T> List<T> toImmutableList(Iterator<T> self) { | ||
| List<T> answer = new ArrayList<>(); | ||
| while (self.hasNext()) { | ||
| answer.add(self.next()); | ||
| } | ||
| return List.copyOf(answer); | ||
| } |
| * @since 6.0.0 | ||
| */ | ||
| public static <T> List<T> toImmutableList(T[] self) { | ||
| return List.of(self); |
| * @since 6.0.0 | ||
| */ | ||
| public static <T> Set<T> toImmutableSet(T[] self) { | ||
| return Set.copyOf(toSet(self)); |
Comment on lines
+15692
to
+15707
| /** | ||
| * Convert an iterator to an immutable List. The iterator will become | ||
| * exhausted of elements after making this conversion. Returns the | ||
| * canonical empty list ({@code List.of()}) when the iterator is empty. | ||
| * | ||
| * @param self an iterator | ||
| * @return an immutable List of the elements from the iterator | ||
| * @since 6.0.0 | ||
| */ | ||
| public static <T> List<T> toImmutableList(Iterator<T> self) { | ||
| List<T> answer = new ArrayList<>(); | ||
| while (self.hasNext()) { | ||
| answer.add(self.next()); | ||
| } | ||
| return List.copyOf(answer); | ||
| } |
Comment on lines
+9967
to
+9977
| /** | ||
| * Converts this array to an immutable List of the same size, with each | ||
| * element added to the list. | ||
| * | ||
| * @param self a boolean array | ||
| * @return An immutable list containing the contents of this array. | ||
| * @since 6.0.0 | ||
| */ | ||
| public static List<Boolean> toImmutableList(boolean[] self) { | ||
| return List.copyOf(toList(self)); | ||
| } |
Comment on lines
+792
to
+857
| /** | ||
| * Accumulates the elements of stream into a new mutable List. | ||
| * <p> | ||
| * Explicit alternative to the native {@link Stream#toList()} (which | ||
| * returns an <em>unmodifiable</em> list since JDK 16). Use this when | ||
| * you need to add to, remove from, or sort the returned list. | ||
| * | ||
| * @param self the stream | ||
| * @param <T> the type of element | ||
| * @return a new mutable {@code java.util.List} instance | ||
| * | ||
| * @since 6.0.0 | ||
| */ | ||
| public static <T> List<T> toMutableList(final Stream<T> self) { | ||
| return self.collect(toListCollector()); | ||
| } | ||
|
|
||
| /** | ||
| * Accumulates the elements of stream into a new mutable List. | ||
| * <p> | ||
| * Explicit-mutability alias for {@link #toList(BaseStream)}, symmetric with | ||
| * {@link #toMutableList(Stream)}. | ||
| * | ||
| * @param self the {@code java.util.stream.BaseStream} | ||
| * @param <T> the type of element | ||
| * @return a new mutable {@code java.util.List} instance | ||
| * | ||
| * @since 6.0.0 | ||
| */ | ||
| public static <T> List<T> toMutableList(final BaseStream<T, ? extends BaseStream> self) { | ||
| return stream(self.iterator()).collect(toListCollector()); | ||
| } | ||
|
|
||
| /** | ||
| * Accumulates the elements of stream into a new mutable Set. | ||
| * <p> | ||
| * Explicit-mutability alias for {@link #toSet(Stream)}. Provided defensively | ||
| * so user intent stays unambiguous if a future JDK release adds a native | ||
| * {@code Stream.toSet()} (which would shadow the GDK extension the way | ||
| * native {@link Stream#toList()} did in JDK 16). | ||
| * | ||
| * @param self the stream | ||
| * @param <T> the type of element | ||
| * @return a new mutable {@code java.util.Set} instance | ||
| * | ||
| * @since 6.0.0 | ||
| */ | ||
| public static <T> Set<T> toMutableSet(final Stream<T> self) { | ||
| return self.collect(toSetCollector()); | ||
| } | ||
|
|
||
| /** | ||
| * Accumulates the elements of stream into a new mutable Set. | ||
| * <p> | ||
| * Explicit-mutability alias for {@link #toSet(BaseStream)}, symmetric with | ||
| * {@link #toMutableSet(Stream)}. | ||
| * | ||
| * @param self the {@code java.util.stream.BaseStream} | ||
| * @param <T> the type of element | ||
| * @return a new mutable {@code java.util.Set} instance | ||
| * | ||
| * @since 6.0.0 | ||
| */ | ||
| public static <T> Set<T> toMutableSet(final BaseStream<T, ? extends BaseStream> self) { | ||
| return stream(self.iterator()).collect(toSetCollector()); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.