On Wed, 28 May 2025 18:31:21 GMT, Alex Menkov <amen...@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
