RFR: 8352251: Implement JEP 518: JFR Cooperative Sampling [v22]

Patricio Chilano Mateo pchilanomate at openjdk.org
Wed May 14 15:04:03 UTC 2025


On Wed, 14 May 2025 14:25:35 GMT, Patricio Chilano Mateo <pchilanomate at openjdk.org> wrote:

>> Markus Grönlund has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   validate_bcp
>
> src/hotspot/share/jfr/periodic/sampling/jfrThreadSampler.cpp line 323:
> 
>> 321: // without thread suspension and CPU context retrieval,
>> 322: // if we carefully order the loads of the thread state.
>> 323: bool JfrSamplerThread::sample_native_thread(JavaThread* jt) {
> 
> This method now begs to be converted into a handshake, it’s pretty much the same logic. I wonder if you considered that already and found some issue. The handshake would also provide the necessary synchronization so there is no need for the extra sample_monitor and wait/notifies. I’m assuming you didn’t go for this because you would still need to add the extra separate flag check in `SafepointMechanism` for the sample in java case, and so you just kept both under it for consistency? But even the sample in java case could use handshakes, we would just need to install an async one (only executed by target) at the end after enqueueing the request. We would still need to keep the `Jfr::check_and_process_sample_request()` method separately but we can do that. I’ve been playing around with these changes: https://github.com/pchilano/jdk/commit/dacce47402c38a2b466c819d2cd6c643b0de09ba
> I think it is simpler to read and removes some of the complexity of the current approach. What do you think?

Also, for the sampling in native case, we could make it more cooperative by adding a handshake operation like the one for all threads but for a list of threads. That way we can try to distribute the work instead of going one by one (this can actually be done with the current code too with some changes).

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24296#discussion_r2089097185


More information about the hotspot-jfr-dev mailing list