RFR: 8282441: [LOOM] The debug agent should attempt to free vthread ThreadNodes [v3]

[Alex Menkov]

On Thu, 20 Nov 2025 20:12:11 GMT, Chris Plummer <cjplummer at openjdk.org> 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.
>
> Chris Plummer has updated the pull request incrementally with one additional commit since the last revision:
> 
>   improve comment

Looks good to me.
Added some minor notes

src/jdk.jdwp.agent/share/native/libjdwp/threadControl.c line 582:

> 580:         /*
> 581:          * Although at first it might seem that a non-zero suspendCount would require
> 582:          * keeping the node, we don't have too if node->suspendCount == suspendAllCount,

Suggestion:

         * keeping the node, we don't have to if node->suspendCount == suspendAllCount,

src/jdk.jdwp.agent/share/native/libjdwp/threadControl.c line 592:

> 590:     }
> 591: 
> 592:     // All of the following conditions must pass if we are to free this node. Note

Suggestion:

    // All of the following conditions must be met to free this node. Note

src/jdk.jdwp.agent/share/native/libjdwp/threadControl.c line 628:

> 626: 
> 627:     ThreadList *list = &runningVThreads;
> 628:     ThreadNode *node = list->first;

Suggestion:

    ThreadNode *node = runningVThreads.first;

-------------

PR Review: https://git.openjdk.org/jdk/pull/28211#pullrequestreview-3490828800
PR Review Comment: https://git.openjdk.org/jdk/pull/28211#discussion_r2548270211
PR Review Comment: https://git.openjdk.org/jdk/pull/28211#discussion_r2548280293
PR Review Comment: https://git.openjdk.org/jdk/pull/28211#discussion_r2548283759