RFR: 8319801: Recursive lightweight locking: aarch64 implementation [v6]
Coleen Phillimore
coleenp at openjdk.org
Wed Jan 24 15:52:32 UTC 2024
On Wed, 24 Jan 2024 14:54:35 GMT, Roman Kennke <rkennke at openjdk.org> wrote:
>> The current implementation requires the first emitted instruction of `C1_MacroAssembler::lock_object` to trap if the obj is null. We do it here with a load.
>>
>> I could instrument `MacroAssembler::lightweight_lock` with a `bool preload_mark` so that C1 does not do the redundant load, while still emitting its first trapping instruction, and not create redundant loads for the interpreter and native wrappers.
>>
>> I think that we want to move all platforms to what PPC is doing, mainly that C2, C1 and native wrapper shares a common lock/unlock implementation (as they lock + unlock under the same balanced assumptions), while have `MacroAssembler::lightweight_lock` solely dedicated to the interpreter.
>>
>> I think I will update `MacroAssembler::lightweight_lock` to have this `preload_mark` bool. You are not the first to ask about this specific line, and it might make it more clear. With a comment about the C1 implicit null check.
>
> Ok I understand, thank you! I actually like it more the way you did it before (without the bool preload_mark flag). An extra comment there may be useful.
> Or maybe consider to load the mark in MacroAssembler as first instruction unconditionally? As far as I can tell mark-word is not used in the lock-stack-full case (very uncommon) and recursive (more common), but it's only really relevant for interpreter and shared-runtime (C2 has its own impl, C1 needs to have the load there anyway), where the extra cycle don't really matter anyway? Would keep the code cleaner. But it means there is an implicit behavioural dependency between the locking impl and the C1 impl, which might be problematic.
> But let's not add bools and different code-paths, this is even more confusing.
Me too, I prefer not having the extra bool passed around.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/16608#discussion_r1465135120
More information about the hotspot-dev
mailing list