RFR: 8343177: JFR: Remove critical section for thread id assignment

[Markus Grönlund]

On Wed, 30 Oct 2024 05:28:28 GMT, David Holmes <dholmes 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
>
> src/hotspot/share/runtime/thread.hpp line 505:
> 
>> 503:   // Sets the argument thread as starting thread. Returns failure if thread
>> 504:   // creation fails due to lack of memory, too many threads etc.
>> 505:   static bool set_as_starting_thread(JavaThread* jt);
> 
> Why change this? Overall I don't understand the "starting thread" changes.

I changed this with the argument that it seemed very unbalanced to have a non-static instance method available on all JavaThreads for calling t->set_as_starting_thread(), for setting a static field, a static field which further is only set once, by the main thread during startup in Threads::create_VM().

The assert method is_starting_thread(Thread*t) is for asserting the special assignment of 1 to the primordial thread.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21756#discussion_r1822431357