RFR JDK-8022442: Fix unchecked warnings in HashMap

[Peter Levart]

On 08/07/2013 12:23 AM, Dan Smith wrote:
> On Aug 6, 2013, at 2:43 PM, Remi Forax <forax at univ-mlv.fr> wrote:
>
>> On 08/06/2013 11:11 PM, Dan Smith wrote:
>>> Please review this warnings cleanup.
>>>
>>> Bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8022442 (not yet visible)
>>> Webrev: http://cr.openjdk.java.net/~dlsmith/8022442/webrev.00/
>>>
>>> —Dan
>> Hi Dan,
>> I've seen that you have introduce a common super interface for entry and tree node,
>> I suppose that  you check that there is no performance regression.
>> I wonder if an abstract class is not better than an interface because as far as I know
>> CHA implemented in hotspot doesn't work on interface
>> (but I may be wrong, there is perhaps a special optimization for arrays).
> To make sure I understand: your concern is that an aastore will be more expensive when assigning to a KeyValueData[] than to an Object[] (or even to SomeOtherClass[])?
>
> For what it's worth, all assignments to table[i] are statically known to be safe.  E.g.:
>
> Entry<K,V> next = (Entry<K,V>) e.next;
> ...
> table[i] = next;
>
> So surely a smart VM only does the check once?
>
> Here are some other things that might be concerns, but don't apply here:
> - interface method invocations: there are no methods in the interface to invoke
> - checkcast to an interface: all the casts are to concrete classes (Entry, TreeBin, TreeNode)
>
> (There are some unchecked casts from KeyValueData to KeyValueData with different type parameters, but I assume these don't cause any checkcasts.)
>
> —Dan
Hi,

FWIW, I compiled old and new HashMap.java and did a javap on the classes.

javap outputs: http://cr.openjdk.java.net/~plevart/misc/hm-8022442/
differences: 
http://cr.openjdk.java.net/~plevart/misc/hm-8022442/hm.javap.stripped.diff

Besides unneeded introduction of local variable discussed already, there 
seem to be 4 new checkcasts in the following locations (new HashMap.java):

   public java.util.HashMap(int, float);
     Code:
        0: aload_0
        1: invokespecial #10                 // Method 
java/util/AbstractMap."<init>":()V
        4: aload_0
        5: getstatic     #11                 // Field 
EMPTY_TABLE:[Ljava/util/HashMap$KeyValueData;
*       8: checkcast     #12                 // class 
"[Ljava/util/HashMap$KeyValueData;"*
       11: putfield      #13                 // Field 
table:[Ljava/util/HashMap$KeyValueData;
       14: aload_0
       15: aconst_null
...

   private void inflateTable(int);
     Code:
...
       22: aload_0
       23: iload_2
       24: anewarray     #44                 // class 
java/util/HashMap$KeyValueData
*      27: checkcast     #12                 // class 
"[Ljava/util/HashMap$KeyValueData;"*
       30: putfield      #13                 // Field 
table:[Ljava/util/HashMap$KeyValueData;
       33: return
...

   void resize(int);
     Code:
...
       20: return
       21: iload_1
       22: anewarray     #44                 // class 
java/util/HashMap$KeyValueData
*      25: checkcast     #12                 // class 
"[Ljava/util/HashMap$KeyValueData;"*
       28: astore        4
       30: aload_0
       31: aload         4
...

   private void readObject(java.io.ObjectInputStream) throws 
java.io.IOException, java.lang.ClassNotFoundException;
     Code:
...
       82: invokevirtual #141                // Method 
sun/misc/Unsafe.putIntVolatile:(Ljava/lang/Object;JI)V
       85: aload_0
       86: getstatic     #11                 // Field 
EMPTY_TABLE:[Ljava/util/HashMap$KeyValueData;
*      89: checkcast     #12                 // class 
"[Ljava/util/HashMap$KeyValueData;"*
       92: putfield      #13                 // Field 
table:[Ljava/util/HashMap$KeyValueData;
       95: aload_1
       96: invokevirtual #142                // Method 
java/io/ObjectInputStream.readInt:()I
...


...but they are all in the infrequently called methods/constructor.


Regards, Peter