RFR: 8291555: Implement alternative fast-locking scheme [v29]
Roman Kennke
rkennke at openjdk.org
Tue Mar 28 13:08:10 UTC 2023
On Fri, 24 Mar 2023 10:47:11 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:
>> Roman Kennke has updated the pull request incrementally with two additional commits since the last revision:
>>
>> - Merge remote-tracking branch 'origin/JDK-8291555-v2' into JDK-8291555-v2
>> - Set condition flags correctly after fast-lock call on aarch64
>
> src/hotspot/cpu/aarch64/aarch64.ad line 3954:
>
>> 3952: // Indicate success on completion.
>> 3953: __ cmp(oop, oop);
>> 3954: __ b(count);
>
> `aarch64_enc_fast_lock` explicitly sets NE on failure path. But this code just jumps to `no_count` without setting the flag. Does the code outside this encoding block rely on flags?
The code outside this encoding block relies on flags, yes. This is very ugly. fast_unlock() jumps to no_count when the CAS fails, where the NE flag is set, therefore we don't need to set it again.
> src/hotspot/share/runtime/synchronizer.cpp line 923:
>
>> 921: static bool is_lock_owned(Thread* thread, oop obj) {
>> 922: assert(UseFastLocking, "only call this with fast-locking enabled");
>> 923: return thread->is_Java_thread() ? reinterpret_cast<JavaThread*>(thread)->lock_stack().contains(obj) : false;
>
> Here and later, should use `JavaThread::cast(thread)` instead of `reinterpret_cast<JavaThread*>`? It also sometimes subsumes the asserts, as `JT::cast` checks the type.
The problem is that the places where that helper function is used receive a Thread* and not a JavaThread* (FastHashCode() and inflate()), and changing those to accept JavaThread* percolates into various areas that I don't want to touch right now (e.g. finalizerService.cpp). That is the reason why that function exists to begin with. I'll do the changes that @shipilev suggested for the time being. We may want to investigate restricting the incoming type in the future.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/10907#discussion_r1150580134
PR Review Comment: https://git.openjdk.org/jdk/pull/10907#discussion_r1150576745
More information about the serviceability-dev
mailing list