On Wed, 5 Feb 2025 04:18:14 GMT, Chris Plummer <cjplum...@openjdk.org> wrote:
> Cleanup SA JavaThread support. Details in first comment:
>
> Testing:
> - Tier1
> - Tier2 svc
> - Tier3
> - Tier5 svc
> - Local testing of debuggee using graal java compiler threads. Verified that
> the compiler threads shows up in jstack output if the property is set.
FWIW this cleanup looks good to me and makes a lot of sense. I guess there will
be a change in behaviour in that some previously excluded threads will now
appear in the stack dump. But that shouldn't affect any real code as nothing
should presume to know the set of threads anyway.
I will leave it to other serviceability folk to click the actual Approve
button, but thanks again for this cleanup!
src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/runtime/JavaThread.java
line 36:
> 34: import sun.jvm.hotspot.utilities.Observer;
> 35:
> 36: /** This class is no longer abstract. Platform dependent functionality is
> now implmented
Suggestion:
/** Platform dependent functionality is now implemented
"no longer" very quickly loses context if you don't know when it was abstract,
or why.
src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/runtime/Threads.java line
161:
> 159: virtualConstructor.addMapping("JvmtiAgentThread",
> JavaThread.class);
> 160: virtualConstructor.addMapping("NotificationThread",
> JavaThread.class);
> 161: virtualConstructor.addMapping("AttachListenerThread",
> JavaThread.class);
It still seems a shame that we have to enumerate all the thread sybtypes.
-------------
PR Review: https://git.openjdk.org/jdk/pull/23456#pullrequestreview-2594628871
PR Review Comment: https://git.openjdk.org/jdk/pull/23456#discussion_r1942248757
PR Review Comment: https://git.openjdk.org/jdk/pull/23456#discussion_r1942249801
