RFR JDK-8011200 (was 7143928) : (coll) Optimize for Empty ArrayList and HashMap

[Mike Duigou]

On Apr 2 2013, at 04:07 , Alan Bateman wrote:

> On 02/04/2013 05:44, Mike Duigou wrote:
>> Hello all;
>> 
>> Last night while pushing another changeset I accidentally pushed a changeset for JKD-7143928. Since the review and testing was not complete on this issue I have backed out that changeset and created a new bug number to continue development. The new webrev to complete the review is:
>> 
>> http://cr.openjdk.java.net/~mduigou/JDK-8011200/0/webrev/
>> 
>> It is currently unchanged from the last posted changeset for 7143928.
>> 
>> Mike
>> 
> I skimmed through the webrev as lazily creating the backing arrays will be help in some environments.
> 
> For ArrayList.readObject then you read the array length in as "initialCapacity" which I think is a bit confusing given the current semantics. An alternative is to just not change readObject unless there is evidence that it is common to reconstitute empty ArrayLists.

I've gone with making ArrayList.writeObject/readObject behave like clone(). They remain forward and backwards compatible with existing impls.

> For HashMap.indexFor then I assume this isn't an assert because it is used early in the VM startup.

Correct.

> This should probably be changed to InternalError and the message needs to be changed too.

It will be removed. :-) I had put this in for my testing.

> 
> Do you envisage usage of inflateTable in LinkedHashMap? I'm wondering why it isn't private.

I wasn't sure when I wrote it. I will make it private.

> HashMap.writeObject - the update to the @serialData text will change the wording emitting in the javadoc.

The "must be a power of 2" is actually an existing but previously undocumented requirement. I will file a CCC case for this.

> I mention this as I think you are planning to push this to jdk7u at some point.

Even though it's been a requirement for the life of this class I won't attempt to push this change to jdk7u.

> I don't have time to go through the test in detail at this time but it looks like it has the original bug number and I assume that will need to change now.

Correct. It will be changed. Transitioning it now.

Mike