RFR (M) 8055283: Expand ResourceHashtable with C_HEAP allocation, removal and some unit tests

[Erik Helin]

Hi Mikael,

On 2015-10-16, Mikael Gerdin wrote:
> Hi all,
> 
> Here's an old favorite. I'm yet again in need of a simple-to-use hash table
> in a place where resource allocation is impossible to use.

Thanks for taking care of this!

> The idea is to add a ResourceObj::allocation_type template parameter to
> ResourceHashtable to allow a user to specify C-heap allocation using
> ResourceObj::C_Heap.
> 
> Currently if one tries to do
> {
> ResourceMark rm;
> ResourceHash* hash = new (ResourceObj::C_HEAP) ResourceHash();
> hash->put(...)
> }
> hash->get(...)
> 
> The get()-call will crash because the allocation of the hash table nodes
> does not respect the C_HEAP parameter.
> 
> To sweeten the deal a little I add support for removing elements and also a
> small bunch of unit tests to verify that the operations work as expected.
> 
> I also discovered a small issue with the primitive_hash() function which is
> the default one:
>   36 template<typename K> unsigned primitive_hash(const K& k) {
>   37   unsigned hash = (unsigned)((uintptr_t)k);
>   38   return hash ^ (hash > 3); // just in case we're dealing with aligned
> ptrs
>   39 }
> 
> It xors hash with the boolean expression (hash > 3) instead of what was
> probably intended (hash >> 3) to deal with aligned pointers as the comment
> suggests.
> It appears that the only user of ResourceHash which doesn't specify its own
> hash function is MethodFamily::_member_index so it would be nice if someone
> could point me at any default method tests to verify that a proper hash
> function doesn't break anything.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8055283
> Webrev: http://cr.openjdk.java.net/~mgerdin/8055283/webrev.1/

Please wrap all your test code in #ifdef ASSERT, we don't want it to be
included in product builds. Then you can also get rid of the #ifdef
ASSERT in the test.

Modulo the above and Thomas's comments, the patch looks good to me. No
need to re-review these changes.

Thanks,
Erik

> Testing: JPRT
> RBT (Kitchensnk, hotspot/test/:hotspot_all)
> 
> 
> Thanks!
> /Mikael