On Tue, 25 Nov 2025 21:56:47 GMT, Alex Menkov <[email protected]> wrote:
>> Don't enabled VIRTUAL_THREAD_START/END events unless absolutely necessary.
>> Solves performance issues when trying to debug apps that create a lot of
>> virtual threads. Details in first comment.
>>
>> With these changes the Skynet benchmark no longer shows any slowdown when
>> launching with debugging enabled or when attaching the debugger.
>>
>> Tested with all tier2, tier3, tier5, and tier6 CI testing (with filters to
>> only run svc tests).
>
> src/jdk.jdwp.agent/share/native/libjdwp/eventFilter.c line 1296:
>
>> 1294: {
>> 1295: jvmtiError error = JVMTI_ERROR_NONE;
>> 1296: int ei = NODE_EI(node);
>
> Suggestion:
>
> EventIndex ei = NODE_EI(node);
ok
> src/jdk.jdwp.agent/share/native/libjdwp/eventFilter.c line 1394:
>
>> 1392: jvmtiError error2 = JVMTI_ERROR_NONE;
>> 1393: jthread thread;
>> 1394: int ei = NODE_EI(node);
>
> Suggestion:
>
> EventIndex ei = NODE_EI(node);
ok
> src/jdk.jdwp.agent/share/native/libjdwp/eventHandler.c line 1691:
>
>> 1689: * a deferred event request.
>> 1690: */
>> 1691: if (gdata->virtualThreadStartEventsPermanentlyEnabled) {
>
> Looks like this block can disable VITRUAL_THREAD_START when
> `rememberVThreadsWhenDisconnected` is set.
virtualThreadStartEventsPermanentlyEnabled is only set true if includeVThreads
is false, which means rememberVThreadsWhenDisconnected is also false (they are
always set the same, as I discuss in my new comment below).
virtualThreadStartEventsPermanentlyEnabled could use a better name because it
means "permanently enabled for handling deferred event enabling", whereas if
includeVThreads is true, VITRUAL_THREAD_START events are always permanently
enabled, but virtualThreadStartEventsPermanentlyEnabled will never be set true.
> src/jdk.jdwp.agent/share/native/libjdwp/util.h line 90:
>
>> 88: jboolean includeVThreads; /* If true, VM.AllThreads includes
>> vthreads. */
>> 89: jboolean rememberVThreadsWhenDisconnected;
>> 90: jboolean virtualThreadStartEventsPermanentlyEnabled;
>
> Could you please add description for this 2 flags (and maybe
> `vthreadsSupported` description also can be enhanced to see relations between
> the flags)
I'm going to get rid of rememberVThreadsWhenDisconnected and just replace
references to it with includeVThreads. Right now they are always set the same.
I was considering someday making it a flag that could be set on the command
line, but I don't seen a need for that anymore. I'd like to get rid of
includeVThreads too, but some tests still rely on it. I'll files a CR for all
this work soon.
I also plan on getting rid of vthreadsSupported. I doubt setting it to false
even works. It's just a remnant from the early days of development of vthread
support. It should just always be assumed true now. I'll file a CR for this
also.
I'll add some comments for virtualThreadStartEventsPermanentlyEnabled.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/28485#discussion_r2566838053
PR Review Comment: https://git.openjdk.org/jdk/pull/28485#discussion_r2566838574
PR Review Comment: https://git.openjdk.org/jdk/pull/28485#discussion_r2566837301
PR Review Comment: https://git.openjdk.org/jdk/pull/28485#discussion_r2566825506
