I just added this comment to the bug report, but since there's a review thread,
I'll post it here too for discussion:
I don't necessarily think that returning the instance from
Collections.emptyList() is a good idea. It changes the serialized format, and
it may also introduce some subtle behavioral changes. The nCopies() method
could instead return a cached instance of a zero-length CopiesList instance.
I question whether this is worthwhile. Is there an actual case where somebody
is creating large numbers of empty lists, or is this just some random idea
somebody thought up? If the latter, then it's usually not worth pursuing such
changes.
There's no justification for this stated in the bug report, e.g., "we ran into a
case where millions of redundant empty lists were being created...." It smells
to me like this is one of those "optimizations" where somebody looked at the
code and decided it would be a good idea.
s'marks
On 11/14/19 4:34 PM, Martin Buchholz wrote:
Hi Kiran, pleased to meet you
Code like this should be written so that the common case has only one boolean
test.
On Thu, Nov 14, 2019 at 1:50 AM Kiran Ravikumar <
kiran.sidhartha.raviku...@oracle.com> wrote:
Hi Guys,
Please review the following optimization to nCopies method to return empty
list and to avoid instantiating an empty CopiesList object.
JBS link : https://bugs.openjdk.java.net/browse/JDK-8233187
Patch:
diff -r f279d8a9bb78
src/java.base/share/classes/java/util/Collections.java ---
a/src/java.base/share/classes/java/util/Collections.java Wed Nov 13
11:27:50 2019 +0000 +++
b/src/java.base/share/classes/java/util/Collections.java Thu Nov 14
09:37:56 2019 +0000 @@ -5105,6 +5105,8 @@ public static <T> List<T>
nCopies(int n, T o) { if (n < 0) throw new IllegalArgumentException("List
length = " + n); + if (n == 0) + return
Collections.emptyList(); return new CopiesList<>(n, o); }
Thanks,
Kiran
