RFR: 8269004 Implement ResizableResourceHashtable [v2]

[Kim Barrett]

On Mon, 28 Jun 2021 19:46:59 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`
>
> Ioi Lam has updated the pull request incrementally with one additional commit since the last revision:
> 
>   @coleenp comments

Changes requested by kbarrett (Reviewer).

src/hotspot/share/utilities/resourceHash.hpp line 38:

> 36:     MEMFLAGS MEM_TYPE
> 37:     >
> 38: class ResourceHashtableBase : public ResourceObj {

Rather than a CRTP base class, I think it might be simpler to have a base class that has a type template parameter that provides the sizing/resizing policy. That type might be used either to specify the type of a new member or even a further base class (to benefit from EBO in the size-is-constant case). The derived class constructor would call the base class constructor with a policy object as an argument.

src/hotspot/share/utilities/resourceHash.hpp line 115:

> 113:   }
> 114: 
> 115:   unsigned size() const { return static_cast<const TABLE_IMPL*>(this)->size_impl(); }

I think size() should return the number of entries. The number of buckets should use a different name (assuming it needs to be publically accessible).

-------------

PR: https://git.openjdk.java.net/jdk/pull/4536