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

Mike Duigou mike.duigou at oracle.com
Tue Apr 9 23:55:53 UTC 2013


On Apr 8 2013, at 16:03 , Martin Buchholz wrote:

> Style: spaces around "="
> +    private static final int DEFAULT_CAPACITY= 10;

I apologize for these minor style problems in this and my other patches. I am really loathe to run any kind of automated source reformatting tool on JDK source. Unfortunately there are a variety of styles sued within the JDK sources and the value of having minimal diffs is too great to contemplate global restyling. I am happy to correct any style or other errors I make and don't notice myself.

> ---
> 
> It's a matter of taste whether to remove the temp var oldCapacity.  I believe it was coded intentionally.
>      public void trimToSize() {
>          modCount++;
> -        int oldCapacity = elementData.length;
> -        if (size < oldCapacity) {
> +        if (size < elementData.length) {
>              elementData = Arrays.copyOf(elementData, size);
>          }

I assumed that oldCapacity was a remnant of a previous implementation prior to the introduction of Arrays.copyOf(). It didn't seem to be worth retaining even for explanatory value.

> ---
> 
> Because "threshold" is a serialized field, its javadoc is part of the public interface of this class, and hence cannot refer to implementation details like EMPTY_TABLE.
>  161     /**
>  162      * The next size value at which to resize (capacity * load factor). If
>  163      * table == EMPTY_TABLE then this is the initial capacity at which the
>  164      * table will be created when inflated.
>  165      * @serial
>  166      */
>  167     int threshold;
> http://docs.oracle.com/javase/7/docs/api/serialized-form.html#java.util.HashMap
> 
> 
> ---
> 
> Consider making roundUpToPowerOf2 public.

I assume you mean as part of some utility class like Integer? Reasonable and increases the chances that some VM engineer will come along and optimize it. Not sure what should be done about negative numbers.

This likely comes too late for Java 8 though. (I know I probably don't have time).

> 
> Best Implementation is not obvious.  I would probably write a loop with core
> x = x & (x - 1)
> until get to zero.
>  274     private static int roundUpToPowerOf2(int number) {
>  275         int rounded = number >= MAXIMUM_CAPACITY
>  276                 ? MAXIMUM_CAPACITY
>  277                 : (rounded = Integer.highestOneBit(number)) != 0
>  278                     ? (Integer.bitCount(number) > 1) ? rounded << 1 : rounded
>  279                     : 1;
>  280 
>  281         return rounded;
>  282     }

The Integer operations are Hotspot intrinsics. This version is coded so that there are no loops. The previous looping implementation was criticized in an earlier review. It may be better to convert this to the Hacker's Delight 3-3 implementation




More information about the core-libs-dev mailing list