On Wed, 14 Aug 2024 09:24:34 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 one > additional commit since the last revision: > > Use jdk.test.lib.Utils.getRandomInstance() Just a couple of comments this time. I originally reviewed v10 and this time I reviewed v10..v18. src/hotspot/share/runtime/synchronizer.hpp line 126: > 124: > 125: static bool quick_notify(oopDesc* obj, JavaThread* current, bool All); > 126: Why add the extra blank line? ------------- Marked as reviewed by dcubed (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/20067#pullrequestreview-2239178344 PR Review Comment: https://git.openjdk.org/jdk/pull/20067#discussion_r1717549253
