On Tue, 19 Dec 2023 01:17:10 GMT, David Holmes <dhol...@openjdk.org> wrote:
>> HeapDumper dumps virtual threads in 2 places:
>> - dumping platform threads (mounted virtual threads are dumped as separate
>> thread object);
>> - dumping heap objects when the object is `java.lang.VirtualThread`.
>>
>> In the 2nd case mounted virtual threads should be skipped (as they are
>> already dumped with correct stack traces/stack references)
>> Check that a virtual thread is mounted is non-trivial, method from
>> JvmtiEnvBase was used for this.
>>
>> Testing: tier1..3, heapdump-related tests:
>> open/test/hotspot/jtreg/serviceability,open/test/hotspot/jtreg/runtime/ErrorHandling,open/test/hotspot/jtreg/gc/epsilon,open/test/jdk/sun/tools/jhsdb
>
> src/hotspot/share/services/heapDumper.cpp line 1647:
>
>> 1645: static bool is_vthread_mounted(oop vt) {
>> 1646: return JvmtiEnvBase::get_JavaThread_or_null(vt) != nullptr;
>> 1647: }
>
> It doesn't seem appropriate to couple this to the JVMTI code (can this code
> be present if JVMTI is not part of the build?). Doesn't the VT state give you
> a good enough approximation of whether it is mounted i.e. RUNNABLE?
Good point. I'll remove dependency on JVMTI.
I don't think approximation would be good here (comparing state to
RUNNABLE/PINNED/TIMED_PINNED or comparing carrierThread with null).
It's racy and we have a chance to not dump unmounted vthread or dump mounted
vthread twice.
Maybe `is_vthread_mounted` should check if the virtual thread continuation has
non-empty chunk.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17134#discussion_r1430893424
