JDK-8191786 : Thread-SMR hash table size should be dynamic
  • Type: Enhancement
  • Component: hotspot
  • Sub-Component: runtime
  • Affected Version: 10
  • Priority: P4
  • Status: Resolved
  • Resolution: Fixed
  • OS: generic
  • CPU: generic
  • Submitted: 2017-11-22
  • Updated: 2021-06-07
  • Resolved: 2021-05-27
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 17
17 b25Fixed
Related Reports
Relates :  
JDK-8167108 - inconsistent handling of SR_lock can lead to crashes
Description
A comment from Robin W's code review on JDK-8167108,
reminded us about a hash table sizing issue:

> 4745   hash_table_size--;
> 4746   hash_table_size |= hash_table_size >> 1;
> ...
>
> This calculation is repeated around line 4922 as well, perhaps put it in a function?

The hash_table_size parameter is currently unused. We were using
a different hash table before that allowed the size to be set.
Unfortunately, that hash table didn't support being freed so we
switched to ResourceHashtable.

We have a project note to come back and update the underlying
hash table to work with dynamic table sizes, but Erik hasn't
had the cycles to do it yet.
Comments
Changeset: 23189a1f Author: Daniel D. Daugherty <dcubed@openjdk.org> Date: 2021-05-27 14:58:26 +0000 URL: https://git.openjdk.java.net/jdk/commit/23189a1f9de5e7c039a4f6b9e5eefe4fa3c6dcef
27-05-2021
[~eosterlund], [~rehn] or [~coleenp] - Do we have a hashtable that allows a dynamic size to be specified AND allows the table to be freed? We allocate a new one of these ThreadScanHashtable objects every time that ThreadsSMRSupport::free_list() or is_a_protected_JavaThread() is called. We're currently using a fixed size of 1031 for our Thread-SMR hash table. Erik's original code (from so very long ago) used this: // Hash table size should be first power of two higher than twice // the length of the Threads list. but that hash table didn't support being freed so we had to switch to ResourceHashtable which has the size fixed at compile time.
11-02-2021
Took a look at the current code base to re-familiarize myself with this issue. src/hotspot/share/runtime/threadSMR.cpp: void ThreadsSMRSupport::free_list(ThreadsList* threads) { <snip> // Hash table size should be first power of two higher than twice the length of the ThreadsList int hash_table_size = MIN2((int)get_java_thread_list()->length(), 32) << 1; hash_table_size = round_up_power_of_2(hash_table_size); // Gather a hash table of the current hazard ptrs: ThreadScanHashtable *scan_table = new ThreadScanHashtable(hash_table_size); <snip> bool ThreadsSMRSupport::is_a_protected_JavaThread(JavaThread *thread) { assert_locked_or_safepoint(Threads_lock); // Hash table size should be first power of two higher than twice // the length of the Threads list. int hash_table_size = MIN2((int)get_java_thread_list()->length(), 32) << 1; hash_table_size = round_up_power_of_2(hash_table_size); // Gather a hash table of the JavaThreads indirectly referenced by // hazard ptrs. ThreadScanHashtable *scan_table = new ThreadScanHashtable(hash_table_size); So we still have duplicated code that calculates hash_table_size manually in two different functions. And... src/hotspot/share/runtime/threadSMR.cpp: class ThreadScanHashtable : public CHeapObj<mtThread> { <snip> int _table_size; // ResourceHashtable SIZE is specified at compile time so our // dynamic _table_size is unused for now; 1031 is the first prime // after 1024. typedef ResourceHashtable<void *, int, &ThreadScanHashtable::ptr_hash, &ThreadScanHashtable::ptr_equals, 1031, ResourceObj::C_HEAP, mtThread> PtrTable; ... the _table_size is still unused. I took a look at the current ResourceHashtable code and it still only supports a table size that is fixed at compile time.
11-02-2021