RFR 8005698 : Handle Frequent HashMap Collisions with Balanced Trees

[Mike Duigou]

Understood that it is necessary to have some mechanism to have reproducible results. The command line system property is by default disabled. Enabling it has only the effect of setting the hashSeed to a non-zero XOR value for each instance. Since the randomness is per instance there's no single value to print out. We considered a switch, like python, to allow using the same non-zero seed value to all instances but it didn't seem to offer much additional debugging capability so we opted to not implement it.

Mike

On May 29 2013, at 05:36 , David Chase wrote:

> It looks like there's no way to set the random seed from the command line.  That's going to make Heisenbugs even less fun than they already are.  Ideally, if a VM crashes, that random seed could get mentioned somewhere in the crash, and someone who is debugging might then be able to specify it through a command-line property.  It's not a guarantee of a repeatable run, but it at least removes an obstacle.

> David
> 
> On 2013-05-29, at 12:13 AM, Mike Duigou <mike.duigou at oracle.com> wrote:
> 
>> Hashtable/WeakHashMap::
>> 
>> - 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.
>> 
>> 
>> HashMap::
>> 
>> - 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.
>> 
>> Hashing::
>> 
>> - Do we know if ThreadLocalRandom requires that the VM be booted? It may be possible to remove even more stuff here.
>> 
>> 
>> InPlaceOpsCollisions::
>> 
>> - The @run directives run the wrong class (an error I have made myself...).
>> 
>> Mike
>> 
>> 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
>> 
>