RFR: 8348347: Cleanup JavaThread subclass support in SA

[David Holmes]

On Wed, 5 Feb 2025 04:18:14 GMT, Chris Plummer <cjplummer at 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