On Thu, 30 May 2024 06:14:21 GMT, David Holmes <[email protected]> wrote:
>> SendaoYan has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> change from java_lang_VirtualThread::is_instance(thread_oop) to
>> hread_oop->is_a(vmClasses::BaseVirtualThread_klass()) in
>> Threads::get_pending_threads()
>
> 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.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/19405#discussion_r1620033427
