RFR: 8316233: VirtualThreadStart events should not be thread-filtered
Serguei Spitsyn
sspitsyn at openjdk.org
Fri Oct 6 18:32:07 UTC 2023
On Wed, 4 Oct 2023 21:59:54 GMT, Leonid Mesnik <lmesnik at openjdk.org> wrote:
>> The JVMTI VirtualThreadStart events have to follow the ThreadStart events pattern and so, should not be thread-filtered.
>> The fix includes:
>> - `jvmti.xml`: remov the attribute `filtered="thread"` in the `VirtuallThreadStart` event spec
>> - `jvmtiEventController.cpp`: remove the `VTHREAD_START_BIT` from the `THREAD_FILTERED_EVENT_BITS` mask and and it to the `NEED_THREAD_LIFE_EVENTS` mask
>> - `jvmtiExport.cpp`: rearrangements in the `JvmtiExport::post_vthread_start()` function
>>
>> The fix also includes a couple of minor unification tweaks:
>> - to align `JvmtiExport::post_thread_end()` with `JvmtiExport::post_vthread_end()` which have a little bit more optimized check for the `JVMTI_PHASE_PRIMORDIAL`.
>> - to rename the local variable `cur_thread` as `thread` to follow the common pattern in `JvmtiExport::post_vthread_start()` and `JvmtiExport::post_vthread_end()`
>>
>> Testing: ran mach5 tiers 1-6. All tests are passed.
>
> src/hotspot/share/prims/jvmtiExport.cpp line 1552:
>
>> 1550: JvmtiEnvThreadStateIterator it(state);
>> 1551: for (JvmtiEnvThreadState* ets = it.first(); ets != nullptr; ets = it.next(ets)) {
>> 1552: JvmtiEnv *env = ets->get_env();
>
> This change as well as renaming cur_thread are not related to the main issue. It would be better to separate them. Easier to track and backport if needed. They are mentioned in PR but not in jira bug, hard to find the reason without GitHub. Might be better to copy them in the bug if you want to keep them.
Thanks. I agree with you in general.
The whole fix is relatively small, so I'd prefer to keep such a minor cleanup in this particular case.
I'll update the bug report with this info.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/16019#discussion_r1349184898
More information about the serviceability-dev
mailing list