RFR: 8291555: Implement alternative fast-locking scheme [v70]
Roman Kennke
rkennke at openjdk.org
Wed May 3 06:15:06 UTC 2023
On Wed, 3 May 2023 03:12:00 GMT, David Holmes <dholmes at openjdk.org> wrote:
>> Roman Kennke has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Add missing new file
>
> src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp line 659:
>
>> 657: // Invariant: tmpReg == 0. tmpReg is EAX which is the implicit cmpxchg comparand.
>> 658: lock();
>> 659: cmpxchgptr(thread, Address(boxReg, OM_OFFSET_NO_MONITOR_VALUE_TAG(owner)));
>
> Sorry I don't quite follow the changes here as this appears to changing the logic for all locking modes - aren't we still supposed to be cas'ing in the "box" (scrReg) in legacy mode rather than the "thread"?
IIRC, I have done that in response to an earlier review by somebody. The previous logic transiently stored box into the owner, and later - if the CAS succeeded - fetches the current thread* and stores that into owner, a few lines down from here. However, I just noticed that I do not remove that other code. So, for the sake of cleanliness of the legacy path, I'm going to revert this (we can & should make that change in a follow-up).
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/10907#discussion_r1183273733
More information about the hotspot-dev
mailing list