RFR (M) 8055283: Expand ResourceHashtable with C_HEAP allocation, removal and some unit tests
Thomas Schatzl
thomas.schatzl at oracle.com
Mon Oct 19 08:10:32 UTC 2015
Hi,
On Fri, 2015-10-16 at 10:30 +0200, 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.
>
> 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/
- resourceHash.?pp: if you change copyrights, please change them to
the correct year :)
- resourceHash.cpp: missing include reference to allocation.hpp (for
AllStatic), debug.hpp (for assert)
I do not think I need a re-review for these changes.
Seems okay to me otherwise.
Thanks,
Thomas
More information about the hotspot-dev
mailing list