[
https://issues.apache.org/jira/browse/GROOVY-12036?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18084036#comment-18084036
]
ASF GitHub Bot commented on GROOVY-12036:
-----------------------------------------
Copilot commented on code in PR #2558:
URL: https://github.com/apache/groovy/pull/2558#discussion_r3316475966
##########
src/main/java/org/codehaus/groovy/runtime/ArrayGroovyMethods.java:
##########
@@ -10111,6 +10235,161 @@ public static <T> Set<T> toSet(T[] self) {
return DefaultGroovyMethods.toSet(Arrays.asList(self));
}
+
//--------------------------------------------------------------------------
+ // toImmutableSet
+
+ /**
+ * Converts this array to an immutable Set, with each unique element added
to the set.
+ * <pre class="language-groovy groovyTestCase">
+ * boolean[] array = [true, false, true]
+ * Set expected = [true, false]
+ * assert array.toImmutableSet() == expected
+ * </pre>
+ *
+ * @param self a boolean array
+ * @return An immutable set containing the unique contents of this array.
+ * @since 6.0.0
+ */
+ public static Set<Boolean> toImmutableSet(boolean[] self) {
+ return Set.copyOf(toSet(self));
+ }
Review Comment:
The primitive-array `toImmutableSet` overloads call
`Set.copyOf(toSet(self))`, which builds a mutable `Set` and then copies it
again. This adds avoidable allocations and work, and for empty arrays still
pays the cost of building an empty set before returning an empty immutable set.
Consider returning `Collections.emptySet()` for empty arrays and otherwise
wrapping the newly created set with `Collections.unmodifiableSet(...)` (no
second copy).
##########
src/main/java/org/codehaus/groovy/runtime/ArrayGroovyMethods.java:
##########
@@ -9961,6 +9961,130 @@ public static <T> List<T> toList(T[] self) {
return new ArrayList<>(Arrays.asList(self));
}
+
//--------------------------------------------------------------------------
+ // toImmutableList
+
+ /**
+ * 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));
+ }
Review Comment:
The primitive-array `toImmutableList` overloads call
`List.copyOf(toList(self))`, which always allocates an intermediate `ArrayList`
(inside `primitiveArrayToList`) and then copies again in `copyOf`. This is
unnecessary overhead, especially for empty arrays where it still allocates
before returning an empty immutable list. Consider short-circuiting empty
arrays to `Collections.emptyList()` and otherwise wrapping the freshly-created
list with `Collections.unmodifiableList(...)` (no second copy).
> GDK: cache Collectors instances in StreamGroovyMethods and
> ParallelCollectionExtensions
> ---------------------------------------------------------------------------------------
>
> Key: GROOVY-12036
> URL: https://issues.apache.org/jira/browse/GROOVY-12036
> Project: Groovy
> Issue Type: Improvement
> Reporter: Paul King
> Assignee: Paul King
> Priority: Major
>
> Cache Collectors.toList() and Collectors.toSet() as static singletons in
> StreamGroovyMethods and ParallelCollectionExtensions, eliminating per-call
> collector allocation in 9 GDK call sites.
> A benefit in Eclipse Collections as noted here:
> https://donraab.medium.com/counting-and-collecting-collectors-d69b7c9aaca0
> Groovy is not a collections library but this is a very minor improvement with
> no downside.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
