RFR: 8357650: ThreadSnapshot to take snapshot of thread for thread dumps [v2]

[Serguei Spitsyn]

On Tue, 27 May 2025 22:48:15 GMT, Alex Menkov <amenkov at openjdk.org> wrote:

>> This is first (hotspot) part of the update for `HotSpotDiagnosticMXBean.dumpThreads` and `jcmd Thread.dump_to_file` to include lock information in thread dumps (JDK-8356870).
>> The update has been split into parts to simplify reviewing.
>> The fix contains an implementation of `jdk.internal.vm.ThreadSnapshot` class to gather required information about a thread.
>> Second (dependent) part includes changes in `HotSpotDiagnosticMXBean.dumpThreads`/`jcmd Thread.dump_to_file`, spec updates and tests for the functionality.
>> 
>> Testing: new `HotSpotDiagnosticMXBean.dumpThreads`/`jcmd Thread.dump_to_file` functionality was tested in loom repo;
>>   sanity tier1 (this fix only)
>
> Alex Menkov has updated the pull request incrementally with one additional commit since the last revision:
> 
>   move to ThreadService

src/hotspot/share/services/threadService.cpp line 1160:

> 1158: 
> 1159:     Type _type;
> 1160:     OopHandle _obj;

Nit: Short comments for `_depth` and `_obj` fields (lines: 1142, 1144, 1160) would be nice to have.

src/hotspot/share/services/threadService.cpp line 1172:

> 1170:   Handle _java_thread;
> 1171:   JavaThread* _thread;
> 1172:   int _depth;

Nit: This naming is confusing and not consistent with naming in JVMTI and other places.
The name `java_thread` is normally used for a `JavaThread*` object.
I would suggest to make it like this:
  Handle _thread_h;         // (or _thread_oop_h, or thread_obj_h)
  JavaThread* _java_thread;

src/hotspot/share/services/threadService.cpp line 1180:

> 1178:   GrowableArray<OwnedLock>* _locks;
> 1179:   Blocker _blocker;
> 1180:   OopHandle _blocker_owner;

Nit: Some short comments explaining the fields would be nice to have, at least for `_depth`, `_retry_handshake`, `_locks`, `_blocker` and `_blocker_owner`.

src/hotspot/share/services/threadService.cpp line 1317:

> 1315:         const ContinuationEntry* ce = _thread->vthread_continuation();
> 1316:         if (ce == nullptr || ce->cont_oop(_thread) != java_lang_VirtualThread::continuation(_java_thread())) {
> 1317:           // TODO: handle

Q: What `TODO` is expected here?

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/25425#discussion_r2111158038
PR Review Comment: https://git.openjdk.org/jdk/pull/25425#discussion_r2110831057
PR Review Comment: https://git.openjdk.org/jdk/pull/25425#discussion_r2110834293
PR Review Comment: https://git.openjdk.org/jdk/pull/25425#discussion_r2111194148