[PATCH] java.util.Collections enhancement

[Tagir Valeev]

Also note that this changes visible behavior. E.g. somebody might
synchronize on the array object returned by toArray call, so this
change might cause unwanted lock sharing.

Once I suggested similar improvement: to return EMPTY_LIST from
Arrays.asList() when supplied array has zero length. It was declined
with the same reason [1]

With best regards,
Tagir Valeev.

[1] http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-September/035197.html

On Tue, Sep 12, 2017 at 4:27 AM, David Holmes <david.holmes at oracle.com> wrote:
> Moving to core-libs-dev ...
>
> On 12/09/2017 5:40 AM, Сергей Цыпанов wrote:
>>
>> Hi,
>>
>> looking into the code of java.util.Collections I've found out that
>> EmptyList#toArray() and EmptySet#toArray() both instantiate new instance of
>> Object[] at each call. As far as Java array is immutable it's possible to
>> cache empty Object array and return this cached instance.
>
>
> There is a slight issue with the specification of toArray here:
>
> "The returned array will be "safe" in that no references to it are
> maintained by this list. (In other words, this method must allocate a new
> array even if this list is backed by an array). "
>
> Of course for a zero-sized array it is perfectly safe to return a cached
> array, but the spec currently requires that a new array be returned.
>
> Cheers,
> David
> -----
>
>> Patch.txt is attached to the body of this mail.
>>
>> Thanks,
>> - Sergei Tsypanov
>>
>> patch.txt
>>
>> diff --git a/src/java.base/share/classes/java/util/Collections.java
>> b/src/java.base/share/classes/java/util/Collections.java
>> --- a/src/java.base/share/classes/java/util/Collections.java
>> +++ b/src/java.base/share/classes/java/util/Collections.java
>> @@ -108,6 +108,11 @@
>>      private static final int INDEXOFSUBLIST_THRESHOLD =   35;
>>       /**
>> +     * Cached empty array
>> +     */
>> +    private static final Object[] EMPTY_ARRAY = {};
>> +
>> +    /**
>>       * Sorts the specified list into ascending order, according to the
>>       * {@linkplain Comparable natural ordering} of its elements.
>>       * All elements in the list must implement the {@link Comparable}
>> @@ -4329,7 +4334,7 @@
>>          public boolean contains(Object obj) {return false;}
>>          public boolean containsAll(Collection<?> c) { return c.isEmpty();
>> }
>>  -        public Object[] toArray() { return new Object[0]; }
>> +        public Object[] toArray() { return EMPTY_ARRAY; }
>>           public <T> T[] toArray(T[] a) {
>>              if (a.length > 0)
>> @@ -4458,7 +4463,7 @@
>>          public boolean contains(Object obj) {return false;}
>>          public boolean containsAll(Collection<?> c) { return c.isEmpty();
>> }
>>  -        public Object[] toArray() { return new Object[0]; }
>> +        public Object[] toArray() { return EMPTY_ARRAY; }
>>           public <T> T[] toArray(T[] a) {
>>              if (a.length > 0)
>
>