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

Axel Boldt-Christmas aboldtch at openjdk.org
Thu Jul 11 10:41:57 UTC 2024


On Thu, 11 Jul 2024 09:25:52 GMT, Yudi Zheng <yzheng at openjdk.org> wrote:

>> Axel Boldt-Christmas has updated the pull request incrementally with four additional commits since the last revision:
>> 
>>  - Add extra comments in LightweightSynchronizer::inflate_fast_locked_object
>>  - Fix typos
>>  - Remove unused variable
>>  - Add missing inline qualifiers
>
> src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp line 843:
> 
>> 841:       movptr(monitor, Address(box, BasicLock::object_monitor_cache_offset_in_bytes()));
>> 842:       // null check with ZF == 0, no valid pointer below alignof(ObjectMonitor*)
>> 843:       cmpptr(monitor, alignof(ObjectMonitor*));
> 
> Is this only for keeping `ZF == 0` and can be replaced by `test; je` if we are not using `jne` to jump to the slow path? Or is there any performance concern? Btw, I think `ZF` is always rewritten before entering into the slow path https://github.com/openjdk/jdk/blob/b32e4a68bca588d908bd81a398eb3171a6876dc5/src/hotspot/cpu/x86/c2_CodeStubs_x86.cpp#L98-L102

You are correct the condition flag is not important here. 

At some point we had more than just `nullptr` and and `ObjectMonitor*` values, but also some small signal values which allowed us to move some slow path code into the runtime. When this was removed I just made the checks do the same on both aarch64 and x86. (Where aarch64 does not have a stub and jumps directly to the continuation requiring the correct condition flags after the branch.)

_Side note: This might be something that will be explored further in the future. And allow to move a lot of the LM_LIGHTWEIGHT slow path code away from the lock node and code stub into the runtime._

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20067#discussion_r1673800250


More information about the serviceability-dev mailing list