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

[Alex Menkov]

On Wed, 28 May 2025 18:31:21 GMT, Alex Menkov <amenkov at openjdk.org> wrote:

>> src/hotspot/share/services/threadService.cpp line 1203:
>> 
>>> 1201:   }
>>> 1202: 
>>> 1203:   bool read_reset_retry() {
>> 
>> What does the `read` mean in the name?
>
> read and then reset the `retry` value (return old value)

removed "retry" logic

>> src/hotspot/share/services/threadService.cpp line 1206:
>> 
>>> 1204:     bool ret = _retry_handshake;
>>> 1205:     // If we re-execute the handshake this method need to return false
>>> 1206:     // when the handshake cannot be performed. (E.g. thread terminating)
>> 
>> Unclear what the comment means. The "retry" logic needs to be clearly explained somewhere.
>
> The logic comes from `java_lang_Thread::async_get_stack_trace()` (implementation of `java.lang.Thread.getStackTrace()`) I don't see why the handshake needs to be executed on JavaThread.

dropped "retry" logic

>> src/hotspot/share/services/threadService.cpp line 1531:
>> 
>>> 1529:   if (ste_klass->should_be_initialized()) {
>>> 1530:     ste_klass->initialize(CHECK_NULL);
>>> 1531:   }
>> 
>> Is this actually necessary? Doesn't this klass always get initialized as part of VM initialization?
>
> I suppose it's not necessary, just for safety

removed

>> src/hotspot/share/services/threadService.cpp line 1560:
>> 
>>> 1558:   // call static StackTraceElement[] StackTraceElement.of(StackTraceElement[] stackTrace)
>>> 1559:   // to properly initialize STE.
>>> 1560:   {
>> 
>> Why the additional block scope?
>
> not needed anymore

Removed

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25425#discussion_r2113111153
PR Review Comment: https://git.openjdk.org/jdk/pull/25425#discussion_r2113109960
PR Review Comment: https://git.openjdk.org/jdk/pull/25425#discussion_r2113110414
PR Review Comment: https://git.openjdk.org/jdk/pull/25425#discussion_r2113110713