[master] RFR: 8302209: [Lilliput] Optimize fix-anon monitor owner path [v2]

[Thomas Stuefe]

On Tue, 14 Feb 2023 14:47:41 GMT, Roman Kennke <rkennke at openjdk.org> wrote:

>> When trying to exit a monitor, we need to check if the current owner is anonymous, and if so, fix it before exiting the monitor. So far, we have been doing this by calling into the slow-path and handle it in the runtime. However, it is fairly easy to do in the fast-path and avoid calling into the runtime altogether. I also included those changes in [upstream fast-locking PR](https://github.com/openjdk/jdk/pull/10907).
>> 
>> Testing:
>>  - [x] tier1 (x86_64, aarch64)
>>  - [x] tier2 (x86_64, aarch64)
>>  - [ ] jcstress -t sync -m quick
>
> Roman Kennke has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains two commits:
> 
>  - Merge branch 'master' into JDK-8302209
>  - 8302209: [Lilliput] Optimize fix-anon monitor owner path

Questions/Comments:

What is the advantage of using a stub here? The code section between aarch64 and x64 are different and only used in one place each. So you don't really save much by reuse here. Seems you could have just written them out as a conditional path into the various xxfastlock() functions. Then you would have saved the jumps into and out of the stub in the case owner==ANONYMOUS case.

Anonymous means some other thread T2 inflated my lock while I (T1) was thin-lock-waiting on the lock, right?

BTW I found it confusing that we have pre-existing functions called ...fast_unlock and ...fast_lock, since it has nothing to do with your FastLocking, right?

We should rename UseFastLocking to UseRomanLocking :-)

src/hotspot/cpu/aarch64/aarch64.ad line 3931:

> 3929: 
> 3930:     if (UseFastLocking) {
> 3931:       // If the owner is anonymous, we need to fix it -- in the slow-path.

update comment

src/hotspot/cpu/aarch64/aarch64.ad line 3932:

> 3930:     if (UseFastLocking) {
> 3931:       // If the owner is anonymous, we need to fix it -- in the slow-path.
> 3932:       __ ldr(disp_hdr, Address(tmp, ObjectMonitor::owner_offset_in_bytes()));

pre-existing, but reusing the name disp_hdr is confusing here. Create alias?

src/hotspot/share/opto/c2_CodeStubs.hpp line 98:

> 96: 
> 97: #ifdef _LP64
> 98: class C2FixAnonOMOwnerStub : public C2CodeStub {

I dislike the name a bit since it does more than fix the owner in monitor, it also pops one slot from the lock stack.

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

PR: https://git.openjdk.org/lilliput/pull/74