RFR: 8191786: Thread-SMR hash table size should be dynamic

[Daniel D.Daugherty]

On Tue, 25 May 2021 01:01:15 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Small change to switch Thread-SMR's hash table from ResourceHashtable to KVHashtable
>> so that a variable sized hash table is used instead of a fixed size hash table (1031 elements).
>> Also refactor common hash table size calculation code into static hash_table_size()
>> function and call it from both places.
>> 
>> Test with Mach5 Tier[1-7] testing.
>
> src/hotspot/share/runtime/threadSMR.cpp line 204:
> 
>> 202:   //   "BasicHashtable<(MEMFLAGS)2>::new_entry(unsigned int)", referenced from:
>> 203:   //   KVHashtable<void*, bool, (MEMFLAGS)2, &(unsigned int primitive_hash<void*>(void* const&)), &(bool primitive_equals<void*>(void* const&, void* const&))>::add_if_absent(void*, bool, bool*) in threadSMR.o
>> 204:   //
> 
> We don't need these details in the comment.

True, but they served their purpose! I'm going to check out the fix
that @robehn posted about how to get past this compilation issue.

> src/hotspot/share/runtime/threadSMR.cpp line 854:
> 
>> 852: 
>> 853: // Hash table size should be first power of two higher than twice the
>> 854: // length of the ThreadsList
> 
> What is the basis for this? Seems reasonable for small numbers of threads but excessive for large numbers.

This is where we need @fisk. This comment and the associated code was
written by @fisk in the early days of the Thread-SMR project. So we need
Erik to go spelunking in the recesses of his memory for why he sized things
this way...

Also, I think the comment is a bit wrong and doesn't match the code below.

> src/hotspot/share/runtime/threadSMR.cpp line 857:
> 
>> 855: static int hash_table_size() {
>> 856:   ThreadsList* threads = ThreadsSMRSupport::get_java_thread_list();
>> 857:   int hash_table_size = MIN2((int)threads->length(), 32) << 1;
> 
> Doesn't this start things off a bit small. If the old size was 1031 why not start in that area?

We need @fisk to time in here with his rationale from the beginning of the Thread-SMR project.

The 1031 came from me and I stole it from some other fixed hash table size in the VM.

Someone please correct me if I'm wrong, but doesn't this hash_table_size value max out
at 64? `MIN2((int)threads->length(), 32)` => `32` when there are more than 32 threads
and `32 << 1` => `64`.

Yeah, we really need @fisk to chime in here.

Also, don't forget that these hash_tables are very, very short lived
since they only exist while we are gathering either hazard ptrs or
JavaThread* protected by hazard ptrs.

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

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