RFR: 8315884: New Object to ObjectMonitor mapping [v9]

[Coleen Phillimore]

On Mon, 15 Jul 2024 00:50:30 GMT, Axel Boldt-Christmas <aboldtch at openjdk.org> wrote:

>> When inflating a monitor the `ObjectMonitor*` is written directly over the `markWord` and any overwritten data is displaced into a displaced `markWord`. This is problematic for concurrent GCs which needs extra care or looser semantics to use this displaced data. In Lilliput this data also contains the klass forcing this to be something that the GC has to take into account everywhere.
>> 
>> This patch introduces an alternative solution where locking only uses the lock bits of the `markWord` and inflation does not override and displace the `markWord`. This is done by keeping associations between objects and `ObjectMonitor*` in an external hash table. Different caching techniques are used to speedup lookups from compiled code.
>> 
>> A diagnostic VM option is introduced called `UseObjectMonitorTable`. It is only supported in combination with the LM_LIGHTWEIGHT locking mode (the default). 
>> 
>> This patch has been evaluated to be performance neutral when `UseObjectMonitorTable` is turned off (the default). 
>> 
>> Below is a more detailed explanation of this change and how `LM_LIGHTWEIGHT` and `UseObjectMonitorTable` works.
>> 
>> # Cleanups
>> 
>> Cleaned up displaced header usage for:
>>   * BasicLock
>>     * Contains some Zero changes
>>     * Renames one exported JVMCI field
>>   * ObjectMonitor
>>     * Updates comments and tests consistencies
>> 
>> # Refactoring
>> 
>> `ObjectMonitor::enter` has been refactored an a `ObjectMonitorContentionMark` witness object has been introduced to the signatures. Which signals that the contentions reference counter is being held. More details are given below in the section about deflation.
>> 
>> The initial purpose of this was to allow `UseObjectMonitorTable` to interact more seamlessly with the `ObjectMonitor::enter` code. 
>> 
>> _There is even more `ObjectMonitor` refactoring which can be done here to create a more understandable and enforceable API. There are a handful of invariants / assumptions which are not always explicitly asserted which could be trivially abstracted and verified by the type system by using similar witness objects._
>> 
>> # LightweightSynchronizer
>> 
>> Working on adapting and incorporating the following section as a comment in the source code
>> 
>> ## Fast Locking
>> 
>>   CAS on locking bits in markWord. 
>>   0b00 (Fast Locked) <--> 0b01 (Unlocked)
>> 
>>   When locking and 0b00 (Fast Locked) is observed, it may be beneficial to avoid inflating by spinning a bit.
>> 
>>   If 0b10 (Inflated) is observed or there is to...
>
> Axel Boldt-Christmas has updated the pull request incrementally with 10 additional commits since the last revision:
> 
>  - Remove try_read
>  - Add explicit to single parameter constructors
>  - Remove superfluous access specifier
>  - Remove unused include
>  - Update assert message OMCache::set_monitor
>  - Fix indentation
>  - Remove outdated comment LightweightSynchronizer::exit
>  - Remove logStream include
>  - Remove strange comment
>  - Fix javaThread include

I have some suggestions that hopefully you can click on if you agree.  Also, some comments.

src/hotspot/share/runtime/lightweightSynchronizer.cpp line 67:

> 65:     }
> 66:     static void* allocate_node(void* context, size_t size, Value const& value) {
> 67:       reinterpret_cast<ObjectMonitorWorld*>(context)->inc_table_count();

Suggestion:

      reinterpret_cast<ObjectMonitorWorld*>(context)->inc_items_count();

src/hotspot/share/runtime/lightweightSynchronizer.cpp line 71:

> 69:     };
> 70:     static void free_node(void* context, void* memory, Value const& value) {
> 71:       reinterpret_cast<ObjectMonitorWorld*>(context)->dec_table_count();

Suggestion:

      reinterpret_cast<ObjectMonitorWorld*>(context)->dec_items_count();

src/hotspot/share/runtime/lightweightSynchronizer.cpp line 125:

> 123:   };
> 124: 
> 125:   void inc_table_count() {

Suggestion:

  void inc_items_count() {

src/hotspot/share/runtime/lightweightSynchronizer.cpp line 126:

> 124: 
> 125:   void inc_table_count() {
> 126:     Atomic::inc(&_table_count);

Suggestion:

    Atomic::inc(&_items_count);

src/hotspot/share/runtime/lightweightSynchronizer.cpp line 129:

> 127:   }
> 128: 
> 129:   void dec_table_count() {

Suggestion:

  void dec_items_count() {

src/hotspot/share/runtime/lightweightSynchronizer.cpp line 130:

> 128: 
> 129:   void dec_table_count() {
> 130:     Atomic::inc(&_table_count);

Suggestion:

    Atomic::inc(&_items_count);

src/hotspot/share/runtime/lightweightSynchronizer.cpp line 134:

> 132: 
> 133:   double get_load_factor() {
> 134:     return (double)_table_count/(double)_table_size;

Suggestion:

    return (double)_items_count/(double)_table_size;

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

PR Review: https://git.openjdk.org/jdk/pull/20067#pullrequestreview-2193868194
PR Review Comment: https://git.openjdk.org/jdk/pull/20067#discussion_r1688563846
PR Review Comment: https://git.openjdk.org/jdk/pull/20067#discussion_r1688563501
PR Review Comment: https://git.openjdk.org/jdk/pull/20067#discussion_r1688565196
PR Review Comment: https://git.openjdk.org/jdk/pull/20067#discussion_r1688565561
PR Review Comment: https://git.openjdk.org/jdk/pull/20067#discussion_r1688565947
PR Review Comment: https://git.openjdk.org/jdk/pull/20067#discussion_r1688566411
PR Review Comment: https://git.openjdk.org/jdk/pull/20067#discussion_r1688566752