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