On Mon, 10 Nov 2025 03:15:06 GMT, Chris Plummer <[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 564:
> 562: * to keep the node around. It's possible for it to be 0 yet we
> still need to
> 563: * keep the node around. Also, it's possbile for it to be non-zero
> yet we
> 564: * don't need to keep the node around. More details int he comments
> below.
Nit: Typo: s/int he/in the/
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?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/28211#discussion_r2536075055
PR Review Comment: https://git.openjdk.org/jdk/pull/28211#discussion_r2536120552
