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

Alan Bateman Alan.Bateman at oracle.com
Tue Apr 2 11:07:43 UTC 2013


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 then it might be good to add a constant 
DEFAULT_INITIAL_CAPACITY or some such to avoid repeating the specified 
default.

I agree with Martin's comment that ArrayList.clear will now clear 
elementData.length elements rather than size so I assume you'll address 
that.

I also agree with Martin on keeping the coding style consistent, 
particularly the missing space in "if(", and missing braces in places 
such as HashMap.writeObject.

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.

For HashMap.indexFor then I assume this isn't an assert because it is 
used early in the VM startup. This should probably be changed to 
InternalError and the message needs to be changed too.

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

HashMap.writeObject - the update to the @serialData text will change the 
wording emitting in the javadoc. I mention this as I think you are 
planning to push this to jdk7u at some point.

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.

-Alan.



More information about the core-libs-dev mailing list