On Mon, 12 Sep 2022 07:54:48 GMT, Roman Kennke <rken...@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
