On Thu, 30 May 2024 06:31:19 GMT, Alan Bateman <[email protected]> wrote:
>> src/hotspot/share/prims/jvmtiEnvBase.cpp line 1486:
>>
>>> 1484: if (owning_thread != nullptr) {
>>> 1485: oop thread_oop = get_vthread_or_thread_oop(owning_thread);
>>> 1486: bool is_virtual =
>>> thread_oop->is_a(vmClasses::BaseVirtualThread_klass());
>>
>> It strikes me that this should be handled by
>> `java_lang_VirtualThread::is_instance` based on whether there is
>> continuation support or not. External code like this should not, IMO, needed
>> to know about `BaseVirtualThread`. @AlanBateman what do you think?
>
> Hopefully the ports will catch up someday and the alternative implementation
> can be removed.
>
> We decided not to rename java.lang.VirtualThread when introducing the
> alternative implementation as it's just too disruptive. The super class that
> both implementations extend is BaseVirtualThread so testing for an instance
> of that is correct for the two implementations.
>
> If it helps the readability then introducing a function to test if a thread
> is a virtual thread might help. It could use VMContinuations if needed but
> right now, testing for an instanceof BaseVirtualThread is okay.
Okay. I still think that should be hidden behind the
`java_lang_VirtualThread::is_instance` as it is an implementation detail the
JVMTI and thread code shouldn't need to know about IMO. Once the alternative
implementation is removed I expect these explicit checks for
`BaseVirtualThread` will need to be reverted and we could avoid that if we make
a change now.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/19405#discussion_r1620065937
