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

Mikael Gerdin mikael.gerdin at oracle.com
Mon Oct 19 08:23:29 UTC 2015

Hi Thomas,

On 2015-10-19 10:10, Thomas Schatzl wrote:

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::allocationtype template parameter to ResourceHashtable to allow a user to specify C-heap allocation using ResourceObj::CHeap. Currently if one tries to do { ResourceMark rm; ResourceHash* hash = new (ResourceObj::CHEAP) ResourceHash(); hash->put(...) } hash->get(...) The get()-call will crash because the allocation of the hash table nodes does not respect the CHEAP 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 primitivehash() function which is the default one: 36 template unsigned primitivehash(const K& k) { 37 unsigned hash = (unsigned)((uintptrt)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::memberindex 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 :)

Oh, but I actually did those changes last year :)

- resourceHash.cpp: missing include reference to allocation.hpp (for AllStatic), debug.hpp (for assert) Will do.

I do not think I need a re-review for these changes. Seems okay to me otherwise.

Thanks /Mikael

Thanks, Thomas

More information about the hotspot-dev mailing list