On Wed, 16 Oct 2024 10:07:31 GMT, Serguei Spitsyn <sspit...@openjdk.org> wrote:
>> src/java.base/share/classes/java/lang/VirtualThread.java line 219:
>>
>>> 217: public void run() {
>>> 218: // notify JVMTI
>>> 219: vthread.notifyJvmtiStart();
>>
>> The notifyJvmtiMount and notifyJvmtiUnmount (native) methods already have
>> the annotations. Is it really required on the caller too? I'm wondering if
>> the comment on JvmtiMountTransition needs to be expanded to explain this.
>
> The method `java/lang/VirtualThread$VThreadContinuation$1.run()` is starting
> and finishing in a VTMS transition. The issue is with the JVMTI
> `NotifyFramePop`. We need a way to disallow adding `FramePop` event requests
> to its frame because such requests cause problems because they are not used
> to post a `FramePop` events nor they are properly cleared. The annotation
> `@JvmtiMountTransition` is needed to help with this. I'm trying to move the
> `notifyJvmtiStart()/notifyJvmtiEnd()` calls to earlier frame to reduce a
> little bit the scope of VTMS transition. What do you think is the best place
> to explain it? Would placed a comment before annotation
> `@JvmtiMountTransition` for method
> `java/lang/VirtualThread$VThreadContinuation$1.run()` good enough?
No issue from me on moving the notifications to the run method. My comment was
more for the JvmtiMountTransition's class description as it's not easy to audit
the use of this annotation.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21397#discussion_r1803270431
