RFR: 8343177: JFR: Remove critical section for thread id assignment [v2]
David Holmes
dholmes at openjdk.org
Thu Oct 31 05:12:29 UTC 2024
On Wed, 30 Oct 2024 13:14:42 GMT, Markus Grönlund <mgronlun at openjdk.org> wrote:
>> Greetings,
>>
>> With Loom, JFR had to change the assignment of unique thread IDs to threads. For JavaThreads, a dependency exists on the threadObj before the thread ID can be determined and assigned. We currently have a lazy assignment scheme that assigns on first use. Several threads, such as thread iterators and sampling threads, can issue the first use. Hence, a critical section protects assignments.
>>
>> However, a critical section at this location makes it challenging to build robust signal handlers, for example, because we cannot read the thread ID.
>>
>> We can remove this critical section with careful rearrangements, ensuring threads are only assigned a thread ID in thread state _thread_new (a thread state that at least the JFR sampler and the JFR iterators exclude).
>>
>> The problem child is JNI attaching threads, created directly into state _thread_in_vm and added to the threads list before the creation and assignment of the threadObj. We can also manage this case by being careful when we sample such a thread, alternatively allowing for a thread ID of 0 or taking account of the 'attaching via jni' property.
>>
>> Testing: jdk_jfr Tier 1- 3, Stress testing
>>
>> Thanks
>> Markus
>
> Markus Grönlund has updated the pull request incrementally with one additional commit since the last revision:
>
> can_assign
Overall looks good. Thanks for the clarifications.
I have one change request with regards to naming below.
Thanks
src/hotspot/share/jfr/jfr.cpp line 104:
> 102:
> 103: void Jfr::initialize_primordial_thread(JavaThread* jt) {
> 104: JfrThreadLocal::initialize_primordial_thread(jt);
The main thread is generally not the process primordial thread. Can we use main_thread or starting_thread here to keep semi-consistent with the Thread/Threads code terminology? Thanks
-------------
Changes requested by dholmes (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/21756#pullrequestreview-2406927627
PR Review Comment: https://git.openjdk.org/jdk/pull/21756#discussion_r1823777549
More information about the hotspot-jfr-dev
mailing list