RFR: 8291555: Replace stack-locking with fast-locking

[Robbin Ehn]

On Mon, 12 Sep 2022 07:54:48 GMT, Roman Kennke <rkennke at openjdk.org> wrote:

>>> How have you handled the interpreter lock-stack-area in your implementation? Is it worth to get rid of it and consolidate with the per-thread lock-stack?
>> 
>> At the moment I had to store a "frame id" for each entry in the lock stack.
>> The frame id is previous fp, grabbed from "link()" when entering the locking code.
>> 
>>     private static final void monitorEnter(Object o) {
>> ....
>>         long monitorFrameId = getCallerFrameId();
>> ``` 
>> When popping we can thus check if there is still monitors/locks for the frame to be popped.
>> Remove activation reads the lock stack, with a bunch of assembly, e.g.:
>> `  access_load_at(T_INT, IN_HEAP, rax, Address(rax, java_lang_Thread::lock_stack_pos_offset()), noreg, noreg);
>> `
>> If we would keep this, loom freezing would need to relativize and derelativize these values.
>> (we only have interpreter)
>> 
>> But, according to JVMS 2.11.10. the VM only needs to automatically unlock synchronized method.
>> This code that unlocks all locks in the frame seems to have been added for JLS 17.1.
>> I have asked for clarification and we only need and should care about JVMS.
>> 
>> So if we could make popframe do more work (popframe needs to unlock all), there seems to be way forward allowing more flexibility.
>> 
>> Still working on trying to make what we have public, even if it's in roughly shape and it's very unclear if that is the correct approach at all.
>
>> > How have you handled the interpreter lock-stack-area in your implementation? Is it worth to get rid of it and consolidate with the per-thread lock-stack?
>> 
>> At the moment I had to store a "frame id" for each entry in the lock stack. The frame id is previous fp, grabbed from "link()" when entering the locking code.
>> 
>> ```
>>     private static final void monitorEnter(Object o) {
>> ....
>>         long monitorFrameId = getCallerFrameId();
>> ```
>> 
>> When popping we can thus check if there is still monitors/locks for the frame to be popped. Remove activation reads the lock stack, with a bunch of assembly, e.g.: ` access_load_at(T_INT, IN_HEAP, rax, Address(rax, java_lang_Thread::lock_stack_pos_offset()), noreg, noreg);` If we would keep this, loom freezing would need to relativize and derelativize these values. (we only have interpreter)
> 
> Hmm ok. I was thinking something similar, but instead of storing pairs (oop/frame-id), push frame-markers on the lock-stack.
> 
> But given that we only need all this for the interpreter, I am wondering if keeping what we have now (e.g. the per-frame-lock-stack in interpreter frame) is the saner thing to do. The overhead seems very small, perhaps very similar to keeping track of frames in the per-thread lock-stack.
> 
>> But, according to JVMS 2.11.10. the VM only needs to automatically unlock synchronized method. This code that unlocks all locks in the frame seems to have been added for JLS 17.1. I have asked for clarification and we only need and should care about JVMS.
>> 
>> So if we could make popframe do more work (popframe needs to unlock all), there seems to be way forward allowing more flexibility.
> 
>> Still working on trying to make what we have public, even if it's in roughly shape and it's very unclear if that is the correct approach at all.
> 
> Nice!
> From your snippets above I am gleaning that your implementation has the actual lock-stack in Java. Is that correct? Is there a particular reason why you need this? Is this for Loom? Would the implementation that I am proposing here also work for your use-case(s)?
> 
> Thanks,
> Roman

@rkennke I will have a look, but may I suggest to open a new PR and just reference this as background discussion?
I think most of the comments above is not relevant enough for a new reviewer to struggle through.
What do you think?

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

PR: https://git.openjdk.org/jdk/pull/9680