On Tue, 3 Nov 2026 02:07:08 GMT, David Holmes <[email protected]> wrote:
>> [First](https://github.com/openjdk/jdk/blob/9b9863e792ac6ae424c4f829d30cb5db90f01d61/src/hotspot/share/runtime/thread.cpp#L230), >> shared between Java and non-Java threads, called in `_thread_new` state. >> [Second](https://github.com/openjdk/jdk/blob/9b9863e792ac6ae424c4f829d30cb5db90f01d61/src/hotspot/share/runtime/javaThread.cpp#L628), >> specific to Java threads, called in `_thread_in_vm` state. >> >> The second one was added in https://github.com/openjdk/jdk/pull/26558, and >> `JfrThreadLocal::on_start` got state-dependent logic. > >> [First](https://github.com/openjdk/jdk/blob/9b9863e792ac6ae424c4f829d30cb5db90f01d61/src/hotspot/share/runtime/thread.cpp#L230), >> shared between Java and non-Java threads, called in _thread_new state. > > Ouch! I had assumed/expected that was for NJT's only. I don't understand why > you would have the two calls either. > @dholmes-ora Good point. I also feel it a bit ugly to call > `Jfr::on_thread_start` twice per Java thread, but that's what > [JDK-8364258](https://github.com/openjdk/jdk/pull/26558) added irrespective > of CPU-time sampling, so I didn't really want to touch this code in my fix, > which is exclusive to CPU-time profiler. > > In fact, I don't see anything in `JfrThreadLocal::on_start` that relies on > `_thread_new` state, so, for Java threads, we can probably call `on_start` > just once from `JavaThread::run()` after switching to `_thread_in_vm`. > Initialization of JfrCPUTimeThreadSampling would be then unconditional. Such > a refactoring will require #33651 to be integrated first. > > @mgronlun Was there a good reason for adding the second > `Jfr::on_thread_start` call? Do you agree with refactoring of your original > fix instead of a local 4-lines change as in this PR? There are indeed good reasons for it: A public consumer must see the following invariant: _thread_in_vm -> JFR Thread Id assigned Please see https://bugs.openjdk.org/browse/JDK-8343177 ------------- PR Comment: https://git.openjdk.org/jdk/pull/33678#issuecomment-6603397586
