RFR: 8306738: Select num workers for safepoint ParallelCleanupTask [v3]

[Axel Boldt-Christmas]

On Fri, 5 May 2023 17:00:44 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> Axel Boldt-Christmas has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Add expected_num_workers
>
> src/hotspot/share/classfile/stringTable.cpp line 538:
> 
>> 536: }
>> 537: 
>> 538: uint StringTable::rehash_table_expected_workers() {
> 
> We always reply either `0` or `1` from these. So logically, it seems to answer whether we need heavy work. So, this method is then simpler, and matching the structure of the relevant `rehash_table`:
> 
> 
> // Quick checks if we would do any heavy-weight safepoint work in rehash_table.
> // Add shortcut cases here when changing rehash_table.
> bool StringTable::needs_safepoint_rehash_table() {
>   // Grow instead of rehash
>   if (should_grow()) return false;
> 
>   // Already rehashed
>   if (_rehashed) return false;
> 
>   // Should and can do alt-rehashing
>   if (!_needs_rehashing) return false;
>   if (!_local_table->is_safepoint_safe()) return false;
> 
>   return true;
> }

I prefer using a boolean return value as well. I just had trouble with coming up with a non confusing name for the method. Maybe `needs_safepoint_rehash_table` is good enough.

I'll rewrite the condition with multiple ifs. 

My knowledge of the concurrent hash table is rather limited, but what does the `Should and can do alt-rehashing` comment mean? AFAIK the `_local_table->is_safepoint_safe()` check is just to see if someone is currently resizing the table when we entered the safepoint.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13616#discussion_r1187189288