On Tue, 13 Aug 2024 14:52:03 GMT, Axel Boldt-Christmas <abold...@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 two 
> additional commits since the last revision:
> 
>  - Remove the last OMWorld references
>  - Rename omworldtable_work to object_monitor_table_work

Two tiny nits.

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

> 170:   static void create() {
> 171:     _table = new ConcurrentTable(initial_log_size(),
> 172:                                 max_log_size(),

nit, can you line up these parameters?

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

> 577: }
> 578: 
> 579: 

Extra blank line.

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

Marked as reviewed by coleenp (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/20067#pullrequestreview-2235807775
PR Review Comment: https://git.openjdk.org/jdk/pull/20067#discussion_r1715446552
PR Review Comment: https://git.openjdk.org/jdk/pull/20067#discussion_r1715449461

Reply via email to