On Tue, 18 Nov 2025 02:45:18 GMT, Serguei Spitsyn <[email protected]> wrote:
>> This is first of what will probably be 3 PRs to improve virtual thread
>> ThreadNode handling in the debug agent, with a goal of improving performance
>> and reducing footprint.
>>
>> This PR focuses on purging virtual thread ThreadNodes in two places:
>>
>> 1. Freeing the ThreadNode after handling a THREAD_START event for a virtual
>> thread.
>> 2. Doing a "GC" of virtual thread ThreadNodes just before doing a "suspend
>> all"
>>
>> At some point in the future I want to attempt to free the ThreadNodes after
>> performing ThreadReference related commands, which will result in the
>> creation of a ThreadNode if not already created. Another area is in
>> handleReportEventCompositeCommand() when the thread is not null and we are
>> using the SUSPEND_NONE policy. This will get more ThreadNodes freed
>> immediately after handling an event.
>>
>> Part of the challenge with this PR is that there are many places in the
>> debug agent that expect a ThreadNode to already have been created for the
>> virtual thread, but now it is common that they have not. The main way this
>> has been addressed is by having findRunningThread() create the ThreadNode if
>> it does not already exist.
>>
>> At some point in the future I will probably add logic to only "GC"
>> ThreadNodes after the number reaches some threshold, but right now I want to
>> free them very aggressively so we'll catch any bugs where there debug agent
>> expects a ThreadNode to exist, but possibly now it might not.
>>
>> Tested by running all tier2, tier5, and tier6 CI svc tests.
>
> src/jdk.jdwp.agent/share/native/libjdwp/threadControl.c line 1399:
>
>> 1397: if (node == NULL) {
>> 1398: node = findThread(&otherThreads, thread);
>> 1399: }
>
> Q: Could you explain a little bit why this change is needed? Do we need an
> update of the comment above?
You need to call findRunningThread() to get the ThreadNode created if there
isn't already one. findThread() does not create the ThreadNode like
findRunningThread() does, so after my changes to free up ThreadNodes,
findThread() was returning NULL, which causes a lot of problems here.
You then need to add the findThread(&otherThreads, thread) call because this is
normally done by findThread() but is purposefully note done by
findRunningThread(). otherThreads hold threads that have been created but not
yet started. It's kind of rare that threads every end up on the list. It means
the debugger somehow got hold of a ThreadReferences (such as through a local
var) and then called a ThreadReference API with it. The usual way it works is
when an event finally comes in on the thread, that is when it gets moved from
otherThreads to runningThreads or runningVThreads.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/28211#discussion_r2536308645
