RFR: 8035584: (s) ArrayList(c) should avoid inflation if c is empty

[Mike Duigou]

I've actually always used scp. :-)

Since I accepted all of your changes as suggested and had no other changes I was just going to go ahead and push once testing was done.

I've now prepared a revised webrev and can still accept feedback.

http://cr.openjdk.java.net/~mduigou/JDK-8035584/1/webrev/

(Note: The webrev also contains https://bugs.openjdk.java.net/browse/JDK-8037097 since I am testing/pushing the two issues together.)

Mike

On Mar 11 2014, at 16:42 , Martin Buchholz <martinrb at google.com> wrote:

> I don't see the updated webrev.  Maybe you also fell victim to "rsync to cr no longer working"?
> 
> 
> On Tue, Mar 11, 2014 at 4:34 PM, Mike Duigou <mike.duigou at oracle.com> wrote:
> 
> On Feb 21 2014, at 14:56 , Martin Buchholz <martinrb at google.com> wrote:
> 
>> You should do <tt> -> code conversion separately, and do it pervasively across the entire JDK.
> 
> From your lips to God's ears.... I keep suggesting this along with a restyle to official style every time we create new repos. Seems unlikely unfortunately as it makes backports harder. 
> 
>> This is not right.
>> +     * {@code (o==null ? get(i)==null : o.equals(get(i)))}
> 
> Corrected.
> 
>> You accidentally deleted a stray space here?
>> 
>> -        this.elementData = EMPTY_ELEMENTDATA;
>> +       this.elementData = EMPTY_ELEMENTDATA;
> 
> Corrected.
> 
>>      public ArrayList(int initialCapacity) {
>> -        super();
>>          if (initialCapacity < 0)
>>              throw new IllegalArgumentException("Illegal Capacity: "+
>>                                                 initialCapacity);
>> -        this.elementData = new Object[initialCapacity];
>> +        this.elementData = (initialCapacity > 0)
>> +                ? new Object[initialCapacity]
>> +                : EMPTY_ELEMENTDATA;
>>      }
>> 
>> When optimizing for special cases, we should try very hard minimize overhead in the common case.  In the above, we now have two branches in the common case.  Instead,
>> 
>> if (initialCapacity > 0) this.elementData = new Object[initialCapacity];
>> else if (initialCapacity == 0) this.elementData = EMPTY_ELEMENTDATA;
>> else barf
> 
> Corrected.
> 
> Thanks as always for the feedback.
> 
> Mike
> 
>> 
>> 
>> 
>> On Fri, Feb 21, 2014 at 2:41 PM, Mike Duigou <mike.duigou at oracle.com> wrote:
>> Hello all;
>> 
>> This changeset consists of two small performance improvements for ArrayList. Both are related to the lazy initialization introduced in JDK-8011200.
>> 
>> The first change is in the ArrayList(int capacity) constructor and forces lazy initialization if the requested capacity is zero. It's been observed that in cases where zero capacity is requested that it is very likely that the list never receives any elements. For these cases we permanently avoid the allocation of an element array.
>> 
>> The second change, noticed by Gustav Åkesson, involves the ArrayList(Collection c) constructor. If c is an empty collection then there is no reason to inflate the backing array for the ArrayList.
>> 
>> http://cr.openjdk.java.net/~mduigou/JDK-8035584/0/webrev/
>> 
>> I also took the opportunity to the <tt></tt> -> {@code } conversion for the javadoc.
>> 
>> Enjoy!
>> 
>> Mike
>> 
> 
>