On Wed, 28 May 2025 07:10:57 GMT, David Holmes <dhol...@openjdk.org> wrote:
>> 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 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.
> src/hotspot/share/services/threadService.cpp line 1262:
>
>> 1260: // The first stage of async deflation does not affect any
>> field
>> 1261: // used by this comparison so the ObjectMonitor* is usable
>> here.
>> 1262: if (mark.has_monitor()) {
>
> Does this depend on locking mode and non-compact-headers?
AFAIU locking mode should not be lightweight, but in the case
mark.has_monitor() returns false, so no need to additional checks
> 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
> 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
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25425#discussion_r2112569181
PR Review Comment: https://git.openjdk.org/jdk/pull/25425#discussion_r2112597974
PR Review Comment: https://git.openjdk.org/jdk/pull/25425#discussion_r2112616322
PR Review Comment: https://git.openjdk.org/jdk/pull/25425#discussion_r2112616667
