RFR 8005698 : Handle Frequent HashMap Collisions with Balanced Trees

Mike Duigou mike.duigou at oracle.com
Wed May 29 04:13:35 UTC 2013


- I assume that the JVM falls over dead if the security manager doesn't allow access to the property, correct? I hadn't considered what should happen in the event of a security exception when I originally copied the GetPropertyAction idiom from elsewhere in the JDK. Perhaps add a security manager test to CheckRandomHashSeed? Or two if we want to make sure that 

- initHashSeed could now return the value with assignment happening in the constructor if we wanted to make hashSeed final. This would mean the unsafe stuff would have to return in Hashtable/HashMap to allow for deserialization.


- TreeBin.comparableClassFor() includes "see above for explanation." but it's not clear where that explanation is.

- I was really worried about the cost and correctness issues with changing null key handling. I have now put my mind at ease. All of the cases I could think of seem to be handled.

- I've spent less time in this round looking at the new Map default operations.


- Do we know if ThreadLocalRandom requires that the VM be booted? It may be possible to remove even more stuff here.


- The @run directives run the wrong class (an error I have made myself...).


On May 28 2013, at 13:03 , Brent Christian wrote:

> On 5/28/13 3:09 AM, Doug Lea wrote:
>> To better enable and simplify future improvements, could you
>> do this -- nest the tree classes within HashMap?
> Done.
>> Also, a note on spliterators: I had added these in the
>> least disruptive way (knowing that major changes were coming)
>> by checking exact class match for HashMap.class. This is because
>> there aren't LinkedHashMap view classes to attach overrides to.
>> While not wrong, and OK for now, at some point this should be redone.
> OK.  I will file a bug so this doesn't get forgotten.
> I also applied the change to how HashMap.putAll() resizes the table (to account for splitTreeBin() only handling doubling of tables).
> The updated webrev is here:
> http://cr.openjdk.java.net/~bchristi/8005698/webrev.02/
> Thanks,
> -Brent

More information about the core-libs-dev mailing list