RFR: 8269004 Implement ResizableResourceHashtable
Coleen Phillimore
coleenp at openjdk.java.net
Wed Jun 23 01:44:29 UTC 2021
On Mon, 21 Jun 2021 04:31:42 GMT, Ioi Lam <iklam at openjdk.org> wrote:
> In HotSpot we have (at least) two hashtable designs in the C++ code:
>
> - share/utilities/hashtable.hpp
> - share/utilities/resourceHash.hpp
>
> Of the two, the `ResourceHashtable` API is much cleaner and most new code has been written with it. However, one issue is that the `SIZE` of `ResourceHashtable` is a compile-time constant. This makes the hash-to-index computation very fast on x64 (gcc can avoid using the slow divq instruction for modulo). However, the downside is we cannot use `ResourceHashtable` when we need a hashtable whose size is determined at run time (and, optionally, resizeable).
>
> This PR refactors `ResourceHashtable` into a base template class `ResourceHashtableBase`, whose `size()` function can be configured by a subclass to be either constant or runtime-configurable.
>
> Note: since we want to preserve the performance of `hash % SIZE`, we can't make `size()` a virtual function.
>
> Preliminary benchmark shows that this refactoring has no impact on the performance of the constant `ResourceHashtable`. See https://github.com/iklam/tools/tree/main/bench/resourceHash:
>
> *before*
> ResourceHashtable: 2.70 sec
>
> *after*
> ResourceHashtable: 2.72 sec
> ResizableResourceHashtable: 5.29 sec
>
> To make sure `ResizableResourceHashtable` works, I rewrote some CDS code to use `ResizableResourceHashtable` instead of `KVHashtable`
This looks really good to me.
src/hotspot/share/cds/classListParser.hpp line 37:
> 35:
> 36: class constantPoolHandle;
> 37: class Thread;
I was looking for the use of constantPoolHandle in the header and I know why the forward declaration is needed. Shouldn't this declaration use a const reference so that the handle code doesn't create an unnecessary copy?
bool ClassListParser::is_matching_cp_entry(constantPoolHandle &pool, int cp_index, TRAPS) {
src/hotspot/share/utilities/resourceHash.hpp line 252:
> 250: // http://stackoverflow.com/questions/8532961/template-argument-of-type-that-is-defined-by-inner-typedef-from-other-template-c
> 251: //typename ResourceHashtableFns<K>::hash_fn HASH = primitive_hash<K>,
> 252: //typename ResourceHashtableFns<K>::equals_fn EQUALS = primitive_equals<K>,
Can you remove this xlC comment? Not sure why we care.
-------------
Marked as reviewed by coleenp (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/4536
More information about the hotspot-dev
mailing list